The elftoolchain project includes these additional defines for various
userland programs. Given that arch-specific defines are still interesting
in the context of userland programs reading or writing ELF metadata, they
should be included in top-level ELF headers.
Details
Details
- Reviewers
will imp - Commits
- rS276539: Update ELF headers to include additional defines
Apply patch, ensure that buildworld doesn't fail.
Diff Detail
Diff Detail
- Repository
- rS FreeBSD src repository - subversion
- Lint
Lint Skipped - Unit
Tests Skipped
Event Timeline
Comment Actions
These look good to my eyes, but I didn't verify that ABI'ly mandated values match the relevant standards.
Comment Actions
As with Warner I didn't compare the specific values with the relevant specs, but assuming these are taken as-is from upstream I'm happy with this, with a few caveats as noted.
elfdump(1) could also use a corresponding update if you're so inclined.
| sys/sys/elf32.h | ||
|---|---|---|
| 73 ↗ | (On Diff #1783) | Can you add a more explicit reference? A quick googling suggests this is specifically for SHT_MIPS_LIBLIST, but I haven't looked to confirm if that is the case or not. (Same comment for the 64-bit case.) |
| sys/sys/elf_common.h | ||
| 421 ↗ | (On Diff #1783) | extra whitespace? |
| 716 ↗ | (On Diff #1783) | Where is this used? |
| 728 ↗ | (On Diff #1783) | I don't really like this -- the note types are inherently specific to the note vendor, so that should really be included in the #define -- e.g., all should be NT_GNU_*. NT_AUXV should be 16 on FreeBSD (although it was named NT_PROCSTAT_AUXV instead). |
Comment Actions
I have more updates for this change forthcoming. To answer the question, yes -- these #define's are taken as is from elftoolchain. I do assume they're correct; if not, they can be fixed in a later revision.
It doesn't pass an 'make universe' -- there are failures in ARM and MIPS. Both of which I was anticipating, because there are machine-specific headers that already define some of these.
Also, I've since added comments for all the new ones that didn't have one yet. Values also taken from elftoolchain.
So, it will not be committed as it stands. Thanks for the reviews so far, will submit an update once I have the issues resolved.
Comment Actions
To clarify, the initial submission was primarily on a 'does this make sense to do here?' basis, not on a 'is this ready to commit?' basis.
Comment Actions
"does this make sense to do here"
On that point I agree strongly with the intent of this change.
Comment Actions
Ed asked me to post an update about this revision.
My motivation for working on these was to improve FreeBSD DTrace support for C++ code. However, after a while I realized that CTF/DTrace itself doesn't really support C++, so I dropped this effort.
As of right now, I don't think there is anything else I can contribute to this revision (patch applied as is in Sept, and make universe failed for the two arches primarily affected). I also don't have any other efforts that depend on these changes, so if anyone else wishes to pick this up, please feel free. Thanks!
Comment Actions
Remove Linux NT_ types for now, pending a better compat implementation
Remove ARM and MIPS constants now defined in elf_common from arch-dependent files