r257754 - PR25910: clang allows two var definitions with the same mangled name

[Andrey Bokhanko via cfe-commits]

Author: asbokhan
Date: Thu Jan 14 04:41:16 2016
New Revision: 257754

URL: http://llvm.org/viewvc/llvm-project?rev=257754=rev
Log:
PR25910: clang allows two var definitions with the same mangled name

Proper diagnostic and resolution of mangled names' conflicts in variables.
When there is a declaration and a definition using the same name but different
types, we emit what is in the definition. When there are two conflicting
definitions, we issue an error.

Differential Revision: http://reviews.llvm.org/D15686

Modified:
cfe/trunk/lib/CodeGen/CodeGenModule.cpp
cfe/trunk/lib/CodeGen/CodeGenModule.h
cfe/trunk/test/CodeGenCXX/duplicate-mangled-name.cpp

Modified: cfe/trunk/lib/CodeGen/CodeGenModule.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/CodeGenModule.cpp?rev=257754=257753=257754=diff
==
--- cfe/trunk/lib/CodeGen/CodeGenModule.cpp (original)
+++ cfe/trunk/lib/CodeGen/CodeGenModule.cpp Thu Jan 14 04:41:16 2016
@@ -1244,19 +1244,23 @@ void CodeGenModule::EmitDeferred() {
 
   for (DeferredGlobal  : CurDeclsToEmit) {
 GlobalDecl D = G.GD;
-llvm::GlobalValue *GV = G.GV;
 G.GV = nullptr;
 
 // We should call GetAddrOfGlobal with IsForDefinition set to true in order
 // to get GlobalValue with exactly the type we need, not something that
 // might had been created for another decl with the same mangled name but
 // different type.
-// FIXME: Support for variables is not implemented yet.
-if (isa(D.getDecl()))
-  GV = cast(GetAddrOfGlobal(D, 
/*IsForDefinition=*/true));
-else
-  if (!GV)
-GV = GetGlobalValue(getMangledName(D));
+llvm::GlobalValue *GV = dyn_cast(
+GetAddrOfGlobal(D, /*IsForDefinition=*/true));
+
+// In case of different address spaces, we may still get a cast, even with
+// IsForDefinition equal to true. Query mangled names table to get
+// GlobalValue.
+if (!GV)
+  GV = GetGlobalValue(getMangledName(D));
+
+// Make sure GetGlobalValue returned non-null.
+assert(GV);
 
 // Check to see if we've already emitted this.  This is necessary
 // for a couple of reasons: first, decls can end up in the
@@ -1264,7 +1268,7 @@ void CodeGenModule::EmitDeferred() {
 // up with definitions in unusual ways (e.g. by an extern inline
 // function acquiring a strong function redefinition).  Just
 // ignore these cases.
-if (GV && !GV->isDeclaration())
+if (!GV->isDeclaration())
   continue;
 
 // Otherwise, emit the definition and move on to the next one.
@@ -1730,7 +1734,7 @@ void CodeGenModule::EmitGlobalDefinition
   }
 
   if (const auto *VD = dyn_cast(D))
-return EmitGlobalVarDefinition(VD);
+return EmitGlobalVarDefinition(VD, !VD->hasDefinition());
   
   llvm_unreachable("Invalid argument to EmitGlobalDefinition()");
 }
@@ -1771,8 +1775,8 @@ CodeGenModule::GetOrCreateLLVMFunction(S
 // error.
 if (IsForDefinition && !Entry->isDeclaration()) {
   GlobalDecl OtherGD;
-  // Check that GD is not yet in ExplicitDefinitions is required to make
-  // sure that we issue an error only once.
+  // Check that GD is not yet in DiagnosedConflictingDefinitions is 
required
+  // to make sure that we issue an error only once.
   if (lookupRepresentativeDecl(MangledName, OtherGD) &&
   (GD.getCanonicalDecl().getDecl() !=
OtherGD.getCanonicalDecl().getDecl()) &&
@@ -1982,10 +1986,15 @@ bool CodeGenModule::isTypeConstant(QualT
 ///
 /// If D is non-null, it specifies a decl that correspond to this.  This is 
used
 /// to set the attributes on the global when it is first created.
+///
+/// If IsForDefinition is true, it is guranteed that an actual global with
+/// type Ty will be returned, not conversion of a variable with the same
+/// mangled name but some other type.
 llvm::Constant *
 CodeGenModule::GetOrCreateLLVMGlobal(StringRef MangledName,
  llvm::PointerType *Ty,
- const VarDecl *D) {
+ const VarDecl *D,
+ bool IsForDefinition) {
   // Lookup the entry, lazily creating it if necessary.
   llvm::GlobalValue *Entry = GetGlobalValue(MangledName);
   if (Entry) {
@@ -2001,11 +2010,34 @@ CodeGenModule::GetOrCreateLLVMGlobal(Str
 if (Entry->getType() == Ty)
   return Entry;
 
+// If there are two attempts to define the same mangled name, issue an
+// error.
+if (IsForDefinition && !Entry->isDeclaration()) {
+  GlobalDecl OtherGD;
+  const VarDecl *OtherD;
+
+  // Check that D is not yet in DiagnosedConflictingDefinitions is required
+  // to make sure that we issue an error only once.
+  if (lookupRepresentativeDecl(MangledName, OtherGD) &&
+  (D->getCanonicalDecl() != OtherGD.getCanonicalDecl().getDecl()) &&
+  (OtherD = 

Re: r257754 - PR25910: clang allows two var definitions with the same mangled name

[Renato Golin via cfe-commits]

On 14 January 2016 at 10:41, Andrey Bokhanko via cfe-commits
 wrote:
> Author: asbokhan
> Date: Thu Jan 14 04:41:16 2016
> New Revision: 257754
>
> URL: http://llvm.org/viewvc/llvm-project?rev=257754=rev
> Log:
> PR25910: clang allows two var definitions with the same mangled name

Hi Andrey,

Just making sure you saw this:
http://lab.llvm.org:8011/builders/clang-cmake-armv7-a15/builds/9057

cheers,
--renato
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Re: r257754 - PR25910: clang allows two var definitions with the same mangled name

[Andrey Bokhanko via cfe-commits]

Renato,

Thanks! -- I committed a fix already:
http://llvm.org/viewvc/llvm-project?view=revision=257757

Yours,
Andrey


On Thu, Jan 14, 2016 at 2:57 PM, Renato Golin  wrote:
> On 14 January 2016 at 10:41, Andrey Bokhanko via cfe-commits
>  wrote:
>> Author: asbokhan
>> Date: Thu Jan 14 04:41:16 2016
>> New Revision: 257754
>>
>> URL: http://llvm.org/viewvc/llvm-project?rev=257754=rev
>> Log:
>> PR25910: clang allows two var definitions with the same mangled name
>
> Hi Andrey,
>
> Just making sure you saw this:
> http://lab.llvm.org:8011/builders/clang-cmake-armv7-a15/builds/9057
>
> cheers,
> --renato
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits