[PATCH] D87026: [DebugInfo] Make offsets of dwarf units 64-bit (19/19).

[Igor Kudrin via Phabricator via llvm-commits]

ikudrin added inline comments.


================
Comment at: llvm/lib/CodeGen/AsmPrinter/AccelTable.cpp:594-601
+  assert(Die.getDebugSectionOffset() <= UINT32_MAX &&
+         "The section offset exceeds the limit.");
   Asm->emitInt32(Die.getDebugSectionOffset());
 }
 
 void AppleAccelTableTypeData::emit(AsmPrinter *Asm) const {
+  assert(Die.getDebugSectionOffset() <= UINT32_MAX &&
----------------
jhenderson wrote:
> dblaikie wrote:
> > These might be report_fatal_error-worthy? (if there's ways to fail the build more nicely (without fatally stopping the program) that'd be nice - not sure how other attempts to try to use the AsmPrinter when the use is invalid fail/whether they can fail gracefully)
> My personal thoughts:
> 
> # If this code can actually be hit, it should be converted to return `Error`/`Expected` up to whatever point does the error handling.
> # `report_fatal_error` could be a temporary fallback if the above point is too big a piece of work for now, but shouldn't be the final state of the code (since `report_fatal_error` results essentially in a crash).
> # `assert` is most appropriate if this situation can't actually be hit (i.e. something else somewhere is protecting the limit).
> # It might make sense to add a check earlier in the code to allow the `assert` here if this `assert` could otherwise be triggered.
In the update, I am adding reporting a fatal error in `DwarfFile::computeSizeAndOffsets()`, where the offsets for DWARF units are calculated and set. As DWARF64 is not supported for Apple targets, these `assert`s are now not expected to be hit with the normal usage.


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

https://reviews.llvm.org/D87026