llvm dev - Sep 2017 - [ThinLTO] static library failure with object files with the same name

Hi all,
  I have a static library with object files with the same name (not the
same full path, but the archive made with llvm-ar does not store the full
path). The library contains object files that have been compiled with
`-flto=thin` (compiled with LDC, not clang, but that shouldn't matter).
When linking to that static library, I get the error:
Assertion failed: (ModuleMap.find(ModuleBuffer.getBufferIdentifier())
=ModuleMap.end() && "Expect unique Buffer Identifier"),
function
generateModuleMap, file ../lib/LTO/ThinLTOCodeGenerator.cpp, line 138.

The error occurs because the buffer identifier uses the filename of the
objects inside the archive, and those are identical for the two files with
different source path.

This problem appears to be fixed for LLD here:
https://reviews.llvm.org/D25495
https://bugs.llvm.org/show_bug.cgi?id=30665

But it persists for linking with the system linker on OSX (while manually
passing libLTO.dylib to the linker).

If I modify lib/LTO/ThinLTOCodeGenerator.cpp to do a poor-man's uniquefying
of the buffer identifier:
```
void ThinLTOCodeGenerator::addModule(StringRef Identifier, StringRef Data) {
-  ThinLTOBuffer Buffer(Data, Identifier.str()));
+  ThinLTOBuffer Buffer(Data, Identifier.str() + utostr(Data.size()));
```
then the problem is solved.

What would be a proper way to fix this issue?
Can it be fixed in lib/LTO, or should I not create an (afaict valid)
archive containing duplicate object file names?

(Note: this issue makes it impossible to use an LTO version of the standard
library with LDC)

Thanks,
  Johan
-------------- next part --------------
An HTML attachment was scrubbed...
URL:
<http://lists.llvm.org/pipermail/llvm-dev/attachments/20170905/046c18b2/attachment.html>

Hi Johan,

Right, per the bug this is fixed in lld (and was already handled in
gold-plugin), but I guess not in ld64. Note that lld and gold-plugin use
the new LTO API, while ld64 (and probably other linkers) are still using
the legacy libLTO (which is what ThinLTOCodeGenerator.cpp is part of).
Fixing it in the location you propose could work for all legacy libLTO
users. But I don't think that adding just the size will (always) be enough
to disambiguate (couldn't the 2 same named members have the same size?) -
although lld is doing the same thing so this may be as good as what is done
there. For gold-plugin we add the byte offset into the archive where the
member starts, which will be unique.
+davide for thoughts since he fixed it on the lld side.

Teresa

On Tue, Sep 5, 2017 at 1:09 PM, Johan Engelen via llvm-dev <
llvm-dev at lists.llvm.org> wrote:
> Hi all,
>   I have a static library with object files with the same name (not the
> same full path, but the archive made with llvm-ar does not store the full
> path). The library contains object files that have been compiled with
> `-flto=thin` (compiled with LDC, not clang, but that shouldn't matter).
> When linking to that static library, I get the error:
> Assertion failed: (ModuleMap.find(ModuleBuffer.getBufferIdentifier()) =>
ModuleMap.end() && "Expect unique Buffer Identifier"),
function
> generateModuleMap, file ../lib/LTO/ThinLTOCodeGenerator.cpp, line 138.
>
> The error occurs because the buffer identifier uses the filename of the
> objects inside the archive, and those are identical for the two files with
> different source path.
>
> This problem appears to be fixed for LLD here:
> https://reviews.llvm.org/D25495
> https://bugs.llvm.org/show_bug.cgi?id=30665
>
> But it persists for linking with the system linker on OSX (while manually
> passing libLTO.dylib to the linker).
>
> If I modify lib/LTO/ThinLTOCodeGenerator.cpp to do a poor-man's
> uniquefying of the buffer identifier:
> ```
> void ThinLTOCodeGenerator::addModule(StringRef Identifier, StringRef
> Data) {
> -  ThinLTOBuffer Buffer(Data, Identifier.str()));
> +  ThinLTOBuffer Buffer(Data, Identifier.str() + utostr(Data.size()));
> ```
> then the problem is solved.
>
> What would be a proper way to fix this issue?
> Can it be fixed in lib/LTO, or should I not create an (afaict valid)
> archive containing duplicate object file names?
>
> (Note: this issue makes it impossible to use an LTO version of the
> standard library with LDC)
>
> Thanks,
>   Johan
>
>
> _______________________________________________
> LLVM Developers mailing list
> llvm-dev at lists.llvm.org
> http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-dev
>
>

-- 
Teresa Johnson |  Software Engineer |  tejohnson at google.com |  408-460-2413
-------------- next part --------------
An HTML attachment was scrubbed...
URL:
<http://lists.llvm.org/pipermail/llvm-dev/attachments/20170905/b9aaf2b7/attachment.html>

On Tue, Sep 5, 2017 at 2:09 PM, Teresa Johnson <tejohnson at google.com>
wrote:
>
> Hi Johan,
>
> Right, per the bug this is fixed in lld (and was already handled in
gold-plugin), but I guess not in ld64. Note that lld and gold-plugin use the new
LTO API, while ld64 (and probably other linkers) are still using the legacy
libLTO (which is what ThinLTOCodeGenerator.cpp is part of). Fixing it in the
location you propose could work for all legacy libLTO users. But I don't
think that adding just the size will (always) be enough to disambiguate
(couldn't the 2 same named members have the same size?) - although lld is
doing the same thing so this may be as good as what is done there. For
gold-plugin we add the byte offset into the archive where the member starts,
which will be unique.
> +davide for thoughts since he fixed it on the lld side.
>
Yes, Teresa is right, this is the correct fix.
I'm not sure what subset of GNU archives Mach-O supports, but the only
way of being safe is using offset in the archive + archive name.
Fun fact, you apparently can have a single GNU archive with two
different members with the same name. Using the offset is the only way
 to disambiguate.
I think we should really consider documenting this assumption
somewhere as many people have been bitten in the past and tracking
down is not trivial as it shows up as assertion failures in the mover
or duplicate/undefined symbols reported as linker errors, as the
ThinLTO logic will pick the an archive randomly.