Page MenuHomePhabricator

psmith (Peter Smith)
User

Projects

User does not belong to any projects.

User Details

User Since
Sep 17 2015, 9:03 AM (278 w, 2 d)

Recent Activity

Yesterday

psmith added a comment to D94557: [ARM] Fixed incorrect lowering when using GNUEABI (libgcc) and 16bit floats.

As I understand it, this is about what library call should be made when there is an eabi target such as arm-none-eabi which should target the Run Time ABI for the Arm Architecture https://github.com/ARM-software/abi-aa/blob/master/rtabi32/rtabi32.rst and other Arm targets such as Linux or Android. I think if we're arguing from the specification then clang is compliant with the eabi, so relocatable objects produced by clang should be compatible with eabi compliant toolchains that supply the eabi functions, this includes non GNU toolchains as well like IAR.

Fri, Jan 15, 3:35 AM · Restricted Project

Thu, Jan 14

psmith added inline comments to D94612: [LLD][ELF][AArch64] Add support for R_AARCH64_LD64_GOTPAGE_LO15 relocation.
Thu, Jan 14, 9:31 AM · Restricted Project
psmith added a comment to D94612: [LLD][ELF][AArch64] Add support for R_AARCH64_LD64_GOTPAGE_LO15 relocation.

Thanks for working on this. It will be very useful for people using GCC yet using LLD as a linker on AArch64. Will try and take a look later today, if not then I should have some time tomorrow morning.

Thu, Jan 14, 8:10 AM · Restricted Project

Fri, Jan 8

psmith accepted D94280: [ELF] --exclude-libs: localize defined libcall symbols referenced by lto.tmp.

LGTM, may be worth waiting for any other comments.

Fri, Jan 8, 10:33 AM · Restricted Project

Wed, Jan 6

psmith added inline comments to D93948: [MC] Merge section flags for user defined sections.
Wed, Jan 6, 2:30 AM · Restricted Project

Dec 17 2020

psmith added a comment to D93467: [ELF] Rename R_TLS to R_TPREL and R_NEG_TLS to R_TPREL_NEG. NFC.

No objections from me, looks like a sensible change.

Dec 17 2020, 10:29 AM · Restricted Project

Dec 16 2020

psmith added a comment to D93241: [AArch64InstPrinter] Change printADRPLabel to print the target address in hexadecimal form.

The calculation looks good to me. It is logically equivalent to that given in the reference manual

Dec 16 2020, 2:03 AM · Restricted Project

Dec 15 2020

psmith added inline comments to D93241: [AArch64InstPrinter] Change printADRPLabel to print the target address in hexadecimal form.
Dec 15 2020, 2:55 AM · Restricted Project

Dec 10 2020

psmith added a comment to D93045: [ELF] AArch64: Handle DT_AARCH64_VARIANT_PCS.

Thanks for working on this. It looks like it implements the ABI correctly (see PR for links to the specification and GCC/Glibc details).

Dec 10 2020, 9:52 AM · Restricted Project

Nov 24 2020

psmith accepted D91995: [ELF] Rename adjustRelaxExpr to adjustTlsExpr and delete the unused `data` parameter.

Looks good to me. Thanks for tidying this up.

Nov 24 2020, 2:35 AM · Restricted Project
psmith added a comment to D91760: [Driver] Default Generic_GCC aarch64 to use -fasynchronous-unwind-tables.

Radio silence so far; I think no news is good news applies in this case. I'm happy to say no objections.

Nov 24 2020, 2:34 AM · Restricted Project

Nov 19 2020

psmith added a comment to D91760: [Driver] Default Generic_GCC aarch64 to use -fasynchronous-unwind-tables.

Personally I'm in favour of clang and gcc behaving the same way unless there is a good reason not to. I've shared the review internally to see if anyone has any concerns. May be worth informing the clang built linux community to see if they will need to make any alterations as a result.

Nov 19 2020, 6:40 AM · Restricted Project

Nov 17 2020

psmith added a comment to D91579: [ELF] --gc-sections: collect unused .gcc_except_table in section groups and associated text sections.

No objections from me. Although unrelated to the patch, isLSDA might be better called fromFDE, or couldBeLSDA? Maybe one for a later follow up.

Nov 17 2020, 3:13 AM · Restricted Project
psmith accepted D91510: [ELF] Fix interaction between --unresolved-symbols= and --[no-]allow-shlib-undefined.

