merrymeerkat added inline comments.
================ Comment at: clang/unittests/Analysis/FlowSensitive/TransferTest.cpp:1551-1552 + const Value *FooVal = Env.getValue(*FooLoc); + // TODO: Initialise values inside unions, then change below to + // ASSERT_TRUE. + ASSERT_FALSE(isa_and_nonnull<IntegerValue>(FooVal)); ---------------- ymandel wrote: > merrymeerkat wrote: > > ymandel wrote: > > > Why push this off to another patch? > > good point! I was pushing it because this is just a quick fix to avoid a > > crash, and the current changes are sufficient for that > SGTM. Please use FIXME instead of TODO, though. But, glad to see this is > enough to avoid the crashing! > > That said, can you expand on where the actual crash was happening? I'm > concerned that its possible that the crash site should be more robust and > that this patch is hiding that weakness. Done! The crash was happening because of a null pointer cast in the builtin transfer function for CFGInitializers: https://github.com/llvm/llvm-project/blob/781eabeb40b8e47e3a46b0b927784e63f0aad9ab/clang/lib/Analysis/FlowSensitive/TypeErasedDataflowAnalysis.cpp#L316 Hmm so do you think it'd be helpful to add a null check in the file above? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D140696/new/ https://reviews.llvm.org/D140696 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
