[PATCH] D139681: [DX] Improve parse error messages

[Chris Bieneman via Phabricator via llvm-commits]

beanz added inline comments.


================
Comment at: llvm/lib/Object/DXContainer.cpp:109-113
     // We need to ensure that each part offset leaves enough space for a part
     // header. To prevent overflow, we subtract the part header size from the
     // buffer size, rather than adding to the offset. Since the file header is
     // larger than the part header we can't reach this code unless the buffer
     // is larger than the part header, so this can't underflow.
----------------
bob80905 wrote:
> This comment blocks seems pretty irrelevant now, so maybe it should be removed / updated.
> I am also concerned this might change some error text behavior, since I don't see the PartOffset variable ever getting the header size value subtracted from it.
> Although it's possible to exit early when PartOffset < LastOffset, what happens if, say, PartOffset = Data.getBufferSize() - sizeof(dxbc::PartHeader) + 1? 
> In the old code, it returns, but in this new code, it might not enter the if block, right?
> 
Yep! I need to update that.

The `PartOffset` field is read from the buffer, so it shouldn't need to be modified. The offset values in the file _should_ point to valid offsets in the file from which we can read at least a part header (4 bytes for the part name and 4 bytes for the part size).

The ParsePartNoSize test below should cover the case where a part offset is too close to the end of the file to have a size field, but I think you're right that there is a missing case where the part size can't handle the part name field.

I'll update the patch.

Thanks!


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D139681/new/

https://reviews.llvm.org/D139681