[PATCH] Fixing a bug where debug info for a local variable gets emitted at file scope

[Yunzhong Gao]

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
-------------- next part --------------
A non-text attachment was scrubbed...
Name: D1281.3.patch
Type: text/x-patch
Size: 11594 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20130809/178db8f9/attachment.bin>