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

[David Blaikie via Phabricator via llvm-commits]

dblaikie 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 &&
----------------
ikudrin wrote:
> 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.
By "not supported" you mean "has no guarantees if you ask for it" or do you mean "will produce an error early telling the user this isn't valid/supported" & thus this assert should (assuming no other programmer bugs - but importantly /not/ assuming some well intentioned user - a user can ask for an invalid thing and should get an error, not an assert failure/crash) ?


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

https://reviews.llvm.org/D87026