2024-04-09 20:22:37

by John Stultz

[permalink] [raw]
Subject: [PATCH] selftests: timers: Fix valid-adjtimex signed left-shift undefined behavior

So, the struct adjtimex freq field takes a signed value who's
units are in shifted (<<16) parts-per-million.

Unfortunately for negative adjustments, the straightforward use
of:
freq = ppm<<16
will trip undefined behavior warnings with clang:

valid-adjtimex.c:66:6: warning: shifting a negative signed value is undefined [-Wshift-negative-value]
-499<<16,
~~~~^
valid-adjtimex.c:67:6: warning: shifting a negative signed value is undefined [-Wshift-negative-value]
-450<<16,
~~~~^
..

So fix our use of shifting negative values in the valid-adjtimex
test case to use multiply by (1<<16) to avoid this.

The patch also aligns the values a bit to make it look nicer.

Cc: Thomas Gleixner <[email protected]>
Cc: Stephen Boyd <[email protected]>
Cc: Anna-Maria Behnsen <[email protected]>
Cc: Frederic Weisbecker <[email protected]>
Cc: Shuah Khan <[email protected]>
Cc: Nathan Chancellor <[email protected]>
Cc: Nick Desaulniers <[email protected]>
Cc: Lee Jones <[email protected]>
Cc: Muhammad Usama Anjum <[email protected]>
Cc: [email protected]
Reported-by: Lee Jones <[email protected]>
Reported-by: Muhammad Usama Anjum <[email protected]>
Link: https://lore.kernel.org/lkml/[email protected]/
Signed-off-by: John Stultz <[email protected]>
---
.../testing/selftests/timers/valid-adjtimex.c | 69 ++++++++++---------
1 file changed, 35 insertions(+), 34 deletions(-)

diff --git a/tools/testing/selftests/timers/valid-adjtimex.c b/tools/testing/selftests/timers/valid-adjtimex.c
index 48b9a803235a..9606d45767e7 100644
--- a/tools/testing/selftests/timers/valid-adjtimex.c
+++ b/tools/testing/selftests/timers/valid-adjtimex.c
@@ -62,45 +62,46 @@ int clear_time_state(void)
#define NUM_FREQ_OUTOFRANGE 4
#define NUM_FREQ_INVALID 2

+#define SHIFTED_PPM (1 << 16)
long valid_freq[NUM_FREQ_VALID] = {
- -499<<16,
- -450<<16,
- -400<<16,
- -350<<16,
- -300<<16,
- -250<<16,
- -200<<16,
- -150<<16,
- -100<<16,
- -75<<16,
- -50<<16,
- -25<<16,
- -10<<16,
- -5<<16,
- -1<<16,
+ -499 * SHIFTED_PPM,
+ -450 * SHIFTED_PPM,
+ -400 * SHIFTED_PPM,
+ -350 * SHIFTED_PPM,
+ -300 * SHIFTED_PPM,
+ -250 * SHIFTED_PPM,
+ -200 * SHIFTED_PPM,
+ -150 * SHIFTED_PPM,
+ -100 * SHIFTED_PPM,
+ -75 * SHIFTED_PPM,
+ -50 * SHIFTED_PPM,
+ -25 * SHIFTED_PPM,
+ -10 * SHIFTED_PPM,
+ -5 * SHIFTED_PPM,
+ -1 * SHIFTED_PPM,
-1000,
- 1<<16,
- 5<<16,
- 10<<16,
- 25<<16,
- 50<<16,
- 75<<16,
- 100<<16,
- 150<<16,
- 200<<16,
- 250<<16,
- 300<<16,
- 350<<16,
- 400<<16,
- 450<<16,
- 499<<16,
+ 1 * SHIFTED_PPM,
+ 5 * SHIFTED_PPM,
+ 10 * SHIFTED_PPM,
+ 25 * SHIFTED_PPM,
+ 50 * SHIFTED_PPM,
+ 75 * SHIFTED_PPM,
+ 100 * SHIFTED_PPM,
+ 150 * SHIFTED_PPM,
+ 200 * SHIFTED_PPM,
+ 250 * SHIFTED_PPM,
+ 300 * SHIFTED_PPM,
+ 350 * SHIFTED_PPM,
+ 400 * SHIFTED_PPM,
+ 450 * SHIFTED_PPM,
+ 499 * SHIFTED_PPM,
};

long outofrange_freq[NUM_FREQ_OUTOFRANGE] = {
- -1000<<16,
- -550<<16,
- 550<<16,
- 1000<<16,
+ -1000 * SHIFTED_PPM,
+ -550 * SHIFTED_PPM,
+ 550 * SHIFTED_PPM,
+ 1000 * SHIFTED_PPM,
};

#define LONG_MAX (~0UL>>1)
--
2.44.0.478.gd926399ef9-goog



2024-04-11 21:03:30

by Shuah Khan

[permalink] [raw]
Subject: Re: [PATCH] selftests: timers: Fix valid-adjtimex signed left-shift undefined behavior

On 4/9/24 14:22, John Stultz wrote:
> So, the struct adjtimex freq field takes a signed value who's
> units are in shifted (<<16) parts-per-million.
>
> Unfortunately for negative adjustments, the straightforward use
> of:
> freq = ppm<<16
> will trip undefined behavior warnings with clang:
>
> valid-adjtimex.c:66:6: warning: shifting a negative signed value is undefined [-Wshift-negative-value]
> -499<<16,
> ~~~~^
> valid-adjtimex.c:67:6: warning: shifting a negative signed value is undefined [-Wshift-negative-value]
> -450<<16,
> ~~~~^
> ...
>
> So fix our use of shifting negative values in the valid-adjtimex
> test case to use multiply by (1<<16) to avoid this.
>
> The patch also aligns the values a bit to make it look nicer.
>
> Cc: Thomas Gleixner <[email protected]>
> Cc: Stephen Boyd <[email protected]>
> Cc: Anna-Maria Behnsen <[email protected]>
> Cc: Frederic Weisbecker <[email protected]>
> Cc: Shuah Khan <[email protected]>
> Cc: Nathan Chancellor <[email protected]>
> Cc: Nick Desaulniers <[email protected]>
> Cc: Lee Jones <[email protected]>
> Cc: Muhammad Usama Anjum <[email protected]>
> Cc: [email protected]
> Reported-by: Lee Jones <[email protected]>
> Reported-by: Muhammad Usama Anjum <[email protected]>
> Link: https://lore.kernel.org/lkml/[email protected]/
> Signed-off-by: John Stultz <[email protected]>
> ---

Applied to linux-kselftest next for Linux6.10-rc1.

thanks,
-- Shuah


2024-04-12 10:14:09

by Thomas Gleixner

[permalink] [raw]
Subject: Re: [PATCH] selftests: timers: Fix valid-adjtimex signed left-shift undefined behavior

Shuah!

On Thu, Apr 11 2024 at 15:01, Shuah Khan wrote:
>
> Applied to linux-kselftest next for Linux6.10-rc1.

I took this already through my tree as I have more timer selftest
related stuff pending and coming up soon along with actual kernel
changes.

Thanks,

tglx