Skip to content

[Repo Assist] fix: l_strtoul/l_strtoull return ULONG_MAX on negative overflow#161

Draft
github-actions[bot] wants to merge 1 commit intomainfrom
repo-assist/fix-strtoul-overflow-neg-2026-04-30-clean-2b43a28c045ffc27
Draft

[Repo Assist] fix: l_strtoul/l_strtoull return ULONG_MAX on negative overflow#161
github-actions[bot] wants to merge 1 commit intomainfrom
repo-assist/fix-strtoul-overflow-neg-2026-04-30-clean-2b43a28c045ffc27

Conversation

@github-actions
Copy link
Copy Markdown
Contributor

🤖 This is an automated pull request from Repo Assist.

Problem

l_strtoul and l_strtoull return the wrong value when both the overflow and negative sign conditions fire simultaneously — e.g.:

l_strtoul("-99999999999999999999999", NULL, 10)  // was: 1  should be: ULONG_MAX

C99 §7.20.1.4 requires these functions to return ULONG_MAX (or ULLONG_MAX) whenever the correct mathematical value cannot be represented, regardless of the sign of the input.

Root cause

When overflow is detected, the accumulator is clamped to ULONG_MAX. Then the return path applied a signed negation:

return neg ? (unsigned long)(-(long)acc) : acc;
```

`(long)ULONG_MAX` is implementation-defined (signed overflow). On typical two's-complement 64-bit hosts, GCC produces `(long)ULONG_MAX = -1`, so `-(long)(-1) = 1`, which is returned instead of `ULONG_MAX`.

The non-overflow negative path (e.g. `"-1" → ULONG_MAX`) was correct because `-(long)1 = -1` and `(unsigned long)(-1) = ULONG_MAX`. Only the overflow + neg combination was broken.

## Fix

Return `ULONG_MAX`/`ULLONG_MAX` immediately when the overflow flag is set, before the sign negation. Also replace the signed intermediate `-(long)acc` with unsigned subtraction `(0UL - acc)` to avoid implementation-defined behaviour on the non-overflow path.

## Test Status

All 9 test binaries pass on Linux x86-64. New regression tests added:

```
[OK] strtoul neg overflow -> ULONG_MAX   (strtoul("-99999999999999999999999") == ULONG_MAX)
[OK] strtoull neg overflow -> ULLONG_MAX (strtoull("-99999999999999999999999") == ULLONG_MAX)

Generated by 🌈 Repo Assist at {run-started}. Learn more.

To install this agentic workflow, run

gh aw add githubnext/agentics/workflows/repo-assist.md@1f672aef974f4246124860fc532f82fe8a93a57e

C99 §7.20.1.4 requires strtoul/strtoull to return ULONG_MAX/ULLONG_MAX
when the converted value cannot be represented (i.e. on overflow),
regardless of the sign of the input.

Previously, when both the overflow flag and the neg flag were set
(e.g. strtoul("-99999999999999999999999", NULL, 10)), the code applied
a signed negation to the overflow sentinel ULONG_MAX:
  (unsigned long)(-(long)ULONG_MAX)
On a typical two's-complement 64-bit host this yields 1, not ULONG_MAX.

Fix: return ULONG_MAX/ULLONG_MAX immediately when overflow is detected,
before the sign negation. Also replace the signed intermediate cast
with unsigned subtraction (0UL - acc) to avoid implementation-defined
behaviour on the non-overflow negative path.

Add regression tests:
  strtoul("-99999999999999999999999")  -> ULONG_MAX
  strtoull("-99999999999999999999999") -> ULLONG_MAX

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

0 participants