Please use GitHub pull requests for new patches. Phabricator shutdown timeline

lld/ELF/Config.h
151	They make the offsets of following variables larger. move to the end. `<xxx, 0>` is more common in lld. The inline element doesn't save.
lld/ELF/Driver.cpp
2595	move the check to the caller
2612	No trailing period. Please check the existing convention.
2615	Please check the existing convention.
lld/ELF/InputFiles.cpp
964	move the check to the caller
lld/ELF/SyntheticSections.cpp
363	this needs `<numeric>`
lld/test/ELF/aarch64-feature-pauth.s
4	`%tsplit` makes temporary output filenames unnecessarily long. Just use `RUN: rm -rf %t && split-file %s %t && cd %t` please check the existing convention.
60	We omit messages before `error:` as the executable may have a different name, e.g. `ld.lld.exe`. Please follow the style of some recently added tests.

Fix issues mentioned by @MaskRay

lld/ELF/Config.h
151	Got it, fixed
lld/ELF/Driver.cpp
2595	Done
2612	Fixed, also changed "one. Either" to "one; either" - multiple sentences IMHO don't look nice in such context
lld/ELF/InputFiles.cpp
964	Done
lld/ELF/SyntheticSections.cpp
363	Fixed, thanks! It looks like this file is included implicitly from other files on my machine, so didn't mention it by myself.
lld/test/ELF/aarch64-feature-pauth.s
4	Fixed, thanks, it definitely enhances readability
60	Got it, fixed

The program properties are in the appendix as an alternative. Do we need to implement both for the same platform? If both are supported then we have the problem of what happens when one relocatable object has a notes section and the other a GNU properties section.

The ELF marking in the specification was very much a placeholder in the specification waiting for some implementation experience in prototypes. The notes section was used mostly as GNU properties are a Linux extension, and this may end up being used outside of a GNU platform. I strongly recommend choosing one or the other. Personally I would go with the note section as the properties are listed in an appendix as an alternative.

Experience from BTI has shown that assembler files, particularly those like crt0.s often creep into projects and are hard to find. Many of these assembler files will have no code pointers in them at all so giving a fatal error may be too harsh.

You may want to follow intel CET, which has a -z cet-report = [none|warn|error] for example -z pauth-report = [none|warn|error] which controls what happens when there are some objects without the note section.

I haven't checked through all the details in the implementation.

llvm/tools/llvm-readobj/ELFDumper.cpp
5361	I recommend printAArch64Note as ARM in all caps tends to refer to the ARM (AArch32 backend). Even if the vendor/owner name is ARM the company.

In D158574#4638533, @peter.smith wrote:

The program properties are in the appendix as an alternative. Do we need to implement both for the same platform? If both are supported then we have the problem of what happens when one relocatable object has a notes section and the other a GNU properties section.

The ELF marking in the specification was very much a placeholder in the specification waiting for some implementation experience in prototypes. The notes section was used mostly as GNU properties are a Linux extension, and this may end up being used outside of a GNU platform. I strongly recommend choosing one or the other. Personally I would go with the note section as the properties are listed in an appendix as an alternative.

The implementation proposed in this patch allows only one type of pauth mark at a time: if one of input files has .note.AARCH64-PAUTH-ABI-tag section and another one has GNU_PROPERTY_AARCH64_FEATURE_PAUTH in .note.gnu.property, the linker will emit an error: see lld/test/ELF/aarch64-feature-pauth.s:15:

error: input files contain both a .note.AARCH64-PAUTH-ABI-tag section and a GNU_PROPERTY_AARCH64_FEATURE_PAUTH feature in a .note.gnu.property section; all the input files must use the same way of specifying AArch64 PAuth compatibility info

I think we can leave this behavior since it's conservative enough, but allows some platforms to use an alternative marking way if they want to.
The compatibility check implemented is pretty generic (treat files as compatible if and only if their PAuth notes are identical), and it can be used as a skeleton if some platform wants something more complex.
Please let me know your thoughts on that - as for now, I don't see major downsides in leaving both ways implemented (if it's explicitly disallowed to mix them), but maybe you see some issues with that.

