Skip to content

Commit

Permalink
chore: [28-x-y] cherry-pick 3 changes from 3-M123
Browse files Browse the repository at this point in the history
* a65e511a14b4 from DirectXShaderCompiler
* f6672dbbe223 from angle
* 1b1f34234346 from chromium
  • Loading branch information
ppontes committed Apr 13, 2024
1 parent 9ec710e commit 42dcdbf
Show file tree
Hide file tree
Showing 8 changed files with 382 additions and 1 deletion.
Original file line number Diff line number Diff line change
@@ -1 +1,2 @@
fix_hlmatrixlowerpass_leaving_call_to_dangling_functionval.patch
cherry-pick-a65e511a14b4.patch
66 changes: 66 additions & 0 deletions patches/DirectXShaderCompiler/cherry-pick-a65e511a14b4.patch
Original file line number Diff line number Diff line change
@@ -0,0 +1,66 @@
From a65e511a14b4bffda1b24052732b09ca130359d1 Mon Sep 17 00:00:00 2001
From: Antonio Maiorano <amaiorano@google.com>
Date: Wed, 03 Apr 2024 15:58:51 -0400
Subject: [PATCH] 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 59f32a9..3f8ffdb 100644
--- a/lib/Transforms/Scalar/ScalarReplAggregatesHLSL.cpp
+++ b/lib/Transforms/Scalar/ScalarReplAggregatesHLSL.cpp
@@ -1003,9 +1003,11 @@
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 0000000..81adf71
--- /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();
+}
1 change: 1 addition & 0 deletions patches/angle/.patches
Original file line number Diff line number Diff line change
@@ -1 +1,2 @@
m123_vulkan_fix_access_to_inactive_attributes.patch
cherry-pick-f6672dbbe223.patch
269 changes: 269 additions & 0 deletions patches/angle/cherry-pick-f6672dbbe223.patch
Original file line number Diff line number Diff line change
@@ -0,0 +1,269 @@
From f6672dbbe223e68396d6dfab11edc342aa435719 Mon Sep 17 00:00:00 2001
From: Shahbaz Youssefi <syoussefi@chromium.org>
Date: Mon, 25 Mar 2024 14:46:56 -0400
Subject: [PATCH] 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 d8aec3c..ad39437 100644
--- a/src/compiler/translator/ParseContext.cpp
+++ b/src/compiler/translator/ParseContext.cpp
@@ -34,27 +34,39 @@

const int kWebGLMaxStructNesting = 4;

-bool ContainsSampler(const TStructure *structType);
-
-bool ContainsSampler(const TType &type)
+struct IsSamplerFunc
{
- if (IsSampler(type.getBasicType()))
+ bool operator()(TBasicType type) { return IsSampler(type); }
+};
+struct IsOpaqueFunc
+{
+ bool operator()(TBasicType type) { return IsOpaqueType(type); }
+};
+
+template <typename OpaqueFunc>
+bool ContainsOpaque(const TStructure *structType);
+
+template <typename OpaqueFunc>
+bool ContainsOpaque(const TType &type)
+{
+ 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;
@@ -1120,7 +1132,7 @@
{
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)";
@@ -4994,12 +5006,9 @@
{
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 1aaeaf8..2604bd7 100644
--- a/src/tests/gl_tests/GLSLTest.cpp
+++ b/src/tests/gl_tests/GLSLTest.cpp
@@ -6716,7 +6716,34 @@
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();
}

@@ -18430,6 +18457,116 @@
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 7b3e4eb..7666947 100644
--- a/src/tests/gl_tests/PixelLocalStorageTest.cpp
+++ b/src/tests/gl_tests/PixelLocalStorageTest.cpp
@@ -5574,8 +5574,7 @@
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();
}
1 change: 1 addition & 0 deletions patches/chromium/.patches
Original file line number Diff line number Diff line change
Expand Up @@ -151,3 +151,4 @@ update_crashpad_to_37afd37401253ebcebcf6e07ce15c8cfecb1a1cc.patch
m122_webcodecs_disable_async_videoframe_readback_to_mitigate_a.patch
fix_paintimage_deserialization_arbitrary-read_issue.patch
reland_sensors_winrt_call_onreadingchangedcallback_via.patch
cherry-pick-1b1f34234346.patch
43 changes: 43 additions & 0 deletions patches/chromium/cherry-pick-1b1f34234346.patch
Original file line number Diff line number Diff line change
@@ -0,0 +1,43 @@
From 1b1f34234346db1df8751e51c7a26c533b308fb4 Mon Sep 17 00:00:00 2001
From: kylechar <kylechar@chromium.org>
Date: Tue, 09 Apr 2024 17:14:26 +0000
Subject: [PATCH] [M120-LTS] Validate buffer length

The BitmapInSharedMemory mojo traits were only validating row length and
not total buffer length.

(cherry picked from commit 1a19ff70bd54847d818566bd7a1e7c384c419746)

(cherry picked from commit f15315f1cb7897e208947a40d538aac693283d7f)

Bug: 331237485
Change-Id: Ia2318899c44e9e7ac72fc7183954e6ce2c702179
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/5396796
Commit-Queue: Kyle Charbonneau <kylechar@chromium.org>
Cr-Original-Original-Commit-Position: refs/heads/main@{#1278417}
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/5420432
Commit-Queue: danakj <danakj@chromium.org>
Cr-Original-Commit-Position: refs/branch-heads/6312@{#786}
Cr-Original-Branched-From: 6711dcdae48edaf98cbc6964f90fac85b7d9986e-refs/heads/main@{#1262506}
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/5433678
Reviewed-by: danakj <danakj@chromium.org>
Reviewed-by: Kyle Charbonneau <kylechar@chromium.org>
Cr-Commit-Position: refs/branch-heads/6099@{#2003}
Cr-Branched-From: e6ee4500f7d6549a9ac1354f8d056da49ef406be-refs/heads/main@{#1217362}
---

diff --git a/services/viz/public/cpp/compositing/bitmap_in_shared_memory_mojom_traits.cc b/services/viz/public/cpp/compositing/bitmap_in_shared_memory_mojom_traits.cc
index a6e5f45..519d554 100644
--- a/services/viz/public/cpp/compositing/bitmap_in_shared_memory_mojom_traits.cc
+++ b/services/viz/public/cpp/compositing/bitmap_in_shared_memory_mojom_traits.cc
@@ -76,6 +76,10 @@
if (!mapping_ptr->IsValid())
return false;

+ if (mapping_ptr->size() < image_info.computeByteSize(data.row_bytes())) {
+ return false;
+ }
+
if (!sk_bitmap->installPixels(image_info, mapping_ptr->memory(),
data.row_bytes(), &DeleteSharedMemoryMapping,
mapping_ptr.get())) {
2 changes: 1 addition & 1 deletion patches/config.json
Original file line number Diff line number Diff line change
Expand Up @@ -13,5 +13,5 @@
{ "patch_dir": "src/electron/patches/webrtc", "repo": "src/third_party/webrtc" },
{ "patch_dir": "src/electron/patches/angle", "repo": "src/third_party/angle" },
{ "patch_dir": "src/electron/patches/libvpx", "repo": "src/third_party/libvpx/source/libvpx" },
{ "patch_dir": "src/electron/patches/dxc", "repo": "src/third_party/dawn/third_party/dxc" }
{ "patch_dir": "src/electron/patches/DirectXShaderCompiler", "repo": "src/third_party/dawn/third_party/dxc" }
]

0 comments on commit 42dcdbf

Please sign in to comment.