vsk updated this revision to Diff 229215.
vsk added a comment.
Don't emit declaration subprograms for functions with reserved names. There may
not be much benefit from referencing these functions from call site tags (e.g.
instead of relying on call site info to work out the arguments passed to
__asan_memcpy, the asan runtime could log those arguments instead).
This fixes a verifier failure:
inlinable function call in a function with debug info must have a !dbg
location
%134 = call i8* @__asan_memcpy(i8* %agg.tmp2.sroa.0.0..sroa_cast10.i, i8*
%agg.tmp.sroa.0.i.0..sroa_cast, i64 32)
@aprantl / @djtodoro, mind taking another look?
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D69970/new/
https://reviews.llvm.org/D69970
Files:
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
Index: llvm/test/DebugInfo/X86/dwarf-callsite-related-attrs.ll
===================================================================
--- llvm/test/DebugInfo/X86/dwarf-callsite-related-attrs.ll
+++ 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 @@
; 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
Index: clang/test/CodeGenCXX/dbg-info-all-calls-described.cpp
===================================================================
--- clang/test/CodeGenCXX/dbg-info-all-calls-described.cpp
+++ 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
Index: clang/test/CodeGen/debug-info-extern-call.c
===================================================================
--- clang/test/CodeGen/debug-info-extern-call.c
+++ 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;
}
Index: clang/lib/Sema/SemaCodeComplete.cpp
===================================================================
--- clang/lib/Sema/SemaCodeComplete.cpp
+++ clang/lib/Sema/SemaCodeComplete.cpp
@@ -692,18 +692,6 @@
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 @@
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;
Index: clang/lib/CodeGen/CGDebugInfo.cpp
===================================================================
--- clang/lib/CodeGen/CGDebugInfo.cpp
+++ clang/lib/CodeGen/CGDebugInfo.cpp
@@ -3748,21 +3748,29 @@
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);
}
@@ -4824,10 +4832,10 @@
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;
Index: clang/include/clang/Basic/IdentifierTable.h
===================================================================
--- clang/include/clang/Basic/IdentifierTable.h
+++ clang/include/clang/Basic/IdentifierTable.h
@@ -384,6 +384,17 @@
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();
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