LGTM, with one suggestion of a name change. I've checked the GNU ld behaviour against the green test changes and LLD matches.

Nov 17 2020, 2:57 AM · Restricted Project

Nov 12 2020

psmith added a comment to D90698: [builtins] Add support for single-precision-only-FPU ARM targets..

Assuming there are no problems with the bots or other downstream builds, please update the PR with a link to this review.

Nov 12 2020, 7:16 AM · Restricted Project
psmith added a comment to D90698: [builtins] Add support for single-precision-only-FPU ARM targets..

Now committed. Commit message has your name and email address.

Nov 12 2020, 7:13 AM · Restricted Project
psmith committed rG0c0eeb78eb0d: [builtins] Add support for single-precision-only-FPU ARM targets. (authored by chaosdefinition).
[builtins] Add support for single-precision-only-FPU ARM targets.
Nov 12 2020, 7:12 AM
psmith closed D90698: [builtins] Add support for single-precision-only-FPU ARM targets..
Nov 12 2020, 7:12 AM · Restricted Project
psmith accepted D91187: [ELF] Make SORT_INIT_PRIORITY support .ctors.N.

Thanks for updating the comments. If the lexical ordering is going to work out the same I don't have any objections.

Nov 12 2020, 3:59 AM · Restricted Project
psmith added a comment to D91127: [ELF] Sort by input order within an input section description.

Thanks for the updates, LGTM too.

Nov 12 2020, 3:54 AM · Restricted Project
psmith added a comment to D90698: [builtins] Add support for single-precision-only-FPU ARM targets..

If there are no more comments, can someone please commit this for me? I don't have commit access to LLVM.

Nov 12 2020, 1:27 AM · Restricted Project

Nov 11 2020

psmith added inline comments to D91127: [ELF] Sort by input order within an input section description.
Nov 11 2020, 3:17 AM · Restricted Project
psmith added a comment to D91187: [ELF] Make SORT_INIT_PRIORITY support .ctors.N.

Will take another look tonight.For reference, I tried helping out someone last year with https://bugs.llvm.org/show_bug.cgi?id=44698 I think they had old objects, or a an old GCC on their system and it was mixing with a modern clang. Maybe worth a read through.

Nov 11 2020, 3:09 AM · Restricted Project

Nov 10 2020

psmith added a comment to D91127: [ELF] Sort by input order within an input section description.

It took a lot of staring at that bit of code to work out what was going on. If I have understood it correctly. In particular I got confused with the sizeBefore and sizeLastSort with sizeBefore being the larger value. It took me a while to spot the continue. If I've understood correctly:

  • sizeBefore is really sizeOfRetBeforeNextSort
  • sizeLastSort is really sizeOfRetAfterPreviousSort

If possible could we use Prev rather than Last as it is more specific. Last can mean the last one we went by, or the last one in the list. I strongly recommend adding something like Next to sizeBefore so that it is clear that sizeBefore >= sizeLastSort.

Nov 10 2020, 3:54 PM · Restricted Project
psmith added a comment to D91180: [ELF] Support multiple SORT in an input section description.

Looks OK to me. Will be best to see if George has any comments about how to do the parsing.

Nov 10 2020, 2:25 PM · Restricted Project
psmith added a comment to D91187: [ELF] Make SORT_INIT_PRIORITY support .ctors.N.

Apologies for being so quiet recently. It looks like this is changing .ctors/.dtors away from SORT_BY_NAME when they aren't mixed with .init_array.

Nov 10 2020, 2:12 PM · Restricted Project

Nov 4 2020

psmith accepted D90698: [builtins] Add support for single-precision-only-FPU ARM targets..

This looks good to me. The __ARM_FP feature test macro is defined in the ACLE and 0x8 is the right flag to check for Double precision support. I've tested at least one single precision only build.

Nov 4 2020, 3:24 AM · Restricted Project

Oct 28 2020

psmith added a comment to D90108: [MC] Error for .globl/.local which change the symbol binding and warn for .weak.

We can surely loose it if it turns out to be a wide spreading problem.

The hard part is that regressions tend to get reported AFTER the release when it's too late to fix them. Maybe a warning for one release, then upgrade to hard error next release gives users more time to find and fix errors, and doesn't result in an unusable release version for anyone?

