jhenderson added inline comments.
================
Comment at: llvm/lib/Object/XCOFFObjectFile.cpp:702
+Expected<StringRef> XCOFFObjectFile::getRawData(uintptr_t Start,
+ uint64_t Size) const {
----------------
Why is `Start` a `uintptr_t` rather than either a `const char *` or `const uint8_t *` (or even a `StringRef` or `ArrayRef<uint8_t>`)?
I also don't see this test being tested.
================
Comment at: llvm/lib/Object/XCOFFObjectFile.cpp:706
+ return createError(toString(std::move(E)) +
+ ": the raw data with offset 0x" +
+ Twine::utohexstr(Start) + " and size 0x" +
----------------
Rather than "the raw data" here, perhaps you should pass in a "Name" parameter which is used to describe what the raw data is, e.g. a symbol or similar, so that the error message is more meaningful: "symbol with offset 0x1234 and size 0x4321 goes past the end of the file".
================
Comment at: llvm/lib/Object/XCOFFObjectFile.cpp:708
+ Twine::utohexstr(Start) + " and size 0x" +
+ Twine::utohexstr(Size) + " go past the end of the file");
+ return StringRef(reinterpret_cast<const char *>(Start), Size);
----------------
================
Comment at: llvm/test/tools/llvm-objcopy/XCOFF/invalid-read.test:4
+
+## Error1: failed to read section data.
+# RUN: yaml2obj %s --docnum=1 -o %t1
----------------
I'd delete the "Error1: " bit of this comment (and similar bits in others), as it doesn't really add much value, but also has a risk of being confused with the FileCheck check prefix `ERROR1`.
Also, there's a copy-and-paste error further down with it :)
================
Comment at: llvm/tools/llvm-objcopy/XCOFF/Reader.cpp:70-73
+ ReadSym.AuxSymbolEntries = StringRef(
+ reinterpret_cast<const char *>(SymbolDRI.p +
+ XCOFF::SymbolTableEntrySize),
+ XCOFF::SymbolTableEntrySize * SymbolEntRef.getNumberOfAuxEntries());
----------------
jhenderson wrote:
> Do we need something to protect us from accidentally running off the end of the file, if it's been truncated somehow?
I'm probably being dumb, but where is this checked?
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D97656/new/
https://reviews.llvm.org/D97656