1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
20
21
22
23
24
25
26
27
28
29
30
31
32
33
34
35
36
37
38
39
40
41
42
43
44
45
46
47
48
49
50
51
52
53
54
55
56
57
58
59
60
61
62
63
64
65
66
67
68
69
70
71
72
73
74
75
76
77
78
79
80
81
82
83
84
85
86
87
88
89
90
91
92
93
94
95
96
97
98
99
100
101
102
103
104
105
106
107
108
109
110
111
112
113
114
115
116
117
118
119
120
121
122
123
124
125
126
127
128
129
130
131
132
133
134
135
136
137
138
139
140
141
142
143
144
145
146
147
148
|
.. _advanced_check_features:
Advanced Check Features
=======================
This page covers additional ways to improve and extend the check you've added to build/clang-plugin.
Adding Tests
------------
No doubt you've seen the tests for existing checks in `build/clang-plugin/tests <https://searchfox.org/mozilla-central/source/build/clang-plugin/tests>`_. Adding tests is straightforward; and your reviewer should insist you do so. Simply copying the existing format of any test and how diagnostics are marked as expected.
One wrinkle - all clang plugin checks are applied to all tests. We try to write tests so that only one check applies to it. If you write a check that triggers on an existing test, try to fix the existing test slightly so the new check does not trigger on it.
Using Bind To Output More Useful Information
--------------------------------------------
You've probably been wondering what the heck ``.bind()`` is for. You've been seeing it all over the place but never has it actually been explained what it's for and when to use it.
``.bind()`` is used to give a name to part of the AST discovered through your matcher, so you can use it later. Let's go back to our sample matcher:
::
AstMatcher->addMatcher(
traverse(TK_IgnoreUnlessSpelledInSource,
ifStmt(allOf(
has(
binaryOperator(
has(
declRefExpr(hasType(enumDecl()))
)
)
),
hasElse(
ifStmt(allOf(
unless(hasElse(anything())),
has(
binaryOperator(
has(
declRefExpr(hasType(enumDecl()))
)
)
)
))
)
))
.bind("node")),
this);
Now the ``.bind("node")`` makes more sense. We're naming the If statement we matched, so we can refer to it later when we call ``Result.Nodes.getNodeAs<IfStmt>("node")``.
Let's say we want to provide the *type* of the enum in our warning message. There are two enums we end up seeing in our matcher - the enum in the first if statement, and the enum in the second. We're going to arbitrarily pick the first and give it the name ``enumType``:
::
AstMatcher->addMatcher(
traverse(TK_IgnoreUnlessSpelledInSource,
ifStmt(allOf(
has(
binaryOperator(
has(
declRefExpr(hasType(enumDecl().bind("enumType")))
)
)
),
hasElse(
ifStmt(allOf(
unless(hasElse(anything())),
has(
binaryOperator(
has(
declRefExpr(hasType(enumDecl()))
)
)
)
))
)
))
.bind("node")),
this);
And in our check() function, we can use it like so:
::
void MissingElseInEnumComparisons::check(
const MatchFinder::MatchResult &Result) {
const auto *MatchedDecl = Result.Nodes.getNodeAs<IfStmt>("node");
const auto *EnumType = Result.Nodes.getNodeAs<EnumDecl>("enumType");
diag(MatchedDecl->getIfLoc(),
"Enum comparisons to %0 in an if/else if block without a trailing else.",
DiagnosticIDs::Warning) << EnumType->getName();
}
Repeated matcher calls
--------------------------
If you find yourself repeating the same several matchers in several spots, you can turn it into a variable to use.
::
auto isTemporaryLifetimeBoundCall =
cxxMemberCallExpr(
onImplicitObjectArgument(anyOf(has(cxxTemporaryObjectExpr()),
has(materializeTemporaryExpr()))),
callee(functionDecl(isMozTemporaryLifetimeBound())));
auto hasTemporaryLifetimeBoundCall =
anyOf(isTemporaryLifetimeBoundCall,
conditionalOperator(
anyOf(hasFalseExpression(isTemporaryLifetimeBoundCall),
hasTrueExpression(isTemporaryLifetimeBoundCall))));
The above example is parameter-less, but if you need to supply a parameter that changes, you can turn it into a lambda:
::
auto hasConstCharPtrParam = [](const unsigned int Position) {
return hasParameter(
Position, hasType(hasCanonicalType(pointsTo(asString("const char")))));
};
auto hasParamOfType = [](const unsigned int Position, const char *Name) {
return hasParameter(Position, hasType(asString(Name)));
};
auto hasIntegerParam = [](const unsigned int Position) {
return hasParameter(Position, hasType(isInteger()));
};
AstMatcher->addMatcher(
callExpr(
hasName("fopen"),
hasConstCharPtrParam(0))
.bind("funcCall"),
this);
Allow-listing existing callsites
--------------------------------
While it's not a great situation, you can set up an allow-list of existing callsites if you need to. A simple allow-list is demonstrated in `NoGetPrincipalURI <https://hg.mozilla.org/mozilla-central/rev/fb60b22ee6616521b386d90aec07b03b77905f4e>`_. The `NoNewThreadsChecker <https://hg.mozilla.org/mozilla-central/rev/f400f164b3947b4dd54089a36ea31cca2d72805b>`_ is an example of a more sophisticated way of setting up a larger allow-list.
Custom Annotations
------------------
It's possible to create custom annotations that will be a no-op when compiled, but can be used by a static analysis check. These can be used to annotate special types of sources and sinks (for example). We have some examples of this in-tree presently (such as ``MOZ_CAN_RUN_SCRIPT``) but currently don't have a detailed walkthrough in this documentation of how to set these up and use them. (Patches welcome.)
|
