martinboehme wrote:

PTAL
In internal testing, I discovered yet another issue: 
`StmtToEnvMap::getEnvironment()` was returning a “stale” version of the 
environment if the statement `S` is in the block currently being processed. We 
want `getEnvironment()` to return the “pending” environment that we’re 
currently mutating, but in fact, it was returning the environment for the 
_last_ time we processed this block (because this is the environment stored in 
`BlockToState`).

This did not use to matter because a) `StmtToEnvMap::getEnvironment()` is only 
used when evaluating short-circuited boolean operators, b) these are almost 
always used in terminator conditions, and c) we used to evaluate terminator 
conditions outside of `transferCfgBlock()`, after the current state had already 
been written to `BlockToState`. But I believe this was always a bug and would 
have surfaced for short-circuited boolean operators used outside of terminator 
conditions.

Now that we evaluate the terminator condition within `transferCfgBlock()`, 
however, this bug has become very visible. I have added a test to 
SignAnalysisTest.cpp that triggers the bug; this test fails without the changes 
to `StmtToEnvMap` introduced in this latest iteration of the patch.

Most of the changes relative to the previously approved version of this patch 
are in Transfer.h, Transfer.cpp, and SignAnalysisTest.cpp. (Sorry for not 
breaking these changes out into a separate commit – my tooling makes this 
somewhat hard, and I hope the changes are easy enough to digest even without 
this.)

https://github.com/llvm/llvm-project/pull/78127
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to