Page MenuHomePhabricator

Clear NewGEPBases after finish using them in CodeGenPrep pass
ClosedPublic

Authored by yuyichao on Jul 17 2020, 8:16 AM.

Details

Summary

AFAICT all other set/map are correctly cleared in runOnFunction.

With assertion enabled this causes a crash when the module is freed and potentially if a later pass delete the instruction (not observed in real world though). Without assertion this can potentially cause confusing result when running on a new Function/Module.

Diff Detail

Event Timeline

yuyichao created this revision.Jul 17 2020, 8:16 AM
Herald added a project: Restricted Project. · View Herald TranscriptJul 17 2020, 8:16 AM
vchuravy added a subscriber: Restricted Project.Jul 17 2020, 8:54 AM
loladiro accepted this revision.Sep 2 2020, 10:47 AM
loladiro added a subscriber: loladiro.

LGTM. Could we have a test for this? There is the -run-twice option to the various tools, which is designed to catch issues like this.

This revision is now accepted and ready to land.Sep 2 2020, 10:47 AM
yuyichao added a comment.EditedSep 3 2020, 10:27 AM

Does run-twice free the module? I’ll have to look for the repro again but I was only able to observe a crash when the module is free’d and it doesn’t even require running the pass again...

Well, the module if freed eventually. Is this issue whether the module if freed before the pass manager? If so I think run-twice will (incidentally also do that). I just though run-twice would catch it, because forgetting to clear such things usually causes incorrect behavior the next time around.