* Namhyung Kim [off-list ref] wrote:

The %pT format specifier is for stack backtrace. Its handler
sprint_trace() does symbol lookup using (address-1) to ensure
the address will not point outside of the function.

If there is a tail-call to the function marked "noreturn",
gcc optimized out the code after the call then causes saved
return address points outside of the function (i.e. the start
of the next function), so pollutes call trace somewhat.
This patch will fix it.

before:
[   18.345923] Call Trace:
[   18.346001]  [<ffffffff812a8502>] panic+0x8c/0x18d
[   18.346257]  [<ffffffffa000012a>] deep01+0x0/0x38 [test_panic]  <--- bad
[   18.346347]  [<ffffffff81104666>] proc_file_write+0x73/0x8d
[   18.346432]  [<ffffffff811000b3>] proc_reg_write+0x8d/0xac
[   18.346516]  [<ffffffff810c7d32>] vfs_write+0xa1/0xc5
[   18.346603]  [<ffffffff810c7e0f>] sys_write+0x45/0x6c
[   18.346801]  [<ffffffff8f02943b>] system_call_fastpath+0x16/0x1b

after:
[   22.224483] Call Trace:
[   22.224569]  [<ffffffff812bce69>] panic+0x8c/0x18d
[   22.224848]  [<ffffffffa000012a>] panic_write+0x20/0x20 [test_panic]  <--- ok
[   22.224979]  [<ffffffff81115fab>] proc_file_write+0x73/0x8d
[   22.225089]  [<ffffffff81111a5f>] proc_reg_write+0x8d/0xac
[   22.225199]  [<ffffffff810d90ee>] vfs_write+0xa1/0xc5
[   22.225304]  [<ffffffff810d91cb>] sys_write+0x45/0x6c
[   22.225408]  [<ffffffff812c07fb>] system_call_fastpath+0x16/0x1b

Ok, this looks really useful - we really want 100% perfect backtraces, kernel 
developers are looking at hundreds of thousands of call traces per year, so 
every little detail helps in the long run!

[ Nit: please omit the timestamp prefixes from the changelog, they add no 
  information and just clutter the git log. ]

The implementation could be done a bit cleaner:

+/**
+ * sprint_trace - Look up a kernel trace symbol and return it in a text buffer
+ * @buffer: buffer to be stored
+ * @address: address to lookup
+ *
+ * This function is for stack trace and does the same thing as sprint_symbol()
+ * but with modified/decreased @address. If there is a tail-call to the
+ * function marked "noreturn", gcc optimized out code after the call so that
+ * the stack-saved return address could point outside of the caller. This
+ * function ensures that kallsyms will find the original caller by decreasing
+ * @address and then adjusts the result by increasing offset.
+ *
+ * This function returns the number of bytes stored in @buffer.
+ */
+int sprint_trace(char *buffer, unsigned long address)
+{
+	char *modname;
+	const char *name;
+	unsigned long offset, size;
+	int len;
+
+	name = kallsyms_lookup(address-1, &size, &offset, &modname, buffer);
+	if (!name)
+		return sprintf(buffer, "0x%lx", address);
+
+	if (name != buffer)
+		strcpy(buffer, name);
+	len = strlen(buffer);
+	buffer += len;
+	offset++;
+
+	if (modname)
+		len += sprintf(buffer, "+%#lx/%#lx [%s]",
+						offset, size, modname);

[ Nit: please do not break the line there, it makes the code less readable. 
  (ignore checkpatch in this case) ]

+	else
+		len += sprintf(buffer, "+%#lx/%#lx", offset, size);
+
+	return len;
+}

This is really just a trivial variant of sprint_symbol() AFAICS, to make 
function return lookups more reliable. You look up address-1 then fix up the 
resulting offset.

I'd suggest to introduce __sprint_symbol() internal helper function, with a 
'symbol_offset' parameter to it.

That way sprint_symbol() can be implemented as a __sprint_symbol(..., 0) call, 
while sprint_trace() can be implemented as a __sprint_symbol(.., -1) call.

Also, while at it, please rename sprint_trace() to something better. This is 
not about tracing per se, this is about *backtraces* - and in particular this 
is about return addresses to noreturn functions pointing outside the kallsyms 
symbol of that function.

So a better name would be sprint_backtrace() or so?

[ Nit: 'B' might be a better abbreviation for 'backtrace'. ]

	Ingo