Summary

integer overflow when running 6.0.0pre1 tests (2)

Metadata

Description

similarly to the previous ticket, i'm building chicken with the clang integer ub sanitizers, which also trap here:

 Can hold maximum value 9223372036854775807...
 Can hold minimum value -9223372036854775808...
 
 Error: illegal instruction
 
 	Call history:
 
 	compiler-tests.scm:352: g1093	  
 	compiler-tests.scm:352: chicken.base#print	  
 	compiler-tests.scm:352: scheme#call-with-current-continuation	  
 	compiler-tests.scm:352: chicken.condition#with-exception-handler	  
 	compiler-tests.scm:352: ##sys#call-with-values	  
 	compiler-tests.scm:352: k1108	  
 	compiler-tests.scm:352: g1111	  
 	compiler-tests.scm:352: chicken.base#print	  
 	compiler-tests.scm:352: scheme#call-with-current-continuation	  
 	compiler-tests.scm:352: chicken.condition#with-exception-handler	  
 	compiler-tests.scm:352: ##sys#call-with-values	  
 	compiler-tests.scm:352: k1126	  
 	compiler-tests.scm:352: g1129	  
 	compiler-tests.scm:353: chicken.base#print	  
 	compiler-tests.scm:353: chicken.base#print	  
 	compiler-tests.scm:353: chicken.base#print	  	<--
 make: *** [rules.make:1017: check] Error 70

this is right before the test code attempts to confirm the c variables can't take a value outside their bounds. i'm not sure where exactly the code that checks whether an integer is representable in a 64 bit c variable lives, but it seems to trigger the overflow before checking whether the operation is actually valid. if so, that would be ub and therefore unsound

Changes and comments

[2024-12-18 13:36:07 UTC] zerica wrote:

linking the full ubsan runtime yields more information:

 Can hold maximum value 9223372036854775807...
 Can hold minimum value -9223372036854775808...
 chicken.h:2759:39: runtime error: negation of -9223372036854775808 cannot be represented in type 'long'; cast to an unsigned type to negate this value to itself
 SUMMARY: UndefinedBehaviorSanitizer: undefined-behavior chicken.h:2759:39

so the ub seems to occur in this function

 inline static C_long C_num_to_long(C_word x)
 {
   if(x & C_FIXNUM_BIT) {
     return (C_long)C_unfix(x);
   } else {
     if (C_bignum_negativep(x)) return -(C_long)C_bignum_digits(x)[0];
     else return (C_long)C_bignum_digits(x)[0];
   }
 }

where `-(C_long)x` is unsound when x is INT64_MIN

[2024-12-18 13:36:07 UTC] zerica wrote:

1734525751769071

[2024-12-18 13:53:39 UTC] zerica wrote:

hm, no, i mightve been wrong here. this was logged before:

 chicken.h:2622:17: runtime error: implicit conversion from type 'unsigned long' of value 9223372036854775808 (64-bit, unsigned) to type 'int64_t' (aka 'long') changed the value to -9223372036854775808 (64-bit, signed)
 SUMMARY: UndefinedBehaviorSanitizer: undefined-behavior chicken.h:2622:17

which points to `C_s64 num = C_bignum_digits(x)[[0]|];` in

 inline static C_s64 C_num_to_int64(C_word x)
 {
   if(x & C_FIXNUM_BIT) {
     return (C_s64)C_unfix(x);
   } else {
     C_s64 num = C_bignum_digits(x)[0];
 #ifndef C_SIXTY_FOUR
     if (C_bignum_size(x) > 1) num |= (C_s64)(((C_u64)C_bignum_digits(x)[1]) << 32);
 #endif
     if (C_bignum_negativep(x)) return -num;
     else return num;
   }
 }

[2024-12-23 15:28:38 UTC] sjamaan wrote:

Could you remind me how one runs the tests with the sanitizer? I'd like to see if any fix would actually work.

[2025-01-08 00:49:00 UTC] zerica wrote:

i have `-fsanitize=signed-integer-overflow,integer-divide-by-zero -fsanitize-trap=signed-integer-overflow,integer-divide-by-zero -fno-sanitize-recover` in my env CFLAGS, which compiles in a SIGILL trigger on those kinds of ub

[2025-01-14 12:09:31 UTC] sjamaan wrote:

What C compiler are you using? My gcc doesn't accept these options, and when I use it with clang (16.0.6), the tests pass...

[2025-01-22 21:45:15 UTC] zerica wrote:

i'm using clang 19.1.7

[2025-01-23 11:12:42 UTC] sjamaan wrote:

OK, that helps, after installing clang 19 I'm getting an illegal instruction straight away on the first test, with those options you mention.

