Bug 1507314 - Baldr: correctly propagate shell flags to nested testig process on Windows (r=bbouvier)
authorLuke Wagner <luke@mozilla.com>
Fri, 16 Nov 2018 12:03:51 -0600
changeset 446777 7154eb3601f3b89ef5b8e559c9f2bae57f6d59f3
parent 446776 9dbfa9bdcd0b393a1a396513c8e08e42e4a3da8a
child 446778 3a4a3b9133e99f3a2c86baedd6fc25564aeb4898
child 446880 b9a2dff8c8f5963bd9384f30036731cd65cd3c8d
push id109925
push userlwagner@mozilla.com
push dateFri, 16 Nov 2018 18:04:15 +0000
treeherdermozilla-inbound@7154eb3601f3 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersbbouvier
bugs1507314
milestone65.0a1
first release with
nightly linux32
7154eb3601f3 / 65.0a1 / 20181116220054 / files
nightly linux64
7154eb3601f3 / 65.0a1 / 20181116220054 / files
nightly mac
7154eb3601f3 / 65.0a1 / 20181116220054 / files
nightly win32
7154eb3601f3 / 65.0a1 / 20181116220054 / files
nightly win64
7154eb3601f3 / 65.0a1 / 20181116220054 / files
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
releases
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
Bug 1507314 - Baldr: correctly propagate shell flags to nested testig process on Windows (r=bbouvier)
js/src/jit-test/tests/wasm/regress/bug1507314.js
js/src/shell/js.cpp
new file mode 100644
--- /dev/null
+++ b/js/src/jit-test/tests/wasm/regress/bug1507314.js
@@ -0,0 +1,6 @@
+// |jit-test| --no-sse4
+if (!wasmCachingIsSupported())
+    quit(0);
+
+var m = wasmCompileInSeparateProcess(wasmTextToBinary('(module (func (export "run") (result i32) (i32.const 42)))'));
+assertEq(new WebAssembly.Instance(m).exports.run(), 42);
--- a/js/src/shell/js.cpp
+++ b/js/src/shell/js.cpp
@@ -6014,72 +6014,79 @@ class AutoPipe
 
     void closeWriter() {
         MOZ_ASSERT(fds_[1] != -1);
         close(fds_[1]);
         fds_[1] = -1;
     }
 };
 