I don't have any specific evidence to suggest that we'd run into a problem doing this, but my personal thinking is that adding an option to allow disabling this error might be a good idea. I wouldn't be surprised to find code bases where fixing the code is non-trivial. Ideally, the option would not be made widely known unless actually asked for and/or heavily labelled with things saying something like "this is a temporary option that will be deleted in a future change unless a compelling reason is found not to".

If there are concerns, I can make .globl foo; .weak foo a warning instead of error. The others are still errors.

Oct 28 2020, 1:35 PM · Restricted Project
psmith added a comment to D86142: [LLD] Search archives for non-tentative defintions..

I've left a few small suggestions but overall no objections. Given that this is the default BFD behaviour and programs that have only been linked with LLD are not likely to use common; I think that if this behaviour is to be put under an option, it is on by default. This will permit users to use both GNU ld and lld with the same command line.

Oct 28 2020, 1:22 PM · Restricted Project, Restricted Project

Oct 27 2020

psmith added a comment to D90108: [MC] Error for .globl/.local which change the symbol binding and warn for .weak.

If we issue a warning instead, users will only notice the semantic difference when they use -Wa,--fatal-warnings. We can classify the potential risks on whether .local is used:

  • .local is used: .local is a very rare directive.
  • .local is not used: the issue is about .weak and .globl . This represents a semantic difference between MC and GNU ld. There may be a few more but I doubt there can be more than a few. Can we try error first and switch to a warning if it turns out to be a problem?
Oct 27 2020, 1:50 AM · Restricted Project

Oct 26 2020

psmith added a comment to D90108: [MC] Error for .globl/.local which change the symbol binding and warn for .weak.

Personally I'd err on the side of consistency with binutils. If they are happy to go to an error then we can. If they aren't then I recommend that we warn rather than error as we risk breaking builds for working software and it isn't always easy to get source code fixed. Not got a strong opinion though so happy to go with the consensus.

Oct 26 2020, 11:49 AM · Restricted Project

Oct 22 2020

psmith added a comment to D89828: [ELF] Set SHF_INFO_LINK for .rel[a].plt and .rel[a].dyn.

LGTM too. Apologies for the delay. This looks like it is consistent with the ELF specification.

Oct 22 2020, 9:16 AM · Restricted Project

Oct 21 2020

psmith accepted D89841: [ELF] --gc-sections: retain dependent sections of non-SHF_ALLOC sections.

LGTM. If we are marking a section live it makes sense to mark its dependent sections live.

Oct 21 2020, 3:26 AM · Restricted Project

Oct 14 2020

psmith added a comment to D88877: [ELF] Drop --whole-archive before processing dependent libraries..

I don't have any objections. My comment in the thread was more along the lines that dependent libraries could interact with whole-archive in hard to predict ways. I'm happy to have dependent libraries ignore whole-archive. I'm fine with MaskRay accepting when he is happy.

Oct 14 2020, 9:04 AM · Restricted Project, lld

Sep 29 2020

psmith accepted D88478: [llvm-readobj][ARM] - Improve support of printing unwind (-u) information for non-relocatable objects..

Looks good to me. Thanks very much for adding the functionality.

Sep 29 2020, 10:38 AM · Restricted Project

Sep 28 2020

psmith accepted D88407: [llvm-readobj/elf] - Fix the PREL31 relocation computation used for dumping arm32 unwind info (-u)..

LGTM. Agree that it should be bit31 that is sign extended. Apologies for not catching this last time.

Sep 28 2020, 5:47 AM · Restricted Project

Sep 25 2020

psmith added a comment to D88228: [yaml2obj][obj2yaml] - Add support for SHT_ARM_EXIDX section..

What's here looks good to me to.

Sep 25 2020, 10:14 AM · Restricted Project

Sep 24 2020

psmith added a comment to D88228: [yaml2obj][obj2yaml] - Add support for SHT_ARM_EXIDX section..

I'll try and take a look tomorrow.

Sep 24 2020, 12:03 PM · Restricted Project

Sep 23 2020

psmith accepted D88076: [llvm-readelf/obj] - Stop printing wrong addresses for arm32 unwind info for non-relocatable objects..

Thanks for the update. LGTM. Happy to see other parts done in follow up commits

Sep 23 2020, 10:29 AM · Restricted Project

