[PATCH] D38924: Fix `FaultMaps` crash when the out streamer is reused

Serguei Katkov via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Sun Oct 15 19:41:57 PDT 2017


skatkov accepted this revision.
skatkov added inline comments.


================
Comment at: lib/Target/X86/X86AsmPrinter.h:137
     SMShadowTracker.reset(0);
     SM.reset();
+    FM.reset();
----------------
sanjoy wrote:
> yuyichao wrote:
> > sanjoy wrote:
> > > Does it make sense to do these things in `doFinalization`?
> > As I said, I'm mainly following `SM` here.
> > 
> > That said, I do think something needs to be done here. IIUC, these maps holds information about the module so it makes sense to initialize them when the pass is going to handle a new module. It might also be good to also reset them in `reset` so that (assuming `reset` is called at the same time the `MCContext` is reset) the map won't hold any invalid pointers. I didn't also do that because `SM` didn't and the invalid pointers themselves won't cause a crash as long as they are cleared before the run starts.
> Ok, that sgtm; but I want @skatkov to take a look.
I guess for clean solution it should really good to update reset and doFinalization to avoid seeing the invalid pointers while they still do not cause a crash themselves. But the point that SM does not do that makes me thinking that it might be a separate patch.


https://reviews.llvm.org/D38924





More information about the llvm-commits mailing list