Use inline assembly to define ELF notes. This permits placing the notes
in a section with the proper type (SHT_NOTE) directly.
Details
Details
- booted a RISC-V world built with this in QEMU. Have not done a full tinderbox yet. RISC-V uses an OSABI of none, so relies on the ABI tag for all branding.
Diff Detail
Diff Detail
- Lint
Lint Passed - Unit
No Test Coverage - Build Status
Buildable 31689 Build 29260: arc lint + arc unit
Event Timeline
Comment Actions
I wanted to see if we were ok with using this approach. I have some further changes I'd like to make to add the ABI tag note to shared libraries and rtld (right now you can't direct exec rtld on RISC-V for example). However, I'd like to avoid further proliferation of the sed hack to do that. NetBSD uses assembly sources for this instead of C, but this seems like a simpler step forward for us. NetBSD requires a genassym file due to the use of assembly and keeping it in C at least avoids that, though the use of __XSTRING() is perhaps a bit ugly.
| lib/csu/common/crtbrand.c | ||
|---|---|---|
| 6 | I would be explicit with the NULs to be clear this is exactly 8 bytes as we need to ensure 4-byte alignment of the following word (and to match the namesz above). | |
| lib/csu/common/crtbrand.c | ||
|---|---|---|
| 6 | The alternative might be to throw in a .p2align 2 to enforce the padding? | |
| lib/csu/common/crtbrand.c | ||
|---|---|---|
| 6 | Then it's less clear that namesz is correct. | |
Comment Actions
- Trim redundant rule.
- Explicitly align note payloads.
- Drop notes.h as it isn't really used.
| lib/csu/common/crtbrand.c | ||
|---|---|---|
| 6 | So I could use labels to derive namesz (and perhaps even descsz) as the NetBSD webpage suggests. | |
| lib/csu/common/crtbrand.c | ||
|---|---|---|
| 6 | Ah yes, that would work. | |
Comment Actions
- Use labels to compute namesz and descsz.
- Revert "Drop notes.h as it isn't really used."
Comment Actions
rtld uses notes.h, so I put it back. I haven't (yet) dropped the #includes of it in crtbrand.c and ignore_init.c. I could perhaps make more use of __XSTRING to use the values from notes.h in the inline assembly, but that seems a bit ugly?
| lib/csu/common/ignore_init.c | ||
|---|---|---|
| 144 | That's a good question. I'd rather do that as a separate change if so though. | |
Comment Actions
Did you considered making crtbrand.c a standalone .S and then link it into crt1.o with ld -r ? It is pure asm, using asm() for it IMO causes more problems like XSTRING than it provides a convenience.
Comment Actions
Yes, that seems to be a much cleaner solution. In any case, I'm completely in favor of anything that gets rid of the SED_FIX_NOTE hack... :)
Comment Actions
I thought about making it a standalone .S file. It would mean more work in the various Makefiles to link it in? I guess they'd all have to be like i386. NetBSD had to use a genassym file. I'd like to avoid that, but not sure we can include, e.g. <sys/param.h> in assembly to get __FreeBSD_version. I'll play with it and see.
Comment Actions
param.h definitely has #ifdef LOCORE stuff so it seems to be used by some asm files.
Comment Actions
- Move notes into assembly.
- Permit <sys/elf_common.h> and <sys/param.h> to work in userland assembly.
- Convert RISC-V CSU.
Comment Actions
I've updated RISC-V to use notes defined in assembly and uploaded it to see what it looks like. I will work on more architectures tomorrow.
| lib/csu/common/crtbrand.S | ||
|---|---|---|
| 31 | This seemed less work than getting it added into ACFLAGS in various places at least as a hack, but I'd be fine moving this out to ACFLAGS if that is preferred. | |
Comment Actions
I think that riscv csu readability is improved with both notes and asm block moved to .S files.
For LOCORE, can you put it into AFLAGS in lib/csu/Makefile.inc ?
Overall this looks fine for riscv, and should be similarly good for other arches.
Comment Actions
- Convert i386.
- Define LOCORE in common ACFLAGS.
- Update amd64.
- Update aarch64.
- Update arm.
- Update mips and powerpc*.
- Consistent sorting.
Comment Actions
- Use .4byte.
- Simplify MACHINE_ARCH for arm.
- ARM requires %note rather than @note.
- Move NT_FREEBSD_ARCH_TAG to assembly.
Comment Actions
@imp, can you review the sys/arm/include/param.h change?
A 'make tinderbox' passed for me, and 'readelf /usr/obj/<mumble/*/bin/sh' shows all the right notes present in each /bin/sh binary.
Comment Actions
These look great. You could do a few of them as separate commits, if you wanted (like the arm param.h), but it's not big deal either way.
I didn't pedantically audit all the new assembler (git is confused about where it came from :).