Closed atrosinenko closed 11 months ago
With some clumsy and possibly incorrectly rewritten compiler command line (because plain -fsanitize=undefined
does not work for such setup due to -nodefaultlibs
, etc.) I got the following UBSan diagnostics:
/old_ssd/ast/msp430-projects/llvm-project/compiler-rt/test/builtins/Unit/extendhfsf2_test.c:24:30: runtime error: nan is outside the range of representable values of type 'unsigned short'
SUMMARY: UndefinedBehaviorSanitizer: undefined-behavior /old_ssd/ast/msp430-projects/llvm-project/compiler-rt/test/builtins/Unit/extendhfsf2_test.c:24:30 in
/old_ssd/ast/msp430-projects/llvm-project/compiler-rt/test/builtins/Unit/extendhfsf2_test.c:24:33: runtime error: nan is outside the range of representable values of type 'unsigned short'
SUMMARY: UndefinedBehaviorSanitizer: undefined-behavior /old_ssd/ast/msp430-projects/llvm-project/compiler-rt/test/builtins/Unit/extendhfsf2_test.c:24:33 in
error in test__extendhfsf2(0xc248) = -3.140625, expected -3.141593
This might be an ubsan fault after all :) Or something within the pipeline that has some similar weird requirements.
This particular diagnostic message can surely be ubsan fault because the compiler command line was very hackish and "best-effort". But the fact everything "works correctly" when adding more debug output seems very suspicious. On the other hand, technically, this can be some controlled UB usage by the compiler developers. :)
Another question is why two float
arguments are passed for comparison to compareResultH(uint16_t result, uint16_t expected)
? As I understand, uint16_t
here is a substitution for some "IEEE754 type with half-precision" but actual arguments are traditional single-precision float
s. As I understand, the compiler tries to convert them to integral type according to their numerical value while the expected behavior here is probably reinterpret_cast.
Looking at other such tests:
https://github.com/access-softek/llvm-project/blob/40ae542656c264727049f14eb629de08d2cee09a/compiler-rt/test/builtins/Unit/extendsfdf2vfp_test.c#L26-L31
Here double
s are just compared with !=
.
https://github.com/access-softek/llvm-project/blob/40ae542656c264727049f14eb629de08d2cee09a/compiler-rt/test/builtins/Unit/extendsftf2_test.c#L26-L34
Here long double
is checked against its binary representation (represented by two uint64_t
values) by compareResultLD
.
In the test__extendhfsf2
compareResultF
was probably expected.
Most probably here're we're dealing with uint16_t being a storage-only representation of half precision FPs on ARM. This is storage-only type, it is implicitly converted to / from float on every operation per specification.
I would simply disable hfsf builtin for us, it does not make any sense.
The
extendhfsf2_test.c
unit test looks suspicious. When debugging optimization-related test failures on MSP430, I have finally managed to make all tests either pass or not, not depending on the optimization level. Exceptextendhfsf2_test
. It passed with-O0
,-O1
,-Os
,-Oz
but not with-O2
and higher. After a bit of debugging, turned out that thetest__extendhfsf2
function was inlined.How to reproduce
Compile the compiler-rt builtins with
Everything is OK. Then apply the following patch:
and add
-O2
to the compiler parameters intest/builtins/Unit/X86_64LinuxConfig/Output/extendhfsf2_test.c.script
.While these changes are not expected to change the behavior, now running
extendhfsf2_test.c.script
prints:For me, it is the call to
compareResultH
seems suspicious because if expects twouint16_t
arguments but twofloat
s are passed.If I add the following patch in addition to forcefully inlining:
it just prints lots of
OK
and exits successfully.