Support debug info for alias variable

[kavitha-natarajan]

Summary

This is in continuation with the patch posted earlier for bug-50052:

https://reviews.llvm.org/D103131

For the below testcase, when compiled with clang compiler, debugger is not able to print alias variable type or value.
$ cat test.c
int oldname = 1;
extern int newname attribute((alias("oldname")));

int main ()
{

return 0;

}

$ clang -g -O0 test.c

$ gdb a.out
(gdb) ptype oldname
type = int
(gdb) ptype newname
type = <data variable, no debug info>
(gdb) p oldname
$1 = 1
(gdb) p newname
'newname' has unknown type; cast it to its declared type

This is because clang is not emitting dwarf information for alias variable. The above mentioned patch supports clang to emit debug info for alias variable as imported entity (DW_TAG_imported_declaration). However, gdb cannot handle the imported declaration for alias variables. GCC emits debug info for alias variables as DW_TAG_variable which gdb can handle. The discussions in the above bug report and patch review links talk about why it is appropriate to emit alias variable as DW_TAG_imported_declaration rather than DW_TAG_variable. Refer section "3.2.3 Imported (or Renamed) Declaration Entries" in DWARF 5 specification.

In the clang patch, CGDebugInfo.cpp:EmitGlobalAlias() function is rewritten to handle nested (recursive) imported declaration and developed a testcase as well. A corresponding gdb patch that can handle DW_TAG_imported_declaration as alias variables is also developed and will be posted to gdb community for review in parallel.

After clang and gdb fixes:

$ gdb a.out
(gdb) ptype oldname
type = int
(gdb) ptype newname
type = int
(gdb) p oldname
$1 = 1
(gdb) p newname
$2 = 1
(gdb)

[dblaikie]

[removing comment - this was intended for a different issue (leaving this here rather than deleting so it's not confusing when people come from the email threads, etc)]

[dblaikie]

Broad question about debug info for aliases: How's this proposed solution compare/contrast to GCC's behavior?

[dblaikie]

In D120989#3362490, @dblaikie wrote:

Broad question about debug info for aliases: How's this proposed solution compare/contrast to GCC's behavior?

Aaand, I see that was described in thhe patch description. Sorry for not reading...

Right, paging in all the context - this review hinges on whether gdb/lldb can handle/be made to handle this sort of debug info correctly.

[kavitha-natarajan]

Rebased and applied pre-merge check formatting.

[kavitha-natarajan]

In D120989#3362491, @dblaikie wrote:

In D120989#3362490, @dblaikie wrote:

Broad question about debug info for aliases: How's this proposed solution compare/contrast to GCC's behavior?

Aaand, I see that was described in thhe patch description. Sorry for not reading...

Right, paging in all the context - this review hinges on whether gdb/lldb can handle/be made to handle this sort of debug info correctly.

I have posted a gdb patch to handle this change for review:
https://sourceware.org/pipermail/gdb-patches/2022-March/186329.html

lldb can handle this debug info correctly.

[dblaikie]

In D120989#3362912, @kavitha-natarajan wrote:

In D120989#3362491, @dblaikie wrote:

In D120989#3362490, @dblaikie wrote:

Broad question about debug info for aliases: How's this proposed solution compare/contrast to GCC's behavior?

Aaand, I see that was described in thhe patch description. Sorry for not reading...

Right, paging in all the context - this review hinges on whether gdb/lldb can handle/be made to handle this sort of debug info correctly.

I have posted a gdb patch to handle this change for review:
https://sourceware.org/pipermail/gdb-patches/2022-March/186329.html

lldb can handle this debug info correctly.

Thanks for the details - can you ping this thread once the gdb thread has progressed/seems like it's moving in this direction?

[dblaikie]

Maybe APple folks would want to move forward with this only when targeting lldb, though - which might unblock this patch. @aprantl @JDevlieghere ? What do you folks think?

[aprantl]

This looks mostly fine to me, I have a couple of superficial comments inline.

clang/lib/CodeGen/CGDebugInfo.cpp

3862

Nit: The LLVM coding style wants all comments to be full sentences, including a trailing .

3869

When would we enter a nullptr into the cache?

5422

See LLVM coding style

clang/lib/CodeGen/CodeGenModule.cpp

1506

std::find()?

[kavitha-natarajan]

@aprantl Updated the comments following LLVM coding style.

[kavitha-natarajan]

clang/lib/CodeGen/CGDebugInfo.cpp

3869

In this change, only llvm::DIImportedEntity entries are pushed to ImportedDeclCache. So it will never return nullptr. It is only a guard to not to fall into the case of no definition in case of non llvm::DIImportedEntity in the cache (undesired and to be handled by the caller).

clang/lib/CodeGen/CodeGenModule.cpp

1506

MangledDeclNames is a llvm::MapVector implementation, so can't use std::find(). The key for MangledDeclNames MapVector is GlobalDecl and StringRef is the value. llvm::Mapvector::find() is search by key. Here we are doing search by value.

[aprantl]

clang/lib/CodeGen/CGDebugInfo.cpp

3870

if we don't expect anything but non-null DINodes to be in the cache, then this whole condition should be

return cast<llvm::DINode>(N);

That will assert if N is null.

5433

LLVM coding style says to remove the redundant else after a return.

5443

.

[kavitha-natarajan]

@aprantl, thanks for your comments. Updated the patch with the fixes.

@dblaikie, haven't got any response from gdb reviewers about the gdb patch yet.

[kavitha-natarajan]

Thanks for the details - can you ping this thread once the gdb thread has progressed/seems like it's moving in this direction?

@dblaikie, received comments for the gdb patch https://sourceware.org/pipermail/gdb-patches/2022-March/186960.html and the next message with my response. Can you please look through?

[dblaikie]

In D120989#3411553, @kavitha-natarajan wrote:

Thanks for the details - can you ping this thread once the gdb thread has progressed/seems like it's moving in this direction?

@dblaikie, received comments for the gdb patch https://sourceware.org/pipermail/gdb-patches/2022-March/186960.html and the next message with my response. Can you please look through?

Sure, sounds like things are moving in the right direction there - so from my perspective, this sounds good. Please wait for @aprantl's sign off as well.

[thakis]

This breaks tests on mac: https://logs.chromium.org/logs/chromium/buildbucket/cr-buildbucket/8817534773179515649/+/u/package_clang/stdout?format=raw

Script:
 --
 : 'RUN: at line 1';   /opt/s/w/ir/cache/builder/src/third_party/llvm-bootstrap/bin/clang -cc1 -internal-isystem /opt/s/w/ir/cache/builder/src/third_party/llvm-bootstrap/lib/clang/15.0.0/include -nostdsysteminc -emit-llvm -debug-info-kind=limited /opt/s/w/ir/cache/builder/src/third_party/llvm/clang/test/CodeGen/debug-info-alias.c -o - | /opt/s/w/ir/cache/builder/src/third_party/llvm-bootstrap/bin/FileCheck /opt/s/w/ir/cache/builder/src/third_party/llvm/clang/test/CodeGen/debug-info-alias.c
 --
 Exit Code: 2
 
 Command Output (stderr):
 --
 /opt/s/w/ir/cache/builder/src/third_party/llvm/clang/test/CodeGen/debug-info-alias.c:10:27: error: aliases are not supported on darwin
 extern int __attribute__((alias("aliased_global"))) __global_alias;
                           ^
 /opt/s/w/ir/cache/builder/src/third_party/llvm/clang/test/CodeGen/debug-info-alias.c:14:27: error: aliases are not supported on darwin
 extern int __attribute__((alias("aliased_global_2"))) global_alias_2;
                           ^
 /opt/s/w/ir/cache/builder/src/third_party/llvm/clang/test/CodeGen/debug-info-alias.c:15:27: error: aliases are not supported on darwin
 extern int __attribute__((alias("global_alias_2"))) __global_alias_2_alias;
                           ^
 3 errors generated.
 FileCheck error: '<stdin>' is empty.
 FileCheck command line:  /opt/s/w/ir/cache/builder/src/third_party/llvm-bootstrap/bin/FileCheck /opt/s/w/ir/cache/builder/src/third_party/llvm/clang/test/CodeGen/debug-info-alias.c
 
 --
 
 ********************
 Testing:  0.. 10.. 20.. 30.. 40.. 50.. 60.. 70.. 80.. 90.. 
 ********************
 Failed Tests (1):
   Clang :: CodeGen/debug-info-alias.c

Probably easiest to pass an explicit triple?

[kavitha-natarajan]

Aliases are not supported on mac. Below commit has made the test to be skipped for mac. Thanks @abrachet for the quick fix.

commit 50de659adcc19c4c197ba5e9a7719325af7151ee
Author: Alex Brachet <[e-mail]>
Date: Thu Apr 7 18:19:54 2022 +0000

[clang] Use -triple, not -target for %clang_cc1

commit 3329dae5cb8a8c91c518dd87c09e88c4fad507bd
Author: Alex Brachet <[e-mail]>
Date: Thu Apr 7 18:17:29 2022 +0000

[clang] Fix macos build broken after D120989

clang/lib/CodeGen/CGDebugInfo.h

#include "clang/Basic/OpenCLExtensionTypes.def"
   /// This is a storage for names that are constructed on demand. For
   /// example, C++ destructors, C++ operators etc..
   llvm::BumpPtrAllocator DebugInfoNames;
   StringRef CWDName;
 
   llvm::DenseMap<const char *, llvm::TrackingMDRef> DIFileCache;
   llvm::DenseMap<const FunctionDecl *, llvm::TrackingMDRef> SPCache;
   /// Cache declarations relevant to DW_TAG_imported_declarations (C++
-  /// using declarations) that aren't covered by other more specific caches.
+  /// using declarations and global alias variables) that aren't covered
+  /// by other more specific caches.
   llvm::DenseMap<const Decl *, llvm::TrackingMDRef> DeclCache;
+  llvm::DenseMap<const Decl *, llvm::TrackingMDRef> ImportedDeclCache;
   llvm::DenseMap<const NamespaceDecl *, llvm::TrackingMDRef> NamespaceCache;
   llvm::DenseMap<const NamespaceAliasDecl *, llvm::TrackingMDRef>
       NamespaceAliasCache;
   llvm::DenseMap<const Decl *, llvm::TypedTrackingMDRef<llvm::DIDerivedType>>
       StaticDataMemberCache;
 
   using ParamDecl2StmtTy = llvm::DenseMap<const ParmVarDecl *, const Stmt *>;
   using Param2DILocTy =
public:
   void EmitGlobalVariable(llvm::GlobalVariable *GV, const VarDecl *Decl);
 
   /// Emit a constant global variable's debug info.
   void EmitGlobalVariable(const ValueDecl *VD, const APValue &Init);
 
   /// Emit information about an external variable.
   void EmitExternalVariable(llvm::GlobalVariable *GV, const VarDecl *Decl);
 
+  /// Emit information about global variable alias.
+  void EmitGlobalAlias(const llvm::GlobalValue *GV, const GlobalDecl Decl);
+
   /// Emit C++ using directive.
   void EmitUsingDirective(const UsingDirectiveDecl &UD);
 
   /// Emit the type explicitly casted to.
   void EmitExplicitCastType(QualType Ty);
 
   /// Emit the type even if it might not be used.
   void EmitAndRetainType(QualType Ty);

clang/lib/CodeGen/CGDebugInfo.cpp

if (const auto *TD = dyn_cast<TypeDecl>(D))
     return getOrCreateType(CGM.getContext().getTypeDeclType(TD),
                            getOrCreateFile(TD->getLocation()));
   auto I = DeclCache.find(D->getCanonicalDecl());
 
   if (I != DeclCache.end()) {
     auto N = I->second;
     if (auto *GVE = dyn_cast_or_null<llvm::DIGlobalVariableExpression>(N))
       return GVE->getVariable();
+    return cast<llvm::DINode>(N);
+  }
+
+  // Search imported declaration cache if it is already defined
+  // as imported declaration.
+  auto IE = ImportedDeclCache.find(D->getCanonicalDecl());
+
+  if (IE != ImportedDeclCache.end()) {
+    auto N = IE->second;
+    if (auto *GVE = dyn_cast_or_null<llvm::DIImportedEntity>(N))
+      return cast<llvm::DINode>(GVE);
     return dyn_cast_or_null<llvm::DINode>(N);
   }
 
   // No definition for now. Emit a forward definition that might be
   // merged with a potential upcoming definition.
   if (const auto *FD = dyn_cast<FunctionDecl>(D))
     return getFunctionForwardDeclaration(FD);
   else if (const auto *VD = dyn_cast<VarDecl>(D))
     return getGlobalVariableForwardDeclaration(VD);
 
   return nullptr;
 }
 
 llvm::DISubprogram *CGDebugInfo::getFunctionDeclaration(const Decl *D) {
   if (!D || DebugKind <= codegenoptions::DebugLineTablesOnly)
     return nullptr;
 
   const auto *FD = dyn_cast<FunctionDecl>(D);
   if (!FD)
     return nullptr;
void CGDebugInfo::EmitExternalVariable(llvm::GlobalVariable *Var,
   llvm::DIScope *DContext = getDeclContextDescriptor(D);
   llvm::DIGlobalVariableExpression *GVE =
       DBuilder.createGlobalVariableExpression(
           DContext, Name, StringRef(), Unit, getLineNumber(D->getLocation()),
           Ty, false, false, nullptr, nullptr, nullptr, Align);
   Var->addDebugInfo(GVE);
 }
 
+void CGDebugInfo::EmitGlobalAlias(const llvm::GlobalValue *GV,
+                                  const GlobalDecl GD) {
+
+  assert(GV);
+
+  if (!CGM.getCodeGenOpts().hasReducedDebugInfo())
+    return;
+
+  const auto *D = cast<ValueDecl>(GD.getDecl());
+  if (D->hasAttr<NoDebugAttr>())
+    return;
+
+  auto AliaseeDecl = CGM.getMangledNameDecl(GV->getName());
+  llvm::DINode *DI;
+
+  if (!AliaseeDecl)
+    // FIXME: Aliasee not declared yet - possibly declared later
+    // For example,
+    //
+    //   1 extern int newname __attribute__((alias("oldname")));
+    //   2 int oldname = 1;
+    //
+    // No debug info would be generated for 'newname' in this case.
+    //
+    // Fix compiler to generate "newname" as imported_declaration
+    // pointing to the DIE of "oldname".
+    return;
+  if (!(DI = getDeclarationOrDefinition(
+            AliaseeDecl.getCanonicalDecl().getDecl())))
+    return;
+
+  llvm::DIScope *DContext = getDeclContextDescriptor(D);
+  auto Loc = D->getLocation();
+
+  llvm::DIImportedEntity *ImportDI = DBuilder.createImportedDeclaration(
+      DContext, DI, getOrCreateFile(Loc), getLineNumber(Loc), D->getName());
+
+  // Record this DIE in the cache for nested declaration reference.
+  ImportedDeclCache[GD.getCanonicalDecl().getDecl()].reset(ImportDI);
+}
+
 llvm::DIScope *CGDebugInfo::getCurrentContextDescriptor(const Decl *D) {
   if (!LexicalBlockStack.empty())
     return LexicalBlockStack.back();
   llvm::DIScope *Mod = getParentModuleOrNull(D);
   return getContextDescriptor(D, Mod ? Mod : TheCU);
 }
 
 void CGDebugInfo::EmitUsingDirective(const UsingDirectiveDecl &UD) {

clang/lib/CodeGen/CodeGenModule.h

public:
   void addDefaultFunctionDefinitionAttributes(llvm::Function &F);
 
   /// Like the overload taking a `Function &`, but intended specifically
   /// for frontends that want to build on Clang's target-configuration logic.
   void addDefaultFunctionDefinitionAttributes(llvm::AttrBuilder &attrs);
 
   StringRef getMangledName(GlobalDecl GD);
   StringRef getBlockMangledName(GlobalDecl GD, const BlockDecl *BD);
+  const GlobalDecl getMangledNameDecl(StringRef);
 
   void EmitTentativeDefinition(const VarDecl *D);
 
   void EmitExternalDeclaration(const VarDecl *D);
 
   void EmitVTable(CXXRecordDecl *Class);
 
   void RefreshTypeCacheForClass(const CXXRecordDecl *Class);

clang/lib/CodeGen/CodeGenModule.cpp

else if (const auto *DD = dyn_cast<CXXDestructorDecl>(D))
     MangleCtx.mangleDtorBlock(DD, GD.getDtorType(), BD, Out);
   else
     MangleCtx.mangleBlock(cast<DeclContext>(D), BD, Out);
 
   auto Result = Manglings.insert(std::make_pair(Out.str(), BD));
   return Result.first->first();
 }
 
+const GlobalDecl CodeGenModule::getMangledNameDecl(StringRef Name) {
+  auto it = MangledDeclNames.begin();
+  while (it != MangledDeclNames.end()) {
+    if (it->second == Name)
+      return it->first;
+    it++;
+  }
+  return GlobalDecl();
+}
+
 llvm::GlobalValue *CodeGenModule::GetGlobalValue(StringRef Name) {
   return getModule().getNamedValue(Name);
 }
 
 /// AddGlobalCtor - Add a function to the list that will be called before
 /// main() runs.
 void CodeGenModule::AddGlobalCtor(llvm::Function *Ctor, int Priority,
                                   llvm::Constant *AssociatedData) {
if (D->hasAttr<WeakAttr>() || D->hasAttr<WeakRefAttr>() ||
     GA->setLinkage(llvm::Function::WeakAnyLinkage);
   }
 
   if (const auto *VD = dyn_cast<VarDecl>(D))
     if (VD->getTLSKind())
       setTLSMode(GA, *VD);
 
   SetCommonAttributes(GD, GA);
+
+  // Emit global alias debug information.
+  if (const auto *VD = dyn_cast<VarDecl>(D)) {
+    if (CGDebugInfo *DI = getModuleDebugInfo()) {
+      DI->EmitGlobalAlias(cast<llvm::GlobalValue>(GA->getAliasee()), GD);
+    }
+  }
 }
 
 void CodeGenModule::emitIFuncDefinition(GlobalDecl GD) {
   const auto *D = cast<ValueDecl>(GD.getDecl());
   const IFuncAttr *IFA = D->getAttr<IFuncAttr>();
   assert(IFA && "Not an ifunc?");
 
   StringRef MangledName = getMangledName(GD);

clang/test/CodeGen/debug-info-alias.c

+// RUN: %clang_cc1 -emit-llvm -debug-info-kind=limited %s -o - | FileCheck %s
+
+// CHECK-DAG: [[ENTITY1:![0-9]+]] = distinct !DIGlobalVariable(name: "aliased_global"
+// CHECK-DAG: [[ENTITY2:![0-9]+]] = distinct !DIGlobalVariable(name: "aliased_global_2"
+// CHECK-DAG: !DIImportedEntity(tag: DW_TAG_imported_declaration, name: "__global_alias", scope: !2, entity: [[ENTITY1]]
+// CHECK-DAG: [[ENTITY3:![0-9]+]] = !DIImportedEntity(tag: DW_TAG_imported_declaration, name: "global_alias_2", scope: !2, entity: [[ENTITY2]]
+// CHECK-DAG: !DIImportedEntity(tag: DW_TAG_imported_declaration, name: "__global_alias_2_alias", scope: !2, entity: [[ENTITY3]]
+
+int aliased_global = 1;
+extern int __attribute__((alias("aliased_global"))) __global_alias;
+
+// Recursive alias:
+int aliased_global_2 = 2;
+extern int __attribute__((alias("aliased_global_2"))) global_alias_2;
+extern int __attribute__((alias("global_alias_2"))) __global_alias_2_alias;