Re: drivers/gpu/drm/i915/i915.prelink.o: warning: objtool: __intel_wait_for_register_fw.cold()+0xce: relocation to !ENDBR: vlv_allow_gt_wake.cold+0x0

[Josh Poimboeuf]

On Tue, Apr 05, 2022 at 04:01:15PM +0200, Peter Zijlstra wrote:
> Subject: objtool/ibt: Allow _THIS_IP_ at: sym+len
> From: Peter Zijlstra <peterz@xxxxxxxxxxxxx>
> Date: Tue Apr  5 15:54:41 CEST 2022
> 
> 0day robot reported:
> 
>   drivers/gpu/drm/i915/i915.prelink.o: warning: objtool: __intel_wait_for_register_fw.cold()+0xce: relocation to !ENDBR: vlv_allow_gt_wake.cold+0x0
> 
> Which turns out to be GCC placing a _THIS_IP_ past the end of the
> function:
> 
> 0000000000001d00 <__intel_wait_for_register_fw.cold>:
>     ...
>     1dce:       48 c7 c7 00 00 00 00    mov    $0x0,%rdi        1dd1: R_X86_64_32S      .text.unlikely+0x1df8
>     1dd5:       e8 00 00 00 00          call   1dda <__intel_wait_for_register_fw.cold+0xda>    1dd6: R_X86_64_PLT32    __trace_bprintk-0x4
>     ...
>     1df6:       0f 0b                   ud2
> 
> Add an exception for this one weird case...
> 
> Reported-by: kernel test robot <lkp@xxxxxxxxx>
> Signed-off-by: Peter Zijlstra (Intel) <peterz@xxxxxxxxxxxxx>

But objtool is complaining about a real problem (albeit with a cryptic
warning).  I don't think we want to paper over that. See patch.

Also, are in-tree users of trace_printk() even allowed??

From: Josh Poimboeuf <jpoimboe@xxxxxxxxxx>
Subject: [PATCH] tracing: Fix _THIS_IP_ usage in trace_printk()

do_trace_printk() uses the _THIS_IP_ macro to save the current
instruction pointer as an argument to a called function.  However,
because _THIS_IP_ relies on an empty label hack to get the IP, the
compiler is actually free to place the label anywhere in the function,
including at the very end -- which, since the label doesn't actually
have any code, is technically at the beginning of whatever function
happens to come next.

For example:

      1d89:	48 c7 c7 00 00 00 00 	mov    $0x0,%rdi
  			1d8c: R_X86_64_32S	.text.unlikely+0x1d3a
      1d90:	e8 00 00 00 00       	callq  1d95 <__intel_wait_for_register_fw.cold+0xd4>
  			1d91: R_X86_64_PLT32	__trace_bprintk-0x4
      1d95:	e8 00 00 00 00       	callq  1d9a <__intel_wait_for_register_fw.cold+0xd9>
  			1d96: R_X86_64_PLT32	__sanitizer_cov_trace_pc-0x4
      1d9a:	bf 01 00 00 00       	mov    $0x1,%edi
      1d9f:	e8 00 00 00 00       	callq  1da4 <__intel_wait_for_register_fw.cold+0xe3>
  			1da0: R_X86_64_PLT32	ftrace_dump-0x4
      1da4:	31 f6                	xor    %esi,%esi
      1da6:	bf 09 00 00 00       	mov    $0x9,%edi
      1dab:	e8 00 00 00 00       	callq  1db0 <__intel_wait_for_register_fw.cold+0xef>
  			1dac: R_X86_64_PLT32	add_taint-0x4
      1db0:	90                   	nop
      1db1:	0f 0b                	ud2

  0000000000001db3 <vlv_allow_gt_wake.cold>:

In this case _THIS_IP_ causes the instruction at 0x1d89 to reference the
next function.  This results in a semi-cryptic objtool warning:

  warning: objtool: __intel_wait_for_register_fw.cold()+0xce: relocation to !ENDBR: vlv_allow_gt_wake.cold+0x

While _THIS_IP_ is inherently imprecise, we can at least coddle the
compiler into putting the label *before* the call by using _THIS_IP_
immediately before the call instead of as an argument to the call.

Reported-by: kernel test robot <lkp@xxxxxxxxx>
Signed-off-by: Josh Poimboeuf <jpoimboe@xxxxxxxxxx>
---
 include/linux/kernel.h | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/include/linux/kernel.h b/include/linux/kernel.h
index 08ba5995aa8b..c399b29840eb 100644
--- a/include/linux/kernel.h
+++ b/include/linux/kernel.h
@@ -390,13 +390,15 @@ do {									\
 	static const char *trace_printk_fmt __used			\
 		__section("__trace_printk_fmt") =			\
 		__builtin_constant_p(fmt) ? fmt : NULL;			\
+	unsigned long __ip;						\
 									\
 	__trace_printk_check_format(fmt, ##args);			\
 									\
+	__ip = _THIS_IP_;						\
 	if (__builtin_constant_p(fmt))					\
-		__trace_bprintk(_THIS_IP_, trace_printk_fmt, ##args);	\
+		__trace_bprintk(__ip, trace_printk_fmt, ##args);	\
 	else								\
-		__trace_printk(_THIS_IP_, fmt, ##args);			\
+		__trace_printk(__ip, fmt, ##args);			\
 } while (0)
 
 extern __printf(2, 3)
-- 
2.34.1