#4113 Incorrect equality comparisons involving long long constants

[Christopher Bazley]

1: Sample code that reproduces the problem.

_Static_assert(0x10000LL != 0, "high bits");
_Static_assert(0x20000000000000ULL != 0x20000000000001ULL, "distinct values");
_Static_assert(65436U != -100LL, "unsigned value differs from negative value");

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

sdcc -mz80 --std-c11 -S longlong-equality.c

3: SDCC version tested (type "sdcc -v" to find it).

SDCC 4.6.4 #0 (Linux), built from SVN r16960 sources.

4: Copy of the error message or incorrect output, or a clear description of the observed versus expected behavior.

longlong-equality.c:1: warning 215: static assertion failed: "high bits"
longlong-equality.c:2: warning 215: static assertion failed: "distinct values"
longlong-equality.c:3: warning 215: static assertion failed: "unsigned value differs from negative value"

All three assertions are true. The first comparison must account for bit 16. The second compares two distinct integers above the range in which double can represent every integer exactly. In the third, 65436U converts to long long and remains positive, so it differs from -100LL.

valCompare can truncate equality comparisons involving long long constants to unsigned int. Separately, isAstEqual compares literal values after conversion to double, which can round distinct large integers to the same value.

The same failures occur on mcs51 (default, medium, large and stack-auto), ds390, z180, r2k, r4k, sm83, tlcs90, hc08, s08, stm8 and pdk13/14/15. They also occur with Z80 in C23 mode.

GCC 16.2.0 and Clang 23.1.0 accept the example without diagnostics: https://godbolt.org/z/nna4fKed9

[Christopher Bazley]

While working on [bugs:#3952], I noticed a workaround in stdckdint.h which avoids using _Generic in the way intended by its designer (to select a function before calling it), for performance reasons relating to feature request 880. In turn, that workaround required me to add casts that I would rather not have added because they undermine type safety.

My candidate implementation of feature request 880 exposes this bug in checked integer arithmetic, so I believe this bug needs to be fixed first. In any case, it is clearly behaviour that is incompatible with Clang and GCC.

[Christopher Bazley]

Please find a candidate patch to fix this bug attached.

[Philipp Klaus Krause]

• This adds to the code duplication in valCompare. How about the following (which should have the same semantics as your patch) instead?

    case EQ_OP:
    case NE_OP:
      bool equal;
      if (SPEC_NOUN (lval->type) == V_FLOAT || SPEC_NOUN (rval->type) == V_FLOAT ||
        SPEC_NOUN (lval->type) == V_FIXED16X16 || SPEC_NOUN (rval->type) == V_FIXED16X16)
        {
          equal = floatFromVal (lval) == floatFromVal (rval);
        }
      else
        {
          /* integrals: ignore signedness */
          TYPE_TARGET_ULONGLONG l, r;

          l = (TYPE_TARGET_ULONGLONG) ullFromVal (lval);
          r = (TYPE_TARGET_ULONGLONG) ullFromVal (rval);
          /* In order to correctly compare 'signed int' and 'unsigned int' it's
             necessary to strip them to 16 bit.
             Literals are reduced to their cheapest type, therefore left and
             right might have different types. It's necessary to find a
             common type: int (used for char too) or long */
          if (!IS_LONGLONG (lval->etype) && !IS_BITINT (lval->etype) && !IS_LONGLONG (rval->etype) && !IS_BITINT (rval->etype))
            {
              r = (TYPE_TARGET_ULONG) r;
              l = (TYPE_TARGET_ULONG) l;
            }
          if (!IS_LONG (lval->etype) && !IS_LONG (rval->etype) &&
              !IS_LONGLONG (lval->etype) && !IS_LONGLONG (rval->etype) &&
              !IS_BITINT (lval->etype) && !IS_BITINT (rval->etype))
            {
              r = (TYPE_TARGET_UINT) r;
              l = (TYPE_TARGET_UINT) l;
            }
          equal = l == r;
        }
      SPEC_CVAL (val->type).v_int = (ctype == EQ_OP) ? equal : !equal;
      break;

• As the comment in valCompare about missing long long support states, there probably is still a problem for relational operators (I guess when precision in the lower bits is lost as a value is converted to floating-point format).

• The fix (both yours and my deduplicated variant above) make a test fail:

results/ucz80/longlong/longlong_test_bit.out:16:--- FAIL: "Assertion failed" on (y & x) == (0x69aaaaaaaaaa55aaull & 0x69555555555555aall) at cases/longlong/longlong_test_bit.c:268

[Christopher Bazley]

Hi Philipp, please find a revised candidate patch attached.

It shares valCompare between AST and iCode constant folding for all six comparison operators, removing the duplicated equality handling and fixing relational comparisons. This significantly reduces the amount of code in SDCCicode.c.

Integer operands undergo integer promotion and common-type conversion before comparison, preserving wide values and signed ordering. The unsigned > 0 simplification now checks the full constant value.

The failing longlong_test_bit assertion exposed incorrect iCode bitwise constant folding. Using valBitwise for &, | and ^ avoids rounding operands through double or truncating them to 32 bits.

The added tests cover wide comparisons, mixed signedness, checked subtraction into a narrower result type, and agreement between constant-folded and runtime bitwise operations.

This fix is a prerequisite for the patch that implements [feature-requests:#880].

[Philipp Klaus Krause]

Most of this looks good to me. I do have a question on the changes in isAstEqual though: the changes clearly fix false positives for long long comparisons, but I think they introduce new false negatives for mixed sign. That might not cause real bugs, since looking at the uses of isAstEqual, I think false negatives just result in some missed optimizations. Still it feels like the false negatives should either be fixed, or clearly stated in a comment.