Experience from BTI has shown that assembler files, particularly those like crt0.s often creep into projects and are hard to find. Many of these assembler files will have no code pointers in them at all so giving a fatal error may be too harsh.

You may want to follow intel CET, which has a -z cet-report = [none|warn|error] for example -z pauth-report = [none|warn|error] which controls what happens when there are some objects without the note section.

Thanks! I'll definitely implement that, didn't know that such an option exists. I'll also add the notes manually to assembly files in libunwind though - just in case someone wants to link that with the option set to error or using a linker other than lld.

I am puzzled by the need of both the generic .note.AARCH64-PAUTH-ABI-tag and the GNU-centric .note.gnu.property.
If .note.AARCH64-PAUTH-ABI-tag is preferred, you can push forward .note.AARCH64-PAUTH-ABI-tag adoption and not bother with .note.gnu.property.
Why should the linker bear complexity related to .note.gnu.property and its interaction with .note.AARCH64-PAUTH-ABI-tag (error, but we could avoid this complexity but not support .note.gnu.property in the first place).

If there is not so such GNU hatred, I'd like that we only implement .note.gnu.property and only implement .note.AARCH64-PAUTH-ABI-tag when it is really needed, i.e. some ELF OSes using PAuth somehow uses .note.AARCH64-PAUTH-ABI-tag.
I'm not certain certain ELF OSes are present. The ABI can be written in a more natural way to favor .note.AARCH64-PAUTH-ABI-tag .

lld/ELF/Config.h
435	Variables unrelated to command-line options are generally placed in `Ctx`.
lld/ELF/Driver.cpp
2600	move f1 outside (prefer `[0].` to `front()->`) and use `ArrayRef(ctx.objectFiles).slice(1)`
2605	`^` => `!=`
lld/ELF/InputFiles.cpp
968	It's conventional to call `errorOrWarn` and continue parsing instead of calling `fatal` `reportFatal` can be changed to return `bool` indicating whether the caller should early return.
974	I am not sure the cast is a bad idea. This is the idiomatic way. `Elf_Nhdr` recognizes endianness and has packed data members.
lld/test/ELF/aarch64-feature-pauth.s
5	Since we `cd %t`, all the `%t` prefix of temporary object files can be omitted. We ensure all temporary files are in `%t` now.
6	cp gnu11.o gnu12.o
7	a non-shared object can be named `.so`, but it's confusing. Suggest `-shared`

The way D158574 and D156882 are organized looks awkward to me. I think the preferred way is

patch A: add constants to BinaryFormat and implement llvm-readobj, with tests in llvm/test/tools/llvm-readobj. Please study previous patches adding llvm-readobj support for new constants
patch B: full-fledged lld implementation

lld/ELF/SyntheticSections.cpp
375	The section name convention is `AArch64` instead of `Aarch64`
384	unneeded

This revision now requires changes to proceed.Sun, Sep 17, 6:00 PM

MaskRay mentioned this in D156882: [AArch64][ELF][lld] Support dynamic R_AARCH64_AUTH_* relocations.Sun, Sep 17, 6:17 PM

I agree with MaskRay that for an Alpha specification there is no good reason to support both marking schemes in relocatable objects. I wish I had deleted the alternative one rather than moving it to an appendix. I'll note that the spec covers relocatable objects only, it is the platform's decision about executables and shared objects.

The encoding difference between the processor specific note section and the GNU property is minimal. Essentially the GNU property is just wrapping the contents of the NOTE section in a property. At the relocatable object this doesn't make a lot of difference as the (platform/vendor schema-version) does not fit well with any existing GNU property [*] so it would still need custom code to process. At the executable, shared-object level there is more of a benefit for GNU properties as the program header that identifies the program properties can make it easier for the dynamic linker to find the information. It is possible to use the raw notes in relocatable objects but use a program property in the executable.

Internally to Arm there is a discussion on how to do ELF marking in relocatable objects (not executables and shared-libraries), for example do we continue to use GNU properties or do we use something like Arm (or RISCV) build attributes. I hope that there will be a proposal that can be shared externally for public comment can be put on the Arm ABI github page at some point in the near future. As ABI work tends to move slowly I suspect it will be too late for this though.