Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[RFC] Defining an ELF attribute for RISC-V target profiles #409

Closed
wants to merge 1 commit into from

Conversation

cmuellner
Copy link
Collaborator

This PR defines an ELF attribute which allows tools to set a RISC-V target profile.

We already have a few ratified RISC-V profiles, but so far, they are treated as "pretty names" (or non-canonical abbreviations) for an ISA base plus a set of extensions.

Further, most of the extensions of a profile are irrelevant for toolchain components (e.g., supervisor mode extensions). Thus, it is not clear how to decompose profiles into a tools-compatible arch string, that allows to reliably reconstruct the profile that a piece of software was written for (i.e., the profile that the execution environment is expected to implement).
Although this could be solved by implementing "dummy" support for all RISC-V extensions in the tools, but does not feel like a good idea.

This commit attempts to resolve this situation by defining a new attribute that allows storing the target profile.

The string representation is used to ensure a future-proof encoding. This should not be a performance issue since profile names are typically very short (less than 10 characters).

Once the target profile is available, the question about the use of this information comes up.
In this PR, a few hints for linker and run-times are provided, without mandating anything.

The PR has been marked as "RFC", as I'd like to hear not only comments about this particular proposal but also general comments about how to improve the RISC-V profile adoption in the SW ecosystem.

This commit defines an ELF attribute which allows tools to set
a RISC-V target profile.

We already have a few ratified RISC-V profiles, but so far they are treated
as "pretty names" (or non-canonical abbreviations) for an ISA base plus a
set of extensions.

Further, most of the extensions of a profile are irrelevant for toolchain
components (e.g. supervisor mode extensions) and thus it is not clear how
to decompose profiles into an tools-compatible arch string, that allows to
reliably reconstruct the profile that a piece of software was written for
(i.e., the profile that the execution environment is expected to implement).

This commit attempts to resolve this situation by defining a new attribute
that allows to store the target profile.

The string representation is used to ensure a future-proof encoding.
This should not be a performance issue, since profile names are
typically very short (less than 10 characters).

This commit also provides a few hints for linkers and run-times, but does
not mandate anything.

Signed-off-by: Christoph Müllner <[email protected]>
@kito-cheng
Copy link
Collaborator

One typical RISC-V profile question is came to my mind again: what if we have enable vendor extension? e.g. RVA22U64+xfoo, does it conformance with RVA22U64?

If it conformance it should means we can run the binary on any platform which is RVA22U64 conformance, but it's not true since not every core has implement xfoo extension, however it could claim conformance IF those functions which use xfoo extension are always guarded by some predictor/check.

If it not conformance...okay, that's easier, that means we should not set this attribute.

@cmuellner
Copy link
Collaborator Author

One typical RISC-V profile question is came to my mind again: what if we have enable vendor extension? e.g. RVA22U64+xfoo, does it conformance with RVA22U64?

If it conformance it should means we can run the binary on any platform which is RVA22U64 conformance, but it's not true since not every core has implement xfoo extension, however it could claim conformance IF those functions which use xfoo extension are always guarded by some predictor/check.

If it not conformance...okay, that's easier, that means we should not set this attribute.

The profile spec uses the term non-profile extensions for all extensions that are neither mandatory nor optional in a profile. This description also covers all vendor extensions, and I think treating vendor extensions the same way as any other non-profile extension is the right thing to do.

So, yes, if a vendor extension (or any other non-profile extension) is part of the arch string, then the ELF cannot be compatible with a profile. However, as mentioned in the PR, there is still the option to add optional support for these extensions, but appropriate mechanisms must be in place to guarantee that no errors or warnings are reported if such extensions are not present in the execution environment. The extension would not be part of the arch string in this case.

To make this possible, we probably need something like .option arch on the linker level. I.e., a merge policy that allows the linking of ELF objects without building a superset of all extensions in all ELF objects. But such a mechanism would be desirable anyway (independent of profiles support).

@kito-cheng
Copy link
Collaborator

Thanks for your clarification, one question come to my mind is: do we really need -march=<profile-name>[+<ext>]*? because add every single non-profile extensions will made the object become non-conformance, so maybe we should only need -march=<profile-name>.

I guess this should also discuss in riscv-non-isa/riscv-toolchain-conventions#36, but anyway, let us continue the discussion here first :P

