On Fri, Jun 2, 2017 at 1:01 PM Sean Silva <chisophugis at gmail.com> wrote:> On Thu, Jun 1, 2017 at 8:47 PM, Petr Hosek via llvm-dev < > llvm-dev at lists.llvm.org> wrote: > >> This is a followup to the discussion that started in D28791. To provide >> the context, we need a way to provide __dso_handle in Fuchsia. __dso_handle >> symbol is mandated by C++ ABI with a value which is an address in one of >> the object's segments, and as such this symbol has to be included >> statically and cannot be a part of a shared library. Different systems >> provide it differently: >> >> 1. On GNU/Linux systems, it's defined in crtbegin.o which is provided by >> libgcc. >> 3. On Android, it's defined in crtbegin.o provided by the Bionic C >> library. >> 3. FreeBSD/OpenBSD/NetBSD provide crtbegin.o as part of the system. >> 4. On macOS, __dso_handle is generated by the linker. >> >> We don't think __dso_handle should be provided by the C library. The name >> __dso_handle is not part of the ABI contract with the C library, it's >> entirely a name chosen by the C++ ABI. Since C++ front-end defines the >> reference to __dso_handle, it should be the compiler that provides the >> definition of it somewhere. The reason GCC defines it in crtbegin.o is >> purely pragmatic; crtbegin.o is an object file that's already linked into >> every shared library and every executable, although there's no principled >> reason for __dso_handle to be defined there. >> >> From the layering point of view, __dso_handle really belongs in >> libc++abi. However, for implementation reasons it needs to be included >> statically into each final link (whether executable or shared library), >> even when libc++abi is built as a shared library. >> >> There several options that we discussed for approaching this: >> >> 1. Provide crtbegin.o/crtend.o implementation in compiler-rt. This is the >> approach I took in D28791. In Fuchsia, we don't need crtbegin.o/crtend.o at >> all, but having these may be useful elsewhere, like LLVM-based Linux >> distributions that won't ship libgcc and will need to provide their own >> crtbegin.o/crtend.o. However, this approach has been met with a lot of >> pushback, where the primary reason has been the fact that >> crtbegin.o/crtend.o contain a lot of legacy cruft that may be difficult to >> get right across all different systems. >> >> 3. Define __dso_handle as part of the Clang builtin library in >> compiler-rt. This is the solution we're currently using with a downstream >> patch ( >> https://fuchsia.googlesource.com/third_party/compiler-rt/+/6c9f8c6ba77090158373490ac010437603c8ff50). >> It works because builtins are already built as a static library, but I'm >> not sure whether it's correct from the layering point of view. >> >> 3. A cleanest solution from the layering perspective would be add another >> archive library to libc++abi, e.g. libc++abi_nonshared.a, that will only >> contain the definition of __dso_handle. The ABI linker script that's >> generated by libc++ could then include this library into every link. This >> work even on systems that already provide __dso_handle. The only aspect >> that might be somewhat complicated is the case when the generation of the >> ABI linker script, e.g. when the ABI library is statically linked into >> libc++. In this case, the libc++abi_nonshared.a would either have to be >> linked manually or we would need to generate another linker script. >> >> 4. Finally, we could extend LLD to define the __dso_handle symbol, >> following the macOS model. This would be a fine solution for us since we >> already use LLD as our default linker, but it may not be as clean as other >> solutions (i.e. using built-in magic for things that can be done via >> general purpose facilities). >> > > Do you have any reason to believe that 4. will be that complicated? If > it's just a matter of defining a symbol that needs to be guaranteed to be > in the current dso, then the linker seems like the natural place to do it > since the linker is the single point of truth for creating the DSO's in the > first place. It seems like the other solutions have quite a bit of > unnecessary complexity due to not being implemented in a "single point of > truth". The hacky nature of the crt*.o hacks (which are basically > piggybacking on the assumption that those are linked into every DSO) is > made clear because Fuchsia differs in this regard, so it wasn't a true > single point of truth. > > Defining __dso_handle when using fuchsia emulation should only be a couple > lines of code in LLD, right? >It's actually pretty straightforward to do this in LLD, see https://reviews.llvm.org/D33856. -------------- next part -------------- An HTML attachment was scrubbed... URL: <http://lists.llvm.org/pipermail/llvm-dev/attachments/20170603/d686d62c/attachment-0001.html>
On Fri, Jun 2, 2017 at 5:03 PM, Petr Hosek <phosek at chromium.org> wrote:> On Fri, Jun 2, 2017 at 1:01 PM Sean Silva <chisophugis at gmail.com> wrote: > >> On Thu, Jun 1, 2017 at 8:47 PM, Petr Hosek via llvm-dev < >> llvm-dev at lists.llvm.org> wrote: >> >>> This is a followup to the discussion that started in D28791. To provide >>> the context, we need a way to provide __dso_handle in Fuchsia. __dso_handle >>> symbol is mandated by C++ ABI with a value which is an address in one of >>> the object's segments, and as such this symbol has to be included >>> statically and cannot be a part of a shared library. Different systems >>> provide it differently: >>> >>> 1. On GNU/Linux systems, it's defined in crtbegin.o which is provided by >>> libgcc. >>> 3. On Android, it's defined in crtbegin.o provided by the Bionic C >>> library. >>> 3. FreeBSD/OpenBSD/NetBSD provide crtbegin.o as part of the system. >>> 4. On macOS, __dso_handle is generated by the linker. >>> >>> We don't think __dso_handle should be provided by the C library. The >>> name __dso_handle is not part of the ABI contract with the C library, it's >>> entirely a name chosen by the C++ ABI. Since C++ front-end defines the >>> reference to __dso_handle, it should be the compiler that provides the >>> definition of it somewhere. The reason GCC defines it in crtbegin.o is >>> purely pragmatic; crtbegin.o is an object file that's already linked into >>> every shared library and every executable, although there's no principled >>> reason for __dso_handle to be defined there. >>> >>> From the layering point of view, __dso_handle really belongs in >>> libc++abi. However, for implementation reasons it needs to be included >>> statically into each final link (whether executable or shared library), >>> even when libc++abi is built as a shared library. >>> >>> There several options that we discussed for approaching this: >>> >>> 1. Provide crtbegin.o/crtend.o implementation in compiler-rt. This is >>> the approach I took in D28791. In Fuchsia, we don't need >>> crtbegin.o/crtend.o at all, but having these may be useful elsewhere, like >>> LLVM-based Linux distributions that won't ship libgcc and will need to >>> provide their own crtbegin.o/crtend.o. However, this approach has been met >>> with a lot of pushback, where the primary reason has been the fact that >>> crtbegin.o/crtend.o contain a lot of legacy cruft that may be difficult to >>> get right across all different systems. >>> >>> 3. Define __dso_handle as part of the Clang builtin library in >>> compiler-rt. This is the solution we're currently using with a downstream >>> patch (https://fuchsia.googlesource.com/third_party/compiler-rt/+/ >>> 6c9f8c6ba77090158373490ac010437603c8ff50). It works because builtins >>> are already built as a static library, but I'm not sure whether it's >>> correct from the layering point of view. >>> >>> 3. A cleanest solution from the layering perspective would be add >>> another archive library to libc++abi, e.g. libc++abi_nonshared.a, that will >>> only contain the definition of __dso_handle. The ABI linker script that's >>> generated by libc++ could then include this library into every link. This >>> work even on systems that already provide __dso_handle. The only aspect >>> that might be somewhat complicated is the case when the generation of the >>> ABI linker script, e.g. when the ABI library is statically linked into >>> libc++. In this case, the libc++abi_nonshared.a would either have to be >>> linked manually or we would need to generate another linker script. >>> >>> 4. Finally, we could extend LLD to define the __dso_handle symbol, >>> following the macOS model. This would be a fine solution for us since we >>> already use LLD as our default linker, but it may not be as clean as other >>> solutions (i.e. using built-in magic for things that can be done via >>> general purpose facilities). >>> >> >> Do you have any reason to believe that 4. will be that complicated? If >> it's just a matter of defining a symbol that needs to be guaranteed to be >> in the current dso, then the linker seems like the natural place to do it >> since the linker is the single point of truth for creating the DSO's in the >> first place. It seems like the other solutions have quite a bit of >> unnecessary complexity due to not being implemented in a "single point of >> truth". The hacky nature of the crt*.o hacks (which are basically >> piggybacking on the assumption that those are linked into every DSO) is >> made clear because Fuchsia differs in this regard, so it wasn't a true >> single point of truth. >> >> Defining __dso_handle when using fuchsia emulation should only be a >> couple lines of code in LLD, right? >> > > It's actually pretty straightforward to do this in LLD, see > https://reviews.llvm.org/D33856. >Since this is literally one line of code and seems to solve the problem in full generality, it seems like the best approach. As long as this doesn't break/affect other platforms, I don't see any reason not to use this approach. -- Sean Silva -------------- next part -------------- An HTML attachment was scrubbed... URL: <http://lists.llvm.org/pipermail/llvm-dev/attachments/20170604/cd4895ba/attachment-0001.html>
As I mentioned in the review thread for https://reviews.llvm.org/D33856, I agree with Sean. Implementing it in LLD seems like the best approach. (I actually like it as it demonstrates how easy things can be done very easily in LLD.) On Sun, Jun 4, 2017 at 5:59 PM, Sean Silva via llvm-dev < llvm-dev at lists.llvm.org> wrote:> > > On Fri, Jun 2, 2017 at 5:03 PM, Petr Hosek <phosek at chromium.org> wrote: > >> On Fri, Jun 2, 2017 at 1:01 PM Sean Silva <chisophugis at gmail.com> wrote: >> >>> On Thu, Jun 1, 2017 at 8:47 PM, Petr Hosek via llvm-dev < >>> llvm-dev at lists.llvm.org> wrote: >>> >>>> This is a followup to the discussion that started in D28791. To provide >>>> the context, we need a way to provide __dso_handle in Fuchsia. __dso_handle >>>> symbol is mandated by C++ ABI with a value which is an address in one of >>>> the object's segments, and as such this symbol has to be included >>>> statically and cannot be a part of a shared library. Different systems >>>> provide it differently: >>>> >>>> 1. On GNU/Linux systems, it's defined in crtbegin.o which is provided >>>> by libgcc. >>>> 3. On Android, it's defined in crtbegin.o provided by the Bionic C >>>> library. >>>> 3. FreeBSD/OpenBSD/NetBSD provide crtbegin.o as part of the system. >>>> 4. On macOS, __dso_handle is generated by the linker. >>>> >>>> We don't think __dso_handle should be provided by the C library. The >>>> name __dso_handle is not part of the ABI contract with the C library, it's >>>> entirely a name chosen by the C++ ABI. Since C++ front-end defines the >>>> reference to __dso_handle, it should be the compiler that provides the >>>> definition of it somewhere. The reason GCC defines it in crtbegin.o is >>>> purely pragmatic; crtbegin.o is an object file that's already linked into >>>> every shared library and every executable, although there's no principled >>>> reason for __dso_handle to be defined there. >>>> >>>> From the layering point of view, __dso_handle really belongs in >>>> libc++abi. However, for implementation reasons it needs to be included >>>> statically into each final link (whether executable or shared library), >>>> even when libc++abi is built as a shared library. >>>> >>>> There several options that we discussed for approaching this: >>>> >>>> 1. Provide crtbegin.o/crtend.o implementation in compiler-rt. This is >>>> the approach I took in D28791. In Fuchsia, we don't need >>>> crtbegin.o/crtend.o at all, but having these may be useful elsewhere, like >>>> LLVM-based Linux distributions that won't ship libgcc and will need to >>>> provide their own crtbegin.o/crtend.o. However, this approach has been met >>>> with a lot of pushback, where the primary reason has been the fact that >>>> crtbegin.o/crtend.o contain a lot of legacy cruft that may be difficult to >>>> get right across all different systems. >>>> >>>> 3. Define __dso_handle as part of the Clang builtin library in >>>> compiler-rt. This is the solution we're currently using with a downstream >>>> patch (https://fuchsia.googlesource.com/third_party/compiler-rt/+/ >>>> 6c9f8c6ba77090158373490ac010437603c8ff50). It works because builtins >>>> are already built as a static library, but I'm not sure whether it's >>>> correct from the layering point of view. >>>> >>>> 3. A cleanest solution from the layering perspective would be add >>>> another archive library to libc++abi, e.g. libc++abi_nonshared.a, that will >>>> only contain the definition of __dso_handle. The ABI linker script that's >>>> generated by libc++ could then include this library into every link. This >>>> work even on systems that already provide __dso_handle. The only aspect >>>> that might be somewhat complicated is the case when the generation of the >>>> ABI linker script, e.g. when the ABI library is statically linked into >>>> libc++. In this case, the libc++abi_nonshared.a would either have to be >>>> linked manually or we would need to generate another linker script. >>>> >>>> 4. Finally, we could extend LLD to define the __dso_handle symbol, >>>> following the macOS model. This would be a fine solution for us since we >>>> already use LLD as our default linker, but it may not be as clean as other >>>> solutions (i.e. using built-in magic for things that can be done via >>>> general purpose facilities). >>>> >>> >>> Do you have any reason to believe that 4. will be that complicated? If >>> it's just a matter of defining a symbol that needs to be guaranteed to be >>> in the current dso, then the linker seems like the natural place to do it >>> since the linker is the single point of truth for creating the DSO's in the >>> first place. It seems like the other solutions have quite a bit of >>> unnecessary complexity due to not being implemented in a "single point of >>> truth". The hacky nature of the crt*.o hacks (which are basically >>> piggybacking on the assumption that those are linked into every DSO) is >>> made clear because Fuchsia differs in this regard, so it wasn't a true >>> single point of truth. >>> >>> Defining __dso_handle when using fuchsia emulation should only be a >>> couple lines of code in LLD, right? >>> >> >> It's actually pretty straightforward to do this in LLD, see >> https://reviews.llvm.org/D33856. >> > > Since this is literally one line of code and seems to solve the problem in > full generality, it seems like the best approach. As long as this doesn't > break/affect other platforms, I don't see any reason not to use this > approach. > > -- Sean Silva > > _______________________________________________ > 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/20170605/e041dbe7/attachment.html>