tbaeder abandoned this revision. tbaeder marked an inline comment as done. tbaeder added inline comments.
================ Comment at: clang/lib/AST/Interp/EvalEmitter.cpp:260-261 + // local variable is used after being destroyed. + InlineDescriptor &ID = *reinterpret_cast<InlineDescriptor *>(B->rawData()); + std::memset(&ID, 0, sizeof(InlineDescriptor)); } ---------------- aaron.ballman wrote: > tbaeder wrote: > > aaron.ballman wrote: > > > I'm not certain this is a good idea -- we've just deallocated `B` and > > > then we're saying "cool, now get me your raw data so I can muck about > > > with it". > > > > > > The comments in `InterpState::deallocate()` say `// Free storage, if > > > necessary.`, so this looks a lot like a use-after-free. Am I missing > > > something? > > `deallocate()` doesn't free the `Block`'s memory though, so we can still > > use it afterwards. That's why I had the problems with > > https://reviews.llvm.org/rG5b54cf1a2892767fe949826a32d7820732028a38 and > > neither a > > I could also move this code to `deallocate` directly. > > > > This is just a security measure so we don't end up emitting a `load` > > instruction for a variable we've already emitted a `destroy` instruction > > for. So just for me, not for users. > Ugh... so we have a function named `deallocate` that doesn't actually > deallocate? Should we rename that? > > It's hard to consider it a security measure when it looks so much like a > use-after-free. Can we make whatever `Desc->DtorFn` resolves to do the > `memset` instead (so it's hidden within `deallocate()` rather than comes > after it)? Doing it in `InterpBlock::invokeDtor()` would be a lot cleaner IMO, however, that is only for the data part, not the metadata. That is important for local variables that we reuse in loops; they get their metadata set once and that's it. I'll just drop this patch since it had limited value anyway. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D142277/new/ https://reviews.llvm.org/D142277 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
