[llvm] [HWASAN] NFC? Remove DW_OP_LLVM_tag_offset from DIExpression::isImplicit (PR #79816)

[Orlando Cazalet-Hyams via llvm-commits]

OCHyams wrote:

Thanks for the review. I'm replying in the main comments so that GitHub doesn't swallow my reply:

> Would this make all expressions with DW_OP_LLVM_tag_offset non implicit?
> I don't know implications of this.

Yeah it would. I don't think the presence of a DW_OP_LLVM_tag_offset should qualify an expression as implicit (basically I am thinking the current code is a "bug").

As for the implications, looking at _current behaviour_ at the callsites of `isImplicit` (there aren't many):

In `combineDIExpressions` in DwarfDebug.cpp ([code](https://github.com/llvm/llvm-project/blob/0217d2e089afba8ca33330713787521ba52a1056/llvm/lib/CodeGen/AsmPrinter/DwarfDebug.cpp#L588)). The function combines expressions; if isImplicit is true for both expressions it avoids applying a DW_OP_stack_value. Here you can see that an expression containing a DW_OP_LLVM_tag_offset  but no DW_OP_stack_value could create unexpected behaviour.

In `MLocTracker::emitLoc` in InstrRefBasedImpl.cpp ([code](https://github.com/llvm/llvm-project/blob/0217d2e089afba8ca33330713787521ba52a1056/llvm/lib/CodeGen/LiveDebugValues/InstrRefBasedImpl.cpp#L1272)). There's an assert checking a DBG_VALUE is not both indirect (describing a memory location) and has an expression that `isImplicit`. Here having a DW_OP_LLVM_tag_offset but no DW_OP_stack_value would trigger the assertion, which seems like unexpected behaviour again.

In `PEI::replaceFrameIndexDebugInstr` in PrologEpilogInserter.cpp ([code](https://github.com/llvm/llvm-project/blob/5a07774fe11b560652b15776ff6477ba17b6cae0/llvm/lib/CodeGen/PrologEpilogInserter.cpp#L1430)), if the DBG_VALUE is indirect (describes a memory location) and `isImplicit`, a DW_OP_deref_size is prepended.

Finally, the assert in `DIExpression::createFragmentExpression` in DebugInfoMetadata.cpp ([code](https://github.com/llvm/llvm-project/blob/3f5fcb59ae582ebfbe3f23050d90f86a2cb76eb0/llvm/lib/IR/DebugInfoMetadata.cpp#L1968)), which is the one that tripped after applying #78606. This assert is confirming the DIExpression is suitable for splitting into fragments. It's not suitable to split some expressions  that compute a value (using DW_OP_stack_value). To quote a comment there "We can't safely split arithmetic or shift operations into multiple fragments because we can't express carry-over between fragments.". It's completely fine to split a memory location into fragments. A DIExpression in a dbg.declare that contains a DW_OP_LLVM_tag_offset and an offset, e.g. DW_OP_plus_uconst, would currently trip this assertion, because currently that would mean the expression `isImplicit`.

I think in all these cases it is a mistake to treat an expression containing only a `DW_OP_LLVM_tag_offset` opcode as "implicit (which this patch fixes).

IMO the implication of the function name `isImplicit` in the context of DWARF, its doc-comment, and these call sites back up this claim. But I would definitely appreciate someone checking my working here. 

https://github.com/llvm/llvm-project/pull/79816