Add check for CERT FLP30-C

[aaron.ballman]

Summary

This patch adds a check for the CERT secure coding rule: FLP30-C. Do not use floating-point variables as loop counters. It flags any for loop induction expression that has floating-point type.

https://www.securecoding.cert.org/confluence/display/c/FLP30-C.+Do+not+use+floating-point+variables+as+loop+counters

[hokein]

LGTM with one nit.

test/clang-tidy/cert-flp30-c.c

6

I see in most of testcases, they are using [[@LINE-1]] instead of [[@LINE+1]]. We'd better follow the [[@LINE+1]] way here.

[aaron.ballman]

Thanks! I've commit in r261324.

test/clang-tidy/cert-flp30-c.c

6

I've changed to using -1 instead of +1, but I don't think we have a hard and fast rule on location aside from "whatever makes the most sense." Given the compound blocks, I personally think +1 is more clear than -1, but meh.

[alexfh]

LG

test/clang-tidy/cert-flp30-c.c

6

Yep, I don't think we're trying to be consistent with placing check lines before or after the checked location. Not that it matters much.

clang-tidy/cert/CERTTidyModule.cpp

 #include "../misc/NonCopyableObjects.h"
 #include "../misc/StaticAssertCheck.h"
 #include "../misc/ThrowByValueCatchByReferenceCheck.h"
+#include "FloatLoopCounter.h"
 #include "SetLongJmpCheck.h"
 #include "StaticObjectExceptionCheck.h"
 #include "ThrownExceptionTypeCheck.h"

     // DCL
     CheckFactories.registerCheck<StaticAssertCheck>(
         "cert-dcl03-c");
-
+    // FLP
+    CheckFactories.registerCheck<FloatLoopCounter>(
+        "cert-flp30-c");
     // FIO
     CheckFactories.registerCheck<NonCopyableObjectsCheck>(
         "cert-fio38-c");

clang-tidy/cert/CMakeLists.txt

 
 add_clang_library(clangTidyCERTModule
   CERTTidyModule.cpp
+  FloatLoopCounter.cpp
   SetLongJmpCheck.cpp
   StaticObjectExceptionCheck.cpp
   ThrownExceptionTypeCheck.cpp

clang-tidy/cert/FloatLoopCounter.h

+//===--- FloatLoopCounter.h - clang-tidy-------------------------*- C++ -*-===//
+//
+//                     The LLVM Compiler Infrastructure
+//
+// This file is distributed under the University of Illinois Open Source
+// License. See LICENSE.TXT for details.
+//
+//===----------------------------------------------------------------------===//
+
+#ifndef LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_CERT_FLOAT_LOOP_COUNTER_H
+#define LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_CERT_FLOAT_LOOP_COUNTER_H
+
+#include "../ClangTidy.h"
+
+namespace clang {
+namespace tidy {
+namespace cert {
+
+/// This check diagnoses when the loop induction expression of a for loop has
+/// floating-point type. The check corresponds to:
+/// https://www.securecoding.cert.org/confluence/display/c/FLP30-C.+Do+not+use+floating-point+variables+as+loop+counters
+///
+/// For the user-facing documentation see:
+/// http://clang.llvm.org/extra/clang-tidy/checks/cert-flp30-c.html
+class FloatLoopCounter : public ClangTidyCheck {
+public:
+  FloatLoopCounter(StringRef Name, ClangTidyContext *Context)
+      : ClangTidyCheck(Name, Context) {}
+  void registerMatchers(ast_matchers::MatchFinder *Finder) override;
+  void check(const ast_matchers::MatchFinder::MatchResult &Result) override;
+};
+
+} // namespace cert
+} // namespace tidy
+} // namespace clang
+
+#endif // LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_CERT_FLOAT_LOOP_COUNTER_H

clang-tidy/cert/FloatLoopCounter.cpp

+//===--- FloatLoopCounter.cpp - clang-tidy---------------------------------===//
+//
+//                     The LLVM Compiler Infrastructure
+//
+// This file is distributed under the University of Illinois Open Source
+// License. See LICENSE.TXT for details.
+//
+//===----------------------------------------------------------------------===//
+
+#include "FloatLoopCounter.h"
+#include "clang/AST/ASTContext.h"
+#include "clang/ASTMatchers/ASTMatchFinder.h"
+
+using namespace clang::ast_matchers;
+
+namespace clang {
+namespace tidy {
+namespace cert {
+
+void FloatLoopCounter::registerMatchers(MatchFinder *Finder) {
+  Finder->addMatcher(
+      forStmt(hasIncrement(expr(hasType(realFloatingPointType())))).bind("for"),
+      this);
+}
+
+void FloatLoopCounter::check(const MatchFinder::MatchResult &Result) {
+  const auto *FS = Result.Nodes.getNodeAs<ForStmt>("for");
+
+  diag(FS->getInc()->getExprLoc(), "loop induction expression should not have "
+                                   "floating-point type");
+}
+
+} // namespace cert
+} // namespace tidy
+} // namespace clang

docs/clang-tidy/checks/cert-flp30-c.rst

+.. title:: clang-tidy - cert-flp30-c
+
+cert-flp30-c
+============
+
+This check flags ``for`` loops where the induction expression has a floating-
+point type.
+
+This check corresponds to the CERT C Coding Standard rule
+`FLP30-C. Do not use floating-point variables as loop counters
+<https://www.securecoding.cert.org/confluence/display/c/FLP30-C.+Do+not+use+floating-point+variables+as+loop+counters>`_.

docs/clang-tidy/checks/list.rst

    cert-err60-cpp
    cert-err61-cpp (redirects to misc-throw-by-value-catch-by-reference) <cert-err61-cpp>
    cert-fio38-c (redirects to misc-non-copyable-objects) <cert-fio38-c>
+   cert-flp30-c
    cert-oop11-cpp (redirects to misc-move-constructor-init) <cert-oop11-cpp>
    cppcoreguidelines-pro-bounds-array-to-pointer-decay
    cppcoreguidelines-pro-bounds-constant-array-index

test/clang-tidy/cert-flp30-c.c

+// RUN: %check_clang_tidy %s cert-flp30-c %t
+
+float g(void);
+
+void func(void) {
+  // CHECK-MESSAGES: :[[@LINE+1]]:37: warning: loop induction expression should not have floating-point type [cert-flp30-c]
+  for (float x = 0.1f; x <= 1.0f; x += 0.1f) {
+  }
+
+  float f = 1.0f;
+  // CHECK-MESSAGES: :[[@LINE+1]]:17: warning: loop induction expression
+  for (; f > 0; --f) {
+  }
+
+  // CHECK-MESSAGES: :[[@LINE+1]]:10: warning: loop induction expression
+  for (;;g()) {
+  }
+
+  for (int i = 0; i < 10; i += 1.0f) {
+  }
+
+  for (int i = 0; i < 10; ++i) {
+  }
+}