ygao added you to the CC list for the revision "Fixing a bug where debug info
for a local variable gets emitted at file scope".
Hi,
When compiling the following test file with "-g -O0" and then debug with gdb:
```
/* test.cpp */
const int d = 100;
extern int foo();
int main()
{
const int d = 4;
const float e = 4;
const char* f = "Woopy";
return d + foo(); // the value of d inside main() is 4.
}
int foo()
{
return d; // the value of d inside foo() is 100.
}
/* end of test.cpp */
```
$ clang -g -O0 test.cpp -o a.out
$ gdb a.out
(gdb) b foo
(gdb) r
(gdb) print d
/* the expected output is 100; but gdb prints 4 instead */
The current behavior is that Clang will try to defer generation of global
variable definitions until its first use. At that point, tryEmitAsConstant()
in CGExpr.cpp calls EmitValueDeclDbgValue(), which then calls a version of
EmitGlobalVariable (to be discussed in more details below) to emit debug info
for the deferred global variable or enum constant. There are two problems in
the current implementation:
1. When tryEmitAsConstant() generates debug info for the deferred global
variables, it does not check that they are indeed global variables. As a
result, some of the local variables have their debug info emitted again.
A fix to this problem is straight-forward; we just need to add a check for
global variables.
2. The version of EmitGlobalVariable() for deferred global variables always
prints out their debug info at file scope (referred to as "the third version"
in the paragraph below). One unpleasant result is that a namespace-scoped
variable will have its debug info emitted without its enclosing namespace info.
Another unpleasant result is that, combined with the first problem mentioned
above, a local variable can have its debug info emitted at file scope.
There are three versions of EmitGlobalVariable() in CGDebugInfo.cpp. The
first version (in the order they appear in CGDebugInfo.cpp) handles all C/C++
non-deferred global variables; the second version handles objective-c
interfaces; and the third version handles enumerator constants and deferred
global variables. I may refer to these functions as "the first version", "the
second version" or "the third version" in later descriptions.
To fix this second problem, the version of EmitGlobalVariable() for deferred
global variables should behave like the version for non-deferred variables
in terms of computing proper enclosing scopes and maybe attaching the proper
mangled names as well. I feel that if we want the two functions to behave
in similar ways, then we should try to use one function instead of having
duplicate codes in two separate functions. So my proposed fix is to pull out
the enumerator constant part from the third version of EmitGlobalVariable()
into its own function, and then merge the remaining bits into the first
version of EmitGlobalVariable().
I am leaving the objective-c version unchanged only because I am not familiar
with objective-c interface. There are a few subtle differences between the
objective-c version and the other two versions, and I am not confident to merge
all three versions together.
I try to give some line-by-line explanations about my changes:
==== CGDebugInfo.cpp: ====
```
Line#3114: a new function EmitEnumConstant() is created to handle the
enumerator constant part in the third version (see discussion above) of
EmitGlobalVariable().
Line#3043-3046: the first version of EmitGlobalVariable() is modified to
handle both deferred and non-deferred global variables. The deferred global
variables will have initializers so the parameter type is changed from
llvm::GlobalVariable to llvm::Value; the parameter name is also changed
to be more clear.
Line#3052: deferred global variables have internal linkage because global
variables with external linkage are not deferred.
```
==== CodeGenFunction.cpp ====
```
Line#1382-1403: check whether the declaration is VarDecl (global variable) or
EnumConstantDecl (enumerator constant), and call EmitGlobalVariable() or
EmitEnumConstant() accordingly. DeclRefExpr->getDecl() is passed in as
parameter instead of DeclRefExpr itself; the function is renamed from
EmitDeclRefExprDbgValue to EmitValueDeclDbgValue to match the change in
parameter type.
The type checking codes are moved here from CGExpr.cpp. There is one
difference on line#1394 where two new conditions are added:
if (VD->isFileVarDecl() && !getLangOpts().EmitAllDecls &&
The first condition "isFileVarDecl()" is to make sure that local variables
do not get their debug info emitted again. The function name is a little bit
of a misnomer because if you look at isFileContext() in DeclBase.h (which is
called by isFileVarDecl), you see that both file-scope and namespace-scope
variables are considered global variables.
-femit-all-decls is an option that disables global variables' deferring.
If this option is turned on, then do not need to emit debug info for global
variables when we see them being used, because their debug info is already
emitted at declaration time.
```
=== CGExpr.cpp ===
```
All the type-checking and dispatching codes are moved to CodeGenFunction.cpp,
so tryEmitAsConstant() only needs to call EmitValueDeclDbgValue().
```
Also, I propose adding the following two tests to the
debuginfo-tests repository.
http://llvm-reviews.chandlerc.com/D1280
Could someone review this for me? Both the fix and the tests
Many thanks,
- Gao.
http://llvm-reviews.chandlerc.com/D1281
Files:
lib/CodeGen/CodeGenFunction.cpp
lib/CodeGen/CGDebugInfo.cpp
lib/CodeGen/CodeGenFunction.h
lib/CodeGen/CGDebugInfo.h
lib/CodeGen/CGExpr.cpp
test/CodeGen/debug-info-scope.c
test/CodeGen/debug-info-line5.h
test/CodeGen/debug-info-line5.cpp
test/CodeGenCXX/debug-info-namespace.cpp
Index: lib/CodeGen/CodeGenFunction.cpp
===================================================================
--- lib/CodeGen/CodeGenFunction.cpp
+++ lib/CodeGen/CodeGenFunction.cpp
@@ -1379,12 +1379,27 @@
return EmitLValue(E).getAddress();
}
-void CodeGenFunction::EmitDeclRefExprDbgValue(const DeclRefExpr *E,
- llvm::Constant *Init) {
+void CodeGenFunction::EmitValueDeclDbgValue(const ValueDecl *Val,
+ llvm::Constant *Init) {
assert (Init && "Invalid DeclRefExpr initializer!");
- if (CGDebugInfo *Dbg = getDebugInfo())
- if (CGM.getCodeGenOpts().getDebugInfo() >= CodeGenOptions::LimitedDebugInfo)
- Dbg->EmitGlobalVariable(E->getDecl(), Init);
+ CGDebugInfo *Dbg = getDebugInfo();
+ if (!Dbg ||
+ CGM.getCodeGenOpts().getDebugInfo() < CodeGenOptions::LimitedDebugInfo)
+ return;
+
+ // Make sure we emit a debug reference to the global variable.
+ if (const VarDecl *VD = dyn_cast<VarDecl>(Val)) {
+ // Do not duplicate DIE entry for local variables; they are not deferred
+ // like global variables are.
+ if (VD->isFileVarDecl() && !getLangOpts().EmitAllDecls &&
+ !getContext().DeclMustBeEmitted(Val))
+ Dbg->EmitGlobalVariable(Init, VD);
+
+ // Make sure we emit a debug reference to an enumerator constant.
+ } else {
+ assert(isa<EnumConstantDecl>(Val));
+ Dbg->EmitEnumConstant(dyn_cast<EnumConstantDecl>(Val));
+ }
}
CodeGenFunction::PeepholeProtection
Index: lib/CodeGen/CGDebugInfo.cpp
===================================================================
--- lib/CodeGen/CGDebugInfo.cpp
+++ lib/CodeGen/CGDebugInfo.cpp
@@ -3040,14 +3040,22 @@
}
/// EmitGlobalVariable - Emit information about a global variable.
-void CGDebugInfo::EmitGlobalVariable(llvm::GlobalVariable *Var,
+/// \param VarOrInit either the global variable itself or the initializer
+/// \param D the global declaration
+void CGDebugInfo::EmitGlobalVariable(llvm::Value *VarOrInit,
const VarDecl *D) {
assert(DebugKind >= CodeGenOptions::LimitedDebugInfo);
// Create global variable debug descriptor.
llvm::DIFile Unit = getOrCreateFile(D->getLocation());
unsigned LineNo = getLineNumber(D->getLocation());
+ llvm::GlobalVariable *Var = dyn_cast_or_null<llvm::GlobalVariable>(VarOrInit);
+ bool IsLocalToUnit = Var ? Var->hasInternalLinkage() : true;
- setLocation(D->getLocation());
+ // For deferred global variables, the current source location is usually
+ // where they are being referenced. Do not change the current source location
+ // to the place where they are declared, lest we get a bogus line table.
+ if (Var)
+ setLocation(D->getLocation());
QualType T = D->getType();
if (T->isIncompleteArrayType()) {
@@ -3061,7 +3069,7 @@
}
StringRef DeclName = D->getName();
StringRef LinkageName;
- if (D->getDeclContext() && !isa<FunctionDecl>(D->getDeclContext())
+ if (Var && D->getDeclContext() && !isa<FunctionDecl>(D->getDeclContext())
&& !isa<ObjCMethodDecl>(D->getDeclContext()))
LinkageName = Var->getName();
if (LinkageName == DeclName)
@@ -3071,7 +3079,7 @@
llvm::DIGlobalVariable GV =
DBuilder.createStaticVariable(DContext, DeclName, LinkageName, Unit,
LineNo, getOrCreateType(T, Unit),
- Var->hasInternalLinkage(), Var,
+ IsLocalToUnit, VarOrInit,
getStaticDataMemberDeclaration(D));
DeclCache.insert(std::make_pair(D->getCanonicalDecl(), llvm::WeakVH(GV)));
}
@@ -3102,27 +3110,16 @@
Var->hasInternalLinkage(), Var);
}
-/// EmitGlobalVariable - Emit global variable's debug info.
-void CGDebugInfo::EmitGlobalVariable(const ValueDecl *VD,
- llvm::Constant *Init) {
+/// EmitEnumConstant - Emit debug info for an enumerator constant
+void CGDebugInfo::EmitEnumConstant(const EnumConstantDecl *ECD)
+{
assert(DebugKind >= CodeGenOptions::LimitedDebugInfo);
- // Create the descriptor for the variable.
- llvm::DIFile Unit = getOrCreateFile(VD->getLocation());
- StringRef Name = VD->getName();
- llvm::DIType Ty = getOrCreateType(VD->getType(), Unit);
- if (const EnumConstantDecl *ECD = dyn_cast<EnumConstantDecl>(VD)) {
- const EnumDecl *ED = cast<EnumDecl>(ECD->getDeclContext());
- assert(isa<EnumType>(ED->getTypeForDecl()) && "Enum without EnumType?");
- Ty = getOrCreateType(QualType(ED->getTypeForDecl(), 0), Unit);
- }
- // Do not use DIGlobalVariable for enums.
- if (Ty.getTag() == llvm::dwarf::DW_TAG_enumeration_type)
- return;
- llvm::DIGlobalVariable GV =
- DBuilder.createStaticVariable(Unit, Name, Name, Unit,
- getLineNumber(VD->getLocation()), Ty, true,
- Init, getStaticDataMemberDeclaration(VD));
- DeclCache.insert(std::make_pair(VD->getCanonicalDecl(), llvm::WeakVH(GV)));
+ llvm::DIFile Unit = getOrCreateFile(ECD->getLocation());
+ llvm::DIType Ty = getOrCreateType(ECD->getType(), Unit);
+
+ const EnumDecl *ED = cast<EnumDecl>(ECD->getDeclContext());
+ assert(isa<EnumType>(ED->getTypeForDecl()) && "Enum without EnumType?");
+ Ty = getOrCreateType(QualType(ED->getTypeForDecl(), 0), Unit);
}
llvm::DIScope CGDebugInfo::getCurrentContextDescriptor(const Decl *D) {
Index: lib/CodeGen/CodeGenFunction.h
===================================================================
--- lib/CodeGen/CodeGenFunction.h
+++ lib/CodeGen/CodeGenFunction.h
@@ -2052,8 +2052,9 @@
LValue EmitStmtExprLValue(const StmtExpr *E);
LValue EmitPointerToDataMemberBinaryExpr(const BinaryOperator *E);
LValue EmitObjCSelectorLValue(const ObjCSelectorExpr *E);
- void EmitDeclRefExprDbgValue(const DeclRefExpr *E, llvm::Constant *Init);
+ void EmitValueDeclDbgValue(const ValueDecl *Val, llvm::Constant *Init);
+
//===--------------------------------------------------------------------===//
// Scalar Expression Emission
//===--------------------------------------------------------------------===//
Index: lib/CodeGen/CGDebugInfo.h
===================================================================
--- lib/CodeGen/CGDebugInfo.h
+++ lib/CodeGen/CGDebugInfo.h
@@ -263,13 +263,13 @@
CGBuilderTy &Builder);
/// EmitGlobalVariable - Emit information about a global variable.
- void EmitGlobalVariable(llvm::GlobalVariable *GV, const VarDecl *Decl);
+ void EmitGlobalVariable(llvm::Value *VarOrInit, const VarDecl *Decl);
/// EmitGlobalVariable - Emit information about an objective-c interface.
void EmitGlobalVariable(llvm::GlobalVariable *GV, ObjCInterfaceDecl *Decl);
- /// EmitGlobalVariable - Emit global variable's debug info.
- void EmitGlobalVariable(const ValueDecl *VD, llvm::Constant *Init);
+ /// EmitEnumConstant - Emit information about an enumerator constant
+ void EmitEnumConstant(const EnumConstantDecl *ECD);
/// \brief - Emit C++ using directive.
void EmitUsingDirective(const UsingDirectiveDecl &UD);
Index: lib/CodeGen/CGExpr.cpp
===================================================================
--- lib/CodeGen/CGExpr.cpp
+++ lib/CodeGen/CGExpr.cpp
@@ -966,15 +966,9 @@
// Emit as a constant.
llvm::Constant *C = CGM.EmitConstantValue(result.Val, resultType, this);
- // Make sure we emit a debug reference to the global variable.
- // This should probably fire even for
- if (isa<VarDecl>(value)) {
- if (!getContext().DeclMustBeEmitted(cast<VarDecl>(value)))
- EmitDeclRefExprDbgValue(refExpr, C);
- } else {
- assert(isa<EnumConstantDecl>(value));
- EmitDeclRefExprDbgValue(refExpr, C);
- }
+ // Make sure we emit a debug reference to the global variable or
+ // enumerator constant.
+ EmitValueDeclDbgValue(value, C);
// If we emitted a reference constant, we need to dereference that.
if (resultIsReference)
Index: test/CodeGen/debug-info-scope.c
===================================================================
--- test/CodeGen/debug-info-scope.c
+++ test/CodeGen/debug-info-scope.c
@@ -1,7 +1,17 @@
// RUN: %clang_cc1 -g -emit-llvm < %s | FileCheck %s
+
+// Make sure that the debug info of the local variable d does not shadow
+// the global variable d in function foo()
+// CHECK: [ DW_TAG_variable ] [d] [line 6] [def]
+const int d = 100;
+extern int foo();
+
// Two variables with same name in separate scope.
// Radar 8330217.
int main() {
+// CHECK-NOT: [ DW_TAG_variable ] [d] [line 14]
+// CHECK: [ DW_TAG_auto_variable ] [d] [line 14]
+ const int d = 4;
int j = 0;
int k = 0;
// CHECK: DW_TAG_auto_variable ] [i]
@@ -12,5 +22,10 @@
// CHECK-NEXT: DW_TAG_lexical_block
for (int i = 0; i < 10; i++)
k++;
- return 0;
+ return d; // make a reference to d so that its debug info gets included
}
+
+int foo()
+{
+ return d;
+}
Index: test/CodeGen/debug-info-line5.h
===================================================================
--- test/CodeGen/debug-info-line5.h
+++ test/CodeGen/debug-info-line5.h
@@ -0,0 +1,2 @@
+// Declaration of "Marker"
+const unsigned Marker = 0xFF999999;
Index: test/CodeGen/debug-info-line5.cpp
===================================================================
--- test/CodeGen/debug-info-line5.cpp
+++ test/CodeGen/debug-info-line5.cpp
@@ -0,0 +1,22 @@
+// RUN: %clangxx -g -gcolumn-info -O0 %s -c -o %t.o
+// RUN: llvm-dwarfdump %t.o | FileCheck %s
+// Line table entries should reference this cpp file, not the header
+
+#include "debug-info-line5.h"
+
+int result;
+int foo(unsigned);
+
+int main()
+{
+ while ( 1 )
+ {
+ result = foo(Marker);
+ }
+ return result;
+}
+
+// CHECK: Address Line Column File ISA Flags
+// CHECK: 14 14 1 0 is_stmt prologue_end
+// CHECK: 15 3 1 0 is_stmt
+// CHECK: 15 3 1 0 is_stmt end_sequence
Index: test/CodeGenCXX/debug-info-namespace.cpp
===================================================================
--- test/CodeGenCXX/debug-info-namespace.cpp
+++ test/CodeGenCXX/debug-info-namespace.cpp
@@ -34,6 +34,16 @@
return i + X::B::i + Y::B::i;
}
+namespace monkey
+{
+ const int ape = 32;
+}
+
+int test()
+{
+ return monkey::ape;
+}
+
// This should work even if 'i' and 'func' were declarations & not definitions,
// but it doesn't yet.
@@ -46,6 +56,8 @@
// CHECK: [[FUNC:![0-9]*]] {{.*}} ; [ DW_TAG_subprogram ] [line 15] [def] [func]
// CHECK: [[FILE2]]} ; [ DW_TAG_file_type ] [{{.*}}foo.cpp]
// CHECK: [[I:![0-9]*]] = {{.*}}, metadata [[NS]], metadata !"i", {{.*}} ; [ DW_TAG_variable ] [i]
+// CHECK: [[VAR:![0-9]*]] = {{.*}}, metadata [[SPACE:![0-9]*]], metadata !"ape", {{.*}} ; [ DW_TAG_variable ] [ape]
+// CHECK: [[SPACE]] = {{.*}}, metadata !"monkey", {{.*}} ; [ DW_TAG_namespace ] [monkey]
// CHECK: [[MODULES]] = metadata !{metadata [[M1:![0-9]*]], metadata [[M2:![0-9]*]], metadata [[M3:![0-9]*]], metadata [[M4:![0-9]*]], metadata [[M5:![0-9]*]], metadata [[M6:![0-9]*]], metadata [[M7:![0-9]*]], metadata [[M8:![0-9]*]], metadata [[M9:![0-9]*]], metadata [[M10:![0-9]*]], metadata [[M11:![0-9]*]], metadata [[M12:![0-9]*]]}
// CHECK: [[M1]] = metadata !{i32 {{[0-9]*}}, metadata [[CTXT]], metadata [[NS]], i32 9} ; [ DW_TAG_imported_module ]
// CHECK: [[M2]] = metadata !{i32 {{[0-9]*}}, metadata [[CU]], metadata [[CTXT]], i32 12} ; [ DW_TAG_imported_module ]
_______________________________________________
cfe-commits mailing list
[email protected]
http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
