- User Since
- Jan 18 2017, 9:47 AM (156 w, 2 d)
Thu, Jan 16
I rebased the patch and addressed the review comments. I also noticed that there was a problem with the patch when explicitly placing non-mergeable sections into one of the "default" mergeable sections (e.g. .debug_str). As these "default" mergeable sections were not being added to the map it wasn't possible to explicitly place symbols into these even if the symbols entsize was compatible (as they would be put into a uniqued section instead). This is not acceptable for these "default" sections since for some of these like e.g. .debug_str, current binutils consumers make the assumption that there can only be one of these sections. The fix for this is WIP and I haven't added LIT tests for it yet; but, I have put up the patch in WIP state to get comments on the approach taken.
Tue, Jan 14
Thanks, I'll address the points that you have raised.
I tried updating the assert and rebasing the patch but the assert is now firing in the It tests. I will have to investigate this tomorrow and update the patch then.
Mon, Jan 13
Updated the diff to address review comments and expand test coverage.
Mon, Jan 6
Thanks for the feedback. I'm sorry that I am being slow on this... v.busy at the moment. Will try to finish the patch in the next few days.
I wonder if is this really a bug or intentional. Clang and LLVM seem to ignore the possibility of symbol interposition (see: -fno-semantic-interposition). What is the LLVM policy for this?
Fri, Jan 3
Thu, Jan 2
Sorry for the delay in updating this.
Dec 3 2019
Dec 2 2019
The feature certainly could be implemented for COFF, so I think changing the name of the command line option is reasonable. The strings in the .deplibs sections map to libraries in an implementation defined manner, so I refer to them as "specifiers". In COFF dependent libraries are specified via "directives" in the object files. It might be worth naming the option something like: --dependent-lib-directives (as directives works for the entries in an ELF .deplibs sections equally as well as specifiers) ?
Dec 1 2019
Nov 21 2019
Hi All, Apologies that I missed the request for input here.
Oct 16 2019
Oct 2 2019
Oct 1 2019
Sep 27 2019
Sep 26 2019
Jun 12 2019
Jun 11 2019
Jun 10 2019
Jun 7 2019
Thanks for the response.
Jun 5 2019
May 21 2019
Great to see someone wanting to use this. During the RFC we decided not to set SHF_EXCLUDE on the .deplibs ELF section that is used to store the strings from these pragmas. This means that other linkers that don't support dependent libraries will not remove the sections and they will end up in the output. This shouldn't cause any functional issues; however, it might be noticed by developers and commented on. Perhaps we should change this decision? Also, have you considered the other option of having the build system invoke the clang with the --dependent-library=<specifier> switch. This is an alternative approach where modifying the source code is not appropriate, an example use is: https://reviews.llvm.org/D61742.
May 16 2019
May 9 2019
May 1 2019
Apr 30 2019
Apr 25 2019
Addressed review comments
I am keen to keep this moving.
Apr 24 2019
Apr 16 2019
No longer shortening "dependent libraries" to "deplibs" except for the .deplibs section (as this takes up bytes on disk).
Apr 15 2019
LGTM! Thanks for waiting for me to confirm. Comments inline:
Apr 12 2019
Sorry for the delay in replying. I was trying to understand if the upgrade path was important. Comments inline:
Apr 10 2019
Apr 9 2019
I have updated the diff to address review comments. I think we can continue to refine this patch in parallel with discussing the design further (I am not dismissing the concerns raised by @compnerd and @jyknight).
Addressed review comments.
Apr 8 2019
Apr 5 2019
Apr 4 2019
Mar 28 2019
Apologies that I didn't check the information on the original bug. The GDB behaviour does look reasonable, thanks.
Mar 22 2019
<Trying again via phabricator rather than replying on the list.>
Mar 21 2019
Jan 16 2019
I strongly feel that we shouldn't implement this. This will tie groups and --gc-sections together when they are orthogonal features; e.g. COMDAT groups will *also* have to be considered by --gc-sections as a single unit. The ability to strip individual sections form COMDAT groups is an important feature. I will add similar comment to the abi list. I'm surprised that gold implements this behavior, back in 2017 gold and bfd-ld disagreed on this.
Jan 10 2019
Jan 9 2019
The wording of the standard is unfortunate. This has been brought up and clarified (that COMDATs are the only legal groups currently) on the list previously.
There is a relevant discussion about non-COMDAT groups here: https://reviews.llvm.org/D56437.
I agree with Rui. My understanding is that groups in ELF were designed specifically to support C++ templates and inline functions which is why COMDAT groups are the only allowed kind.
Interestingly, I have been having a somewhat related discussion on this review: https://reviews.llvm.org/D53234.
Could the annobin tool use SHF_LINK_ORDER instead?
Dec 21 2018
Nov 28 2018
Nov 26 2018
Trying again... Phabricator didn't like the markup in my last comment attempt.
Nov 16 2018
Oct 31 2018
Hi Aron, I'm interested in reviewing this. Would you mind responding to my comments on https://reviews.llvm.org/D53234 first, as you have based the correctness of this on the correctness of that change?
Sep 5 2018
Thanks for the comments everyone.
Aug 31 2018
Added missing newline at end of test file