[llvm] 7dc9d73 - [DebugInfo][InstrRef] Handle llvm.frameaddress intrinsics gracefully

[Jeremy Morse via llvm-commits]

Author: Jeremy Morse
Date: 2021-07-27T13:44:37+01:00
New Revision: 7dc9d7373186827a92d6ca08ad7192208dfea389

URL: https://github.com/llvm/llvm-project/commit/7dc9d7373186827a92d6ca08ad7192208dfea389
DIFF: https://github.com/llvm/llvm-project/commit/7dc9d7373186827a92d6ca08ad7192208dfea389.diff

LOG: [DebugInfo][InstrRef] Handle llvm.frameaddress intrinsics gracefully

When working out which instruction defines a value, the
instruction-referencing variable location code has a few special cases for
physical registers:
 * Arguments are never defined by instructions,
 * Constant physical registers always read the same value, are never def'd

This patch adds a third case for the llvm.frameaddress intrinsics: you can
read the framepointer in any block if you so choose, and use it as a
variable location, as shown in the added test.

This rather violates one of the assumptions behind instruction referencing,
that LLVM-ir shouldn't be able to read from an arbitrary register at some
arbitrary point in the program. The solution for now is to just emit a
DBG_PHI that reads the register value: this works, but if we wanted to do
something clever with DBG_PHIs in the future then this would probably get
in the way. As it stands, this patch avoids a crash.

Differential Revision: https://reviews.llvm.org/D106659

Added: 
    llvm/test/DebugInfo/X86/instr-ref-framereg-read.ll

Modified: 
    llvm/lib/CodeGen/MachineFunction.cpp

Removed: 
    


################################################################################
diff  --git a/llvm/lib/CodeGen/MachineFunction.cpp b/llvm/lib/CodeGen/MachineFunction.cpp
index dc4b1a6de570f..0a454b68aca34 100644
--- a/llvm/lib/CodeGen/MachineFunction.cpp
+++ b/llvm/lib/CodeGen/MachineFunction.cpp
@@ -1139,6 +1139,14 @@ auto MachineFunction::salvageCopySSA(MachineInstr &MI)
     // We can produce a DBG_PHI that identifies the constant physreg. Doesn't
     // matter where we put it, as it's constant valued.
     assert(CurInst->isCopy());
+  } else if (State.first == TRI.getFrameRegister(*this)) {
+    // LLVM IR is allowed to read the framepointer by calling a
+    // llvm.frameaddress.* intrinsic. We can support this by emitting a
+    // DBG_PHI $fp. This isn't ideal, because it extends the behaviours /
+    // position that DBG_PHIs appear at, limiting what can be done later.
+    // TODO: see if there's a better way of expressing these variable
+    // locations.
+    ;
   } else {
     // Assert that this is the entry block. If it isn't, then there is some
     // code construct we don't recognise that deals with physregs across

diff  --git a/llvm/test/DebugInfo/X86/instr-ref-framereg-read.ll b/llvm/test/DebugInfo/X86/instr-ref-framereg-read.ll
new file mode 100644
index 0000000000000..7ed037d7b652e
--- /dev/null
+++ b/llvm/test/DebugInfo/X86/instr-ref-framereg-read.ll
@@ -0,0 +1,42 @@
+; RUN: llc %s -o - -start-after=codegenprepare -stop-before=finalize-isel \
+; RUN:     -mtriple=x86_64-unknown-unknown -experimental-debug-variable-locations \
+; RUN: | FileCheck %s
+
+; Test for a crash / weird behaviour when llvm.frameaddress.* is called.
+; Today, as a concession, we emit a DBG_PHI allowing the frame register value
+; to be read, but it could be expressed in other ways. Check that this works
+; outside of the entry block, to avoid the frame register being recognised
+; as def'd by frame-setup code or as a function argument.
+
+; CHECK-LABEL: bb.1.notentry:
+; CHECK: DBG_PHI $rbp
+
+declare void @llvm.dbg.value(metadata, metadata, metadata)
+declare i8 *@llvm.frameaddress.p0i8(i32)
+
+ ; Function Attrs: mustprogress nofree nosync nounwind sspstrong uwtable
+define hidden i8 * @foo() !dbg !7 {
+entry:
+  br label  %notentry
+
+notentry:
+  %0 = tail call i8* @llvm.frameaddress.p0i8(i32 0), !dbg !12
+  call void @llvm.dbg.value(metadata i8* %0, metadata !11, metadata !DIExpression()), !dbg !12
+  ret i8 *%0
+}
+
+!llvm.dbg.cu = !{!0}
+!llvm.module.flags = !{!3, !4, !5, !6}
+!0 = distinct !DICompileUnit(language: DW_LANG_C_plus_plus, file: !1, producer: "", isOptimized: true, runtimeVersion: 0, emissionKind: FullDebug)
+!1 = !DIFile(filename: "foo.cpp", directory: ".")
+!2 = !DIBasicType(name: "int", size: 8, encoding: DW_ATE_signed)
+!3 = !{i32 2, !"Dwarf Version", i32 4}
+!4 = !{i32 2, !"Debug Info Version", i32 3}
+!5 = !{i32 1, !"wchar_size", i32 2}
+!6 = !{i32 7, !"PIC Level", i32 2}
+!7 = distinct !DISubprogram(name: "foo", linkageName: "foo", scope: !1, file: !1, line: 6, type: !8, scopeLine: 6, flags: DIFlagPrototyped, spFlags: DISPFlagDefinition | DISPFlagOptimized, unit: !0, retainedNodes: !10)
+!8 = !DISubroutineType(types: !9)
+!9 = !{!2, !2}
+!10 = !{!11}
+!11 = !DILocalVariable(name: "baz", scope: !7, file: !1, line: 7, type: !2)
+!12 = !DILocation(line: 10, scope: !7)