[PATCH] D54361: [ELF] .gdb_index: fix CuOff when a .debug_info section contains more than 1 CU

[David Blaikie via Phabricator via llvm-commits]

dblaikie added a comment.

(ah, I see I hadn't posted my first feedback earlier (written but unsubmitted) - oops, sorry about that)



================
Comment at: ELF/SyntheticSections.cpp:2432
+      //
+      // We assume both CUs[*].CuOff and Set.Offset are increasing.
+      while (I < CUs.size() && CUs[I].CuOffset < Set.Offset)
----------------
CUs[*].CuOff is necessarily increasing, because it's the offset as the CUs are read from the input, but there's no reason to believe that Set.Offset is necessarily increasing - pubnames could be in any order relative to the CUs.


================
Comment at: lld/trunk/ELF/SyntheticSections.cpp:2432
+      //
+      // We assume both CUs[*].CuOff and Set.Offset are increasing.
+      while (I < CUs.size() && CUs[I].CuOffset < Set.Offset)
----------------
Could you fix this? This assumption seems pretty subtle/likely to break valid DWARF in surprising/hard to diagnose ways.

Moving the declaration of I into the for(Set) loop (ie: putting it just before this while loop) should suffice - and a test case with 2 pubnames in the reverse order to CUs would demonstrate the fix?

(would be nice to do a binary search rather than a linear one on the CU offsets, since they are naturally sorted by offset)




Repository:
  rL LLVM

https://reviews.llvm.org/D54361