Clang Static Analyzer check if a function was called twice

Question

I have a new custom checker (TransactionChecker.cpp).

Here is the TransacationState:

struct TransactionState {
private:
  enum Kind { OpenedT, StartedT, FinalizedT, ClosedT } K;
  TransactionState(Kind InK) : K(InK) {}

public:
  bool isOpened() const { return K == OpenedT; }
  bool isClosed() const { return K == ClosedT; }
  bool isStarted() const { return K == StartedT; }
  bool isFinalized() const { return K == FinalizedT; }

  static TransactionState getOpened() { return TransactionState(OpenedT); }
  static TransactionState getClosed() { return TransactionState(ClosedT); }
  static TransactionState getStarted() { return TransactionState(StartedT); }
  static TransactionState getFinalized() {
    return TransactionState(FinalizedT);
  }

  bool operator==(const TransactionState &X) const { return K == X.K; }
  void Profile(llvm::FoldingSetNodeID &ID) const { ID.AddInteger(K); }
};

My header file and test.c

void checkDoubleOpen(){
  TRANSACTION *T = open_transaction();
  T = open_transaction();  // expected-warning {{Open a previously open transaction}}


#pragma clang system_header


typedef struct __sTRANSACTION {
  unsigned char *_p;
  int value;
} TRANSACTION;

void startTransaction(TRANSACTION *T,int val);
int finalizeTransaction(TRANSACTION *T);
TRANSACTION* open_transaction();
int close_transaction(TRANSACTION*);

void fakeSystemHeaderCall(TRANSACTION *);

After run:

clang -cc1 -analyze -analyzer-checker=alpha.unix.Transaction test.c

I want to print that warning.

I tried with REGISTER_MAP_WITH_PROGRAMSTATE(MAPSymbolTS, SymbolRef, TransactionState)

void TransactionChecker::checkPostCall(const CallEvent &Call,
                                       CheckerContext &C) const {
  if (!Call.isGlobalCFunction())
    return;

  if (!Call.isCalled(OpenTran))
    return;

  ProgramStateRef State = C.getState();

  // Get the symbolic value corresponding to the file handle.
  FunctionDecl FileDesc = Call.getReturnValue().getAsSymbol();

  if (!FileDesc)
       return;

 const struct TransactionState *TState = State->get<MAPSymbolTS>(FileDesc);
  if (!TState) {
    // Generate the next transition (an edge in the exploded graph).
    State = State->set<MAPSymbolTS>(FileDesc, TransactionState::getOpened());
    C.addTransition(State);
  } else {
    reportOpenAfterOpen(Call, C);
  }
}

but without succes.

I think I need a new map: key = unknown (function's name + id profile) and value TransactionState but don't know how to create it.

I don't know if you have already seen it, but this guide [How to write a Checker in 24 hours](https://llvm.org/devmtg/2012-11/Zaks-Rose-Checker24Hours.pdf) does something very similar. They check that `fclose` is called once per file opened, but the same ideas could help to find if a function is called twice. What you have already done looks similar to this though, so sorry if you have already read it. — , Mar 17 '18 at 19:19

score 1 · Answer 1 · answered Sep 11 '19 at 16:09

Interpretation of question

You want to report whenever there is a path that calls open_transaction twice without an intervening close_transaction.

Overview

As noted in a comment, this is sort of like the tutorial checker SimpleStreamChecker.cpp. However, that checker is tracking the state of multiple objects, while here the state is global to the program. That makes it more similar to BlockInCriticalSectionChecker.cpp, so we'll imitate that one.

Whereas the tutorial checker uses a map, here we only need to keep track of a single value. I'll use an unsigned counter:

REGISTER_TRAIT_WITH_PROGRAMSTATE(CalledTwiceCounter, unsigned)

When we see a call to open_transaction, increment the counter:

  if (FD->getIdentifier() == II_open) {
    // Update the abstract state to reflect the number of calls.
    unsigned counter = state->get<CalledTwiceCounter>();
    counter++;
    state = state->set<CalledTwiceCounter>(counter);
    C.addTransition(state);

and then report a defect if the counter exceeds 2.

Similarly, decrement it when we see close_transaction.

Complete example

CalledTwiceChecker.cpp:

// CalledTwiceChecker.cpp
// https://stackoverflow.com/questions/48241792/clang-static-analyzer-check-if-a-function-was-called-twice

#include "clang/StaticAnalyzer/Checkers/BuiltinCheckerRegistration.h"
#include "clang/StaticAnalyzer/Core/BugReporter/BugType.h"
#include "clang/StaticAnalyzer/Core/Checker.h"
#include "clang/StaticAnalyzer/Core/CheckerManager.h"
#include "clang/StaticAnalyzer/Core/PathSensitive/CheckerContext.h"
#include "clang/StaticAnalyzer/Core/PathSensitive/ProgramState.h"
#include "clang/StaticAnalyzer/Core/PathSensitive/ProgramStateTrait.h"

using namespace clang;
using namespace ento;

namespace {

class CalledTwiceChecker : public Checker< eval::Call > {
  mutable IdentifierInfo *II_open, *II_close;
  mutable std::unique_ptr<BuiltinBug> BT_calledTwice;

public:
  CalledTwiceChecker()
    : II_open(nullptr), II_close(nullptr) {}

  bool evalCall(const CallExpr *CE, CheckerContext &C) const;
};

} // end anonymous namespace

// Number of times the function of interest has been called on the
// current path.  Automatically initialized to zero.
//
// Based on similar code in BlockInCriticalSectionChecker.cpp.
REGISTER_TRAIT_WITH_PROGRAMSTATE(CalledTwiceCounter, unsigned)

bool CalledTwiceChecker::evalCall(const CallExpr *CE, CheckerContext &C) const {
  const FunctionDecl *FD = C.getCalleeDecl(CE);
  if (!FD || FD->getKind() != Decl::Function) {
    return false;
  }

  ASTContext &Ctx = C.getASTContext();
  if (!II_open) {
    II_open = &Ctx.Idents.get("open_transaction");
  }
  if (!II_close) {
    II_close = &Ctx.Idents.get("close_transaction");
  }

  ProgramStateRef state = C.getState();

  if (FD->getIdentifier() == II_open) {
    // Update the abstract state to reflect the number of calls.
    unsigned counter = state->get<CalledTwiceCounter>();
    counter++;
    state = state->set<CalledTwiceCounter>(counter);
    C.addTransition(state);
    //llvm::errs() << "incremented counter to " << counter << "\n";

    // Note: It is questionable to allow the counter to increase without
    // bound in a static analysis, but the Clang SA engine seems to cap
    // the number of loop iterations at 4, so this is evidently not
    // immediately catastrophic.

    // Possibly report a defect.
    if (counter >= 2) {
      ExplodedNode *N = C.generateErrorNode();
      if (N) {
        if (!BT_calledTwice) {
          BT_calledTwice.reset(new BuiltinBug(
              this, "Called twice", "open_transaction called twice."));
        }
        C.emitReport(llvm::make_unique<BugReport>(
            *BT_calledTwice, BT_calledTwice->getDescription(), N));
      }
    }
    return true;
  }

  if (FD->getIdentifier() == II_close) {
    unsigned counter = state->get<CalledTwiceCounter>();
    if (counter > 0) {
      counter--;
      state = state->set<CalledTwiceCounter>(counter);
      C.addTransition(state);
      return true;
    }
    else {
      return false;
    }
  }

  return false;
}

void ento::registerCalledTwiceChecker(CheckerManager &mgr) {
  mgr.registerChecker<CalledTwiceChecker>();
}

bool ento::shouldRegisterCalledTwiceChecker(const LangOptions &LO) {
  return true;
}

To hook this in to the rest of Clang, add entries to:

clang/include/clang/StaticAnalyzer/Checkers/Checkers.td and
clang/lib/StaticAnalyzer/Checkers/CMakeLists.txt

Example input to test it:

// calltwice.c
// Tests for CalledTwiceChecker.

void open_transaction();
void close_transaction();

void open_once()
{
  open_transaction();        // not reported
}

void open_twice()
{
  open_transaction();
  open_transaction();        // reported
}

void open_one_each_path(int x)
{
  if (x) {
    open_transaction();
  }
  else {
    open_transaction();      // not reported
  }
}

void open_close_open()
{
  open_transaction();
  close_transaction();
  open_transaction();        // not reported
}

void open_close_open_open()
{
  open_transaction();
  close_transaction();
  open_transaction();
  open_transaction();        // reported
}

int something();

void open_loop()
{
  while (something()) {
    open_transaction();      // reported
  }
}

Analysis run on that input:

$ gcc -E -o calltwice.i calltwice.c
$ ~/bld/llvm-project/build/bin/clang -cc1 -analyze -analyzer-checker=alpha.core.CalledTwice calltwice.i
calltwice.c:15:3: warning: open_transaction called twice
  open_transaction();
  ^~~~~~~~~~~~~~~~~~
calltwice.c:40:3: warning: open_transaction called twice
  open_transaction();
  ^~~~~~~~~~~~~~~~~~
calltwice.c:48:5: warning: open_transaction called twice
    open_transaction();
    ^~~~~~~~~~~~~~~~~~
3 warnings generated.

Clang Static Analyzer check if a function was called twice

1 Answers1

Interpretation of question

Overview

Complete example