[PATCH] D63843: [Object][XCOFF] Add support for 64-bit file header and section header dumping.

[Hubert Tong via Phabricator via llvm-commits]

hubert.reinterpretcast added a comment.

This patch seems to have suffered from scope creep. The replacement of `error_code` has led to a considerable shifting around of the code that makes it more difficult to review the other part of the patch. Since it has already happened, I guess we will push through; however, I believe this sort of situation should be avoided.



================
Comment at: llvm/lib/Object/XCOFFObjectFile.cpp:476
+  // for the string tables size. Not having as string table is not an error.
+  if (Offset + 4 > Obj->Data.getBufferSize())
+    return XCOFFStringTable{0, nullptr};
----------------
Just practising my theoretical-overflow avoidance:
```
assert(Obj->Data.getBufferSize() >= Offset);
if (Obj->Data.getBufferSize() - Offset < 4)
```

Or use a helper function:
```
if (auto EC = Binary::checkOffset(Obj->Data, Offset, 4))
```


================
Comment at: llvm/lib/Object/XCOFFObjectFile.cpp:488
+  // Check that the object contains enough data for the string table.
+  if (Obj->Data.getBufferSize() < Offset + Size)
+    return errorCodeToError(object_error::unexpected_eof);
----------------
Use `getObject`.


================
Comment at: llvm/lib/Object/XCOFFObjectFile.cpp:492
+  // Check that the String table is properly terminated.
+  const char *StingTablePtr =
+      reinterpret_cast<const char *>(Obj->base() + Offset);
----------------
s/StingTablePtr/StringTablePtr/g;


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D63843