[clang] 568db78 - [CGDebugInfo] Emit subprograms for decls when AT_tail_call is understood (reland with fixes)

Vedant Kumar via cfe-commits cfe-commits at lists.llvm.org
Tue Nov 19 12:52:52 PST 2019


Author: Vedant Kumar
Date: 2019-11-19T12:49:27-08:00
New Revision: 568db780bb7267651a902da8e85bc59fc89aea70

URL: https://github.com/llvm/llvm-project/commit/568db780bb7267651a902da8e85bc59fc89aea70
DIFF: https://github.com/llvm/llvm-project/commit/568db780bb7267651a902da8e85bc59fc89aea70.diff

LOG: [CGDebugInfo] Emit subprograms for decls when AT_tail_call is understood (reland with fixes)

Currently, clang emits subprograms for declared functions when the
target debugger or DWARF standard is known to support entry values
(DW_OP_entry_value & the GNU equivalent).

Treat DW_AT_tail_call the same way to allow debuggers to follow cross-TU
tail calls.

Pre-patch debug session with a cross-TU tail call:

```
  * frame #0: 0x0000000100000fa4 main`target at b.c:4:3 [opt]
    frame #1: 0x0000000100000f99 main`main at a.c:8:10 [opt]
```

Post-patch (note that the tail-calling frame, "helper", is visible):

```
  * frame #0: 0x0000000100000fa4 main`target at b.c:4:3 [opt]
    frame #1: 0x0000000100000f80 main`helper [opt] [artificial]
    frame #2: 0x0000000100000f99 main`main at a.c:8:10 [opt]
