=?utf-8?q?Donát?= Nagy <donat.n...@ericsson.com>
Message-ID:
In-Reply-To: <llvm.org/llvm/llvm-project/pull/72...@github.com>
================
@@ -81,22 +81,21 @@ bool BuiltinFunctionChecker::evalCall(const CallEvent &Call,
case Builtin::BI__builtin_alloca_with_align:
case Builtin::BI__builtin_alloca: {
- // FIXME: Refactor into StoreManager itself?
- MemRegionManager& RM = C.getStoreManager().getRegionManager();
- const AllocaRegion* R =
- RM.getAllocaRegion(CE, C.blockCount(), C.getLocationContext());
-
- // Set the extent of the region in bytes. This enables us to use the
- // SVal of the argument directly. If we save the extent in bits, we
- // cannot represent values like symbol*8.
- auto Size = Call.getArgSVal(0);
- if (Size.isUndef())
- return true; // Return true to model purity.
-
- state = setDynamicExtent(state, R, Size.castAs<DefinedOrUnknownSVal>(),
- C.getSValBuilder());
+ SValBuilder &SVB = C.getSValBuilder();
+ const loc::MemRegionVal R =
+ SVB.getAllocaRegionVal(CE, C.getLocationContext(), C.blockCount());
- C.addTransition(state->BindExpr(CE, LCtx, loc::MemRegionVal(R)));
+ // Set the extent of the region in bytes. This enables us to use the SVal
+ // of the argument directly. If we saved the extent in bits, it'd be more
+ // difficult to reason about values like symbol*8.
+ auto Size = Call.getArgSVal(0);
+ if (auto DefSize = Size.getAs<DefinedOrUnknownSVal>()) {
+ state = setDynamicExtent(state, R.getRegion(), *DefSize, SVB);
+ // FIXME: perhaps the following transition should be moved out of the
----------------
DonatNagyE wrote:
I checked the situation with an assert and a straightforward testcase, and it
turns out that currently the `getAs()` always succeeds, because
`core.CallAndMessage` stops the execution with a bug report when an undefined
argument is passed to a function. (However, if I disabled this functionality of
`core.CallAndMessage`, the test -- obviously -- triggered the assertion.)
Based on this, I decided to keep the current defensive check, because I didn't
want to add a hidden and marginal dependency relationship between these two
checkers. I know that we may assume that `core` checkers are always on, but
that doesn't mean that I should hide landmines like `assert(core.CallAndMessage
is active and works as I assume)` in unrelated checkers.
I updated the comment to explain this situation, and I also moved the
`addTransition()` call out of the `if` because that's the more logical place
for that method call and this `supporting fire` from `core.CallAndMessage`
guarantees that this change won't have unintended side effects (because the
`if` succeeds in normal operation).
https://github.com/llvm/llvm-project/pull/72402
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
