leaningtech / cheerp-meta

Cheerp - a C/C++ compiler for Web applications - compiles to WebAssembly and JavaScript
https://labs.leaningtech.com/cheerp
Other
1.02k stars 50 forks source link

Preparing parameters without type casting ? #114

Closed zyz9740 closed 2 years ago

zyz9740 commented 2 years ago

We found an issue related to type casting when we tried to convert a C program to a web assembly. Source code and reproduce script is in the attachment. This program does a complex computing and computes a hash value in the end. There are two faults in the compiled wasm.

  1. When I try to print the value of g_552 twice in line 573 and 574, it prints two different value. The first value is 65531, the second value is 131067.
  2. The computed hash value is wrong (The correct hash value should be 0x40963639, which can be gotten by executing C program directly)

I have try to see the text format(wat) of the wasm, and I found the probably cause as follows. It may help :

In the first fault, when preparing the params of printf (func 12), it masking the higher 16 bits by expression ( local_3 & 65535 ) , due to the fact that g_552 is a 16-bit number. However, in the second preparation, it directly stores the local_3 to local_5 without masking the higher 16 bits, leading to error. ( line 8332 to 8359 )

In the second fault, the value of g_552 is stored in the gloabl_5, it also extend the 32-bit interger to 64-bit, without masking the higher 16 bits as well. (func 51, line 8789 to 8795) call 29 call 48 global.get 5 i64.extend_i32_u i32.const 1 call 47 i32.const 0 issue.zip

carlopi commented 2 years ago

Thank you for this test case, I can reproduce and there actually is a corner case in the WebAssembly writer where truncation was not being properly handled.

I will later file a PR with the fix, thanks for pointing to this problem.

Do note that -cheerp-linear-output=asmjs works correctly. While waiting for either the nighly build or the the fix to be buillt locally you can keep developing providing the -cheerp-linear-output=asmjs flag.

zyz9740 commented 2 years ago

Thanks for your quick reply !

carlopi commented 2 years ago

The commit solving this issue (https://github.com/leaningtech/cheerp-compiler/commit/aa40df1d0a61aa93fd4f9dde7e3bdff5aec9f1ba) has been merged.

To add some explanation, 8 bit and 16 bits are stored as WebAssembly 32 bits numbers, an.d truncation are always the final user responsibility. In a corner case (values represented as WebAssembly's globals) the proper truncation was not performed, and that lead to the bug.

Thanks again, closing this as it's fixed.