[PATCH] D79028: [DebugInfo][CodeView] Include namespace into emitted globals

[Alexandre Ganea via Phabricator via llvm-commits]

aganea requested review of this revision.
aganea added a comment.

Thanks @amccarth ! Would you mind taking a second look please? The stream now matches MSVC behavior.



================
Comment at: llvm/lib/CodeGen/AsmPrinter/CodeViewDebug.cpp:3116
+        OS,
+        DataSym == SymbolKind::S_LTHREAD32
+            ? DIGV->getName()
----------------
rnk wrote:
> I think this isn't quite right. Consider:
> ```
> $ cat t.cpp
> namespace foo {
> thread_local int global_tls;
> int global_int() { return ++global_tls; }
> static thread_local int static_tls;
> int static_int() { return ++static_tls; }
> int func_int() {
>   thread_local int func_tls;
>   return ++func_tls;
> }
> int func_static_int() {
>   static int func_static;
>   return ++func_static;
> }
> }
> 
> $ cl  -c t.cpp  -Z7 && llvm-pdbutil dump -symbols t.obj  | grep 'THREA\|DATA'
> Microsoft (R) C/C++ Optimizing Compiler Version 19.25.28612 for x64
> Copyright (C) Microsoft Corporation.  All rights reserved.
> 
> t.cpp
>        0 | S_LTHREAD32 [size = 23] `func_tls`
>        0 | S_LDATA32 [size = 26] `func_static`
>        0 | S_GTHREAD32 [size = 30] `foo::global_tls`
>        0 | S_LTHREAD32 [size = 30] `foo::static_tls`
> ```
> 
> It's function local TLS that shouldn't receive a fully qualified name. Let's handle that separately. Those are probably less common. I'm OK committing the previous patchset and handling function local globals separately.
> 
> To fix function local globals (thread local or otherwise), we have to walk up the DIGV scope and see if any of them is a subprogram. We have some existing logic for this for UDTs (typedefs).
I've updated to only handle namespaces. I'll write a follow-up patch to fix the subprogram prefix after landing this.

I've also noted that `static`s inside a class don't seem to generate the right qualifier either in the debug stream:
```
$ cat a.cpp
struct Data {
  inline static thread_local int DataStaticTLS = 5;
  inline static int DataGlobalStatic = 7;
};
int bar() {
  foo::Data D;
  return D.DataStaticTLS + D.DataGlobalStatic;
}
$ cl /Z7 /GS- /c a.cpp /Od /std:c++17 && link /DEBUG /INCREMENTAL:NO a.obj /NODEFAULTLIB /FORCE /ENTRY:bar /SUBSYSTEM:console && llvm-pdbutil.exe dump --all a.pdb | grep S_G.*Data
     132 | S_GTHREAD32 [size = 36] `Data::DataStaticTLS`
     168 | S_GDATA32 [size = 40] `Data::DataGlobalStatic`
```
With clang-cl (this patch applied):
```
     132 | S_GTHREAD32 [size = 28] `DataStaticTLS`
     160 | S_GDATA32 [size = 32] `DataGlobalStatic`
```


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D79028/new/

https://reviews.llvm.org/D79028