[clang] [clang][dataflow] Process terminator condition within `transferCFGBlock()`. (PR #78127)

via cfe-commits cfe-commits at lists.llvm.org
Wed Jan 17 23:44:24 PST 2024


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


More information about the cfe-commits mailing list