* [PATCH] selftests: timers: Fix valid-adjtimex signed left-shift undefined behavior
@ 2024-04-09 20:22 John Stultz
2024-04-10 19:13 ` Muhammad Usama Anjum
` (2 more replies)
0 siblings, 3 replies; 5+ messages in thread
From: John Stultz @ 2024-04-09 20:22 UTC (permalink / raw)
To: LKML
Cc: John Stultz, Thomas Gleixner, Stephen Boyd, Anna-Maria Behnsen,
Frederic Weisbecker, Shuah Khan, Nathan Chancellor,
Nick Desaulniers, Lee Jones, Muhammad Usama Anjum,
linux-kselftest
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 <tglx@linutronix.de>
Cc: Stephen Boyd <sboyd@kernel.org>
Cc: Anna-Maria Behnsen <anna-maria@linutronix.de>
Cc: Frederic Weisbecker <frederic@kernel.org>
Cc: Shuah Khan <shuah@kernel.org>
Cc: Nathan Chancellor <nathan@kernel.org>
Cc: Nick Desaulniers <ndesaulniers@google.com>
Cc: Lee Jones <joneslee@google.com>
Cc: Muhammad Usama Anjum <usama.anjum@collabora.com>
Cc: linux-kselftest@vger.kernel.org
Reported-by: Lee Jones <joneslee@google.com>
Reported-by: Muhammad Usama Anjum <usama.anjum@collabora.com>
Link: https://lore.kernel.org/lkml/0c6d4f0d-2064-4444-986b-1d1ed782135f@collabora.com/
Signed-off-by: John Stultz <jstultz@google.com>
---
.../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
^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [PATCH] selftests: timers: Fix valid-adjtimex signed left-shift undefined behavior
2024-04-09 20:22 [PATCH] selftests: timers: Fix valid-adjtimex signed left-shift undefined behavior John Stultz
@ 2024-04-10 19:13 ` Muhammad Usama Anjum
2024-04-10 20:14 ` [tip: timers/urgent] " tip-bot2 for John Stultz
2024-04-11 21:01 ` [PATCH] " Shuah Khan
2 siblings, 0 replies; 5+ messages in thread
From: Muhammad Usama Anjum @ 2024-04-10 19:13 UTC (permalink / raw)
To: John Stultz, LKML
Cc: Muhammad Usama Anjum, Thomas Gleixner, Stephen Boyd,
Anna-Maria Behnsen, Frederic Weisbecker, Shuah Khan,
Nathan Chancellor, Nick Desaulniers, Lee Jones, linux-kselftest
On 4/10/24 1:22 AM, 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 <tglx@linutronix.de>
> Cc: Stephen Boyd <sboyd@kernel.org>
> Cc: Anna-Maria Behnsen <anna-maria@linutronix.de>
> Cc: Frederic Weisbecker <frederic@kernel.org>
> Cc: Shuah Khan <shuah@kernel.org>
> Cc: Nathan Chancellor <nathan@kernel.org>
> Cc: Nick Desaulniers <ndesaulniers@google.com>
> Cc: Lee Jones <joneslee@google.com>
> Cc: Muhammad Usama Anjum <usama.anjum@collabora.com>
> Cc: linux-kselftest@vger.kernel.org
> Reported-by: Lee Jones <joneslee@google.com>
> Reported-by: Muhammad Usama Anjum <usama.anjum@collabora.com>
> Link: https://lore.kernel.org/lkml/0c6d4f0d-2064-4444-986b-1d1ed782135f@collabora.com/
> Signed-off-by: John Stultz <jstultz@google.com>
Reviewed-by: Muhammad Usama Anjum <usama.anjum@collabora.com>
> ---
> .../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)
--
BR,
Muhammad Usama Anjum
^ permalink raw reply [flat|nested] 5+ messages in thread
* [tip: timers/urgent] selftests: timers: Fix valid-adjtimex signed left-shift undefined behavior
2024-04-09 20:22 [PATCH] selftests: timers: Fix valid-adjtimex signed left-shift undefined behavior John Stultz
2024-04-10 19:13 ` Muhammad Usama Anjum
@ 2024-04-10 20:14 ` tip-bot2 for John Stultz
2024-04-11 21:01 ` [PATCH] " Shuah Khan
2 siblings, 0 replies; 5+ messages in thread
From: tip-bot2 for John Stultz @ 2024-04-10 20:14 UTC (permalink / raw)
To: linux-tip-commits
Cc: Lee Jones, Muhammad Usama Anjum, John Stultz, Thomas Gleixner,
x86, linux-kernel
The following commit has been merged into the timers/urgent branch of tip:
Commit-ID: 076361362122a6d8a4c45f172ced5576b2d4a50d
Gitweb: https://git.kernel.org/tip/076361362122a6d8a4c45f172ced5576b2d4a50d
Author: John Stultz <jstultz@google.com>
AuthorDate: Tue, 09 Apr 2024 13:22:12 -07:00
Committer: Thomas Gleixner <tglx@linutronix.de>
CommitterDate: Wed, 10 Apr 2024 22:07:42 +02:00
selftests: timers: Fix valid-adjtimex signed left-shift undefined behavior
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 trips 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,
~~~~^
..
Fix it by using a multiply by (1 << 16) instead of shifting negative values
in the valid-adjtimex test case. Align the values for better readability.
Reported-by: Lee Jones <joneslee@google.com>
Reported-by: Muhammad Usama Anjum <usama.anjum@collabora.com>
Signed-off-by: John Stultz <jstultz@google.com>
Signed-off-by: Thomas Gleixner <tglx@linutronix.de>
Reviewed-by: Muhammad Usama Anjum <usama.anjum@collabora.com>
Link: https://lore.kernel.org/r/20240409202222.2830476-1-jstultz@google.com
Link: https://lore.kernel.org/lkml/0c6d4f0d-2064-4444-986b-1d1ed782135f@collabora.com/
---
tools/testing/selftests/timers/valid-adjtimex.c | 73 +++++++---------
1 file changed, 36 insertions(+), 37 deletions(-)
diff --git a/tools/testing/selftests/timers/valid-adjtimex.c b/tools/testing/selftests/timers/valid-adjtimex.c
index 48b9a80..d13ebde 100644
--- a/tools/testing/selftests/timers/valid-adjtimex.c
+++ b/tools/testing/selftests/timers/valid-adjtimex.c
@@ -21,9 +21,6 @@
* MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
* GNU General Public License for more details.
*/
-
-
-
#include <stdio.h>
#include <stdlib.h>
#include <time.h>
@@ -62,45 +59,47 @@ 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)
^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [PATCH] selftests: timers: Fix valid-adjtimex signed left-shift undefined behavior
2024-04-09 20:22 [PATCH] selftests: timers: Fix valid-adjtimex signed left-shift undefined behavior John Stultz
2024-04-10 19:13 ` Muhammad Usama Anjum
2024-04-10 20:14 ` [tip: timers/urgent] " tip-bot2 for John Stultz
@ 2024-04-11 21:01 ` Shuah Khan
2024-04-12 10:01 ` Thomas Gleixner
2 siblings, 1 reply; 5+ messages in thread
From: Shuah Khan @ 2024-04-11 21:01 UTC (permalink / raw)
To: John Stultz, LKML
Cc: Thomas Gleixner, Stephen Boyd, Anna-Maria Behnsen,
Frederic Weisbecker, Shuah Khan, Nathan Chancellor,
Nick Desaulniers, Lee Jones, Muhammad Usama Anjum,
linux-kselftest, Shuah Khan
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 <tglx@linutronix.de>
> Cc: Stephen Boyd <sboyd@kernel.org>
> Cc: Anna-Maria Behnsen <anna-maria@linutronix.de>
> Cc: Frederic Weisbecker <frederic@kernel.org>
> Cc: Shuah Khan <shuah@kernel.org>
> Cc: Nathan Chancellor <nathan@kernel.org>
> Cc: Nick Desaulniers <ndesaulniers@google.com>
> Cc: Lee Jones <joneslee@google.com>
> Cc: Muhammad Usama Anjum <usama.anjum@collabora.com>
> Cc: linux-kselftest@vger.kernel.org
> Reported-by: Lee Jones <joneslee@google.com>
> Reported-by: Muhammad Usama Anjum <usama.anjum@collabora.com>
> Link: https://lore.kernel.org/lkml/0c6d4f0d-2064-4444-986b-1d1ed782135f@collabora.com/
> Signed-off-by: John Stultz <jstultz@google.com>
> ---
Applied to linux-kselftest next for Linux6.10-rc1.
thanks,
-- Shuah
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] selftests: timers: Fix valid-adjtimex signed left-shift undefined behavior
2024-04-11 21:01 ` [PATCH] " Shuah Khan
@ 2024-04-12 10:01 ` Thomas Gleixner
0 siblings, 0 replies; 5+ messages in thread
From: Thomas Gleixner @ 2024-04-12 10:01 UTC (permalink / raw)
To: Shuah Khan, John Stultz, LKML
Cc: Stephen Boyd, Anna-Maria Behnsen, Frederic Weisbecker, Shuah Khan,
Nathan Chancellor, Nick Desaulniers, Lee Jones,
Muhammad Usama Anjum, linux-kselftest, Shuah Khan
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
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2024-04-12 10:01 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2024-04-09 20:22 [PATCH] selftests: timers: Fix valid-adjtimex signed left-shift undefined behavior John Stultz
2024-04-10 19:13 ` Muhammad Usama Anjum
2024-04-10 20:14 ` [tip: timers/urgent] " tip-bot2 for John Stultz
2024-04-11 21:01 ` [PATCH] " Shuah Khan
2024-04-12 10:01 ` Thomas Gleixner
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox