Skip to content
Open
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
48 changes: 44 additions & 4 deletions clang/lib/CodeGen/CodeGenModule.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -588,8 +588,9 @@ static const llvm::GlobalValue *getAliasedGlobal(const llvm::GlobalValue *GV) {
}

static bool checkAliasedGlobal(
const ASTContext &Context, DiagnosticsEngine &Diags, SourceLocation Location,
bool IsIFunc, const llvm::GlobalValue *Alias, const llvm::GlobalValue *&GV,
const CodeGenModule *CGM, const ASTContext &Context,
DiagnosticsEngine &Diags, SourceLocation Location, bool IsIFunc,
const llvm::GlobalValue *Alias, const llvm::GlobalValue *&GV,
const llvm::MapVector<GlobalDecl, StringRef> &MangledDeclNames,
SourceRange AliasRange) {
GV = getAliasedGlobal(Alias);
Expand All @@ -598,6 +599,23 @@ static bool checkAliasedGlobal(
return false;
}

// Only resolve unique internal linkage symbols for C code
if (!CGM->getLangOpts().CPlusPlus) {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this C-only? Consider something like:

long bar(const struct A*p) __attribute__((__alias__("foo")));
namespace {
extern "C" long foo(const struct A*p) {return 1;}
}

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yup, we are trying to build a codebase written entirely in C... I'm not too sure how namespace and extern "C" would translate in C tho...

for (const auto &[Decl, Name] : MangledDeclNames) {
if (const auto *ND = dyn_cast<NamedDecl>(Decl.getDecl())) {
IdentifierInfo *II = ND->getIdentifier();
if (II && II->getName() == GV->getName() &&
Name.contains(llvm::FunctionSamples::UniqSuffix)) {
GlobalDecl GD;
if (CGM->lookupRepresentativeDecl(Name, GD)) {
GV = CGM->getModule().getNamedValue(Name);
break;
}
}
}
}
}

if (GV->hasCommonLinkage()) {
const llvm::Triple &Triple = Context.getTargetInfo().getTriple();
if (Triple.getObjectFormat() == llvm::Triple::XCOFF) {
Expand Down Expand Up @@ -687,8 +705,8 @@ void CodeGenModule::checkAliases() {
StringRef MangledName = getMangledName(GD);
llvm::GlobalValue *Alias = GetGlobalValue(MangledName);
const llvm::GlobalValue *GV = nullptr;
if (!checkAliasedGlobal(getContext(), Diags, Location, IsIFunc, Alias, GV,
MangledDeclNames, Range)) {
if (!checkAliasedGlobal(this, getContext(), Diags, Location, IsIFunc, Alias,
GV, MangledDeclNames, Range)) {
Error = true;
continue;
}
Expand Down Expand Up @@ -4038,6 +4056,7 @@ void CodeGenModule::EmitGlobal(GlobalDecl GD) {
CXXGlobalInits.push_back(nullptr);
}

const auto *ND = dyn_cast<NamedDecl>(GD.getDecl());
StringRef MangledName = getMangledName(GD);
if (GetGlobalValue(MangledName) != nullptr) {
// The value has already been used and should therefore be emitted.
Expand All @@ -4046,6 +4065,12 @@ void CodeGenModule::EmitGlobal(GlobalDecl GD) {
// The value must be emitted, but cannot be emitted eagerly.
assert(!MayBeEmittedEagerly(Global));
addDeferredDeclToEmit(GD);
} else if (!getLangOpts().CPlusPlus && ND &&
GetGlobalValue(ND->getName()) != nullptr &&
MangledName.contains(llvm::FunctionSamples::UniqSuffix)) {
// Emit static C function that is mangled with
// -funique-internal-linkage-names.
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not really happy with this. The code for deferred declarations and diagnosing name conflicts is complicated enough without adding the possibility that the name of a declaration can change later.

Can we just suppress mangling in cases where we reference a symbol without knowing if it's internal, then it turns out to be internal later?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The problem is we need -funique-internal-linkage-names to keep all the static function symbols unique... It is to make sure that we have unique symbols names in the DWARF info after linking. Besides.... it's also a bit weird to specify something like __attribute__((__alias__(_ZL3fooPK1A.__uniq.43626431813877880438002914689114607187))) in the C Code.. It's only a problem with attribute alias tho... Do you think we should add more unit tests for this change?

addDeferredDeclToEmit(GD);
} else {
// Otherwise, remember that we saw a deferred decl with this name. The
// first use of the mangled name will cause it to move into
Expand Down Expand Up @@ -6189,6 +6214,21 @@ void CodeGenModule::EmitGlobalFunctionDefinition(GlobalDecl GD,
/*DontDefer=*/true,
ForDefinition));

if (!getLangOpts().CPlusPlus &&
getCXXABI().getMangleContext().shouldMangleDeclName(D)) {
// -funique-internal-linkage-names may change the symbol name of C function.
// Replace all uses of old symbol with the emitted global value.
if (IdentifierInfo *II = D->getIdentifier()) {
if (II->getName() != GV->getName() &&
GV->getName().contains(llvm::FunctionSamples::UniqSuffix)) {
if (llvm::GlobalValue *GVDef =
getModule().getNamedValue(D->getName())) {
GVDef->replaceAllUsesWith(GV);
}
}
}
}

// Already emitted.
if (!GV->isDeclaration())
return;
Expand Down
10 changes: 10 additions & 0 deletions clang/test/CodeGen/unique-internal-linkage-names-alias.c
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
// RUN: %clang_cc1 %s -emit-llvm -funique-internal-linkage-names -o - | FileCheck %s

struct A;
static long foo(const struct A*p);

long bar(const struct A*p);
long bar(const struct A*p) __attribute__((__alias__("foo")));

// CHECK: define internal i64 @_ZL3fooPK1A.__uniq.[[ATTR:[0-9]+]](ptr noundef %p) #1 {
static long foo(const struct A*p) {return 1;}
Loading