================
@@ -380,37 +309,41 @@ void BlockInCriticalSectionChecker::checkPostCall(const
CallEvent &Call,
return;
}
- if (std::optional<MutexDescriptor> LockDesc =
- checkDescriptorMatch(Call, C, /*IsLock=*/true)) {
- if (!std::holds_alternative<RAIIMutexDescriptor>(*LockDesc))
- handleLock(*LockDesc, Call, C, C.getState());
+ const ThreadingCallDescription *Desc = lookupThreadingCall(Call);
+ if (!Desc)
return;
- }
- if (std::optional<MutexDescriptor> UnlockDesc =
- checkDescriptorMatch(Call, C, /*IsLock=*/false)) {
- handleUnlock(*UnlockDesc, Call, C);
+
+ // RAII constructors are modeled in evalCall so they are not inlined.
+ if (isa<CXXConstructorCall>(Call))
+ return;
+
+ switch (Desc->Role) {
+ case Role::Lock:
+ handleLock(*Desc, Call, C, C.getState());
+ break;
+ case Role::Unlock:
+ handleUnlock(*Desc, Call, C);
+ break;
}
}
bool BlockInCriticalSectionChecker::evalCall(const CallEvent &Call,
CheckerContext &C) const {
- if (std::optional<MutexDescriptor> LockDesc =
- checkDescriptorMatch(Call, C, /*IsLock=*/true)) {
- if (std::holds_alternative<RAIIMutexDescriptor>(*LockDesc)) {
- ProgramStateRef State = C.getState();
- // Escape the object under construction to model the side-effects of the
- // constructor.
- if (const auto *Ctor = dyn_cast<AnyCXXConstructorCall>(&Call)) {
- const MemRegion *ObjRegion = Ctor->getCXXThisVal().getAsRegion();
- State = State->invalidateRegions(ObjRegion, C.getCFGElementRef(),
- C.blockCount(), C.getStackFrame(),
- /*CausesPointerEscape=*/false);
- }
- handleLock(*LockDesc, Call, C, State);
- return true;
- }
+ const ThreadingCallDescription *Desc = lookupThreadingCall(Call);
+ if (!Desc || !isa<CXXConstructorCall>(Call))
+ return false;
----------------
steakhal wrote:
Also here, claude thinks that:
Previously the gate was
std::holds_alternative<RAIIMutexDescriptor>(*LockDesc); now any matched entry
that happens to
be a constructor gets evaluated as a lock, suppresses inlining, and
invalidates the object — including the
Role::Unlock entries. Structurally the function claims to model "RAII lock
guard constructors" (per the new comment at
:222) but the condition it actually tests is "some threading call that is a
constructor".
Today the only entries a constructor can name-match are {"std","lock"} /
{"std","unlock"} (a class std::lock /
std::unlock), so this is latent rather than exploitable — but it is a
foot-gun for the next entry added to the map,
and it makes Role dead in evalCall. Consider keying off Desc->GetRegion ==
getObjectUnderConstruction, or adding an
explicit IsRAIICtor flag / asserting Desc->Role == Role::Lock.
Evidence: not verified as a behavior change — I could not build a
non-contrived program that reaches it without also
tripping the pre-existing cast<CXXMemberCall> assertion in getCXXThisRegion
(which base hits too, from checkPostCall;
see below). Reported as a robustness concern, not a regression.
https://github.com/llvm/llvm-project/pull/224230
_______________________________________________
cfe-commits mailing list
[email protected]
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
