This is an archive of the discontinued LLVM Phabricator instance.

[clang][Interp] Fix lifetime diagnostics for dead records
ClosedPublic

Authored by tbaeder on Jun 5 2023, 2:39 AM.

Details

Summary
This used to crash the interpreter, either because we ran into the
assertion in CheckMutable() or because we accessed a Descriptor* pointer
preceding the field of a record. Those are preceded by an
InlineDescriptor though.

Also, we forgot to handle the metadata when moving the Block over to a DeadBlock.

(Regarding the InlineDescriptor stuff from above... I didn't add it in this patch because the problem does not require it, but I'll do fix that later).

Diff Detail

Event Timeline

tbaeder created this revision.Jun 5 2023, 2:39 AM
Herald added a project: Restricted Project. · View Herald TranscriptJun 5 2023, 2:39 AM
tbaeder requested review of this revision.Jun 5 2023, 2:39 AM
Herald added a project: Restricted Project. · View Herald TranscriptJun 5 2023, 2:39 AM
Herald added a subscriber: cfe-commits. · View Herald Transcript

I think this is mostly good, but I did run into a question about removing the assertion so I might still be missing something about the changes.

clang/lib/AST/Interp/Interp.cpp
241 ↗(On Diff #528332)

I understand that we were hitting this assertion, but why is removing the assertion the correct approach? I thought it made sense to check for pointer liveness here.

tbaeder retitled this revision from [clang][Inter] Fix lifetime diagnostics for dead records to [clang][Interp] Fix lifetime diagnostics for dead records.Aug 7 2023, 12:37 AM
tbaeder added inline comments.Aug 17 2023, 4:32 AM
clang/lib/AST/Interp/Interp.cpp
241 ↗(On Diff #528332)

I re-checked and I think I removed this only for testing and it should stay in. The test succeeds like this.

tbaeder updated this revision to Diff 551087.Aug 17 2023, 4:32 AM
This revision is now accepted and ready to land.Aug 18 2023, 5:56 AM
This revision was landed with ongoing or failed builds.Aug 20 2023, 2:53 AM
This revision was automatically updated to reflect the committed changes.