#4127 Integer literals beyond unsigned long long range go undiagnosed

[Christopher Bazley]

1: Sample code that reproduces the problem.

unsigned long long object = 18446744073709551616ULL;

2: Exact command used to run SDCC on this sample code.

sdcc -mz80 --std=c2y -S bug.c

3: SDCC version tested.

SDCC : mcs51/z80/z180/r2k/r2ka/r3ka/r4k/r5k/r6k/sm83/tlcs90/ez80/z80n/r800/ds390/TININative/ds400/hc08/s08/stm8/pdk13/pdk14/pdk15 TD- 4.6.4 #0 (Linux)
published under GNU General Public License (GPL)

Built from clean official SVN r16978 sources on Linux.

4: Observed and expected behaviour.

SDCC exits successfully and emits no diagnostic. A diagnostic is required by
WG14 N3886,
6.4.5.1 paragraph 2:

Each constant shall have a type and the value of a constant shall be in the range of representable
values for its type.

Every constant value must be representable in its type. SDCC silently accepts the value 2^64 written as decimal, hexadecimal or binary with ULL suffix. The implementation provides a 64-bit unsigned long long and advertises __SDCC_BITINT_MAXWIDTH as 64.

5: Additional checks.

GCC 13.3.0, invoked with -std=c2x -pedantic-errors -fsyntax-only, rejects every invalid variant.

Configurations tested: MCS51 default/large/medium/stack-auto, DS390, Z80, Z180,
Rabbit 2000/4000, SM83, TLCS-90, HC08, S08, STM8 default/large, PDK13/14/15,
and eZ80. These are compiler and diagnostic checks, not runtime tests.

Implementation observation: The decimal example is emitted as eight zero bytes in Z80 assembly. sepStrToUll in src/SDCCval.c accumulates digits with unsigned multiplication and addition without overflow checks, allowing wraparound before later type selection. This is a literal-reading failure, not the integer-expression overflow reported in bug [#4093].

[Christopher Bazley]

Here's a side-by-side comparison of SDCC trunk with GCC 16.2: https://godbolt.org/z/1Kh5dnjEE

[Christopher Bazley]

Please consider the attached candidate bug fix patch.

[Philipp Klaus Krause]

I guess the patch is ok. It feels a bit inelegant to hardcode the 64 bit long long, but we probably have that in too many other places already (such as the hardcoded TYPE_TARGET_ULONGLONG), so this won't hurt further. Still might want to throw a comment or wassert in there to clearly state that assumption (vs. LONGLONGSIZE).

[Maarten Brock]

What are you testing here? That the preprocessor can correctly replace the macro? That two identical literals are indeed identical? If the compiler messes up reading the first literal it probably messes up reading the second literal in exactly the same way.

  ASSERT (18446744073709551615ULL == ULLONG_MAX);
  ASSERT (0xffffffffffffffffULL == ULLONG_MAX);
  ASSERT (01777777777777777777777ULL == ULLONG_MAX);
  ASSERT (9223372036854775807LL == LLONG_MAX);
  ASSERT (18446744073709551614ULL == ULLONG_MAX - 1ULL);
  ASSERT (00000000000000000000000000000000000000000000000001ULL == 1ULL);

[Christopher Bazley]

Thanks for your review comments, @maartenbrock and @spth.

To answer your questions, @maartenbrock:

What are you testing here?

That "Valid boundary values must retain all 64 bits when read".

That the preprocessor can correctly replace the macro? That two identical literals are indeed identical?

No, although I can see why that might appear to be the case. As you correctly identified, the test was flawed because it assumed that the <limits.h> macros would produce the valid boundary values in some mysterious way independent of literal parsing.

The revised patch addresses both issues: it uses TYPE_TARGET_ULONGLONG and derives its maximum from that type, with a wassert checking agreement with LONGLONGSIZE. The regression tests use volatile objects and arithmetic checks (of unsigned integer type wraparound) instead of comparing literals with limits macros containing equivalent literals.

All 80 diagnostic checks passed across eight target configurations; all six runtime assertions passed on Z80 and MCS51-large.

[Philipp Klaus Krause]

Looks good to me. Personally, I'd add a test for binary along the decimal, octal and hex ones, but even without that the patch is ok.

[Christopher Bazley]

Fixed by [r17020].