lkml.org 
[lkml]   [2017]   [Dec]   [28]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
From
SubjectRe: precedence bug in MAKE_PROCESS_CPUCLOCK macro?
On Sat, 23 Dec 2017, Nick Desaulniers wrote:
> I'm seeing the following warning compiling with Clang:
>
> kernel/time/posix-cpu-timers.c:1397:29: warning: shifting a negative
> signed value is undefined
> [-Wshift-negative-value]
> return posix_cpu_clock_get(THREAD_CLOCK, tp);
> ^~~~~~~~~~~~
> kernel/time/posix-cpu-timers.c:1367:22: note: expanded from macro 'THREAD_CLOCK'
> #define THREAD_CLOCK MAKE_THREAD_CPUCLOCK(0, CPUCLOCK_SCHED)
> ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> ./include/linux/posix-timers.h:48:2: note: expanded from macro
> 'MAKE_THREAD_CPUCLOCK'
> MAKE_PROCESS_CPUCLOCK((tid), (clock) | CPUCLOCK_PERTHREAD_MASK)
> ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> ./include/linux/posix-timers.h:46:23: note: expanded from macro
> 'MAKE_PROCESS_CPUCLOCK'
> ((~(clockid_t) (pid) << 3) | (clockid_t) (clock))
> ~~~~~~~~~~~~~~~~~~ ^
>
> If I understand C's operator precedence rules
> (http://en.cppreference.com/w/c/language/operator_precedence)
> correctly, then I suspect the problem is in the sub-expression:
>
> (~(clockid_t) (pid) << 3)
>
> where pid (an argument to the macro) is first cast to a clockid_t (aka
> [signed] int), then negated, then shifted by 3 (oops, undefined
> behavior).
>
> Should the result after negation be cast to an unsigned int, or should
> the left shift happen before negation?
>
> CPUCLOCK_PID and CLOCKID_TO_FD seem to shift then negate, but
> FD_TO_CLOCKID seems to have the same issue as MAKE_PROCESS_CPUCLOCK.
>
> Changing the sub-expression to:
>
> (~(clockid_t) ((pid) << 3))
>
> changes what it evaluates to. Changing it to:
>
> (~(unsigned) (pid) << 3)
>
> or
>
> ((unsigned) ~(clockid_t) (pid) << 3)
>
> or
>
> (((unsigned) ~(clockid_t) (pid)) << 3) /* ugly */

All of these are butt ugly. And the same problem exists for FD_TO_CLOCKID.

My preference would be to replace all these crappy macros with simple
inline functions.

Thanks,

tglx

\
 
 \ /
  Last update: 2017-12-28 16:50    [W:0.068 / U:0.276 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site