tor-browser

The Tor Browser
git clone https://git.dasho.dev/tor-browser.git
Log | Files | Refs | README | LICENSE

advanced-check-features.rst (6049B)


      1 .. _advanced_check_features:
      2 
      3 Advanced Check Features
      4 =======================
      5 
      6 This page covers additional ways to improve and extend the check you've added to build/clang-plugin.
      7 
      8 Adding Tests
      9 ------------
     10 
     11 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.
     12 
     13 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.
     14 
     15 Using Bind To Output More Useful Information
     16 --------------------------------------------
     17 
     18 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.
     19 
     20 ``.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:
     21 
     22 ::
     23 
     24  AstMatcher->addMatcher(
     25    traverse(TK_IgnoreUnlessSpelledInSource,
     26      ifStmt(allOf(
     27              has(
     28                   binaryOperator(
     29                       has(
     30                           declRefExpr(hasType(enumDecl()))
     31                       )
     32                   )
     33               ),
     34               hasElse(
     35                   ifStmt(allOf(
     36                      unless(hasElse(anything())),
     37                      has(
     38                           binaryOperator(
     39                               has(
     40                                   declRefExpr(hasType(enumDecl()))
     41                               )
     42                           )
     43                       )
     44                   ))
     45              )
     46           ))
     47          .bind("node")),
     48      this);
     49 
     50 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")``.
     51 
     52 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``:
     53 
     54 ::
     55 
     56  AstMatcher->addMatcher(
     57    traverse(TK_IgnoreUnlessSpelledInSource,
     58      ifStmt(allOf(
     59              has(
     60                   binaryOperator(
     61                       has(
     62                           declRefExpr(hasType(enumDecl().bind("enumType")))
     63                       )
     64                   )
     65               ),
     66               hasElse(
     67                   ifStmt(allOf(
     68                      unless(hasElse(anything())),
     69                      has(
     70                           binaryOperator(
     71                               has(
     72                                   declRefExpr(hasType(enumDecl()))
     73                               )
     74                           )
     75                       )
     76                   ))
     77              )
     78           ))
     79          .bind("node")),
     80      this);
     81 
     82 And in our check() function, we can use it like so:
     83 
     84 ::
     85 
     86  void MissingElseInEnumComparisons::check(
     87      const MatchFinder::MatchResult &Result) {
     88    const auto *MatchedDecl = Result.Nodes.getNodeAs<IfStmt>("node");
     89    const auto *EnumType = Result.Nodes.getNodeAs<EnumDecl>("enumType");
     90 
     91    diag(MatchedDecl->getIfLoc(),
     92         "Enum comparisons to %0 in an if/else if block without a trailing else.",
     93         DiagnosticIDs::Warning) << EnumType->getName();
     94  }
     95 
     96 Repeated matcher calls
     97 --------------------------
     98 
     99 If you find yourself repeating the same several matchers in several spots, you can turn it into a variable to use.
    100 
    101 ::
    102 
    103  auto isTemporaryLifetimeBoundCall =
    104      cxxMemberCallExpr(
    105          onImplicitObjectArgument(anyOf(has(cxxTemporaryObjectExpr()),
    106                                         has(materializeTemporaryExpr()))),
    107          callee(functionDecl(isMozTemporaryLifetimeBound())));
    108 
    109  auto hasTemporaryLifetimeBoundCall =
    110      anyOf(isTemporaryLifetimeBoundCall,
    111            conditionalOperator(
    112                anyOf(hasFalseExpression(isTemporaryLifetimeBoundCall),
    113                      hasTrueExpression(isTemporaryLifetimeBoundCall))));
    114 
    115 The above example is parameter-less, but if you need to supply a parameter that changes, you can turn it into a lambda:
    116 
    117 ::
    118 
    119  auto hasConstCharPtrParam = [](const unsigned int Position) {
    120    return hasParameter(
    121        Position, hasType(hasCanonicalType(pointsTo(asString("const char")))));
    122  };
    123 
    124  auto hasParamOfType = [](const unsigned int Position, const char *Name) {
    125    return hasParameter(Position, hasType(asString(Name)));
    126  };
    127 
    128  auto hasIntegerParam = [](const unsigned int Position) {
    129    return hasParameter(Position, hasType(isInteger()));
    130  };
    131 
    132  AstMatcher->addMatcher(
    133      callExpr(
    134        hasName("fopen"),
    135        hasConstCharPtrParam(0))
    136          .bind("funcCall"),
    137      this);
    138 
    139 
    140 Allow-listing existing callsites
    141 --------------------------------
    142 
    143 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.
    144 
    145 
    146 Custom Annotations
    147 ------------------
    148 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.)