gbdev/rgbds

math.asm test failure with armv7a-unknown-linux-gnueabihf

orbea opened this issue · 6 comments

OS: Gentoo
rgbds: 0.7.0
Gentoo issue: https://bugs.gentoo.org/928268

I enabled the tests for the rgbds Gentoo ebuild.

src_test() {
	local dir
	for dir in asm link fix gfx; do
		pushd "test/${dir}" >/dev/null || die
		./test.sh || die
		popd >/dev/null || die
	done
}

And now an asm test failure for armv7a-unknown-linux-gnueabihf has been reported by a Gentoo tinderbox.

+error: math.asm(29):
+    Assertion failed
+error: Assembly aborted (1 error)!
math.err mismatch!

Unfortunately I do not have this hardware to test, any ideas how to solve this?

The Gentoo build log is available here.

It appears to be failing on this line.

Oh god, here come the floating-point nightmares. Can you add a println FMOD(-5.0, 0.0) line, and re-run? (The location within the file doesn't matter. It will be printed as another diff, so no need to change anything else either.)

As I stated I lack this hardware, but I asked the reporter if they can help test.

If not I can try to cross-compile it later...

The issue here is that the result of those operations is, respectively, infinity and NaN. Those values don't convert to integers cleanly. Some CPUs do it one way, some do it another way. Converting infinity or NaN to an integer in C is definitely UB and not portable in the slightest.

Since rgbasm just has fixed-point, I'd suggest converting Infinity to the maximum positive value, -Infinity to the maximum negative value, and NaN to 0 (same as how various other non-asm-time-computable expressions become 0).

It might be possible to reproduce this on x86_64 using ubsan with clang ( 18.1.2).

FLAGS='-O0 -g -fno-omit-frame-pointer -fsanitize=undefined' && make Q= CXXFLAGS="$FLAGS" LDFLAGS="$FLAGS" CXX=clang++
math...
--- /dev/null	2024-03-23 23:47:52.120956538 -0700
+++ /tmp/tmp.0Tczm9DMP3	2024-03-30 19:17:50.224177174 -0700
@@ -0,0 +1,2 @@
+src/asm/fixpoint.cpp:28:18: runtime error: -inf is outside the range of representable values of type 'int'
+SUMMARY: UndefinedBehaviorSanitizer: undefined-behavior src/asm/fixpoint.cpp:28:18 
math.err mismatch!

Honestly, I'd suggest a simple if (!isfinite(value)) value = 0; fix for all floats before being converted back to fixed — there's simply no reasonable representation for infinity or NaN, so you might as well kill the problem in one go.