Sep 22 2020

psmith added a comment to D88076: [llvm-readelf/obj] - Stop printing wrong addresses for arm32 unwind info for non-relocatable objects..

I think there may still be problems in a non-relocatable ELF file when there is more than one executable output section as there is only one .ARM.exidx output section and only one sh_link.

Sep 22 2020, 11:45 AM · Restricted Project

Sep 21 2020

psmith accepted D88037: [ELF][test] Delete large temporary files and make some temporary files smaller with two text segments.

Looks good to me. Thanks for cleaning this up, it will be of great help to 32-bit bots.

Sep 21 2020, 11:27 AM · Restricted Project

Sep 17 2020

psmith added a comment to D87824: [lld][ELF][test] Add additional LTO testing.

LGTM too, thanks for adding the tests.

Sep 17 2020, 7:43 AM · Restricted Project

Sep 9 2020

psmith added a comment to D86762: [ELF] Add documentation for --warn-backrefs: a GNU ld compatibility checking tool (and lesser of layering detection).

Thanks for the update I'm happy with the edits.

Sep 9 2020, 12:47 PM · Restricted Project
psmith added a comment to D87121: LLD symbol ordering file binary name tag.

Will have to have a think. My initial reaction to "apply this file only when these conditions hold" is akin to moving part of the build system into the linker. It could be done, but I'm not sure it should be. For example if there were a linker script it would have to precisely match the inputs to make sense.

Sep 9 2020, 1:50 AM

Sep 4 2020

psmith added a comment to D86762: [ELF] Add documentation for --warn-backrefs: a GNU ld compatibility checking tool (and lesser of layering detection).

I've put some comments, mostly concentrating on the early part. I've not had a chance to go through the examples at the bottom in detail. I can't spot anything obviously wrong though.

Sep 4 2020, 3:14 AM · Restricted Project

Sep 2 2020

psmith added a comment to D69411: [MC] Resolve the difference of symbols in consecutive MCDataFragements.

I don't have any objections to the approval. Thanks for updating the comment.

Sep 2 2020, 12:40 PM · Restricted Project

Sep 1 2020

psmith added a comment to D69411: [MC] Resolve the difference of symbols in consecutive MCDataFragements.

Thanks for the update and sorry to take so long to comment. I can't see anything immediately wrong and I think limiting this to fragments in the same subsection makes sense.

Sep 1 2020, 2:47 PM · Restricted Project
psmith added a comment to D86762: [ELF] Add documentation for --warn-backrefs: a GNU ld compatibility checking tool (and lesser of layering detection).

I had a look in Levine's Linkers and Loaders book to see if I could find any source for back-references, all I could find was in 6.4 "Not all linkers do this; many just make a single sequential pass over the directory and miss any backwards depedencies from a file to another file earlier in the library. Tools like tsort and lorder can minimize the difficulty of using single-pass linkers but it's not uncommon for programmers to explicitly list the same library several times on the linker command line to force multiple passes and thus finally resolve all symbols."

Sep 1 2020, 1:01 AM · Restricted Project

Aug 26 2020

psmith added a comment to D86579: Add lld/ELF release notes for release/11.x.

Thanks for putting this together, looks good. I've added a few suggestions, and some typo spots.

Aug 26 2020, 10:51 AM · Restricted Project

Aug 25 2020

psmith added a comment to D86422: [ELF] .note.gnu.property: error for invalid pr_datasize.

LGTM too, I'm happy my comments were addressed, thanks.

Aug 25 2020, 3:35 AM · Restricted Project

Aug 24 2020

psmith added a comment to D86422: [ELF] .note.gnu.property: error for invalid pr_datasize.