@cmuellner
Copy link
Collaborator Author

Thanks for your clarification, one question come to my mind is: do we really need -march=<profile-name>[+<ext>]*? because add every single non-profile extensions will made the object become non-conformance, so maybe we should only need -march=<profile-name>.

I guess this should also discuss in riscv-non-isa/riscv-toolchain-conventions#36, but anyway, let us continue the discussion here first :P

Using profiles as pretty names (that expand to a list of extensions) in an march string is reasonable syntactic sugar. However, <profile>_<non-profile ext> can certainly not result in Tag_RISCV_profile being set to <profile>.
So, we don't need it, but we might still want it (but there are valid arguments for and against it).

A related question is how to set the profiles in the tools (e.g. -march=<profile> or -mprofile=<profile>). Initially, I preferred to reuse -march a while ago (as outlined in riscv-non-isa/riscv-toolchain-conventions#26 and as proposed in riscv-non-isa/riscv-toolchain-conventions#36), but I don't have any preferences anymore (again, there are valid arguments for and against each mechanism).

A proposal for implementing the Tag_RISCV_profile support could be:

  • Set Tag_RISCV_profile to a profile A, which is defined in an march string, if the march string only consists of profile A and mandatory extensions of profile A.
  • If optional or non-profile extensions are enabled along with a profile in an march string, then the profile is expanded into all its mandatory extensions, which are supported by the compiler.

@rwmjones
Copy link

rwmjones commented Dec 7, 2023

Just want to add a note here that the existing C flag (EF_RISCV_RVC) in ELF files is effectively useless, as it only really tells you how the toolchain was configured (and is in some instances flat-out wrong). I wrote more about how it doesn't work well here: https://github.com/rwmjones/rhrq-riscv-extensions/blob/main/compressed.md#gas-gnu-assembler-syntax

Is the purpose of this ELF attribute to be:

  • How the toolchain was configured
  • How the compiler/assembler was configured on the command line
  • What the minimum possible profile that could run this binary be
  • Reflecting instructions actually used by the binary
  • Something else?

I think that ought to be clearer to avoid the mistakes made with the existing flag.

Edit: Another question - is the attribute informational? Or is it intended that the linker or loader actually does something with the flag, such as [examples] refusing to link different objects in some circumstances, or refusing to load a binary that doesn't conform to a profile?

@jakubjelinek
Copy link

I'd like to say that while marking ABI changing options that way is perhaps useful, for ABI compatible ones it can cause much more harm than the benefits.
E.g. Solaris as/ld like to mark ELF objects/binaries/libraries with hwcaps
of what instructions it actually uses.
That works nicely if the whole binary or library is compiled with the same
options.
But as soon as those options are changed per compilation unit or on a per
function bases, this becomes complete nightmare, because it says the binary
or library can only run on CPU with xyz capabilities, even when that is
actually not true. Any time one uses e.g. GCC target attribute/pragma and
either manual runtime decision what code to use or some toolchain support
for it (multi-versioning, declare simd, ifuncs), such markings stand in the
way. You can look at how e.g. gcc itself needs to cancel this through
config/hwcaps.m4 and -mclear-hwcap in various places.

Just a recent example is when I've added hw accelerated SHA1 support
to libiberty/GNU binutils bfd linker:
https://sourceware.org/pipermail/binutils/2023-November/130820.html
That is an example of manual runtime selection, code asks CPU if it supports
needed ISAs and uses one of the (in this case) two possible implementations
depending on that. This change needed follow-ups for Solaris
https://gcc.gnu.org/pipermail/gcc-patches/2023-November/638629.html
So, it would be really great if RISC V didn't follow the Solaris lead and didn't impose
such mess upon users.

@cmuellner
Copy link
Collaborator Author

Just want to add a note here that the existing C flag (EF_RISCV_RVC) in ELF files is effectively useless, as it only really tells you how the toolchain was configured (and is in some instances flat-out wrong). I wrote more about how it doesn't work well here: https://github.com/rwmjones/rhrq-riscv-extensions/blob/main/compressed.md#gas-gnu-assembler-syntax

I don't think that EF_RISCV_RVC is useless. It is a necessary flag that is set if at least one piece of the ELF file needs compressed instructions, which has an impact on the ABI (e.g. to allow 16-bit jump targets).

Also, EF_RISCV_RVC has a different meaning than "c (or zca) is in the Tag_RISCV_arch string". If you compile with -march=rv64g but use .option arch,+c/.option rc in assembly code, then EF_RISCV_RVC will be set but c won't be part of Tag_RISCV_arch.

Further, there are two observations which are valid for all extensions (not only C):

  • enabling an extension in the march string does not imply that instructions of this extension will be part of the ELF code
  • not enabling an extension in the march string does not imply that instruction of this extension will not be part of the ELF code

The first observation should be obvious (if there is no opportunity to use an available instruction, then it can't be used). The second observation comes from override mechanisms (like .option push), where the programmer provides the guarantee not to violate the requirements stated in Tag_RISCV_arch (e.g. by using mechanisms like hwprobe()).

Is the purpose of this ELF attribute to be:

  • How the toolchain was configured

No. Toolchain configuration is only used to set defaults (i.e., to define a baseline).
Distros can then use scripts to enforce that deviations from the defaults will be justified (e.g. by scanning compiler arguments in RPM spec files).

  • How the compiler/assembler was configured on the command line

Yes, the build process documents the intended environment for which this ELF was built.
This information is gathered from defaults (toolchain configuration) and optional overrides on the command line.

  • What the minimum possible profile that could run this binary be

This goes beyond this proposal: minimum implies that we can order profiles, which is out of scope for the PR.
It is the job of the execution environment to make a meaningful interpretation of this information.
A simple meaningful interpretation would be to check if the ELF's target profile matches the execution environment's profile.
More complex execution environments could possibly handle multiple profiles.

  • Reflecting instructions actually used by the binary

That's impossible to say:

  • There is more than one execution path in any non-trivial binary
  • There are run-time aspects that influence the execution path (e.g. hwprobe()+ifunc)
  • There are run-time code generation aspects that influence the executed instructions (e.g. hwprobe()+JIT compiler)

I think that ought to be clearer to avoid the mistakes made with the existing flag.

I'm not sure what the mistake was in your opinion.
What would you expect from the flag?

@rwmjones
Copy link

rwmjones commented Dec 7, 2023

I don't think that EF_RISCV_RVC is useless. It is a necessary flag that is set if at least one piece of the ELF file needs compressed instructions, which has an impact on the ABI (e.g. to allow 16-bit jump targets).

That's definitely the theory, but the practical reality is that it doesn't work that way. See the link. (This may be a reason to fix the implementation, but my point is really that we need to very precisely define the meaning of the new attribute, how it is used, and what it is for to avoid implementations being broken as they are for RVC right now).

Re ifuncs etc.

What does the profile attribute mean for code which is using ifuncs? Say that code does use hwprobe and has an optional vector path. The compiler would probably automatically mark the ELF library as "RVA23" (where I assume V will be mandatory). But it would perfectly well run on RVA22 (where V is optional). Should the runtime reject the program marked "RVA23" even though it could run? Should the toolchain be smart and detect ifuncs (and how?). Would we require library writers to hand-annotate such libraries?

It is the job of the execution environment to make a meaningful interpretation of this information.

That seems vague. I guess in practice we'd use it for informational purposes. (Edit: To be clear I'm not arguing against this proposal, an informational attribute could be useful, but saying that it could be more useful if we had a clearer idea what it was for, how it would be set, how it would propagate through linking, and who if anyone consumes it)

@kito-cheng
Copy link
Collaborator

@jakubjelinek thanks for sharing the story from Solaris, RISC-V did have record the arch string within ELF object files, and I think so far we have enough stuffs to prevent that become useless, we have .option arch to enable ISA extension for specific code region without changing the arch string attribute for the file, also our target attribute already leverage that.

e.g.

A source code compile with rv64gc, and there is a function come with target attribute to enable vector, then it still mark as rv64gc rather than rv64gcv.

// foo won't change the file scope attribute.
int foo () __attribute__(target("arch=+v")){
 ...
 /*
  The code gen for foo will be some thing like this:
   .option push
   .option arch, +v
  ...
   .option pop
 */
}

int main() {
   if (vector_is_avaliable)
      foo()
}

@asb
Copy link
Collaborator

asb commented Dec 7, 2023

That's definitely the theory, but the practical reality is that it doesn't work that way. See the link. (This may be a reason to fix the implementation, but my point is really that we need to very precisely define the meaning of the new attribute, how it is used, and what it is for to avoid implementations being broken as they are for RVC right now).

I do agree that per-file scoped attributes can have unclear or confusing semantics in the presence of functions compiled for different attributes. I'm not sure I quite follow your concern about the RVC flag in general though. Per the documentation:

This bit is set when the binary targets the C ABI, which allows instructions to be aligned to 16-bit boundaries (the base RV32 and RV64 ISAs only allow 32-bit instruction alignment). When linking objects which specify EF_RISCV_RVC, the linker is permitted to use RVC instructions such as C.JAL in the linker relaxation process.

That seems consistent with the behaviour you note. As with all the attributes, I can see that you may additionally want to know if any 16-bit aligned instructions were used (or similarly for other ISA extensions - if any instructions from that extension were actually used). But that's outside of scope of what we have today.

@asb
Copy link
Collaborator

asb commented Dec 7, 2023

Thanks for your clarification, one question come to my mind is: do we really need -march=<profile-name>[+<ext>]*? because add every single non-profile extensions will made the object become non-conformance, so maybe we should only need -march=<profile-name>.
I guess this should also discuss in riscv-non-isa/riscv-toolchain-conventions#36, but anyway, let us continue the discussion here first :P

Using profiles as pretty names (that expand to a list of extensions) in an march string is reasonable syntactic sugar. However, <profile>_<non-profile ext> can certainly not result in Tag_RISCV_profile being set to <profile>. So, we don't need it, but we might still want it (but there are valid arguments for and against it).

A related question is how to set the profiles in the tools (e.g. -march=<profile> or -mprofile=<profile>). Initially, I preferred to reuse -march a while ago (as outlined in riscv-non-isa/riscv-toolchain-conventions#26 and as proposed in riscv-non-isa/riscv-toolchain-conventions#36), but I don't have any preferences anymore (again, there are valid arguments for and against each mechanism).

A proposal for implementing the Tag_RISCV_profile support could be:

* Set `Tag_RISCV_profile` to a profile A, which is defined in an march string, if the march string only consists of profile A and mandatory extensions of profile A.

* If optional or non-profile extensions are enabled along with a profile in an march string, then the profile is expanded into all its mandatory extensions, which are supported by the compiler.

Yes, I was going to feedback that extensions defined as optional in the profile is the big question mark here. If an extension is unconditionally enabled that's defined as optional in the profile, meaning the binary might not run on all RVANN profile systems is there any value in trying to set the profile tag at all? Perhaps it should just be reserved for when targeting the baseline profile. In the common case (i.e. outside of the OS kernel) the S* extensions aren't going to make a difference, so perhaps there's an argument for ignoring those when determinig if the profile tag can be set.

@cmuellner
Copy link
Collaborator Author

I'd like to say that while marking ABI changing options that way is perhaps useful, for ABI compatible ones it can cause much more harm than the benefits. E.g. Solaris as/ld like to mark ELF objects/binaries/libraries with hwcaps of what instructions it actually uses. That works nicely if the whole binary or library is compiled with the same options. But as soon as those options are changed per compilation unit or on a per function bases, this becomes complete nightmare, because it says the binary or library can only run on CPU with xyz capabilities, even when that is actually not true. Any time one uses e.g. GCC target attribute/pragma and either manual runtime decision what code to use or some toolchain support for it (multi-versioning, declare simd, ifuncs), such markings stand in the way. You can look at how e.g. gcc itself needs to cancel this through config/hwcaps.m4 and -mclear-hwcap in various places.

Just a recent example is when I've added hw accelerated SHA1 support to libiberty/GNU binutils bfd linker: https://sourceware.org/pipermail/binutils/2023-November/130820.html That is an example of manual runtime selection, code asks CPU if it supports needed ISAs and uses one of the (in this case) two possible implementations depending on that. This change needed follow-ups for Solaris https://gcc.gnu.org/pipermail/gcc-patches/2023-November/638629.html So, it would be really great if RISC V didn't follow the Solaris lead and didn't impose such mess upon users.

This PR certainly does not attempt to limit optional extensions (or anything that can be probed at run-time).
In the referenced patch the hwprobe() mechanism is used to detect the Intel SHA extension at run time. This clearly states, that the SHA1 instructions are not to be expected to be available on the machine you run this code. This PR does not restrict such a mechanism in any way (it explicitly mentions that this can be done).

This PR targets the extensions that are expected to be available.
Currently, most operating systems use rv64gc as baseline, but this is a very low bar.
E.g. it might be reasonable to require Zba (address calculation) and Zicbom (cache management) as well.
The intent of the RISC-V profiles is to define a set of guaranteed extensions, so that a SW distribution does not have to implement every extension beyond rv64gc as optional feature. See also: https://github.com/riscv/riscv-profiles/blob/main/rva23-profile.adoc.

Raising the amount of required extension is not uncommon. E.g. -march=amd64 is minimum for most Intel distros. But that was not always like this (i686 and i386 were reasonable targets back then). And, of course, distros should have the liberty to pick a profile that is best suited for their users.

@cmuellner
Copy link
Collaborator Author

Re ifuncs etc.

What does the profile attribute mean for code which is using ifuncs? Say that code does use hwprobe and has an optional vector path. The compiler would probably automatically mark the ELF library as "RVA23" (where I assume V will be mandatory). But it would perfectly well run on RVA22 (where V is optional). Should the runtime reject the program marked "RVA23" even though it could run? Should the toolchain be smart and detect ifuncs (and how?). Would we require library writers to hand-annotate such libraries?

The toolchain marks the ELF as RVA23U64 if the toolchain is told to do that.
If Fedora decides to target RVA23U64 in the next release, then V will be mandatory.
And no, such binaries will not run reliably in an RVA22U64 execution environment, because of V.
Analogy: Can you execute an i686 binary in an i386 distro when running on an i686 machine?

And also no, the toolchain does not care about run-time detection.
This is just about the expected extensions, not optional run-time detection (which can and should not be restricted).

It is the job of the execution environment to make a meaningful interpretation of this information.

That seems vague. I guess in practice we'd use it for informational purposes. (Edit: To be clear I'm not arguing against this proposal, an informational attribute could be useful, but saying that it could be more useful if we had a clearer idea what it was for, how it would be set, how it would propagate through linking, and who if anyone consumes it)

Well, being vague is on purpose. I certainly don't want to dictate what the execution environment has to do, because different use-cases might have different requirements. As user I would be happy if a general purpose distro would at least warn me that I am about to execute an i686 binary on an i386 system. In an embedded system that falls out from a Buildroot build such a warning mechanism would be pointless.

@rwmjones
Copy link

rwmjones commented Dec 7, 2023

And no, such binaries will not run reliably in an RVA22U64 execution environment, because of V.

I probably wasn't clear enough here. I mean an object which uses hwprobe / ifunc to offer two paths, one requiring V and one using fallback instructions. The compiler would (probably) automatically mark this as RVA23, or whatever profile requires V, but it would still run without V. The alternative seems to involve manual annotation, which may be fine, but we ought to be clear about the need for this curation.

@cmuellner
Copy link
Collaborator Author

Thanks for your clarification, one question come to my mind is: do we really need -march=<profile-name>[+<ext>]*? because add every single non-profile extensions will made the object become non-conformance, so maybe we should only need -march=<profile-name>.
I guess this should also discuss in riscv-non-isa/riscv-toolchain-conventions#36, but anyway, let us continue the discussion here first :P

Using profiles as pretty names (that expand to a list of extensions) in an march string is reasonable syntactic sugar. However, <profile>_<non-profile ext> can certainly not result in Tag_RISCV_profile being set to <profile>. So, we don't need it, but we might still want it (but there are valid arguments for and against it).
A related question is how to set the profiles in the tools (e.g. -march=<profile> or -mprofile=<profile>). Initially, I preferred to reuse -march a while ago (as outlined in riscv-non-isa/riscv-toolchain-conventions#26 and as proposed in riscv-non-isa/riscv-toolchain-conventions#36), but I don't have any preferences anymore (again, there are valid arguments for and against each mechanism).
A proposal for implementing the Tag_RISCV_profile support could be:

* Set `Tag_RISCV_profile` to a profile A, which is defined in an march string, if the march string only consists of profile A and mandatory extensions of profile A.

* If optional or non-profile extensions are enabled along with a profile in an march string, then the profile is expanded into all its mandatory extensions, which are supported by the compiler.

Yes, I was going to feedback that extensions defined as optional in the profile is the big question mark here. If an extension is unconditionally enabled that's defined as optional in the profile, meaning the binary might not run on all RVANN profile systems is there any value in trying to set the profile tag at all? Perhaps it should just be reserved for when targeting the baseline profile. In the common case (i.e. outside of the OS kernel) the S* extensions aren't going to make a difference, so perhaps there's an argument for ignoring those when determinig if the profile tag can be set.

I should have used "if and only if" to express this more precisely.
Setting the target profile is not compatible with unconditionally enabling optional (or even non-profile) extensions.
If we set the profile tag in this case, then the whole idea of this PR becomes pointless. But using the profile as pretty-name in the march string is fine in this case.

Regarding the S* extensions: Initially, I wanted to restrict this PR to RVxUnn profiles (e.g. RVA23U64 or RVM23U32). I even defined numeric fields to encode x={A, B, M} and the profile year nn. But that felt like something that could be regretted in the future. But I currently don't see much of a use of this flag for other profiles than RVxUnn. And also, there is Zkr, which is optional in RVA23S64, that adds the CSR useed that is relevant for U mode code.

@cmuellner
Copy link
Collaborator Author

And no, such binaries will not run reliably in an RVA22U64 execution environment, because of V.

I probably wasn't clear enough here. I mean an object which uses hwprobe / ifunc to offer two paths, one requiring V and one using fallback instructions. The compiler would (probably) automatically mark this as RVA23, or whatever profile requires V, but it would still run without V. The alternative seems to involve manual annotation, which may be fine, but we ought to be clear about the need for this curation.

I don't think that the compiler should automatically assume a profile.
In fact, many profiles could be applied (just like many arch strings can be used).
This must be controlled by the user (either as default when building the toolchain or via command line).
If omitted, then no profile is set.

In your example you need to be aware that RVA23 could also imply auto-vectorization and other reasons to emit vector instructions. So the RVA23 binary is not guaranteed to run on RVA22 machines even if the hand-written vector assembly part is gated by a dynamic discovery mechanism.

@rwmjones
Copy link

More feedback from the toolchain team at Red Hat:

For example the Linux x86 psABI defines a special section called .note.gnu.properties which contains ELF format notes that describe all kinds of properties alongs with details of how they should be combined:

https://gitlab.com/x86-psABIs/Linux-ABI

This is probably the most comprehensive system. The AArch64 architecture on the other hand uses custom tags in the .dynamic section to indicate desired processor features. Other architectures have used bits in the ELF file header's e_flags field, and so on.

Most of these systems are not very well thought out however and tend not to be extensible to handle lots of different configurations or properties.

(Note again while I'm not opposed to this, I don't think it'll prove to be actually useful to anyone)

@cmuellner
Copy link
Collaborator Author

More feedback from the toolchain team at Red Hat:

For example the Linux x86 psABI defines a special section called .note.gnu.properties which contains ELF format notes that describe all kinds of properties alongs with details of how they should be combined:

https://gitlab.com/x86-psABIs/Linux-ABI

This is probably the most comprehensive system. The AArch64 architecture on the other hand uses custom tags in the .dynamic section to indicate desired processor features. Other architectures have used bits in the ELF file header's e_flags field, and so on.

Most of these systems are not very well thought out however and tend not to be extensible to handle lots of different configurations or properties.

(Note again while I'm not opposed to this, I don't think it'll prove to be actually useful to anyone)

Thanks for the comment and the additional references.

I want to better understand the "not useful to anyone" argument a bit more.
My interpretation of this is, that distros have well-defined target HW requirements and ensure that their packages will run fine on HW that meets these requirements. Third-party SW is not supported and there is no interest in detecting third-party code that was built for a HW target that is (potentially) incompatible.

If this interpretation is right, then this PR indeed lacks a strong enough use case to justify its inclusion in the repo.

@rwmjones
Copy link

rwmjones commented Dec 11, 2023

I think there are two issues.

Firstly as you state, for the distros we either compile all our own software together and certify it on particular hardware, and require third party ISVs to also certify. However there's still some value in having documentation about how an object was built (which is what the .note.gnu.properties does). This documentation lets us go in after the fact and discover problems, or else write tools like annobin/annocheck which do some automatic scans of software.

Secondly if we want the toolchain or runtime to do something clever - eg. reject binaries which won't work with a nice error message - then we would need something more sophisticated, and also have a good story about how tools like compilers and linkers must generate and propagate these attribute(s). Other arches have tried variations of this before and AFAICT none has been very satisfactory.

(Note I'm far from an expert on all this, and I'm shuttling concerns between this issue and the real experts. Will try to get them to participate more in this.)

@cmuellner
Copy link
Collaborator Author

This PR was discussed in today's psABI call. Here are the key messages that I got from the (good discussion in the) call:

  1. This PR needs a clear use case (people from Linux distros and Android don't see how/why they would use this information).
  2. Without a clear use case, it is likely that the feature will turn out not to fit a use case that will be identified in the future.
  3. Any additional value of a target profile in an ELF object might be a gap in the arch string that needs to be closed. Examples are Zic64b, Za64rs, Zkr, and Zkt, which currently don't impact code generation, but only some of these extensions are supported by toolchain components.

This PR will be closed sooner or later because it lacks a relevant use case.

Thanks for all the good comments on this ticket and the call!

@nickclifton
Copy link

Just to clarify a few things...

However there's still some value in having documentation about how an object was built (which is what the .note.gnu.properties does).

Actually that section contains details on the hardware requirements for the code in the object file.
It does not actually describe specific compiler command` line options that were used, nor the tool that was used to build it. To find out which tool built an object file, and what options were used, the best place to look is in the DW_AT_producer attribute in the DWARF information attached to the object file. (Assuming that DWARF debug information has been generated for the file),

Or you can use a specialized compiler plugin to record the information about the compilation process and store this information somewhere in the object file. This is what the annobin plugin does, although since it is focused on security it only records those options which are related to code hardening. It does not record options related to ABI or ISA selection. (It could do this, it just has not been coded to do so yet).

This documentation lets us go in after the fact and discover problems, or else write tools like annobin/annocheck which do some automatic scans of software.

Annocheck makes use of both information sources (the data recorded by the annobin plugin as well as the DW_AT_producer attribute) as part of its job to ensure that executable binaries have been built with all of the required hardening options enabled.

@rwmjones
Copy link

Nick thanks for commenting here. How much of annobin is specific to x86-64, and how much would work for other arches (RV64 in this case)? Will it require significant porting work? Will it requires any changes in ELF specs?

@nickclifton
Copy link

Nick thanks for commenting here. How much of annobin is specific to x86-64,

None. :-)

Well not quite. The annobin plugin does record details about some x86-64 specific command line options (the ISA selected and the -mstackrealign option) but the plugin and the annocheck tool are mostly architecture agnostic. There are versions of the plugin for all architectures supported by RHEL.

It is also worth noting that both the plugin and the annocheck tool can work in a cross compilation environment. So you can cross compile RiscV binaries on, say, an x86_64 host and test them with annocheck on that same x86_64 host or on a native RiscV box.

and how much would work for other arches (RV64 in this case)?

None - it is already supported. :-) Including a RiscV specific version of the annobin plugin. Although currently no riscv specific command line options are recorded. This could be changed quite easily however. There are builds available for Fedora if you are interested:
http://fedora.riscv.rocks/koji/packageinfo?packageID=14713

(And to answer the question more fully, it is fairly simple to add support for other architectures. The annobin sources are small and there are only a few places where target specific additions are needed).

Will it require significant porting work?
No.

Will it requires any changes in ELF specs?

No. Annobin works entirely within the ELF standard and does not need any additions or architecture specific values to be defined.

wangpc-pp added a commit to wangpc-pp/llvm-project that referenced this pull request Dec 28, 2023
This PR implements the draft
riscv-non-isa/riscv-toolchain-conventions#36.

Currently, we replace specified profile in `-march` with standard
arch string.

We may need to pass it to backend so that we can emit an ELF attr
proposed by
riscv-non-isa/riscv-elf-psabi-doc#409.
@cmuellner cmuellner closed this Jan 29, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants