Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
1 change: 1 addition & 0 deletions ChangeLog.md
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,7 @@ See docs/process.md for more on how version tagging works.
state normally stored on the stack is hidden within the runtime and does not
occupy linear memory at all. The default for `DEFAULT_PTHREAD_STACK_SIZE` was
also reduced from 2MB to 64KB to match.
- Improved error messages for writing custom JS libraries. (#18266)

3.1.26 - 11/17/22
-----------------
Expand Down
5 changes: 3 additions & 2 deletions src/compiler.js
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@
// LLVM => JavaScript compiler, main entry point

const fs = require('fs');
global.vm = require('vm');
global.assert = require('assert');
global.nodePath = require('path');

Expand Down Expand Up @@ -36,7 +37,7 @@ global.read = (filename) => {
};

function load(f) {
eval.call(null, read(f));
(0, eval)(read(f) + '//# sourceURL=' + find(f));
};

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

// Work around a node.js bug where stdout buffer is not flushed at process exit:
Expand Down
2 changes: 1 addition & 1 deletion src/jsifier.js
Original file line number Diff line number Diff line change
Expand Up @@ -526,7 +526,7 @@ function ${name}(${args}) {
const post = processMacros(preprocess(read(postFile), postFile));
print(post);

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

print('\n//FORWARDED_DATA:' + JSON.stringify({
librarySymbols: librarySymbols,
Expand Down
19 changes: 5 additions & 14 deletions src/modules.js
Original file line number Diff line number Diff line change
Expand Up @@ -212,26 +212,17 @@ global.LibraryManager = {
}
try {
processed = processMacros(preprocess(src, filename));
eval(processed);
vm.runInThisContext(processed, { filename: filename.replace(/\.\w+$/, '.preprocessed$&') });
} catch (e) {
const details = [e, e.lineNumber ? `line number: ${e.lineNumber}` : ''];
error(`failure to execute js library "${filename}":`);
if (VERBOSE) {
details.push((e.stack || '').toString().replace('Object.<anonymous>', filename));
}
if (processed) {
error(`failure to execute js library "${filename}": ${details}`);
if (VERBOSE) {
if (processed) {
error(`preprocessed source (you can run a js engine on this to get a clearer error message sometimes):\n=============\n${processed}\n=============`);
} else {
error('use -sVERBOSE to see more details');
}
} else {
error(`failure to process js library "${filename}": ${details}`);
if (VERBOSE) {
error(`original source:\n=============\n${src}\n=============`);
} else {
error('use -sVERBOSE to see more details');
}
} else {
error('use -sVERBOSE to see more details');
}
throw e;
} finally {
Expand Down
153 changes: 74 additions & 79 deletions src/parseTools.js
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@
* Tests live in test/other/test_parseTools.js.
*/

const FOUR_GB = 4 * 1024 * 1024 * 1024;
globalThis.FOUR_GB = 4 * 1024 * 1024 * 1024;
Copy link
Collaborator

Choose a reason for hiding this comment

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

Perhaps we should look at why this change was needed? Presumably it means that some things that were previously available in preprocessor statements might no longer be? I wonder how many such cases there are?

Copy link
Collaborator Author

@RReverser RReverser Nov 29, 2022

Choose a reason for hiding this comment

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

The only things that should / would become unavailable now are local variables in scope, since now I'm using the safer global eval, as every other place except the preprocessor, already used. All the settings and helper functions are already assigned to the global object so continue working as expected.

This file declared two "global" constants - FOUR_GB and FLOAT_TYPES - and, from my quick grep, only FOUR_GB is actualy used in libraries. It worked before because it happened to be in the same scope as eval, but instead, like other settings, it should be set on the global object to be consistently available everywhere else. Which is what I did here.

In theory it's possible that someone used local variables from preprocessor function too (e.g. i or line or filenameHint) but that would be extremely fragile and I really hope nobody did that...

At least the tests pass now, but with share-everything eval it used before, you can never be sure what external users relied on 🤷‍♂️ Mandatory https://xkcd.com/1172/ as always applies.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Makes sense. lgtm

const FLOAT_TYPES = new Set(['float', 'double']);

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

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

if (!inStyle) {
const trimmed = line.trim();
if (trimmed.startsWith('#')) {
const first = trimmed.split(' ', 1)[0];
if (first == '#if' || first == '#ifdef' || first == '#elif') {
if (first == '#ifdef') {
warn('use of #ifdef in js library. Use #if instead.');
}
if (first == '#elif') {
const curr = showStack.pop();
if (curr == SHOW || curr == IGNORE_ALL) {
// If we showed to previous block we enter the IGNORE_ALL state
// and stay there until endif is seen
showStack.push(IGNORE_ALL);
continue;
}
}
const after = trimmed.substring(trimmed.indexOf(' '));
const truthy = !!eval(after);
showStack.push(truthy ? SHOW : IGNORE);
} else if (first === '#include') {
if (showCurrentLine()) {
let filename = line.substr(line.indexOf(' ') + 1);
if (filename.startsWith('"')) {
filename = filename.substr(1, filename.length - 2);
}
const included = read(filename);
const result = preprocess(included, filename);
if (result) {
ret += `// include: ${filename}\n`;
ret += result;
ret += `// end include: ${filename}\n`;
}
}
} else if (first === '#else') {
assert(showStack.length > 0);
if (!inStyle) {
const trimmed = line.trim();
if (trimmed.startsWith('#')) {
const first = trimmed.split(' ', 1)[0];
if (first == '#if' || first == '#ifdef' || first == '#elif') {
if (first == '#ifdef') {
warn('use of #ifdef in js library. Use #if instead.');
}
if (first == '#elif') {
const curr = showStack.pop();
if (curr == IGNORE) {
showStack.push(SHOW);
} else {
showStack.push(IGNORE);
if (curr == SHOW || curr == IGNORE_ALL) {
// If we showed to previous block we enter the IGNORE_ALL state
// and stay there until endif is seen
showStack.push(IGNORE_ALL);
continue;
}
} else if (first === '#endif') {
assert(showStack.length > 0);
showStack.pop();
} else if (first === '#warning') {
if (showCurrentLine()) {
printErr(`${filenameHint}:${i + 1}: #warning ${trimmed.substring(trimmed.indexOf(' ')).trim()}`);
}
const after = trimmed.substring(trimmed.indexOf(' '));
const truthy = !!vm.runInThisContext(after, { filename: filenameHint, lineOffset: i, columnOffset: line.indexOf(after) });
showStack.push(truthy ? SHOW : IGNORE);
} else if (first === '#include') {
if (showCurrentLine()) {
let filename = line.substr(line.indexOf(' ') + 1);
if (filename.startsWith('"')) {
filename = filename.substr(1, filename.length - 2);
}
} else if (first === '#error') {
if (showCurrentLine()) {
error(`${filenameHint}:${i + 1}: #error ${trimmed.substring(trimmed.indexOf(' ')).trim()}`);
const included = read(filename);
const result = preprocess(included, filename);
if (result) {
ret += `// include: ${filename}\n`;
ret += result;
ret += `// end include: ${filename}\n`;
}
}
} else if (first === '#else') {
assert(showStack.length > 0);
const curr = showStack.pop();
if (curr == IGNORE) {
showStack.push(SHOW);
} else {
throw new Error(`Unknown preprocessor directive on line ${i}: ``${line}```);
showStack.push(IGNORE);
}
} else {
} else if (first === '#endif') {
assert(showStack.length > 0);
showStack.pop();
} else if (first === '#warning') {
if (showCurrentLine()) {
// Never emit more than one empty line at a time.
if (emptyLine && !line) {
continue;
}
ret += line + '\n';
if (!line) {
emptyLine = true;
} else {
emptyLine = false;
}
printErr(`${filenameHint}:${i + 1}: #warning ${trimmed.substring(trimmed.indexOf(' ')).trim()}`);
}
} else if (first === '#error') {
if (showCurrentLine()) {
error(`${filenameHint}:${i + 1}: #error ${trimmed.substring(trimmed.indexOf(' ')).trim()}`);
}
} else {
throw new Error(`${filenameHint}:${i + 1}: Unknown preprocessor directive ${first}`);
}
} else { // !inStyle
} else {
if (showCurrentLine()) {
// Never emit more than one empty line at a time.
if (emptyLine && !line) {
continue;
}
ret += line + '\n';
if (!line) {
emptyLine = true;
} else {
emptyLine = false;
}
}
}
} catch (e) {
printErr('parseTools.js preprocessor error in ' + filenameHint + ':' + (i + 1) + ': \"' + line + '\"!');
throw e;
} else { // !inStyle
if (showCurrentLine()) {
ret += line + '\n';
}
}
}
assert(showStack.length == 0, `preprocessing error in file ${filenameHint}, \
Expand Down
3 changes: 2 additions & 1 deletion tools/preprocessor.js
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@

const fs = require('fs');
const path = require('path');
global.vm = require('vm');

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

global.load = function(f) {
eval.call(null, read(f));
(0, eval)(read(f) + '//# sourceURL=' + find(f));
};

const settingsFile = arguments_[0];
Expand Down