Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

test failed on SPARC #60

Closed
tankf33der opened this issue Mar 5, 2019 · 2 comments
Closed

test failed on SPARC #60

tankf33der opened this issue Mar 5, 2019 · 2 comments
Labels
bug This is not expected behavior, and needs fixing

Comments

@tankf33der
Copy link

solaris, sparc

janet:1255:> (assert (= (length (table (/ 0 0) 2 1 3)) 1) "nan key table ctor")
 
✘  nan key table ctor
false

linux, x64

janet:110:> (assert (= (length (table (/ 0 0) 2 1 3)) 1) "nan key table ctor")
src/core/vm.c:340:5: runtime error: division by zero
SUMMARY: UndefinedBehaviorSanitizer: undefined-behavior src/core/vm.c:340:5 in
✔true

i want both sparc and linux return true on this test from suite0.janet

@bakpakin bakpakin added bug This is not expected behavior, and needs fixing help wanted labels Mar 5, 2019
@bakpakin
Copy link
Member

bakpakin commented Mar 5, 2019

The runtime error from UBSan can probably be ignored - we rely on (/ 0 0) == qNaN, and that should hold true on most architectures, and should for SPARC.

Janet uses the nanboxing technique internally, and relies on NaNs generated by floating point operations to be of a certain form that is different than the artificially created ones that are actually tagged pointers (quiet NaNs vs. signaling, but's it's really more about the specific bits in the signalling NaNs) . There are a bunch of fiddly macros in janet.h for dealing with this.

If I had to guess, the table has length 2, so src/core/table.c, line 135 is not properly guarding the
insertion of a NaN key in the table.

    if (janet_checktype(key, JANET_NUMBER) && isnan(janet_unwrap_number(key))) return;

Some things to try:

  • Compile again with -DJANET_NO_NANBOX. This should hopefully work.
  • Assert that 0.0 / 0.0 is really producing a NaN number type.

Sample diff:

diff --git a/Makefile b/Makefile
index e04662a..5b75117 100644
--- a/Makefile
+++ b/Makefile
@@ -35,6 +35,7 @@ MANPATH?=$(PREFIX)/share/man/man1/
 DEBUGGER=gdb
 
 CFLAGS=-std=c99 -Wall -Wextra -Isrc/include -fpic -O2 -fvisibility=hidden \
+          -DJANET_NO_NANBOX -fsanitize=undefined \
           -DJANET_BUILD=$(JANET_BUILD)
 LDFLAGS=-rdynamic
 
diff --git a/src/boot/system_test.c b/src/boot/system_test.c
index cf991f9..1b03a7d 100644
--- a/src/boot/system_test.c
+++ b/src/boot/system_test.c
@@ -48,5 +48,7 @@ int system_test() {
     assert(janet_equals(janet_cstringv("a string."), janet_cstringv("a string.")));
     assert(janet_equals(janet_csymbolv("sym"), janet_csymbolv("sym")));
 
+    assert(JANET_NUMBER == janet_type(janet_wrap_number(0.0 / 0.0)));
+
     return 0;
 }

@tankf33der
Copy link
Author

-DJANET_NO_NANBOX helped.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug This is not expected behavior, and needs fixing
Projects
None yet
Development

No branches or pull requests

2 participants