Quuxplusone / LLVMBugzillaTest

0 stars 0 forks source link

Internal compile error with alias attribute #4727

Open Quuxplusone opened 15 years ago

Quuxplusone commented 15 years ago
Bugzilla Link PR4241
Status NEW
Importance P normal
Reported by Mike Van Emmerik (mike.emmerik@sun.com)
Reported on 2009-05-22 02:05:25 -0700
Last modified on 2011-10-14 02:31:48 -0700
Version unspecified
Hardware Other Solaris
CC anton@korobeynikov.info, baldrick@free.fr, chandlerc@gmail.com, efriedma@quicinc.com, llvm-bugs@lists.llvm.org, mike.emmerik@sun.com, Nathan.Keynes@sun.com
Fixed by commit(s)
Attachments alias.patch (1461 bytes, text/plain)
test.c (290 bytes, text/plain)
gv-alias.c (1172 bytes, text/x-csrc)
gcc-alias.patch (1989 bytes, text/plain)
Blocks
Blocked by
See also
Created attachment 3011
Patch that WORKS AROUND the problem; may NOT be a complete fix

llvm-backend.cpp assumes that if a function entry is renamed, then an object
with the same name already exists in the symbol table. I speculate that when
the alias attribute is used, this is not valid.

In any case, when the code below is compiled, an ICE results because
getGlobalVariable() returns NULL:

void cfi_cmdset_0001() {}
void  *cfi_cmdset_0003() __attribute__((alias("cfi_cmdset_0001")));
extern typeof(cfi_cmdset_0003) cfi_cmdset_0003;

Steps to reproduce
==================
Compile the above or attached code with
  llvm-gcc -c -o foo test.c

Actual results
==============
test.c:8: internal compiler error: Segmentation Fault
Please submit a full bug report,
with preprocessed source if appropriate.

Expected Results
================
Normal compilation with no error; valid foo.o produced.

Build Date and Platform
=======================
2009/May/20 on Solaris 2.11 (X86) llvm-gcc is 4.2-2.4 (gcc 4.2, llvm 2.4)

Additional platforms
====================
Also happens on llvm 2.5

Workaround
==========

The code can be fixed by the attached patch. It checks for G being null, and if
so, doesn't attempt to replace the presumed old symbol with the cast of the new
one. NOTE: this may not be a proper fix; the test should determine whether
there really has been a rename or an alias, and handle the two cases
appropriately.

      // If FnEntry got renamed, then there is already an object with this name
      // in the symbol table.  If this happens, the old one must be a forward
      // decl, just replace it with a cast of the new one.
      if (FnEntry->getName() != Name) {
        GlobalVariable *G = TheModule->getGlobalVariable(Name, true);
        assert(G && G->isDeclaration() && "A global turned into a function?");
+       if (G) {                    /* G can be NULL! */
        // Replace any uses of "G" with uses of FnEntry.
        Value *GInNewType = TheFolder->CreateBitCast(FnEntry, G->getType());
        G->replaceAllUsesWith(GInNewType);

        // Update the decl that points to G.
        changeLLVMValue(G, GInNewType);

        // Now we can give GV the proper name.
        FnEntry->takeName(G);

        // G is now dead, nuke it.
        G->eraseFromParent();
+     } /* End if */
      }
Quuxplusone commented 15 years ago

Attached test.c (290 bytes, text/plain): Testcase

Quuxplusone commented 15 years ago

The testcase in question works fine with trunk on x86-32; have you tried a trunk build?

Quuxplusone commented 15 years ago

I see this failing with svn head.

Quuxplusone commented 15 years ago
my target doesn't support aliases, so I can't test this.  However, the code
looks very broken: it is assuming that the conflicting value is a global.  Try
changing:

        GlobalVariable *G = TheModule->getGlobalVariable(Name, true);
to:
        GlobalValue *G = TheModule->getNamedValue(Name);

There is probably a corresponding case for global name conflicts somewhere else.
Quuxplusone commented 15 years ago
> The testcase in question works fine with trunk on x86-32; have you tried a
trunk build?
Yes, I have now. It still fails. I suspected the Solaris linker at one point
(even though linking isn't involved here), and recompiled with the gnu linker.
It still fails.
Quuxplusone commented 15 years ago
> The testcase in question works fine with trunk on x86-32; have you tried a
trunk build?
Yes, I have now. It still fails. I suspected the Solaris linker at one point
(even though linking isn't involved here), and recompiled with the gnu linker.
It still fails.

> Try changing:

>        GlobalVariable *G = TheModule->getGlobalVariable(Name, true);
>to:
>        GlobalValue *G = TheModule->getNamedValue(Name);

That fixes it, thanks Chris!

> There is probably a corresponding case for global name conflicts somewhere
else.

Unfortunately, I don't know the code well enough to find this.
Quuxplusone commented 15 years ago
Hi Chris,

   I assume you're referring to the section of code immediately below in llvm-backend.cpp that builds the GlobalVariable declarations?

    if (GV->getName() != Name) {
      Function *F = TheModule->getFunction(Name);
      assert(F && F->isDeclaration() && "A function turned into a global?");
   ...

 That section also looks highly dubious, but I haven't been able to contrive a test case that actually breaks it so far - I'm not certain it's actually reachable code.

Cheers,
Nathan
Quuxplusone commented 15 years ago

Attached gv-alias.c (1172 bytes, text/x-csrc): Test case for related case (global variable looking for function)

Quuxplusone commented 15 years ago

Attached alias.patch (1461 bytes, text/plain): Patch that WORKS AROUND the problem; may NOT be a complete fix

Quuxplusone commented 15 years ago

Attached gcc-alias.patch (1989 bytes, text/plain): Combined patch for alias issues in gcc/llvm-backend.cpp

Quuxplusone commented 13 years ago

If this is still a bug at all, it's a dragonegg bug. llvm-gcc is dead.

Quuxplusone commented 13 years ago

It fails in dragonegg too.