Skip to content

Commit 568db78

Browse files
committed
[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 5b9a072 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
1 parent 75b5db3 commit 568db78

File tree

6 files changed

+72
-31
lines changed

6 files changed

+72
-31
lines changed

Diff for: clang/include/clang/Basic/IdentifierTable.h

+11
Original file line numberDiff line numberDiff line change
@@ -384,6 +384,17 @@ class alignas(IdentifierInfoAlignment) IdentifierInfo {
384384
return getName().startswith("<#") && getName().endswith("#>");
385385
}
386386

387+
/// Determine whether \p this is a name reserved for the implementation (C99
388+
/// 7.1.3, C++ [lib.global.names]).
389+
bool isReservedName(bool doubleUnderscoreOnly = false) const {
390+
if (getLength() < 2)
391+
return false;
392+
const char *Name = getNameStart();
393+
return Name[0] == '_' &&
394+
(Name[1] == '_' ||
395+
(Name[1] >= 'A' && Name[1] <= 'Z' && !doubleUnderscoreOnly));
396+
}
397+
387398
/// Provide less than operator for lexicographical sorting.
388399
bool operator<(const IdentifierInfo &RHS) const {
389400
return getName() < RHS.getName();

Diff for: clang/lib/CodeGen/CGDebugInfo.cpp

+18-10
Original file line numberDiff line numberDiff line change
@@ -3765,21 +3765,29 @@ void CGDebugInfo::EmitFunctionDecl(GlobalDecl GD, SourceLocation Loc,
37653765
void CGDebugInfo::EmitFuncDeclForCallSite(llvm::CallBase *CallOrInvoke,
37663766
QualType CalleeType,
37673767
const FunctionDecl *CalleeDecl) {
3768-
auto &CGOpts = CGM.getCodeGenOpts();
3769-
if (!CGOpts.EnableDebugEntryValues || !CGM.getLangOpts().Optimize ||
3770-
!CallOrInvoke)
3768+
if (!CallOrInvoke)
37713769
return;
3772-
37733770
auto *Func = CallOrInvoke->getCalledFunction();
37743771
if (!Func)
37753772
return;
3773+
if (Func->getSubprogram())
3774+
return;
3775+
3776+
// Do not emit a declaration subprogram for a builtin or if call site info
3777+
// isn't required. Also, elide declarations for functions with reserved names,
3778+
// as call site-related features aren't interesting in this case (& also, the
3779+
// compiler may emit calls to these functions without debug locations, which
3780+
// makes the verifier complain).
3781+
if (CalleeDecl->getBuiltinID() != 0 ||
3782+
getCallSiteRelatedAttrs() == llvm::DINode::FlagZero)
3783+
return;
3784+
if (const auto *Id = CalleeDecl->getIdentifier())
3785+
if (Id->isReservedName())
3786+
return;
37763787

37773788
// If there is no DISubprogram attached to the function being called,
37783789
// create the one describing the function in order to have complete
37793790
// call site debug info.
3780-
if (Func->getSubprogram())
3781-
return;
3782-
37833791
if (!CalleeDecl->isStatic() && !CalleeDecl->isInlined())
37843792
EmitFunctionDecl(CalleeDecl, CalleeDecl->getLocation(), CalleeType, Func);
37853793
}
@@ -4841,10 +4849,10 @@ llvm::DINode::DIFlags CGDebugInfo::getCallSiteRelatedAttrs() const {
48414849
bool SupportsDWARFv4Ext =
48424850
CGM.getCodeGenOpts().DwarfVersion == 4 &&
48434851
(CGM.getCodeGenOpts().getDebuggerTuning() == llvm::DebuggerKind::LLDB ||
4844-
(CGM.getCodeGenOpts().EnableDebugEntryValues &&
4845-
CGM.getCodeGenOpts().getDebuggerTuning() == llvm::DebuggerKind::GDB));
4852+
CGM.getCodeGenOpts().getDebuggerTuning() == llvm::DebuggerKind::GDB);
48464853

4847-
if (!SupportsDWARFv4Ext && CGM.getCodeGenOpts().DwarfVersion < 5)
4854+
if (!SupportsDWARFv4Ext && CGM.getCodeGenOpts().DwarfVersion < 5 &&
4855+
!CGM.getCodeGenOpts().EnableDebugEntryValues)
48484856
return llvm::DINode::FlagZero;
48494857

48504858
return llvm::DINode::FlagAllCallsDescribed;

Diff for: clang/lib/Sema/SemaCodeComplete.cpp

+2-14
Original file line numberDiff line numberDiff line change
@@ -692,18 +692,6 @@ getRequiredQualification(ASTContext &Context, const DeclContext *CurContext,
692692
return Result;
693693
}
694694

695-
/// Determine whether \p Id is a name reserved for the implementation (C99
696-
/// 7.1.3, C++ [lib.global.names]).
697-
static bool isReservedName(const IdentifierInfo *Id,
698-
bool doubleUnderscoreOnly = false) {
699-
if (Id->getLength() < 2)
700-
return false;
701-
const char *Name = Id->getNameStart();
702-
return Name[0] == '_' &&
703-
(Name[1] == '_' ||
704-
(Name[1] >= 'A' && Name[1] <= 'Z' && !doubleUnderscoreOnly));
705-
}
706-
707695
// Some declarations have reserved names that we don't want to ever show.
708696
// Filter out names reserved for the implementation if they come from a
709697
// system header.
@@ -713,13 +701,13 @@ static bool shouldIgnoreDueToReservedName(const NamedDecl *ND, Sema &SemaRef) {
713701
return false;
714702

715703
// Ignore reserved names for compiler provided decls.
716-
if (isReservedName(Id) && ND->getLocation().isInvalid())
704+
if (Id->isReservedName() && ND->getLocation().isInvalid())
717705
return true;
718706

719707
// For system headers ignore only double-underscore names.
720708
// This allows for system headers providing private symbols with a single
721709
// underscore.
722-
if (isReservedName(Id, /*doubleUnderscoreOnly=*/true) &&
710+
if (Id->isReservedName(/*doubleUnderscoreOnly=*/true) &&
723711
SemaRef.SourceMgr.isInSystemHeader(
724712
SemaRef.SourceMgr.getSpellingLoc(ND->getLocation())))
725713
return true;

Diff for: clang/test/CodeGen/debug-info-extern-call.c

+31-7
Original file line numberDiff line numberDiff line change
@@ -1,15 +1,39 @@
1-
// 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
2-
// CHECK-EXT: !DISubprogram(name: "fn1"
1+
// When entry values are emitted, expect a subprogram for extern decls so that
2+
// the dwarf generator can describe call site parameters at extern call sites.
3+
//
4+
// RUN: %clang -Xclang -femit-debug-entry-values -g -O2 -target x86_64-none-linux-gnu -S -emit-llvm %s -o - \
5+
// RUN: | FileCheck %s -check-prefix=DECLS-FOR-EXTERN
36

4-
// RUN: %clang -g -O2 -target x86_64-none-linux-gnu -S -emit-llvm %s -o - | FileCheck %s
5-
// CHECK-NOT: !DISubprogram(name: "fn1"
7+
// Similarly, when the debugger tuning is gdb, expect a subprogram for extern
8+
// decls so that the dwarf generator can describe information needed for tail
9+
// call frame reconstrution.
10+
//
11+
// RUN: %clang -g -O2 -target x86_64-none-linux-gnu -ggdb -S -emit-llvm %s -o - \
12+
// RUN: | FileCheck %s -check-prefix=DECLS-FOR-EXTERN
13+
//
14+
// Do not emit a subprogram for extern decls when entry values are disabled and
15+
// the tuning is not set to gdb.
16+
//
17+
// RUN: %clang -g -O2 -target x86_64-none-linux-gnu -gsce -S -emit-llvm %s -o - \
18+
// RUN: | FileCheck %s -check-prefix=NO-DECLS-FOR-EXTERN
19+
20+
// DECLS-FOR-EXTERN: !DISubprogram(name: "fn1"
21+
// DECLS-FOR-EXTERN-NOT: !DISubprogram(name: "memcmp"
22+
// DECLS-FOR-EXTERN-NOT: !DISubprogram(name: "__some_reserved_name"
23+
24+
// NO-DECLS-FOR-EXTERN-NOT: !DISubprogram(name: "fn1"
25+
// NO-DECLS-FOR-EXTERN-NOT: !DISubprogram(name: "memcmp"
26+
// NO-DECLS-FOR-EXTERN-NOT: !DISubprogram(name: "__some_reserved_name"
627

728
extern int fn1(int a, int b);
29+
extern int memcmp(const void *s1, const void *s2, unsigned long n);
30+
extern void __some_reserved_name(void);
831

9-
int fn2 () {
32+
int fn2 (int *src, int *dst) {
1033
int x = 4, y = 5;
1134
int res = fn1(x, y);
12-
13-
return res;
35+
int res2 = memcmp(dst, src, res);
36+
__some_reserved_name();
37+
return res + res2;
1438
}
1539

Diff for: clang/test/CodeGenCXX/dbg-info-all-calls-described.cpp

+1
Original file line numberDiff line numberDiff line change
@@ -56,6 +56,7 @@
5656

5757
// NO-ATTR-NOT: FlagAllCallsDescribed
5858

59+
// HAS-ATTR-DAG: DISubprogram(name: "declaration1", {{.*}}, flags: DIFlagPrototyped
5960
// HAS-ATTR-DAG: DISubprogram(name: "declaration2", {{.*}}, flags: DIFlagPrototyped | DIFlagAllCallsDescribed, spFlags: DISPFlagDefinition
6061
// HAS-ATTR-DAG: DISubprogram(name: "struct1", {{.*}}, flags: DIFlagPrototyped, spFlags: DISPFlagOptimized)
6162
// HAS-ATTR-DAG: DISubprogram(name: "struct1", {{.*}}, flags: DIFlagPrototyped | DIFlagAllCallsDescribed, spFlags: DISPFlagDefinition

Diff for: llvm/test/DebugInfo/X86/dwarf-callsite-related-attrs.ll

+9
Original file line numberDiff line numberDiff line change
@@ -25,6 +25,14 @@
2525

2626
@sink = global i32 0, align 4, !dbg !0
2727

28+
define void @__has_no_subprogram() {
29+
entry:
30+
%0 = load volatile i32, i32* @sink, align 4
31+
%inc = add nsw i32 %0, 1
32+
store volatile i32 %inc, i32* @sink, align 4
33+
ret void
34+
}
35+
2836
; ASM: DW_TAG_subprogram
2937
; ASM: DW_AT_call_all_calls
3038
; OBJ: [[bat_sp:.*]]: DW_TAG_subprogram
@@ -70,6 +78,7 @@ entry:
7078
; OBJ: DW_AT_call_tail_call
7179
define void @_Z3foov() !dbg !25 {
7280
entry:
81+
tail call void @__has_no_subprogram()
7382
tail call void @_Z3barv(), !dbg !26
7483
tail call void @_Z3batv(), !dbg !27
7584
tail call void @_Z3barv(), !dbg !26

0 commit comments

Comments
 (0)