[llvm] [llvm-objcopy][ELF] Add an option to remove notes (PR #118739)
James Henderson via llvm-commits
llvm-commits at lists.llvm.org
Mon Dec 23 00:41:57 PST 2024
jh7370 wrote:
> > I'm concerned that this approach seems to only work with notes in segments, when the option seems to apply reasonably to ET_REL objects too, where the notes are in sections only. Indeed, more generally llvm-objcopy is designed to work primarily with sections, with direct interactions with segments being very limited. I think it would more naturally fit with llvm-objcopy's design to work with note sections first. You could swap the section contents is the same way as other replace section operations work, so that much of the existing writing behaviour works unchanged.
>
> We can't rely on the existing code to update `SHT_NOTE` sections that reside in `PT_NOTE` segments. Currently, when the contents of such a section are replaced, the new data is written over the current segment contents, and if the section is shortened, the gap retains the old data. For `PT_NOTE` segments, this means that either some notes are preserved or duplicated (if the gap starts at the bound of a note), or the note entries in the segment starting from that point are corrupted. Thus, the offsets and sizes of such sections and the size of such segments should be adjusted.
Yeah, you'd need to compress the sections together and then shrink the segment. I'm not sure if there's existing code to modify a segment in this way yet.
> However, the existing code is suitable for sections that are not associated with segments. I've updated the patch, so sections in relocatable objects are also supported now.
I'm happy with segment support being added later.
> > Clearly, something special would need to be done for PT_NOTE segments, especially those without sections. One option might be to create an artificial section that covers the whole segment (or any parts of the segment not covered by an existing section at least) so that the section-based approach outlined above would still work, then have special handling to adjust the file/memsz fields after the sections have been updated.
>
> This would either expose these temporary sections in the output file, or require some non-trivial changes to the code. And yet this does not solve the issue of gaps in `PT_NOTE` segments.
I was thinking something similar to how the ELF Header and Program Header table work, but those are pseudo-segments, not pseudo-sections. Nonetheless, I don't think it would be too hard to add a flag to the artificial section to indicate it shouldn't be included in the section header table. Again though, I'm happy for that to be deferred to a later patch, if it makes things simpler in the first one.
> > Somewhat related to the previous point, I'm concerned with how `updateSegmentData` might interact with `--remove-section` for sections in segments. The current approach of the latter is that removed section data is overwritten with null bytes, preserving the segment size. I understand that for a PT_NOTE this may not be quite so desirable, but I think we need to do something to prevent mixing the two behaviours, because what the end result should be is non-obvious. One way would be to emit an error if a note section/segment is touched by both --remove-section and --remove-note.
>
> I suspect that there may also be some problems with using the new command together with `--add-section` and `--update-section`, as the correct order of execution is not clear. I think the simplest solution is just to reject such combinations of options, so I've updated the patch accordingly.
IIRC, we loosely model --update-section as --remove-section followed by --add-section. --add-section is deliberately done after --remove-section, to allow for a section to be replaced (and what would be the point of removing a section that was just added after all). I guess --remove-note would be similar to --update-section in that it's essentially another way of changing the contents of a note section. As such, it would come after --remove-section operations, but it's more ambiguous as to what order it applies to with regards to the other. I don't think --add-section + --remove-note interact, so I guess it doesn't matter there. --update-section + --remove-note is mostly nonsensical, where the same section is touched.
One possible exception to the above is adding/updating a note section, but wanting to remove a note from the added/modified section, because the input data wasn't quite in the right format. I don't see a use-case for --remove-note coming before the operations, but this would indicate it should come after, if it could be made to work. I also don't think the use-case is that strong, so an error is equally fine by me.
I'm not going to look at the details today - it'll have to wait until the new year when I'm back in the (home-)office.
https://github.com/llvm/llvm-project/pull/118739
More information about the llvm-commits
mailing list