[PATCH] D101540: [DebugInfo] Fix crash when emitting an invalidated SDDbgValue

[Orlando Cazalet-Hyams via Phabricator via llvm-commits]

Orlando added a comment.

The change makes sense and LGTM.

Is the test relying on a debug info bug happening I wonder? If so, that would make the test slightly fragile (assuming that bugs will eventually get fixed). Looking at the generated MIR there seems to be a 1:1 mapping of IR to MIR instructions, which I think suggests that the SDDbgValue wasn't updated at some point.

I had a play with this and I've manged to further reduce the test. You can replace the function body with this:

  define dso_local i32 @intel_pmu_enable_bts() local_unnamed_addr !dbg !16 {
  entry:
    %0 = extractvalue { i32, i64 } zeroinitializer, 1
    %1 = load i32, i32* @intel_pmu_enable_bts_config, align 4
    call void @llvm.dbg.value(metadata !DIArgList(i64 %0, i32 %1), metadata !20, metadata !DIExpression(DW_OP_LLVM_arg, 0, DW_OP_LLVM_arg, 1, DW_OP_or, DW_OP_stack_value)), !dbg !23
    ret i32 %1
  }

I guess the test case is may not relying on a "bug" - it feels similar to the situation in IR when an instruction is deleted, the pass doesn't call `salvageDebugInfo`, and `Instruction`s dtor defensively sets metadata uses to undef. The debug info isn't incorrect, it's just worse than it needs to be. I would be happier if we could generate a test for this which doesn't rely on this kind of thing, because if the underlying issue is fixed the test might become hard to regenerate, but maybe this code path is only ever hit that way? I'm not sure.



================
Comment at: llvm/test/DebugInfo/Generic/invalidated-dbg-value-is-undef.ll:12
+
+; CHECK-NOT: DBG_VALUE
+
----------------
You could add `--implicit-check-not=DBG_VALUE` to FileCheck and remove the `CHECK-NOT`s? No strong opinion on this though.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D101540/new/

https://reviews.llvm.org/D101540