How can I get the full runtime (like you mentioned above) in order to get better diagnostics? I tried adding `-fsanitize=undefined`, but that just results in boatloads of linker errors like the following random snapshot from the output:

 c-backend.c:(.text+0xa78ec): undefined reference to `__ubsan_handle_function_type_mismatch_abort'
 /nix/store/vk4mlknqk9yjbqa68a7rvpfxfdw3rad7-binutils-2.43.1/bin/ld: user-pass.o: in function `C_user_2dpass_toplevel':
 user-pass.c:(.text+0xf5): undefined reference to `__ubsan_handle_builtin_unreachable'

This happens both on NixOS and Debian (after struggling for a while figuring out I needed `libclang-rt-19-dev` to get `libclang_rt.ubsan_standalone.a`). Is there some magic option I need to pass to make it work?

[2025-01-24 07:32:25 UTC] zerica wrote:

in my testing i usually rely on passing the standalone runtime manually at link time (so basically adding `/usr/lib/clang/19/lib/x86_64-chimera-linux-musl/libclang_rt.ubsan_standalone.a` to ldflags), if there's any shrinkwrapped flag i'm not aware of it...

[2025-01-28 14:06:51 UTC] sjamaan wrote:

Could you share the exact configure/make commands you're using? I can't make it work even when I put in the literal location of the standalone library.

[2025-01-30 11:59:36 UTC] zerica wrote:

working on an extracted `chicken-6.0.0pre1.tar.gz`, having only applied my patch in https://bugs.call-cc.org/ticket/1846:

 $ CC=clang CFLAGS="-fsanitize=signed-integer-overflow,integer-divide-by-zero" ./configure
 $ LINKER_OPTIONS="-fsanitize=signed-integer-overflow,integer-divide-by-zero" make check -j16

apparently it works if the same flag is passed to both the compiler and the linker! and for the record, here's the output of the compiler tests:

 '/tmp/chicken-6.0.0pre1/chicken' 'compiler-tests.scm' '-output-file' 'a.c' '-verbose' '-include-path' '/tmp/chicken-6.0.0pre1' '-consult-types-file' '../types.db' '-ignore-repository'
 
 Warning: (compiler-tests.scm:218) - assignment to imported value binding `foo'
 
 Warning: Wrong number of arguments
   In file `compiler-tests.scm:10',
   In procedure `foo',
   In procedure call:
 
     (bar 2)
 
   Procedure `bar' is called with 1 argument but 0 arguments are expected.
 
   Procedure `bar' has this type:
 
     (-> fixnum)
 
 Note: Test is always true
   At the toplevel,
   In conditional expression:
 
     (if g2242 (##sys#make-c-string (##sys#foreign-string-argument g2242)) #f)
 
   Test condition has always true value of type:
 
     string
 
 Note: Test is always false
   At the toplevel,
   In conditional expression:
 
     (if #f (##core#callunit repl) (##core#undefined))
 
   Test condition is always false.
 
 Note: Test is always true
   At the toplevel,
   In conditional expression:
 
     (if #t (##core#callunit repl) (##core#undefined))
 
   Test condition has always true value of type:
 
     true
 'clang' 'a.c' '-o' 'a.o' '-c' '-fno-strict-aliasing' '-fwrapv' '-DHAVE_CHICKEN_CONFIG_H' '-DC_ENABLE_PTABLES' '-fsanitize=signed-integer-overflow,integer-divide-by-zero' '-I/tmp/chicken-6.0.0pre1' '-I/usr/local/include/chicken'
 rm a.c
 'clang' 'a.o' '-o' 'a.out' '-fsanitize=signed-integer-overflow,integer-divide-by-zero' '-L/tmp/chicken-6.0.0pre1' '-Wl,-rpath=/tmp/chicken-6.0.0pre1' '-L/tmp/chicken-6.0.0pre1' '-Wl,-rpath=/usr/local/lib' '-l' 'chicken' '-lm' '-ldl'
 rm a.o
 12
 12
 12
 12
 12
 bar
 1 2 3 
 1 2 3 :1:2:3
 1 2 3 /tmp/chicken-6.0.0pre1/tests/../chicken.h:2626:39: runtime error: negation of -9223372036854775808 cannot be represented in type 'int64_t' (aka 'long'); cast to an unsigned type to negate this value to itself
 SUMMARY: UndefinedBehaviorSanitizer: undefined-behavior /tmp/chicken-6.0.0pre1/tests/../chicken.h:2626:39 
 /tmp/chicken-6.0.0pre1/tests/../chicken.h:2675:24: runtime error: negation of -9223372036854775808 cannot be represented in type 'int64_t' (aka 'long'); cast to an unsigned type to negate this value to itself
 SUMMARY: UndefinedBehaviorSanitizer: undefined-behavior /tmp/chicken-6.0.0pre1/tests/../chicken.h:2675:24 
 
 Good, unrepresentable C strings cause errors
 Testing unsigned FFI type "unsigned-integer32" (32 bits):
 Can hold maximum value 4294967295...
 Cannot hold one more than maximum value, 4294967296...
 Cannot hold -1 (any fixnum negative value)
 Cannot hold -2^64 (any bignum negative value < smallest int64)
 Testing signed FFI type "integer32" (32 bits):
 Can hold maximum value 2147483647...
 Can hold minimum value -2147483648...
 Cannot hold one more than maximum value 2147483648...
 Cannot hold one less than minimum value -2147483648...
 Testing unsigned FFI type "unsigned-integer64" (64 bits):
 Can hold maximum value 18446744073709551615...
 Cannot hold one more than maximum value, 18446744073709551616...
 Cannot hold -1 (any fixnum negative value)
 Cannot hold -2^64 (any bignum negative value < smallest int64)
 Testing signed FFI type "integer64" (64 bits):
 Can hold maximum value 9223372036854775807...
 Can hold minimum value -9223372036854775808...
 Cannot hold one more than maximum value 9223372036854775808...
 Cannot hold one less than minimum value -9223372036854775808...
 Testing unsigned FFI type "unsigned-integer" (32 bits):
 Can hold maximum value 4294967295...
 Cannot hold one more than maximum value, 4294967296...
 Cannot hold -1 (any fixnum negative value)
 Cannot hold -2^64 (any bignum negative value < smallest int64)
 Testing signed FFI type "integer" (32 bits):
 Can hold maximum value 2147483647...
 Can hold minimum value -2147483648...
 Cannot hold one more than maximum value 2147483648...
 Cannot hold one less than minimum value -2147483648...
 Testing signed FFI type "(enum intlimits)" (32 bits):
 Can hold maximum value 2147483647...
 Can hold minimum value -2147483648...
 Cannot hold one more than maximum value 2147483648...
 Cannot hold one less than minimum value -2147483648...
 Testing unsigned FFI type "unsigned-long" (64 bits):
 Can hold maximum value 18446744073709551615...
 Cannot hold one more than maximum value, 18446744073709551616...
 Cannot hold -1 (any fixnum negative value)
 Cannot hold -2^64 (any bignum negative value < smallest int64)
 Testing signed FFI type "/tmp/chicken-6.0.0pre1/tests/../chicken.h:2759:39: runtime error: negation of -9223372036854775808 cannot be represented in type 'long'; cast to an unsigned type to negate this value to itself
 SUMMARY: UndefinedBehaviorSanitizer: undefined-behavior /tmp/chicken-6.0.0pre1/tests/../chicken.h:2759:39 
 long" (64 bits):
 Can hold maximum value 9223372036854775807...
 Can hold minimum value -9223372036854775808...
 Cannot hold one more than maximum value 9223372036854775808...
 Cannot hold one less than minimum value -9223372036854775808...
 Testing signed FFI type "ssize_t" (64 bits):
 Can hold maximum value 9223372036854775807...
 Can hold minimum value -9223372036854775808...
 Cannot hold one more than maximum value 9223372036854775808...
 Cannot hold one less than minimum value -9223372036854775808...
 Testing unsigned FFI type "size_t" (64 bits):
 Can hold maximum value 18446744073709551615...
 Cannot hold one more than maximum value, 18446744073709551616...
 Cannot hold -1 (any fixnum negative value)
 Cannot hold -2^64 (any bignum negative value < smallest int64)
 2
 2

[2025-01-31 14:16:38 UTC] sjamaan wrote:

Thanks! With these options I can reproduce the error.

Note that you still need to use `UBSAN_OPTIONS=halt_on_error=1` to actually get the tests to fail (otherwise it just prints a notice and continues).

Having said all this, I'm now very much unsure whether this is even a valid bug. We rely heavily on `-fwrapv` to nail down signed overflow handling, and I believe that doubly applies in these cases.

After some searching, I found this gcc discussion, which points to this clang ubsanitizer PR. Even there, the folks involved are confused.

In any case, even from these discussions it's not entirely clear to me how `-fwrapv` and `-fsanitize=signed-integer-overflow` are supposed to interact. From my reading, they are somewhat mutually exclusive. But there's also some nuance in how the sanitizer behaves when `-fwrapv` is present. The patch that eventually landed even explicitly mentions that users of `-fwrapv` may want to explicitly use `-fno-sanitize=signed-integer-overflow` to disable noisy warnings....

[2025-01-31 14:40:02 UTC] zerica wrote:

> We rely heavily on `-fwrapv` to nail down signed overflow handling, and I believe that doubly applies in these cases.

ah, that's perfectly reasonable then! i guess i was expecting the sanitizers to take `-fwrapv` into account in a way they actually don't... either way, i'll take this as confirmation that these specifics sanitizers can be skipped :)

[2025-01-31 15:40:13 UTC] sjamaan changed status from new to closed

[2025-01-31 15:40:13 UTC] sjamaan set resolution to invalid

[2025-01-31 15:40:13 UTC] sjamaan wrote:

Replying to Erica Z: > ah, that's perfectly reasonable then! i guess i was expecting the sanitizers to take `-fwrapv` into account in a way they actually don't... either way, i'll take this as confirmation that these specifics sanitizers can be skipped :)

Thanks for confirming. And also thanks for your patience guiding me to get this sanitizer working in the first place! I'll close this and #1846 as invalid then.