Skip to content

Commit ef5259a

Browse files
Merge branch 'main' into simd_2024
2 parents 667f1a2 + e2e7d56 commit ef5259a

File tree

100 files changed

+2356
-1340
lines changed

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

100 files changed

+2356
-1340
lines changed

clang/docs/analyzer/user-docs.rst

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -12,3 +12,4 @@ Contents:
1212
user-docs/FilingBugs
1313
user-docs/CrossTranslationUnit
1414
user-docs/TaintAnalysisConfiguration
15+
user-docs/FAQ
Lines changed: 208 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,208 @@
1+
FAQ and How to Deal with Common False Positives
2+
===============================================
3+
4+
.. contents::
5+
:local:
6+
7+
Custom Assertions
8+
-----------------
9+
10+
Q: How do I tell the analyzer that I do not want the bug being reported here since my custom error handler will safely end the execution before the bug is reached?
11+
12+
You can tell the analyzer that this path is unreachable by teaching it about your `custom assertion handlers <annotations.html#custom_assertions>`_. For example, you can modify the code segment as following:
13+
14+
.. code-block:: c
15+
16+
void customAssert() __attribute__((analyzer_noreturn));
17+
int foo(int *b) {
18+
if (!b)
19+
customAssert();
20+
return *b;
21+
}
22+
23+
Null Pointer Dereference
24+
------------------------
25+
26+
Q: The analyzer reports a null dereference, but I know that the pointer is never null. How can I tell the analyzer that a pointer can never be null?
27+
28+
The reason the analyzer often thinks that a pointer can be null is because the preceding code checked compared it against null. If you are absolutely sure that it cannot be null, remove the preceding check and, preferably, add an assertion as well. For example:
29+
30+
.. code-block:: c
31+
32+
void usePointer(int *b);
33+
int foo(int *b) {
34+
usePointer(b);
35+
return *b;
36+
}
37+
38+
Dead Store
39+
----------
40+
41+
Q: How do I tell the static analyzer that I don't care about a specific dead store?
42+
43+
When the analyzer sees that a value stored into a variable is never used, it's going to produce a message similar to this one:
44+
45+
.. code-block:: none
46+
47+
Value stored to 'x' is never read
48+
49+
You can use the ``(void)x;`` idiom to acknowledge that there is a dead store in your code but you do not want it to be reported in the future.
50+
51+
Unused Instance Variable
52+
------------------------
53+
54+
Q: How do I tell the static analyzer that I don't care about a specific unused instance variable in Objective-C?
55+
56+
When the analyzer sees that a value stored into a variable is never used, it is going to produce a message similar to this one:
57+
58+
.. code-block:: none
59+
60+
Instance variable 'commonName' in class 'HappyBird' is never used by the methods in its @implementation
61+
62+
You can add ``__attribute__((unused))`` to the instance variable declaration to suppress the warning.
63+
64+
Unlocalized String
65+
------------------
66+
67+
Q: How do I tell the static analyzer that I don't care about a specific unlocalized string?
68+
69+
When the analyzer sees that an unlocalized string is passed to a method that will present that string to the user, it is going to produce a message similar to this one:
70+
71+
.. code-block:: none
72+
73+
User-facing text should use localized string macro
74+
75+
If your project deliberately uses unlocalized user-facing strings (for example, in a debugging UI that is never shown to users), you can suppress the analyzer warnings (and document your intent) with a function that just returns its input but is annotated to return a localized string:
76+
77+
.. code-block:: objc
78+
79+
__attribute__((annotate("returns_localized_nsstring")))
80+
static inline NSString *LocalizationNotNeeded(NSString *s) {
81+
return s;
82+
}
83+
84+
You can then call this function when creating your debugging UI:
85+
86+
.. code-block:: objc
87+
88+
[field setStringValue:LocalizationNotNeeded(@"Debug")];
89+
90+
Some projects may also find it useful to use NSLocalizedString but add "DNL" or "Do Not Localize" to the string contents as a convention:
91+
92+
.. code-block:: objc
93+
94+
UILabel *testLabel = [[UILabel alloc] init];
95+
NSString *s = NSLocalizedString(@"Hello <Do Not Localize>", @"For debug purposes");
96+
[testLabel setText:s];
97+
98+
Dealloc in Manual Retain/Release
99+
--------------------------------
100+
101+
Q: How do I tell the analyzer that my instance variable does not need to be released in -dealloc under Manual Retain/Release?
102+
103+
If your class only uses an instance variable for part of its lifetime, it may maintain an invariant guaranteeing that the instance variable is always released before -dealloc. In this case, you can silence a warning about a missing release by either adding ``assert(_ivar == nil)`` or an explicit release ``[_ivar release]`` (which will be a no-op when the variable is nil) in -dealloc.
104+
105+
Deciding Nullability
106+
--------------------
107+
108+
Q: How do I decide whether a method's return type should be _Nullable or _Nonnull?
109+
110+
Depending on the implementation of the method, this puts you in one of five situations:
111+
112+
1. You actually never return nil.
113+
2. You do return nil sometimes, and callers are supposed to handle that. This includes cases where your method is documented to return nil given certain inputs.
114+
3. You return nil based on some external condition (such as an out-of-memory error), but the client can't do anything about it either.
115+
4. You return nil only when the caller passes input documented to be invalid. That means it's the client's fault.
116+
5. You return nil in some totally undocumented case.
117+
118+
In (1) you should annotate the method as returning a ``_Nonnull`` object.
119+
120+
In (2) the method should be marked ``_Nullable``.
121+
122+
In (3) you should probably annotate the method ``_Nonnull``. Why? Because no callers will actually check for nil, given that they can't do anything about the situation and don't know what went wrong. At this point things have gone so poorly that there's basically no way to recover.
123+
124+
The least happy case is (4) because the resulting program will almost certainly either crash or just silently do the wrong thing. If this is a new method or you control the callers, you can use ``NSParameterAssert()`` (or the equivalent) to check the precondition and remove the nil return. But if you don't control the callers and they rely on this behavior, you should return mark the method ``_Nonnull`` and return nil cast to _Nonnull anyway.
125+
126+
If you're in (5), document it, then figure out if you're now in (2), (3), or (4).
127+
128+
Intentional Nullability Violation
129+
---------------------------------
130+
131+
Q: How do I tell the analyzer that I am intentionally violating nullability?
132+
133+
In some cases, it may make sense for methods to intentionally violate nullability. For example, your method may — for reasons of backward compatibility — chose to return nil and log an error message in a method with a non-null return type when the client violated a documented precondition rather than check the precondition with ``NSAssert()``. In these cases, you can suppress the analyzer warning with a cast:
134+
135+
.. code-block:: objc
136+
137+
return (id _Nonnull)nil;
138+
139+
Note that this cast does not affect code generation.
140+
141+
Ensuring Loop Body Execution
142+
----------------------------
143+
144+
Q: The analyzer assumes that a loop body is never entered. How can I tell it that the loop body will be entered at least once?
145+
146+
In cases where you know that a loop will always be entered at least once, you can use assertions to inform the analyzer. For example:
147+
148+
.. code-block:: c
149+
150+
int foo(int length) {
151+
int x = 0;
152+
assert(length > 0);
153+
for (int i = 0; i < length; i++)
154+
x += 1;
155+
return length/x;
156+
}
157+
158+
By adding ``assert(length > 0)`` in the beginning of the function, you tell the analyzer that your code is never expecting a zero or a negative value, so it won't need to test the correctness of those paths.
159+
160+
Suppressing Specific Warnings
161+
-----------------------------
162+
163+
Q: How can I suppress a specific analyzer warning?
164+
165+
When you encounter an analyzer bug/false positive, check if it's one of the issues discussed above or if the analyzer `annotations <annotations.html#custom_assertions>`_ can resolve the issue by helping the static analyzer understand the code better. Second, please `report it <filing_bugs.html>`_ to help us improve user experience.
166+
167+
Sometimes there's really no "good" way to eliminate the issue. In such cases you can "silence" it directly by annotating the problematic line of code with the help of Clang attribute 'suppress':
168+
169+
.. code-block:: c
170+
171+
int foo() {
172+
int *x = nullptr;
173+
...
174+
[[clang::suppress]] {
175+
// all warnings in this scope are suppressed
176+
int y = *x;
177+
}
178+
179+
// null pointer dereference warning suppressed on the next line
180+
[[clang::suppress]]
181+
return *x
182+
}
183+
184+
int bar(bool coin_flip) {
185+
// suppress all memory leak warnings about this allocation
186+
[[clang::suppress]]
187+
int *result = (int *)malloc(sizeof(int));
188+
189+
if (coin_flip)
190+
return 0; // including this leak path
191+
192+
return *result; // as well as this leak path
193+
}
194+
195+
Excluding Code from Analysis
196+
----------------------------
197+
198+
Q: How can I selectively exclude code the analyzer examines?
199+
200+
When the static analyzer is using clang to parse source files, it implicitly defines the preprocessor macro ``__clang_analyzer__``. One can use this macro to selectively exclude code the analyzer examines. Here is an example:
201+
202+
.. code-block:: c
203+
204+
#ifndef __clang_analyzer__
205+
// Code not to be analyzed
206+
#endif
207+
208+
This usage is discouraged because it makes the code dead to the analyzer from now on. Instead, we prefer that users file bugs against the analyzer when it flags false positives.

