[PATCH] D57790: [CodeView] Fix cycles in debug info when merging Types with global hashes

[Alexandre Ganea via Phabricator via llvm-commits]

aganea marked an inline comment as done.
aganea added inline comments.


================
Comment at: include/llvm/DebugInfo/CodeView/TypeHashing.h:119
+    }
+    if (ReqSecondPass) {
+      auto HashIt = Hashes.begin();
----------------
zturner wrote:
> aganea wrote:
> > rnk wrote:
> > > I think this would have to go to fix point, not just two passes. Consider the case where each record in the type stream refers to the next, i.e. a pointer to a pointer to a pointer ... to void, and everything is backwards. =(
> > Good point. I can make a test case, and modify to handle that.
> I think we can do slightly better than go to fixed point.  First go forwards, then go backwards over the remaining records.  That's verrrrrry likely to get you to the end in 2 passes.  If nothing else, definitely a better heuristic then always going forward.
Tried that, but given we're dealing with `CVTypeArray` as a `Range` type, we can't iterate back; unless we use something like `LazyRandomTypeCollection` for caching after the first run. Given that forward references seem to happen on MASM OBJs only, using a `LazyRandomTypeCollection` for every OBJ sounds a bit overkill. So instead I opted for several passes, like `TypeStreamMerger`.


================
Comment at: tools/llvm-readobj/llvm-readobj.cpp:647
+
+  if (opts::CodeViewMergedTypes) {
+    ScopedPrinter W(outs());
----------------
I had to move this here, because otherwise in the old place the TPI stream would reference deleted memory (because `BinaryOrErr` is deleted above when `dumpInput()` exits)


Repository:
  rL LLVM

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

https://reviews.llvm.org/D57790