Do an explicit WARN_ONCE(!divisor) instead of hoping the 'undefined
behaviour' the compiler generates for a compile-time 1/0 is in any
way useful.

Return 0 (rather than ~(u64)0) because it is less likely to cause
further serious issues.

Add WARN_ONCE() in the divide overflow path.

A new change for v2 of the patchset.
Whereas gcc inserts (IIRC) 'ud2' clang is likely to let the code
continue and generate 'random' results for any 'undefined behaviour'.

v3: Use WARN_ONCE() and return 0 instead of BUG_ON().
    Explicitely #include <linux/bug.h>

Signed-off-by: David Laight <david.laight.linux@gmail.com>
---
 lib/math/div64.c | 25 ++++++++++++++-----------
 1 file changed, 14 insertions(+), 11 deletions(-)

diff --git a/lib/math/div64.c b/lib/math/div64.c
index a5c966a36836..397578dc9a0b 100644
--- a/lib/math/div64.c
+++ b/lib/math/div64.c
@@ -19,6 +19,7 @@
  */
 
 #include <linux/bitops.h>
+#include <linux/bug.h>
 #include <linux/export.h>
 #include <linux/math.h>
 #include <linux/math64.h>
@@ -186,6 +187,15 @@ EXPORT_SYMBOL(iter_div_u64_rem);
 #ifndef mul_u64_u64_div_u64
 u64 mul_u64_u64_div_u64(u64 a, u64 b, u64 d)
 {
+	if (WARN_ONCE(!d, "%s: division of (%#llx * %#llx) by zero, returning 0",
+		      __func__, a, b)) {
+		/*
+		 * Return 0 (rather than ~(u64)0) because it is less likely to
+		 * have unexpected side effects.
+		 */
+		return 0;
+	}
+
 	if (ilog2(a) + ilog2(b) <= 62)
 		return div64_u64(a * b, d);
 
@@ -212,12 +222,10 @@ u64 mul_u64_u64_div_u64(u64 a, u64 b, u64 d)
 
 #endif
 
-	/* make sure d is not zero, trigger exception otherwise */
-#pragma GCC diagnostic push
-#pragma GCC diagnostic ignored "-Wdiv-by-zero"
-	if (unlikely(d == 0))
-		return 1/0;
-#pragma GCC diagnostic pop
+	if (WARN_ONCE(n_hi >= d,
+		      "%s: division of (%#llx * %#llx = %#llx%016llx) by %#llx overflows, returning ~0",
+		      __func__, a, b, n_hi, n_lo, d))
+		return ~(u64)0;
 
 	int shift = __builtin_ctzll(d);
 
@@ -233,11 +241,6 @@ u64 mul_u64_u64_div_u64(u64 a, u64 b, u64 d)
 		 */
 	}
 
-	if (n_hi >= d) {
-		/* overflow: result is unrepresentable in a u64 */
-		return -1;
-	}
-
 	/* Do the full 128 by 64 bits division */
 
 	shift = __builtin_clzll(d);
-- 
2.39.5

On Sat, 14 Jun 2025, David Laight wrote:

> Do an explicit WARN_ONCE(!divisor) instead of hoping the 'undefined
> behaviour' the compiler generates for a compile-time 1/0 is in any
> way useful.
> 
> Return 0 (rather than ~(u64)0) because it is less likely to cause
> further serious issues.

I still disagree with this patch. Whether or not what the compiler 
produces is useful is beside the point. What's important here is to have 
a coherent behavior across all division flavors and what's proposed here 
is not.

