[llvm-commits] [llvm] r115753 - /llvm/trunk/lib/Linker/LinkModules.cpp

[Eric Christopher]

On Oct 5, 2010, at 11:16 PM, Bill Wendling wrote:

> Author: void
> Date: Wed Oct  6 01:16:30 2010
> New Revision: 115753
> 
> URL: http://llvm.org/viewvc/llvm-project?rev=115753&view=rev
> Log:
> If the destination module all ready has a copy of the global coming from the
> source module *and* it must be merged (instead of simply replaced or appended
> to), then merge instead of replacing or adding another global.
> 
> The ObjC __image_info section was being appended to because of this
> failure. This caused a crash because the linker expects the image info section
> to be a specific size.
> 
> <rdar://problem/8198537>
> 
> +// RequiresMerge - Return true if the source global variable needs to be merged
> +// with the destination global variable. I.e., there shouldn't be a new global
> +// variable created in the destination Module, rather the initializers should be
> +// merged in an intelligent fashion.
> +static bool RequiresMerge(const GlobalVariable *SGV, const GlobalVariable *DGV){
> +  const StringRef SrcSec(SGV->getSection());
> +  const StringRef DstSec(DGV->getSection());
> +
> +  // The Objective-C __image_info section should be unique.
> +  if (SrcSec == DstSec &&
> +      (SrcSec.find("__objc_imageinfo") != StringRef::npos ||
> +       SrcSec.find("__image_info") != StringRef::npos))
> +    return true;
> +
> +  return false;
> +}
> +

Definitely think this should be "RequiresUnique" instead of RequiresMerge, perhaps
UniqueSection(SGV->getSection(), DGV->getSection())?  Merging sounds too much
like the ELF SEC_MERGE or general merging of data.  

> @@ -819,10 +848,10 @@
>       // Grab destination global variable or alias.
>       GlobalValue *DGV = cast<GlobalValue>(ValueMap[SGV]->stripPointerCasts());
> 
> -      // If dest if global variable, check that initializers match.
> +      // If dest is a global variable, check that initializers match.
>       if (GlobalVariable *DGVar = dyn_cast<GlobalVariable>(DGV)) {
>         if (DGVar->hasInitializer()) {
> -          if (SGV->hasExternalLinkage()) {
> +          if (SGV->hasExternalLinkage() || RequiresMerge(SGV, DGVar)) {
>             if (DGVar->getInitializer() != SInit)
>               return Error(Err, "Global Variable Collision on '" +
>                            SGV->getName() +

It'll make this verification here sound like it makes more sense.

Thoughts?

-eric