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

wasm2c tests fail on 32-bit due to assumption that pointers are 64-bit #2099

Closed
rathann opened this issue Dec 11, 2022 · 3 comments · Fixed by #2124
Closed

wasm2c tests fail on 32-bit due to assumption that pointers are 64-bit #2099

rathann opened this issue Dec 11, 2022 · 3 comments · Fixed by #2124

Comments

@rathann
Copy link
Contributor

rathann commented Dec 11, 2022

The spectest_make_externref() in test/spec-wasm2c-prefix.c:203 assumes that u64 (uint64_t) can be converted to wasm_rt_externref_t (void *). This is not the case on 32-bit platforms:

- test/regress/regress-2039.txt
  expected error code 0, got 1.
  STDERR MISMATCH:
  --- expected
  +++ actual
  @@ -0,0 +1,19 @@
  +out/test/regress/regress-2039/regress-2039-main.c: In function ‘spectest_make_externref’:
  +out/test/regress/regress-2039/regress-2039-main.c:205:10: error: cast to pointer from integer of different size [-Werror=int-to-pointer-cast]
  +  205 |   return (wasm_rt_externref_t)(x + 1);  // externref(0) is not null
  +      |          ^
  +At top level:
  +cc1: note: unrecognized command-line option ‘-Wno-tautological-constant-out-of-range-compare’ may have been intended to silence earlier diagnostics
  +cc1: note: unrecognized command-line option ‘-Wno-ignored-optimization-argument’ may have been intended to silence earlier diagnostics
  +cc1: all warnings being treated as errors
  +Traceback (most recent call last):
  +  File "/builddir/build/BUILD/wabt-1.0.31/test/run-spec-wasm2c.py", line 554, in <module>
  +    sys.exit(main(sys.argv[1:]))
  +  File "/builddir/build/BUILD/wabt-1.0.31/test/run-spec-wasm2c.py", line 536, in main
  +    o_filenames.append(Compile(cc, main_filename, out_dir, includes))
  +  File "/builddir/build/BUILD/wabt-1.0.31/test/run-spec-wasm2c.py", line 408, in Compile
  +    cc.RunWithArgsForStdout(*args)
  +  File "/builddir/build/BUILD/wabt-1.0.31/test/utils.py", line 88, in RunWithArgsForStdout
  +    raise error
  +utils.Error: Error running "gcc -I/builddir/build/BUILD/wabt-1.0.31/wasm2c -c out/test/regress/regress-2039/regress-2039-main.c -o out/test/regress/regress-2039/regress-2039-main.o -O2 -Wall -Werror -Wno-unused -Wno-ignored-optimization-argument -Wno-tautological-constant-out-of-range-compare -Wno-infinite-recursion -Wno-array-bounds -Wbidi-chars=none -fno-optimize-sibling-calls -frounding-math -fsignaling-nans -std=c99 -D_DEFAULT_SOURCE" (1):
  +None
  STDOUT MISMATCH:
  --- expected
  +++ actual
  @@ -1 +0,0 @@
  -0/0 tests passed.

The same error occurs with all wasm2c tests. This can be fixed with a trivial patch:

diff -up wabt-1.0.31/test/spec-wasm2c-prefix.c.32bit wabt-1.0.31/test/spec-wasm2c-prefix.c
--- wabt-1.0.31/test/spec-wasm2c-prefix.c.32bit 2022-11-14 05:43:40.000000000 +0100
+++ wabt-1.0.31/test/spec-wasm2c-prefix.c       2022-12-11 22:17:36.181299604 +0100
@@ -200,7 +200,7 @@ static bool is_equal_wasm_rt_funcref_t(w
          (x.module_instance == y.module_instance);
 }

-wasm_rt_externref_t spectest_make_externref(u64 x) {
+wasm_rt_externref_t spectest_make_externref(uintptr_t x) {
   return (wasm_rt_externref_t)(x + 1);  // externref(0) is not null
 }

Additionally, the example code in wasm2c/examples/fac/fac.c makes assumptions that memory addresses are 64-bit, too, for example in lines 71, 81, 97 and 106. The code should use uintptr_t instead in the above lines, too.

@keithw
Copy link
Member

keithw commented Dec 12, 2022

Thanks -- if you have cycles to submit this as a PR, happy to take it.

Ideally we would pass the tests on i386 in general and run them in CI. I think #1044 and #1973 are tracking this. My understanding was that the interpreter and wasm2c both currently have problems on i386.

@keithw
Copy link
Member

keithw commented Dec 12, 2022

Also btw if this is helpful: fac.c isn't example code itself; it's the C version of fac.wat after running it through wasm2c. (Rebuilt with make update-wasm2c-fac). The code is coming from the source in the src/template directory.

rathann added a commit to rathann/wabt that referenced this issue Jan 8, 2023
The spectest_make_externref() in test/spec-wasm2c-prefix.c:203 assumes
that u64 (uint64_t) can be converted to wasm_rt_externref_t (void *).
This is not the case on 32-bit platforms.

```
- test/regress/regress-2039.txt
  expected error code 0, got 1.
  STDERR MISMATCH:
  --- expected
  +++ actual
  @@ -0,0 +1,19 @@
  +out/test/regress/regress-2039/regress-2039-main.c: In function ‘spectest_make_externref’:
  +out/test/regress/regress-2039/regress-2039-main.c:205:10: error: cast to pointer from integer of different size [-Werror=int-to-pointer-cast]
  +  205 |   return (wasm_rt_externref_t)(x + 1);  // externref(0) is not null
  +      |          ^
```

The same error occurs with all wasm2c tests.

This fixes WebAssembly#2099.
@rathann
Copy link
Contributor Author

rathann commented Jan 8, 2023

@keithw I've submitted a PR with just the test failure fix for now. fac.c (or, actually, wasm2c.declarations.c) seems to have more pointer width issues than just the above.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants