From 6a77cff1cd5012343dd73fa20237f4d55bb6a8d3 Mon Sep 17 00:00:00 2001 From: Nikita Popov Date: Thu, 3 Apr 2025 17:39:49 +0200 Subject: [PATCH 1/2] [Verifier] Require that dbg.declare variable is a ptr As far as I understand, the first operand of dbg_declare should be a pointer (inside a metadata wrapper). However, using a non-pointer is currently not rejected, and we have some tests that use non-pointer types. As far as I can tell, these tests either meant to use dbg_value or are just incorrect hand-crafted tests. Ran into this while trying to fix #134008. --- llvm/lib/IR/Verifier.cpp | 7 +++++-- llvm/test/CodeGen/AArch64/fast-isel-dbg.ll | 4 ++-- .../test/CodeGen/AArch64/selectiondag-order.ll | 3 ++- llvm/test/CodeGen/MIR/X86/diexpr-win32.mir | 2 +- llvm/test/CodeGen/X86/selectiondag-order.ll | 3 ++- llvm/test/DebugInfo/ARM/lowerbdgdeclare_vla.ll | 4 ++-- llvm/test/Transforms/Coroutines/coro-debug.ll | 18 +++++++++--------- .../Transforms/LoopVectorize/discriminator.ll | 4 ++-- 8 files changed, 25 insertions(+), 20 deletions(-) diff --git a/llvm/lib/IR/Verifier.cpp b/llvm/lib/IR/Verifier.cpp index 7c6cd414554e3..81ddb62fb9561 100644 --- a/llvm/lib/IR/Verifier.cpp +++ b/llvm/lib/IR/Verifier.cpp @@ -6666,9 +6666,12 @@ void Verifier::visit(DbgVariableRecord &DVR) { CheckDI(MD && (isa(MD) || isa(MD) || (isa(MD) && !cast(MD)->getNumOperands())), "invalid #dbg record address/value", &DVR, MD); - if (auto *VAM = dyn_cast(MD)) + if (auto *VAM = dyn_cast(MD)) { visitValueAsMetadata(*VAM, F); - else if (auto *AL = dyn_cast(MD)) + if (DVR.isDbgDeclare()) + CheckDI(VAM->getValue()->getType()->isPointerTy(), + "location of #dbg_declare must be a pointer", &DVR, MD); + } else if (auto *AL = dyn_cast(MD)) visitDIArgList(*AL, F); CheckDI(isa_and_nonnull(DVR.getRawVariable()), diff --git a/llvm/test/CodeGen/AArch64/fast-isel-dbg.ll b/llvm/test/CodeGen/AArch64/fast-isel-dbg.ll index d17c14747b942..ac6fa71e9a9e4 100644 --- a/llvm/test/CodeGen/AArch64/fast-isel-dbg.ll +++ b/llvm/test/CodeGen/AArch64/fast-isel-dbg.ll @@ -6,8 +6,8 @@ target triple="aarch64--" ; CHECK-LABEL: name: func ; CHECK: DBG_VALUE -define void @func(i32 %a) !dbg !4 { - call void @llvm.dbg.declare(metadata i32 %a, metadata !5, metadata !DIExpression()), !dbg !7 +define void @func(ptr %a) !dbg !4 { + call void @llvm.dbg.declare(metadata ptr %a, metadata !5, metadata !DIExpression()), !dbg !7 ret void } diff --git a/llvm/test/CodeGen/AArch64/selectiondag-order.ll b/llvm/test/CodeGen/AArch64/selectiondag-order.ll index fb40653723fec..a633fc0785896 100644 --- a/llvm/test/CodeGen/AArch64/selectiondag-order.ll +++ b/llvm/test/CodeGen/AArch64/selectiondag-order.ll @@ -55,8 +55,9 @@ end: ; preds = %body define i64 @simulateWithDbgDeclare(<2 x i32> %a) local_unnamed_addr { entry: + %ptr = alloca i32 %rand = tail call i64 @lrand48() #3 - tail call void @llvm.dbg.declare(metadata i64 %rand, metadata !6, metadata !7), !dbg !8 + tail call void @llvm.dbg.declare(metadata ptr %ptr, metadata !6, metadata !7), !dbg !8 br label %body body: ; preds = %body, %entry diff --git a/llvm/test/CodeGen/MIR/X86/diexpr-win32.mir b/llvm/test/CodeGen/MIR/X86/diexpr-win32.mir index b1bcf24f8c5f4..d8d76758a08a0 100644 --- a/llvm/test/CodeGen/MIR/X86/diexpr-win32.mir +++ b/llvm/test/CodeGen/MIR/X86/diexpr-win32.mir @@ -82,7 +82,7 @@ entry: %0 = bitcast ptr %s to ptr %bytes = load i32, ptr %0, !dbg !34 - call void @llvm.dbg.declare(metadata i32 %bytes, metadata !35, metadata !28), !dbg !34 + call void @llvm.dbg.value(metadata i32 %bytes, metadata !35, metadata !28), !dbg !34 %1 = add i32 %bytes, %acc, !dbg !36 ret i32 %1, !dbg !36 } diff --git a/llvm/test/CodeGen/X86/selectiondag-order.ll b/llvm/test/CodeGen/X86/selectiondag-order.ll index 163e2cb90b2fe..924211325c2b0 100644 --- a/llvm/test/CodeGen/X86/selectiondag-order.ll +++ b/llvm/test/CodeGen/X86/selectiondag-order.ll @@ -55,8 +55,9 @@ end: ; preds = %body define i64 @simulateWithDbgDeclare(<2 x i32> %a) local_unnamed_addr { entry: + %ptr = alloca i32 %rand = tail call i64 @lrand48() #3 - tail call void @llvm.dbg.declare(metadata i64 %rand, metadata !6, metadata !7), !dbg !8 + tail call void @llvm.dbg.declare(metadata ptr %ptr, metadata !6, metadata !7), !dbg !8 br label %body body: ; preds = %body, %entry diff --git a/llvm/test/DebugInfo/ARM/lowerbdgdeclare_vla.ll b/llvm/test/DebugInfo/ARM/lowerbdgdeclare_vla.ll index 94b527a445d3a..35b7b044abb55 100644 --- a/llvm/test/DebugInfo/ARM/lowerbdgdeclare_vla.ll +++ b/llvm/test/DebugInfo/ARM/lowerbdgdeclare_vla.ll @@ -19,9 +19,9 @@ target triple = "thumbv7-apple-ios8.0.0" ; Function Attrs: nounwind optsize readnone define void @run(float %r) #0 !dbg !4 { entry: - tail call void @llvm.dbg.declare(metadata float %r, metadata !11, metadata !DIExpression()), !dbg !22 + tail call void @llvm.dbg.value(metadata float %r, metadata !11, metadata !DIExpression()), !dbg !22 %conv = fptosi float %r to i32, !dbg !23 - tail call void @llvm.dbg.declare(metadata i32 %conv, metadata !12, metadata !DIExpression()), !dbg !23 + tail call void @llvm.dbg.value(metadata i32 %conv, metadata !12, metadata !DIExpression()), !dbg !23 %vla = alloca float, i32 %conv, align 4, !dbg !24 tail call void @llvm.dbg.declare(metadata ptr %vla, metadata !14, metadata !DIExpression(DW_OP_deref)), !dbg !24 ; The VLA alloca should be described by a dbg.declare: diff --git a/llvm/test/Transforms/Coroutines/coro-debug.ll b/llvm/test/Transforms/Coroutines/coro-debug.ll index 3a4eeddbe6198..17a0b80c5b5e5 100644 --- a/llvm/test/Transforms/Coroutines/coro-debug.ll +++ b/llvm/test/Transforms/Coroutines/coro-debug.ll @@ -29,8 +29,8 @@ sw.bb: ; preds = %entry %direct = load i32, ptr %x.addr, align 4, !dbg !14 %gep = getelementptr inbounds [16 x i8], ptr undef, i32 %direct, !dbg !14 call void @llvm.dbg.declare(metadata ptr %gep, metadata !27, metadata !13), !dbg !14 - call void @llvm.dbg.declare(metadata i32 %conv, metadata !26, metadata !13), !dbg !14 - call void @llvm.dbg.declare(metadata i32 %direct, metadata !25, metadata !13), !dbg !14 + call void @llvm.dbg.value(metadata i32 %conv, metadata !26, metadata !13), !dbg !14 + call void @llvm.dbg.value(metadata i32 %direct, metadata !25, metadata !13), !dbg !14 call void @llvm.dbg.declare(metadata ptr %x.addr, metadata !12, metadata !13), !dbg !14 call void @llvm.dbg.declare(metadata ptr %coro_hdl, metadata !15, metadata !13), !dbg !16 call void @llvm.dbg.declare(metadata ptr %late_local, metadata !29, metadata !13), !dbg !16 @@ -66,7 +66,7 @@ coro_Cleanup: ; preds = %sw.epilog, %sw.bb1 %5 = load ptr, ptr %coro_hdl, align 8, !dbg !24 %6 = call ptr @llvm.coro.free(token %0, ptr %5), !dbg !24 call void @free(ptr %6), !dbg !24 - call void @llvm.dbg.declare(metadata i32 %asm_res, metadata !32, metadata !13), !dbg !16 + call void @llvm.dbg.value(metadata i32 %asm_res, metadata !32, metadata !13), !dbg !16 br label %coro_Suspend, !dbg !24 coro_Suspend: ; preds = %coro_Cleanup, %sw.default @@ -176,14 +176,14 @@ attributes #7 = { noduplicate } ; CHECK: %[[DBG_PTR:.*]] = alloca ptr ; CHECK: #dbg_declare(ptr %[[DBG_PTR]], ![[RESUME_COROHDL:[0-9]+]], !DIExpression(DW_OP_deref, DW_OP_plus_uconst, ; CHECK: #dbg_declare(ptr %[[DBG_PTR]], ![[RESUME_X:[0-9]+]], !DIExpression(DW_OP_deref, DW_OP_plus_uconst, [[EXPR_TAIL:.*]]) -; CHECK: #dbg_declare(ptr %[[DBG_PTR]], ![[RESUME_DIRECT:[0-9]+]], !DIExpression(DW_OP_deref, DW_OP_plus_uconst, [[EXPR_TAIL]]) ; CHECK: store ptr {{.*}}, ptr %[[DBG_PTR]] ; CHECK-NOT: alloca ptr -; CHECK: #dbg_declare(i8 0, ![[RESUME_CONST:[0-9]+]], !DIExpression(DW_OP_LLVM_convert, 8, DW_ATE_signed, DW_OP_LLVM_convert, 32, DW_ATE_signed), +; CHECK: call void @coro.devirt.trigger(ptr null) +; CHECK: #dbg_value(i8 0, ![[RESUME_CONST:[0-9]+]], !DIExpression(DW_OP_LLVM_convert, 8, DW_ATE_signed, DW_OP_LLVM_convert, 32, DW_ATE_signed), +; CHECK: #dbg_value(ptr %[[DBG_PTR]], ![[RESUME_DIRECT:[0-9]+]], !DIExpression(DW_OP_deref, DW_OP_plus_uconst, {{[0-9]+}}, DW_OP_deref), ; Note that keeping the undef value here could be acceptable, too. ; CHECK-NOT: #dbg_declare(ptr undef, !{{[0-9]+}}, !DIExpression(), -; CHECK: call void @coro.devirt.trigger(ptr null) -; CHECK: #dbg_value(ptr {{.*}}, ![[RESUME_DIRECT_VALUE:[0-9]+]], !DIExpression(DW_OP_deref, DW_OP_plus_uconst, {{[0-9]+}}, DW_OP_deref), +; CHECK: #dbg_value(ptr %[[DBG_PTR]], ![[RESUME_DIRECT_VALUE:[0-9]+]], !DIExpression(DW_OP_deref, DW_OP_plus_uconst, {{[0-9]+}}, DW_OP_deref), ; Check that the dbg.declare intrinsic of invoke instruction is hanled correctly. ; CHECK: %[[ALLOCATED_STORAGE:.+]] = invoke ptr @allocate() ; CHECK-NEXT: to label %[[NORMAL_DEST:.+]] unwind @@ -193,7 +193,7 @@ attributes #7 = { noduplicate } ; CHECK-NEXT: to label %[[DEFAULT_DEST:.+]] [label ; CHECK: [[DEFAULT_DEST]]: ; CHECK-NOT: {{.*}}: -; CHECK: #dbg_declare(i32 %[[CALLBR_RES]] +; CHECK: #dbg_value(i32 %[[CALLBR_RES]] ; CHECK: define internal fastcc void @f.destroy(ptr noundef nonnull align 8 dereferenceable(40) %0) #0 personality i32 0 !dbg ![[DESTROY:[0-9]+]] ; CHECK: define internal fastcc void @f.cleanup(ptr noundef nonnull align 8 dereferenceable(40) %0) #0 personality i32 0 !dbg ![[CLEANUP:[0-9]+]] @@ -202,8 +202,8 @@ attributes #7 = { noduplicate } ; CHECK: ![[RESUME]] = distinct !DISubprogram(name: "f", linkageName: "flink" ; CHECK: ![[RESUME_COROHDL]] = !DILocalVariable(name: "coro_hdl", scope: ![[RESUME]] ; CHECK: ![[RESUME_X]] = !DILocalVariable(name: "x", arg: 1, scope: ![[RESUME]] -; CHECK: ![[RESUME_DIRECT]] = !DILocalVariable(name: "direct_mem", scope: ![[RESUME]] ; CHECK: ![[RESUME_CONST]] = !DILocalVariable(name: "direct_const", scope: ![[RESUME]] +; CHECK: ![[RESUME_DIRECT]] = !DILocalVariable(name: "direct_mem", scope: ![[RESUME]] ; CHECK: ![[RESUME_DIRECT_VALUE]] = !DILocalVariable(name: "direct_value", scope: ![[RESUME]] ; CHECK: ![[DESTROY]] = distinct !DISubprogram(name: "f", linkageName: "flink" diff --git a/llvm/test/Transforms/LoopVectorize/discriminator.ll b/llvm/test/Transforms/LoopVectorize/discriminator.ll index b66a70b9768c4..fe71b6bd9e765 100644 --- a/llvm/test/Transforms/LoopVectorize/discriminator.ll +++ b/llvm/test/Transforms/LoopVectorize/discriminator.ll @@ -32,8 +32,8 @@ define void @_Z3foov() local_unnamed_addr #0 !dbg !6 { %7 = load i32, ptr %6, align 4, !dbg !17, !tbaa !15 %8 = add nsw i32 %7, %5, !dbg !17 ;PSEUDO_PROBE-COUNT-5: call void @llvm.pseudoprobe(i64 6699318081062747564, i64 2, i32 0, i64 -1), !dbg ![[#PROBE:]] -;DBG_VALUE: #dbg_declare{{.*}} ![[DBG:[0-9]*]] - call void @llvm.dbg.declare(metadata i32 %8, metadata !22, metadata !DIExpression()), !dbg !17 +;DBG_VALUE: #dbg_value{{.*}} ![[DBG:[0-9]*]] + call void @llvm.dbg.value(metadata i32 %8, metadata !22, metadata !DIExpression()), !dbg !17 store i32 %8, ptr %6, align 4, !dbg !17, !tbaa !15 %indvars.iv.next = add nuw nsw i64 %indvars.iv, 1, !dbg !18 %exitcond = icmp eq i64 %indvars.iv.next, 4096, !dbg !19 From 541460c5bdeca866e3bb4c1e70a2f94fc0cd8ee2 Mon Sep 17 00:00:00 2001 From: Nikita Popov Date: Fri, 4 Apr 2025 12:17:01 +0200 Subject: [PATCH 2/2] Address review feedback --- llvm/docs/SourceLevelDebugging.rst | 8 ++++---- llvm/lib/IR/Verifier.cpp | 3 ++- llvm/test/CodeGen/AArch64/selectiondag-order.ll | 3 +-- llvm/test/CodeGen/X86/selectiondag-order.ll | 3 +-- 4 files changed, 8 insertions(+), 9 deletions(-) diff --git a/llvm/docs/SourceLevelDebugging.rst b/llvm/docs/SourceLevelDebugging.rst index c1a95efd2d8bc..b3007756a8d07 100644 --- a/llvm/docs/SourceLevelDebugging.rst +++ b/llvm/docs/SourceLevelDebugging.rst @@ -208,10 +208,10 @@ comma-separated arguments in parentheses, as with a `call`. #dbg_declare([Value|MDNode], DILocalVariable, DIExpression, DILocation) This record provides information about a local element (e.g., variable). -The first argument is an SSA value corresponding to a variable address, and is -typically a static alloca in the function entry block. The second argument is a -`local variable `_ containing a description of -the variable. The third argument is a `complex expression +The first argument is an SSA ``ptr`` value corresponding to a variable address, +and is typically a static alloca in the function entry block. The second +argument is a `local variable `_ containing a +description of the variable. The third argument is a `complex expression `_. The fourth argument is a `source location `_. A ``#dbg_declare`` record describes the *address* of a source variable. diff --git a/llvm/lib/IR/Verifier.cpp b/llvm/lib/IR/Verifier.cpp index 81ddb62fb9561..7423e746dfa9a 100644 --- a/llvm/lib/IR/Verifier.cpp +++ b/llvm/lib/IR/Verifier.cpp @@ -6671,8 +6671,9 @@ void Verifier::visit(DbgVariableRecord &DVR) { if (DVR.isDbgDeclare()) CheckDI(VAM->getValue()->getType()->isPointerTy(), "location of #dbg_declare must be a pointer", &DVR, MD); - } else if (auto *AL = dyn_cast(MD)) + } else if (auto *AL = dyn_cast(MD)) { visitDIArgList(*AL, F); + } CheckDI(isa_and_nonnull(DVR.getRawVariable()), "invalid #dbg record variable", &DVR, DVR.getRawVariable()); diff --git a/llvm/test/CodeGen/AArch64/selectiondag-order.ll b/llvm/test/CodeGen/AArch64/selectiondag-order.ll index a633fc0785896..32534fa79a34a 100644 --- a/llvm/test/CodeGen/AArch64/selectiondag-order.ll +++ b/llvm/test/CodeGen/AArch64/selectiondag-order.ll @@ -53,9 +53,8 @@ end: ; preds = %body ; AARCH64-CHECK: BB1_1: -define i64 @simulateWithDbgDeclare(<2 x i32> %a) local_unnamed_addr { +define i64 @simulateWithDbgDeclare(<2 x i32> %a, ptr %ptr) local_unnamed_addr { entry: - %ptr = alloca i32 %rand = tail call i64 @lrand48() #3 tail call void @llvm.dbg.declare(metadata ptr %ptr, metadata !6, metadata !7), !dbg !8 br label %body diff --git a/llvm/test/CodeGen/X86/selectiondag-order.ll b/llvm/test/CodeGen/X86/selectiondag-order.ll index 924211325c2b0..63ab06e660550 100644 --- a/llvm/test/CodeGen/X86/selectiondag-order.ll +++ b/llvm/test/CodeGen/X86/selectiondag-order.ll @@ -53,9 +53,8 @@ end: ; preds = %body ; X86-CHECK: callq lrand48 ; X86-CHECK: movq %rax, %rbx -define i64 @simulateWithDbgDeclare(<2 x i32> %a) local_unnamed_addr { +define i64 @simulateWithDbgDeclare(<2 x i32> %a, ptr %ptr) local_unnamed_addr { entry: - %ptr = alloca i32 %rand = tail call i64 @lrand48() #3 tail call void @llvm.dbg.declare(metadata ptr %ptr, metadata !6, metadata !7), !dbg !8 br label %body