On Wed, Sep 04, 2024 at 01:26:11PM +0300, Andy Shevchenko wrote: > On Tue, Sep 03, 2024 at 09:52:01PM -0700, Josh Poimboeuf wrote: > > I'm not sure I buy that, we should look closer to understand what the > > issue is. Can you share the config and/or toolchain version(s) need to > > trigger the warning? > > .config is from the original report [1], toolchain is > Debian clang version 18.1.8 (9) > Target: x86_64-pc-linux-gnu > Thread model: posix > InstalledDir: /usr/bin > > (Just whatever Debian unstable provides) > > [1]: https://lore.kernel.org/oe-kbuild-all/202408290219.BrPO8twi-lkp@xxxxxxxxx/ The warning is due to a (minor?) Clang bug, almost like it tried to optimize but didn't quite finish. Here's the disassembly: 0000000000000010 <fan_mode_show>: 10: e8 00 00 00 00 call 15 <fan_mode_show+0x5> 11: R_X86_64_PLT32 __fentry__-0x4 15: 55 push %rbp 16: 48 89 e5 mov %rsp,%rbp 19: 41 57 push %r15 1b: 41 56 push %r14 1d: 53 push %rbx 1e: 50 push %rax 1f: 48 89 d3 mov %rdx,%rbx 22: 49 89 fe mov %rdi,%r14 25: e8 00 00 00 00 call 2a <fan_mode_show+0x1a> 26: R_X86_64_PLT32 __sanitizer_cov_trace_pc-0x4 2a: 4d 8b 76 78 mov 0x78(%r14),%r14 2e: 31 f6 xor %esi,%esi 30: 49 8d 7e 08 lea 0x8(%r14),%rdi 34: e8 00 00 00 00 call 39 <fan_mode_show+0x29> 35: R_X86_64_PLT32 mutex_lock_nested-0x4 39: 4d 89 f7 mov %r14,%r15 3c: 49 83 c7 08 add $0x8,%r15 40: 74 5b je 9d <fan_mode_show+0x8d> 42: 49 8b 06 mov (%r14),%rax 45: 48 8d 55 e0 lea -0x20(%rbp),%rdx 49: be 2b 00 00 00 mov $0x2b,%esi 4e: 48 8b 78 08 mov 0x8(%rax),%rdi 52: e8 00 00 00 00 call 57 <fan_mode_show+0x47> 53: R_X86_64_PLT32 .text.read_ec_data-0x4 57: 4c 89 ff mov %r15,%rdi 5a: 41 89 c6 mov %eax,%r14d 5d: e8 00 00 00 00 call 62 <fan_mode_show+0x52> 5e: R_X86_64_PLT32 mutex_unlock-0x4 62: 45 85 f6 test %r14d,%r14d 65: 74 07 je 6e <fan_mode_show+0x5e> 67: e8 00 00 00 00 call 6c <fan_mode_show+0x5c> 68: R_X86_64_PLT32 __sanitizer_cov_trace_pc-0x4 6c: eb 1b jmp 89 <fan_mode_show+0x79> 6e: e8 00 00 00 00 call 73 <fan_mode_show+0x63> 6f: R_X86_64_PLT32 __sanitizer_cov_trace_pc-0x4 73: 48 8b 55 e0 mov -0x20(%rbp),%rdx 77: 48 89 df mov %rbx,%rdi 7a: 48 c7 c6 00 00 00 00 mov $0x0,%rsi 7d: R_X86_64_32S .rodata.str1.1+0x508 81: e8 00 00 00 00 call 86 <fan_mode_show+0x76> 82: R_X86_64_PLT32 sysfs_emit-0x4 86: 41 89 c6 mov %eax,%r14d 89: 49 63 c6 movslq %r14d,%rax 8c: 48 83 c4 08 add $0x8,%rsp 90: 5b pop %rbx 91: 41 5e pop %r14 93: 41 5f pop %r15 95: 5d pop %rbp 96: 31 ff xor %edi,%edi 98: 31 d2 xor %edx,%edx 9a: 31 f6 xor %esi,%esi 9c: c3 ret 9d: e8 00 00 00 00 call a2 <__param_ctrl_ps2_aux_port+0x2> 9e: R_X86_64_PLT32 __sanitizer_cov_trace_pc-0x4 <end of function> And the interesting bit: 30: 49 8d 7e 08 lea 0x8(%r14),%rdi # rdi = &priv->vpc_mutex 34: e8 00 00 00 00 call mutex_lock_nested 39: 4d 89 f7 mov %r14,%r15 # r15 = r14 = priv 3c: 49 83 c7 08 add $0x8,%r15 # r15 = &priv->vpc_mutex 40: 74 5b je 9d <fan_mode_show+0x8d> # if &priv->vpc_mutex == NULL, goto 9d ... 9d: e8 00 00 00 00 call a2 <__param_ctrl_ps2_aux_port+0x2> 9e: R_X86_64_PLT32 __sanitizer_cov_trace_pc-0x4 <oof> If '&priv->vpc_mutex' is NULL, it jumps to 9d, where it calls __sanitizer_cov_trace_pc(). After that returns, it runs off the rails. Apparently Clang decided somehow (LTO?) that '&priv->vpc_mutex' can never be NULL, but it didn't quite finish the optimization. Maybe some bad interaction between LTO and KCOV? Here's the triggering code: > static ssize_t fan_mode_show(struct device *dev, > struct device_attribute *attr, > char *buf) > { > struct ideapad_private *priv = dev_get_drvdata(dev); > unsigned long result; > int err; > > scoped_guard(mutex, &priv->vpc_mutex) > err = read_ec_data(priv->adev->handle, VPCCMD_R_FAN, &result); Here's the pre-processed code for the scoped_guard() invocation and its DEFINE_GUARD() dependency, edited for readability: > typedef struct mutex * class_mutex_t; > > static inline void class_mutex_destructor(struct mutex **p) > { > struct mutex *_T = *p; > if (_T) > mutex_unlock(_T); > } > > static inline struct mutex *class_mutex_constructor(struct mutex *_T) > { > struct mutex *t = ({ mutex_lock_nested(_T, 0); _T; }); > return t; > } > > static inline void *class_mutex_lock_ptr(struct mutex **_T) > { > return *_T; > } > > for (struct mutex *scope = class_mutex_constructor(&priv->vpc_mutex), *done = ((void *)0); > class_mutex_lock_ptr(&scope) && !done; > done = (void *)1) { > > err = read_ec_data(priv->adev->handle, VPCCMD_R_FAN, &result); > } The fake "for loop" is needed to be able to initialize the scope variable inline. But basically it's doing this: > if (class_mutex_constructor(&priv->vpc_mutex)) > err = read_ec_data(priv->adev->handle, VPCCMD_R_FAN, &result); In this case, mutex is an unconditional guard, so the constructor just returns the original value of '&priv->vpc_mutex'. So if the original '&priv->vpc_mutex' is never NULL, the condition would always be true. -- Josh
