Page MenuHomePhabricator

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

Authored by courbet on Jun 3 2019, 2:02 AM.

Details

Reviewers
aprantl
probinson
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.

Event Timeline

courbet created this revision.Jun 3 2019, 2:02 AM
Herald added a project: Restricted Project. · View Herald TranscriptJun 3 2019, 2:02 AM
Herald added a subscriber: hiraditya. · View Herald Transcript
probinson added inline comments.
llvm/lib/CodeGen/AsmPrinter/DebugLocEntry.h
185

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

courbet marked an inline comment as done.Jun 3 2019, 5:28 AM
courbet added inline comments.
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 added inline comments.Jun 3 2019, 6:15 AM
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 marked an inline comment as done.Jun 3 2019, 6:19 AM
courbet added inline comments.
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 accepted this revision.Jun 3 2019, 6:29 AM

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

This revision is now accepted and ready to land.Jun 3 2019, 6:29 AM
aprantl accepted this revision.Jun 3 2019, 8:38 AM