[llvm] [llvm-objcopy][ELF] Add an option to remove notes (PR #118739)

Igor Kudrin via llvm-commits llvm-commits at lists.llvm.org
Mon Dec 23 16:32:04 PST 2024


igorkudrin wrote:

> 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.

And that exactly what my code for updating segments did. There was now existing code for anything similar. Current code expects sections to stay in the same places within segments.

> > 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.

OK, I've removed the support for segments for now to simplify the patch. I'll create a separate review for that after this patch is landed.

> > 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.

If we add the flag to mark a section as artificial, we will have to check it in many places, such as `ELFWriter<ELFT>::writeEhdr()`, where the size of the collection is used to calculate the value of the `e_shnum` field, or any code that scans the sections. Perhaps it would be easier to use a separate collection for them, in which case only the code that handles such sections would need to be updated.

> > 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.

One can argue that the intention might be to remove some unwanted notes and add new ones using these commands... or to make sure that the requested notes do not appear in the produced output, even if they are in the section added by these commands. So, to avoid speculation and overthinking, it is easier to reject such combinations.

> 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.

Thanks!

https://github.com/llvm/llvm-project/pull/118739


More information about the llvm-commits mailing list