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

[James Henderson via Phabricator via llvm-commits]

jhenderson 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 &&
----------------
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.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D87026