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
2022-04-06T03:43:57Z

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@infradead.org>
> 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@intel.com>
> Signed-off-by: Peter Zijlstra (Intel) <peterz@infradead.org>

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@redhat.com>
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@intel.com>
Signed-off-by: Josh Poimboeuf <jpoimboe@redhat.com>
---
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