llvmorg-github-actions[bot] wrote:
<!--LLVM PR SUMMARY COMMENT--> @llvm/pr-subscribers-clang @llvm/pr-subscribers-clangir Author: Konstantinos Parasyris (koparasy) <details> <summary>Changes</summary> Enforce that a static_local_guard is always paired with a static_local_info attribute via a GlobalOp verifier. Part of #<!-- -->219048 --- Full diff: https://github.com/llvm/llvm-project/pull/224744.diff 6 Files Affected: - (modified) clang/lib/CIR/Dialect/IR/CIRDialect.cpp (+5) - (modified) clang/lib/CIR/Dialect/Transforms/LoweringPrepare.cpp (+6-1) - (added) clang/test/CIR/IR/invalid-static-local-info.cir (+13) - (modified) clang/test/CIR/IR/invalid-static-local.cir (+3) - (modified) clang/test/CIR/IR/invalid-tls.cir (+1) - (modified) clang/test/CIR/IR/static-local.cir (+9-1) ``````````diff diff --git a/clang/lib/CIR/Dialect/IR/CIRDialect.cpp b/clang/lib/CIR/Dialect/IR/CIRDialect.cpp index 38ef8409634c7b..643e87e69dcec4 100644 --- a/clang/lib/CIR/Dialect/IR/CIRDialect.cpp +++ b/clang/lib/CIR/Dialect/IR/CIRDialect.cpp @@ -2293,6 +2293,11 @@ mlir::LogicalResult cir::GlobalOp::verify() { "Cannot have a static-local global-op with a constructor or " "destructor, they require in-function initialization via LocalInitOp"); + // A guard implies the info attribute; + if (getStaticLocalGuard().has_value() && !getStaticLocalInfo().has_value()) + return emitOpError( + "'static_local_guard' requires 'static_local_info' to be present"); + if (getTlsRefs()) { if (getStaticLocalGuard().has_value()) return emitOpError("cannot have both static local and tls references"); diff --git a/clang/lib/CIR/Dialect/Transforms/LoweringPrepare.cpp b/clang/lib/CIR/Dialect/Transforms/LoweringPrepare.cpp index b5e00eb331903a..353d3d6c4614b5 100644 --- a/clang/lib/CIR/Dialect/Transforms/LoweringPrepare.cpp +++ b/clang/lib/CIR/Dialect/Transforms/LoweringPrepare.cpp @@ -1365,7 +1365,12 @@ void LoweringPreparePass::handleStaticLocal(cir::GlobalOp globalOp, // CIRGen, so this pass does not need a live ASTContext to read them. std::optional<cir::StaticLocalInfoAttr> infoOption = globalOp.getStaticLocalInfo(); - assert(infoOption.has_value()); + // Verified IR guarantees this + if (!infoOption.has_value()) { + globalOp->emitError( + "static-local global with a guard is missing 'static_local_info'"); + return; + } cir::StaticLocalInfoAttr info = infoOption.value(); builder.setInsertionPointAfter(localInitOp); diff --git a/clang/test/CIR/IR/invalid-static-local-info.cir b/clang/test/CIR/IR/invalid-static-local-info.cir new file mode 100644 index 00000000000000..db866e5d8d9ff6 --- /dev/null +++ b/clang/test/CIR/IR/invalid-static-local-info.cir @@ -0,0 +1,13 @@ +// RUN: cir-opt %s -verify-diagnostics -split-input-file + +// A guarded static-local relies on 'static_local_info' during LoweringPrepare, +// so the GlobalOp verifier rejects a 'static_local_guard' without it. This +// guards the static-local lowering path against hand-written or serialized +// .cir that would otherwise crash lowering with a missing info attribute. + +!s32i = !cir.int<s, 32> + +module { + // expected-error@+1 {{'static_local_guard' requires 'static_local_info' to be present}} + cir.global "private" internal static_local_guard<"_ZGVZ1fvE1x"> @_ZZ1fvE1x : !s32i +} diff --git a/clang/test/CIR/IR/invalid-static-local.cir b/clang/test/CIR/IR/invalid-static-local.cir index 2a4d22d9ceae0d..ee8d188c58f769 100644 --- a/clang/test/CIR/IR/invalid-static-local.cir +++ b/clang/test/CIR/IR/invalid-static-local.cir @@ -23,6 +23,7 @@ module { // Global is marked static_local_guard, but get_global is not static_local cir.global "private" internal static_local_guard<"_ZGVZ1fvE1y"> @_ZZ1fvE1y : !s32i + {static_local_info = #cir.static_local_info<local = true, tls = none, is_inline = false, tsk = undeclared>} cir.func @test_static_local_mismatch_reverse() { // expected-error @below {{static_local attribute mismatch}} @@ -40,6 +41,7 @@ module { // local_init is both static_local and thread_local cir.global "private" internal static_local_guard<"_ZGVZ1fvE1y"> @_ZZ1fvE1y : !s32i + {static_local_info = #cir.static_local_info<local = true, tls = none, is_inline = false, tsk = undeclared>} cir.func @test_static_local_and_tls() { %0 = cir.get_global static_local @_ZZ1fvE1y : !cir.ptr<!s32i> @@ -63,6 +65,7 @@ module { // local_init not at function scope. cir.global "private" internal static_local_guard<"_ZGVZ1fvE1y"> @_ZZ1fvE1y : !s32i + {static_local_info = #cir.static_local_info<local = true, tls = none, is_inline = false, tsk = undeclared>} cir.global "private" internal @_AnotherGlobal = ctor : !s32i { // expected-error @below {{'cir.local_init' op expects ancestor op 'cir.func'}} diff --git a/clang/test/CIR/IR/invalid-tls.cir b/clang/test/CIR/IR/invalid-tls.cir index 99bd87d235f5b7..27dbe07d0bcc09 100644 --- a/clang/test/CIR/IR/invalid-tls.cir +++ b/clang/test/CIR/IR/invalid-tls.cir @@ -18,6 +18,7 @@ module { module { // expected-error@+1{{op cannot have both static local and tls references}} cir.global "private" internal tls_model = tls_dyn tls_refs = <"asdf", "asdf", "asdf"> static_local_guard<"asdf"> @_ZZ1fvE1y : !s32i + {static_local_info = #cir.static_local_info<local = true, tls = dynamic, is_inline = false, tsk = undeclared>} } // ----- diff --git a/clang/test/CIR/IR/static-local.cir b/clang/test/CIR/IR/static-local.cir index e0623bbdf09bae..41f76bb58a6f5c 100644 --- a/clang/test/CIR/IR/static-local.cir +++ b/clang/test/CIR/IR/static-local.cir @@ -4,17 +4,25 @@ module { -// Test static_local_guard attribute on global and static_local on get_global +// Test static_local_guard attribute on global and static_local on get_global. +// The GlobalOp verifier requires static_local_guard and static_local_info to +// be paired, matching what CIRGen emits, so each guarded global also carries +// the info attribute here. cir.global "private" internal static_local_guard<"_ZGVZ1fvE1x"> @_ZZ1fvE1x : !s32i + {static_local_info = #cir.static_local_info<local = true, tls = none, is_inline = false, tsk = undeclared>} // CHECK: cir.global "private" internal static_local_guard<"_ZGVZ1fvE1x"> @_ZZ1fvE1x : !s32i cir.global "private" internal static_local_guard<"_HasInitGuard"> @_HasInit : !s32i + {static_local_info = #cir.static_local_info<local = true, tls = none, is_inline = false, tsk = undeclared>} // CHECK: cir.global "private" internal static_local_guard<"_HasInitGuard"> @_HasInit : !s32i cir.global "private" internal static_local_guard<"_HasInitGuard2"> @_HasInit2 : !s32i + {static_local_info = #cir.static_local_info<local = true, tls = none, is_inline = false, tsk = undeclared>} // CHECK: cir.global "private" internal static_local_guard<"_HasInitGuard2"> @_HasInit2 : !s32i cir.global "private" internal static_local_guard<"_HasInitGuard3"> @_HasInit3 : !s32i + {static_local_info = #cir.static_local_info<local = true, tls = none, is_inline = false, tsk = undeclared>} // CHECK: cir.global "private" internal static_local_guard<"_HasInitGuard3"> @_HasInit3 : !s32i cir.global "private" internal static_local_guard<"_HasInitGuard4"> @_HasInit4 : !s32i + {static_local_info = #cir.static_local_info<local = true, tls = none, is_inline = false, tsk = undeclared>} // CHECK: cir.global "private" internal static_local_guard<"_HasInitGuard4"> @_HasInit4 : !s32i cir.func @test_static_local() { `````````` </details> https://github.com/llvm/llvm-project/pull/224744 _______________________________________________ cfe-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
