Page MenuHomePhabricator

[clang] fix frontend crash in auto type templates
Needs ReviewPublic

Authored by inclyc on Aug 14 2022, 2:34 PM.

Details

Summary

Reported by https://github.com/llvm/llvm-project/issues/57142

In this case, AutoTypes.FindNodeOrInsertPos found appropriate AT
value and should return that directly, however before this patch we
assumed that it always get InsertPos back and create a new AT for
it. In fact InsertPos is a nullptr, which causes segmentfault.

Diff Detail

Event Timeline

inclyc created this revision.Aug 14 2022, 2:34 PM
Herald added a project: Restricted Project. · View Herald TranscriptAug 14 2022, 2:34 PM
inclyc updated this revision to Diff 452566.Aug 14 2022, 2:41 PM

improve test case

inclyc published this revision for review.Aug 14 2022, 2:42 PM
inclyc added subscribers: cfe-commits, Restricted Project.
Herald added a project: Restricted Project. · View Herald TranscriptAug 14 2022, 2:42 PM
inclyc edited the summary of this revision. (Show Details)Aug 14 2022, 2:43 PM
inclyc updated this revision to Diff 452569.Aug 14 2022, 2:59 PM

update comments

mizvekov added inline comments.Aug 14 2022, 3:37 PM
clang/lib/AST/ASTContext.cpp
5735–5736

The original code is correct, AutoTypes.FindNodeOrInsertPos(ID, InsertPos) returning non-null should be impossible here, as we are just refreshing the insert position after having possibly invalidated it when we created the canonical node.

We had looked for the node before and didn't find it, so finding it just after having created the canonical node means this is a canonicalization bug.

I think this could have used an assert on the return value to better explain that is going on.

FWIW, someone else already started working on this bug some time ago, and I made some comments there which I think explain the problem better: See https://reviews.llvm.org/D126172

Might be worth a ping since might have stalled on it though.