Magic-Mirror/llvm-capstone
1
0
Fork 0
mirror of https://github.com/capstone-engine/llvm-capstone.git synced 2024-11-27 15:41:46 +00:00
Code Issues Actions 2 Packages Projects Releases Wiki Activity

[analyzer] Nullability: Enable analysis of non-inlined nullable objc properties.

Browse Source 
The NullabilityChecker has a very early policy decision that non-inlined
property accesses will be inferred as returning nonnull, despite nullability
annotations to the contrary. This decision eliminates false positives related
to very common code patterns that look like this:

if (foo.prop) {
    [bar doStuffWithNonnull:foo.prop];
}

While this probably represents a correct nil-check, the analyzer can't
determine correctness without gaining visibility into the property
implementation.

Unfortunately, inferring nullable properties as nonnull comes at the cost of
significantly reduced code coverage. My goal here is to enable detection of
many property-related nullability violations without a large increase
in false positives.

The approach is to introduce a heuristic: after accessing the value of
a property, if the analyzer at any time proves that the property value is
nonnull (which would happen in particular due to a nil-check conditional),
then subsequent property accesses on that code path will be *inferred*
as nonnull. This captures the pattern described above, which I believe
to be the dominant source of false positives in real code.

https://reviews.llvm.org/D131655
This commit is contained in:
Paul Pelzl 2022-12-12 13:53:16 -08:00 committed by Artem Dergachev
parent 1b753240d5
commit 6ab01d4a5c
2 changed files with 178 additions and 8 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

129
clang/lib/StaticAnalyzer/Checkers/NullabilityChecker.cpp
View File

