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.

Sign up for GitHub

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

Jump to bottom

side module calls main module with a 64bit param #8291

Closed
wants to merge 6 commits into from
Closed

side module calls main module with a 64bit param #8291

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
aac4d8a
- test case which fails when side module calls main module with a 64b…
Mar 13, 2019
90c0b07
-fix up test case such that it fails correctly
Mar 14, 2019
e37f083
- fix test case loading of side_module
Mar 19, 2019
67185f0
fix pylinter error
Mar 19, 2019
bb46d38
- run cmd
Mar 21, 2019
3128f52
- fixing up tests so that they fail correctly
Mar 21, 2019
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
16 changes: 16 additions & 0 deletions tests/other/noffi-side.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,16 @@
/*
* Copyright 2019 The Emscripten Authors. All rights reserved.
* Emscripten is available under two separate licenses, the MIT license and the
* University of Illinois/NCSA Open Source License. Both these licenses can be
* found in the LICENSE file.
*/

#include <cstdint>
#include <emscripten.h>

extern "C"{
extern void passBigInt(uint64_t);
}
EMSCRIPTEN_KEEPALIVE void callPassBigInt(){
passBigInt(1152921504606846975);
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think we expect that this would get translated into a call_indirect, but this gets translated into a $env._passBigInt instead.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The indirection here is handled on the JS side. The imported function is a JS stub that gets filled in as symbols are resolved by loading modules.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

right and this JS stub is causing problems when we turn off JS legalization.

}
22 changes: 21 additions & 1 deletion tests/other/noffi.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,9 +11,15 @@
#include <stdexcept>
#include <cstdint>
#include <dlfcn.h>
#include <emscripten.h>

using namespace std;

extern "C"{
EMSCRIPTEN_KEEPALIVE void passBigInt(uint64_t u){
printf("got %lld\n", u);
}
}
uint64_t getbigint(){
int ran = rand() % 100;// v1 in the range 0 to 99
++ran;
Expand All @@ -24,8 +30,22 @@ uint64_t getbigint(){
}
int main()
{
float safeY = 0.0f;
printf("start of main");
uint64_t mybig = 0;
EM_ASM({
FS.mkdir('/working');
FS.mount(NODEFS, { root: '.' }, '/working');
});
void *handle = dlopen("/working/side.wasm", RTLD_NOW);
typedef void (*sideModule_fn)(void);
sideModule_fn exportedfn = (sideModule_fn)dlsym(handle, "_Z14callPassBigIntv");
const char *err = dlerror();
if (err) {
printf("ERROR: dlsym failed: for sideModule_fn");
//abort();
}
printf("calling side.wasm \n");
exportedfn();

try{
mybig = getbigint();
Expand Down
7 changes: 7 additions & 0 deletions tests/test_other.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8102,6 +8102,13 @@ def test_no_legalize_js_ffi(self):
e_legalstub_i32 = re.search('\(func.*\$legalstub\$dyn.*\(type \$\d+\).*\(result i32\)', text)
assert i_legalimport_i64, 'legal import not generated for invoke call'
assert e_legalstub_i32, 'legal stub not generated for dyncall'
args = ['-s', 'LEGALIZE_JS_FFI=0', '-s', 'SIDE_MODULE=1', '-O3', '-s', 'DISABLE_EXCEPTION_CATCHING=0']
print(args)
cmd = [PYTHON, EMCC, path_from_root('tests', 'other', 'noffi-side.cpp'), '-g', '-o', 'side.wasm'] + args
print(' '.join(cmd))
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I do see this fail in the node command, with function signature mismatch - but this emcc command is never executed here. So side.wasm does not exist. Is that the cause of the problem?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@kripken sorry about that. i have pushed a fix to the test so it fails properly now. the error is
exception thrown: TypeError: invalid type,TypeError: invalid type

run_process(cmd)
ret = run_process(NODE_JS + ['a.out.js'], stdout=PIPE).stdout
self.assertContained('1152921504606846975', ret)

def test_sysconf_phys_pages(self):
for args, expected in [
Expand Down