A bug in ReadArchiveBuffer() caused gccld to his an assertion during code
optimization passes.  This bug stems from a std::string in ReadArchiveBuffer()
that is passed by reference to the newly created Module object that it passes
back to its caller.  Since the std::string is deallocated on exit from
ReadArchiveBuffer(), the caller has a Module object refering to freed memory,
and this can lead to odd behavior in gccld (and potentially other programs).

This is fixed by revision 1.12 of llvm/lib/Bytecode/Reader/ArchiveReader.cpp.

Change that.  After some more thought, I believe my original fix is incorrect. 
The Module class should be copying the string, so my fix probably just masks the
problem without really fixing it.

Yeah, your patch didn't change anything (in fact, it causes an std::string to be
leaked), so please revert it.  The string object should be copied into the module.

Can you indicate better how to reproduce the problem?

-Chris

Change reverted.

To reproduce this bug, I attempted to compile Apache 1.3.29 with llvmgcc, which
failed.  After some work, I discovered that I could reproduce the error as follows:

1) Take libstandard.a and put it into a directory.
2) Attempt to link it with buildmark.o and modules.o

Below is the assertion that gets hit when this occurs:

gccld -o httpd buildmark.o modules.o silly/libstandard.a 
gccld: /home/vadve/criswell/llvm/lib/VMCore/Value.cpp:81: void
llvm::Value::replaceAllUsesWith(llvm::Value*): Assertion `New->getType() ==
getType() && "replaceAllUses of value with new value of different type!"' failed.
Abort (core dumped)

Now, the funny thing is, you'd think this was a regular bug in a pass or
something, but it doesn't appear to be:

1) Moving libstandard.a out of a directory makes the bug disappear.
2) Extracting the .o files from libstandard.a and linking them in directly makes
the bug disappear.
3) If you modify ArchiveReader.cpp so that it does not use name Module's after
names gathered from SVR4 long archive names, the bug disappears.

So, it appears to be a subtle memory bug somewhere, but I just can't see where
in the code it might be.

> So, it appears to be a subtle memory bug somewhere, but I just can't see where
> in the code it might be.

Have you tried running the program through valgrind?

-Chris

Yes.  Valgrind isn't reporting any memory errors; just memory leaks.

What does the stack trace look like from the assertion failure?  What types are
the two values being replaceAllUsesOfWith'd?

-Chris

The two types being resolved are:

GV: .includes_handlers_34
I : .autoindex_handlers_34

The stacktrace is (line numbers may be off due to additional debugging statements):

#0  0x401390a1 in kill () from /lib/libc.so.6
#1  0x40138e99 in raise () from /lib/libc.so.6
#2  0x4013a2a8 in abort () from /lib/libc.so.6
#3  0x4013315d in __assert_fail () from /lib/libc.so.6
#4  0x0822178f in llvm::Value::replaceAllUsesWith(llvm::Value*) (
    this=0x8a4aca0, New=0x89f5530)
    at /home/vadve/criswell/llvm/lib/VMCore/Value.cpp:81
#5  0x082880c3 in (anonymous namespace)::ConstantMerge::run(llvm::Module&) (
    this=0x8b8f698, M=@0x8398c50)
    at /home/vadve/criswell/llvm/lib/Transforms/IPO/ConstantMerge.cpp:56
#6  0x08267245 in llvm::PassManagerTraits<llvm::Module>::runPass(llvm::Pass*,
llvm::Module*) (P=0x8b8f698, M=0x8398c50) at PassManagerT.h:724
#7  0x0825b683 in llvm::PassManagerT<llvm::Module>::runOnUnit(llvm::Module*) (
    this=0x89245a0, M=0x8398c50) at PassManagerT.h:253
#8  0x0825cd2b in llvm::PassManagerTraits<llvm::Module>::run(llvm::Module&) (
    this=0x89245a0, M=@0x8398c50) at PassManagerT.h:733
#9  0x08217317 in llvm::PassManager::run(llvm::Module&) (this=0xbfffed70, 
    M=@0x8398c50) at /home/vadve/criswell/llvm/lib/VMCore/Pass.cpp:81
#10 0x081d11cc in llvm::GenerateBytecode(llvm::Module*, bool, bool,
std::ostream*) (M=0x8398c50, Strip=false, Internalize=true, Out=0xbfffefd0)
    at /home/vadve/criswell/llvm/tools/gccld/GenerateCode.cpp:132
#11 0x081d8fda in main (argc=7, argv=0xbffff1d4, envp=0xbffff1f4)
    at /home/vadve/criswell/llvm/tools/gccld/gccld.cpp:237

Sorry, the last comment gives the names of the types.  Here are the actual
types:

%.autoindex_handlers_34 = internal constant [2 x { sbyte*, int
(%struct.request_rec*)* }] [ { sbyte*, int (%struct.request_rec*)* } { sbyte*
getelementptr ([21 x sbyte]* %.str_187, long 0, long 0), int
(%struct.request_rec*)* %.handle_autoindex_33 }, { sbyte*, int
(%struct.request_rec*)* } { sbyte* null, int (%struct.request_rec*)* null } ]   

%.includes_handlers_34 = internal constant [5 x { sbyte*, int
(%struct.request_rec*)* }] [ { sbyte*, int (%struct.request_rec*)* } { sbyte*
getelementptr ([26 x sbyte]* %.str_212, long 0, long 0), int
(%struct.request_rec*)* %.send_shtml_file_31 }, { sbyte*, int
(%struct.request_rec*)* } { sbyte* getelementptr ([27 x sbyte]* %.str_213, long
0, long 0), int (%struct.request_rec*)* %.send_shtml_file_31 }, { sbyte*, int
(%struct.request_rec*)* } { sbyte* getelementptr ([14 x sbyte]* %.str_214, long
0, long 0), int (%struct.request_rec*)* %.send_parsed_file_30 }, { sbyte*, int
(%struct.request_rec*)* } { sbyte* getelementptr ([10 x sbyte]* %.str_209, long
0, long 0), int (%struct.request_rec*)* %.xbithack_handler_32 }, { sbyte*, int
(%struct.request_rec*)* } { sbyte* null, int (%struct.request_rec*)* null } ]   

Off the top of my head, I don't think these two constants are the same.

You're right, those constants are definitely not the same.  Can you send me
paths to the bytecode files in question, and the command used to reproduce this?
 It looks like a very wierd problem...

-Chris

The files are in /home/vadve/criswell/test/apache.
The following command will reproduce the problem:

gccld -o httpd buildmark.o modules.o silly/libstandard.a

You will probably want to make a copy of the directory as I'm fiddling around
with files there.

I can't reproduce the problem.  gccld does not assert or core dump, and it
valgrinds clean...  I tried both with and without the patch you reverted.

-Chris

Ok, I see the problem.  It's a bug in the ConstantMerge.cpp pass: you're right
it's a really nasty memory problem.

The flow in ConstantMerge looks like this:
1. It decides that two global constants should be merged, say A into B, by
checking a map from the initializer of A to the global B.
2. It calls A->replaceAllUsesWith(B), so that everything using A now uses B
3. It deletes A.

The problem is step #2.  If another global initializer used A as part of the
initializer, this requires changing the initializer to use B instead, which
means that the pointer to that initializer is invalidated.  Later, we look this
sucker up in the map, and find an invalidated pointer, causing all kinds of bad
things to happen.

Neeto.  I'm not sure what a reasonable and efficient fix for this is, but I'll
work on it.  Thanks for tracking this down John!

-Chris

Fixed:
http://mail.cs.uiuc.edu/pipermail/llvm-commits/Week-of-Mon-20031222/010425.html

Because this is not a determinstic problem, I have no testcase.

-Chris