Skip to content

fix(math): signed overflow in lcm() and SIGFPE when both inputs are zero - #1596

Open
newmathphys wants to merge 1 commit into
TheAlgorithms:masterfrom
newmathphys:fix/lcm-overflow-and-divzero
Open

newmathphys wants to merge 1 commit into
TheAlgorithms:masterfrom
newmathphys:fix/lcm-overflow-and-divzero

Conversation

@newmathphys

Copy link
Copy Markdown

math/lcm.c has two defects in the same expression:

int lcm(int a, int b) { return (a * b) / gcd(a, b); }

1. The intermediate product overflows even when the answer does not

(a * b) is computed before the division. For a = b = 50000 the true lcm(50000, 50000) is 50000 - it fits in int with room to spare (about 40 000 times over) - but the intermediate product is 2500000000, which exceeds INT_MAX (2147483647). In C11 that is undefined behaviour, and the program prints a wrong answer:

$ gcc -w -O0 lib.c -o lcm_plain && ./lcm_plain 50000 50000
lcm(50000, 50000) = -35899          # should be 50000

$ gcc -w -O0 -fsanitize=undefined lib.c -o lcm_ubsan && ./lcm_ubsan 50000 50000
lib.c:19:35: runtime error: signed integer overflow: 50000 * 50000 cannot be
             represented in type 'int'
lcm(50000, 50000) = -35899

The file's own documented test cases (15,20 -> 60 and 12,18 -> 36) both stay far below sqrt(INT_MAX) ~ 46341, which is why this went unnoticed.

2. SIGFPE when both inputs are zero

gcd(0, 0) returns 0 (the a == 0 branch returns b), so the divisor is zero:

$ ./lcm_plain 0 0
Floating point exception (core dumped)   # exit code 136

lcm(0, 0) is conventionally 0.

The fix

int lcm(int a, int b)
{
    if (a == 0 || b == 0)
        return 0;

    return (a / gcd(a, b)) * b;
}
  • Divide before multiplying. gcd(a, b) divides a exactly, so a / gcd(a, b) is exact and the intermediate value never exceeds the true LCM.
  • Guard the zero case so gcd(0, 0) == 0 cannot become a divisor.

Verification after the patch

$ gcc -w -O0 -fsanitize=undefined lib_fixed.c -o lcm_fixed_bin
$ ./lcm_fixed_bin 50000 50000
lcm(50000, 50000) = 50000          # correct, no sanitizer diagnostic
$ ./lcm_fixed_bin 0 0
lcm(0, 0) = 0                      # no SIGFPE
$ ./lcm_fixed_bin 15 20
lcm(15, 20) = 60                   # unchanged
$ ./lcm_fixed_bin 12 18
lcm(12, 18) = 36                   # unchanged

Scope

The change is local to lcm(). gcd() is untouched. If lcm(a, b) itself does not fit in int, the result still cannot be represented - that is inherent in the int return type and is not claimed to be fixed here.


Found by stating a contract for the arithmetic and letting a solver refute it, then confirming with gcc.

`int lcm(int a, int b) { return (a * b) / gcd(a, b); }` has two defects in the
same expression.

1. The intermediate product overflows even when the answer does not.
   For a = b = 50000 the true lcm is 50000 - it fits in int with ~40 000x
   headroom - but `a * b` is 2500000000, which exceeds INT_MAX (2147483647).
   That is UB in C11, and the program prints a wrong answer:

     $ ./lcm_plain 50000 50000
     lcm(50000, 50000) = -35899          # should be 50000
     $ ./lcm_ubsan 50000 50000
     runtime error: signed integer overflow: 50000 * 50000 cannot be
     represented in type 'int'

   The file's own test cases (15,20 -> 60 and 12,18 -> 36) both stay far below
   sqrt(INT_MAX) ~ 46341, which is why this went unnoticed.

2. SIGFPE when both inputs are zero: gcd(0, 0) returns 0 (the `a == 0` branch
   returns b), so the divisor is zero.

     $ ./lcm_plain 0 0
     Floating point exception (core dumped)   # exit code 136

Fix: a zero guard plus DIVIDE BEFORE MULTIPLYING, `(a / gcd(a, b)) * b`.
gcd(a, b) divides a exactly, so the intermediate value never exceeds the true LCM.

Verified after the patch (built with -fsanitize=undefined):
  50000,50000 -> 50000      0,0 -> 0
  15,20 -> 60 (unchanged)   12,18 -> 36 (unchanged)
  no sanitizer diagnostic on any of the four inputs.

gcd() is untouched; the change is local to lcm(). If lcm(a, b) itself does not fit
in int the result is still unrepresentable - that follows from the return type.
newmathphys pushed a commit to newmathphys/kern that referenced this pull request Oct 26, 2026
Opened TheAlgorithms/C#1596 from the fork
newmathphys/C, branch fix/lcm-overflow-and-divzero based on upstream
master@e5dad3f. One file, +13 -1, mergeable: True.

The patch was applied to a FRESH upstream/master (git apply --check clean) and
the fix was re-verified with a -fsanitize=undefined build ON THE PATCHED
REPOSITORY FILE:
  50000,50000 -> 50000      0,0 -> 0
  15,20 -> 60 (unchanged)   12,18 -> 36 (unchanged)
  no sanitizer diagnostic on any of the four inputs.

Recorded the PR number in docs/pr/thealgorithms_c_lcm_PR.md (now marked
SUBMITTED), docs/EXAMPLE_WHY_KERN.md, README.md, README.ru.md and both defect
journals (#364).

This is the third upstream report: #1595 (catalan.c, C), #1075 (Rust), #1596
(lcm.c, C).

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant