r203052 - Construct GlobalValues with the correct linkage instead of using setLinkage.

[Rafael Espíndola]

I can reproduce that this crashes with

 ./bin/clang -cc1 -triple x86_64-apple-macosx -emit-llvm  ~/llvm/test.m -w

because it tries to declare a

l_OBJC_PROTOCOL_$_p1 with a weak linkage.

Nick, does ld64 support having undefined reference to l prefixed
symbols be resolved to definitions in another file?


On 12 September 2014 15:20, Steven Wu <stevenwu at apple.com> wrote:
> Hi Rafael
>
> Sorry for a separate (and late) email since I don’t have the email back to
> March. And sorry for sending it again since I used the wrong email address
> for the previous and gets rejected by cfe-commit.
> I found a problem compiling ObjC recently and I can only trace to the change
> in this patch.
> This patch is indeed a functional change when coming to protocols that are
> forward declared and implemented in other files (which now generates a
> warning but not back then).
> Originally, the codegen uses a deal with this case by initialize all
> dependent protocols to ExternalLinkage and change the linkage back when
> these protocols get processed.
> For the external and forward-declared protocols will not get processed and
> remained ExternalLinkage.
> Here is a testcase:
>
>  @interface NSObject @end
>
>  @protocol p1;
>
>  @protocol p2 <p1>
>
>  - (void)s1;
>
>  @end
>
>  @interface I1 : NSObject <p2>
>
>  - (void)s2;
>  @end
>
>  @implementation I1
>
>  - (void)s1 {
>        NSLog(@"s1");
>  }
>  - (void)s2 {
>        NSLog(@"s2");
>  }
>  @end
>
> I am not familiar with ObjC standard so I am not sure if this is 100% legal
> but this patch causes the compiler fatal error.
> Steven
>
>
> Author: rafael
> Date: Wed Mar  5 19:10:46 2014
> New Revision: 203052
>
> URL: http://llvm.org/viewvc/llvm-project?rev=203052&view=rev
> Log:
> Construct GlobalValues with the correct linkage instead of using setLinkage.
>
> Modified:
>     cfe/trunk/lib/CodeGen/CGObjCMac.cpp
>
> Modified: cfe/trunk/lib/CodeGen/CGObjCMac.cpp
> URL:
> http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/CGObjCMac.cpp?rev=203052&r1=203051&r2=203052&view=diff
> ==============================================================================
> --- cfe/trunk/lib/CodeGen/CGObjCMac.cpp (original)
> +++ cfe/trunk/lib/CodeGen/CGObjCMac.cpp Wed Mar  5 19:10:46 2014
> @@ -2660,8 +2660,8 @@ llvm::Constant *CGObjCMac::GetOrEmitProt
>                                                     Values);
>
>    if (Entry) {
> -    // Already created, fix the linkage and update the initializer.
> -    Entry->setLinkage(llvm::GlobalValue::PrivateLinkage);
> +    // Already created, update the initializer.
> +    assert(Entry->getLinkage() == llvm::GlobalValue::PrivateLinkage);
>      Entry->setInitializer(Init);
>    } else {
>      Entry =
> @@ -6242,10 +6242,9 @@ llvm::Constant *CGObjCNonFragileABIMac::
>      // reference or not. At module finalization we add the empty
>      // contents for protocols which were referenced but never defined.
>      Entry =
> -      new llvm::GlobalVariable(CGM.getModule(), ObjCTypes.ProtocolnfABITy,
> false,
> -                               llvm::GlobalValue::ExternalLinkage,
> -                               0,
> -                               "\01l_OBJC_PROTOCOL_$_" + PD->getName());
> +        new llvm::GlobalVariable(CGM.getModule(),
> ObjCTypes.ProtocolnfABITy,
> +                                 false, llvm::GlobalValue::WeakAnyLinkage,
> 0,
> +                                 "\01l_OBJC_PROTOCOL_$_" + PD->getName());
>      Entry->setSection("__DATA,__datacoal_nt,coalesced");
>    }
>
> @@ -6358,8 +6357,8 @@ llvm::Constant *CGObjCNonFragileABIMac::
>                                                     Values);
>
>    if (Entry) {
> -    // Already created, fix the linkage and update the initializer.
> -    Entry->setLinkage(llvm::GlobalValue::WeakAnyLinkage);
> +    // Already created, update the initializer.
> +    assert(Entry->getLinkage() == llvm::GlobalValue::WeakAnyLinkage);
>      Entry->setInitializer(Init);
>    } else {
>      Entry =
>
>