-
Notifications
You must be signed in to change notification settings - Fork 1.2k
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
ufuzz failure #4466
Comments
@kzc TIL: $ node -v
v0.8.28
$ echo 'if(0)L:;else L:;' | node
$ echo 'M:if(0)L:;else L:;' | node
[stdin]:1
M:if(0)L:;else L:;
^
SyntaxError: Label 'L' has already been declared
at Object.<anonymous> ([stdin]-wrapper:6:22) $ node -v
v0.10.48
$ echo 'if(0)L:;else L:;' | node
$ echo 'M:if(0)L:;else L:;' | node
[stdin]:1
M:if(0)L:;else L:;
^
SyntaxError: Label 'L' has already been declared
at Object.<anonymous> ([stdin]-wrapper:6:22) $ node -v
v0.12.18
$ echo 'if(0)L:;else L:;' | node
$ echo 'M:if(0)L:;else L:;' | node
[stdin]:1
M:if(0)L:;else L:;
^
SyntaxError: Label 'L' has already been declared
at Object.exports.runInThisContext (vm.js:73:16) $ node -v
v4.9.1
$ echo 'if(0)L:;else L:;' | node
$ echo 'M:if(0)L:;else L:;' | node
[stdin]:1
M:if(0)L:;else L:;
^
SyntaxError: Label 'L' has already been declared
at Object.exports.runInThisContext (vm.js:53:16) $ node -v
v6.17.1
$ echo 'if(0)L:;else L:;' | node
$ echo 'M:if(0)L:;else L:;' | node
[stdin]:1
M:if(0)L:;else L:;
^
SyntaxError: Label 'L' has already been declared
at createScript (vm.js:56:10) $ node -v
v8.17.0
$ echo 'if(0)L:;else L:;' | node
$ echo 'M:if(0)L:;else L:;' | node
[stdin]:1
M:if(0)L:;else L:;
^
SyntaxError: Label 'L' has already been declared
at createScript (vm.js:80:10) $ node -v
v10.22.1
$ echo 'if(0)L:;else L:;' | node
$ echo 'M:if(0)L:;else L:;' | node
[stdin]:1
M:if(0)L:;else L:;
^
SyntaxError: Label 'L' has already been declared
at new Script (vm.js:83:7) $ node -v
v12.19.0
$ echo 'if(0)L:;else L:;' | node
$ echo 'M:if(0)L:;else L:;' | node
$ node -v
v14.13.1
$ echo 'if(0)L:;else L:;' | node
$ echo 'M:if(0)L:;else L:;' | node |
The behavior in most recent versions of V8 seems to be correct and matches an old version of spidermonkey:
The label belongs to each empty statement, not the |
None of the other platforms I've tried have the same bug either − so that's Google Genuine Advantage™️ |
... and while we are on about quirks − do you know of any stdio bug on macOS that would do something like this? 🤔 https://github.com/mishoo/UglifyJS/runs/1612437614#step:4:224 |
Regarding the label issue, all software has bugs. It was fixed in the most recent version of V8. No big deal. I'm not running the latest macos version, but the Mac version of Node has always had stdio buffering issues. I suggest to revert 9809567 in order to make stdio blocking. Or just don't bother to test on Mac at all. |
True − just admiring how basic, probably zero-day, and how long it stuck around. I don't think it's undetected, nor do I think the fix was deliberate. 😏
Another long-standing issue then 😅 I've only added that Currently only takes up 20% of testing capacity so the rest are still fully functional for both Node.js & OS of choice. |
Looks like it's not going away any time soon: Speaking of out-of-date macOS, me too 🙄 − and since that looks like your primary development platform, let me boot it up and see if I can make (The suggested solution in those issue reports don't work for all versions of Node.js IIRC − the hack deepens... 👻) |
You'll notice my participation in those and other threads. Years ago I proposed a simple solution to the flushing stdio bug upon |
fwiw, this PR is an adaptation of my proposal to fix the NodeJS A short term "workaround" at the time was to make stdio blocking in Node 6 by default. But people complained about the speed of synchronous unbuffered stdio so I would not be surprised if they reverted that and reintroduced the problem without a fix. |
So the direct execution of Line 49 in 1896694
... which is odd in the sense that my test script doesn't even call process.exit() − it continues to run after truncation is observed seemingly indefinitely.
|
I no longer follow NodeJS development after that experience. I wouldn't be surprised if other problems arose in their stdio subsystem in subsequent versions. |
No worries − at least I know with good confidence that the current I'll poke a little further until I get bored 😜 |
I don't know about the current version. I don't run uglify-js, ufuzz or test/reduce much. I maintain a private fork of uglify-es with dozens of my own patches and probably an equal number of yours (which is increasingly difficult lately due to code divergence). In my fork stdio is still set to blocking. I still follow uglify-js development because it's interesting. |
Here's the script I use to somewhat reliably reproduce the issue: var child = require("child_process").spawn("node", [
"test/ufuzz",
"-V",
], {
stdio: [ "ignore", "pipe", "pipe" ],
});
var stdout = "", stderr = "";
var original = 0, uglified = 0;
child.stdout.on("data", function(data) {
stdout += data;
});
child.stderr.on("data", function(data) {
stderr += data;
stderr = stderr.replace(/[\s\S]*?\/\/ (original|uglified) code/g, function(match, type) {
if (type === "original") original++;
if (type === "uglified") uglified++;
return "";
});
}).pipe(process.stdout);
setInterval(function() {
var end = stdout.lastIndexOf("\r");
if (end < 0) return;
console.log(stdout.slice(0, end), original, uglified, stderr.length);
stdout = stdout.slice(end + 1);
}, 5000); ... and once it pops, $ cat test.js | node
9 of Infinity 0 0 0
27 of Infinity 0 0 0
39 of Infinity 0 0 0
52 of Infinity 0 0 0
59 of Infinity 0 0 0
69 of Infinity 0 0 0
82 of Infinity 0 0 0
98 of Infinity 0 0 0
//=============================================================
// original code
...
+function() {
105 of Infinity 1 0 8209
118 of Infinity 1 0 8209
129 of Infinity 1 0 8209
135 of Infinity 1 0 8209
146 of Infinity 1 0 8209
155 of Infinity 1 0 8209
163 of Infinity 1 0 8209
180 of Infinity 1 0 8209
188 of Infinity 1 0 8209
200 of Infinity 1 0 8209
211 of Infinity 1 0 8209
224 of Infinity 1 0 8209 You can see it shoots past the second checkpoint (200) without any verbose output at all. |
I've added |
|
So may be a racing condition in Node.js or I shall put that original workaround into |
Without blocking the small Mac OS stdio buffers fill up quick and then undefined behaviour ensues. The same thing probably happens on Linux except their stdio buffers are larger - 64K as I recall. |
Windows stdio is always synchronous in NodeJS as I recall. |
The text was updated successfully, but these errors were encountered: