Skip to content

do not export getValue and setValue by default #5839

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

Merged
merged 11 commits into from
Dec 1, 2017
6 changes: 5 additions & 1 deletion ChangeLog.markdown
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,11 @@ To browse or download snapshots of old tagged versions, visit https://github.com

Not all changes are documented here. In particular, new features, user-oriented fixes, options, command-line parameters, usage changes, deprecations, significant internal modifications and optimizations etc. generally deserve a mention. To examine the full set of changes between versions, visit the link to full changeset diff at the end of each section.

Current trunk code
Current Trunk
-------------
- Breaking change: Do not export getValue/setValue runtime methods by default. You can still use them by calling them directly in code optimized with the main file (pre-js, post-js, js libraries; if the optimizer sees they are used, it preserves them), but if you try to use them on `Module` then you must export them by adding them to `EXTRA_EXPORTED_RUNTIME_METHODS`. In `-O0` or when `ASSERTIONS` is on, a run-time error message explains that, if they are attempted to be used incorrectly.

v1.37.17: 7/25/2017
------------------
- Updated to libc++'s "v2" ABI, which provides better alignment for string data and other improvements. This is an ABI-incompatible change, so bitcode files from previous versions will not be compatible.
- To see a list of commits in the active development branch 'incoming', which have not yet been packaged in a release, see
Expand Down
13 changes: 11 additions & 2 deletions src/modules.js
Original file line number Diff line number Diff line change
Expand Up @@ -272,11 +272,20 @@ EXPORTED_RUNTIME_METHODS = unset(EXPORTED_RUNTIME_METHODS_SET);
EXTRA_EXPORTED_RUNTIME_METHODS = [];

function maybeExport(name) {
// if requested to be exported, export it
if (name in EXPORTED_RUNTIME_METHODS_SET) {
return 'Module["' + name + '"] = ' + name + ';';
} else {
return '';
}
// do not export it. but if ASSERTIONS, emit a
// stub with an error, so the user gets a message
// if it is used, that they should export it
if (ASSERTIONS) {
// check if it already exists, to support EXPORT_ALL and other cases
// (we could optimize this, but in ASSERTIONS mode code size doesn't
// matter anyhow)
return 'if (!Module["' + name + '"]) Module["' + name + '"] = function() { abort("\'' + name + '\' was not exported. add it to EXTRA_EXPORTED_RUNTIME_METHODS.") };';
}
return '';
}

