[DebugInfo] Fix possible invalid dereference of non-engaged Optional.

[courbet]

Summary

This triggers with EXPENSIVE_CHECKS on for a bunch of checks (e.g. fold-sext-trunc.ll).
Unexpected Failures goes from 144 to 29.

[probinson]

llvm/lib/CodeGen/AsmPrinter/DebugLocEntry.h

185

Can you point to a test where one or both values do not have a fragment?

[courbet]

llvm/lib/CodeGen/AsmPrinter/DebugLocEntry.h

185

I see that happening when there is a single value that does not have a fragment. My stdlib has:

__glibcxx_requires_irreflexive(__first, __last);

as a first statement in sort, which with _GLIBCXX_DEBUG expands to:

_GLIBCXX_DEBUG_VERIFY(_First == _Last || !(*_First < *_First) ...

The above calls operator<. I can reproduce with:

./bin/llc < /usr/local/google/home/courbet/llvm/llvm-project/llvm/test/CodeGen/X86/fold-sext-trunc.ll -mtriple=x86_64--

[probinson]

llvm/lib/CodeGen/AsmPrinter/DebugLocEntry.h

185

Hmm is there a test that fails if you switch the condition to AFI.hasValue() > BFI.hasValue() ? I am just wondering about the sorting behavior in the absence of fragments.

[courbet]

llvm/lib/CodeGen/AsmPrinter/DebugLocEntry.h

185

No, nothing fails. I only saw this being triggered by the irreflexivity check I mentioned above, which works with both < and >.

[probinson]

Well, the results are at least defined now. Please make sure the commit log mentions "e.g. fold-sext-trunc.ll".
LGTM

[courbet]

Thanks.

llvm/lib/CodeGen/AsmPrinter/DebugLocEntry.h

case DebugLocEntry::Value::E_ConstantInt:
     return A.Constant.CIP == B.Constant.CIP;
   }
   llvm_unreachable("unhandled EntryKind");
 }
 
 /// Compare two fragments based on their offset.
 inline bool operator<(const DebugLocEntry::Value &A,
                       const DebugLocEntry::Value &B) {
-  return A.getExpression()->getFragmentInfo()->OffsetInBits <
-         B.getExpression()->getFragmentInfo()->OffsetInBits;
+  const auto AFI = A.getExpression()->getFragmentInfo();
+  const auto BFI = A.getExpression()->getFragmentInfo();
+  if (AFI.hasValue() && BFI.hasValue())
+   return AFI->OffsetInBits < BFI->OffsetInBits;
+  return AFI.hasValue() < BFI.hasValue();
 }
 
 }
 
 #endif