[PATCH] D38329: [llvm-objcopy] Fix latent bug that allowed some Sections to be improperly cast to StringTableSections

[James Henderson via Phabricator via llvm-commits]

jhenderson accepted this revision.
jhenderson added a comment.
This revision is now accepted and ready to land.

LGTM with typo fixes.

In https://reviews.llvm.org/D38329#887626, @jakehehrlich wrote:

> I'm starting to notice a slightly non-trivial match up between classof and makeSection. Is there a way we could make this precise? Ideally I'd like to somehow contain the means of construction for a type within the class itself. I'd really like the following properties to be enforced by the library
>
> 1. You can't construct a StringTableSection that dosn't pass StringTableSection::classof
> 2. The checks for SHT_* and SHF_ALLOC shouldn't be duplicated in construction and classof. When one changes the other should change as well.


I agree with this, although I think it's a separate change. I need to think a bit more about what a good solution might be.



================
Comment at: tools/llvm-objcopy/Object.h:119
+// In the dynamic case the string table is allocated. Changing a dynamic string
+// table would mean altering virtual addrsses and thus the memory image. So
+// dynamic string tables should not have an interface to modify them or
----------------
addrsses -> addresses


================
Comment at: tools/llvm-objcopy/Object.h:124
+// classof method checks that the particular instance is not allocated. This
+// the agrees with the makeSection method used to construct most sections.
 class StringTableSection : public SectionBase {
----------------
the agrees -> then agrees


Repository:
  rL LLVM

https://reviews.llvm.org/D38329