Skip to content

Commit d7f762e

Browse files
committed
Change use of NO_DYNAMIC_EXECUTION in embind to DYNAMIC_EXECUTION.
Adds a validation step to the nodejs command when running the tests with DYNAMIC_EXECUTION=0 to ensure the generated script has no eval. This required adding the ability to specify args to the nodejs command to jsrun.py
1 parent 9ecca00 commit d7f762e

File tree

5 files changed

+25
-19
lines changed

5 files changed

+25
-19
lines changed

src/embind/embind.js

Lines changed: 7 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -50,10 +50,10 @@ var LibraryEmbind = {
5050
Module['flushPendingDeletes'] = flushPendingDeletes;
5151
Module['setDelayFunction'] = setDelayFunction;
5252
#if IN_TEST_HARNESS
53-
#if NO_DYNAMIC_EXECUTION
53+
#if DYNAMIC_EXECUTION == 0
5454
// Without dynamic execution, dynamically created functions will have no
5555
// names. This lets the test suite know that.
56-
Module['NO_DYNAMIC_EXECUTION'] = true;
56+
Module['DYNAMIC_EXECUTION'] = false;
5757
#endif
5858
#if EMBIND_STD_STRING_IS_UTF8
5959
Module['EMBIND_STD_STRING_IS_UTF8'] = true;
@@ -194,7 +194,7 @@ var LibraryEmbind = {
194194
$createNamedFunction__deps: ['$makeLegalFunctionName'],
195195
$createNamedFunction: function(name, body) {
196196
name = makeLegalFunctionName(name);
197-
#if NO_DYNAMIC_EXECUTION
197+
#if DYNAMIC_EXECUTION == 0
198198
return function() {
199199
"use strict";
200200
return body.apply(this, arguments);
@@ -847,9 +847,9 @@ var LibraryEmbind = {
847847
if (!(constructor instanceof Function)) {
848848
throw new TypeError('new_ called with constructor type ' + typeof(constructor) + " which is not a function");
849849
}
850-
#if NO_DYNAMIC_EXECUTION
850+
#if DYNAMIC_EXECUTION == 0
851851
if (constructor === Function) {
852-
throw new Error('new_ cannot create a new Function with NO_DYNAMIC_EXECUTION.');
852+
throw new Error('new_ cannot create a new Function with DYNAMIC_EXECUTION == 0.');
853853
}
854854
#endif
855855

@@ -913,7 +913,7 @@ var LibraryEmbind = {
913913

914914
var returns = (argTypes[0].name !== "void");
915915

916-
#if NO_DYNAMIC_EXECUTION
916+
#if DYNAMIC_EXECUTION == 0
917917
var argsWired = new Array(argCount - 2);
918918
return function() {
919919
if (arguments.length !== argCount - 2) {
@@ -1042,7 +1042,7 @@ var LibraryEmbind = {
10421042
signature = readLatin1String(signature);
10431043

10441044
function makeDynCaller(dynCall) {
1045-
#if NO_DYNAMIC_EXECUTION
1045+
#if DYNAMIC_EXECUTION == 0
10461046
return function() {
10471047
var args = new Array(arguments.length + 1);
10481048
args[0] = rawFunction;

src/embind/emval.js

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -151,7 +151,7 @@ var LibraryEmVal = {
151151
var obj = new constructor(arg0, arg1, arg2);
152152
return __emval_register(obj);
153153
} */
154-
#if NO_DYNAMIC_EXECUTION
154+
#if DYNAMIC_EXECUTION == 0
155155
var argsList = new Array(argCount + 1);
156156
return function(constructor, argTypes, args) {
157157
argsList[0] = constructor;
@@ -202,7 +202,7 @@ var LibraryEmVal = {
202202
return newer(handle, argTypes, args);
203203
},
204204

205-
#if NO_DYNAMIC_EXECUTION
205+
#if DYNAMIC_EXECUTION == 0
206206
$emval_get_global: function() {
207207
function testGlobal(obj) {
208208
obj['$$$embind_global$$$'] = obj;
@@ -354,7 +354,7 @@ var LibraryEmVal = {
354354
var types = __emval_lookupTypes(argCount, argTypes);
355355

356356
var retType = types[0];
357-
#if NO_DYNAMIC_EXECUTION
357+
#if DYNAMIC_EXECUTION == 0
358358
var argN = new Array(argCount - 1);
359359
var invokerFunction = function(handle, name, destructors, args) {
360360
var offset = 0;

tests/embind/embind.test.js

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -162,7 +162,7 @@ module({
162162
});
163163

164164
test("setting and getting property on unrelated class throws error", function() {
165-
var className = cm['NO_DYNAMIC_EXECUTION'] ? '' : 'HasTwoBases';
165+
var className = cm['DYNAMIC_EXECUTION'] === false ? '' : 'HasTwoBases';
166166
var a = new cm.HasTwoBases;
167167
var e = assert.throws(cm.BindingError, function() {
168168
Object.getOwnPropertyDescriptor(cm.HeldBySmartPtr.prototype, 'i').set.call(a, 10);
@@ -1598,7 +1598,7 @@ module({
15981598

15991599
test("smart pointer object has correct constructor name", function() {
16001600
var e = new cm.HeldBySmartPtr(10, "foo");
1601-
var expectedName = cm['NO_DYNAMIC_EXECUTION'] ? "" : "HeldBySmartPtr";
1601+
var expectedName = cm['DYNAMIC_EXECUTION'] === false ? "": "HeldBySmartPtr";
16021602
assert.equal(expectedName, e.constructor.name);
16031603
e.delete();
16041604
});
@@ -2342,7 +2342,7 @@ module({
23422342
});
23432343

23442344
BaseFixture.extend("function names", function() {
2345-
if (cm['NO_DYNAMIC_EXECUTION']) {
2345+
if (cm['DYNAMIC_EXECUTION'] === false) {
23462346
assert.equal('', cm.ValHolder.name);
23472347
assert.equal('', cm.ValHolder.prototype.setVal.name);
23482348
assert.equal('', cm.ValHolder.makeConst.name);

tests/test_other.py

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2399,6 +2399,10 @@ def test_embind(self):
23992399
if fail:
24002400
self.assertNotEqual(proc.returncode, 0)
24012401
else:
2402+
if 'DYNAMIC_EXECUTION=0' in args:
2403+
output = run_js(self.in_dir('a.out.js'), stdout=PIPE, stderr=PIPE, assert_returncode=0, engine=NODE_JS,
2404+
shell_args=['--disallow-code-generation-from-strings'])
2405+
24022406
with open(self.in_dir('a.out.js'), 'ab') as f:
24032407
for tf in testFiles:
24042408
f.write(open(tf, 'rb').read())

tools/jsrun.py

Lines changed: 8 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -27,7 +27,7 @@ def timeout_run(proc, timeout=None, note='unnamed process', full_output=False, n
2727
return '\n'.join(out) if full_output else out[0]
2828

2929

30-
def make_command(filename, engine=None, args=[]):
30+
def make_command(filename, engine=None, args=[], shell_args=[]):
3131
if type(engine) is not list:
3232
engine = [engine]
3333
# Emscripten supports multiple javascript runtimes. The default is nodejs but
@@ -43,10 +43,12 @@ def make_command(filename, engine=None, args=[]):
4343
is_d8 = 'd8' in jsengine
4444
# Disable true async compilation (async apis will in fact be synchronous) for now
4545
# due to https://bugs.chromium.org/p/v8/issues/detail?id=6263
46-
shell_option_flags = ['--no-wasm-async-compilation'] if is_d8 else []
46+
shell_args.extend(['--no-wasm-async-compilation'] if is_d8 else [])
4747
# Separates engine flags from script flags
48-
flag_separator = ['--'] if is_d8 or 'jsc' in jsengine else []
49-
return engine + [filename] + shell_option_flags + flag_separator + args
48+
if is_d8 or 'jsc' in jsengine:
49+
return engine + [filename] + shell_args + ['--'] + args
50+
else:
51+
return engine + shell_args + [filename] + args
5052

5153

5254
def check_engine(engine):
@@ -87,7 +89,7 @@ def run_js_tool(filename, engine, jsargs=[], *args, **kw):
8789

8890

8991
# TODO(sbc): Move to into test directory
90-
def run_js(filename, engine=None, args=[], check_timeout=False, stdin=None, stdout=PIPE, stderr=None, cwd=None,
92+
def run_js(filename, engine=None, args=[], shell_args=[], check_timeout=False, stdin=None, stdout=PIPE, stderr=None, cwd=None,
9193
full_output=False, assert_returncode=0, error_limit=-1, skip_check=False):
9294
"""Execute javascript code generated by tests, with possible timeout."""
9395
if not os.path.exists(filename):
@@ -112,7 +114,7 @@ def run_js(filename, engine=None, args=[], check_timeout=False, stdin=None, stdo
112114
# commands += os.path.basename(curr) + ',' + json.dumps(args) + '\n'
113115
# open(commands_file, 'w').write(commands)
114116

115-
command = make_command(filename, engine, args)
117+
command = make_command(filename, engine, args, shell_args)
116118
try:
117119
proc = Popen(
118120
command,

0 commit comments

Comments
 (0)