Overall looking good. I think there may be an endianness problem with reinterpret_cast although I could easily be wrong without testing (I don't think we have a target that uses .note.gnu.property with big-endian support in LLD).

Aug 24 2020, 1:06 AM · Restricted Project

Aug 20 2020

psmith added a comment to D86263: [ELF] Keep st_type for symbol assignment.

Had a chance to look over lunch. Thanks looks good to me too.

Aug 20 2020, 5:02 AM · Restricted Project
psmith added a comment to D86263: [ELF] Keep st_type for symbol assignment.

Not had a chance to look into the changes in detail yet, but on first glance it looks good. Given that the compiler uses STT_FUNC for the entry of a symbol I think it is reasonable to retain the type only for an alias.

Aug 20 2020, 2:42 AM · Restricted Project

Aug 19 2020

psmith added a comment to D86142: [LLD] Search archives for non-tentative defintions..

In ELF the last paragraph of the section on STT_COMMON only mentions that a dynamic linker may change its resolution rules to prefer non-common over a common definition. I'm guessing extending this to static linker is something that some other linkers have done and there are some programs that currently depend on that behaviour.

Symbols with type STT_COMMON label uninitialized common blocks. In relocatable objects, these symbols are not allocated and must have the special section index SHN_COMMON (see below). In shared objects and executables these symbols must be allocated to some section in the defining object.
Aug 19 2020, 1:23 AM · Restricted Project, Restricted Project

Aug 14 2020

psmith accepted D84001: [ELF] Allow mixed SHF_LINK_ORDER & non-SHF_LINK_ORDER sections and sort within InputSectionDescription.

Given that it is possible to write a linker script that does work with *(pattern1 pattern2) then I don't have any objections. I've tentatively approved as it looks like was the most cautious here.

Aug 14 2020, 5:47 AM · Restricted Project
psmith accepted D85867: [ELF] Assign file offsets of non-SHF_ALLOC after SHF_ALLOC and set sh_addr=0 to non-SHF_ALLOC.

Thanks for the update. I'm happy with the changes. I've tentatively approved, James/Edd please shout if you have any objections?

Aug 14 2020, 5:38 AM · Restricted Project
psmith added a comment to D85056: [ELF] Add --keep-section to expose linkerscript KEEP directive as a linker flag.

I do have some sympathy with wanting to use a command line option to keep an individual section from the command line as it has been useful in Arm's proprietary linker, although this is mainly due to the convenience of not having to create or modify another file.

One observation I'd make about the proposed implementation is that it looks like it only implements a subset of the Linker Script KEEP command that supports precise matches of a section name. Quoting from the GNU linker manual:

When link-time garbage collection is in use (`--gc-sections'), it is often useful to mark sections that should not be eliminated. This is accomplished by surrounding an input section's wildcard entry with KEEP(), as in KEEP(*(.init)) or KEEP(SORT_BY_NAME(*)(.ctors)).

That permits the full power of the input section description to discriminate via object, as there can be many sections with the same name. Wildcards can sometimes be useful too.

I think it would be worth considering a richer interface for keep-sections. In Arm's proprietary linker we permitted a similar syntax as for the equivalent linker script. This did mean quoting or escaping parentheses but did provide equivalence in what could be achieved.

Having a non-INSERT-AFTER/BEFORE SECTIONS command is considered an external linker script and can change the default layout decisions. I can also feel sympathy with the users but I am also wary of adding these non-orthogonal features. The recent D76482 (__build_id_start = .) and this patch make me think of output section descriptions (a fragment of a SECTIONS command) which do not affect section layout.

@psmith @grimar I think we probably should start a conversation with binutils about such a feature. If they find needs as well, we will have a common ground, it'd be great. They need to be given the decision making opportunity to reduce the risk they create a similar but incompatible feature in the future.

One idea is:

OVERRIDE SECTIONS {
  .foo : { KEEP(*(.foo)) }
  .bar : { KEEP(*(.bar)) }
  sym = .;   // symbol assignments are disallowed
}

The output section descriptions will override .foo & .bar in the external linker script. If the external linker script does not describe .foo or .bar, the command will change the orphan sections.

Another syntax:

SECTIONS {
  .foo : { KEEP(*(.foo)) }
  .bar : { KEEP(*(.bar)) }
} REPLACE .foo;
Aug 14 2020, 5:28 AM · Restricted Project

Aug 13 2020

psmith added a comment to D85867: [ELF] Assign file offsets of non-SHF_ALLOC after SHF_ALLOC and set sh_addr=0 to non-SHF_ALLOC.

Just to check I understand.

Aug 13 2020, 12:49 AM · Restricted Project

Aug 12 2020

psmith added a comment to D85785: [ELF] -r: allow SHT_X86_64_UNWIND to be merged into SHT_PROGBITS.

I think this should be ok for .ARM.exidx sections as they have a SHF_LINK_ORDER dependency which prevents them from being merged in relocatable links.

Aug 12 2020, 1:32 AM · Restricted Project

Aug 11 2020

psmith accepted D85651: [LLD][ELF] - Do not produce an invalid dynamic relocation order with --shuffle-sections..

Looks good to me. Will be worth seeing if MaskRay has any comments.

Aug 11 2020, 1:09 AM · Restricted Project

Aug 10 2020

psmith added a comment to D85618: [ELF] Move the outSecOff addend from relocAlloc/relocNonAlloc/... to InputSectionBase::relocate.

Thanks for the update, I'm happy when George is.

Aug 10 2020, 10:02 AM · Restricted Project
psmith accepted D85661: [ELF] Avoid creating a 2.1GB output file in arm-exidx-range.s.

Thanks for spotting that. I'd personally prefer to fix in the linker script, rather than make the code RW but I don't have a strong preference.

Aug 10 2020, 10:00 AM · Restricted Project
psmith added a comment to D85618: [ELF] Move the outSecOff addend from relocAlloc/relocNonAlloc/... to InputSectionBase::relocate.

Simplification looks good to me. I'm not sure about the test changes as I would have expected the change to be NFC. If these are test improvements it will be worth altering separately.

Aug 10 2020, 12:59 AM · Restricted Project

Aug 6 2020

psmith added a comment to D85239: [DOCS] Add more detail to stack protector documentation.

I agree it's not what I'd like. I'm not sure how to react to MaskRays comment. The top of the ClangCommandLineReference.rst says:

-------------------------------------------------------------------
NOTE: This file is automatically generated by running clang-tblgen
-gen-opt-docs. Do not edit this file by hand!!
-------------------------------------------------------------------

This uses Options.td to generate the file. I don't know how true this is anymore given that both files are checked in. They may have been separated. I've seen at least one instance in the log where the whole file has been regenerated.

Aug 6 2020, 11:27 AM · Restricted Project
psmith committed rG839d974ee0e4: [DOCS] Add more detail to stack protector documentation (authored by psmith).
[DOCS] Add more detail to stack protector documentation
Aug 6 2020, 5:59 AM
psmith closed D85239: [DOCS] Add more detail to stack protector documentation.
Aug 6 2020, 5:59 AM · Restricted Project

Aug 5 2020

psmith abandoned D75347: [MC][ELF][ARM] Add relocations for some ARM pc-relative fixups..

Yes. I think the consensus was to match the GCC behaviour, I'll abandon the revision.

Aug 5 2020, 8:20 AM · Restricted Project
psmith added a comment to D85100: [ELF] Allow sections after a non-SHF_ALLOC section to be covered by PT_LOAD.

FWIW this looks good to me too.

Aug 5 2020, 8:19 AM · Restricted Project, Restricted Project
psmith added a comment to D85086: [ELF] --oformat=binary: use LMA to compute file offsets.

No objections from me either. I guess LGTM if George's last comment can be resolved.

Aug 5 2020, 8:08 AM · Restricted Project
psmith added inline comments to D85239: [DOCS] Add more detail to stack protector documentation.
Aug 5 2020, 4:57 AM · Restricted Project
psmith updated the diff for D85239: [DOCS] Add more detail to stack protector documentation.

uploaded diff with both Options.td and ClangCommandLineReference.rst

Aug 5 2020, 4:57 AM · Restricted Project
psmith added a comment to D85239: [DOCS] Add more detail to stack protector documentation.

This file is auto generated. The real documentation should go to clang/include/clang/Driver/Options.td

Aug 5 2020, 4:56 AM · Restricted Project
psmith added a comment to D84610: [ELF] --icf: don't fold text sections with LSDA.

I agree I think it is best to err on the side of correctness. As I understand it we could do better (merge more sections) but this would involve pulling apart .eh_frame sections to show equivalence. I think that could come later.

Aug 5 2020, 1:17 AM · Restricted Project
psmith added a comment to D84825: [ELF] Change tombstone values to (.debug_ranges/.debug_loc) 1 and (other .debug_*) 0.

My apologies for not commenting earlier. I haven't had a lot of spare time and I only have a weak opinion, certainly far weaker than the other commenters.

Aug 5 2020, 1:12 AM · Restricted Project

Aug 4 2020

psmith requested review of D85239: [DOCS] Add more detail to stack protector documentation.
Aug 4 2020, 12:34 PM · Restricted Project
psmith added a comment to D85086: [ELF] --oformat=binary: use LMA to compute file offsets.

I agree with MaskRay that if we do support the feature then we should make sure it works or document its limitations, even if our limitations are "Implemented to the degree that it supports the FreeBSD Kernel, we recommend producing an ELF file and using llvm-objcopy --output-target=binary."

Aug 4 2020, 1:09 AM · Restricted Project

Aug 1 2020

psmith added a comment to D85056: [ELF] Add --keep-section to expose linkerscript KEEP directive as a linker flag.

I do have some sympathy with wanting to use a command line option to keep an individual section from the command line as it has been useful in Arm's proprietary linker, although this is mainly due to the convenience of not having to create or modify another file.

Aug 1 2020, 5:30 AM · Restricted Project

Jul 31 2020

psmith added a comment to D76482: [lld][ELF] Provide optional hidden symbols for build ID.

It is a pity that the section name is not a valid C identifier otherwise start_ and stop_ would have been generated automatically.

Jul 31 2020, 5:50 AM · Restricted Project

Jul 29 2020

psmith added a comment to D82014: [compiler-rt] Replaced __SOFT_FP__ with __SOFTFP__.

Ping @kamleshbhalui

I hope your or some reviewer can test the buildability on ARM.

Jul 29 2020, 9:47 AM · Restricted Project, Restricted Project

Jul 20 2020

psmith accepted D84129: [ELF] -r: rewrite SHT_GROUP content if some members are combined or discarded.

This looks good to me. Regardless of whether we allow --gc-sections it will be worth getting this right for /DISCARD/. Will be worth waiting to see if there are any further comments from other reviewers.

Jul 20 2020, 2:58 AM · Restricted Project
psmith added a comment to D84131: [ELF] Support -r --gc-sections.

I guess if the use case for -r is a kernel module then garbage collection can make sense as the "object" is not relinked. What happens when there is no GC root? I think it will be worth detecting that and turning GC off as otherwise everything will get removed which is unlikely to be what was wanted.

Jul 20 2020, 2:48 AM · Restricted Project

Jul 18 2020

psmith added a comment to D72904: [ELF] Allow SHF_LINK_ORDER sections to have sh_link=0.

Whilst the actual change seems fine, in that it does what it sets out to do, I believe, I'm struggling to follow why a SHF_LINK_ORDER section should have a sh_link of 0 at all. Surely the section should just then not have SHF_LINK_ORDER? What sections actually have sh_link=0 and SHF_LINK_ORDER?

Jul 18 2020, 1:58 PM · Restricted Project
psmith added inline comments to D84001: [ELF] Allow mixed SHF_LINK_ORDER & non-SHF_LINK_ORDER sections and sort within InputSectionDescription.
Jul 18 2020, 1:52 PM · Restricted Project

Jul 17 2020

psmith added a comment to D84001: [ELF] Allow mixed SHF_LINK_ORDER & non-SHF_LINK_ORDER sections and sort within InputSectionDescription.

I'll take a look over the weekend, also at D72904, apologies won't be able to get to it today.

Jul 17 2020, 12:41 AM · Restricted Project

Jul 13 2020

psmith added a comment to D83655: [AsmPrinter] Split up .gcc_except_table.

What would happen if there was a single .text section containing multiple functions, would that produce multiple .gcc_except_table sections each with a SHF_LINK_ORDER dependency on the single .text section? I think that this is OK as there are no other sections that refer to .gcc_except_table and there doesn't need to be a total order. Ideally we could have one .gcc_except_table section per associated .text section.

Jul 13 2020, 1:04 PM · Restricted Project

Jul 8 2020

psmith added inline comments to D83243: [ELF] Rename canRelax to toExecRelax. NFC.
Jul 8 2020, 12:59 AM · Restricted Project

Jul 7 2020

psmith added a comment to D83264: [ELF] Add -z dead-reloc-in-nonalloc=<section_glob>=<value>.

I think an option makes sense. I have a small reservation about making users order their matches on the command line. We should make it more explicit in the help and the manual, or try and sort the matches using a heuristic order of specificity, for example no wildcards is more specific than wildcards, longer text is more specific than shorter. Other than that I think it looks OK.

Jul 7 2020, 2:09 AM · Restricted Project
psmith accepted D83243: [ELF] Rename canRelax to toExecRelax. NFC.

LGTM it looks like a good simplification. Worth waiting a little bit to see if there are any objections.

Jul 7 2020, 1:33 AM · Restricted Project

Jul 6 2020

psmith accepted D83138: [ELF][ARM] Represent R_ARM_LDO32 as R_DTPREL instead of R_ABS.

LGTM, it looks like it is difficult to generate local dynamic from clang, although it is possible with GCC. I was able to make a test application, that also had the advantage of working for shared libraries and PIE as R_ABS does not in that case.

Jul 6 2020, 2:06 AM · Restricted Project

Jul 3 2020

psmith added a comment to D83138: [ELF][ARM] Represent R_ARM_LDO32 as R_DTPREL instead of R_ABS.

Thanks, will take a look over the weekend. Would like to make sure all is well running some TLS test cases on a native Arm machine, fully expect it to be OK.

Jul 3 2020, 10:44 AM · Restricted Project
psmith added a comment to D82899: [ELF] Resolve R_DTPREL in .debug_* referencing discarded symbols to -1.

I'm ok with this as is. I'll need to check over what should be happening on Arm, but I think that can be done in a separate change.

Jul 3 2020, 5:53 AM · Restricted Project

Jul 1 2020

psmith added inline comments to D82899: [ELF] Resolve R_DTPREL in .debug_* referencing discarded symbols to -1.
Jul 1 2020, 2:04 PM · Restricted Project
psmith added a comment to D82567: [LLD][ELF][Windows] Allow LLD to overwrite existing output files that are in use.

This looks OK to me, happy for George to approve . I'm not a Windows filesystem expert by any means, it seems like the rename then delete follows the pattern in https://boostgsoc13.github.io/boost.afio/doc/html/afio/FAQ/deleting_open_files.html though.

Jul 1 2020, 1:32 PM · Restricted Project

Jun 24 2020

psmith accepted D82433: [ELF] -r: don't parse @ (symbol versioning) for .symver inline asm in bitcode.

Looks good to me. Agree that symbol versioning should be passed through in a relocatable link.

Jun 24 2020, 1:02 AM · Restricted Project

Jun 23 2020

psmith added a comment to D81184: [lld][ELF][AArch64] Handle R_AARCH64_PLT32 relocation.

Thanks for the update, I don't have any more comments. Happy when MaskRay is happy.

Jun 23 2020, 1:59 PM · Restricted Project

Jun 19 2020

psmith added inline comments to D70326: [docs] LLVM Security Group and Process.
Jun 19 2020, 2:07 AM · Restricted Project

Jun 18 2020

psmith added a comment to D82014: [compiler-rt] Replaced __SOFT_FP__ with __SOFTFP__.

I think that this is highly likely to be a typo, it looks like it dates to the contribution of compiler-rt so there isn't any history we can use. I believe that compiler-rt builtins originated in Apple so there is a possibility that SOFTFP was used in the compiler at contribution time. If you wanted to be safe you could support both SOFTFP and SOFT_FP but I don't think that is necessary. I would like to think that for a soft-fp target the tests for these functions fail or are at least xfailed at the moment. Are you able to compile the tests for a soft-fp target to check?

Jun 18 2020, 11:26 AM · Restricted Project, Restricted Project

Jun 16 2020

psmith added a comment to D81784: [ELF] Resolve relocations in .debug_* referencing (discarded symbols or ICF folded section symbols) to tombstone values.

I only partially followed the discussion on llvm-dev. The resolution sounds sensible to me. Will be good to get someone involved in the upstream discussion to check it over.

Jun 16 2020, 12:39 PM · Restricted Project

Jun 12 2020

psmith added a comment to D81184: [lld][ELF][AArch64] Handle R_AARCH64_PLT32 relocation.

Looking good so far. Would be good to add a range extension thunk test case. For example:

 .section .text.1, "ax", %progbits
 .global _start
 .type _start, %function
_start: .word callee@PLT - .
Jun 12 2020, 6:26 AM · Restricted Project

Jun 10 2020

psmith committed rG60f5b0ec7ce2: [ELF][AArch64] Correct relocation codes for R_<CLS>_PLT32 (authored by psmith).
[ELF][AArch64] Correct relocation codes for R_<CLS>_PLT32
Jun 10 2020, 6:34 AM