[WHLSL] Code that accesses an undefined variable crashes
authorsbarati@apple.com <sbarati@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Fri, 21 Jun 2019 22:14:56 +0000 (22:14 +0000)
committersbarati@apple.com <sbarati@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Fri, 21 Jun 2019 22:14:56 +0000 (22:14 +0000)
https://bugs.webkit.org/show_bug.cgi?id=198775

Reviewed by Myles C. Maxfield.

Source/WebCore:

Myles mostly fixed this in r246631 when he made NameResolver propagate
its error to its parent NameResolver. However, there was still one bug
where we ended up calling setError twice for an if statement. This patch
fixes that and adds tests.

Tests: webgpu/whlsl-use-undefined-variable-2.html
       webgpu/whlsl-use-undefined-variable.html

* Modules/webgpu/WHLSL/WHLSLNameResolver.cpp:
(WebCore::WHLSL::NameResolver::visit):

LayoutTests:

* webgpu/whlsl-use-undefined-variable-2-expected.txt: Added.
* webgpu/whlsl-use-undefined-variable-2.html: Added.
* webgpu/whlsl-use-undefined-variable-expected.txt: Added.
* webgpu/whlsl-use-undefined-variable.html: Added.

git-svn-id: https://svn.webkit.org/repository/webkit/trunk@246700 268f45cc-cd09-0410-ab3c-d52691b4dbfc

LayoutTests/ChangeLog
LayoutTests/webgpu/whlsl-use-undefined-variable-2-expected.txt [new file with mode: 0644]
LayoutTests/webgpu/whlsl-use-undefined-variable-2.html [new file with mode: 0644]
LayoutTests/webgpu/whlsl-use-undefined-variable-expected.txt [new file with mode: 0644]
LayoutTests/webgpu/whlsl-use-undefined-variable.html [new file with mode: 0644]
Source/WebCore/ChangeLog
Source/WebCore/Modules/webgpu/WHLSL/WHLSLNameResolver.cpp

index bc1b991..96ab87c 100644 (file)
@@ -1,3 +1,15 @@
+2019-06-21  Saam Barati  <sbarati@apple.com>
+
+        [WHLSL] Code that accesses an undefined variable crashes
+        https://bugs.webkit.org/show_bug.cgi?id=198775
+
+        Reviewed by Myles C. Maxfield.
+
+        * webgpu/whlsl-use-undefined-variable-2-expected.txt: Added.
+        * webgpu/whlsl-use-undefined-variable-2.html: Added.
+        * webgpu/whlsl-use-undefined-variable-expected.txt: Added.
+        * webgpu/whlsl-use-undefined-variable.html: Added.
+
 2019-06-21  Truitt Savell  <tsavell@apple.com>
 
         Unreviewed, rolling out r246611.
