llvm dev - Sep 2017 - LLD: patch to fix libCOFF calling exit() on success in a library function

Correct, I am using libCOFF, libELF, and libMACHO all as a library. Ideally
all cases would return and report an error and clean up memory, etc,
instead of calling exit. However this is sufficient for my needs for now.
It is ok for LLD to crash if I supply an invalid command line argument, I
won't do that.

On Thu, Aug 31, 2017 at 5:47 PM, Rui Ueyama <ruiu at google.com> wrote:
> COFF lld uses exit() unlike ELF lld due to lack of user needs, but are you
> trying to use it as a library? If so, I believe you need to fix other
> places where exit() is called on success or for a trivial error (such as
> invalid command line argument) as well.
>
> On Thu, Aug 31, 2017 at 10:45 AM, Andrew Kelley via llvm-dev <
> llvm-dev at lists.llvm.org> wrote:
>
>> I believe that LLD is not supposed to call exit on success when you
>> call lld::coff::link.
>>
>> From downstream fork of LLD: https://github.com/zig-la
>> ng/zig/commit/41da9fdb69065082f57c604b12eb02ca166cb18d
>>
>>
>> diff --git a/lld/COFF/Driver.cpp b/lld/COFF/Driver.cpp
>> index 854c3e69098..8b17f039870 100644
>> --- a/lld/COFF/Driver.cpp
>> +++ b/lld/COFF/Driver.cpp
>> @@ -1030,7 +1030,7 @@ void LinkerDriver::link(ArrayRef<const char
*>
>> ArgsArr) {
>>    if (!Args.hasArgNoClaim(OPT_INPUT)) {
>>      fixupExports();
>>      createImportLibrary(/*AsLib=*/true);
>> -    exit(0);
>> +    return;
>>    }
>>
>>    // Handle /delayload
>> @@ -1122,7 +1122,7 @@ void LinkerDriver::link(ArrayRef<const char
*>
>> ArgsArr) {
>>    // This is useful because MSVC link.exe can generate complete PDBs.
>>    if (Args.hasArg(OPT_msvclto)) {
>>      invokeMSVC(Args);
>> -    exit(0);
>> +    return;
>>    }
>>
>>    // Do LTO by compiling bitcode input files to a set of native COFF
>> files then
>> @@ -1173,8 +1173,7 @@ void LinkerDriver::link(ArrayRef<const char
*>
>> ArgsArr) {
>>    // Write the result.
>>    writeResult(&Symtab);
>>
>> -  // Call exit to avoid calling destructors.
>> -  exit(0);
>> +  return;
>>  }
>>
>>  } // namespace coff
>>
>>
>> _______________________________________________
>> LLVM Developers mailing list
>> llvm-dev at lists.llvm.org
>> http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-dev
>>
>>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL:
<http://lists.llvm.org/pipermail/llvm-dev/attachments/20170831/299c2f66/attachment.html>

I think I prefer adding CanExitEarly flag to COFF lld just like ELF lld. We
call `exit` at the end of link() because exiting without calling
destructors of globallly-allocated objects makes the linker noticeably
faster, so I don't want to replace it with return.

On Thu, Aug 31, 2017 at 3:16 PM, Andrew Kelley <superjoe30 at gmail.com>
wrote:
> Correct, I am using libCOFF, libELF, and libMACHO all as a library.
> Ideally all cases would return and report an error and clean up memory,
> etc, instead of calling exit. However this is sufficient for my needs for
> now. It is ok for LLD to crash if I supply an invalid command line
> argument, I won't do that.
>
>
> On Thu, Aug 31, 2017 at 5:47 PM, Rui Ueyama <ruiu at google.com>
wrote:
>
>> COFF lld uses exit() unlike ELF lld due to lack of user needs, but are
>> you trying to use it as a library? If so, I believe you need to fix
other
>> places where exit() is called on success or for a trivial error (such
as
>> invalid command line argument) as well.
>>
>> On Thu, Aug 31, 2017 at 10:45 AM, Andrew Kelley via llvm-dev <
>> llvm-dev at lists.llvm.org> wrote:
>>
>>> I believe that LLD is not supposed to call exit on success when you
>>> call lld::coff::link.
>>>
>>> From downstream fork of LLD: https://github.com/zig-la
>>> ng/zig/commit/41da9fdb69065082f57c604b12eb02ca166cb18d
>>>
>>>
>>> diff --git a/lld/COFF/Driver.cpp b/lld/COFF/Driver.cpp
>>> index 854c3e69098..8b17f039870 100644
>>> --- a/lld/COFF/Driver.cpp
>>> +++ b/lld/COFF/Driver.cpp
>>> @@ -1030,7 +1030,7 @@ void LinkerDriver::link(ArrayRef<const
char *>
>>> ArgsArr) {
>>>    if (!Args.hasArgNoClaim(OPT_INPUT)) {
>>>      fixupExports();
>>>      createImportLibrary(/*AsLib=*/true);
>>> -    exit(0);
>>> +    return;
>>>    }
>>>
>>>    // Handle /delayload
>>> @@ -1122,7 +1122,7 @@ void LinkerDriver::link(ArrayRef<const
char *>
>>> ArgsArr) {
>>>    // This is useful because MSVC link.exe can generate complete
PDBs.
>>>    if (Args.hasArg(OPT_msvclto)) {
>>>      invokeMSVC(Args);
>>> -    exit(0);
>>> +    return;
>>>    }
>>>
>>>    // Do LTO by compiling bitcode input files to a set of native
COFF
>>> files then
>>> @@ -1173,8 +1173,7 @@ void LinkerDriver::link(ArrayRef<const
char *>
>>> ArgsArr) {
>>>    // Write the result.
>>>    writeResult(&Symtab);
>>>
>>> -  // Call exit to avoid calling destructors.
>>> -  exit(0);
>>> +  return;
>>>  }
>>>
>>>  } // namespace coff
>>>
>>>
>>> _______________________________________________
>>> LLVM Developers mailing list
>>> llvm-dev at lists.llvm.org
>>> http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-dev
>>>
>>>
>>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL:
<http://lists.llvm.org/pipermail/llvm-dev/attachments/20170901/0552337b/attachment.html>

That sounds good to me

On Sep 1, 2017 5:20 PM, "Rui Ueyama" <ruiu at google.com> wrote:

I think I prefer adding CanExitEarly flag to COFF lld just like ELF lld. We
call `exit` at the end of link() because exiting without calling
destructors of globallly-allocated objects makes the linker noticeably
faster, so I don't want to replace it with return.

On Thu, Aug 31, 2017 at 3:16 PM, Andrew Kelley <superjoe30 at gmail.com>
wrote:
> Correct, I am using libCOFF, libELF, and libMACHO all as a library.
> Ideally all cases would return and report an error and clean up memory,
> etc, instead of calling exit. However this is sufficient for my needs for
> now. It is ok for LLD to crash if I supply an invalid command line
> argument, I won't do that.
>
>
> On Thu, Aug 31, 2017 at 5:47 PM, Rui Ueyama <ruiu at google.com>
wrote:
>
>> COFF lld uses exit() unlike ELF lld due to lack of user needs, but are
>> you trying to use it as a library? If so, I believe you need to fix
other
>> places where exit() is called on success or for a trivial error (such
as
>> invalid command line argument) as well.
>>
>> On Thu, Aug 31, 2017 at 10:45 AM, Andrew Kelley via llvm-dev <
>> llvm-dev at lists.llvm.org> wrote:
>>
>>> I believe that LLD is not supposed to call exit on success when you
>>> call lld::coff::link.
>>>
>>> From downstream fork of LLD: https://github.com/zig-la
>>> ng/zig/commit/41da9fdb69065082f57c604b12eb02ca166cb18d
>>>
>>>
>>> diff --git a/lld/COFF/Driver.cpp b/lld/COFF/Driver.cpp
>>> index 854c3e69098..8b17f039870 100644
>>> --- a/lld/COFF/Driver.cpp
>>> +++ b/lld/COFF/Driver.cpp
>>> @@ -1030,7 +1030,7 @@ void LinkerDriver::link(ArrayRef<const
char *>
>>> ArgsArr) {
>>>    if (!Args.hasArgNoClaim(OPT_INPUT)) {
>>>      fixupExports();
>>>      createImportLibrary(/*AsLib=*/true);
>>> -    exit(0);
>>> +    return;
>>>    }
>>>
>>>    // Handle /delayload
>>> @@ -1122,7 +1122,7 @@ void LinkerDriver::link(ArrayRef<const
char *>
>>> ArgsArr) {
>>>    // This is useful because MSVC link.exe can generate complete
PDBs.
>>>    if (Args.hasArg(OPT_msvclto)) {
>>>      invokeMSVC(Args);
>>> -    exit(0);
>>> +    return;
>>>    }
>>>
>>>    // Do LTO by compiling bitcode input files to a set of native
COFF
>>> files then
>>> @@ -1173,8 +1173,7 @@ void LinkerDriver::link(ArrayRef<const
char *>
>>> ArgsArr) {
>>>    // Write the result.
>>>    writeResult(&Symtab);
>>>
>>> -  // Call exit to avoid calling destructors.
>>> -  exit(0);
>>> +  return;
>>>  }
>>>
>>>  } // namespace coff
>>>
>>>
>>> _______________________________________________
>>> LLVM Developers mailing list
>>> llvm-dev at lists.llvm.org
>>> http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-dev
>>>
>>>
>>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL:
<http://lists.llvm.org/pipermail/llvm-dev/attachments/20170901/72c195e8/attachment.html>