@ -80,7 +80,7 @@ enum class ErrorKind : int {
class NullabilityChecker
: public Checker<check::Bind, check::PreCall, check::PreStmt<ReturnStmt>,
check::PostCall, check::PostStmt<ExplicitCastExpr>,
check::PostObjCMessage, check::DeadSymbols,
check::PostObjCMessage, check::DeadSymbols, eval::Assume,
check::Location, check::Event<ImplicitNullDerefEvent>> {
public:
@ -102,6 +102,8 @@ public:
void checkEvent(ImplicitNullDerefEvent Event) const;
void checkLocation(SVal Location, bool IsLoad, const Stmt *S,
CheckerContext &C) const;
ProgramStateRef evalAssume(ProgramStateRef State, SVal Cond,
bool Assumption) const;
void printState(raw_ostream &Out, ProgramStateRef State, const char *NL,
const char *Sep) const override;
@ -129,7 +131,7 @@ public:
// When set to false no nullability information will be tracked in
// NullabilityMap. It is possible to catch errors like passing a null pointer
// to a callee that expects nonnull argument without the information that is
// stroed in the NullabilityMap. This is an optimization.
// stored in the NullabilityMap. This is an optimization.
bool NeedTracking = false;
private:
@ -230,10 +232,41 @@ bool operator==(NullabilityState Lhs, NullabilityState Rhs) {
Lhs.getNullabilitySource() == Rhs.getNullabilitySource();
}
// For the purpose of tracking historical property accesses, the key for lookup
// is an object pointer (could be an instance or a class) paired with the unique
// identifier for the property being invoked on that object.
using ObjectPropPair = std::pair<const MemRegion *, const IdentifierInfo *>;
// Metadata associated with the return value from a recorded property access.
struct ConstrainedPropertyVal {
// This will reference the conjured return SVal for some call
// of the form [object property]
DefinedOrUnknownSVal Value;
// If the SVal has been determined to be nonnull, that is recorded here
bool isConstrainedNonnull;
ConstrainedPropertyVal(DefinedOrUnknownSVal SV)
: Value(SV), isConstrainedNonnull(false) {}
void Profile(llvm::FoldingSetNodeID &ID) const {
Value.Profile(ID);
ID.AddInteger(isConstrainedNonnull ? 1 : 0);
}
};
bool operator==(const ConstrainedPropertyVal &Lhs,
const ConstrainedPropertyVal &Rhs) {
return Lhs.Value == Rhs.Value &&
Lhs.isConstrainedNonnull == Rhs.isConstrainedNonnull;
}
} // end anonymous namespace
REGISTER_MAP_WITH_PROGRAMSTATE(NullabilityMap, const MemRegion *,
NullabilityState)
REGISTER_MAP_WITH_PROGRAMSTATE(PropertyAccessesMap, ObjectPropPair,
ConstrainedPropertyVal)
// We say "the nullability type invariant is violated" when a location with a
// non-null type contains NULL or a function with a non-null return type returns
@ -467,6 +500,19 @@ void NullabilityChecker::checkDeadSymbols(SymbolReaper &SR,
State = State->remove<NullabilityMap>(I->first);
}
}
// When an object goes out of scope, we can free the history associated
// with any property accesses on that object
PropertyAccessesMapTy PropertyAccesses = State->get<PropertyAccessesMap>();
for (PropertyAccessesMapTy::iterator I = PropertyAccesses.begin(),
E = PropertyAccesses.end();
I != E; ++I) {
const MemRegion *ReceiverRegion = I->first.first;
if (!SR.isLiveRegion(ReceiverRegion)) {
State = State->remove<PropertyAccessesMap>(I->first);
}
}
// When one of the nonnull arguments are constrained to be null, nullability
// preconditions are violated. It is not enough to check this only when we
// actually report an error, because at that time interesting symbols might be
@ -854,6 +900,32 @@ static Nullability getReceiverNullability(const ObjCMethodCall &M,
return Nullability::Unspecified;
}
// The return value of a property access is typically a temporary value which
// will not be tracked in a persistent manner by the analyzer. We use
// evalAssume() in order to immediately record constraints on those temporaries
// at the time they are imposed (e.g. by a nil-check conditional).
ProgramStateRef NullabilityChecker::evalAssume(ProgramStateRef State, SVal Cond,
bool Assumption) const {
PropertyAccessesMapTy PropertyAccesses = State->get<PropertyAccessesMap>();
for (PropertyAccessesMapTy::iterator I = PropertyAccesses.begin(),
E = PropertyAccesses.end();
I != E; ++I) {
if (!I->second.isConstrainedNonnull) {
ConditionTruthVal IsNonNull = State->isNonNull(I->second.Value);
if (IsNonNull.isConstrainedTrue()) {
ConstrainedPropertyVal Replacement = I->second;
Replacement.isConstrainedNonnull = true;
State = State->set<PropertyAccessesMap>(I->first, Replacement);
} else if (IsNonNull.isConstrainedFalse()) {
// Space optimization: no point in tracking constrained-null cases
State = State->remove<PropertyAccessesMap>(I->first);
}
}
}
return State;
}
/// Calculate the nullability of the result of a message expr based on the
/// nullability of the receiver, the nullability of the return value, and the
/// constraints.
@ -950,12 +1022,53 @@ void NullabilityChecker::checkPostObjCMessage(const ObjCMethodCall &M,
// No tracked information. Use static type information for return value.
Nullability RetNullability = getNullabilityAnnotation(RetType);
// Properties might be computed. For this reason the static analyzer creates a
// new symbol each time an unknown property is read. To avoid false pozitives
// do not treat unknown properties as nullable, even when they explicitly
// marked nullable.
if (M.getMessageKind() == OCM_PropertyAccess && !C.wasInlined)
RetNullability = Nullability::Nonnull;
// Properties might be computed, which means the property value could
// theoretically change between calls even in commonly-observed cases like
// this:
//
// if (foo.prop) { // ok, it's nonnull here...
// [bar doStuffWithNonnullVal:foo.prop]; // ...but what about
// here?
// }
//
// If the property is nullable-annotated, a naive analysis would lead to many
// false positives despite the presence of probably-correct nil-checks. To
// reduce the false positive rate, we maintain a history of the most recently
// observed property value. For each property access, if the prior value has
// been constrained to be not nil then we will conservatively assume that the
// next access can be inferred as nonnull.
if (RetNullability != Nullability::Nonnull &&
M.getMessageKind() == OCM_PropertyAccess && !C.wasInlined) {
bool LookupResolved = false;
if (const MemRegion *ReceiverRegion = getTrackRegion(M.getReceiverSVal())) {
if (IdentifierInfo *Ident = M.getSelector().getIdentifierInfoForSlot(0)) {
LookupResolved = true;
ObjectPropPair Key = std::make_pair(ReceiverRegion, Ident);
const ConstrainedPropertyVal *PrevPropVal =
State->get<PropertyAccessesMap>(Key);
if (PrevPropVal && PrevPropVal->isConstrainedNonnull) {
RetNullability = Nullability::Nonnull;
} else {
// If a previous property access was constrained as nonnull, we hold
// on to that constraint (effectively inferring that all subsequent
// accesses on that code path can be inferred as nonnull). If the
// previous property access was *not* constrained as nonnull, then
// let's throw it away in favor of keeping the SVal associated with
// this more recent access.
if (auto ReturnSVal =
M.getReturnValue().getAs<DefinedOrUnknownSVal>()) {
State = State->set<PropertyAccessesMap>(
Key, ConstrainedPropertyVal(*ReturnSVal));
}
}
}
}
if (!LookupResolved) {
// Fallback: err on the side of suppressing the false positive.
RetNullability = Nullability::Nonnull;
}
}
Nullability ComputedNullab = getMostNullable(RetNullability, SelfNullability);
if (ComputedNullab == Nullability::Nullable) {

57
clang/test/Analysis/nullability.mm
View File

@ -34,6 +34,13 @@
#include "Inputs/system-header-simulator-for-nullability.h"
extern void __assert_fail(__const char *__assertion, __const char *__file,
unsigned int __line, __const char *__function)
__attribute__((__noreturn__));
#define assert(expr) \
((expr) ? (void)(0) : __assert_fail(#expr, __FILE__, __LINE__, __func__))
@interface TestObject : NSObject
- (int *_Nonnull)returnsNonnull;
- (int *_Nullable)returnsNullable;
@ -42,6 +49,9 @@
- (void)takesNullable:(int *_Nullable)p;
- (void)takesUnspecified:(int *)p;
@property(readonly, strong) NSString *stuff;
@property(readonly, nonnull) int *propReturnsNonnull;
@property(readonly, nullable) int *propReturnsNullable;
@property(readonly) int *propReturnsUnspecified;
@end
TestObject * getUnspecifiedTestObject();
@ -182,6 +192,53 @@ void testObjCMessageResultNullability() {
}
}
void testObjCPropertyReadNullability() {
TestObject *o = getNonnullTestObject();
switch (getRandom()) {
case 0:
[o takesNonnull:o.propReturnsNonnull]; // no-warning
break;
case 1:
[o takesNonnull:o.propReturnsUnspecified]; // no-warning
break;
case 2:
[o takesNonnull:o.propReturnsNullable]; // expected-warning {{Nullable pointer is passed to a callee that requires a non-null 1st parameter}}
break;
case 3:
// If a property is constrained nonnull, assume it remains nonnull
if (o.propReturnsNullable) {
[o takesNonnull:o.propReturnsNullable]; // no-warning
[o takesNonnull:o.propReturnsNullable]; // no-warning
}
break;
case 4:
if (!o.propReturnsNullable) {
[o takesNonnull:o.propReturnsNullable]; // expected-warning {{Nullable pointer is passed to a callee that requires a non-null 1st parameter}}
}
break;
case 5:
if (!o.propReturnsNullable) {
if (o.propReturnsNullable) {
// Nonnull constraint from the more recent call wins
[o takesNonnull:o.propReturnsNullable]; // no-warning
}
}
break;
case 6:
// Constraints on property return values are receiver-qualified
if (o.propReturnsNullable) {
[o takesNonnull:o.propReturnsNullable]; // no-warning
[o takesNonnull:getNonnullTestObject().propReturnsNullable]; // expected-warning {{Nullable pointer is passed to a callee that requires a non-null 1st parameter}}
}
break;
case 7:
// Assertions must be effective at suppressing warnings
assert(o.propReturnsNullable);
[o takesNonnull:o.propReturnsNullable]; // no-warning
break;
}
}
Dummy * _Nonnull testDirectCastNullableToNonnull() {
Dummy *p = returnsNullable();
takesNonnull((Dummy * _Nonnull)p); // no-warning