diff --git a/LayoutTests/webgpu/whlsl-use-undefined-variable-2-expected.txt b/LayoutTests/webgpu/whlsl-use-undefined-variable-2-expected.txt
new file mode 100644 (file)
index 0000000..dd4c6dc
--- /dev/null
@@ -0,0 +1,5 @@
+PASS Should not crash.
+PASS successfullyParsed is true
+
+TEST COMPLETE
+
diff --git a/LayoutTests/webgpu/whlsl-use-undefined-variable-2.html b/LayoutTests/webgpu/whlsl-use-undefined-variable-2.html
new file mode 100644 (file)
index 0000000..9f96c90
--- /dev/null
@@ -0,0 +1,43 @@
+<!DOCTYPE html>
+<html>
+<head>
+<script src="../resources/js-test-pre.js"></script>
+</head>
+<body>
+<script>
+const shaderSource = `
+[numthreads(1, 1, 1)]
+compute void computeShader(device float[] buffer : register(u0), float3 threadID : SV_DispatchThreadID) {
+    float4 vec = float4(x,x,x,x);
+}
+`;
+let resultsFloat32Array;
+async function start() {
+    const adapter = await navigator.gpu.requestAdapter();
+    const device = await adapter.requestDevice();
+
+    const shaderModule = device.createShaderModule({code: shaderSource, isWHLSL: true});
+    const computeStage = {module: shaderModule, entryPoint: "computeShader"};
+
+    const bindGroupLayoutDescriptor = {bindings: [{binding: 0, visibility: 7, type: "storage-buffer"}]};
+    const bindGroupLayout = device.createBindGroupLayout(bindGroupLayoutDescriptor);
+    const pipelineLayoutDescriptor = {bindGroupLayouts: [bindGroupLayout]};
+    const pipelineLayout = device.createPipelineLayout(pipelineLayoutDescriptor);
+
+    const computePipelineDescriptor = {computeStage, layout: pipelineLayout};
+    const computePipeline = device.createComputePipeline(computePipelineDescriptor);
+
+    testPassed("Should not crash.");
+}
+window.jsTestIsAsync = true;
+window.addEventListener("load", function() {
+    start().then(function() {
+        finishJSTest();
+    }, function() {
+        finishJSTest();
+    });
+});
+</script>
+<script src="../resources/js-test-post.js"></script>
+</body>
+</html>
diff --git a/LayoutTests/webgpu/whlsl-use-undefined-variable-expected.txt b/LayoutTests/webgpu/whlsl-use-undefined-variable-expected.txt
new file mode 100644 (file)
index 0000000..dd4c6dc
--- /dev/null
@@ -0,0 +1,5 @@
+PASS Should not crash.
+PASS successfullyParsed is true
+
+TEST COMPLETE
+
diff --git a/LayoutTests/webgpu/whlsl-use-undefined-variable.html b/LayoutTests/webgpu/whlsl-use-undefined-variable.html
new file mode 100644 (file)
index 0000000..1470dc5
--- /dev/null
@@ -0,0 +1,46 @@
+<!DOCTYPE html>
+<html>
+<head>
+<script src="../resources/js-test-pre.js"></script>
+</head>
+<body>
+<script>
+const shaderSource = `
+[numthreads(1, 1, 1)]
+compute void computeShader(device float[] buffer : register(u0), float3 threadID : SV_DispatchThreadID) {
+    float foo;
+    if (foo) {
+        x = x + 1;
+    }
+}
+`;
+let resultsFloat32Array;
+async function start() {
+    const adapter = await navigator.gpu.requestAdapter();
+    const device = await adapter.requestDevice();
+
+    const shaderModule = device.createShaderModule({code: shaderSource, isWHLSL: true});
+    const computeStage = {module: shaderModule, entryPoint: "computeShader"};
+
+    const bindGroupLayoutDescriptor = {bindings: [{binding: 0, visibility: 7, type: "storage-buffer"}]};
+    const bindGroupLayout = device.createBindGroupLayout(bindGroupLayoutDescriptor);
+    const pipelineLayoutDescriptor = {bindGroupLayouts: [bindGroupLayout]};
+    const pipelineLayout = device.createPipelineLayout(pipelineLayoutDescriptor);
+
+    const computePipelineDescriptor = {computeStage, layout: pipelineLayout};
+    const computePipeline = device.createComputePipeline(computePipelineDescriptor);
+
+    testPassed("Should not crash.");
+}
+window.jsTestIsAsync = true;
+window.addEventListener("load", function() {
+    start().then(function() {
+        finishJSTest();
+    }, function() {
+        finishJSTest();
+    });
+});
+</script>
+<script src="../resources/js-test-post.js"></script>
+</body>
+</html>
index a83fc6c..2e9b173 100644 (file)
@@ -1,3 +1,21 @@
+2019-06-21  Saam Barati  <sbarati@apple.com>
+
+        [WHLSL] Code that accesses an undefined variable crashes
+        https://bugs.webkit.org/show_bug.cgi?id=198775
+
+        Reviewed by Myles C. Maxfield.
+
+        Myles mostly fixed this in r246631 when he made NameResolver propagate
+        its error to its parent NameResolver. However, there was still one bug
+        where we ended up calling setError twice for an if statement. This patch
+        fixes that and adds tests.
+
+        Tests: webgpu/whlsl-use-undefined-variable-2.html
+               webgpu/whlsl-use-undefined-variable.html
+
+        * Modules/webgpu/WHLSL/WHLSLNameResolver.cpp:
+        (WebCore::WHLSL::NameResolver::visit):
+
 2019-06-21  Truitt Savell  <tsavell@apple.com>
 
         Unreviewed, rolling out r246611.
index d875da8..016b855 100644 (file)
@@ -120,12 +120,18 @@ void NameResolver::visit(AST::Block& block)
 void NameResolver::visit(AST::IfStatement& ifStatement)
 {
     checkErrorAndVisit(ifStatement.conditional());
-    NameContext nameContext(&m_nameContext);
-    NameResolver newNameResolver(*this, nameContext);
-    newNameResolver.checkErrorAndVisit(ifStatement.body());
-    if (newNameResolver.error())
-        setError();
-    else if (ifStatement.elseBody()) {
+    if (error())
+        return;
+
+    {
+        NameContext nameContext(&m_nameContext);
+        NameResolver newNameResolver(*this, nameContext);
+        newNameResolver.checkErrorAndVisit(ifStatement.body());
+    }
+    if (error())
+        return;
+
+    if (ifStatement.elseBody()) {
         NameContext nameContext(&m_nameContext);
         NameResolver newNameResolver(*this, nameContext);
         newNameResolver.checkErrorAndVisit(*ifStatement.elseBody());
@@ -135,6 +141,9 @@ void NameResolver::visit(AST::IfStatement& ifStatement)
 void NameResolver::visit(AST::WhileLoop& whileLoop)
 {
     checkErrorAndVisit(whileLoop.conditional());
+    if (error())
+        return;
+
     NameContext nameContext(&m_nameContext);
     NameResolver newNameResolver(*this, nameContext);
     newNameResolver.checkErrorAndVisit(whileLoop.body());
@@ -142,9 +151,12 @@ void NameResolver::visit(AST::WhileLoop& whileLoop)
 
 void NameResolver::visit(AST::DoWhileLoop& whileLoop)
 {
-    NameContext nameContext(&m_nameContext);
-    NameResolver newNameResolver(*this, nameContext);
-    newNameResolver.checkErrorAndVisit(whileLoop.body());
+    {
+        NameContext nameContext(&m_nameContext);
+        NameResolver newNameResolver(*this, nameContext);
+        newNameResolver.checkErrorAndVisit(whileLoop.body());
+    }
+
     checkErrorAndVisit(whileLoop.conditional());
 }