chore: cherry-pick 8 changes from Release-3-M120 (#40901)

* chore: [26-x-y] cherry-pick 5 changes from Release-3-M120

* 5b2fddadaa12 from chromium
* cd9486849ba3 from sqlite
* 50a1bddfca85 from chromium
* 0c1d249c3fe2 from angle
* 01f439363dcb from angle

* chore: [26-x-y] remove unused and unnecessary patches.

* chore: update patches

---------

Co-authored-by: PatchUp <73610968+patchup[bot]@users.noreply.github.com>
This commit is contained in:
Pedro Pontes
2024-01-08 12:58:45 -08:00
committed by GitHub
parent 0d99c8a222
commit 6cdb2fb3af
16 changed files with 1573 additions and 640 deletions

View File

@@ -1,3 +1,4 @@
cherry-pick-285c7712c506.patch
cherry-pick-2bf945775fe6.patch
cherry-pick-cafe56b591ed.patch
m120_translator_optimize_field-name-collision_check.patch
m120_translator_fail_compilation_if_too_many_struct_fields.patch
m120_translator_limit_private_variable_size_to_64kb.patch
m120_vulkan_don_t_crash_when_glcopyteximage2d_redefines_itself.patch

View File

@@ -1,153 +0,0 @@
From 285c7712c50654e3d7238b059c4631bc91285514 Mon Sep 17 00:00:00 2001
From: Shahbaz Youssefi <syoussefi@chromium.org>
Date: Thu, 13 Jul 2023 15:23:49 -0400
Subject: [PATCH] M116: Translator: Unconditionally limit variable sizes
... instead of just for WebGL. This is to avoid hitting driver bugs
that were prevented with this check for WebGL on a compromised renderer
that can create non-WebGL contexts.
Bug: chromium:1464682
Change-Id: I2b1c5a8c51f06225f5f850109d30778d97e574c7
Reviewed-on: https://chromium-review.googlesource.com/c/angle/angle/+/4717371
Reviewed-by: Roman Lavrov <romanl@google.com>
---
diff --git a/src/compiler/translator/Compiler.cpp b/src/compiler/translator/Compiler.cpp
index 7b1ac4e..383feeb 100644
--- a/src/compiler/translator/Compiler.cpp
+++ b/src/compiler/translator/Compiler.cpp
@@ -397,9 +397,10 @@
bool TCompiler::shouldLimitTypeSizes() const
{
- // WebGL shaders limit the size of variables' types in shaders,
- // including arrays, structs and interface blocks.
- return IsWebGLBasedSpec(mShaderSpec);
+ // Prevent unrealistically large variable sizes in shaders. This works around driver bugs
+ // around int-size limits (such as 2GB). The limits are generously large enough that no real
+ // shader should ever hit it.
+ return true;
}
bool TCompiler::Init(const ShBuiltInResources &resources)
diff --git a/src/compiler/translator/ValidateTypeSizeLimitations.cpp b/src/compiler/translator/ValidateTypeSizeLimitations.cpp
index 2a033ad..19a4821 100644
--- a/src/compiler/translator/ValidateTypeSizeLimitations.cpp
+++ b/src/compiler/translator/ValidateTypeSizeLimitations.cpp
@@ -23,10 +23,10 @@
// Arbitrarily enforce that all types declared with a size in bytes of over 2 GB will cause
// compilation failure.
//
-// For local and global variables, the limit is much lower (1MB) as that much memory won't fit in
+// For local and global variables, the limit is much lower (16MB) as that much memory won't fit in
// the GPU registers anyway.
constexpr size_t kMaxVariableSizeInBytes = static_cast<size_t>(2) * 1024 * 1024 * 1024;
-constexpr size_t kMaxPrivateVariableSizeInBytes = static_cast<size_t>(1) * 1024 * 1024;
+constexpr size_t kMaxPrivateVariableSizeInBytes = static_cast<size_t>(16) * 1024 * 1024;
// Traverses intermediate tree to ensure that the shader does not
// exceed certain implementation-defined limits on the sizes of types.
diff --git a/src/compiler/translator/util.cpp b/src/compiler/translator/util.cpp
index a91f8b0..a866b25 100644
--- a/src/compiler/translator/util.cpp
+++ b/src/compiler/translator/util.cpp
@@ -282,6 +282,9 @@
return kBoolGLType[type.getNominalSize() - 1];
+ case EbtYuvCscStandardEXT:
+ return GL_UNSIGNED_INT;
+
case EbtSampler2D:
return GL_SAMPLER_2D;
case EbtSampler3D:
diff --git a/src/tests/gl_tests/WebGLCompatibilityTest.cpp b/src/tests/gl_tests/WebGLCompatibilityTest.cpp
index 9ae56f5..a8d2ce4 100644
--- a/src/tests/gl_tests/WebGLCompatibilityTest.cpp
+++ b/src/tests/gl_tests/WebGLCompatibilityTest.cpp
@@ -5284,8 +5284,8 @@
constexpr char kVSArrayTooLarge[] =
R"(varying vec4 color;
-// 1 MB / 32 aligned bytes per mat2 = 32768
-const int array_size = 32769;
+// 16 MB / 32 aligned bytes per mat2 = 524288
+const int array_size = 524289;
void main()
{
mat2 array[array_size];
@@ -5297,7 +5297,7 @@
constexpr char kVSArrayMuchTooLarge[] =
R"(varying vec4 color;
-const int array_size = 55600;
+const int array_size = 757000;
void main()
{
mat2 array[array_size];
@@ -5361,9 +5361,9 @@
constexpr char kTooLargeGlobalMemory1[] =
R"(precision mediump float;
-// 1 MB / 16 bytes per vec4 = 65536
-vec4 array[32768];
-vec4 array2[32769];
+// 16 MB / 16 bytes per vec4 = 1048576
+vec4 array[524288];
+vec4 array2[524289];
void main()
{
@@ -5376,9 +5376,9 @@
constexpr char kTooLargeGlobalMemory2[] =
R"(precision mediump float;
-// 1 MB / 16 bytes per vec4 = 65536
-vec4 array[32767];
-vec4 array2[32767];
+// 16 MB / 16 bytes per vec4 = 1048576
+vec4 array[524287];
+vec4 array2[524287];
vec4 x, y, z;
void main()
@@ -5392,12 +5392,12 @@
constexpr char kTooLargeGlobalAndLocalMemory1[] =
R"(precision mediump float;
-// 1 MB / 16 bytes per vec4 = 65536
-vec4 array[32768];
+// 16 MB / 16 bytes per vec4 = 1048576
+vec4 array[524288];
void main()
{
- vec4 array2[32769];
+ vec4 array2[524289];
if (array[0].x + array[1].x == 2.0)
gl_FragColor = vec4(0.0, 1.0, 0.0, 1.0);
else
@@ -5408,18 +5408,18 @@
constexpr char kTooLargeGlobalAndLocalMemory2[] =
R"(precision mediump float;
-// 1 MB / 16 bytes per vec4 = 65536
-vec4 array[32768];
+// 16 MB / 16 bytes per vec4 = 1048576
+vec4 array[524288];
float f()
{
- vec4 array2[16384];
+ vec4 array2[524288];
return array2[0].x;
}
float g()
{
- vec4 array3[16383];
+ vec4 array3[524287];
return array3[0].x;
}

View File

@@ -1,197 +0,0 @@
From 2bf945775fe634eb9e420c2263dae6043bbb5ece Mon Sep 17 00:00:00 2001
From: Shahbaz Youssefi <syoussefi@chromium.org>
Date: Fri, 14 Jul 2023 12:30:15 -0400
Subject: [PATCH] M116: Translator: Limit variable sizes vs uint overflow
Bug: chromium:1464680
Change-Id: Iee41a2da7a7a330e6cc4d6da59a6e9836ee9dd36
Reviewed-on: https://chromium-review.googlesource.com/c/angle/angle/+/4717372
Reviewed-by: Roman Lavrov <romanl@google.com>
---
diff --git a/src/compiler/translator/ValidateTypeSizeLimitations.cpp b/src/compiler/translator/ValidateTypeSizeLimitations.cpp
index 19a4821..f0ff9cb 100644
--- a/src/compiler/translator/ValidateTypeSizeLimitations.cpp
+++ b/src/compiler/translator/ValidateTypeSizeLimitations.cpp
@@ -7,6 +7,7 @@
#include "compiler/translator/ValidateTypeSizeLimitations.h"
#include "angle_gl.h"
+#include "common/mathutil.h"
#include "compiler/translator/Diagnostics.h"
#include "compiler/translator/Symbol.h"
#include "compiler/translator/SymbolTable.h"
@@ -113,7 +114,8 @@
void validateTotalPrivateVariableSize()
{
- if (mTotalPrivateVariablesSize > kMaxPrivateVariableSizeInBytes)
+ if (mTotalPrivateVariablesSize.ValueOrDefault(std::numeric_limits<size_t>::max()) >
+ kMaxPrivateVariableSizeInBytes)
{
mDiagnostics->error(
TSourceLoc{},
@@ -231,7 +233,7 @@
TDiagnostics *mDiagnostics;
std::vector<int> mLoopSymbolIds;
- size_t mTotalPrivateVariablesSize;
+ angle::base::CheckedNumeric<size_t> mTotalPrivateVariablesSize;
};
} // namespace
diff --git a/src/tests/gl_tests/WebGLCompatibilityTest.cpp b/src/tests/gl_tests/WebGLCompatibilityTest.cpp
index a8d2ce4..542d49f 100644
--- a/src/tests/gl_tests/WebGLCompatibilityTest.cpp
+++ b/src/tests/gl_tests/WebGLCompatibilityTest.cpp
@@ -5426,7 +5426,7 @@
float h()
{
vec4 value;
- float value2
+ float value2;
return value.x + value2;
}
@@ -5438,6 +5438,131 @@
gl_FragColor = vec4(1.0, 0.0, 0.0, 1.0);
})";
+ constexpr char kTooLargeGlobalMemoryOverflow[] =
+ R"(precision mediump float;
+
+// 16 MB / 16 bytes per vec4 = 1048576
+// Create 256 arrays so each is small, but the total overflows a 32-bit number
+vec4 array[1048576], array2[1048576], array3[1048576], array4[1048576], array5[1048576];
+vec4 array6[1048576], array7[1048576], array8[1048576], array9[1048576], array10[1048576];
+vec4 array11[1048576], array12[1048576], array13[1048576], array14[1048576], array15[1048576];
+vec4 array16[1048576], array17[1048576], array18[1048576], array19[1048576], array20[1048576];
+vec4 array21[1048576], array22[1048576], array23[1048576], array24[1048576], array25[1048576];
+vec4 array26[1048576], array27[1048576], array28[1048576], array29[1048576], array30[1048576];
+vec4 array31[1048576], array32[1048576], array33[1048576], array34[1048576], array35[1048576];
+vec4 array36[1048576], array37[1048576], array38[1048576], array39[1048576], array40[1048576];
+vec4 array41[1048576], array42[1048576], array43[1048576], array44[1048576], array45[1048576];
+vec4 array46[1048576], array47[1048576], array48[1048576], array49[1048576], array50[1048576];
+vec4 array51[1048576], array52[1048576], array53[1048576], array54[1048576], array55[1048576];
+vec4 array56[1048576], array57[1048576], array58[1048576], array59[1048576], array60[1048576];
+vec4 array61[1048576], array62[1048576], array63[1048576], array64[1048576], array65[1048576];
+vec4 array66[1048576], array67[1048576], array68[1048576], array69[1048576], array70[1048576];
+vec4 array71[1048576], array72[1048576], array73[1048576], array74[1048576], array75[1048576];
+vec4 array76[1048576], array77[1048576], array78[1048576], array79[1048576], array80[1048576];
+vec4 array81[1048576], array82[1048576], array83[1048576], array84[1048576], array85[1048576];
+vec4 array86[1048576], array87[1048576], array88[1048576], array89[1048576], array90[1048576];
+vec4 array91[1048576], array92[1048576], array93[1048576], array94[1048576], array95[1048576];
+vec4 array96[1048576], array97[1048576], array98[1048576], array99[1048576], array100[1048576];
+vec4 array101[1048576], array102[1048576], array103[1048576], array104[1048576], array105[1048576];
+vec4 array106[1048576], array107[1048576], array108[1048576], array109[1048576], array110[1048576];
+vec4 array111[1048576], array112[1048576], array113[1048576], array114[1048576], array115[1048576];
+vec4 array116[1048576], array117[1048576], array118[1048576], array119[1048576], array120[1048576];
+vec4 array121[1048576], array122[1048576], array123[1048576], array124[1048576], array125[1048576];
+vec4 array126[1048576], array127[1048576], array128[1048576], array129[1048576], array130[1048576];
+vec4 array131[1048576], array132[1048576], array133[1048576], array134[1048576], array135[1048576];
+vec4 array136[1048576], array137[1048576], array138[1048576], array139[1048576], array140[1048576];
+vec4 array141[1048576], array142[1048576], array143[1048576], array144[1048576], array145[1048576];
+vec4 array146[1048576], array147[1048576], array148[1048576], array149[1048576], array150[1048576];
+vec4 array151[1048576], array152[1048576], array153[1048576], array154[1048576], array155[1048576];
+vec4 array156[1048576], array157[1048576], array158[1048576], array159[1048576], array160[1048576];
+vec4 array161[1048576], array162[1048576], array163[1048576], array164[1048576], array165[1048576];
+vec4 array166[1048576], array167[1048576], array168[1048576], array169[1048576], array170[1048576];
+vec4 array171[1048576], array172[1048576], array173[1048576], array174[1048576], array175[1048576];
+vec4 array176[1048576], array177[1048576], array178[1048576], array179[1048576], array180[1048576];
+vec4 array181[1048576], array182[1048576], array183[1048576], array184[1048576], array185[1048576];
+vec4 array186[1048576], array187[1048576], array188[1048576], array189[1048576], array190[1048576];
+vec4 array191[1048576], array192[1048576], array193[1048576], array194[1048576], array195[1048576];
+vec4 array196[1048576], array197[1048576], array198[1048576], array199[1048576], array200[1048576];
+vec4 array201[1048576], array202[1048576], array203[1048576], array204[1048576], array205[1048576];
+vec4 array206[1048576], array207[1048576], array208[1048576], array209[1048576], array210[1048576];
+vec4 array211[1048576], array212[1048576], array213[1048576], array214[1048576], array215[1048576];
+vec4 array216[1048576], array217[1048576], array218[1048576], array219[1048576], array220[1048576];
+vec4 array221[1048576], array222[1048576], array223[1048576], array224[1048576], array225[1048576];
+vec4 array226[1048576], array227[1048576], array228[1048576], array229[1048576], array230[1048576];
+vec4 array231[1048576], array232[1048576], array233[1048576], array234[1048576], array235[1048576];
+vec4 array236[1048576], array237[1048576], array238[1048576], array239[1048576], array240[1048576];
+vec4 array241[1048576], array242[1048576], array243[1048576], array244[1048576], array245[1048576];
+vec4 array246[1048576], array247[1048576], array248[1048576], array249[1048576], array250[1048576];
+vec4 array251[1048576], array252[1048576], array253[1048576], array254[1048576], array255[1048576];
+vec4 array256[1048576];
+
+void main()
+{
+ float f = array[0].x; f += array2[0].x; f += array3[0].x; f += array4[0].x; f += array5[0].x;
+ f += array6[0].x; f += array7[0].x; f += array8[0].x; f += array9[0].x; f += array10[0].x;
+ f += array11[0].x; f += array12[0].x; f += array13[0].x; f += array14[0].x; f += array15[0].x;
+ f += array16[0].x; f += array17[0].x; f += array18[0].x; f += array19[0].x; f += array20[0].x;
+ f += array21[0].x; f += array22[0].x; f += array23[0].x; f += array24[0].x; f += array25[0].x;
+ f += array26[0].x; f += array27[0].x; f += array28[0].x; f += array29[0].x; f += array30[0].x;
+ f += array31[0].x; f += array32[0].x; f += array33[0].x; f += array34[0].x; f += array35[0].x;
+ f += array36[0].x; f += array37[0].x; f += array38[0].x; f += array39[0].x; f += array40[0].x;
+ f += array41[0].x; f += array42[0].x; f += array43[0].x; f += array44[0].x; f += array45[0].x;
+ f += array46[0].x; f += array47[0].x; f += array48[0].x; f += array49[0].x; f += array50[0].x;
+ f += array51[0].x; f += array52[0].x; f += array53[0].x; f += array54[0].x; f += array55[0].x;
+ f += array56[0].x; f += array57[0].x; f += array58[0].x; f += array59[0].x; f += array60[0].x;
+ f += array61[0].x; f += array62[0].x; f += array63[0].x; f += array64[0].x; f += array65[0].x;
+ f += array66[0].x; f += array67[0].x; f += array68[0].x; f += array69[0].x; f += array70[0].x;
+ f += array71[0].x; f += array72[0].x; f += array73[0].x; f += array74[0].x; f += array75[0].x;
+ f += array76[0].x; f += array77[0].x; f += array78[0].x; f += array79[0].x; f += array80[0].x;
+ f += array81[0].x; f += array82[0].x; f += array83[0].x; f += array84[0].x; f += array85[0].x;
+ f += array86[0].x; f += array87[0].x; f += array88[0].x; f += array89[0].x; f += array90[0].x;
+ f += array91[0].x; f += array92[0].x; f += array93[0].x; f += array94[0].x; f += array95[0].x;
+ f += array96[0].x; f += array97[0].x; f += array98[0].x; f += array99[0].x; f += array100[0].x;
+ f += array101[0].x; f += array102[0].x; f += array103[0].x; f += array104[0].x;
+ f += array105[0].x; f += array106[0].x; f += array107[0].x; f += array108[0].x;
+ f += array109[0].x; f += array110[0].x; f += array111[0].x; f += array112[0].x;
+ f += array113[0].x; f += array114[0].x; f += array115[0].x; f += array116[0].x;
+ f += array117[0].x; f += array118[0].x; f += array119[0].x; f += array120[0].x;
+ f += array121[0].x; f += array122[0].x; f += array123[0].x; f += array124[0].x;
+ f += array125[0].x; f += array126[0].x; f += array127[0].x; f += array128[0].x;
+ f += array129[0].x; f += array130[0].x; f += array131[0].x; f += array132[0].x;
+ f += array133[0].x; f += array134[0].x; f += array135[0].x; f += array136[0].x;
+ f += array137[0].x; f += array138[0].x; f += array139[0].x; f += array140[0].x;
+ f += array141[0].x; f += array142[0].x; f += array143[0].x; f += array144[0].x;
+ f += array145[0].x; f += array146[0].x; f += array147[0].x; f += array148[0].x;
+ f += array149[0].x; f += array150[0].x; f += array151[0].x; f += array152[0].x;
+ f += array153[0].x; f += array154[0].x; f += array155[0].x; f += array156[0].x;
+ f += array157[0].x; f += array158[0].x; f += array159[0].x; f += array160[0].x;
+ f += array161[0].x; f += array162[0].x; f += array163[0].x; f += array164[0].x;
+ f += array165[0].x; f += array166[0].x; f += array167[0].x; f += array168[0].x;
+ f += array169[0].x; f += array170[0].x; f += array171[0].x; f += array172[0].x;
+ f += array173[0].x; f += array174[0].x; f += array175[0].x; f += array176[0].x;
+ f += array177[0].x; f += array178[0].x; f += array179[0].x; f += array180[0].x;
+ f += array181[0].x; f += array182[0].x; f += array183[0].x; f += array184[0].x;
+ f += array185[0].x; f += array186[0].x; f += array187[0].x; f += array188[0].x;
+ f += array189[0].x; f += array190[0].x; f += array191[0].x; f += array192[0].x;
+ f += array193[0].x; f += array194[0].x; f += array195[0].x; f += array196[0].x;
+ f += array197[0].x; f += array198[0].x; f += array199[0].x; f += array200[0].x;
+ f += array201[0].x; f += array202[0].x; f += array203[0].x; f += array204[0].x;
+ f += array205[0].x; f += array206[0].x; f += array207[0].x; f += array208[0].x;
+ f += array209[0].x; f += array210[0].x; f += array211[0].x; f += array212[0].x;
+ f += array213[0].x; f += array214[0].x; f += array215[0].x; f += array216[0].x;
+ f += array217[0].x; f += array218[0].x; f += array219[0].x; f += array220[0].x;
+ f += array221[0].x; f += array222[0].x; f += array223[0].x; f += array224[0].x;
+ f += array225[0].x; f += array226[0].x; f += array227[0].x; f += array228[0].x;
+ f += array229[0].x; f += array230[0].x; f += array231[0].x; f += array232[0].x;
+ f += array233[0].x; f += array234[0].x; f += array235[0].x; f += array236[0].x;
+ f += array237[0].x; f += array238[0].x; f += array239[0].x; f += array240[0].x;
+ f += array241[0].x; f += array242[0].x; f += array243[0].x; f += array244[0].x;
+ f += array245[0].x; f += array246[0].x; f += array247[0].x; f += array248[0].x;
+ f += array249[0].x; f += array250[0].x; f += array251[0].x; f += array252[0].x;
+ f += array253[0].x; f += array254[0].x; f += array255[0].x; f += array256[0].x;
+ if (f == 2.0)
+ gl_FragColor = vec4(0.0, 1.0, 0.0, 1.0);
+ else
+ gl_FragColor = vec4(1.0, 0.0, 0.0, 1.0);
+})";
+
GLuint program = CompileProgram(essl1_shaders::vs::Simple(), kTooLargeGlobalMemory1);
EXPECT_EQ(0u, program);
@@ -5449,6 +5574,9 @@
program = CompileProgram(essl1_shaders::vs::Simple(), kTooLargeGlobalAndLocalMemory2);
EXPECT_EQ(0u, program);
+
+ program = CompileProgram(essl1_shaders::vs::Simple(), kTooLargeGlobalMemoryOverflow);
+ EXPECT_EQ(0u, program);
}
// Linking should fail when corresponding vertex/fragment uniform blocks have different precision

View File

@@ -1,286 +0,0 @@
From cafe56b591edb77f041be70b58cac3a61565644a Mon Sep 17 00:00:00 2001
From: Geoff Lang <geofflang@chromium.org>
Date: Fri, 23 Jun 2023 14:46:28 -0400
Subject: [PATCH] M116: GL: Ensure all instanced attributes have a buffer with data
Apple OpenGL drivers sometimes crash when given an instanced draw with
a buffer that has never been given data.
It's not efficient to check if the attribute is both zero-sized and
instanced so just ensure that every time a zero-sized buffer is bound
to an attribute, it gets initialized with some data.
Bug: chromium:1456243
Change-Id: I66b7c7017843153db2df3bc50010cba765d03c5f
Reviewed-on: https://chromium-review.googlesource.com/c/angle/angle/+/4642048
Commit-Queue: Geoff Lang <geofflang@chromium.org>
Reviewed-by: Shahbaz Youssefi <syoussefi@chromium.org>
(cherry picked from commit 4e6124dae892690204f8e5996aeaad14f45e0a97)
Reviewed-on: https://chromium-review.googlesource.com/c/angle/angle/+/4727452
---
diff --git a/include/platform/FeaturesGL_autogen.h b/include/platform/FeaturesGL_autogen.h
index aa0565c..2f6e094 100644
--- a/include/platform/FeaturesGL_autogen.h
+++ b/include/platform/FeaturesGL_autogen.h
@@ -501,6 +501,12 @@
"supportsShaderPixelLocalStorageEXT", FeatureCategory::OpenGLFeatures,
"Backend GL context supports EXT_shader_pixel_local_storage extension", &members,
"http://anglebug.com/7279"};
+
+ FeatureInfo ensureNonEmptyBufferIsBoundForDraw = {
+ "ensureNonEmptyBufferIsBoundForDraw", FeatureCategory::OpenGLFeatures,
+ "Apple OpenGL drivers crash when drawing with a zero-sized buffer bound using a non-zero "
+ "divisor.",
+ &members, "http://crbug.com/1456243"};
};
inline FeaturesGL::FeaturesGL() = default;
diff --git a/include/platform/gl_features.json b/include/platform/gl_features.json
index 032f29a..b358cea 100644
--- a/include/platform/gl_features.json
+++ b/include/platform/gl_features.json
@@ -699,6 +699,14 @@
"Backend GL context supports EXT_shader_pixel_local_storage extension"
],
"issue": "http://anglebug.com/7279"
+ },
+ {
+ "name": "ensure_non_empty_buffer_is_bound_for_draw",
+ "category": "Features",
+ "description": [
+ "Apple OpenGL drivers crash when drawing with a zero-sized buffer bound using a non-zero divisor."
+ ],
+ "issue": "http://crbug.com/1456243"
}
]
}
diff --git a/scripts/code_generation_hashes/ANGLE_features.json b/scripts/code_generation_hashes/ANGLE_features.json
index d4576c2..503001c 100644
--- a/scripts/code_generation_hashes/ANGLE_features.json
+++ b/scripts/code_generation_hashes/ANGLE_features.json
@@ -2,7 +2,7 @@
"include/platform/FeaturesD3D_autogen.h":
"9923fb44d0a6f31948d0c8f46ee1d9e2",
"include/platform/FeaturesGL_autogen.h":
- "a795a806d71b0e6d1f9e6d95c6e11971",
+ "fef16ab3946346a2a7d5b76bb39471a4",
"include/platform/FeaturesMtl_autogen.h":
"407426c8874de9295482ace9c94bd812",
"include/platform/FeaturesVk_autogen.h":
@@ -16,13 +16,13 @@
"include/platform/gen_features.py":
"062989f7a8f3ff3b383f98fc8908dc33",
"include/platform/gl_features.json":
- "3335055a70e35ebb7bf74c6d7c58897b",
+ "c9aead89696e7fd0c8bfe5c5ca85ca63",
"include/platform/mtl_features.json":
"c66d170e7a8eb3448030f4c423ed0133",
"include/platform/vk_features.json":
"416bbb28b9fa1a3c4ef141f243c0a9e6",
"util/angle_features_autogen.cpp":
- "73169f63c755192c3b4bd27d6f4096ca",
+ "288daaec490eb816883d744f108d74c9",
"util/angle_features_autogen.h":
- "7aa8120eb8f8fd335946b8c27074745d"
+ "daf25d3e4ffea143d1c082416513f7e7"
}
\ No newline at end of file
diff --git a/src/libANGLE/renderer/gl/BufferGL.cpp b/src/libANGLE/renderer/gl/BufferGL.cpp
index c99fd5d..9651838 100644
--- a/src/libANGLE/renderer/gl/BufferGL.cpp
+++ b/src/libANGLE/renderer/gl/BufferGL.cpp
@@ -296,6 +296,11 @@
return angle::Result::Continue;
}
+size_t BufferGL::getBufferSize() const
+{
+ return mBufferSize;
+}
+
GLuint BufferGL::getBufferID() const
{
return mBufferID;
diff --git a/src/libANGLE/renderer/gl/BufferGL.h b/src/libANGLE/renderer/gl/BufferGL.h
index 7b57594..fe9138e 100644
--- a/src/libANGLE/renderer/gl/BufferGL.h
+++ b/src/libANGLE/renderer/gl/BufferGL.h
@@ -56,6 +56,7 @@
bool primitiveRestartEnabled,
gl::IndexRange *outRange) override;
+ size_t getBufferSize() const;
GLuint getBufferID() const;
private:
diff --git a/src/libANGLE/renderer/gl/VertexArrayGL.cpp b/src/libANGLE/renderer/gl/VertexArrayGL.cpp
index dc981de..fda9099 100644
--- a/src/libANGLE/renderer/gl/VertexArrayGL.cpp
+++ b/src/libANGLE/renderer/gl/VertexArrayGL.cpp
@@ -646,6 +646,7 @@
angle::Result VertexArrayGL::updateAttribPointer(const gl::Context *context, size_t attribIndex)
{
+ const angle::FeaturesGL &features = GetFeaturesGL(context);
const VertexAttribute &attrib = mState.getVertexAttribute(attribIndex);
@@ -687,8 +688,16 @@
// is not NULL.
StateManagerGL *stateManager = GetStateManagerGL(context);
- GLuint bufferId = GetNativeBufferID(arrayBuffer);
+ BufferGL *bufferGL = GetImplAs<BufferGL>(arrayBuffer);
+ GLuint bufferId = bufferGL->getBufferID();
stateManager->bindBuffer(gl::BufferBinding::Array, bufferId);
+ if (features.ensureNonEmptyBufferIsBoundForDraw.enabled && bufferGL->getBufferSize() == 0)
+ {
+ constexpr uint32_t data = 0;
+ ANGLE_TRY(bufferGL->setData(context, gl::BufferBinding::Array, &data, sizeof(data),
+ gl::BufferUsage::StaticDraw));
+ ASSERT(bufferGL->getBufferSize() > 0);
+ }
ANGLE_TRY(callVertexAttribPointer(context, static_cast<GLuint>(attribIndex), attrib,
binding.getStride(), binding.getOffset()));
diff --git a/src/libANGLE/renderer/gl/renderergl_utils.cpp b/src/libANGLE/renderer/gl/renderergl_utils.cpp
index 6911247..ab2a608 100644
--- a/src/libANGLE/renderer/gl/renderergl_utils.cpp
+++ b/src/libANGLE/renderer/gl/renderergl_utils.cpp
@@ -2465,6 +2465,9 @@
// EXT_shader_pixel_local_storage
ANGLE_FEATURE_CONDITION(features, supportsShaderPixelLocalStorageEXT,
functions->hasGLESExtension("GL_EXT_shader_pixel_local_storage"));
+
+ // http://crbug.com/1456243
+ ANGLE_FEATURE_CONDITION(features, ensureNonEmptyBufferIsBoundForDraw, IsApple() || IsAndroid());
}
void InitializeFrontendFeatures(const FunctionsGL *functions, angle::FrontendFeatures *features)
diff --git a/src/tests/angle_end2end_tests_expectations.txt b/src/tests/angle_end2end_tests_expectations.txt
index 59ec7c2..44ff3e4 100644
--- a/src/tests/angle_end2end_tests_expectations.txt
+++ b/src/tests/angle_end2end_tests_expectations.txt
@@ -380,6 +380,7 @@
7294 WIN D3D11 : StateChangeTestES3.StencilWriteMask/* = SKIP
7316 WIN D3D11 : StateChangeTestES3.StencilTestAndFunc/* = SKIP
7329 WIN D3D11 : StateChangeTestES3.PrimitiveRestart/* = SKIP
+1456243 WIN D3D11 : WebGL2CompatibilityTest.DrawWithZeroSizedBuffer/* = SKIP
// Android
6095 ANDROID GLES : GLSLTest_ES3.InitGlobalComplexConstant/* = SKIP
diff --git a/src/tests/gl_tests/WebGLCompatibilityTest.cpp b/src/tests/gl_tests/WebGLCompatibilityTest.cpp
index 7dc56cd..bd7ecd1 100644
--- a/src/tests/gl_tests/WebGLCompatibilityTest.cpp
+++ b/src/tests/gl_tests/WebGLCompatibilityTest.cpp
@@ -1632,10 +1632,10 @@
constexpr GLuint kMaxIntAsGLuint = static_cast<GLuint>(std::numeric_limits<GLint>::max());
constexpr GLuint kIndexData[] = {
- kMaxIntAsGLuint,
- kMaxIntAsGLuint + 1,
- kMaxIntAsGLuint + 2,
- kMaxIntAsGLuint + 3,
+ kMaxIntAsGLuint,
+ kMaxIntAsGLuint + 1,
+ kMaxIntAsGLuint + 2,
+ kMaxIntAsGLuint + 3,
};
GLBuffer indexBuffer;
@@ -3687,8 +3687,8 @@
constexpr float readPixelsData[] = {-5000.0f, 0.0f, 0.0f, 1.0f};
const GLushort textureData[] = {
- gl::float32ToFloat16(readPixelsData[0]), gl::float32ToFloat16(readPixelsData[1]),
- gl::float32ToFloat16(readPixelsData[2]), gl::float32ToFloat16(readPixelsData[3])};
+ gl::float32ToFloat16(readPixelsData[0]), gl::float32ToFloat16(readPixelsData[1]),
+ gl::float32ToFloat16(readPixelsData[2]), gl::float32ToFloat16(readPixelsData[3])};
for (auto extension : FloatingPointTextureExtensions)
{
@@ -3748,8 +3748,8 @@
constexpr float readPixelsData[] = {7108.0f, -10.0f, 0.0f, 1.0f};
const GLushort textureData[] = {
- gl::float32ToFloat16(readPixelsData[0]), gl::float32ToFloat16(readPixelsData[1]),
- gl::float32ToFloat16(readPixelsData[2]), gl::float32ToFloat16(readPixelsData[3])};
+ gl::float32ToFloat16(readPixelsData[0]), gl::float32ToFloat16(readPixelsData[1]),
+ gl::float32ToFloat16(readPixelsData[2]), gl::float32ToFloat16(readPixelsData[3])};
for (auto extension : FloatingPointTextureExtensions)
{
@@ -3811,8 +3811,8 @@
constexpr float readPixelsData[] = {7000.0f, 100.0f, 33.0f, 1.0f};
const GLushort textureData[] = {
- gl::float32ToFloat16(readPixelsData[0]), gl::float32ToFloat16(readPixelsData[1]),
- gl::float32ToFloat16(readPixelsData[2]), gl::float32ToFloat16(readPixelsData[3])};
+ gl::float32ToFloat16(readPixelsData[0]), gl::float32ToFloat16(readPixelsData[1]),
+ gl::float32ToFloat16(readPixelsData[2]), gl::float32ToFloat16(readPixelsData[3])};
for (auto extension : FloatingPointTextureExtensions)
{
@@ -3874,8 +3874,8 @@
constexpr float readPixelsData[] = {7000.0f, 100.0f, 33.0f, -1.0f};
const GLushort textureData[] = {
- gl::float32ToFloat16(readPixelsData[0]), gl::float32ToFloat16(readPixelsData[1]),
- gl::float32ToFloat16(readPixelsData[2]), gl::float32ToFloat16(readPixelsData[3])};
+ gl::float32ToFloat16(readPixelsData[0]), gl::float32ToFloat16(readPixelsData[1]),
+ gl::float32ToFloat16(readPixelsData[2]), gl::float32ToFloat16(readPixelsData[3])};
for (auto extension : FloatingPointTextureExtensions)
{
@@ -5803,6 +5803,26 @@
}
}
+// Test for a mishandling of instanced vertex attributes with zero-sized buffers bound on Apple
+// OpenGL drivers.
+TEST_P(WebGL2CompatibilityTest, DrawWithZeroSizedBuffer)
+{
+ ANGLE_GL_PROGRAM(program, essl3_shaders::vs::Simple(), essl3_shaders::fs::Red());
+ glUseProgram(program);
+
+ GLBuffer buffer;
+ glBindBuffer(GL_ARRAY_BUFFER, buffer);
+
+ GLint posLocation = glGetAttribLocation(program, essl3_shaders::PositionAttrib());
+ glEnableVertexAttribArray(posLocation);
+
+ glVertexAttribDivisor(posLocation, 1);
+ glVertexAttribPointer(posLocation, 1, GL_UNSIGNED_BYTE, GL_FALSE, 9,
+ reinterpret_cast<void *>(0x41424344));
+
+ glDrawArrays(GL_TRIANGLES, 0, 6);
+}
+
ANGLE_INSTANTIATE_TEST_ES2_AND_ES3(WebGLCompatibilityTest);
GTEST_ALLOW_UNINSTANTIATED_PARAMETERIZED_TEST(WebGL2CompatibilityTest);
diff --git a/util/angle_features_autogen.cpp b/util/angle_features_autogen.cpp
index adb9610..a060dd2 100644
--- a/util/angle_features_autogen.cpp
+++ b/util/angle_features_autogen.cpp
@@ -118,6 +118,7 @@
{Feature::EnablePrecisionQualifiers, "enablePrecisionQualifiers"},
{Feature::EnablePreRotateSurfaces, "enablePreRotateSurfaces"},
{Feature::EnableProgramBinaryForCapture, "enableProgramBinaryForCapture"},
+ {Feature::EnsureNonEmptyBufferIsBoundForDraw, "ensureNonEmptyBufferIsBoundForDraw"},
{Feature::ExpandIntegerPowExpressions, "expandIntegerPowExpressions"},
{Feature::ExplicitlyEnablePerSampleShading, "explicitlyEnablePerSampleShading"},
{Feature::ExposeNonConformantExtensionsAndVersions, "exposeNonConformantExtensionsAndVersions"},
diff --git a/util/angle_features_autogen.h b/util/angle_features_autogen.h
index 3d8c47f..4064425 100644
--- a/util/angle_features_autogen.h
+++ b/util/angle_features_autogen.h
@@ -112,6 +112,7 @@
EnablePrecisionQualifiers,
EnablePreRotateSurfaces,
EnableProgramBinaryForCapture,
+ EnsureNonEmptyBufferIsBoundForDraw,
ExpandIntegerPowExpressions,
ExplicitlyEnablePerSampleShading,
ExposeNonConformantExtensionsAndVersions,

View File

@@ -0,0 +1,215 @@
From 0000000000000000000000000000000000000000 Mon Sep 17 00:00:00 2001
From: Shahbaz Youssefi <syoussefi@chromium.org>
Date: Thu, 30 Nov 2023 14:12:42 -0500
Subject: M120: Translator: Fail compilation if too many struct fields
If there are too many struct fields, SPIR-V cannot be produced (as it
has a hard limit of 16383 fields). The Nvidia GL driver has also been
observed to fail when there are too many fields.
Bug: chromium:1505009
Change-Id: I29fd61d180175e89e7db9ca8ba49ab07585b5f9a
Reviewed-on: https://chromium-review.googlesource.com/c/angle/angle/+/5143827
Reviewed-by: Cody Northrop <cnorthrop@google.com>
diff --git a/src/compiler/translator/ParseContext.cpp b/src/compiler/translator/ParseContext.cpp
index 7c90ea4f1d23a8af0cab1d44124eea021a27a16d..e174725beb764407185e471a9916ffd164493cd8 100644
--- a/src/compiler/translator/ParseContext.cpp
+++ b/src/compiler/translator/ParseContext.cpp
@@ -4726,6 +4726,8 @@ TIntermDeclaration *TParseContext::addInterfaceBlock(
const TVector<unsigned int> *arraySizes,
const TSourceLoc &arraySizesLine)
{
+ checkDoesNotHaveTooManyFields(blockName, fieldList, nameLine);
+
// Ensure there are no duplicate field names
checkDoesNotHaveDuplicateFieldNames(fieldList, nameLine);
@@ -6289,6 +6291,21 @@ void TParseContext::checkDoesNotHaveDuplicateFieldNames(const TFieldList *fields
}
}
+void TParseContext::checkDoesNotHaveTooManyFields(const ImmutableString &name,
+ const TFieldList *fields,
+ const TSourceLoc &location)
+{
+ // Check that there are not too many fields. SPIR-V has a limit of 16383 fields, and it would
+ // be reasonable to apply that limit to all outputs. For example, it was observed that 32768
+ // fields cause the Nvidia GL driver to fail compilation, so such a limit is not too specific to
+ // SPIR-V.
+ constexpr size_t kMaxFieldCount = 16383;
+ if (fields->size() > kMaxFieldCount)
+ {
+ error(location, "Too many fields in the struct (limit is 16383)", name);
+ }
+}
+
TFieldList *TParseContext::addStructFieldList(TFieldList *fields, const TSourceLoc &location)
{
return fields;
@@ -6392,6 +6409,8 @@ TTypeSpecifierNonArray TParseContext::addStructure(const TSourceLoc &structLine,
}
}
+ checkDoesNotHaveTooManyFields(structName, fieldList, structLine);
+
// Ensure there are no duplicate field names
checkDoesNotHaveDuplicateFieldNames(fieldList, structLine);
diff --git a/src/compiler/translator/ParseContext.h b/src/compiler/translator/ParseContext.h
index b63dbbadd146d1a004513823de72c89240a31929..c83b73271b557ed122668d7e655deae5aa23bc48 100644
--- a/src/compiler/translator/ParseContext.h
+++ b/src/compiler/translator/ParseContext.h
@@ -355,6 +355,9 @@ class TParseContext : angle::NonCopyable
const TVector<unsigned int> *arraySizes);
void checkDoesNotHaveDuplicateFieldNames(const TFieldList *fields, const TSourceLoc &location);
+ void checkDoesNotHaveTooManyFields(const ImmutableString &name,
+ const TFieldList *fields,
+ const TSourceLoc &location);
TFieldList *addStructFieldList(TFieldList *fields, const TSourceLoc &location);
TFieldList *combineStructFieldLists(TFieldList *processedFields,
const TFieldList *newlyAddedFields,
diff --git a/src/tests/compiler_tests/ExpressionLimit_test.cpp b/src/tests/compiler_tests/ExpressionLimit_test.cpp
index d399e1792d97edb76d5e0247e4166e094cecb7e5..e17eace1b4b9a4185d6833fb7895f5fd49ae7679 100644
--- a/src/tests/compiler_tests/ExpressionLimit_test.cpp
+++ b/src/tests/compiler_tests/ExpressionLimit_test.cpp
@@ -16,12 +16,6 @@ class ExpressionLimitTest : public testing::Test
static const int kMaxExpressionComplexity = 16;
static const int kMaxCallStackDepth = 16;
static const int kMaxFunctionParameters = 16;
- static const char *kExpressionTooComplex;
- static const char *kCallStackTooDeep;
- static const char *kHasRecursion;
- static const char *kTooManyParameters;
- static const char *kTooComplexSwitch;
- static const char *kGlobalVariableInit;
virtual void SetUp()
{
@@ -125,9 +119,7 @@ class ExpressionLimitTest : public testing::Test
GenerateDeepFunctionStack(length, &ss);
- ss << "void main() {\n"
- << " gl_FragColor = function" << length << "();\n"
- << "}";
+ ss << "void main() {\n" << " gl_FragColor = function" << length << "();\n" << "}";
return ss.str();
}
@@ -138,9 +130,7 @@ class ExpressionLimitTest : public testing::Test
GenerateDeepFunctionStack(length, &ss);
- ss << "void main() {\n"
- << " gl_FragColor = vec4(0,0,0,0);\n"
- << "}";
+ ss << "void main() {\n" << " gl_FragColor = vec4(0,0,0,0);\n" << "}";
return ss.str();
}
@@ -149,9 +139,7 @@ class ExpressionLimitTest : public testing::Test
{
std::stringstream ss;
- ss << "precision mediump float;\n"
- << "\n"
- << "float foo(";
+ ss << "precision mediump float;\n" << "\n" << "float foo(";
for (int i = 0; i < parameters; ++i)
{
ss << "float f" << i;
@@ -244,15 +232,13 @@ class ExpressionLimitTest : public testing::Test
ShBuiltInResources resources;
};
-const char *ExpressionLimitTest::kExpressionTooComplex = "Expression too complex";
-const char *ExpressionLimitTest::kCallStackTooDeep = "Call stack too deep";
-const char *ExpressionLimitTest::kHasRecursion =
- "Recursive function call in the following call chain";
-const char *ExpressionLimitTest::kTooManyParameters = "Function has too many parameters";
-const char *ExpressionLimitTest::kTooComplexSwitch =
- "too complex expressions inside a switch statement";
-const char *ExpressionLimitTest::kGlobalVariableInit =
- "global variable initializers must be constant expressions";
+constexpr char kExpressionTooComplex[] = "Expression too complex";
+constexpr char kCallStackTooDeep[] = "Call stack too deep";
+constexpr char kHasRecursion[] = "Recursive function call in the following call chain";
+constexpr char kTooManyParameters[] = "Function has too many parameters";
+constexpr char kTooComplexSwitch[] = "too complex expressions inside a switch statement";
+constexpr char kGlobalVariableInit[] = "global variable initializers must be constant expressions";
+constexpr char kTooManyFields[] = "Too many fields in the struct";
TEST_F(ExpressionLimitTest, ExpressionComplexity)
{
@@ -632,3 +618,31 @@ TEST_F(ExpressionLimitTest, NestingInsideGlobalInitializer)
compileOptions, nullptr));
sh::Destruct(compiler);
}
+
+TEST_F(ExpressionLimitTest, TooManyStructFields)
+{
+ ShShaderSpec spec = SH_WEBGL2_SPEC;
+ ShShaderOutput output = SH_ESSL_OUTPUT;
+ ShHandle compiler = sh::ConstructCompiler(GL_FRAGMENT_SHADER, spec, output, &resources);
+ ShCompileOptions compileOptions = {};
+
+ std::ostringstream fs;
+ fs << R"(#version 300 es
+precision highp float;
+struct TooManyFields
+{
+)";
+ for (uint32_t i = 0; i < (1 << 16); ++i)
+ {
+ fs << " float field" << i << ";\n";
+ }
+ fs << R"(};
+uniform B { TooManyFields s; };
+out vec4 color;
+void main() {
+ color = vec4(s.field0, 0.0, 0.0, 1.0);
+})";
+
+ EXPECT_TRUE(CheckShaderCompilation(compiler, fs.str().c_str(), compileOptions, kTooManyFields));
+ sh::Destruct(compiler);
+}
diff --git a/src/tests/gl_tests/GLSLTest.cpp b/src/tests/gl_tests/GLSLTest.cpp
index 9b42b6d9221366e9bc7b6263bf8e14b77ca7694d..46f8645e5a56e527223ea4657d14dbc8619cb631 100644
--- a/src/tests/gl_tests/GLSLTest.cpp
+++ b/src/tests/gl_tests/GLSLTest.cpp
@@ -18064,6 +18064,33 @@ void main() {
ANGLE_GL_PROGRAM(program, essl3_shaders::vs::Simple(), fs.str().c_str());
}
+// Test that structs with too many fields are rejected. In SPIR-V, the instruction that defines the
+// struct lists the fields which means the length of the instruction is a function of the field
+// count. Since SPIR-V instruction sizes are limited to 16 bits, structs with more fields cannot be
+// represented.
+TEST_P(GLSLTest_ES3, TooManyFieldsInStruct)
+{
+ std::ostringstream fs;
+ fs << R"(#version 300 es
+precision highp float;
+struct TooManyFields
+{
+)";
+ for (uint32_t i = 0; i < (1 << 16); ++i)
+ {
+ fs << " float field" << i << ";\n";
+ }
+ fs << R"(};
+uniform B { TooManyFields s; };
+out vec4 color;
+void main() {
+ color = vec4(s.field0, 0.0, 0.0, 1.0);
+})";
+
+ GLuint shader = CompileShader(GL_FRAGMENT_SHADER, fs.str().c_str());
+ EXPECT_EQ(0u, shader);
+}
+
} // anonymous namespace
ANGLE_INSTANTIATE_TEST_ES2_AND_ES3(GLSLTest);

View File

@@ -0,0 +1,416 @@
From 0000000000000000000000000000000000000000 Mon Sep 17 00:00:00 2001
From: Shahbaz Youssefi <syoussefi@chromium.org>
Date: Thu, 30 Nov 2023 15:42:32 -0500
Subject: M120: Translator: Limit private variable size to 64KB
This is indirectly fixing an issue where passing large arrays in SPIR-V
such that an internal cast is needed (such as array inside interface
block copied to local varaible) causes an overflow of the instruction
length limit (in the absence of OpCopyLogical).
By limiting the size of private variables to 32KB, this limitation is
indirectly enforced. It was observed that all the test shaders added in
this CL fail on the Nvidia OpenGL drivers, so such a limit seems to be
reasonble.
Bug: chromium:1505009
Change-Id: I75a1e40a538120ffc69ae7edafbdba5830c6b0bb
Reviewed-on: https://chromium-review.googlesource.com/c/angle/angle/+/5143828
Reviewed-by: Cody Northrop <cnorthrop@google.com>
diff --git a/src/compiler/translator/Compiler.cpp b/src/compiler/translator/Compiler.cpp
index 383feeb477d2445d5e599b39e125fe455169da08..8736b3dcfbcb43f9087b78224658a9680d7ee68c 100644
--- a/src/compiler/translator/Compiler.cpp
+++ b/src/compiler/translator/Compiler.cpp
@@ -770,11 +770,6 @@ bool TCompiler::checkAndSimplifyAST(TIntermBlock *root,
return false;
}
- if (shouldLimitTypeSizes() && !ValidateTypeSizeLimitations(root, &mSymbolTable, &mDiagnostics))
- {
- return false;
- }
-
if (!ValidateFragColorAndFragData(mShaderType, mShaderVersion, mSymbolTable, &mDiagnostics))
{
return false;
@@ -1046,6 +1041,13 @@ bool TCompiler::checkAndSimplifyAST(TIntermBlock *root,
return false;
}
+ // Run after RemoveUnreferencedVariables, validate that the shader does not have excessively
+ // large variables.
+ if (shouldLimitTypeSizes() && !ValidateTypeSizeLimitations(root, &mSymbolTable, &mDiagnostics))
+ {
+ return false;
+ }
+
// Built-in function emulation needs to happen after validateLimitations pass.
GetGlobalPoolAllocator()->lock();
initBuiltInFunctionEmulator(&mBuiltInFunctionEmulator, compileOptions);
diff --git a/src/compiler/translator/ValidateTypeSizeLimitations.cpp b/src/compiler/translator/ValidateTypeSizeLimitations.cpp
index f0ff9cb11ac39e62672285300c8f41641f12c617..8f02c65b5ec5fd20b8bcee2bc595cfb278f758b4 100644
--- a/src/compiler/translator/ValidateTypeSizeLimitations.cpp
+++ b/src/compiler/translator/ValidateTypeSizeLimitations.cpp
@@ -24,10 +24,11 @@ namespace
// Arbitrarily enforce that all types declared with a size in bytes of over 2 GB will cause
// compilation failure.
//
-// For local and global variables, the limit is much lower (16MB) as that much memory won't fit in
+// For local and global variables, the limit is much lower (64KB) as that much memory won't fit in
// the GPU registers anyway.
-constexpr size_t kMaxVariableSizeInBytes = static_cast<size_t>(2) * 1024 * 1024 * 1024;
-constexpr size_t kMaxPrivateVariableSizeInBytes = static_cast<size_t>(16) * 1024 * 1024;
+constexpr size_t kMaxVariableSizeInBytes = static_cast<size_t>(2) * 1024 * 1024 * 1024;
+constexpr size_t kMaxPrivateVariableSizeInBytes = static_cast<size_t>(64) * 1024;
+constexpr size_t kMaxTotalPrivateVariableSizeInBytes = static_cast<size_t>(16) * 1024 * 1024;
// Traverses intermediate tree to ensure that the shader does not
// exceed certain implementation-defined limits on the sizes of types.
@@ -70,43 +71,115 @@ class ValidateTypeSizeLimitationsTraverser : public TIntermTraverser
continue;
}
- const TType &variableType = asSymbol->getType();
-
- // Create a ShaderVariable from which to compute
- // (conservative) sizing information.
- ShaderVariable shaderVar;
- setCommonVariableProperties(variableType, variable, &shaderVar);
-
- // Compute the std140 layout of this variable, assuming
- // it's a member of a block (which it might not be).
- Std140BlockEncoder layoutEncoder;
- BlockEncoderVisitor visitor("", "", &layoutEncoder);
- // Since the size limit's arbitrary, it doesn't matter
- // whether the row-major layout is correctly determined.
- bool isRowMajorLayout = false;
- TraverseShaderVariable(shaderVar, isRowMajorLayout, &visitor);
- if (layoutEncoder.getCurrentOffset() > kMaxVariableSizeInBytes)
+ if (!validateVariableSize(variable, asSymbol->getLine()))
{
- error(asSymbol->getLine(),
- "Size of declared variable exceeds implementation-defined limit",
- asSymbol->getName());
return false;
}
+ }
+
+ return true;
+ }
+
+ void visitFunctionPrototype(TIntermFunctionPrototype *node) override
+ {
+ const TFunction *function = node->getFunction();
+ const size_t paramCount = function->getParamCount();
+
+ for (size_t paramIndex = 0; paramIndex < paramCount; ++paramIndex)
+ {
+ validateVariableSize(*function->getParam(paramIndex), node->getLine());
+ }
+ }
+
+ bool validateVariableSize(const TVariable &variable, const TSourceLoc &location)
+ {
+ const TType &variableType = variable.getType();
+
+ // Create a ShaderVariable from which to compute
+ // (conservative) sizing information.
+ ShaderVariable shaderVar;
+ setCommonVariableProperties(variableType, variable, &shaderVar);
+
+ // Compute the std140 layout of this variable, assuming
+ // it's a member of a block (which it might not be).
+ Std140BlockEncoder layoutEncoder;
+ BlockEncoderVisitor visitor("", "", &layoutEncoder);
+ // Since the size limit's arbitrary, it doesn't matter
+ // whether the row-major layout is correctly determined.
+ bool isRowMajorLayout = false;
+ TraverseShaderVariable(shaderVar, isRowMajorLayout, &visitor);
+ if (layoutEncoder.getCurrentOffset() > kMaxVariableSizeInBytes)
+ {
+ error(location, "Size of declared variable exceeds implementation-defined limit",
+ variable.name());
+ return false;
+ }
+
+ // Skip over struct declarations. As long as they are not used (or if they are used later
+ // in a less-restricted context (such as a UBO or SSBO)), they can be larger than
+ // kMaxPrivateVariableSizeInBytes.
+ if (variable.symbolType() == SymbolType::Empty && variableType.isStructSpecifier())
+ {
+ return true;
+ }
+
+ switch (variableType.getQualifier())
+ {
+ // List of all types that need to be limited (for example because they cause overflows
+ // in drivers, or create trouble for the SPIR-V gen as the number of an instruction's
+ // arguments cannot be more than 64KB (see OutputSPIRVTraverser::cast)).
+
+ // Local/global variables
+ case EvqTemporary:
+ case EvqGlobal:
+ case EvqConst:
+
+ // Function arguments
+ case EvqParamIn:
+ case EvqParamOut:
+ case EvqParamInOut:
+ case EvqParamConst:
+
+ // Varyings
+ case EvqVaryingIn:
+ case EvqVaryingOut:
+ case EvqSmoothOut:
+ case EvqFlatOut:
+ case EvqNoPerspectiveOut:
+ case EvqCentroidOut:
+ case EvqSampleOut:
+ case EvqNoPerspectiveCentroidOut:
+ case EvqNoPerspectiveSampleOut:
+ case EvqSmoothIn:
+ case EvqFlatIn:
+ case EvqNoPerspectiveIn:
+ case EvqCentroidIn:
+ case EvqNoPerspectiveCentroidIn:
+ case EvqNoPerspectiveSampleIn:
+ case EvqVertexOut:
+ case EvqFragmentIn:
+ case EvqGeometryIn:
+ case EvqGeometryOut:
+ case EvqPerVertexIn:
+ case EvqPerVertexOut:
+ case EvqPatchIn:
+ case EvqPatchOut:
+ case EvqTessControlIn:
+ case EvqTessControlOut:
+ case EvqTessEvaluationIn:
+ case EvqTessEvaluationOut:
- const bool isPrivate = variableType.getQualifier() == EvqTemporary ||
- variableType.getQualifier() == EvqGlobal ||
- variableType.getQualifier() == EvqConst;
- if (isPrivate)
- {
if (layoutEncoder.getCurrentOffset() > kMaxPrivateVariableSizeInBytes)
{
- error(asSymbol->getLine(),
+ error(location,
"Size of declared private variable exceeds implementation-defined limit",
- asSymbol->getName());
+ variable.name());
return false;
}
mTotalPrivateVariablesSize += layoutEncoder.getCurrentOffset();
- }
+ break;
+ default:
+ break;
}
return true;
@@ -115,7 +188,7 @@ class ValidateTypeSizeLimitationsTraverser : public TIntermTraverser
void validateTotalPrivateVariableSize()
{
if (mTotalPrivateVariablesSize.ValueOrDefault(std::numeric_limits<size_t>::max()) >
- kMaxPrivateVariableSizeInBytes)
+ kMaxTotalPrivateVariableSizeInBytes)
{
mDiagnostics->error(
TSourceLoc{},
diff --git a/src/tests/angle_end2end_tests_expectations.txt b/src/tests/angle_end2end_tests_expectations.txt
index 3d356609031820b92c230ac7ff836327b78ec834..cfa70eef00a1e8d67f9fbf07ee74928b55da8b92 100644
--- a/src/tests/angle_end2end_tests_expectations.txt
+++ b/src/tests/angle_end2end_tests_expectations.txt
@@ -111,6 +111,8 @@ b/273271471 WIN INTEL VULKAN : ShaderAlgorithmTest.rgb_to_hsl_vertex_shader/* =
7872 WIN INTEL OPENGL : VertexAttributeTest.AliasingMatrixAttribLocations/ES2_OpenGL = SKIP
7872 WIN INTEL OPENGL : VertexAttributeTest.ShortUnnormalized/ES2_OpenGL = SKIP
7872 WIN INTEL OPENGL : ViewportTest.DoubleWindowCentered/ES2_OpenGL = SKIP
+8441 WIN INTEL OPENGL : GLSLTest_ES3.LargeInterfaceBlockArray/* = SKIP
+8441 WIN INTEL OPENGL : GLSLTest_ES3.LargeInterfaceBlockNestedArray/* = SKIP
// Linux
6065 LINUX INTEL VULKAN : SimpleStateChangeTestES31.DrawThenUpdateUBOThenDrawThenDrawIndexed/* = SKIP
@@ -147,6 +149,10 @@ b/273271471 WIN INTEL VULKAN : ShaderAlgorithmTest.rgb_to_hsl_vertex_shader/* =
6977 LINUX NVIDIA OpenGL : MipmapTestES31.GenerateLowerMipsWithDraw/* = SKIP
7301 LINUX NVIDIA OpenGL : CopyTexImageTest.RGBAToRGB/ES2_OpenGL_EmulateCopyTexImage2DFromRenderbuffers/* = SKIP
7371 LINUX NVIDIA OpenGL : FramebufferTest_ES3.SurfaceDimensionsChangeAndFragCoord/* = SKIP
+8441 NVIDIA OPENGL : GLSLTest_ES3.LargeInterfaceBlockArray/* = SKIP
+8441 NVIDIA OPENGL : GLSLTest_ES3.LargeInterfaceBlockNestedArray/* = SKIP
+8441 NVIDIA GLES : GLSLTest_ES3.LargeInterfaceBlockArray/* = SKIP
+8441 NVIDIA GLES : GLSLTest_ES3.LargeInterfaceBlockNestedArray/* = SKIP
// Nvidia Vulkan
7236 NVIDIA VULKAN : GLSLTest_ES31.TessellationControlShaderMatrixCopyBug/* = SKIP
@@ -1046,6 +1052,8 @@ b/273271471 WIN INTEL VULKAN : ShaderAlgorithmTest.rgb_to_hsl_vertex_shader/* =
7389 MAC OPENGL : Texture2DTest.ManySupersedingTextureUpdates/* = SKIP
8437 MAC OPENGL : GLSLTest_ES3.LotsOfFieldsInStruct/* = SKIP
+8437 MAC OPENGL : GLSLTest_ES3.LargeInterfaceBlockArray/* = SKIP
+8437 MAC OPENGL : GLSLTest_ES3.LargeInterfaceBlockNestedArray/* = SKIP
// GL, GLES run into issues with cleanup
7495 WIN OpenGL : EGLMultiContextTest.ReuseUnterminatedDisplay/* = SKIP
diff --git a/src/tests/compiler_tests/RecordConstantPrecision_test.cpp b/src/tests/compiler_tests/RecordConstantPrecision_test.cpp
index 07923f991423f4ec1ff8cbe81fb822c2b526d149..9446576ac797c0e5db8f9c63d79adff744ea488e 100644
--- a/src/tests/compiler_tests/RecordConstantPrecision_test.cpp
+++ b/src/tests/compiler_tests/RecordConstantPrecision_test.cpp
@@ -141,11 +141,11 @@ TEST_F(RecordConstantPrecisionTest, HigherPrecisionConstantInIndex)
uniform mediump float u;
void main()
{
- const highp int a = 33000;
- mediump float b[34000];
+ const highp int a = 330;
+ mediump float b[340];
gl_FragColor = vec4(b[a]);
})";
compile(shaderString);
ASSERT_FALSE(foundInCode("const highp int s"));
- ASSERT_TRUE(foundInCode("b[33000]"));
+ ASSERT_TRUE(foundInCode("b[330]"));
}
diff --git a/src/tests/gl_tests/GLSLTest.cpp b/src/tests/gl_tests/GLSLTest.cpp
index 46f8645e5a56e527223ea4657d14dbc8619cb631..344539809b19db18b125a81791117177178de687 100644
--- a/src/tests/gl_tests/GLSLTest.cpp
+++ b/src/tests/gl_tests/GLSLTest.cpp
@@ -18091,6 +18091,138 @@ void main() {
EXPECT_EQ(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
+// length higher than can fit in SPIR-V.
+TEST_P(GLSLTest_ES3, LargeInterfaceBlockArrayPassedToFunction)
+{
+ constexpr char kFS[] = R"(#version 300 es
+precision highp float;
+uniform Large { float a[65536]; };
+float f(float b[65536])
+{
+ b[0] = 1.0;
+ return b[0] + b[1];
+}
+out vec4 color;
+void main() {
+ color = vec4(f(a), 0.0, 0.0, 1.0);
+})";
+
+ GLuint shader = CompileShader(GL_FRAGMENT_SHADER, kFS);
+ EXPECT_EQ(0u, shader);
+}
+
+// Make sure the shader in LargeInterfaceBlockArrayPassedToFunction works if the large local is
+// avoided.
+TEST_P(GLSLTest_ES3, LargeInterfaceBlockArray)
+{
+ int maxUniformBlockSize = 0;
+ glGetIntegerv(GL_MAX_UNIFORM_BLOCK_SIZE, &maxUniformBlockSize);
+ ANGLE_SKIP_TEST_IF(maxUniformBlockSize < 16384 * 4);
+
+ constexpr char kFS[] = R"(#version 300 es
+precision highp float;
+uniform Large { float a[16384]; };
+out vec4 color;
+void main() {
+ color = vec4(a[0], 0.0, 0.0, 1.0);
+})";
+
+ ANGLE_GL_PROGRAM(program, essl3_shaders::vs::Simple(), kFS);
+}
+
+// Similar to LargeInterfaceBlockArrayPassedToFunction, but the array is nested in a struct.
+TEST_P(GLSLTest_ES3, LargeInterfaceBlockNestedArrayPassedToFunction)
+{
+ constexpr char kFS[] = R"(#version 300 es
+precision highp float;
+struct S { float a[65536]; };
+uniform Large { S s; };
+float f(float b[65536])
+{
+ b[0] = 1.0;
+ return b[0] + b[1];
+}
+out vec4 color;
+void main() {
+ color = vec4(f(s.a), 0.0, 0.0, 1.0);
+})";
+
+ GLuint shader = CompileShader(GL_FRAGMENT_SHADER, kFS);
+ EXPECT_EQ(0u, shader);
+}
+
+// Make sure the shader in LargeInterfaceBlockNestedArrayPassedToFunction works if the large local
+// is avoided.
+TEST_P(GLSLTest_ES3, LargeInterfaceBlockNestedArray)
+{
+ int maxUniformBlockSize = 0;
+ glGetIntegerv(GL_MAX_UNIFORM_BLOCK_SIZE, &maxUniformBlockSize);
+ ANGLE_SKIP_TEST_IF(maxUniformBlockSize < 16384 * 4);
+
+ constexpr char kFS[] = R"(#version 300 es
+precision highp float;
+struct S { float a[16384]; };
+uniform Large { S s; };
+out vec4 color;
+void main() {
+ color = vec4(s.a[0], 0.0, 0.0, 1.0);
+})";
+
+ ANGLE_GL_PROGRAM(program, essl3_shaders::vs::Simple(), kFS);
+}
+
+// Similar to LargeInterfaceBlockArrayPassedToFunction, but the large array is copied to a local
+// variable instead.
+TEST_P(GLSLTest_ES3, LargeInterfaceBlockArrayCopiedToLocal)
+{
+ constexpr char kFS[] = R"(#version 300 es
+precision highp float;
+uniform Large { float a[65536]; };
+out vec4 color;
+void main() {
+ float b[65536] = a;
+ color = vec4(b[0], 0.0, 0.0, 1.0);
+})";
+
+ GLuint shader = CompileShader(GL_FRAGMENT_SHADER, kFS);
+ EXPECT_EQ(0u, shader);
+}
+
+// Similar to LargeInterfaceBlockArrayCopiedToLocal, but the array is nested in a struct
+TEST_P(GLSLTest_ES3, LargeInterfaceBlockNestedArrayCopiedToLocal)
+{
+ constexpr char kFS[] = R"(#version 300 es
+precision highp float;
+struct S { float a[65536]; };
+uniform Large { S s; };
+out vec4 color;
+void main() {
+ S s2 = s;
+ color = vec4(s2.a[0], 0.0, 0.0, 1.0);
+})";
+
+ GLuint shader = CompileShader(GL_FRAGMENT_SHADER, kFS);
+ EXPECT_EQ(0u, shader);
+}
+
+// Test that too large varyings are rejected.
+TEST_P(GLSLTest_ES3, LargeArrayVarying)
+{
+ constexpr char kFS[] = R"(#version 300 es
+precision highp float;
+in float a[65536];
+out vec4 color;
+void main() {
+ color = vec4(a[0], 0.0, 0.0, 1.0);
+})";
+
+ GLuint shader = CompileShader(GL_FRAGMENT_SHADER, kFS);
+ EXPECT_EQ(0u, shader);
+}
+
} // anonymous namespace
ANGLE_INSTANTIATE_TEST_ES2_AND_ES3(GLSLTest);

View File

@@ -0,0 +1,180 @@
From 0000000000000000000000000000000000000000 Mon Sep 17 00:00:00 2001
From: Shahbaz Youssefi <syoussefi@chromium.org>
Date: Thu, 30 Nov 2023 13:53:00 -0500
Subject: M120: Translator: Optimize field-name-collision check
As each field of the struct was encountered, its name was linearly
checked against previously added fields. That's O(n^2).
The name collision check is now moved to when the struct is completely
defined, and is done with an unordered_map.
Bug: chromium:1505009
Change-Id: I3fbc23493e5a03e61b631af615cffaf9995fd566
Reviewed-on: https://chromium-review.googlesource.com/c/angle/angle/+/5143826
Reviewed-by: Cody Northrop <cnorthrop@google.com>
diff --git a/src/compiler/translator/ParseContext.cpp b/src/compiler/translator/ParseContext.cpp
index 28ac378cab6cb3812a43b6064733d7354ee694bc..7c90ea4f1d23a8af0cab1d44124eea021a27a16d 100644
--- a/src/compiler/translator/ParseContext.cpp
+++ b/src/compiler/translator/ParseContext.cpp
@@ -4726,6 +4726,9 @@ TIntermDeclaration *TParseContext::addInterfaceBlock(
const TVector<unsigned int> *arraySizes,
const TSourceLoc &arraySizesLine)
{
+ // Ensure there are no duplicate field names
+ checkDoesNotHaveDuplicateFieldNames(fieldList, nameLine);
+
const bool isGLPerVertex = blockName == "gl_PerVertex";
// gl_PerVertex is allowed to be redefined and therefore not reserved
if (!isGLPerVertex)
@@ -6269,28 +6272,25 @@ TDeclarator *TParseContext::parseStructArrayDeclarator(const ImmutableString &id
return new TDeclarator(identifier, arraySizes, loc);
}
-void TParseContext::checkDoesNotHaveDuplicateFieldName(const TFieldList::const_iterator begin,
- const TFieldList::const_iterator end,
- const ImmutableString &name,
- const TSourceLoc &location)
+void TParseContext::checkDoesNotHaveDuplicateFieldNames(const TFieldList *fields,
+ const TSourceLoc &location)
{
- for (auto fieldIter = begin; fieldIter != end; ++fieldIter)
+ TUnorderedMap<ImmutableString, uint32_t, ImmutableString::FowlerNollVoHash<sizeof(size_t)>>
+ fieldNames;
+ for (TField *field : *fields)
{
- if ((*fieldIter)->name() == name)
+ // Note: operator[] adds this name to the map if it doesn't already exist, and initializes
+ // its value to 0.
+ uint32_t count = ++fieldNames[field->name()];
+ if (count != 1)
{
- error(location, "duplicate field name in structure", name);
+ error(location, "Duplicate field name in structure", field->name());
}
}
}
TFieldList *TParseContext::addStructFieldList(TFieldList *fields, const TSourceLoc &location)
{
- for (TFieldList::const_iterator fieldIter = fields->begin(); fieldIter != fields->end();
- ++fieldIter)
- {
- checkDoesNotHaveDuplicateFieldName(fields->begin(), fieldIter, (*fieldIter)->name(),
- location);
- }
return fields;
}
@@ -6298,12 +6298,8 @@ TFieldList *TParseContext::combineStructFieldLists(TFieldList *processedFields,
const TFieldList *newlyAddedFields,
const TSourceLoc &location)
{
- for (TField *field : *newlyAddedFields)
- {
- checkDoesNotHaveDuplicateFieldName(processedFields->begin(), processedFields->end(),
- field->name(), location);
- processedFields->push_back(field);
- }
+ processedFields->insert(processedFields->end(), newlyAddedFields->begin(),
+ newlyAddedFields->end());
return processedFields;
}
@@ -6396,7 +6392,10 @@ TTypeSpecifierNonArray TParseContext::addStructure(const TSourceLoc &structLine,
}
}
- // ensure we do not specify any storage qualifiers on the struct members
+ // Ensure there are no duplicate field names
+ checkDoesNotHaveDuplicateFieldNames(fieldList, structLine);
+
+ // Ensure we do not specify any storage qualifiers on the struct members
for (unsigned int typeListIndex = 0; typeListIndex < fieldList->size(); typeListIndex++)
{
TField &field = *(*fieldList)[typeListIndex];
diff --git a/src/compiler/translator/ParseContext.h b/src/compiler/translator/ParseContext.h
index 9e1354ef816705fb512b40b329794e0282129807..b63dbbadd146d1a004513823de72c89240a31929 100644
--- a/src/compiler/translator/ParseContext.h
+++ b/src/compiler/translator/ParseContext.h
@@ -354,10 +354,7 @@ class TParseContext : angle::NonCopyable
const TSourceLoc &loc,
const TVector<unsigned int> *arraySizes);
- void checkDoesNotHaveDuplicateFieldName(const TFieldList::const_iterator begin,
- const TFieldList::const_iterator end,
- const ImmutableString &name,
- const TSourceLoc &location);
+ void checkDoesNotHaveDuplicateFieldNames(const TFieldList *fields, const TSourceLoc &location);
TFieldList *addStructFieldList(TFieldList *fields, const TSourceLoc &location);
TFieldList *combineStructFieldLists(TFieldList *processedFields,
const TFieldList *newlyAddedFields,
diff --git a/src/tests/angle_end2end_tests_expectations.txt b/src/tests/angle_end2end_tests_expectations.txt
index cadf4afaed84c10040b87c8936c4a0ebbfd6d514..3d356609031820b92c230ac7ff836327b78ec834 100644
--- a/src/tests/angle_end2end_tests_expectations.txt
+++ b/src/tests/angle_end2end_tests_expectations.txt
@@ -1045,6 +1045,8 @@ b/273271471 WIN INTEL VULKAN : ShaderAlgorithmTest.rgb_to_hsl_vertex_shader/* =
7389 SWIFTSHADER : Texture2DTest.ManySupersedingTextureUpdates/* = SKIP
7389 MAC OPENGL : Texture2DTest.ManySupersedingTextureUpdates/* = SKIP
+8437 MAC OPENGL : GLSLTest_ES3.LotsOfFieldsInStruct/* = SKIP
+
// GL, GLES run into issues with cleanup
7495 WIN OpenGL : EGLMultiContextTest.ReuseUnterminatedDisplay/* = SKIP
7495 WIN GLES : EGLMultiContextTest.ReuseUnterminatedDisplay/* = SKIP
diff --git a/src/tests/gl_tests/GLSLTest.cpp b/src/tests/gl_tests/GLSLTest.cpp
index 0d0d3b5468019b897af2487d72112fab829f47da..9b42b6d9221366e9bc7b6263bf8e14b77ca7694d 100644
--- a/src/tests/gl_tests/GLSLTest.cpp
+++ b/src/tests/gl_tests/GLSLTest.cpp
@@ -18020,6 +18020,50 @@ TEST_P(GLSLTest_ES31, ESSL31ExtensionMacros)
ASSERT_GL_NO_ERROR();
}
+// Test that Metal compiler doesn't inline non-const globals
+TEST_P(WebGLGLSLTest, InvalidGlobalsNotInlined)
+{
+ constexpr char kFS[] = R"(#version 100
+ precision highp float;
+ float v1 = 0.5;
+ float v2 = v1;
+
+ float f1() {
+ return v2;
+ }
+
+ void main() {
+ gl_FragColor = vec4(v1 + f1(),0.0,0.0, 1.0);
+ })";
+ ANGLE_GL_PROGRAM(program, essl1_shaders::vs::Simple(), kFS);
+ ASSERT_GL_NO_ERROR();
+}
+
+// Test that a struct can have lots of fields. Regression test for an inefficient O(n^2) check for
+// fields having unique names.
+TEST_P(GLSLTest_ES3, LotsOfFieldsInStruct)
+{
+ std::ostringstream fs;
+ fs << R"(#version 300 es
+precision highp float;
+struct LotsOfFields
+{
+)";
+ // Note: 16383 is the SPIR-V limit for struct member count.
+ for (uint32_t i = 0; i < 16383; ++i)
+ {
+ fs << " float field" << i << ";\n";
+ }
+ fs << R"(};
+uniform B { LotsOfFields s; };
+out vec4 color;
+void main() {
+ color = vec4(s.field0, 0.0, 0.0, 1.0);
+})";
+
+ ANGLE_GL_PROGRAM(program, essl3_shaders::vs::Simple(), fs.str().c_str());
+}
+
} // anonymous namespace
ANGLE_INSTANTIATE_TEST_ES2_AND_ES3(GLSLTest);

View File

@@ -0,0 +1,135 @@
From 0000000000000000000000000000000000000000 Mon Sep 17 00:00:00 2001
From: Shahbaz Youssefi <syoussefi@chromium.org>
Date: Tue, 5 Dec 2023 13:36:53 -0500
Subject: M120: Vulkan: Don't crash when glCopyTexImage2D redefines itself
The Vulkan backend marks a level being redefined as such before doing
the copy. If a single-level texture was being redefined, it releases it
so it can be immediately reallocated. If the source of the copy is the
same texture, this causes a crash.
This can be properly supported by using a temp image to do the copy, but
that is not implemented in this change.
Bug: chromium:1501798
Change-Id: I3a902b1e9eec41afd385d9c75a8c95dc986070a8
Reviewed-on: https://chromium-review.googlesource.com/c/angle/angle/+/5143829
Reviewed-by: Cody Northrop <cnorthrop@google.com>
diff --git a/src/libANGLE/renderer/vulkan/TextureVk.cpp b/src/libANGLE/renderer/vulkan/TextureVk.cpp
index da27d3cfb0932408f14cd2fd6df88294f4b07363..885982b95e70f0c49f89b565fa0f3331333eaac1 100644
--- a/src/libANGLE/renderer/vulkan/TextureVk.cpp
+++ b/src/libANGLE/renderer/vulkan/TextureVk.cpp
@@ -723,8 +723,28 @@ angle::Result TextureVk::copyImage(const gl::Context *context,
gl::GetInternalFormatInfo(internalFormat, GL_UNSIGNED_BYTE);
const vk::Format &vkFormat = renderer->getFormat(internalFormatInfo.sizedInternalFormat);
+ // The texture level being redefined might be the same as the one bound to the framebuffer.
+ // This _could_ be supported by using a temp image before redefining the level (and potentially
+ // discarding the image). However, this is currently unimplemented.
+ FramebufferVk *framebufferVk = vk::GetImpl(source);
+ RenderTargetVk *colorReadRT = framebufferVk->getColorReadRenderTarget();
+ vk::ImageHelper *srcImage = &colorReadRT->getImageForCopy();
+ const bool isCubeMap = index.getType() == gl::TextureType::CubeMap;
+ gl::LevelIndex levelIndex(getNativeImageIndex(index).getLevelIndex());
+ const uint32_t layerIndex = index.hasLayer() ? index.getLayerIndex() : 0;
+ const uint32_t redefinedFace = isCubeMap ? layerIndex : 0;
+ const uint32_t sourceFace = isCubeMap ? colorReadRT->getLayerIndex() : 0;
+ const bool isSelfCopy = mImage == srcImage && levelIndex == colorReadRT->getLevelIndex() &&
+ redefinedFace == sourceFace;
+
ANGLE_TRY(redefineLevel(context, index, vkFormat, newImageSize));
+ if (isSelfCopy)
+ {
+ UNIMPLEMENTED();
+ return angle::Result::Continue;
+ }
+
return copySubImageImpl(context, index, gl::Offset(0, 0, 0), sourceArea, internalFormatInfo,
source);
}
@@ -1798,7 +1818,8 @@ angle::Result TextureVk::redefineLevel(const gl::Context *context,
mImage->getLevelCount() == 1 && mImage->getFirstAllocatedLevel() == levelIndexGL;
// If incompatible, and redefining the single-level image, release it so it can be
- // recreated immediately. This is an optimization to avoid an extra copy.
+ // recreated immediately. This is needed so that the texture can be reallocated with
+ // the correct format/size.
if (!isCompatibleRedefinition && isUpdateToSingleLevelImage)
{
releaseImage(contextVk);
diff --git a/src/tests/angle_end2end_tests_expectations.txt b/src/tests/angle_end2end_tests_expectations.txt
index cfa70eef00a1e8d67f9fbf07ee74928b55da8b92..8b4dac5beab0b8d884116147ba3e61f6133e92a9 100644
--- a/src/tests/angle_end2end_tests_expectations.txt
+++ b/src/tests/angle_end2end_tests_expectations.txt
@@ -29,6 +29,8 @@
6989 GLES : BlitFramebufferTestES31.OOBResolve/* = SKIP
7881 VULKAN : MultithreadingTestES3.UnsynchronizedTextureReads/* = SKIP
7881 VULKAN : MultithreadingTestES3.UnsynchronizedTextureReads2/* = SKIP
+// Incorrectly handled pretty much in all backends
+8446 : CopyTexImageTestES3.RedefineSameLevel/* = SKIP
6743 OPENGL : SimpleStateChangeTestES3.RespecifyBufferAfterBeginTransformFeedback/* = SKIP
6743 GLES : SimpleStateChangeTestES3.RespecifyBufferAfterBeginTransformFeedback/* = SKIP
diff --git a/src/tests/gl_tests/CopyTexImageTest.cpp b/src/tests/gl_tests/CopyTexImageTest.cpp
index 3d0cf40ab244a5463c2e4b3d53470d6f932e357b..d6949280ed301fc918e397dad26b9efec1c32f23 100644
--- a/src/tests/gl_tests/CopyTexImageTest.cpp
+++ b/src/tests/gl_tests/CopyTexImageTest.cpp
@@ -1262,6 +1262,56 @@ TEST_P(CopyTexImageTestES3, 3DSubImageDrawMismatchedTextureTypes)
glBindTexture(GL_TEXTURE_3D, 0);
}
+// Make sure a single-level texture can be redefined through glCopyTexImage2D from a framebuffer
+// bound to the same texture. Regression test for a bug in the Vulkan backend where the texture was
+// released before the copy.
+TEST_P(CopyTexImageTestES3, RedefineSameLevel)
+{
+ constexpr GLsizei kSize = 32;
+ constexpr GLsizei kHalfSize = kSize / 2;
+
+ // Create a single-level texture with four colors in different regions.
+ std::vector<GLColor> initData(kSize * kSize);
+ for (GLsizei y = 0; y < kSize; ++y)
+ {
+ const bool isTop = y < kHalfSize;
+ for (GLsizei x = 0; x < kSize; ++x)
+ {
+ const bool isLeft = x < kHalfSize;
+
+ GLColor color = isLeft && isTop ? GLColor::red
+ : isLeft && !isTop ? GLColor::green
+ : !isLeft && isTop ? GLColor::blue
+ : GLColor::yellow;
+ color.A = 123;
+ initData[y * kSize + x] = color;
+ }
+ }
+
+ GLTexture tex;
+ glBindTexture(GL_TEXTURE_2D, tex);
+ glTexImage2D(GL_TEXTURE_2D, 0, GL_RGBA, kSize, kSize, 0, GL_RGBA, GL_UNSIGNED_BYTE,
+ initData.data());
+
+ // Bind the framebuffer to the same texture
+ GLFramebuffer framebuffer;
+ glBindFramebuffer(GL_FRAMEBUFFER, framebuffer);
+ glFramebufferTexture2D(GL_FRAMEBUFFER, GL_COLOR_ATTACHMENT0, GL_TEXTURE_2D, tex, 0);
+
+ // Redefine the texture
+ glCopyTexImage2D(GL_TEXTURE_2D, 0, GL_RGB, kHalfSize / 2, kHalfSize / 2, kHalfSize, kHalfSize,
+ 0);
+
+ // Verify copy is done correctly.
+ ASSERT_GL_FRAMEBUFFER_COMPLETE(GL_FRAMEBUFFER);
+
+ EXPECT_PIXEL_RECT_EQ(0, 0, kHalfSize / 2, kHalfSize / 2, GLColor::red);
+ EXPECT_PIXEL_RECT_EQ(kHalfSize / 2, 0, kHalfSize / 2, kHalfSize / 2, GLColor::blue);
+ EXPECT_PIXEL_RECT_EQ(0, kHalfSize / 2, kHalfSize / 2, kHalfSize / 2, GLColor::green);
+ EXPECT_PIXEL_RECT_EQ(kHalfSize / 2, kHalfSize / 2, kHalfSize / 2, kHalfSize / 2,
+ GLColor::yellow);
+}
+
ANGLE_INSTANTIATE_TEST(CopyTexImageTest,
ANGLE_ALL_TEST_PLATFORMS_ES2,
ES2_D3D11_PRESENT_PATH_FAST(),

View File

@@ -152,3 +152,5 @@ cherry-pick-021598ea43c1.patch
cherry-pick-76340163a820.patch
cherry-pick-f15cfb9371c4.patch
cherry-pick-4ca62c7a8b88.patch
cherry-pick-5b2fddadaa12.patch
cherry-pick-50a1bddfca85.patch

View File

@@ -0,0 +1,117 @@
From 0000000000000000000000000000000000000000 Mon Sep 17 00:00:00 2001
From: Austin Eng <enga@chromium.org>
Date: Tue, 19 Dec 2023 17:25:51 +0000
Subject: Use cross thread handles to bind args for async webgpu context
creation
(cherry picked from commit 542b278a0c1de7202f4bf5e3e5cbdc2dd6c337d4)
Fixed: 1506923
Change-Id: I174703cbd993471e3afb39c0cfa4cce2770755f7
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/5113019
Reviewed-by: Corentin Wallez <cwallez@chromium.org>
Commit-Queue: Austin Eng <enga@chromium.org>
Reviewed-by: Stephen White <senorblanco@chromium.org>
Cr-Original-Commit-Position: refs/heads/main@{#1237179}
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/5133239
Cr-Commit-Position: refs/branch-heads/6099@{#1551}
Cr-Branched-From: e6ee4500f7d6549a9ac1354f8d056da49ef406be-refs/heads/main@{#1217362}
diff --git a/third_party/blink/renderer/modules/webgpu/gpu.cc b/third_party/blink/renderer/modules/webgpu/gpu.cc
index dbe8ab27798ca1a9aa80f34cbfc88f52adfe3b84..9627347a3b495dffa20f3aa0cfbcaa524eced686 100644
--- a/third_party/blink/renderer/modules/webgpu/gpu.cc
+++ b/third_party/blink/renderer/modules/webgpu/gpu.cc
@@ -39,11 +39,13 @@
#include "third_party/blink/renderer/platform/graphics/gpu/dawn_control_client_holder.h"
#include "third_party/blink/renderer/platform/graphics/gpu/webgpu_callback.h"
#include "third_party/blink/renderer/platform/graphics/web_graphics_context_3d_provider_util.h"
+#include "third_party/blink/renderer/platform/heap/cross_thread_handle.h"
#include "third_party/blink/renderer/platform/heap/garbage_collected.h"
#include "third_party/blink/renderer/platform/heap/thread_state.h"
#include "third_party/blink/renderer/platform/instrumentation/use_counter.h"
#include "third_party/blink/renderer/platform/privacy_budget/identifiability_digest_helpers.h"
#include "third_party/blink/renderer/platform/weborigin/kurl.h"
+#include "third_party/blink/renderer/platform/wtf/cross_thread_functional.h"
namespace blink {
@@ -300,9 +302,19 @@ void GPU::RequestAdapterImpl(ScriptState* script_state,
CreateWebGPUGraphicsContext3DProviderAsync(
execution_context->Url(),
execution_context->GetTaskRunner(TaskType::kWebGPU),
- WTF::BindOnce(
- [](GPU* gpu, ExecutionContext* execution_context,
+ CrossThreadBindOnce(
+ [](CrossThreadHandle<GPU> gpu_handle,
+ CrossThreadHandle<ExecutionContext> execution_context_handle,
std::unique_ptr<WebGraphicsContext3DProvider> context_provider) {
+ auto unwrap_gpu = MakeUnwrappingCrossThreadHandle(gpu_handle);
+ auto unwrap_execution_context =
+ MakeUnwrappingCrossThreadHandle(execution_context_handle);
+ if (!unwrap_gpu || !unwrap_execution_context) {
+ return;
+ }
+ auto* gpu = unwrap_gpu.GetOnCreationThread();
+ auto* execution_context =
+ unwrap_execution_context.GetOnCreationThread();
const KURL& url = execution_context->Url();
context_provider =
CheckContextProvider(url, std::move(context_provider));
@@ -324,7 +336,8 @@ void GPU::RequestAdapterImpl(ScriptState* script_state,
std::move(callback).Run();
}
},
- WrapPersistent(this), WrapPersistent(execution_context)));
+ MakeCrossThreadHandle(this),
+ MakeCrossThreadHandle(execution_context)));
return;
}
diff --git a/third_party/blink/renderer/platform/graphics/web_graphics_context_3d_provider_util.cc b/third_party/blink/renderer/platform/graphics/web_graphics_context_3d_provider_util.cc
index f859f3e62c54d26453a145321f697c5116c13348..3d9890b9b4a58a30a11e501fdb9297f4a57b601b 100644
--- a/third_party/blink/renderer/platform/graphics/web_graphics_context_3d_provider_util.cc
+++ b/third_party/blink/renderer/platform/graphics/web_graphics_context_3d_provider_util.cc
@@ -121,8 +121,8 @@ CreateWebGPUGraphicsContext3DProvider(const KURL& url) {
void CreateWebGPUGraphicsContext3DProviderAsync(
const KURL& url,
scoped_refptr<base::SingleThreadTaskRunner> current_thread_task_runner,
- base::OnceCallback<void(std::unique_ptr<WebGraphicsContext3DProvider>)>
- callback) {
+ WTF::CrossThreadOnceFunction<
+ void(std::unique_ptr<WebGraphicsContext3DProvider>)> callback) {
if (IsMainThread()) {
std::move(callback).Run(
Platform::Current()->CreateWebGPUGraphicsContext3DProvider(url));
@@ -140,8 +140,7 @@ void CreateWebGPUGraphicsContext3DProviderAsync(
AccessMainThreadForWebGraphicsContext3DProvider()),
FROM_HERE,
CrossThreadBindOnce(&CreateWebGPUGraphicsContextOnMainThreadAsync, url,
- current_thread_task_runner,
- CrossThreadBindOnce(std::move(callback))));
+ current_thread_task_runner, std::move(callback)));
}
}
diff --git a/third_party/blink/renderer/platform/graphics/web_graphics_context_3d_provider_util.h b/third_party/blink/renderer/platform/graphics/web_graphics_context_3d_provider_util.h
index 8fcab24bfec2c9b2e9edf9885b66de4f99949b35..8b785cc30acdfffed0f59eb53b073d0cdedc2151 100644
--- a/third_party/blink/renderer/platform/graphics/web_graphics_context_3d_provider_util.h
+++ b/third_party/blink/renderer/platform/graphics/web_graphics_context_3d_provider_util.h
@@ -10,6 +10,7 @@
#include "third_party/blink/public/platform/web_graphics_context_3d_provider.h"
#include "third_party/blink/renderer/platform/platform_export.h"
#include "third_party/blink/renderer/platform/weborigin/kurl.h"
+#include "third_party/blink/renderer/platform/wtf/functional.h"
namespace blink {
@@ -42,8 +43,8 @@ CreateWebGPUGraphicsContext3DProvider(const KURL& url);
PLATFORM_EXPORT void CreateWebGPUGraphicsContext3DProviderAsync(
const KURL& url,
scoped_refptr<base::SingleThreadTaskRunner> current_thread_task_runner,
- base::OnceCallback<void(std::unique_ptr<WebGraphicsContext3DProvider>)>
- callback);
+ WTF::CrossThreadOnceFunction<
+ void(std::unique_ptr<WebGraphicsContext3DProvider>)> callback);
} // namespace blink

View File

@@ -0,0 +1,61 @@
From 0000000000000000000000000000000000000000 Mon Sep 17 00:00:00 2001
From: Hongchan Choi <hongchan@chromium.org>
Date: Tue, 12 Dec 2023 02:34:29 +0000
Subject: Clamp the input value correctly before scheduling an AudioParam event
When the AudioParam value is set via the setter, it internally calls
the setValueAtTime() function to schedule the change. However, the
current code does not correctly clamp the value within the nominal
range. This CL fixes the problem.
(cherry picked from commit c97b506c1e32951dd39e11e453e1ecc29cc0b35c)
Bug: 1505086
Test: Locally confirmed with both negative and positive param values.
Change-Id: Ibb0aae168161af9ea95c5e11a929b3aa2c621c73
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/5100625
Reviewed-by: Michael Wilson <mjwilson@chromium.org>
Commit-Queue: Hongchan Choi <hongchan@chromium.org>
Cr-Original-Commit-Position: refs/heads/main@{#1235028}
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/5112838
Commit-Queue: Rubber Stamper <rubber-stamper@appspot.gserviceaccount.com>
Bot-Commit: Rubber Stamper <rubber-stamper@appspot.gserviceaccount.com>
Auto-Submit: Hongchan Choi <hongchan@chromium.org>
Cr-Commit-Position: refs/branch-heads/6099@{#1497}
Cr-Branched-From: e6ee4500f7d6549a9ac1354f8d056da49ef406be-refs/heads/main@{#1217362}
diff --git a/third_party/blink/renderer/modules/webaudio/audio_param.cc b/third_party/blink/renderer/modules/webaudio/audio_param.cc
index 8c7b9d07bb68ec51d21ea2132cc5ecbc39e5cd95..95a40d39c9214fd6555523bd7e7bd91e36d2c6c0 100644
--- a/third_party/blink/renderer/modules/webaudio/audio_param.cc
+++ b/third_party/blink/renderer/modules/webaudio/audio_param.cc
@@ -120,12 +120,15 @@ void AudioParam::setValue(float value) {
void AudioParam::setValue(float value, ExceptionState& exception_state) {
WarnIfOutsideRange("value", value);
- // This is to signal any errors, if necessary, about conflicting
- // automations.
- setValueAtTime(value, Context()->currentTime(), exception_state);
- // This is to change the value so that an immediate query for the
- // value returns the expected values.
+ // Change the intrinsic value so that an immediate query for the value
+ // returns the value that the user code provided. It also clamps the value
+ // to the nominal range.
Handler().SetValue(value);
+
+ // Use the intrinsic value (after clamping) to schedule the actual
+ // automation event.
+ setValueAtTime(Handler().IntrinsicValue(), Context()->currentTime(),
+ exception_state);
}
float AudioParam::defaultValue() const {
diff --git a/third_party/blink/web_tests/webaudio/AudioParam/worklet-warnings-expected.txt b/third_party/blink/web_tests/webaudio/AudioParam/worklet-warnings-expected.txt
index 7bb2d0aec7feaed69424f209a2e3e031c7a9e512..ebe05a2c239d35be4729cc187aa77de6a44f5a41 100644
--- a/third_party/blink/web_tests/webaudio/AudioParam/worklet-warnings-expected.txt
+++ b/third_party/blink/web_tests/webaudio/AudioParam/worklet-warnings-expected.txt
@@ -1,5 +1,4 @@
CONSOLE WARNING: AudioWorkletNode("noise-generator").amplitude.value 99 outside nominal range [0, 1]; value will be clamped.
-CONSOLE WARNING: AudioWorkletNode("noise-generator").amplitude.setValueAtTime value 99 outside nominal range [0, 1]; value will be clamped.
CONSOLE WARNING: AudioWorkletNode("noise-generator").amplitude.setValueAtTime value -1 outside nominal range [0, 1]; value will be clamped.
CONSOLE WARNING: AudioWorkletNode("noise-generator").amplitude.linearRampToValueAtTime value 5 outside nominal range [0, 1]; value will be clamped.
This is a testharness.js-based test.

View File

@@ -27,5 +27,9 @@
"src/electron/patches/webrtc": "src/third_party/webrtc",
"src/electron/patches/dawn": "src/third_party/dawn"
"src/electron/patches/dawn": "src/third_party/dawn",
"src/electron/patches/angle": "src/third_party/angle",
"src/electron/patches/sqlite": "src/third_party/sqlite/src"
}

1
patches/sqlite/.patches Normal file
View File

@@ -0,0 +1 @@
fix_a_spurious_misuse_of_aggregate_function_error_that_could_occur.patch

View File

@@ -0,0 +1,323 @@
From 0000000000000000000000000000000000000000 Mon Sep 17 00:00:00 2001
From: dan <Dan Kennedy>
Date: Thu, 2 Nov 2023 21:02:53 +0000
Subject: Fix a spurious "misuse of aggregate function" error that could occur
when an aggregate function was used within the FROM clause of a sub-select of
the select that owns the aggregate. e.g. "SELECT (SELECT x FROM (SELECT
sum(t1.a) AS x)) FROM t1". [forum:/forumpost/c9970a37ed | Forum post
c9970a37ed].
FossilOrigin-Name: 4470f657d2069972d02a00983252dec1f814d90c0d8d0906e320e955111e8c11
(cherry picked from commit 5e4233a9e48b124d4d342b757b34e4ae849f5cf8)
diff --git a/amalgamation/rename_exports.h b/amalgamation/rename_exports.h
index 1dd9873cd698b9708c1e0897e6f270b5dd4e20c1..3b9835c71a5117dfeb249f30d930047c8722962c 100644
--- a/amalgamation/rename_exports.h
+++ b/amalgamation/rename_exports.h
@@ -1,4 +1,4 @@
-// Copyright 2023 The Chromium Authors. All rights reserved.
+// Copyright 2024 The Chromium Authors. All rights reserved.
// Use of this source code is governed by a BSD-style license that can be
// found in the LICENSE file.
diff --git a/amalgamation/sqlite3.c b/amalgamation/sqlite3.c
index 19637a9234893df86d3d7b12b16617bca56f36e4..3b0548b328bc750665c7d0a67302c5f6097fde4e 100644
--- a/amalgamation/sqlite3.c
+++ b/amalgamation/sqlite3.c
@@ -458,7 +458,7 @@ extern "C" {
*/
#define SQLITE_VERSION "3.42.0"
#define SQLITE_VERSION_NUMBER 3042000
-#define SQLITE_SOURCE_ID "2023-05-16 12:36:15 ec7dafdf189a95ffcd452db5e3b5370d8f7007c56a7dee464cc700fdd1027368"
+#define SQLITE_SOURCE_ID "2023-05-16 12:36:15 19c0cc92aaff58e70789b761d7dcd23bc81869624e3aafea35673b5c3939a6ff"
/*
** CAPI3REF: Run-Time Library Version Numbers
@@ -18921,6 +18921,7 @@ struct NameContext {
int nRef; /* Number of names resolved by this context */
int nNcErr; /* Number of errors encountered while resolving names */
int ncFlags; /* Zero or more NC_* flags defined below */
+ int nNestedSelect; /* Number of nested selects using this NC */
Select *pWinSelect; /* SELECT statement for any window functions */
};
@@ -105274,11 +105275,12 @@ static int resolveExprStep(Walker *pWalker, Expr *pExpr){
while( pNC2
&& sqlite3ReferencesSrcList(pParse, pExpr, pNC2->pSrcList)==0
){
- pExpr->op2++;
+ pExpr->op2 += (1 + pNC2->nNestedSelect);
pNC2 = pNC2->pNext;
}
assert( pDef!=0 || IN_RENAME_OBJECT );
if( pNC2 && pDef ){
+ pExpr->op2 += pNC2->nNestedSelect;
assert( SQLITE_FUNC_MINMAX==NC_MinMaxAgg );
assert( SQLITE_FUNC_ANYORDER==NC_OrderAgg );
testcase( (pDef->funcFlags & SQLITE_FUNC_MINMAX)!=0 );
@@ -105839,6 +105841,7 @@ static int resolveSelectStep(Walker *pWalker, Select *p){
/* Recursively resolve names in all subqueries in the FROM clause
*/
+ if( pOuterNC ) pOuterNC->nNestedSelect++;
for(i=0; i<p->pSrc->nSrc; i++){
SrcItem *pItem = &p->pSrc->a[i];
if( pItem->pSelect && (pItem->pSelect->selFlags & SF_Resolved)==0 ){
@@ -105863,6 +105866,7 @@ static int resolveSelectStep(Walker *pWalker, Select *p){
}
}
}
+ if( pOuterNC ) pOuterNC->nNestedSelect--;
/* Set up the local name-context to pass to sqlite3ResolveExprNames() to
** resolve the result-set expression list.
diff --git a/amalgamation/sqlite3.h b/amalgamation/sqlite3.h
index 820dbeeb157b79d3380e089f1fdd5862a4cab087..c3738817186bbfd60aa7d19a3327d6a4ca118bc3 100644
--- a/amalgamation/sqlite3.h
+++ b/amalgamation/sqlite3.h
@@ -148,7 +148,7 @@ extern "C" {
*/
#define SQLITE_VERSION "3.42.0"
#define SQLITE_VERSION_NUMBER 3042000
-#define SQLITE_SOURCE_ID "2023-05-16 12:36:15 ec7dafdf189a95ffcd452db5e3b5370d8f7007c56a7dee464cc700fdd1027368"
+#define SQLITE_SOURCE_ID "2023-05-16 12:36:15 19c0cc92aaff58e70789b761d7dcd23bc81869624e3aafea35673b5c3939a6ff"
/*
** CAPI3REF: Run-Time Library Version Numbers
diff --git a/amalgamation_dev/rename_exports.h b/amalgamation_dev/rename_exports.h
index 1dd9873cd698b9708c1e0897e6f270b5dd4e20c1..3b9835c71a5117dfeb249f30d930047c8722962c 100644
--- a/amalgamation_dev/rename_exports.h
+++ b/amalgamation_dev/rename_exports.h
@@ -1,4 +1,4 @@
-// Copyright 2023 The Chromium Authors. All rights reserved.
+// Copyright 2024 The Chromium Authors. All rights reserved.
// Use of this source code is governed by a BSD-style license that can be
// found in the LICENSE file.
diff --git a/amalgamation_dev/sqlite3.c b/amalgamation_dev/sqlite3.c
index 36a6a8306fffaf38ff6d2feb6541c0eccd8df8fc..b10707a5fb419fc64694de2642371e43899022b8 100644
--- a/amalgamation_dev/sqlite3.c
+++ b/amalgamation_dev/sqlite3.c
@@ -458,7 +458,7 @@ extern "C" {
*/
#define SQLITE_VERSION "3.42.0"
#define SQLITE_VERSION_NUMBER 3042000
-#define SQLITE_SOURCE_ID "2023-05-16 12:36:15 ec7dafdf189a95ffcd452db5e3b5370d8f7007c56a7dee464cc700fdd1027368"
+#define SQLITE_SOURCE_ID "2023-05-16 12:36:15 19c0cc92aaff58e70789b761d7dcd23bc81869624e3aafea35673b5c3939a6ff"
/*
** CAPI3REF: Run-Time Library Version Numbers
@@ -18934,6 +18934,7 @@ struct NameContext {
int nRef; /* Number of names resolved by this context */
int nNcErr; /* Number of errors encountered while resolving names */
int ncFlags; /* Zero or more NC_* flags defined below */
+ int nNestedSelect; /* Number of nested selects using this NC */
Select *pWinSelect; /* SELECT statement for any window functions */
};
@@ -105287,11 +105288,12 @@ static int resolveExprStep(Walker *pWalker, Expr *pExpr){
while( pNC2
&& sqlite3ReferencesSrcList(pParse, pExpr, pNC2->pSrcList)==0
){
- pExpr->op2++;
+ pExpr->op2 += (1 + pNC2->nNestedSelect);
pNC2 = pNC2->pNext;
}
assert( pDef!=0 || IN_RENAME_OBJECT );
if( pNC2 && pDef ){
+ pExpr->op2 += pNC2->nNestedSelect;
assert( SQLITE_FUNC_MINMAX==NC_MinMaxAgg );
assert( SQLITE_FUNC_ANYORDER==NC_OrderAgg );
testcase( (pDef->funcFlags & SQLITE_FUNC_MINMAX)!=0 );
@@ -105852,6 +105854,7 @@ static int resolveSelectStep(Walker *pWalker, Select *p){
/* Recursively resolve names in all subqueries in the FROM clause
*/
+ if( pOuterNC ) pOuterNC->nNestedSelect++;
for(i=0; i<p->pSrc->nSrc; i++){
SrcItem *pItem = &p->pSrc->a[i];
if( pItem->pSelect && (pItem->pSelect->selFlags & SF_Resolved)==0 ){
@@ -105876,6 +105879,7 @@ static int resolveSelectStep(Walker *pWalker, Select *p){
}
}
}
+ if( pOuterNC ) pOuterNC->nNestedSelect--;
/* Set up the local name-context to pass to sqlite3ResolveExprNames() to
** resolve the result-set expression list.
diff --git a/amalgamation_dev/sqlite3.h b/amalgamation_dev/sqlite3.h
index 820dbeeb157b79d3380e089f1fdd5862a4cab087..c3738817186bbfd60aa7d19a3327d6a4ca118bc3 100644
--- a/amalgamation_dev/sqlite3.h
+++ b/amalgamation_dev/sqlite3.h
@@ -148,7 +148,7 @@ extern "C" {
*/
#define SQLITE_VERSION "3.42.0"
#define SQLITE_VERSION_NUMBER 3042000
-#define SQLITE_SOURCE_ID "2023-05-16 12:36:15 ec7dafdf189a95ffcd452db5e3b5370d8f7007c56a7dee464cc700fdd1027368"
+#define SQLITE_SOURCE_ID "2023-05-16 12:36:15 19c0cc92aaff58e70789b761d7dcd23bc81869624e3aafea35673b5c3939a6ff"
/*
** CAPI3REF: Run-Time Library Version Numbers
diff --git a/manifest b/manifest
index 0841fab32a918f21970cf1806a984261f1aaccfb..e7b2f86954fa8078cb3211b5e5cfcb319b50b6dc 100644
--- a/manifest
+++ b/manifest
@@ -634,14 +634,14 @@ F src/pragma.h e690a356c18e98414d2e870ea791c1be1545a714ba623719deb63f7f226d8bb7
F src/prepare.c 6350675966bd0e7ac3a464af9dbfe26db6f0d4237f4e1f1acdb17b12ad371e6e
F src/printf.c b9320cdbeca0b336c3f139fd36dd121e4167dd62b35fbe9ccaa9bab44c0af38d
F src/random.c 606b00941a1d7dd09c381d3279a058d771f406c5213c9932bbd93d5587be4b9c
-F src/resolve.c 3e53e02ce87c9582bd7e7d22f13f4094a271678d9dc72820fa257a2abb5e4032
+F src/resolve.c 9bcc9021a5b849ba8ccd2103147b75a3a98d885e00212b48672fe1ed8356338b
F src/rowset.c ba9515a922af32abe1f7d39406b9d35730ed65efab9443dc5702693b60854c92
F src/select.c 738c3a3d6929f8be66c319bad17f6b297bd60a4eb14006075c48a28487dc7786
F src/shell.c.in a8971a2ae4adee5f0a73d7a6c095026e8a2958ff3e9f56887a014df07733ca0c
F src/sqlite.h.in c14a4471fcd897a03631ac7ad3d05505e895e7b6419ec5b96cae9bc4df7a9fc6
F src/sqlite3.rc 5121c9e10c3964d5755191c80dd1180c122fc3a8
F src/sqlite3ext.h da473ce2b3d0ae407a6300c4a164589b9a6bfdbec9462688a8593ff16f3bb6e4
-F src/sqliteInt.h a3ced8b9ebc573189c87b69f24bf10d2b9cd3cefefaae52623a2fa79e6fdd408
+F src/sqliteInt.h 522f19804d86e193dbfd966cd0dd033e0f6ec092e60c3812ae066657fe152653
F src/sqliteLimit.h d7323ffea5208c6af2734574bae933ca8ed2ab728083caa117c9738581a31657
F src/status.c 160c445d7d28c984a0eae38c144f6419311ed3eace59b44ac6dafc20db4af749
F src/table.c 0f141b58a16de7e2fbe81c308379e7279f4c6b50eb08efeec5892794a0ba30d1
@@ -731,7 +731,7 @@ F test/affinity2.test ce1aafc86e110685b324e9a763eab4f2a73f737842ec3b687bd965867d
F test/affinity3.test f094773025eddf31135c7ad4cde722b7696f8eb07b97511f98585addf2a510a9
F test/aggerror.test a867e273ef9e3d7919f03ef4f0e8c0d2767944f2
F test/aggfault.test 777f269d0da5b0c2524c7ff6d99ae9a93db4f1b1839a914dd2a12e3035c29829
-F test/aggnested.test 7269d07ac879fce161cb26c8fabe65cba5715742fac8a1fccac570dcdaf28f00
+F test/aggnested.test e1977bdc0a154b99c139b879b78c46030aa6ee97fb06bf65d6784a536e25b743
F test/alias.test 4529fbc152f190268a15f9384a5651bbbabc9d87
F test/all.test 2ecb8bbd52416642e41c9081182a8df05d42c75637afd4488aace78cc4b69e13
F test/alter.test 313073774ab5c3f2ef1d3f0d03757c9d3a81284ae7e1b4a6ca34db088f886896
@@ -1920,7 +1920,7 @@ F test/win32heap.test 10fd891266bd00af68671e702317726375e5407561d859be1aa04696f2
F test/win32lock.test e0924eb8daac02bf80e9da88930747bd44dd9b230b7759fed927b1655b467c9c
F test/win32longpath.test 4baffc3acb2e5188a5e3a895b2b543ed09e62f7c72d713c1feebf76222fe9976
F test/win32nolock.test ac4f08811a562e45a5755e661f45ca85892bdbbc
-F test/window1.test 5ba48e9d33231e6ef16f21426bade9ccc52abf65a10587bff90a6c14fe174594
+F test/window1.test 95d0d3e43a54600beba759f9a9f4f4bf546a596b1a8cc3f70dd26bf22ce7e41a
F test/window2.tcl 492c125fa550cda1dd3555768a2303b3effbeceee215293adf8871efc25f1476
F test/window2.test e466a88bd626d66edc3d352d7d7e1d5531e0079b549ba44efb029d1fbff9fd3c
F test/window3.tcl acea6e86a4324a210fd608d06741010ca83ded9fde438341cb978c49928faf03
diff --git a/src/resolve.c b/src/resolve.c
index adfcc8dbe9cf41581a23cfc42a6d699bf86d384f..b453a5b456ea1e0db814e1bd13c3fbe706a19a38 100644
--- a/src/resolve.c
+++ b/src/resolve.c
@@ -1212,11 +1212,12 @@ static int resolveExprStep(Walker *pWalker, Expr *pExpr){
while( pNC2
&& sqlite3ReferencesSrcList(pParse, pExpr, pNC2->pSrcList)==0
){
- pExpr->op2++;
+ pExpr->op2 += (1 + pNC2->nNestedSelect);
pNC2 = pNC2->pNext;
}
assert( pDef!=0 || IN_RENAME_OBJECT );
if( pNC2 && pDef ){
+ pExpr->op2 += pNC2->nNestedSelect;
assert( SQLITE_FUNC_MINMAX==NC_MinMaxAgg );
assert( SQLITE_FUNC_ANYORDER==NC_OrderAgg );
testcase( (pDef->funcFlags & SQLITE_FUNC_MINMAX)!=0 );
@@ -1777,6 +1778,7 @@ static int resolveSelectStep(Walker *pWalker, Select *p){
/* Recursively resolve names in all subqueries in the FROM clause
*/
+ if( pOuterNC ) pOuterNC->nNestedSelect++;
for(i=0; i<p->pSrc->nSrc; i++){
SrcItem *pItem = &p->pSrc->a[i];
if( pItem->pSelect && (pItem->pSelect->selFlags & SF_Resolved)==0 ){
@@ -1801,6 +1803,7 @@ static int resolveSelectStep(Walker *pWalker, Select *p){
}
}
}
+ if( pOuterNC ) pOuterNC->nNestedSelect--;
/* Set up the local name-context to pass to sqlite3ResolveExprNames() to
** resolve the result-set expression list.
diff --git a/src/sqliteInt.h b/src/sqliteInt.h
index 2c893770b04e06932b9c4405ff9b41a13aff68d0..ab94b56c638724a9de8ea2da49c944fb85a1923f 100644
--- a/src/sqliteInt.h
+++ b/src/sqliteInt.h
@@ -3332,6 +3332,7 @@ struct NameContext {
int nRef; /* Number of names resolved by this context */
int nNcErr; /* Number of errors encountered while resolving names */
int ncFlags; /* Zero or more NC_* flags defined below */
+ int nNestedSelect; /* Number of nested selects using this NC */
Select *pWinSelect; /* SELECT statement for any window functions */
};
diff --git a/test/aggnested.test b/test/aggnested.test
index 1b8b608803912d5d8b7a7562dd31ec4a46627da4..6599fbf90940e49ce3467c67b7c8800aa90b17c0 100644
--- a/test/aggnested.test
+++ b/test/aggnested.test
@@ -358,6 +358,60 @@ do_execsql_test 6.2.2 {
FROM t2 GROUP BY 'constant_string';
} {{}}
+#-------------------------------------------------------------------------
+reset_db
+
+do_execsql_test 7.0 {
+ CREATE TABLE invoice (
+ id INTEGER PRIMARY KEY AUTOINCREMENT NOT NULL,
+ amount DOUBLE PRECISION DEFAULT NULL,
+ name VARCHAR(100) DEFAULT NULL
+ );
+
+ INSERT INTO invoice (amount, name) VALUES
+ (4.0, 'Michael'), (15.0, 'Bara'), (4.0, 'Michael'), (6.0, 'John');
+}
+
+do_execsql_test 7.1 {
+ SELECT sum(amount), name
+ from invoice
+ group by name
+ having (select v > 6 from (select sum(amount) v) t)
+} {
+ 15.0 Bara
+ 8.0 Michael
+}
+
+do_execsql_test 7.2 {
+ SELECT (select 1 from (select sum(amount))) FROM invoice
+} {1}
+
+do_execsql_test 8.0 {
+ CREATE TABLE t1(x INT);
+ INSERT INTO t1 VALUES(100);
+ INSERT INTO t1 VALUES(20);
+ INSERT INTO t1 VALUES(3);
+ SELECT (SELECT y FROM (SELECT sum(x) AS y) AS t2 ) FROM t1;
+} {123}
+
+do_execsql_test 8.1 {
+ SELECT (
+ SELECT y FROM (
+ SELECT z AS y FROM (SELECT sum(x) AS z) AS t2
+ )
+ ) FROM t1;
+} {123}
+
+do_execsql_test 8.2 {
+ SELECT (
+ SELECT a FROM (
+ SELECT y AS a FROM (
+ SELECT z AS y FROM (SELECT sum(x) AS z) AS t2
+ )
+ )
+ ) FROM t1;
+} {123}
+
diff --git a/test/window1.test b/test/window1.test
index 783a739e3f1428f107932319a53e9cde91e79557..8044d43547718bb54b301c185adf60bf70ab1ae8 100644
--- a/test/window1.test
+++ b/test/window1.test
@@ -1881,7 +1881,7 @@ do_catchsql_test 57.3 {
SELECT max(y) OVER( ORDER BY (SELECT x FROM (SELECT sum(y) AS x FROM t1)))
)
FROM t3;
-} {1 {misuse of aggregate: sum()}}
+} {0 5}
# 2020-06-06 ticket 1f6f353b684fc708
reset_db

View File

@@ -5,3 +5,4 @@ fix_disable_implies_dcheck_for_node_stream_array_buffers.patch
chore_allow_customizing_microtask_policy_per_context.patch
cherry-pick-57d372c3e399.patch
merged_promises_async_stack_traces_fix_the_case_when_the_closure.patch
turboshaft_fix_structuraloptimization_because_of_ignored.patch

View File

@@ -0,0 +1,113 @@
From 0000000000000000000000000000000000000000 Mon Sep 17 00:00:00 2001
From: Pedro Pontes <pepontes@microsoft.com>
Date: Fri, 5 Jan 2024 05:04:35 -0800
Subject: Fix StructuralOptimization because of ignored side-effects
Side-effects in the 1st else block were not taken into account.
Drive-by: minor cleanups to StructuralOptimizationReducer.
Bug: v8:12783
Change-Id: I9666bae56c1e9f026567e8e3f0fcbad3836e8297
Fixed: chromium:1509576
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/5104648
Commit-Queue: Nico Hartmann <nicohartmann@chromium.org>
Auto-Submit: Darius Mercadier <dmercadier@chromium.org>
Reviewed-by: Nico Hartmann <nicohartmann@chromium.org>
Cr-Commit-Position: refs/heads/main@{#91432}
diff --git a/src/compiler/turboshaft/structural-optimization-reducer.h b/src/compiler/turboshaft/structural-optimization-reducer.h
index bf5a49361d884367cb3048005cbe599b59a1af2d..364e5adbbfe9ec56d03e0cb0e04c8ce6f9d3f4f1 100644
--- a/src/compiler/turboshaft/structural-optimization-reducer.h
+++ b/src/compiler/turboshaft/structural-optimization-reducer.h
@@ -80,7 +80,7 @@ namespace v8::internal::compiler::turboshaft {
template <class Next>
class StructuralOptimizationReducer : public Next {
public:
- using Next::Asm;
+ TURBOSHAFT_REDUCER_BOILERPLATE()
OpIndex ReduceInputGraphBranch(OpIndex input_index, const BranchOp& branch) {
LABEL_BLOCK(no_change) {
@@ -100,6 +100,13 @@ class StructuralOptimizationReducer : public Next {
OpIndex switch_var = OpIndex::Invalid();
while (true) {
+ // The "false" destination will be inlined before the switch is emitted,
+ // so it should only contain pure operations.
+ if (!ContainsOnlyPureOps(current_branch->if_false, Asm().input_graph())) {
+ TRACE("\t [break] End of only-pure-ops cascade reached.\n");
+ break;
+ }
+
// If we encounter a condition that is not equality, we can't turn it
// into a switch case.
const EqualOp* equal = Asm()
@@ -164,13 +171,6 @@ class StructuralOptimizationReducer : public Next {
// Iterate to the next if_false block in the cascade.
current_branch = &maybe_branch.template Cast<BranchOp>();
-
- // As long as the else blocks contain only pure ops, we can keep
- // traversing the if-else cascade.
- if (!ContainsOnlyPureOps(current_branch->if_false, Asm().input_graph())) {
- TRACE("\t [break] End of only-pure-ops cascade reached.\n");
- break;
- }
}
// Probably better to keep short if-else cascades as they are.
@@ -186,7 +186,7 @@ class StructuralOptimizationReducer : public Next {
InlineAllOperationsWithoutLast(block);
}
- TRACE("[reduce] Successfully emit a Switch with %z cases.", cases.size());
+ TRACE("[reduce] Successfully emit a Switch with %zu cases.", cases.size());
// The last current_if_true block that ends the cascade becomes the default
// case.
diff --git a/test/mjsunit/compiler/regress-crbug-1509576.js b/test/mjsunit/compiler/regress-crbug-1509576.js
new file mode 100644
index 0000000000000000000000000000000000000000..f538296edc4dd1c430a2cec6f88fda82273b10e8
--- /dev/null
+++ b/test/mjsunit/compiler/regress-crbug-1509576.js
@@ -0,0 +1,39 @@
+// Copyright 2023 the V8 project authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+// Flags: --allow-natives-syntax
+
+function escape(s) { }
+
+function f(i) {
+ let str = "";
+ escape(str);
+
+ // This "if (i == 3)" should not be merged into the subsequent switch, because
+ // there is a side-effect in between.
+ if (i == 3) {
+ // This will trigger a deopt
+ str += "("
+ }
+
+ str += "function";
+
+ switch (i) {
+ case -10:
+ escape(str);
+ case 1:
+ case 3:
+ }
+
+ // This `eval` creates some kind of closure of the function inside the
+ // function, not sure how that works exactly, but it's needed to repro :D
+ eval();
+
+ return str;
+}
+
+%PrepareFunctionForOptimization(f);
+assertEquals(f(0), "function");
+%OptimizeFunctionOnNextCall(f);
+assertEquals(f(3), "(function");