- User Since
- Jan 18 2017, 9:47 AM (167 w, 6 d)
Use IR rather than C for the clang test.
I was able to simplify the clang patch somewhat as a "lowering error" isn't any more revealing than just emitting a generic "backend error".
Report error using LLVM diagnostics framework.
Thu, Apr 2
Wed, Apr 1
Tue, Mar 31
use reportError instead of report_fatal_error.
Split testing of error and output to defeat redirection race.
Mon, Mar 30
Addressed header file order review comment
Addressed review comments.
Cosmetic test improvements.
Thu, Mar 26
Avoided use of ,unique, assembly feature when not using the integrated assembler.
Wed, Mar 25
Corrected silly mistake.
Mon, Mar 23
Added llvm_unreachable to getSectionPrefixForGlobal()
Fri, Mar 20
Rebased and removed changes to MCContext::renameELFSection().
Thu, Mar 19
Tue, Mar 17
Mon, Mar 16
Addressed review comments and rebased onto master.
Tue, Mar 10
Feb 10 2020
Feb 5 2020
I think that this patch removes the need for uniquing sections for symbols with associated symbols which are explicitly assigned to a section name e.g. via attribute((section name)).
Feb 4 2020
updated the patch and description
Jan 23 2020
I have updated the patch. It now handles both "default" sections and "implicit" sections ("implicit" sections are those that are created by MC normally as it encounters symbols). I have improved the test coverage, organised the tests and improved the comments for the test cases. Sorry it took a while to update the patch.. this was a more difficult change than I had anticipated. I have tried a few different approaches, I am not entirely happy with the current implementation but it is difficult to simply further.
Jan 16 2020
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.
Jan 14 2020
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.
Jan 13 2020
Updated the diff to address review comments and expand test coverage.
Jan 6 2020
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?
Jan 3 2020
Jan 2 2020
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