+static const char sWasmCompileAndSerializeFlag[] = "--wasm-compile-and-serialize";
+
 static bool
 CompileAndSerializeInSeparateProcess(JSContext* cx, const uint8_t* bytecode, size_t bytecodeLength,
                                      wasm::Bytes* serialized)
 {
     AutoPipe stdIn, stdOut;
     if (!stdIn.init() || !stdOut.init()) {
         return false;
     }
 
     AutoCStringVector argv(cx);
 
     UniqueChars argv0 = DuplicateString(cx, sArgv[0]);
     if (!argv0 || !argv.append(std::move(argv0))) {
         return false;
     }
 
-    UniqueChars argv1 = DuplicateString("--wasm-compile-and-serialize");
-    if (!argv1 || !argv.append(std::move(argv1))) {
+    // Propagate shell flags first, since they must precede the non-option
+    // file-descriptor args (passed on Windows, below).
+    for (unsigned i = 0; i < sPropagatedFlags.length(); i++) {
+        UniqueChars flags = DuplicateString(cx, sPropagatedFlags[i]);
+        if (!flags || !argv.append(std::move(flags))) {
+            return false;
+        }
+    }
+
+    UniqueChars arg;
+
+    arg = DuplicateString(sWasmCompileAndSerializeFlag);
+    if (!arg || !argv.append(std::move(arg))) {
         return false;
     }
 
 #ifdef XP_WIN
     // The spawned process will have all the stdIn/stdOut file handles open, but
     // without the power of fork, we need some other way to communicate the
     // integer fd values so we encode them in argv and WasmCompileAndSerialize()
     // has a matching #ifdef XP_WIN to parse them out. Communicate both ends of
     // both pipes so the child process can closed the unused ends.
 
-    UniqueChars argv2 = JS_smprintf("%d", stdIn.reader());
-    if (!argv2 || !argv.append(std::move(argv2))) {
-        return false;
-    }
-
-    UniqueChars argv3 = JS_smprintf("%d", stdIn.writer());
-    if (!argv3 || !argv.append(std::move(argv3))) {
-        return false;
-    }
-
-    UniqueChars argv4 = JS_smprintf("%d", stdOut.reader());
-    if (!argv4 || !argv.append(std::move(argv4))) {
-        return false;
-    }
-
-    UniqueChars argv5 = JS_smprintf("%d", stdOut.writer());
-    if (!argv5 || !argv.append(std::move(argv5))) {
-        return false;
-    }
-#endif
-
-    for (unsigned i = 0; i < sPropagatedFlags.length(); i++) {
-        UniqueChars flags = DuplicateString(cx, sPropagatedFlags[i]);
-        if (!flags || !argv.append(std::move(flags))) {
-            return false;
-        }
-    }
-
+    arg = JS_smprintf("%d", stdIn.reader());
+    if (!arg || !argv.append(std::move(arg))) {
+        return false;
+    }
+
+    arg = JS_smprintf("%d", stdIn.writer());
+    if (!arg || !argv.append(std::move(arg))) {
+        return false;
+    }
+
+    arg = JS_smprintf("%d", stdOut.reader());
+    if (!arg || !argv.append(std::move(arg))) {
+        return false;
+    }
+
+    arg = JS_smprintf("%d", stdOut.writer());
+    if (!arg || !argv.append(std::move(arg))) {
+        return false;
+    }
+#endif
+
+    // Required by both _spawnv and exec.
     if (!argv.append(nullptr)) {
         return false;
     }
 
 #ifdef XP_WIN
     if (!EscapeForShell(cx, argv)) {
         return false;
     }
@@ -6153,22 +6160,36 @@ static bool
 WasmCompileAndSerialize(JSContext* cx)
 {
     MOZ_ASSERT(wasm::HasCachingSupport(cx));
 
 #ifdef XP_WIN
     // See CompileAndSerializeInSeparateProcess for why we've had to smuggle
     // these fd values through argv. Closing the writing ends is necessary for
     // the reading ends to hit EOF.
-    MOZ_RELEASE_ASSERT(sArgc >= 6);
-    MOZ_ASSERT(!strcmp(sArgv[1], "--wasm-compile-and-serialize"));
-    int stdIn = atoi(sArgv[2]);   // stdIn.reader()
-    close(atoi(sArgv[3]));        // stdIn.writer()
-    close(atoi(sArgv[4]));        // stdOut.reader()
-    int stdOut = atoi(sArgv[5]);  // stdOut.writer()
+    int flagIndex = 0;
+    for (; flagIndex < sArgc; flagIndex++) {
+        if (!strcmp(sArgv[flagIndex], sWasmCompileAndSerializeFlag)) {
+            break;
+        }
+    }
+    MOZ_RELEASE_ASSERT(flagIndex < sArgc);
+
+    int fdsIndex = flagIndex + 1;
+    MOZ_RELEASE_ASSERT(fdsIndex + 4 == sArgc);
+
+    int stdInReader  = atoi(sArgv[fdsIndex + 0]);
+    int stdInWriter  = atoi(sArgv[fdsIndex + 1]);
+    int stdOutReader = atoi(sArgv[fdsIndex + 2]);
+    int stdOutWriter = atoi(sArgv[fdsIndex + 3]);
+
+    int stdIn = stdInReader;
+    close(stdInWriter);
+    close(stdOutReader);
+    int stdOut = stdOutWriter;
 #else
     int stdIn = STDIN_FILENO;
     int stdOut = STDOUT_FILENO;
 #endif
 
     wasm::MutableBytes bytecode = js_new<wasm::ShareableBytes>();
     if (!ReadAll(stdIn, &bytecode->bytes)) {
         return false;