Skip to content

Commit

Permalink
Don't crash when a selectany symbol would get common linkage
Browse files Browse the repository at this point in the history
Things can't both be in comdats and have common linkage, so never give things
in comdats common linkage. Common linkage is only used in .c files, and the
only thing that can trigger a comdat in c is selectany from what I can tell.
Fixes PR23243.

Also address an over-the-shoulder review comment from rnk by moving the
hasAttr<SelectAnyAttr>() in Decl.cpp around a bit. It only makes a minor
difference for selectany on global variables, so it goes well with the rest of
this patch.

http://reviews.llvm.org/D9042


git-svn-id: https://llvm.org/svn/llvm-project/cfe/trunk@235053 91177308-0d34-0410-b5e6-96231b3b80d8
  • Loading branch information
nico committed Apr 15, 2015
1 parent b01e0c6 commit 7bbbb59
Show file tree
Hide file tree
Showing 4 changed files with 15 additions and 6 deletions.
6 changes: 3 additions & 3 deletions lib/AST/Decl.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1915,7 +1915,7 @@ VarDecl::isThisDeclarationADefinition(ASTContext &C) const {
if (hasInit())
return Definition;

if (hasAttr<AliasAttr>())
if (hasAttr<AliasAttr>() || hasAttr<SelectAnyAttr>())
return Definition;

// A variable template specialization (other than a static data member
Expand All @@ -1925,14 +1925,14 @@ VarDecl::isThisDeclarationADefinition(ASTContext &C) const {
getTemplateSpecializationKind() != TSK_ExplicitSpecialization)
return DeclarationOnly;

if (!hasAttr<SelectAnyAttr>() && hasExternalStorage())
if (hasExternalStorage())
return DeclarationOnly;

// [dcl.link] p7:
// A declaration directly contained in a linkage-specification is treated
// as if it contains the extern specifier for the purpose of determining
// the linkage of the declared name and whether it is a definition.
if (!hasAttr<SelectAnyAttr>() && isSingleLineLanguageLinkage(*this))
if (isSingleLineLanguageLinkage(*this))
return DeclarationOnly;

// C99 6.9.2p2:
Expand Down
9 changes: 7 additions & 2 deletions lib/CodeGen/CodeGenModule.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -2134,7 +2134,8 @@ void CodeGenModule::EmitGlobalVarDefinition(const VarDecl *D) {
}

static bool isVarDeclStrongDefinition(const ASTContext &Context,
const VarDecl *D, bool NoCommon) {
CodeGenModule &CGM, const VarDecl *D,
bool NoCommon) {
// Don't give variables common linkage if -fno-common was specified unless it
// was overridden by a NoCommon attribute.
if ((NoCommon || D->hasAttr<NoCommonAttr>()) && !D->hasAttr<CommonAttr>())
Expand All @@ -2159,6 +2160,10 @@ static bool isVarDeclStrongDefinition(const ASTContext &Context,
if (D->hasAttr<WeakImportAttr>())
return true;

// A variable cannot be both common and exist in a comdat.
if (shouldBeInCOMDAT(CGM, *D))
return true;

// Declarations with a required alignment do not have common linakge in MSVC
// mode.
if (Context.getLangOpts().MSVCCompat) {
Expand Down Expand Up @@ -2227,7 +2232,7 @@ llvm::GlobalValue::LinkageTypes CodeGenModule::getLLVMLinkageForDeclarator(
// C++ doesn't have tentative definitions and thus cannot have common
// linkage.
if (!getLangOpts().CPlusPlus && isa<VarDecl>(D) &&
!isVarDeclStrongDefinition(Context, cast<VarDecl>(D),
!isVarDeclStrongDefinition(Context, *this, cast<VarDecl>(D),
CodeGenOpts.NoCommon))
return llvm::GlobalVariable::CommonLinkage;

Expand Down
4 changes: 3 additions & 1 deletion test/CodeGen/ms-declspecs.c
Original file line number Diff line number Diff line change
Expand Up @@ -6,8 +6,10 @@ const __declspec(selectany) int x2 = 2;
// CHECK: @x2 = weak_odr constant i32 2, comdat, align 4

// selectany turns extern variable declarations into definitions.
extern __declspec(selectany) int x3;
__declspec(selectany) int x3;
extern __declspec(selectany) int x4;
// CHECK: @x3 = weak_odr global i32 0, comdat, align 4
// CHECK: @x4 = weak_odr global i32 0, comdat, align 4

struct __declspec(align(16)) S {
char x;
Expand Down
2 changes: 2 additions & 0 deletions test/CodeGen/ms-declspecs.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,9 @@ extern "C++" __declspec(selectany) int x3;
extern "C" {
__declspec(selectany) int x4;
}
__declspec(selectany) int x5;
// CHECK: @"\01?x1@@3HA" = weak_odr global i32 0, comdat, align 4
// CHECK: @x2 = weak_odr global i32 0, comdat, align 4
// CHECK: @"\01?x3@@3HA" = weak_odr global i32 0, comdat, align 4
// CHECK: @x4 = weak_odr global i32 0, comdat, align 4
// CHECK: @"\01?x5@@3HA" = weak_odr global i32 0, comdat, align 4

0 comments on commit 7bbbb59

Please sign in to comment.