From 6db0b9c70dd4198870883117e05f39c55d7da938 Mon Sep 17 00:00:00 2001 From: Pedro Pontes Date: Mon, 8 Jan 2024 13:33:38 -0800 Subject: [PATCH] chore: cherry-pick 8 changes from Release-3-M120 (#40900) * chore: [27-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: update patches --------- Co-authored-by: PatchUp <73610968+patchup[bot]@users.noreply.github.com> --- patches/angle/.patches | 4 + ...ompilation_if_too_many_struct_fields.patch | 215 +++++++++ ..._limit_private_variable_size_to_64kb.patch | 416 ++++++++++++++++++ ..._optimize_field-name-collision_check.patch | 180 ++++++++ ...en_glcopyteximage2d_redefines_itself.patch | 135 ++++++ patches/chromium/.patches | 2 + .../chromium/cherry-pick-50a1bddfca85.patch | 117 +++++ .../chromium/cherry-pick-5b2fddadaa12.patch | 61 +++ patches/config.json | 6 +- patches/sqlite/.patches | 1 + ...gate_function_error_that_could_occur.patch | 323 ++++++++++++++ patches/v8/.patches | 1 + ...turaloptimization_because_of_ignored.patch | 137 ++++++ 13 files changed, 1597 insertions(+), 1 deletion(-) create mode 100644 patches/angle/.patches create mode 100644 patches/angle/m120_translator_fail_compilation_if_too_many_struct_fields.patch create mode 100644 patches/angle/m120_translator_limit_private_variable_size_to_64kb.patch create mode 100644 patches/angle/m120_translator_optimize_field-name-collision_check.patch create mode 100644 patches/angle/m120_vulkan_don_t_crash_when_glcopyteximage2d_redefines_itself.patch create mode 100644 patches/chromium/cherry-pick-50a1bddfca85.patch create mode 100644 patches/chromium/cherry-pick-5b2fddadaa12.patch create mode 100644 patches/sqlite/.patches create mode 100644 patches/sqlite/fix_a_spurious_misuse_of_aggregate_function_error_that_could_occur.patch create mode 100644 patches/v8/merged_turboshaft_fix_structuraloptimization_because_of_ignored.patch diff --git a/patches/angle/.patches b/patches/angle/.patches new file mode 100644 index 0000000000000..3258fb40c5c2e --- /dev/null +++ b/patches/angle/.patches @@ -0,0 +1,4 @@ +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 diff --git a/patches/angle/m120_translator_fail_compilation_if_too_many_struct_fields.patch b/patches/angle/m120_translator_fail_compilation_if_too_many_struct_fields.patch new file mode 100644 index 0000000000000..65f682d09fd5c --- /dev/null +++ b/patches/angle/m120_translator_fail_compilation_if_too_many_struct_fields.patch @@ -0,0 +1,215 @@ +From 0000000000000000000000000000000000000000 Mon Sep 17 00:00:00 2001 +From: Shahbaz Youssefi +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 + +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 *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 *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 0fa89f59a38165b8d526a99bae60c7b752dec15d..d7eb48eacfe58fc128fc2d24088ac18ea5196c05 100644 +--- a/src/tests/gl_tests/GLSLTest.cpp ++++ b/src/tests/gl_tests/GLSLTest.cpp +@@ -18168,6 +18168,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); diff --git a/patches/angle/m120_translator_limit_private_variable_size_to_64kb.patch b/patches/angle/m120_translator_limit_private_variable_size_to_64kb.patch new file mode 100644 index 0000000000000..25ec741fbfcaa --- /dev/null +++ b/patches/angle/m120_translator_limit_private_variable_size_to_64kb.patch @@ -0,0 +1,416 @@ +From 0000000000000000000000000000000000000000 Mon Sep 17 00:00:00 2001 +From: Shahbaz Youssefi +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 + +diff --git a/src/compiler/translator/Compiler.cpp b/src/compiler/translator/Compiler.cpp +index 1f3a4d52f2f45e7ed10b9b5512f2f92a06877256..c70c419631a1c4d382bcc8ba36c35d0ba06c2c5d 100644 +--- a/src/compiler/translator/Compiler.cpp ++++ b/src/compiler/translator/Compiler.cpp +@@ -771,11 +771,6 @@ bool TCompiler::checkAndSimplifyAST(TIntermBlock *root, + return false; + } + +- if (shouldLimitTypeSizes() && !ValidateTypeSizeLimitations(root, &mSymbolTable, &mDiagnostics)) +- { +- return false; +- } +- + if (!ValidateFragColorAndFragData(mShaderType, mShaderVersion, mSymbolTable, &mDiagnostics)) + { + return false; +@@ -1056,6 +1051,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(2) * 1024 * 1024 * 1024; +-constexpr size_t kMaxPrivateVariableSizeInBytes = static_cast(16) * 1024 * 1024; ++constexpr size_t kMaxVariableSizeInBytes = static_cast(2) * 1024 * 1024 * 1024; ++constexpr size_t kMaxPrivateVariableSizeInBytes = static_cast(64) * 1024; ++constexpr size_t kMaxTotalPrivateVariableSizeInBytes = static_cast(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::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 91233461298150dff48e4d044eb8535e1bcfb7b6..f1343f8e8ad65b6fa292c630214c66aaf3ff5a64 100644 +--- a/src/tests/angle_end2end_tests_expectations.txt ++++ b/src/tests/angle_end2end_tests_expectations.txt +@@ -110,6 +110,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 +@@ -146,6 +148,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 +@@ -1055,6 +1061,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 d7eb48eacfe58fc128fc2d24088ac18ea5196c05..f4b7ce6222ff4a29cb20b75bc102e2c8ae478189 100644 +--- a/src/tests/gl_tests/GLSLTest.cpp ++++ b/src/tests/gl_tests/GLSLTest.cpp +@@ -18195,6 +18195,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); diff --git a/patches/angle/m120_translator_optimize_field-name-collision_check.patch b/patches/angle/m120_translator_optimize_field-name-collision_check.patch new file mode 100644 index 0000000000000..6f96167982c66 --- /dev/null +++ b/patches/angle/m120_translator_optimize_field-name-collision_check.patch @@ -0,0 +1,180 @@ +From 0000000000000000000000000000000000000000 Mon Sep 17 00:00:00 2001 +From: Shahbaz Youssefi +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 + +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 *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> ++ 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 *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 04b1a80e4ccabf0043617d90a1a06cff25deab98..91233461298150dff48e4d044eb8535e1bcfb7b6 100644 +--- a/src/tests/angle_end2end_tests_expectations.txt ++++ b/src/tests/angle_end2end_tests_expectations.txt +@@ -1054,6 +1054,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 6443d47740d7f625fde2ebca2ef4a34d512d0883..0fa89f59a38165b8d526a99bae60c7b752dec15d 100644 +--- a/src/tests/gl_tests/GLSLTest.cpp ++++ b/src/tests/gl_tests/GLSLTest.cpp +@@ -18124,6 +18124,50 @@ void main() + EXPECT_PIXEL_COLOR_EQ(0, 0, GLColor::green); + } + ++// 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); diff --git a/patches/angle/m120_vulkan_don_t_crash_when_glcopyteximage2d_redefines_itself.patch b/patches/angle/m120_vulkan_don_t_crash_when_glcopyteximage2d_redefines_itself.patch new file mode 100644 index 0000000000000..2abc2de842dde --- /dev/null +++ b/patches/angle/m120_vulkan_don_t_crash_when_glcopyteximage2d_redefines_itself.patch @@ -0,0 +1,135 @@ +From 0000000000000000000000000000000000000000 Mon Sep 17 00:00:00 2001 +From: Shahbaz Youssefi +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 + +diff --git a/src/libANGLE/renderer/vulkan/TextureVk.cpp b/src/libANGLE/renderer/vulkan/TextureVk.cpp +index 54d1184518e1cc6e83157b901a37e4ce1e9b58a5..98f66a1d4b02f78dcb3bdae673d21f4bd087fea7 100644 +--- a/src/libANGLE/renderer/vulkan/TextureVk.cpp ++++ b/src/libANGLE/renderer/vulkan/TextureVk.cpp +@@ -879,8 +879,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); + } +@@ -1949,7 +1969,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 f1343f8e8ad65b6fa292c630214c66aaf3ff5a64..e60ea8f9970d14a551128528d18e7be3bc08efcc 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 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(), diff --git a/patches/chromium/.patches b/patches/chromium/.patches index 21050f771b9de..09677e3e14159 100644 --- a/patches/chromium/.patches +++ b/patches/chromium/.patches @@ -144,3 +144,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 diff --git a/patches/chromium/cherry-pick-50a1bddfca85.patch b/patches/chromium/cherry-pick-50a1bddfca85.patch new file mode 100644 index 0000000000000..fe5c6d2fc91d0 --- /dev/null +++ b/patches/chromium/cherry-pick-50a1bddfca85.patch @@ -0,0 +1,117 @@ +From 0000000000000000000000000000000000000000 Mon Sep 17 00:00:00 2001 +From: Austin Eng +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 +Commit-Queue: Austin Eng +Reviewed-by: Stephen White +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 f2f4e8be0cc73f5b569fd3345bd3b64f4539041d..5752c80fbbfc91d492f497c87f181d3211c71d86 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_handle, ++ CrossThreadHandle execution_context_handle, + std::unique_ptr 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 current_thread_task_runner, +- base::OnceCallback)> +- callback) { ++ WTF::CrossThreadOnceFunction< ++ void(std::unique_ptr)> 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 current_thread_task_runner, +- base::OnceCallback)> +- callback); ++ WTF::CrossThreadOnceFunction< ++ void(std::unique_ptr)> callback); + + } // namespace blink + diff --git a/patches/chromium/cherry-pick-5b2fddadaa12.patch b/patches/chromium/cherry-pick-5b2fddadaa12.patch new file mode 100644 index 0000000000000..3974cd39168f5 --- /dev/null +++ b/patches/chromium/cherry-pick-5b2fddadaa12.patch @@ -0,0 +1,61 @@ +From 0000000000000000000000000000000000000000 Mon Sep 17 00:00:00 2001 +From: Hongchan Choi +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 +Commit-Queue: Hongchan Choi +Cr-Original-Commit-Position: refs/heads/main@{#1235028} +Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/5112838 +Commit-Queue: Rubber Stamper +Bot-Commit: Rubber Stamper +Auto-Submit: Hongchan Choi +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. diff --git a/patches/config.json b/patches/config.json index 5d3eab55fce3f..5b37268c912d8 100644 --- a/patches/config.json +++ b/patches/config.json @@ -23,5 +23,9 @@ "src/electron/patches/webrtc": "src/third_party/webrtc", - "src/electron/patches/libavif": "src/third_party/libavif/src" + "src/electron/patches/libavif": "src/third_party/libavif/src", + + "src/electron/patches/angle": "src/third_party/angle", + + "src/electron/patches/sqlite": "src/third_party/sqlite/src" } diff --git a/patches/sqlite/.patches b/patches/sqlite/.patches new file mode 100644 index 0000000000000..3bdae5b1ad162 --- /dev/null +++ b/patches/sqlite/.patches @@ -0,0 +1 @@ +fix_a_spurious_misuse_of_aggregate_function_error_that_could_occur.patch diff --git a/patches/sqlite/fix_a_spurious_misuse_of_aggregate_function_error_that_could_occur.patch b/patches/sqlite/fix_a_spurious_misuse_of_aggregate_function_error_that_could_occur.patch new file mode 100644 index 0000000000000..d696fe2854f26 --- /dev/null +++ b/patches/sqlite/fix_a_spurious_misuse_of_aggregate_function_error_that_could_occur.patch @@ -0,0 +1,323 @@ +From 0000000000000000000000000000000000000000 Mon Sep 17 00:00:00 2001 +From: dan +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; ipSrc->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; ipSrc->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; ipSrc->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 diff --git a/patches/v8/.patches b/patches/v8/.patches index 22feca0335654..a11ccd309fee8 100644 --- a/patches/v8/.patches +++ b/patches/v8/.patches @@ -3,3 +3,4 @@ do_not_export_private_v8_symbols_on_windows.patch fix_build_deprecated_attribute_for_older_msvc_versions.patch chore_allow_customizing_microtask_policy_per_context.patch cherry-pick-cbd09b2ca928.patch +merged_turboshaft_fix_structuraloptimization_because_of_ignored.patch diff --git a/patches/v8/merged_turboshaft_fix_structuraloptimization_because_of_ignored.patch b/patches/v8/merged_turboshaft_fix_structuraloptimization_because_of_ignored.patch new file mode 100644 index 0000000000000..1db93bacac78e --- /dev/null +++ b/patches/v8/merged_turboshaft_fix_structuraloptimization_because_of_ignored.patch @@ -0,0 +1,137 @@ +From 0000000000000000000000000000000000000000 Mon Sep 17 00:00:00 2001 +From: Darius M +Date: Fri, 8 Dec 2023 14:15:46 +0100 +Subject: Merged: [turboshaft] 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, chromium:1509576 +(cherry picked from commit 4a664b390577de3d3572010da0dc1138d78ab2c4) + +Change-Id: Id4e230ee0fd408c821747d3350d688c8b0098ae3 +Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/5114883 +Reviewed-by: Matthias Liedtke +Commit-Queue: Matthias Liedtke +Auto-Submit: Darius Mercadier +Cr-Commit-Position: refs/branch-heads/12.0@{#20} +Cr-Branched-From: ed7b4caf1fb8184ad9e24346c84424055d4d430a-refs/heads/12.0.267@{#1} +Cr-Branched-From: 210e75b19db4352c9b78dce0bae11c2dc3077df4-refs/heads/main@{#90651} + +diff --git a/src/compiler/turboshaft/structural-optimization-reducer.h b/src/compiler/turboshaft/structural-optimization-reducer.h +index bf5a49361d884367cb3048005cbe599b59a1af2d..7745a8bd2fd037282dcc370862b3a9b8d6809adf 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 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() +@@ -116,17 +123,11 @@ class StructuralOptimizationReducer : public Next { + // MachineOptimizationReducer should normalize equality to put constants + // right. + const Operation& right_op = Asm().input_graph().Get(equal->right()); +- if (!right_op.Is()) { +- TRACE("\t [bailout] No constant on the right side of Equal.\n"); ++ if (!right_op.Is()) { ++ TRACE("\t [bailout] No Word32 constant on the right side of Equal.\n"); + break; + } +- +- // We can only turn Word32 constant equals to switch cases. + const ConstantOp& const_op = right_op.Cast(); +- if (const_op.kind != ConstantOp::Kind::kWord32) { +- TRACE("\t [bailout] Constant is not of type Word32.\n"); +- break; +- } + + // If we encounter equal to a different value, we can't introduce + // a switch. +@@ -164,13 +165,6 @@ class StructuralOptimizationReducer : public Next { + + // Iterate to the next if_false block in the cascade. + current_branch = &maybe_branch.template Cast(); +- +- // 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 +180,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");