Messages in this thread | | | From | Nick Desaulniers <> | Date | Sat, 23 Dec 2017 22:26:01 -0500 | Subject | precedence bug in MAKE_PROCESS_CPUCLOCK macro? |
| |
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 */
does not.
I'm happy to send a patch with your suggestion.
| |