Skip to content

Commit 6acd733

Browse files
RReverserRReverser
authored
Improve error messages from jsifier (#18266)
Got a bit tired of opaque error messages coming from JS library processing when I accidentally introduce a syntax error, so here's a simple but effective improvement made possible by replacing `eval` with `vm.runScriptInContext`. I'm actually not sure why Node.js doesn't just use the same error output for eval as well, but at least this helps 🤷‍♂️ While at it, also added filenames to `load` functions used by compiler.js itself, so the stacktrace contains actual filenames instead of nested evals and did few other minor cleanups to avoid error message duplication. Example preprocessor error message before: ``` parseTools.js preprocessor error in shell_minimal.js:126: "#if MIN_CHROME_VERSION < 45 || MINEDGE_VERSION < 12 || MIN_FIREFOX_VERSION < 34 || MIN_IE_VERSION != TARGET_NOT_SUPPORTED || MIN_SAFARI_VERSION < 90000"! Internal compiler error in src/compiler.js! Please create a bug report at https://github.com/emscripten-core/emscripten/issues/ with a log of the build and the input files used to run. Exception message: "ReferenceError: MINEDGE_VERSION is not defined" | ReferenceError: MINEDGE_VERSION is not defined at eval (eval at preprocess (eval at load (C:\Users\me\Documents\emscripten\src\compiler.js:39:8)), <anonymous>:1:29) at preprocess (eval at load (C:\Users\me\Documents\emscripten\src\compiler.js:39:8), <anonymous>:97:32) at finalCombiner (eval at load (C:\Users\me\Documents\emscripten\src\compiler.js:39:8), <anonymous>:475:25) at runJSify (eval at load (C:\Users\me\Documents\emscripten\src\compiler.js:39:8), <anonymous>:543:3) ... ``` After: ``` Internal compiler error in src/compiler.js! Please create a bug report at https://github.com/emscripten-core/emscripten/issues/ with a log of the build and the input files used to run. Exception message: "shell_minimal.js:126 MIN_CHROME_VERSION < 45 || MINEDGE_VERSION < 12 || MIN_FIREFOX_VERSION < 34 || MIN_IE_VERSION != TARGET_NOT_SUPPORTED || MIN_SAFARI_VERSION < 90000 ^ ReferenceError: MINEDGE_VERSION is not defined at shell_minimal.js:126:32 at Script.runInThisContext (vm.js:134:12) at Object.runInThisContext (vm.js:310:38) at preprocess (C:\Users\me\Documents\emscripten\src\parseTools.js:96:33) at finalCombiner (C:\Users\me\Documents\emscripten\src\jsifier.js:475:25) at runJSify (C:\Users\me\Documents\emscripten\src\jsifier.js:543:3) ... ``` Example post-processed JS error message before: ``` error: failure to execute js library "library_pthread.js": error: use -sVERBOSE to see more details Internal compiler error in src/compiler.js! Please create a bug report at https://github.com/emscripten-core/emscripten/issues/ with a log of the build and the input files used to run. Exception message: "SyntaxError: Unexpected token 'function'" | SyntaxError: Unexpected token 'function' at Object.load (eval at load (C:\Users\me\Documents\emscripten\src\compiler.js:39:8), <anonymous>:215:14) at runJSify (eval at load (C:\Users\me\Documents\emscripten\src\compiler.js:39:8), <anonymous>:78:18) ... ``` After: ``` error: failure to execute js library "library_pthread.js": error: use -sVERBOSE to see more details Internal compiler error in src/compiler.js! Please create a bug report at https://github.com/emscripten-core/emscripten/issues/ with a log of the build and the input files used to run. Exception message: "library_pthread.preprocessed.js:330 $spawnThread function(threadParams) { ^^^^^^^^ SyntaxError: Unexpected token 'function' at new Script (vm.js:102:7) at createScript (vm.js:262:10) at Object.runInThisContext (vm.js:310:10) at Object.load (C:\Users\me\Documents\emscripten\src\modules.js:215:12) at runJSify (C:\Users\me\Documents\emscripten\src\jsifier.js:78:18) ... ```
1 parent afa75f3 commit 6acd733

File tree

6 files changed

+86
-97
lines changed

6 files changed

+86
-97
lines changed

‎ChangeLog.md

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -33,6 +33,7 @@ See docs/process.md for more on how version tagging works.
3333
state normally stored on the stack is hidden within the runtime and does not
3434
occupy linear memory at all. The default for `DEFAULT_PTHREAD_STACK_SIZE` was
3535
also reduced from 2MB to 64KB to match.
36+
- Improved error messages for writing custom JS libraries. (#18266)
3637

3738
3.1.26 - 11/17/22
3839
-----------------

‎src/compiler.js

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,7 @@
88
// LLVM => JavaScript compiler, main entry point
99

1010
const fs = require('fs');
11+
global.vm = require('vm');
1112
global.assert = require('assert');
1213
global.nodePath = require('path');
1314

@@ -36,7 +37,7 @@ global.read = (filename) => {
3637
};
3738

3839
function load(f) {
39-
eval.call(null, read(f));
40+
(0, eval)(read(f) + '//# sourceURL=' + find(f));
4041
};
4142

4243
// Basic utilities
@@ -92,7 +93,7 @@ try {
9293
// Compiler failed on internal compiler error!
9394
printErr('Internal compiler error in src/compiler.js!');
9495
printErr('Please create a bug report at https://github.com/emscripten-core/emscripten/issues/');
95-
printErr('with a log of the build and the input files used to run. Exception message: "' + err + '" | ' + err.stack);
96+
printErr('with a log of the build and the input files used to run. Exception message: "' + (err.stack || err));
9697
}
9798

9899
// Work around a node.js bug where stdout buffer is not flushed at process exit:

‎src/jsifier.js

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -526,7 +526,7 @@ function ${name}(${args}) {
526526
const post = processMacros(preprocess(read(postFile), postFile));
527527
print(post);
528528

529-
print(processMacros(preprocess(shellParts[1], shellFile)));
529+
print(processMacros(preprocess(shellParts[1], shellFile, shellParts[0].match(/\n/g).length)));
530530

531531
print('\n//FORWARDED_DATA:' + JSON.stringify({
532532
librarySymbols: librarySymbols,

‎src/modules.js

Lines changed: 5 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -212,26 +212,17 @@ global.LibraryManager = {
212212
}
213213
try {
214214
processed = processMacros(preprocess(src, filename));
215-
eval(processed);
215+
vm.runInThisContext(processed, { filename: filename.replace(/\.\w+$/, '.preprocessed$&') });
216216
} catch (e) {
217-
const details = [e, e.lineNumber ? `line number: ${e.lineNumber}` : ''];
217+
error(`failure to execute js library "${filename}":`);
218218
if (VERBOSE) {
219-
details.push((e.stack || '').toString().replace('Object.<anonymous>', filename));
220-
}
221-
if (processed) {
222-
error(`failure to execute js library "${filename}": ${details}`);
223-
if (VERBOSE) {
219+
if (processed) {
224220
error(`preprocessed source (you can run a js engine on this to get a clearer error message sometimes):\n=============\n${processed}\n=============`);
225221
} else {
226-
error('use -sVERBOSE to see more details');
227-
}
228-
} else {
229-
error(`failure to process js library "${filename}": ${details}`);
230-
if (VERBOSE) {
231222
error(`original source:\n=============\n${src}\n=============`);
232-
} else {
233-
error('use -sVERBOSE to see more details');
234223
}
224+
} else {
225+
error('use -sVERBOSE to see more details');
235226
}
236227
throw e;
237228
} finally {

‎src/parseTools.js

Lines changed: 74 additions & 79 deletions
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,7 @@
88
* Tests live in test/other/test_parseTools.js.
99
*/
1010

11-
const FOUR_GB = 4 * 1024 * 1024 * 1024;
11+
globalThis.FOUR_GB = 4 * 1024 * 1024 * 1024;
1212
const FLOAT_TYPES = new Set(['float', 'double']);
1313

1414
let currentlyParsedFilename = '';
@@ -34,7 +34,7 @@ function processMacros(text) {
3434
// Also handles #include x.js (similar to C #include <file>)
3535
// Param filenameHint can be passed as a description to identify the file that is being processed, used
3636
// to locate errors for reporting and for html files to stop expansion between <style> and </style>.
37-
function preprocess(text, filenameHint) {
37+
function preprocess(text, filenameHint, lineOffset = 0) {
3838
if (EXPORT_ES6 && USE_ES6_IMPORT_META) {
3939
// `eval`, Terser and Closure don't support module syntax; to allow it,
4040
// we need to temporarily replace `import.meta` and `await import` usages
@@ -63,97 +63,92 @@ function preprocess(text, filenameHint) {
6363
let emptyLine = false;
6464

6565
try {
66-
for (let i = 0; i < lines.length; i++) {
67-
let line = lines[i];
68-
try {
69-
if (line[line.length - 1] === '\r') {
70-
line = line.substr(0, line.length - 1); // Windows will have '\r' left over from splitting over '\r\n'
71-
}
72-
if (isHtml && line.includes('<style') && !inStyle) {
73-
inStyle = true;
74-
}
75-
if (isHtml && line.includes('</style') && inStyle) {
76-
inStyle = false;
77-
}
66+
for (let [i, line] of lines.entries()) {
67+
i += lineOffset; // adjust line number in case this is e.g. 2nd part of shell.js
68+
if (line[line.length - 1] === '\r') {
69+
line = line.substr(0, line.length - 1); // Windows will have '\r' left over from splitting over '\r\n'
70+
}
71+
if (isHtml && line.includes('<style') && !inStyle) {
72+
inStyle = true;
73+
}
74+
if (isHtml && line.includes('</style') && inStyle) {
75+
inStyle = false;
76+
}
7877

79-
if (!inStyle) {
80-
const trimmed = line.trim();
81-
if (trimmed.startsWith('#')) {
82-
const first = trimmed.split(' ', 1)[0];
83-
if (first == '#if' || first == '#ifdef' || first == '#elif') {
84-
if (first == '#ifdef') {
85-
warn('use of #ifdef in js library. Use #if instead.');
86-
}
87-
if (first == '#elif') {
88-
const curr = showStack.pop();
89-
if (curr == SHOW || curr == IGNORE_ALL) {
90-
// If we showed to previous block we enter the IGNORE_ALL state
91-
// and stay there until endif is seen
92-
showStack.push(IGNORE_ALL);
93-
continue;
94-
}
95-
}
96-
const after = trimmed.substring(trimmed.indexOf(' '));
97-
const truthy = !!eval(after);
98-
showStack.push(truthy ? SHOW : IGNORE);
99-
} else if (first === '#include') {
100-
if (showCurrentLine()) {
101-
let filename = line.substr(line.indexOf(' ') + 1);
102-
if (filename.startsWith('"')) {
103-
filename = filename.substr(1, filename.length - 2);
104-
}
105-
const included = read(filename);
106-
const result = preprocess(included, filename);
107-
if (result) {
108-
ret += `// include: ${filename}\n`;
109-
ret += result;
110-
ret += `// end include: ${filename}\n`;
111-
}
112-
}
113-
} else if (first === '#else') {
114-
assert(showStack.length > 0);
78+
if (!inStyle) {
79+
const trimmed = line.trim();
80+
if (trimmed.startsWith('#')) {
81+
const first = trimmed.split(' ', 1)[0];
82+
if (first == '#if' || first == '#ifdef' || first == '#elif') {
83+
if (first == '#ifdef') {
84+
warn('use of #ifdef in js library. Use #if instead.');
85+
}
86+
if (first == '#elif') {
11587
const curr = showStack.pop();
116-
if (curr == IGNORE) {
117-
showStack.push(SHOW);
118-
} else {
119-
showStack.push(IGNORE);
88+
if (curr == SHOW || curr == IGNORE_ALL) {
89+
// If we showed to previous block we enter the IGNORE_ALL state
90+
// and stay there until endif is seen
91+
showStack.push(IGNORE_ALL);
92+
continue;
12093
}
121-
} else if (first === '#endif') {
122-
assert(showStack.length > 0);
123-
showStack.pop();
124-
} else if (first === '#warning') {
125-
if (showCurrentLine()) {
126-
printErr(`${filenameHint}:${i + 1}: #warning ${trimmed.substring(trimmed.indexOf(' ')).trim()}`);
94+
}
95+
const after = trimmed.substring(trimmed.indexOf(' '));
96+
const truthy = !!vm.runInThisContext(after, { filename: filenameHint, lineOffset: i, columnOffset: line.indexOf(after) });
97+
showStack.push(truthy ? SHOW : IGNORE);
98+
} else if (first === '#include') {
99+
if (showCurrentLine()) {
100+
let filename = line.substr(line.indexOf(' ') + 1);
101+
if (filename.startsWith('"')) {
102+
filename = filename.substr(1, filename.length - 2);
127103
}
128-
} else if (first === '#error') {
129-
if (showCurrentLine()) {
130-
error(`${filenameHint}:${i + 1}: #error ${trimmed.substring(trimmed.indexOf(' ')).trim()}`);
104+
const included = read(filename);
105+
const result = preprocess(included, filename);
106+
if (result) {
107+
ret += `// include: ${filename}\n`;
108+
ret += result;
109+
ret += `// end include: ${filename}\n`;
131110
}
111+
}
112+
} else if (first === '#else') {
113+
assert(showStack.length > 0);
114+
const curr = showStack.pop();
115+
if (curr == IGNORE) {
116+
showStack.push(SHOW);
132117
} else {
133-
throw new Error(`Unknown preprocessor directive on line ${i}: ``${line}```);
118+
showStack.push(IGNORE);
134119
}
135-
} else {
120+
} else if (first === '#endif') {
121+
assert(showStack.length > 0);
122+
showStack.pop();
123+
} else if (first === '#warning') {
136124
if (showCurrentLine()) {
137-
// Never emit more than one empty line at a time.
138-
if (emptyLine && !line) {
139-
continue;
140-
}
141-
ret += line + '\n';
142-
if (!line) {
143-
emptyLine = true;
144-
} else {
145-
emptyLine = false;
146-
}
125+
printErr(`${filenameHint}:${i + 1}: #warning ${trimmed.substring(trimmed.indexOf(' ')).trim()}`);
126+
}
127+
} else if (first === '#error') {
128+
if (showCurrentLine()) {
129+
error(`${filenameHint}:${i + 1}: #error ${trimmed.substring(trimmed.indexOf(' ')).trim()}`);
147130
}
131+
} else {
132+
throw new Error(`${filenameHint}:${i + 1}: Unknown preprocessor directive ${first}`);
148133
}
149-
} else { // !inStyle
134+
} else {
150135
if (showCurrentLine()) {
136+
// Never emit more than one empty line at a time.
137+
if (emptyLine && !line) {
138+
continue;
139+
}
151140
ret += line + '\n';
141+
if (!line) {
142+
emptyLine = true;
143+
} else {
144+
emptyLine = false;
145+
}
152146
}
153147
}
154-
} catch (e) {
155-
printErr('parseTools.js preprocessor error in ' + filenameHint + ':' + (i + 1) + ': \"' + line + '\"!');
156-
throw e;
148+
} else { // !inStyle
149+
if (showCurrentLine()) {
150+
ret += line + '\n';
151+
}
157152
}
158153
}
159154
assert(showStack.length == 0, `preprocessing error in file ${filenameHint}, \

‎tools/preprocessor.js

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,7 @@
1616

1717
const fs = require('fs');
1818
const path = require('path');
19+
global.vm = require('vm');
1920

2021
const arguments_ = process['argv'].slice(2);
2122
const debug = false;
@@ -46,7 +47,7 @@ global.read = function(filename) {
4647
};
4748

4849
global.load = function(f) {
49-
eval.call(null, read(f));
50+
(0, eval)(read(f) + '//# sourceURL=' + find(f));
5051
};
5152

5253
const settingsFile = arguments_[0];

0 commit comments

Comments
 (0)