clang/include/clang/StaticAnalyzer/Core/PathSensitive/CallEvent.h

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -690,6 +690,11 @@ class CXXInstanceCall : public AnyFunctionCall {
690690
ValueList &Values,
691691
RegionAndSymbolInvalidationTraits *ETraits) const override;
692692

693+
/// Returns the decl refered to by the "dynamic type" of the current object
694+
/// and if the class can be a sub-class or not.
695+
/// If the Pointer is null, the flag has no meaning.
696+
std::pair<const CXXRecordDecl *, bool> getDeclForDynamicType() const;
697+
693698
public:
694699
/// Returns the expression representing the implicit 'this' object.
695700
virtual const Expr *getCXXThisExpr() const { return nullptr; }

clang/lib/StaticAnalyzer/Core/CallEvent.cpp

Lines changed: 29 additions & 28 deletions
Original file line numberDiff line numberDiff line change
@@ -711,18 +711,17 @@ void CXXInstanceCall::getExtraInvalidatedValues(
711711
if (const auto *D = cast_or_null<CXXMethodDecl>(getDecl())) {
712712
if (!D->isConst())
713713
return;
714-
// Get the record decl for the class of 'This'. D->getParent() may return a
715-
// base class decl, rather than the class of the instance which needs to be
716-
// checked for mutable fields.
717-
// TODO: We might as well look at the dynamic type of the object.
718-
const Expr *Ex = getCXXThisExpr()->IgnoreParenBaseCasts();
719-
QualType T = Ex->getType();
720-
if (T->isPointerType()) // Arrow or implicit-this syntax?
721-
T = T->getPointeeType();
722-
const CXXRecordDecl *ParentRecord = T->getAsCXXRecordDecl();
723-
assert(ParentRecord);
714+
715+
// Get the record decl for the class of 'This'. D->getParent() may return
716+
// a base class decl, rather than the class of the instance which needs to
717+
// be checked for mutable fields.
718+
const CXXRecordDecl *ParentRecord = getDeclForDynamicType().first;
719+
if (!ParentRecord || !ParentRecord->hasDefinition())
720+
return;
721+
724722
if (ParentRecord->hasMutableFields())
725723
return;
724+
726725
// Preserve CXXThis.
727726
const MemRegion *ThisRegion = ThisVal.getAsRegion();
728727
if (!ThisRegion)
@@ -748,6 +747,21 @@ SVal CXXInstanceCall::getCXXThisVal() const {
748747
return ThisVal;
749748
}
750749

750+
std::pair<const CXXRecordDecl *, bool>
751+
CXXInstanceCall::getDeclForDynamicType() const {
752+
const MemRegion *R = getCXXThisVal().getAsRegion();
753+
if (!R)
754+
return {};
755+
756+
DynamicTypeInfo DynType = getDynamicTypeInfo(getState(), R);
757+
if (!DynType.isValid())
758+
return {};
759+
760+
assert(!DynType.getType()->getPointeeType().isNull());
761+
return {DynType.getType()->getPointeeCXXRecordDecl(),
762+
DynType.canBeASubClass()};
763+
}
764+
751765
RuntimeDefinition CXXInstanceCall::getRuntimeDefinition() const {
752766
// Do we have a decl at all?
753767
const Decl *D = getDecl();
@@ -759,21 +773,7 @@ RuntimeDefinition CXXInstanceCall::getRuntimeDefinition() const {
759773
if (!MD->isVirtual())
760774
return AnyFunctionCall::getRuntimeDefinition();
761775

762-
// Do we know the implicit 'this' object being called?
763-
const MemRegion *R = getCXXThisVal().getAsRegion();
764-
if (!R)
765-
return {};
766-
767-
// Do we know anything about the type of 'this'?
768-
DynamicTypeInfo DynType = getDynamicTypeInfo(getState(), R);
769-
if (!DynType.isValid())
770-
return {};
771-
772-
// Is the type a C++ class? (This is mostly a defensive check.)
773-
QualType RegionType = DynType.getType()->getPointeeType();
774-
assert(!RegionType.isNull() && "DynamicTypeInfo should always be a pointer.");
775-
776-
const CXXRecordDecl *RD = RegionType->getAsCXXRecordDecl();
776+
auto [RD, CanBeSubClass] = getDeclForDynamicType();
777777
if (!RD || !RD->hasDefinition())
778778
return {};
779779

@@ -800,16 +800,17 @@ RuntimeDefinition CXXInstanceCall::getRuntimeDefinition() const {
800800
// Does the decl that we found have an implementation?
801801
const FunctionDecl *Definition;
802802
if (!Result->hasBody(Definition)) {
803-
if (!DynType.canBeASubClass())
803+
if (!CanBeSubClass)
804804
return AnyFunctionCall::getRuntimeDefinition();
805805
return {};
806806
}
807807

808808
// We found a definition. If we're not sure that this devirtualization is
809809
// actually what will happen at runtime, make sure to provide the region so
810810
// that ExprEngine can decide what to do with it.
811-
if (DynType.canBeASubClass())
812-
return RuntimeDefinition(Definition, R->StripCasts());
811+
if (CanBeSubClass)
812+
return RuntimeDefinition(Definition,
813+
getCXXThisVal().getAsRegion()->StripCasts());
813814
return RuntimeDefinition(Definition, /*DispatchRegion=*/nullptr);
814815
}
815816

clang/test/Analysis/const-method-call.cpp

Lines changed: 46 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -271,3 +271,49 @@ void checkThatConstMethodCallDoesInvalidateObjectForCircularReferences() {
271271
// FIXME: Should be UNKNOWN.
272272
clang_analyzer_eval(t.x); // expected-warning{{TRUE}}
273273
}
274+
275+
namespace gh77378 {
276+
template <typename Signature> class callable;
277+
278+
template <typename R> class callable<R()> {
279+
struct CallableType {
280+
bool operator()();
281+
};
282+
using MethodType = R (CallableType::*)();
283+
CallableType *object_{nullptr};
284+
MethodType method_;
285+
286+
public:
287+
callable() = default;
288+
289+
template <typename T>
290+
constexpr callable(const T &obj)
291+
: object_{reinterpret_cast<CallableType *>(&const_cast<T &>(obj))},
292+
method_{reinterpret_cast<MethodType>(
293+
static_cast<bool (T::*)() const>(&T::operator()))} {}
294+
295+
constexpr bool const_method() const {
296+
return (object_->*(method_))();
297+
}
298+
299+
callable call() const & {
300+
static const auto L = [this]() {
301+
while (true) {
302+
// This should not crash when conservative eval calling the member function
303+
// when it unwinds the call stack due to exhausting the budget or reaching
304+
// the inlining limit.
305+
if (this->const_method()) {
306+
break;
307+
}
308+
}
309+
return true;
310+
};
311+
return L;
312+
}
313+
};
314+
315+
void entry() {
316+
callable<bool()>{}.call().const_method();
317+
// expected-warning@-1 {{Address of stack memory associated with temporary object of type 'callable<bool ()>' is still referred to by the static variable 'L' upon returning to the caller. This will be a dangling reference}}
318+
}
319+
} // namespace gh77378

0 commit comments

Comments
 (0)