var PassManager = {
Expand Down
2 changes: 1 addition & 1 deletion src/preamble.js
Original file line number Diff line number Diff line change
Expand Up @@ -806,7 +806,7 @@ function demangle(func) {
stringToUTF8(s, buf, len);
var status = _malloc(4);
var ret = __cxa_demangle_func(buf, 0, 0, status);
if (getValue(status, 'i32') === 0 && ret) {
if ({{{ makeGetValue('status', '0', 'i32') }}} === 0 && ret) {
return Pointer_stringify(ret);
}
// otherwise, libcxxabi failed
Expand Down
2 changes: 0 additions & 2 deletions src/settings.js
Original file line number Diff line number Diff line change
Expand Up @@ -353,8 +353,6 @@ var EXPORTED_RUNTIME_METHODS = [ // Runtime elements that are exported on Module
'Runtime',
'ccall',
'cwrap',
'setValue',
'getValue',
'ALLOC_NORMAL',
'ALLOC_STACK',
'ALLOC_STATIC',
Expand Down
16 changes: 16 additions & 0 deletions tests/core/getValue_setValue.cpp
Original file line number Diff line number Diff line change
@@ -0,0 +1,16 @@
#include<emscripten.h>

int main() {
#ifdef DIRECT
EM_ASM({
setValue(8, 1234, 'i32');
Module['print']('|' + getValue(8, 'i32') + '|');
});
#else
EM_ASM({
Module['setValue'](8, 1234, 'i32');
Module['print']('|' + Module['getValue'](8, 'i32') + '|');
});
#endif
}

1 change: 1 addition & 0 deletions tests/core/getValue_setValue.txt
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
|1234|
1 change: 1 addition & 0 deletions tests/core/getValue_setValue_assert.txt
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
'setValue' was not exported. add it to EXTRA_EXPORTED_RUNTIME_METHODS.
22 changes: 21 additions & 1 deletion tests/test_core.py
Original file line number Diff line number Diff line change
Expand Up @@ -4412,7 +4412,7 @@ def test_utime(self):
def test_utf(self):
self.banned_js_engines = [SPIDERMONKEY_ENGINE] # only node handles utf well
Settings.EXPORTED_FUNCTIONS = ['_main', '_malloc']

Settings.EXTRA_EXPORTED_RUNTIME_METHODS = ['getValue', 'setValue']
self.do_run_in_out_file_test('tests', 'core', 'test_utf')

def test_utf32(self):
Expand Down Expand Up @@ -5936,6 +5936,26 @@ def process(filename):
self.emcc_args += ['--closure', '1']
self.do_run_in_out_file_test('tests', 'core', 'test_ccall', post_build=post)

def test_getValue_setValue(self):
# these used to be exported, but no longer are by default
def test(output_prefix='', args=[]):
old = self.emcc_args[:]
self.emcc_args += args
self.do_run(open(path_from_root('tests', 'core', 'getValue_setValue.cpp')).read(),
open(path_from_root('tests', 'core', 'getValue_setValue' + output_prefix + '.txt')).read())
self.emcc_args = old
# see that direct usage (not on module) works. we don't export, but the use
# keeps it alive through JSDCE
test(args=['-DDIRECT'])
# see that with assertions, we get a nice error message
Settings.EXTRA_EXPORTED_RUNTIME_METHODS = []
Settings.ASSERTIONS = 1
test('_assert')
Settings.ASSERTIONS = 0
# see that when we export them, things work on the module
Settings.EXTRA_EXPORTED_RUNTIME_METHODS = ['getValue', 'setValue']
test()

@no_wasm_backend('DEAD_FUNCTIONS elimination is done by the JSOptimizer')
def test_dead_functions(self):
src = r'''
Expand Down
15 changes: 9 additions & 6 deletions tests/test_other.py
Original file line number Diff line number Diff line change
Expand Up @@ -4842,7 +4842,7 @@ def test_no_filesystem(self):
assert no_size < 360000

def test_no_nuthin(self):
print('part one: check NO_FILESYSTEM is automatically set, and effective')
# check NO_FILESYSTEM is automatically set, and effective
def test(opts, ratio, absolute):
print('opts, ratio, absolute:', opts, ratio, absolute)
def get_size(name):
Expand All @@ -4865,13 +4865,14 @@ def do(name, source, moar_opts):
if '--closure' in opts: # no EXPORTED_RUNTIME_METHODS makes closure much more effective
assert sizes['no_nuthin'] < 0.975*sizes['no_fs']
assert sizes['no_fs_manual'] < sizes['no_fs'] # manual can remove a tiny bit more
test([], 0.75, 360000)
test(['-s', 'ASSERTIONS=0'], 0.75, 360000) # we don't care about code size with assertions
test(['-O1'], 0.66, 210000)
test(['-O2'], 0.50, 70000)
test(['-O3', '--closure', '1'], 0.60, 50000)
test(['-O3', '--closure', '2'], 0.60, 41000) # might change now and then

print('part two: focus on EXPORTED_RUNTIME_METHODS effects, on hello_world_em_asm')
def test_no_nuthin_2(self):
# focus on EXPORTED_RUNTIME_METHODS effects, on hello_world_em_asm
def test(opts, ratio, absolute):
print('opts, ratio, absolute:', opts, ratio, absolute)
def get_size(name):
Expand All @@ -4890,7 +4891,7 @@ def do(name, moar_opts):
assert sizes['no_nuthin'] < absolute
if '--closure' in opts: # no EXPORTED_RUNTIME_METHODS makes closure much more effective
assert sizes['no_nuthin'] < 0.975*sizes['normal']
test([], 1, 220000)
test(['-s', 'ASSERTIONS=0'], 1, 220000) # we don't care about code size with assertions
test(['-O1'], 1, 215000)
test(['-O2'], 0.99, 75000)
test(['-O3', '--closure', '1'], 0.975, 50000)
Expand All @@ -4909,14 +4910,16 @@ def test_EXPORTED_RUNTIME_METHODS(self):
def test(opts, has, not_has):
print(opts, has, not_has)
self.clear()
Popen([PYTHON, EMCC, path_from_root('tests', 'hello_world.c')] + opts).communicate()
# check without assertions, as with assertions we add stubs for the things we remove (which
# print nice error messages)
Popen([PYTHON, EMCC, path_from_root('tests', 'hello_world.c'), '-s', 'ASSERTIONS=0'] + opts).communicate()
self.assertContained('hello, world!', run_js('a.out.js'))
src = open('a.out.js').read()
self.assertContained(has, src)
self.assertNotContained(not_has, src)

test([], 'Module["intArray', 'Module["waka')
test(['-s', 'EXPORTED_RUNTIME_METHODS=[]'], 'Module["print', 'Module["intArray')
test(['-s', 'EXPORTED_RUNTIME_METHODS=[]'], '', 'Module["intArray')
test(['-s', 'EXPORTED_RUNTIME_METHODS=["intArrayToString"]'], 'Module["intArray', 'Module["waka')
test(['-s', 'EXPORTED_RUNTIME_METHODS=[]', '-s', 'EXTRA_EXPORTED_RUNTIME_METHODS=["intArrayToString"]'], 'Module["intArray', 'Module["waka')

Expand Down