Arguably, a compile time 1/0 might not be what we want either. The 
compiler forces an "illegal instruction" exception when what we want is 
a "floating point" exception (strange to have floating point exceptions 
for integer divisions but that's what it is).

So I'd suggest the following instead:

----- >8
From Nicolas Pitre <npitre@baylibre.com>
Subject: [PATCH] mul_u64_u64_div_u64(): improve division-by-zero handling

Forcing 1/0 at compile time makes the compiler (on x86 at least) to emit 
an undefined instruction to trigger the exception. But that's not what 
we want. Modify the code so that an actual runtime div-by-0 exception
is triggered to be coherent with the behavior of all the other division
flavors.

And don't use 1 for the dividend as the compiler would convert the 
actual division into a simple compare.

Signed-off-by: Nicolas Pitre <npitre@baylibre.com>

diff --git a/lib/math/div64.c b/lib/math/div64.c
index 5faa29208bdb..e6839b40e271 100644
--- a/lib/math/div64.c
+++ b/lib/math/div64.c
@@ -212,12 +212,12 @@ u64 mul_u64_u64_div_u64(u64 a, u64 b, u64 c)
 
 #endif
 
-	/* make sure c is not zero, trigger exception otherwise */
-#pragma GCC diagnostic push
-#pragma GCC diagnostic ignored "-Wdiv-by-zero"
-	if (unlikely(c == 0))
-		return 1/0;
-#pragma GCC diagnostic pop
+	/* make sure c is not zero, trigger runtime exception otherwise */
+	if (unlikely(c == 0)) {
+		unsigned long zero = 0;
+		asm ("" : "+r" (zero)); /* hide actual value from the compiler */
+		return ~0UL/zero;
+	}
 
 	int shift = __builtin_ctzll(c);

On Sat, 14 Jun 2025 11:17:33 -0400 (EDT)
Nicolas Pitre <nico@fluxnic.net> wrote:

> On Sat, 14 Jun 2025, David Laight wrote:
> 
> > Do an explicit WARN_ONCE(!divisor) instead of hoping the 'undefined
> > behaviour' the compiler generates for a compile-time 1/0 is in any
> > way useful.
> > 
> > Return 0 (rather than ~(u64)0) because it is less likely to cause
> > further serious issues.  
> 
> I still disagree with this patch. Whether or not what the compiler 
> produces is useful is beside the point. What's important here is to have 
> a coherent behavior across all division flavors and what's proposed here 
> is not.
> 
> Arguably, a compile time 1/0 might not be what we want either. The 
> compiler forces an "illegal instruction" exception when what we want is 
> a "floating point" exception (strange to have floating point exceptions 
> for integer divisions but that's what it is).
> 
> So I'd suggest the following instead:
> 
> ----- >8  
> From Nicolas Pitre <npitre@baylibre.com>
> Subject: [PATCH] mul_u64_u64_div_u64(): improve division-by-zero handling
> 
> Forcing 1/0 at compile time makes the compiler (on x86 at least) to emit 
> an undefined instruction to trigger the exception. But that's not what 
> we want. Modify the code so that an actual runtime div-by-0 exception
> is triggered to be coherent with the behavior of all the other division
> flavors.
> 
> And don't use 1 for the dividend as the compiler would convert the 
> actual division into a simple compare.

The alternative would be BUG() or BUG_ON() - but Linus really doesn't
like those unless there is no alternative.

I'm pretty sure that both divide overflow (quotient too large) and
divide by zero are 'Undefined behaviour' in C.
Unless the compiler detects and does something 'strange' it becomes
cpu architecture defined.
It is actually a right PITA that many cpu trap for overflow
and/or divide by zero (x86 traps for both, m68k traps for divide by
zero but sets the overflow flag for overflow (with unchanged outputs),
can't find my arm book, sparc doesn't have divide).

	David

> 
> Signed-off-by: Nicolas Pitre <npitre@baylibre.com>
> 
> diff --git a/lib/math/div64.c b/lib/math/div64.c
> index 5faa29208bdb..e6839b40e271 100644
> --- a/lib/math/div64.c
> +++ b/lib/math/div64.c
> @@ -212,12 +212,12 @@ u64 mul_u64_u64_div_u64(u64 a, u64 b, u64 c)
>  
>  #endif
>  
> -	/* make sure c is not zero, trigger exception otherwise */
> -#pragma GCC diagnostic push
> -#pragma GCC diagnostic ignored "-Wdiv-by-zero"
> -	if (unlikely(c == 0))
> -		return 1/0;
> -#pragma GCC diagnostic pop
> +	/* make sure c is not zero, trigger runtime exception otherwise */
> +	if (unlikely(c == 0)) {
> +		unsigned long zero = 0;
> +		asm ("" : "+r" (zero)); /* hide actual value from the compiler */
> +		return ~0UL/zero;
> +	}
>  
>  	int shift = __builtin_ctzll(c);
>

On Sat, 14 Jun 2025, David Laight wrote:

> On Sat, 14 Jun 2025 11:17:33 -0400 (EDT)
> Nicolas Pitre <nico@fluxnic.net> wrote:
> 
> > On Sat, 14 Jun 2025, David Laight wrote:
> > 
> > > Do an explicit WARN_ONCE(!divisor) instead of hoping the 'undefined
> > > behaviour' the compiler generates for a compile-time 1/0 is in any
> > > way useful.
> > > 
> > > Return 0 (rather than ~(u64)0) because it is less likely to cause
> > > further serious issues.  
> > 
> > I still disagree with this patch. Whether or not what the compiler 
> > produces is useful is beside the point. What's important here is to have 
> > a coherent behavior across all division flavors and what's proposed here 
> > is not.
> > 
> > Arguably, a compile time 1/0 might not be what we want either. The 
> > compiler forces an "illegal instruction" exception when what we want is 
> > a "floating point" exception (strange to have floating point exceptions 
> > for integer divisions but that's what it is).
> > 
> > So I'd suggest the following instead:
> > 
> > ----- >8  
> > From Nicolas Pitre <npitre@baylibre.com>
> > Subject: [PATCH] mul_u64_u64_div_u64(): improve division-by-zero handling
> > 
> > Forcing 1/0 at compile time makes the compiler (on x86 at least) to emit 
> > an undefined instruction to trigger the exception. But that's not what 
> > we want. Modify the code so that an actual runtime div-by-0 exception
> > is triggered to be coherent with the behavior of all the other division
> > flavors.
> > 
> > And don't use 1 for the dividend as the compiler would convert the 
> > actual division into a simple compare.
> 
> The alternative would be BUG() or BUG_ON() - but Linus really doesn't
> like those unless there is no alternative.
> 
> I'm pretty sure that both divide overflow (quotient too large) and
> divide by zero are 'Undefined behaviour' in C.
> Unless the compiler detects and does something 'strange' it becomes
> cpu architecture defined.

Exactly. Let each architecture produce what people expect of them when a 
zero divisor is encountered.

> It is actually a right PITA that many cpu trap for overflow
> and/or divide by zero (x86 traps for both, m68k traps for divide by
> zero but sets the overflow flag for overflow (with unchanged outputs),
> can't find my arm book, sparc doesn't have divide).

Some ARMs don't have divide either. But the software lib the compiler is 
relying on in those cases deals with a zero divisor already. We should 
involve the same path here.


Nicolas