chore: cherry-pick 3 changes from 0-M127 (#43159)
* chore: [30-x-y] cherry-pick 3 changes from 0-M127 * 44b7fbf35b10 from chromium * c9815acd5a88 from dawn * 9463ce9cd8d9 from DirectXShaderCompiler * chore: e patches all to make GH actions happy --------- Co-authored-by: Charles Kerr <charles@charleskerr.com> Co-authored-by: Shelley Vohr <shelley.vohr@gmail.com>
This commit is contained in:
Pedro Pontes
GitHub
parent
1ec867c8a1
commit
3b45a30d2a
6 changed files with 580 additions and 0 deletions
|
|
@ -4,3 +4,4 @@ cherry-pick-b845fed99111.patch
|
||||||
cherry-pick-771e74ab497d.patch
|
cherry-pick-771e74ab497d.patch
|
||||||
cherry-pick-8f07d39227f6.patch
|
cherry-pick-8f07d39227f6.patch
|
||||||
cherry-pick-b3c64851765c.patch
|
cherry-pick-b3c64851765c.patch
|
||||||
|
cherry-pick-9463ce9cd8d9.patch
|
||||||
|
|
|
||||||
261
patches/DirectXShaderCompiler/cherry-pick-9463ce9cd8d9.patch
Normal file
261
patches/DirectXShaderCompiler/cherry-pick-9463ce9cd8d9.patch
Normal file
|
|
@ -0,0 +1,261 @@
|
||||||
|
From 0000000000000000000000000000000000000000 Mon Sep 17 00:00:00 2001
|
||||||
|
From: David Neto <dneto@google.com>
|
||||||
|
Date: Tue, 16 Jul 2024 16:34:52 -0400
|
||||||
|
Subject: Add CMake option DXC_CODEGEN_EXCEPTIONS_TRAP (#6764)
|
||||||
|
|
||||||
|
When enabled, any hlsl::Exception thrown during code generation and
|
||||||
|
optimization will cause the process to trap.
|
||||||
|
|
||||||
|
Bug: 346618785
|
||||||
|
Change-Id: I9ac966d339ec3090e3455d51a9d7516cc5a3f153
|
||||||
|
|
||||||
|
HLMatrixLower: allow exceptions to propagate out (#6710)
|
||||||
|
|
||||||
|
If an exception is thrown, don't block it in the TempOverloadPool
|
||||||
|
destructor. Allow it to propagate out as a user-visible error.
|
||||||
|
|
||||||
|
Explicitly clear the TempOverloadPool before returning from the
|
||||||
|
HLMatrixLowerPass::runOnModule. In the normal case, when no exception is
|
||||||
|
thrown, that will still verify that all the overloads actually have been
|
||||||
|
lowered, and will assert out if they aren't.
|
||||||
|
|
||||||
|
Bug: 346618785
|
||||||
|
Change-Id: Id421ca324e4d799477a41abb14b966e8f39be482
|
||||||
|
Reviewed-on: https://chromium-review.googlesource.com/c/external/github.com/microsoft/DirectXShaderCompiler/+/5715228
|
||||||
|
Reviewed-by: Natalie Chouinard <chouinard@google.com>
|
||||||
|
|
||||||
|
diff --git a/CMakeLists.txt b/CMakeLists.txt
|
||||||
|
index 15097b8473bf9f6c2b6c28ac095936616aa917f6..ac35ebd1a6afd55934da502ddb0e0c286da10db8 100644
|
||||||
|
--- a/CMakeLists.txt
|
||||||
|
+++ b/CMakeLists.txt
|
||||||
|
@@ -104,6 +104,9 @@ endif()
|
||||||
|
option(DXC_DISABLE_ALLOCATOR_OVERRIDES "Disable usage of allocator overrides" OFF)
|
||||||
|
mark_as_advanced(DXC_DISABLE_ALLOCATOR_OVERRIDES)
|
||||||
|
|
||||||
|
+option(DXC_CODEGEN_EXCEPTIONS_TRAP "An exception in code generation generates a trap, ending the compiler process" OFF)
|
||||||
|
+mark_as_advanced(DXC_CODEGEN_EXCEPTIONS_TRAP)
|
||||||
|
+
|
||||||
|
# adjust link option to enable debugging from kernel mode; not compatible with incremental linking
|
||||||
|
if(NOT CMAKE_VERSION VERSION_LESS "3.13" AND WIN32 AND NOT CMAKE_C_COMPILER_ARCHITECTURE_ID STREQUAL "ARM64EC")
|
||||||
|
add_link_options(/DEBUGTYPE:CV,FIXUP,PDATA /INCREMENTAL:NO)
|
||||||
|
diff --git a/include/dxc/config.h.cmake b/include/dxc/config.h.cmake
|
||||||
|
index 6d7450323ec864ef5f6e8796c46423d49f278b76..7226ed75a108f63c3c894b59eb7e0dbc5fb66002 100644
|
||||||
|
--- a/include/dxc/config.h.cmake
|
||||||
|
+++ b/include/dxc/config.h.cmake
|
||||||
|
@@ -1,2 +1,4 @@
|
||||||
|
/* Disable overriding memory allocators. */
|
||||||
|
#cmakedefine DXC_DISABLE_ALLOCATOR_OVERRIDES
|
||||||
|
+/* Generate a trap if an hlsl::Exception is thrown during code generation */
|
||||||
|
+#cmakedefine DXC_CODEGEN_EXCEPTIONS_TRAP
|
||||||
|
diff --git a/lib/HLSL/HLMatrixLowerPass.cpp b/lib/HLSL/HLMatrixLowerPass.cpp
|
||||||
|
index d5959eb9335465f67d8e7ef7d7ab4eb720274226..3bb9f65834ecfd0c7a25c8a176f62c124e9967e5 100644
|
||||||
|
--- a/lib/HLSL/HLMatrixLowerPass.cpp
|
||||||
|
+++ b/lib/HLSL/HLMatrixLowerPass.cpp
|
||||||
|
@@ -60,7 +60,12 @@ class TempOverloadPool {
|
||||||
|
public:
|
||||||
|
TempOverloadPool(llvm::Module &Module, const char *BaseName)
|
||||||
|
: Module(Module), BaseName(BaseName) {}
|
||||||
|
- ~TempOverloadPool() { clear(); }
|
||||||
|
+ ~TempOverloadPool() {
|
||||||
|
+ if (!Funcs.empty()) {
|
||||||
|
+ // The flow has thrown an exception. Let that exception
|
||||||
|
+ // propagate out and be reported as a compile error.
|
||||||
|
+ }
|
||||||
|
+ }
|
||||||
|
|
||||||
|
Function *get(FunctionType *Ty);
|
||||||
|
bool contains(FunctionType *Ty) const { return Funcs.count(Ty) != 0; }
|
||||||
|
@@ -248,12 +253,15 @@ bool HLMatrixLowerPass::runOnModule(Module &M) {
|
||||||
|
m_matToVecStubs = nullptr;
|
||||||
|
m_vecToMatStubs = nullptr;
|
||||||
|
|
||||||
|
- // If you hit an assert during TempOverloadPool destruction,
|
||||||
|
+ // If you hit an assert while clearing TempOverloadPool,
|
||||||
|
// it means that either a matrix producer was lowered,
|
||||||
|
// causing a translation stub to be created,
|
||||||
|
// but the consumer of that matrix was never (properly) lowered.
|
||||||
|
// Or the opposite: a matrix consumer was lowered and not its producer.
|
||||||
|
|
||||||
|
+ matToVecStubs.clear();
|
||||||
|
+ vecToMatStubs.clear();
|
||||||
|
+
|
||||||
|
return true;
|
||||||
|
}
|
||||||
|
|
||||||
|
diff --git a/tools/clang/lib/CodeGen/BackendUtil.cpp b/tools/clang/lib/CodeGen/BackendUtil.cpp
|
||||||
|
index 294ca05946eed6d87a3ad4e2b56a641d24065760..1e9adb40f4bae4cada42b5581582d50dc3676594 100644
|
||||||
|
--- a/tools/clang/lib/CodeGen/BackendUtil.cpp
|
||||||
|
+++ b/tools/clang/lib/CodeGen/BackendUtil.cpp
|
||||||
|
@@ -8,6 +8,10 @@
|
||||||
|
//===----------------------------------------------------------------------===//
|
||||||
|
|
||||||
|
#include "clang/CodeGen/BackendUtil.h"
|
||||||
|
+#include "dxc/HLSL/DxilGenerationPass.h" // HLSL Change
|
||||||
|
+#include "dxc/HLSL/HLMatrixLowerPass.h" // HLSL Change
|
||||||
|
+#include "dxc/Support/Global.h" // HLSL Change
|
||||||
|
+#include "dxc/config.h" // HLSL Change
|
||||||
|
#include "clang/Basic/Diagnostic.h"
|
||||||
|
#include "clang/Basic/LangOptions.h"
|
||||||
|
#include "clang/Basic/TargetOptions.h"
|
||||||
|
@@ -41,10 +45,8 @@
|
||||||
|
#include "llvm/Transforms/ObjCARC.h"
|
||||||
|
#include "llvm/Transforms/Scalar.h"
|
||||||
|
#include "llvm/Transforms/Utils/SymbolRewriter.h"
|
||||||
|
+#include <cstdio>
|
||||||
|
#include <memory>
|
||||||
|
-#include "dxc/HLSL/DxilGenerationPass.h" // HLSL Change
|
||||||
|
-#include "dxc/HLSL/HLMatrixLowerPass.h" // HLSL Change
|
||||||
|
-#include "dxc/Support/Global.h" // HLSL Change
|
||||||
|
|
||||||
|
using namespace clang;
|
||||||
|
using namespace llvm;
|
||||||
|
@@ -780,6 +782,13 @@ void clang::EmitBackendOutput(DiagnosticsEngine &Diags,
|
||||||
|
} catch (const ::hlsl::Exception &hlslException) {
|
||||||
|
Diags.Report(Diags.getCustomDiagID(DiagnosticsEngine::Error, "%0\n"))
|
||||||
|
<< StringRef(hlslException.what());
|
||||||
|
+#if defined(DXC_CODEGEN_EXCEPTIONS_TRAP)
|
||||||
|
+ // llvm::errs() doesn't work in release builds on Linux.
|
||||||
|
+ // Use C-style fprintf because it works everywhere.
|
||||||
|
+ fprintf(stderr, "internal codegen error: %s\n", hlslException.what());
|
||||||
|
+ fflush(stderr);
|
||||||
|
+ LLVM_BUILTIN_TRAP;
|
||||||
|
+#endif
|
||||||
|
} // HLSL Change Ends
|
||||||
|
|
||||||
|
// If an optional clang TargetInfo description string was passed in, use it to
|
||||||
|
diff --git a/tools/clang/test/DXC/Passes/HLMatrixLower/dont_crash_on_invalid_cast.ll b/tools/clang/test/DXC/Passes/HLMatrixLower/dont_crash_on_invalid_cast.ll
|
||||||
|
new file mode 100644
|
||||||
|
index 0000000000000000000000000000000000000000..fca52d11595d3ac99dedb28219f0746a26add6c9
|
||||||
|
--- /dev/null
|
||||||
|
+++ b/tools/clang/test/DXC/Passes/HLMatrixLower/dont_crash_on_invalid_cast.ll
|
||||||
|
@@ -0,0 +1,130 @@
|
||||||
|
+; RUN: not %dxopt %s -hlsl-passes-resume -hlmatrixlower -S | FileCheck %s
|
||||||
|
+
|
||||||
|
+
|
||||||
|
+; The HL matrix lowering pass can sometimes throw an exception
|
||||||
|
+; due to an invalid LLVM-level cast<Ty> call. Make sure that
|
||||||
|
+; propagates out to a user-level error.
|
||||||
|
+
|
||||||
|
+; Note: There is still a bug in the compiler here. Not all matrix
|
||||||
|
+; lowerings are covered by the pass.
|
||||||
|
+
|
||||||
|
+; TODO: Fix the underlying bug https://github.com/microsoft/DirectXShaderCompiler/issues/6723
|
||||||
|
+; Once that is fixed, this test changes or should be deleted.
|
||||||
|
+
|
||||||
|
+
|
||||||
|
+; CHECK: Operation failed - error code
|
||||||
|
+
|
||||||
|
+;
|
||||||
|
+; Buffer Definitions:
|
||||||
|
+;
|
||||||
|
+; cbuffer $Globals
|
||||||
|
+; {
|
||||||
|
+;
|
||||||
|
+; [0 x i8] (type annotation not present)
|
||||||
|
+;
|
||||||
|
+; }
|
||||||
|
+;
|
||||||
|
+;
|
||||||
|
+; Resource Bindings:
|
||||||
|
+;
|
||||||
|
+; Name Type Format Dim ID HLSL Bind Count
|
||||||
|
+; ------------------------------ ---------- ------- ----------- ------- -------------- ------
|
||||||
|
+; $Globals cbuffer NA NA CB0 cb4294967295 1
|
||||||
|
+;
|
||||||
|
+target datalayout = "e-m:e-p:32:32-i1:32-i8:32-i16:32-i32:32-i64:64-f16:32-f32:32-f64:64-n8:16:32:64"
|
||||||
|
+target triple = "dxil-ms-dx"
|
||||||
|
+
|
||||||
|
+%ConstantBuffer = type opaque
|
||||||
|
+%class.matrix.float.2.4 = type { [2 x <4 x float>] }
|
||||||
|
+%struct.e = type { [1 x %struct.d], [1 x %struct.d] }
|
||||||
|
+%struct.d = type { %struct.a }
|
||||||
|
+%struct.a = type { float, %class.matrix.float.2.4 }
|
||||||
|
+
|
||||||
|
+@"$Globals" = external constant %ConstantBuffer
|
||||||
|
+@g.0.0.0 = internal global [1 x float] undef, align 4
|
||||||
|
+@g.0.0.1 = internal global [1 x %class.matrix.float.2.4] undef, align 4
|
||||||
|
+@g.1.0.0 = internal global [1 x float] undef, align 4
|
||||||
|
+@g.1.0.1 = internal global [1 x %class.matrix.float.2.4] undef, align 4
|
||||||
|
+
|
||||||
|
+; Function Attrs: nounwind
|
||||||
|
+define void @main() #0 {
|
||||||
|
+entry:
|
||||||
|
+ %h.0.1 = alloca %class.matrix.float.2.4, !dbg !26 ; line:13 col:17
|
||||||
|
+ store float 0.000000e+00, float* getelementptr inbounds ([1 x float], [1 x float]* @g.0.0.0, i32 0, i32 0), !dbg !26 ; line:13 col:17
|
||||||
|
+ %0 = call %class.matrix.float.2.4 @"dx.hl.init.rn.%class.matrix.float.2.4 (i32, <8 x float>)"(i32 0, <8 x float> zeroinitializer) #0, !dbg !26 ; line:13 col:17
|
||||||
|
+ %1 = call %class.matrix.float.2.4 @"dx.hl.cast.rowMatToColMat.%class.matrix.float.2.4 (i32, %class.matrix.float.2.4)"(i32 7, %class.matrix.float.2.4 %0) #0, !dbg !26 ; line:13 col:17
|
||||||
|
+ %2 = call %class.matrix.float.2.4 @"dx.hl.matldst.colStore.%class.matrix.float.2.4 (i32, %class.matrix.float.2.4*, %class.matrix.float.2.4)"(i32 1, %class.matrix.float.2.4* getelementptr inbounds ([1 x %class.matrix.float.2.4], [1 x %class.matrix.float.2.4]* @g.0.0.1, i32 0, i32 0), %class.matrix.float.2.4 %1) #0, !dbg !26 ; line:13 col:17
|
||||||
|
+ store float 0.000000e+00, float* getelementptr inbounds ([1 x float], [1 x float]* @g.1.0.0, i32 0, i32 0), !dbg !26 ; line:13 col:17
|
||||||
|
+ %3 = call %class.matrix.float.2.4 @"dx.hl.init.rn.%class.matrix.float.2.4 (i32, <8 x float>)"(i32 0, <8 x float> zeroinitializer) #0, !dbg !26 ; line:13 col:17
|
||||||
|
+ %4 = call %class.matrix.float.2.4 @"dx.hl.cast.rowMatToColMat.%class.matrix.float.2.4 (i32, %class.matrix.float.2.4)"(i32 7, %class.matrix.float.2.4 %3) #0, !dbg !26 ; line:13 col:17
|
||||||
|
+ %5 = call %class.matrix.float.2.4 @"dx.hl.matldst.colStore.%class.matrix.float.2.4 (i32, %class.matrix.float.2.4*, %class.matrix.float.2.4)"(i32 1, %class.matrix.float.2.4* getelementptr inbounds ([1 x %class.matrix.float.2.4], [1 x %class.matrix.float.2.4]* @g.1.0.1, i32 0, i32 0), %class.matrix.float.2.4 %4) #0, !dbg !26 ; line:13 col:17
|
||||||
|
+ %6 = load float, float* getelementptr inbounds ([1 x float], [1 x float]* @g.1.0.0, i32 0, i32 0), !dbg !32 ; line:17 col:9
|
||||||
|
+ %7 = getelementptr inbounds %class.matrix.float.2.4, %class.matrix.float.2.4* %h.0.1, i32 0, i32 0, i32 0, !dbg !32 ; line:17 col:9
|
||||||
|
+ %8 = load <4 x float>, <4 x float>* getelementptr inbounds ([1 x %class.matrix.float.2.4], [1 x %class.matrix.float.2.4]* @g.1.0.1, i32 0, i32 0, i32 0, i32 0), !dbg !32 ; line:17 col:9
|
||||||
|
+ store <4 x float> %8, <4 x float>* %7, !dbg !32 ; line:17 col:9
|
||||||
|
+ %9 = getelementptr inbounds %class.matrix.float.2.4, %class.matrix.float.2.4* %h.0.1, i32 0, i32 0, i32 1, !dbg !32 ; line:17 col:9
|
||||||
|
+ %10 = load <4 x float>, <4 x float>* getelementptr inbounds ([1 x %class.matrix.float.2.4], [1 x %class.matrix.float.2.4]* @g.1.0.1, i32 0, i32 0, i32 0, i32 1), !dbg !32 ; line:17 col:9
|
||||||
|
+ store <4 x float> %10, <4 x float>* %9, !dbg !32 ; line:17 col:9
|
||||||
|
+ ret void, !dbg !33 ; line:18 col:3
|
||||||
|
+}
|
||||||
|
+
|
||||||
|
+; Function Attrs: nounwind
|
||||||
|
+declare void @llvm.memcpy.p0i8.p0i8.i64(i8* nocapture, i8* nocapture readonly, i64, i32, i1) #0
|
||||||
|
+
|
||||||
|
+; Function Attrs: nounwind readnone
|
||||||
|
+declare %class.matrix.float.2.4 @"dx.hl.init.rn.%class.matrix.float.2.4 (i32, <8 x float>)"(i32, <8 x float>) #1
|
||||||
|
+
|
||||||
|
+; Function Attrs: nounwind readnone
|
||||||
|
+declare %class.matrix.float.2.4 @"dx.hl.cast.rowMatToColMat.%class.matrix.float.2.4 (i32, %class.matrix.float.2.4)"(i32, %class.matrix.float.2.4) #1
|
||||||
|
+
|
||||||
|
+; Function Attrs: nounwind
|
||||||
|
+declare %class.matrix.float.2.4 @"dx.hl.matldst.colStore.%class.matrix.float.2.4 (i32, %class.matrix.float.2.4*, %class.matrix.float.2.4)"(i32, %class.matrix.float.2.4*, %class.matrix.float.2.4) #0
|
||||||
|
+
|
||||||
|
+attributes #0 = { nounwind }
|
||||||
|
+attributes #1 = { nounwind readnone }
|
||||||
|
+
|
||||||
|
+!llvm.module.flags = !{!0}
|
||||||
|
+!pauseresume = !{!1}
|
||||||
|
+!llvm.ident = !{!2}
|
||||||
|
+!dx.version = !{!3}
|
||||||
|
+!dx.valver = !{!4}
|
||||||
|
+!dx.shaderModel = !{!5}
|
||||||
|
+!dx.typeAnnotations = !{!6, !15}
|
||||||
|
+!dx.entryPoints = !{!19}
|
||||||
|
+!dx.fnprops = !{!23}
|
||||||
|
+!dx.options = !{!24, !25}
|
||||||
|
+
|
||||||
|
+!0 = !{i32 2, !"Debug Info Version", i32 3}
|
||||||
|
+!1 = !{!"hlsl-hlemit", !"hlsl-hlensure"}
|
||||||
|
+!2 = !{!"dxc(private) 1.8.0.4640 (issue-785, 45018c752d)"}
|
||||||
|
+!3 = !{i32 1, i32 0}
|
||||||
|
+!4 = !{i32 1, i32 8}
|
||||||
|
+!5 = !{!"cs", i32 6, i32 0}
|
||||||
|
+!6 = !{i32 0, %struct.e undef, !7, %struct.d undef, !10, %struct.a undef, !11}
|
||||||
|
+!7 = !{i32 152, !8, !9}
|
||||||
|
+!8 = !{i32 6, !"c", i32 3, i32 0}
|
||||||
|
+!9 = !{i32 6, !"f", i32 3, i32 80}
|
||||||
|
+!10 = !{i32 72, !8}
|
||||||
|
+!11 = !{i32 72, !12, !13}
|
||||||
|
+!12 = !{i32 6, !"b", i32 3, i32 0, i32 7, i32 9}
|
||||||
|
+!13 = !{i32 6, !"c", i32 2, !14, i32 3, i32 16, i32 7, i32 9}
|
||||||
|
+!14 = !{i32 2, i32 4, i32 2}
|
||||||
|
+!15 = !{i32 1, void ()* @main, !16}
|
||||||
|
+!16 = !{!17}
|
||||||
|
+!17 = !{i32 1, !18, !18}
|
||||||
|
+!18 = !{}
|
||||||
|
+!19 = !{void ()* @main, !"main", null, !20, null}
|
||||||
|
+!20 = !{null, null, !21, null}
|
||||||
|
+!21 = !{!22}
|
||||||
|
+!22 = !{i32 0, %ConstantBuffer* @"$Globals", !"$Globals", i32 0, i32 -1, i32 1, i32 0, null}
|
||||||
|
+!23 = !{void ()* @main, i32 5, i32 1, i32 1, i32 1}
|
||||||
|
+!24 = !{i32 64}
|
||||||
|
+!25 = !{i32 -1}
|
||||||
|
+!26 = !DILocation(line: 13, column: 17, scope: !27, inlinedAt: !30)
|
||||||
|
+!27 = !DISubprogram(name: "??__Eg@@YAXXZ", scope: !28, file: !28, line: 13, type: !29, isLocal: true, isDefinition: true, scopeLine: 13, flags: DIFlagPrototyped, isOptimized: false)
|
||||||
|
+!28 = !DIFile(filename: "a.hlsl", directory: "")
|
||||||
|
+!29 = !DISubroutineType(types: !18)
|
||||||
|
+!30 = distinct !DILocation(line: 16, scope: !31)
|
||||||
|
+!31 = !DISubprogram(name: "main", scope: !28, file: !28, line: 16, type: !29, isLocal: false, isDefinition: true, scopeLine: 16, flags: DIFlagPrototyped, isOptimized: false, function: void ()* @main)
|
||||||
|
+!32 = !DILocation(line: 17, column: 9, scope: !31)
|
||||||
|
+!33 = !DILocation(line: 18, column: 3, scope: !31)
|
||||||
|
|
@ -137,6 +137,7 @@ fix_font_face_resolution_when_renderer_is_blocked.patch
|
||||||
x11_use_localized_display_label_only_for_browser_process.patch
|
x11_use_localized_display_label_only_for_browser_process.patch
|
||||||
feat_enable_passing_exit_code_on_service_process_crash.patch
|
feat_enable_passing_exit_code_on_service_process_crash.patch
|
||||||
feat_enable_customizing_symbol_color_in_framecaptionbutton.patch
|
feat_enable_customizing_symbol_color_in_framecaptionbutton.patch
|
||||||
|
cherry-pick-44b7fbf35b10.patch
|
||||||
cherry-pick-d54105311590.patch
|
cherry-pick-d54105311590.patch
|
||||||
cherry-pick-43b8b682d05c.patch
|
cherry-pick-43b8b682d05c.patch
|
||||||
cherry-pick-c5dd8839bfaf.patch
|
cherry-pick-c5dd8839bfaf.patch
|
||||||
|
|
|
||||||
288
patches/chromium/cherry-pick-44b7fbf35b10.patch
Normal file
288
patches/chromium/cherry-pick-44b7fbf35b10.patch
Normal file
|
|
@ -0,0 +1,288 @@
|
||||||
|
From 0000000000000000000000000000000000000000 Mon Sep 17 00:00:00 2001
|
||||||
|
From: Hiroshige Hayashizaki <hiroshige@chromium.org>
|
||||||
|
Date: Tue, 16 Jul 2024 03:44:29 +0000
|
||||||
|
Subject: Handle ThrottlingURLLoader deletion during throttle calls
|
||||||
|
|
||||||
|
Theoretically `ThrottlingURLLoader` can be deleted during
|
||||||
|
throttle calls and some call sites have already protection
|
||||||
|
for such cases. This CL adds the protection for more call sites.
|
||||||
|
|
||||||
|
This CL also adds more unit tests for cancelling/deleting
|
||||||
|
`ThrottlingURLLoader` during throttle calls.
|
||||||
|
|
||||||
|
(cherry picked from commit c40f8866cfd6438725cc58e5db2d792e6d9f869b)
|
||||||
|
|
||||||
|
Bug: 349342289
|
||||||
|
Change-Id: I80d64be9ba1a3ac920315f5b4012b29c9737e414
|
||||||
|
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/5665925
|
||||||
|
Commit-Queue: Hiroshige Hayashizaki <hiroshige@chromium.org>
|
||||||
|
Reviewed-by: Tsuyoshi Horo <horo@chromium.org>
|
||||||
|
Cr-Original-Commit-Position: refs/heads/main@{#1323986}
|
||||||
|
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/5710951
|
||||||
|
Bot-Commit: Rubber Stamper <rubber-stamper@appspot.gserviceaccount.com>
|
||||||
|
Reviewed-by: Kouhei Ueno <kouhei@chromium.org>
|
||||||
|
Cr-Commit-Position: refs/branch-heads/6533@{#1515}
|
||||||
|
Cr-Branched-From: 7e0b87ec6b8cb5cb2969e1479fc25776e582721d-refs/heads/main@{#1313161}
|
||||||
|
|
||||||
|
diff --git a/third_party/blink/common/loader/throttling_url_loader.cc b/third_party/blink/common/loader/throttling_url_loader.cc
|
||||||
|
index d2de163172880bd23f7478c6743c2608a5f04d94..28f55e1b3867cbd1ff3817c21c1f648fef4fcbc7 100644
|
||||||
|
--- a/third_party/blink/common/loader/throttling_url_loader.cc
|
||||||
|
+++ b/third_party/blink/common/loader/throttling_url_loader.cc
|
||||||
|
@@ -660,8 +660,12 @@ void ThrottlingURLLoader::OnReceiveResponse(
|
||||||
|
for (auto& entry : throttles_) {
|
||||||
|
auto* throttle = entry.throttle.get();
|
||||||
|
base::Time start = base::Time::Now();
|
||||||
|
+ auto weak_ptr = weak_factory_.GetWeakPtr();
|
||||||
|
throttle->BeforeWillProcessResponse(response_url_, *response_head,
|
||||||
|
&has_pending_restart);
|
||||||
|
+ if (!weak_ptr) {
|
||||||
|
+ return;
|
||||||
|
+ }
|
||||||
|
RecordExecutionTimeHistogram("BeforeWillProcessResponse", start);
|
||||||
|
if (!HandleThrottleResult(throttle)) {
|
||||||
|
return;
|
||||||
|
@@ -681,8 +685,12 @@ void ThrottlingURLLoader::OnReceiveResponse(
|
||||||
|
auto* throttle = entry.throttle.get();
|
||||||
|
bool throttle_deferred = false;
|
||||||
|
base::Time start = base::Time::Now();
|
||||||
|
+ auto weak_ptr = weak_factory_.GetWeakPtr();
|
||||||
|
throttle->WillProcessResponse(response_url_, response_head.get(),
|
||||||
|
&throttle_deferred);
|
||||||
|
+ if (!weak_ptr) {
|
||||||
|
+ return;
|
||||||
|
+ }
|
||||||
|
RecordExecutionTimeHistogram(GetStageNameForHistogram(DEFERRED_RESPONSE),
|
||||||
|
start);
|
||||||
|
if (!HandleThrottleResult(throttle, throttle_deferred, &deferred))
|
||||||
|
@@ -852,7 +860,11 @@ void ThrottlingURLLoader::OnComplete(
|
||||||
|
for (auto& entry : throttles_) {
|
||||||
|
auto* throttle = entry.throttle.get();
|
||||||
|
base::Time start = base::Time::Now();
|
||||||
|
+ auto weak_ptr = weak_factory_.GetWeakPtr();
|
||||||
|
throttle->WillOnCompleteWithError(status);
|
||||||
|
+ if (!weak_ptr) {
|
||||||
|
+ return;
|
||||||
|
+ }
|
||||||
|
RecordExecutionTimeHistogram("WillOnCompleteWithError", start);
|
||||||
|
if (!HandleThrottleResult(throttle)) {
|
||||||
|
return;
|
||||||
|
diff --git a/third_party/blink/common/loader/throttling_url_loader_unittest.cc b/third_party/blink/common/loader/throttling_url_loader_unittest.cc
|
||||||
|
index 2c73705d12445c13067e937b4bfae1c99290da09..a7e037b2dde9390d9cc15d863ed926809f9afccf 100644
|
||||||
|
--- a/third_party/blink/common/loader/throttling_url_loader_unittest.cc
|
||||||
|
+++ b/third_party/blink/common/loader/throttling_url_loader_unittest.cc
|
||||||
|
@@ -338,9 +338,9 @@ class TestURLLoaderThrottle : public blink::URLLoaderThrottle {
|
||||||
|
network::mojom::URLResponseHead* response_head,
|
||||||
|
bool* defer) override {
|
||||||
|
will_process_response_called_++;
|
||||||
|
+ response_url_ = response_url;
|
||||||
|
if (will_process_response_callback_)
|
||||||
|
will_process_response_callback_.Run(delegate_.get(), defer);
|
||||||
|
- response_url_ = response_url;
|
||||||
|
}
|
||||||
|
|
||||||
|
void BeforeWillProcessResponse(
|
||||||
|
@@ -422,6 +422,11 @@ class ThrottlingURLLoaderTest : public testing::Test {
|
||||||
|
factory_.factory_remote().FlushForTesting();
|
||||||
|
}
|
||||||
|
|
||||||
|
+ void ResetLoader() {
|
||||||
|
+ ResetThrottleRawPointer();
|
||||||
|
+ loader_.reset();
|
||||||
|
+ }
|
||||||
|
+
|
||||||
|
void ResetThrottleRawPointer() { throttle_ = nullptr; }
|
||||||
|
|
||||||
|
// Be the first member so it is destroyed last.
|
||||||
|
@@ -467,6 +472,25 @@ TEST_F(ThrottlingURLLoaderTest, CancelBeforeStart) {
|
||||||
|
EXPECT_EQ(1u, client_.on_complete_called());
|
||||||
|
}
|
||||||
|
|
||||||
|
+TEST_F(ThrottlingURLLoaderTest, DeleteBeforeStart) {
|
||||||
|
+ base::RunLoop run_loop;
|
||||||
|
+ throttle_->set_will_start_request_callback(base::BindLambdaForTesting(
|
||||||
|
+ [this, &run_loop](blink::URLLoaderThrottle::Delegate* delegate,
|
||||||
|
+ bool* defer) {
|
||||||
|
+ ResetLoader();
|
||||||
|
+ run_loop.Quit();
|
||||||
|
+ }));
|
||||||
|
+
|
||||||
|
+ CreateLoaderAndStart();
|
||||||
|
+ run_loop.Run();
|
||||||
|
+
|
||||||
|
+ EXPECT_EQ(1u, factory_.create_loader_and_start_called());
|
||||||
|
+
|
||||||
|
+ EXPECT_EQ(0u, client_.on_received_response_called());
|
||||||
|
+ EXPECT_EQ(0u, client_.on_received_redirect_called());
|
||||||
|
+ EXPECT_EQ(0u, client_.on_complete_called());
|
||||||
|
+}
|
||||||
|
+
|
||||||
|
TEST_F(ThrottlingURLLoaderTest, DeferBeforeStart) {
|
||||||
|
throttle_->set_will_start_request_callback(base::BindLambdaForTesting(
|
||||||
|
[](blink::URLLoaderThrottle::Delegate* delegate, bool* defer) {
|
||||||
|
@@ -667,6 +691,88 @@ TEST_F(ThrottlingURLLoaderTest, CancelBeforeRedirect) {
|
||||||
|
EXPECT_EQ(1u, client_.on_complete_called());
|
||||||
|
}
|
||||||
|
|
||||||
|
+TEST_F(ThrottlingURLLoaderTest, DeleteBeforeRedirect) {
|
||||||
|
+ base::RunLoop run_loop;
|
||||||
|
+ throttle_->set_will_redirect_request_callback(base::BindLambdaForTesting(
|
||||||
|
+ [this, &run_loop](
|
||||||
|
+ blink::URLLoaderThrottle::Delegate* delegate, bool* /* defer */,
|
||||||
|
+ std::vector<std::string>* /* removed_headers */,
|
||||||
|
+ net::HttpRequestHeaders* /* modified_headers */,
|
||||||
|
+ net::HttpRequestHeaders* /* modified_cors_exempt_headers */) {
|
||||||
|
+ ResetLoader();
|
||||||
|
+ run_loop.Quit();
|
||||||
|
+ }));
|
||||||
|
+
|
||||||
|
+ CreateLoaderAndStart();
|
||||||
|
+
|
||||||
|
+ factory_.NotifyClientOnReceiveRedirect();
|
||||||
|
+
|
||||||
|
+ run_loop.Run();
|
||||||
|
+
|
||||||
|
+ EXPECT_EQ(0u, client_.on_received_response_called());
|
||||||
|
+ EXPECT_EQ(0u, client_.on_received_redirect_called());
|
||||||
|
+ EXPECT_EQ(0u, client_.on_complete_called());
|
||||||
|
+}
|
||||||
|
+
|
||||||
|
+TEST_F(ThrottlingURLLoaderTest, CancelBeforeWillRedirect) {
|
||||||
|
+ throttle_->set_before_will_redirect_request_callback(
|
||||||
|
+ base::BindLambdaForTesting(
|
||||||
|
+ [](blink::URLLoaderThrottle::Delegate* delegate,
|
||||||
|
+ RestartWithURLReset* restart_with_url_reset,
|
||||||
|
+ std::vector<std::string>* /* removed_headers */,
|
||||||
|
+ net::HttpRequestHeaders* /* modified_headers */,
|
||||||
|
+ net::HttpRequestHeaders* /* modified_cors_exempt_headers */) {
|
||||||
|
+ delegate->CancelWithError(net::ERR_ACCESS_DENIED);
|
||||||
|
+ }));
|
||||||
|
+
|
||||||
|
+ base::RunLoop run_loop;
|
||||||
|
+ client_.set_on_complete_callback(
|
||||||
|
+ base::BindLambdaForTesting([&run_loop](int error) {
|
||||||
|
+ EXPECT_EQ(net::ERR_ACCESS_DENIED, error);
|
||||||
|
+ run_loop.Quit();
|
||||||
|
+ }));
|
||||||
|
+
|
||||||
|
+ CreateLoaderAndStart();
|
||||||
|
+
|
||||||
|
+ factory_.NotifyClientOnReceiveRedirect();
|
||||||
|
+
|
||||||
|
+ run_loop.Run();
|
||||||
|
+
|
||||||
|
+ EXPECT_EQ(1u, throttle_->will_start_request_called());
|
||||||
|
+ EXPECT_EQ(1u, throttle_->will_redirect_request_called());
|
||||||
|
+ EXPECT_EQ(0u, throttle_->before_will_process_response_called());
|
||||||
|
+ EXPECT_EQ(0u, throttle_->will_process_response_called());
|
||||||
|
+
|
||||||
|
+ EXPECT_EQ(0u, client_.on_received_response_called());
|
||||||
|
+ EXPECT_EQ(0u, client_.on_received_redirect_called());
|
||||||
|
+ EXPECT_EQ(1u, client_.on_complete_called());
|
||||||
|
+}
|
||||||
|
+
|
||||||
|
+TEST_F(ThrottlingURLLoaderTest, DeleteBeforeWillRedirect) {
|
||||||
|
+ base::RunLoop run_loop;
|
||||||
|
+ throttle_->set_before_will_redirect_request_callback(
|
||||||
|
+ base::BindLambdaForTesting(
|
||||||
|
+ [this, &run_loop](
|
||||||
|
+ blink::URLLoaderThrottle::Delegate* delegate,
|
||||||
|
+ RestartWithURLReset* restart_with_url_reset,
|
||||||
|
+ std::vector<std::string>* /* removed_headers */,
|
||||||
|
+ net::HttpRequestHeaders* /* modified_headers */,
|
||||||
|
+ net::HttpRequestHeaders* /* modified_cors_exempt_headers */) {
|
||||||
|
+ ResetLoader();
|
||||||
|
+ run_loop.Quit();
|
||||||
|
+ }));
|
||||||
|
+
|
||||||
|
+ CreateLoaderAndStart();
|
||||||
|
+
|
||||||
|
+ factory_.NotifyClientOnReceiveRedirect();
|
||||||
|
+
|
||||||
|
+ run_loop.Run();
|
||||||
|
+
|
||||||
|
+ EXPECT_EQ(0u, client_.on_received_response_called());
|
||||||
|
+ EXPECT_EQ(0u, client_.on_received_redirect_called());
|
||||||
|
+ EXPECT_EQ(0u, client_.on_complete_called());
|
||||||
|
+}
|
||||||
|
+
|
||||||
|
TEST_F(ThrottlingURLLoaderTest, DeferBeforeRedirect) {
|
||||||
|
base::RunLoop run_loop1;
|
||||||
|
throttle_->set_will_redirect_request_callback(base::BindLambdaForTesting(
|
||||||
|
@@ -880,6 +986,77 @@ TEST_F(ThrottlingURLLoaderTest, CancelBeforeResponse) {
|
||||||
|
EXPECT_EQ(1u, client_.on_complete_called());
|
||||||
|
}
|
||||||
|
|
||||||
|
+TEST_F(ThrottlingURLLoaderTest, DeleteBeforeResponse) {
|
||||||
|
+ base::RunLoop run_loop;
|
||||||
|
+ throttle_->set_will_process_response_callback(base::BindLambdaForTesting(
|
||||||
|
+ [this, &run_loop](blink::URLLoaderThrottle::Delegate* delegate,
|
||||||
|
+ bool* defer) {
|
||||||
|
+ ResetLoader();
|
||||||
|
+ run_loop.Quit();
|
||||||
|
+ }));
|
||||||
|
+
|
||||||
|
+ CreateLoaderAndStart();
|
||||||
|
+
|
||||||
|
+ factory_.NotifyClientOnReceiveResponse();
|
||||||
|
+
|
||||||
|
+ run_loop.Run();
|
||||||
|
+
|
||||||
|
+ EXPECT_EQ(0u, client_.on_received_response_called());
|
||||||
|
+ EXPECT_EQ(0u, client_.on_received_redirect_called());
|
||||||
|
+ EXPECT_EQ(0u, client_.on_complete_called());
|
||||||
|
+}
|
||||||
|
+
|
||||||
|
+TEST_F(ThrottlingURLLoaderTest, CancelBeforeWillProcessResponse) {
|
||||||
|
+ throttle_->set_before_will_process_response_callback(
|
||||||
|
+ base::BindLambdaForTesting(
|
||||||
|
+ [](blink::URLLoaderThrottle::Delegate* delegate,
|
||||||
|
+ RestartWithURLReset* restart_with_url_reset) {
|
||||||
|
+ delegate->CancelWithError(net::ERR_ACCESS_DENIED);
|
||||||
|
+ }));
|
||||||
|
+
|
||||||
|
+ base::RunLoop run_loop;
|
||||||
|
+ client_.set_on_complete_callback(
|
||||||
|
+ base::BindLambdaForTesting([&run_loop](int error) {
|
||||||
|
+ EXPECT_EQ(net::ERR_ACCESS_DENIED, error);
|
||||||
|
+ run_loop.Quit();
|
||||||
|
+ }));
|
||||||
|
+
|
||||||
|
+ CreateLoaderAndStart();
|
||||||
|
+
|
||||||
|
+ factory_.NotifyClientOnReceiveResponse();
|
||||||
|
+
|
||||||
|
+ run_loop.Run();
|
||||||
|
+
|
||||||
|
+ EXPECT_EQ(1u, throttle_->will_start_request_called());
|
||||||
|
+ EXPECT_EQ(0u, throttle_->will_redirect_request_called());
|
||||||
|
+ EXPECT_EQ(1u, throttle_->before_will_process_response_called());
|
||||||
|
+ EXPECT_EQ(0u, throttle_->will_process_response_called());
|
||||||
|
+ EXPECT_EQ(0u, client_.on_received_response_called());
|
||||||
|
+ EXPECT_EQ(0u, client_.on_received_redirect_called());
|
||||||
|
+ EXPECT_EQ(1u, client_.on_complete_called());
|
||||||
|
+}
|
||||||
|
+
|
||||||
|
+TEST_F(ThrottlingURLLoaderTest, DeleteBeforeWillProcessResponse) {
|
||||||
|
+ base::RunLoop run_loop;
|
||||||
|
+ throttle_->set_before_will_process_response_callback(
|
||||||
|
+ base::BindLambdaForTesting(
|
||||||
|
+ [this, &run_loop](blink::URLLoaderThrottle::Delegate* delegate,
|
||||||
|
+ RestartWithURLReset* restart_with_url_reset) {
|
||||||
|
+ ResetLoader();
|
||||||
|
+ run_loop.Quit();
|
||||||
|
+ }));
|
||||||
|
+
|
||||||
|
+ CreateLoaderAndStart();
|
||||||
|
+
|
||||||
|
+ factory_.NotifyClientOnReceiveResponse();
|
||||||
|
+
|
||||||
|
+ run_loop.Run();
|
||||||
|
+
|
||||||
|
+ EXPECT_EQ(0u, client_.on_received_response_called());
|
||||||
|
+ EXPECT_EQ(0u, client_.on_received_redirect_called());
|
||||||
|
+ EXPECT_EQ(0u, client_.on_complete_called());
|
||||||
|
+}
|
||||||
|
+
|
||||||
|
TEST_F(ThrottlingURLLoaderTest, DeferBeforeResponse) {
|
||||||
|
base::RunLoop run_loop1;
|
||||||
|
throttle_->set_will_process_response_callback(base::BindRepeating(
|
||||||
|
|
@ -1 +1,2 @@
|
||||||
dawn_dxc_disable_dxc_pass_structurize-loop-exits-for-unroll.patch
|
dawn_dxc_disable_dxc_pass_structurize-loop-exits-for-unroll.patch
|
||||||
|
deps_update_dxc_to_patched_branch.patch
|
||||||
|
|
|
||||||
28
patches/dawn/deps_update_dxc_to_patched_branch.patch
Normal file
28
patches/dawn/deps_update_dxc_to_patched_branch.patch
Normal file
|
|
@ -0,0 +1,28 @@
|
||||||
|
From 0000000000000000000000000000000000000000 Mon Sep 17 00:00:00 2001
|
||||||
|
From: David Neto <dneto@google.com>
|
||||||
|
Date: Wed, 17 Jul 2024 14:09:26 +0000
|
||||||
|
Subject: DEPS: Update DXC to patched branch
|
||||||
|
|
||||||
|
Also, add DXC_CODEGEN_EXCEPTIONS_TRAP=1 to DXC gn config.
|
||||||
|
|
||||||
|
Bug: chromium:346618785, chromium:350696474
|
||||||
|
Change-Id: I4c1ac753d824c4790e78d8f36231c8f2fe0e2722
|
||||||
|
Reviewed-on: https://dawn-review.googlesource.com/c/dawn/+/198756
|
||||||
|
Reviewed-by: Natalie Chouinard <chouinard@google.com>
|
||||||
|
|
||||||
|
diff --git a/third_party/gn/dxc/BUILD.gn b/third_party/gn/dxc/BUILD.gn
|
||||||
|
index aadb64eeb4fba2f2da442a8a7f4b285a469c5cdd..0060484cefcf1835138b111258c33e09fab1279c 100644
|
||||||
|
--- a/third_party/gn/dxc/BUILD.gn
|
||||||
|
+++ b/third_party/gn/dxc/BUILD.gn
|
||||||
|
@@ -453,7 +453,10 @@ cmake_configure_file("clang-config-h") {
|
||||||
|
cmake_configure_file("dxc-config-h") {
|
||||||
|
input = "$dawn_dxc_dir/include/dxc/config.h.cmake"
|
||||||
|
output = "$target_gen_dir/include/dxc/config.h"
|
||||||
|
- values = [ "DXC_DISABLE_ALLOCATOR_OVERRIDES=1" ]
|
||||||
|
+ values = [
|
||||||
|
+ "DXC_DISABLE_ALLOCATOR_OVERRIDES=1",
|
||||||
|
+ "DXC_CODEGEN_EXCEPTIONS_TRAP=1",
|
||||||
|
+ ]
|
||||||
|
}
|
||||||
|
|
||||||
|
#######################################################################
|
||||||
Loading…
Add table
Reference in a new issue