Commit
This commit does not belong to any branch on this repository, and may belong to a fork outside of the repository.
chore: cherry-pick 8 changes from 3-M123 and M1nn (#41856)
* chore: [27-x-y] cherry-pick 3 changes from 3-M123 * a65e511a14b4 from DirectXShaderCompiler * f6672dbbe223 from angle * 1b1f34234346 from chromium * chore: [27-x-y] cherry-pick 4 later changes from M1nn * chore: update patches --------- Co-authored-by: PatchUp <73610968+patchup[bot]@users.noreply.github.com>
- Loading branch information
1 parent
6d5b65e
commit 70a6658
Showing
15 changed files
with
2,148 additions
and
1 deletion.
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,2 @@ | ||
fix_hlmatrixlowerpass_leaving_call_to_dangling_functionval.patch | ||
cherry-pick-a65e511a14b4.patch |
66 changes: 66 additions & 0 deletions
66
patches/DirectXShaderCompiler/cherry-pick-a65e511a14b4.patch
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,66 @@ | ||
From 0000000000000000000000000000000000000000 Mon Sep 17 00:00:00 2001 | ||
From: Antonio Maiorano <amaiorano@google.com> | ||
Date: Wed, 3 Apr 2024 15:58:51 -0400 | ||
Subject: Fix ASAN use-after-free on unreferenced self-assignment of struct | ||
instance (#6466) | ||
|
||
When deleting an unused memcpy, ScalarReplAggregatesHLSL was attempting | ||
to delete both the target and the source of the memcpy without first | ||
checking if they were both same, resulting in a double-delete. | ||
|
||
Bug: chromium:331123811 | ||
Change-Id: Idaef95a06b10a7fb6f0ca2e662972a44ec662fbc | ||
Reviewed-on: https://chromium-review.googlesource.com/c/external/github.com/microsoft/DirectXShaderCompiler/+/5419225 | ||
Reviewed-by: David Neto <dneto@google.com> | ||
Reviewed-by: dan sinclair <dsinclair@chromium.org> | ||
Reviewed-by: Ben Clayton <bclayton@chromium.org> | ||
|
||
diff --git a/lib/Transforms/Scalar/ScalarReplAggregatesHLSL.cpp b/lib/Transforms/Scalar/ScalarReplAggregatesHLSL.cpp | ||
index b3589884aa3b690e1aad5ca7a19218a222591d39..8e2eff7f69ec378393c7a9afc4040cd9e921c42d 100644 | ||
--- a/lib/Transforms/Scalar/ScalarReplAggregatesHLSL.cpp | ||
+++ b/lib/Transforms/Scalar/ScalarReplAggregatesHLSL.cpp | ||
@@ -989,9 +989,11 @@ void DeleteMemcpy(MemCpyInst *MI) { | ||
if (op0->user_empty()) | ||
op0->eraseFromParent(); | ||
} | ||
- if (Instruction *op1 = dyn_cast<Instruction>(Op1)) { | ||
- if (op1->user_empty()) | ||
- op1->eraseFromParent(); | ||
+ if (Op0 != Op1) { | ||
+ if (Instruction *op1 = dyn_cast<Instruction>(Op1)) { | ||
+ if (op1->user_empty()) | ||
+ op1->eraseFromParent(); | ||
+ } | ||
} | ||
} | ||
|
||
diff --git a/tools/clang/test/DXC/unreferenced_struct_selft_assignment_crash.hlsl b/tools/clang/test/DXC/unreferenced_struct_selft_assignment_crash.hlsl | ||
new file mode 100644 | ||
index 0000000000000000000000000000000000000000..81adf71867c9868992372e12dc1ba81aebb48344 | ||
--- /dev/null | ||
+++ b/tools/clang/test/DXC/unreferenced_struct_selft_assignment_crash.hlsl | ||
@@ -0,0 +1,24 @@ | ||
+// RUN: %dxc -T cs_6_0 %s | FileCheck %s | ||
+ | ||
+// Validate that self-assignment of a static struct instance that is not | ||
+// referenced does not crash the compiler. This was resulting in an ASAN | ||
+// use-after-free in ScalarReplAggregatesHLSL because DeleteMemcpy would | ||
+// attempt to delete both source and target, even if both were the same. | ||
+// CHECK: define void @main() { | ||
+// CHECK-NEXT: ret void | ||
+// CHECK-NEXT: } | ||
+ | ||
+struct MyStruct { | ||
+ int m0; | ||
+}; | ||
+ | ||
+static MyStruct s; | ||
+ | ||
+void foo() { | ||
+ s = s; | ||
+} | ||
+ | ||
+[numthreads(1, 1, 1)] | ||
+void main() { | ||
+ foo(); | ||
+} |
File renamed without changes.
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,267 @@ | ||
From 0000000000000000000000000000000000000000 Mon Sep 17 00:00:00 2001 | ||
From: Shahbaz Youssefi <syoussefi@chromium.org> | ||
Date: Mon, 25 Mar 2024 14:46:56 -0400 | ||
Subject: M123: Translator: Disallow samplers in structs in interface blocks | ||
|
||
As disallowed by the spec: | ||
|
||
> Types and declarators are the same as for other uniform variable | ||
> declarations outside blocks, with these exceptions: | ||
> | ||
> * opaque types are not allowed | ||
|
||
Bug: chromium:328859176 | ||
Change-Id: Ib94977860102329e520e635c3757827c93ca2163 | ||
Reviewed-on: https://chromium-review.googlesource.com/c/angle/angle/+/5391986 | ||
Auto-Submit: Shahbaz Youssefi <syoussefi@chromium.org> | ||
Reviewed-by: Geoff Lang <geofflang@chromium.org> | ||
Commit-Queue: Shahbaz Youssefi <syoussefi@chromium.org> | ||
(cherry picked from commit a0fa06f6d79ced897c0fe2795551268199d29806) | ||
Reviewed-on: https://chromium-review.googlesource.com/c/angle/angle/+/5435737 | ||
Reviewed-by: Yuly Novikov <ynovikov@chromium.org> | ||
|
||
diff --git a/src/compiler/translator/ParseContext.cpp b/src/compiler/translator/ParseContext.cpp | ||
index e174725beb764407185e471a9916ffd164493cd8..cb9eb3a4a566348f11aa5962037164147bc65684 100644 | ||
--- a/src/compiler/translator/ParseContext.cpp | ||
+++ b/src/compiler/translator/ParseContext.cpp | ||
@@ -34,27 +34,39 @@ namespace | ||
|
||
const int kWebGLMaxStructNesting = 4; | ||
|
||
-bool ContainsSampler(const TStructure *structType); | ||
+struct IsSamplerFunc | ||
+{ | ||
+ bool operator()(TBasicType type) { return IsSampler(type); } | ||
+}; | ||
+struct IsOpaqueFunc | ||
+{ | ||
+ bool operator()(TBasicType type) { return IsOpaqueType(type); } | ||
+}; | ||
+ | ||
+template <typename OpaqueFunc> | ||
+bool ContainsOpaque(const TStructure *structType); | ||
|
||
-bool ContainsSampler(const TType &type) | ||
+template <typename OpaqueFunc> | ||
+bool ContainsOpaque(const TType &type) | ||
{ | ||
- if (IsSampler(type.getBasicType())) | ||
+ if (OpaqueFunc{}(type.getBasicType())) | ||
{ | ||
return true; | ||
} | ||
if (type.getBasicType() == EbtStruct) | ||
{ | ||
- return ContainsSampler(type.getStruct()); | ||
+ return ContainsOpaque<OpaqueFunc>(type.getStruct()); | ||
} | ||
|
||
return false; | ||
} | ||
|
||
-bool ContainsSampler(const TStructure *structType) | ||
+template <typename OpaqueFunc> | ||
+bool ContainsOpaque(const TStructure *structType) | ||
{ | ||
for (const auto &field : structType->fields()) | ||
{ | ||
- if (ContainsSampler(*field->type())) | ||
+ if (ContainsOpaque<OpaqueFunc>(*field->type())) | ||
return true; | ||
} | ||
return false; | ||
@@ -1057,7 +1069,7 @@ bool TParseContext::checkIsNotOpaqueType(const TSourceLoc &line, | ||
{ | ||
if (pType.type == EbtStruct) | ||
{ | ||
- if (ContainsSampler(pType.userDef)) | ||
+ if (ContainsOpaque<IsSamplerFunc>(pType.userDef)) | ||
{ | ||
std::stringstream reasonStream = sh::InitializeStream<std::stringstream>(); | ||
reasonStream << reason << " (structure contains a sampler)"; | ||
@@ -4923,12 +4935,9 @@ TIntermDeclaration *TParseContext::addInterfaceBlock( | ||
{ | ||
TField *field = (*fieldList)[memberIndex]; | ||
TType *fieldType = field->type(); | ||
- if (IsOpaqueType(fieldType->getBasicType())) | ||
+ if (ContainsOpaque<IsOpaqueFunc>(*fieldType)) | ||
{ | ||
- std::string reason("unsupported type - "); | ||
- reason += fieldType->getBasicString(); | ||
- reason += " types are not allowed in interface blocks"; | ||
- error(field->line(), reason.c_str(), fieldType->getBasicString()); | ||
+ error(field->line(), "Opaque types are not allowed in interface blocks", blockName); | ||
} | ||
|
||
const TQualifier qualifier = fieldType->getQualifier(); | ||
diff --git a/src/tests/gl_tests/GLSLTest.cpp b/src/tests/gl_tests/GLSLTest.cpp | ||
index f4b7ce6222ff4a29cb20b75bc102e2c8ae478189..8ac5b758b128ded933d727f7dccb0ce8f8eb338b 100644 | ||
--- a/src/tests/gl_tests/GLSLTest.cpp | ||
+++ b/src/tests/gl_tests/GLSLTest.cpp | ||
@@ -6716,7 +6716,34 @@ void main() | ||
gl_FragColor = vec4(f(us), 0, 0, 1); | ||
})"; | ||
|
||
- CompileShader(GL_FRAGMENT_SHADER, kFS); | ||
+ GLuint fs = CompileShader(GL_FRAGMENT_SHADER, kFS); | ||
+ EXPECT_NE(fs, 0u); | ||
+ ASSERT_GL_NO_ERROR(); | ||
+} | ||
+ | ||
+// Test that structs with samplers are not allowed in interface blocks. This is forbidden per | ||
+// GLES3: | ||
+// | ||
+// > Types and declarators are the same as for other uniform variable declarations outside blocks, | ||
+// > with these exceptions: | ||
+// > * opaque types are not allowed | ||
+TEST_P(GLSLTest_ES3, StructWithSamplersDisallowedInInterfaceBlock) | ||
+{ | ||
+ const char kFS[] = R"(#version 300 es | ||
+precision mediump float; | ||
+struct S { sampler2D samp; bool b; }; | ||
+ | ||
+layout(std140) uniform Buffer { S s; } buffer; | ||
+ | ||
+out vec4 color; | ||
+ | ||
+void main() | ||
+{ | ||
+ color = texture(buffer.s.samp, vec2(0)); | ||
+})"; | ||
+ | ||
+ GLuint fs = CompileShader(GL_FRAGMENT_SHADER, kFS); | ||
+ EXPECT_EQ(fs, 0u); | ||
ASSERT_GL_NO_ERROR(); | ||
} | ||
|
||
@@ -18195,6 +18222,116 @@ void main() { | ||
EXPECT_EQ(0u, shader); | ||
} | ||
|
||
+// Same as TooManyFieldsInStruct, but with samplers in the struct. | ||
+TEST_P(GLSLTest_ES3, TooManySamplerFieldsInStruct) | ||
+{ | ||
+ std::ostringstream fs; | ||
+ fs << R"(#version 300 es | ||
+precision highp float; | ||
+struct TooManyFields | ||
+{ | ||
+)"; | ||
+ for (uint32_t i = 0; i < (1 << 16); ++i) | ||
+ { | ||
+ fs << " sampler2D field" << i << ";\n"; | ||
+ } | ||
+ fs << R"(}; | ||
+uniform TooManyFields s; | ||
+out vec4 color; | ||
+void main() { | ||
+ color = texture(s.field0, vec2(0)); | ||
+})"; | ||
+ | ||
+ GLuint shader = CompileShader(GL_FRAGMENT_SHADER, fs.str().c_str()); | ||
+ EXPECT_EQ(0u, shader); | ||
+} | ||
+ | ||
+// More complex variation of ManySamplerFieldsInStruct. This one compiles fine. | ||
+TEST_P(GLSLTest_ES3, ManySamplerFieldsInStructComplex) | ||
+{ | ||
+ // D3D and OpenGL may be more restrictive about this many samplers. | ||
+ ANGLE_SKIP_TEST_IF(IsD3D() || IsOpenGL()); | ||
+ | ||
+ std::ostringstream fs; | ||
+ fs << R"(#version 300 es | ||
+precision highp float; | ||
+ | ||
+struct X { | ||
+ mediump sampler2D a[0xf00]; | ||
+ mediump sampler2D b[0xf00]; | ||
+ mediump sampler2D c[0xf000]; | ||
+ mediump sampler2D d[0xf00]; | ||
+}; | ||
+ | ||
+struct Y { | ||
+ X s1; | ||
+ mediump sampler2D a[0xf00]; | ||
+ mediump sampler2D b[0xf000]; | ||
+ mediump sampler2D c[0x14000]; | ||
+}; | ||
+ | ||
+struct S { | ||
+ Y s1; | ||
+}; | ||
+ | ||
+struct structBuffer { S s; }; | ||
+ | ||
+uniform structBuffer b; | ||
+ | ||
+out vec4 color; | ||
+void main() | ||
+{ | ||
+ color = texture(b.s.s1.s1.c[0], vec2(0)); | ||
+})"; | ||
+ | ||
+ GLuint shader = CompileShader(GL_FRAGMENT_SHADER, fs.str().c_str()); | ||
+ EXPECT_NE(0u, shader); | ||
+} | ||
+ | ||
+// Make sure a large array of samplers works. | ||
+TEST_P(GLSLTest, ManySamplers) | ||
+{ | ||
+ // D3D and OpenGL may be more restrictive about this many samplers. | ||
+ ANGLE_SKIP_TEST_IF(IsD3D() || IsOpenGL()); | ||
+ | ||
+ std::ostringstream fs; | ||
+ fs << R"(precision highp float; | ||
+ | ||
+uniform mediump sampler2D c[0x12000]; | ||
+ | ||
+void main() | ||
+{ | ||
+ gl_FragColor = texture2D(c[0], vec2(0)); | ||
+})"; | ||
+ | ||
+ GLuint shader = CompileShader(GL_FRAGMENT_SHADER, fs.str().c_str()); | ||
+ EXPECT_NE(0u, shader); | ||
+} | ||
+ | ||
+// Make sure a large array of samplers works when declared in a struct. | ||
+TEST_P(GLSLTest, ManySamplersInStruct) | ||
+{ | ||
+ // D3D and OpenGL may be more restrictive about this many samplers. | ||
+ ANGLE_SKIP_TEST_IF(IsD3D() || IsOpenGL()); | ||
+ | ||
+ std::ostringstream fs; | ||
+ fs << R"(precision highp float; | ||
+ | ||
+struct X { | ||
+ mediump sampler2D c[0x12000]; | ||
+}; | ||
+ | ||
+uniform X x; | ||
+ | ||
+void main() | ||
+{ | ||
+ gl_FragColor = texture2D(x.c[0], vec2(0)); | ||
+})"; | ||
+ | ||
+ GLuint shader = CompileShader(GL_FRAGMENT_SHADER, fs.str().c_str()); | ||
+ EXPECT_NE(0u, shader); | ||
+} | ||
+ | ||
// Test that passing large arrays to functions are compiled correctly. Regression test for the | ||
// SPIR-V generator that made a copy of the array to pass to the function, by decomposing and | ||
// reconstructing it (in the absence of OpCopyLogical), but the reconstruction instruction has a | ||
diff --git a/src/tests/gl_tests/PixelLocalStorageTest.cpp b/src/tests/gl_tests/PixelLocalStorageTest.cpp | ||
index c49ba5741ad565ad9637fb2188a472ccbebc6284..126936271eb25eec601349a560fabc6f0f7d4b75 100644 | ||
--- a/src/tests/gl_tests/PixelLocalStorageTest.cpp | ||
+++ b/src/tests/gl_tests/PixelLocalStorageTest.cpp | ||
@@ -5573,8 +5573,7 @@ TEST_P(PixelLocalStorageCompilerTest, Declarations) | ||
EXPECT_FALSE(log.compileFragmentShader(kPLSInStruct)); | ||
EXPECT_TRUE(log.has("ERROR: 0:5: 'pixelLocalANGLE' : disallowed type in struct")); | ||
EXPECT_TRUE( | ||
- log.has("ERROR: 0:10: 'pixelLocalANGLE' : unsupported type - pixelLocalANGLE types are not " | ||
- "allowed in interface blocks")); | ||
+ log.has("ERROR: 0:10: 'PLSBlock' : Opaque types are not allowed in interface blocks")); | ||
|
||
ASSERT_GL_NO_ERROR(); | ||
} |
Oops, something went wrong.