```

This was reverted in 5b9a072c because it attached declaration
subprograms to inlinable builtin calls, which interacted badly with the
MergeICmps pass. The fix is to not attach declarations to builtins.

rdar://46577651

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

Added: 
    

Modified: 
    clang/include/clang/Basic/IdentifierTable.h
    clang/lib/CodeGen/CGDebugInfo.cpp
    clang/lib/Sema/SemaCodeComplete.cpp
    clang/test/CodeGen/debug-info-extern-call.c
    clang/test/CodeGenCXX/dbg-info-all-calls-described.cpp
    llvm/test/DebugInfo/X86/dwarf-callsite-related-attrs.ll

Removed: 
    


################################################################################
diff  --git a/clang/include/clang/Basic/IdentifierTable.h b/clang/include/clang/Basic/IdentifierTable.h
index 7aa1d074a591..ea5d7adeb2da 100644
--- a/clang/include/clang/Basic/IdentifierTable.h
+++ b/clang/include/clang/Basic/IdentifierTable.h
@@ -384,6 +384,17 @@ class alignas(IdentifierInfoAlignment) IdentifierInfo {
     return getName().startswith("<#") && getName().endswith("#>");
   }
 
+  /// Determine whether \p this is a name reserved for the implementation (C99
+  /// 7.1.3, C++ [lib.global.names]).
+  bool isReservedName(bool doubleUnderscoreOnly = false) const {
+    if (getLength() < 2)
+      return false;
+    const char *Name = getNameStart();
+    return Name[0] == '_' &&
+           (Name[1] == '_' ||
+            (Name[1] >= 'A' && Name[1] <= 'Z' && !doubleUnderscoreOnly));
+  }
+
   /// Provide less than operator for lexicographical sorting.
   bool operator<(const IdentifierInfo &RHS) const {
     return getName() < RHS.getName();

diff  --git a/clang/lib/CodeGen/CGDebugInfo.cpp b/clang/lib/CodeGen/CGDebugInfo.cpp
index 75c4b2ae2339..116517a9cb99 100644
--- a/clang/lib/CodeGen/CGDebugInfo.cpp
+++ b/clang/lib/CodeGen/CGDebugInfo.cpp
@@ -3765,21 +3765,29 @@ void CGDebugInfo::EmitFunctionDecl(GlobalDecl GD, SourceLocation Loc,
 void CGDebugInfo::EmitFuncDeclForCallSite(llvm::CallBase *CallOrInvoke,
                                           QualType CalleeType,
                                           const FunctionDecl *CalleeDecl) {
-  auto &CGOpts = CGM.getCodeGenOpts();
-  if (!CGOpts.EnableDebugEntryValues || !CGM.getLangOpts().Optimize ||
-      !CallOrInvoke)
+  if (!CallOrInvoke)
     return;
-
   auto *Func = CallOrInvoke->getCalledFunction();
   if (!Func)
     return;
+  if (Func->getSubprogram())
+    return;
+
+  // Do not emit a declaration subprogram for a builtin or if call site info
+  // isn't required. Also, elide declarations for functions with reserved names,
+  // as call site-related features aren't interesting in this case (& also, the
+  // compiler may emit calls to these functions without debug locations, which
+  // makes the verifier complain).
+  if (CalleeDecl->getBuiltinID() != 0 ||
+      getCallSiteRelatedAttrs() == llvm::DINode::FlagZero)
+    return;
+  if (const auto *Id = CalleeDecl->getIdentifier())
+    if (Id->isReservedName())
+      return;
 
   // If there is no DISubprogram attached to the function being called,
   // create the one describing the function in order to have complete
   // call site debug info.
-  if (Func->getSubprogram())
-    return;
-
   if (!CalleeDecl->isStatic() && !CalleeDecl->isInlined())
     EmitFunctionDecl(CalleeDecl, CalleeDecl->getLocation(), CalleeType, Func);
 }
@@ -4841,10 +4849,10 @@ llvm::DINode::DIFlags CGDebugInfo::getCallSiteRelatedAttrs() const {
   bool SupportsDWARFv4Ext =
       CGM.getCodeGenOpts().DwarfVersion == 4 &&
       (CGM.getCodeGenOpts().getDebuggerTuning() == llvm::DebuggerKind::LLDB ||
-       (CGM.getCodeGenOpts().EnableDebugEntryValues &&
-       CGM.getCodeGenOpts().getDebuggerTuning() == llvm::DebuggerKind::GDB));
+       CGM.getCodeGenOpts().getDebuggerTuning() == llvm::DebuggerKind::GDB);
 
-  if (!SupportsDWARFv4Ext && CGM.getCodeGenOpts().DwarfVersion < 5)
+  if (!SupportsDWARFv4Ext && CGM.getCodeGenOpts().DwarfVersion < 5 &&
+      !CGM.getCodeGenOpts().EnableDebugEntryValues)
     return llvm::DINode::FlagZero;
 
   return llvm::DINode::FlagAllCallsDescribed;

diff  --git a/clang/lib/Sema/SemaCodeComplete.cpp b/clang/lib/Sema/SemaCodeComplete.cpp
index ade6e46d1bca..81eed4330cdc 100644
--- a/clang/lib/Sema/SemaCodeComplete.cpp
+++ b/clang/lib/Sema/SemaCodeComplete.cpp
@@ -692,18 +692,6 @@ getRequiredQualification(ASTContext &Context, const DeclContext *CurContext,
   return Result;
 }
 
-/// Determine whether \p Id is a name reserved for the implementation (C99
-/// 7.1.3, C++ [lib.global.names]).
-static bool isReservedName(const IdentifierInfo *Id,
-                           bool doubleUnderscoreOnly = false) {
-  if (Id->getLength() < 2)
-    return false;
-  const char *Name = Id->getNameStart();
-  return Name[0] == '_' &&
-         (Name[1] == '_' ||
-          (Name[1] >= 'A' && Name[1] <= 'Z' && !doubleUnderscoreOnly));
-}
-
 // Some declarations have reserved names that we don't want to ever show.
 // Filter out names reserved for the implementation if they come from a
 // system header.
@@ -713,13 +701,13 @@ static bool shouldIgnoreDueToReservedName(const NamedDecl *ND, Sema &SemaRef) {
     return false;
 
   // Ignore reserved names for compiler provided decls.
-  if (isReservedName(Id) && ND->getLocation().isInvalid())
+  if (Id->isReservedName() && ND->getLocation().isInvalid())
     return true;
 
   // For system headers ignore only double-underscore names.
   // This allows for system headers providing private symbols with a single
   // underscore.
-  if (isReservedName(Id, /*doubleUnderscoreOnly=*/true) &&
+  if (Id->isReservedName(/*doubleUnderscoreOnly=*/true) &&
       SemaRef.SourceMgr.isInSystemHeader(
           SemaRef.SourceMgr.getSpellingLoc(ND->getLocation())))
     return true;

diff  --git a/clang/test/CodeGen/debug-info-extern-call.c b/clang/test/CodeGen/debug-info-extern-call.c
index e35669b78f93..da3764f7359e 100644
--- a/clang/test/CodeGen/debug-info-extern-call.c
+++ b/clang/test/CodeGen/debug-info-extern-call.c
@@ -1,15 +1,39 @@
-// RUN: %clang -Xclang -femit-debug-entry-values -g -O2 -target x86_64-none-linux-gnu -S -emit-llvm %s -o - | FileCheck %s -check-prefix=CHECK-EXT
-// CHECK-EXT: !DISubprogram(name: "fn1"
+// When entry values are emitted, expect a subprogram for extern decls so that
+// the dwarf generator can describe call site parameters at extern call sites.
+//
+// RUN: %clang -Xclang -femit-debug-entry-values -g -O2 -target x86_64-none-linux-gnu -S -emit-llvm %s -o - \
+// RUN:   | FileCheck %s -check-prefix=DECLS-FOR-EXTERN
 
-// RUN: %clang -g -O2 -target x86_64-none-linux-gnu -S -emit-llvm %s -o - | FileCheck %s
-// CHECK-NOT: !DISubprogram(name: "fn1"
+// Similarly, when the debugger tuning is gdb, expect a subprogram for extern
+// decls so that the dwarf generator can describe information needed for tail
+// call frame reconstrution.
+//
+// RUN: %clang -g -O2 -target x86_64-none-linux-gnu -ggdb -S -emit-llvm %s -o - \
+// RUN:   | FileCheck %s -check-prefix=DECLS-FOR-EXTERN
+//
+// Do not emit a subprogram for extern decls when entry values are disabled and
+// the tuning is not set to gdb.
+//
+// RUN: %clang -g -O2 -target x86_64-none-linux-gnu -gsce -S -emit-llvm %s -o - \
+// RUN:   | FileCheck %s -check-prefix=NO-DECLS-FOR-EXTERN
+
+// DECLS-FOR-EXTERN: !DISubprogram(name: "fn1"
+// DECLS-FOR-EXTERN-NOT: !DISubprogram(name: "memcmp"
+// DECLS-FOR-EXTERN-NOT: !DISubprogram(name: "__some_reserved_name"
+
+// NO-DECLS-FOR-EXTERN-NOT: !DISubprogram(name: "fn1"
+// NO-DECLS-FOR-EXTERN-NOT: !DISubprogram(name: "memcmp"
+// NO-DECLS-FOR-EXTERN-NOT: !DISubprogram(name: "__some_reserved_name"
 
 extern int fn1(int a, int b);
+extern int memcmp(const void *s1, const void *s2, unsigned long n);
+extern void __some_reserved_name(void);
 
-int fn2 () {
+int fn2 (int *src, int *dst) {
   int x = 4, y = 5;
   int res = fn1(x, y);
-
-  return res;
+  int res2 = memcmp(dst, src, res);
+  __some_reserved_name();
+  return res + res2;
 }
 

diff  --git a/clang/test/CodeGenCXX/dbg-info-all-calls-described.cpp b/clang/test/CodeGenCXX/dbg-info-all-calls-described.cpp
index 1cb2b6c609f3..667c2469b55e 100644
--- a/clang/test/CodeGenCXX/dbg-info-all-calls-described.cpp
+++ b/clang/test/CodeGenCXX/dbg-info-all-calls-described.cpp
@@ -56,6 +56,7 @@
 
 // NO-ATTR-NOT: FlagAllCallsDescribed
 
+// HAS-ATTR-DAG: DISubprogram(name: "declaration1", {{.*}}, flags: DIFlagPrototyped
 // HAS-ATTR-DAG: DISubprogram(name: "declaration2", {{.*}}, flags: DIFlagPrototyped | DIFlagAllCallsDescribed, spFlags: DISPFlagDefinition
 // HAS-ATTR-DAG: DISubprogram(name: "struct1", {{.*}}, flags: DIFlagPrototyped, spFlags: DISPFlagOptimized)
 // HAS-ATTR-DAG: DISubprogram(name: "struct1", {{.*}}, flags: DIFlagPrototyped | DIFlagAllCallsDescribed, spFlags: DISPFlagDefinition

diff  --git a/llvm/test/DebugInfo/X86/dwarf-callsite-related-attrs.ll b/llvm/test/DebugInfo/X86/dwarf-callsite-related-attrs.ll
index c37ce1eb7fbe..33e06faba57b 100644
--- a/llvm/test/DebugInfo/X86/dwarf-callsite-related-attrs.ll
+++ b/llvm/test/DebugInfo/X86/dwarf-callsite-related-attrs.ll
@@ -25,6 +25,14 @@
 
 @sink = global i32 0, align 4, !dbg !0
 
+define void @__has_no_subprogram() {
+entry:
+  %0 = load volatile i32, i32* @sink, align 4
+  %inc = add nsw i32 %0, 1
+  store volatile i32 %inc, i32* @sink, align 4
+  ret void
+}
+
 ; ASM: DW_TAG_subprogram
 ; ASM:   DW_AT_call_all_calls
 ; OBJ: [[bat_sp:.*]]: DW_TAG_subprogram
@@ -70,6 +78,7 @@ entry:
 ; OBJ:     DW_AT_call_tail_call
 define void @_Z3foov() !dbg !25 {
 entry:
+  tail call void @__has_no_subprogram()
   tail call void @_Z3barv(), !dbg !26
   tail call void @_Z3batv(), !dbg !27
   tail call void @_Z3barv(), !dbg !26


        


More information about the cfe-commits mailing list