Szelethus created this revision. Szelethus added reviewers: NoQ, dkrupp, o.gyorgy, dcoughlin, Charusso, a.sidorin, xazax.hun, rnkovacs. Szelethus added a project: clang. Herald added subscribers: cfe-commits, gamesh411, donat.nagy, mikhail.ramalho, szepet, baloghadamsoftware, whisperity.
As per discussed on the mailing list <http://lists.llvm.org/pipermail/cfe-dev/2019-May/062298.html>, this patch introduces a new package, that is conceptually in between an alpha and a production ready package called `beta`. Beta packages and checkers may lack a better better bug reporting,some finetuning on the FP/TP ratio, but as an experimental feature they are considered stable enough to be run on production code. We philosophically define alpha as not "power user feature"s, or "high fp/tp reporting checkers that is usable sometimes", but as unfinished. For checkers in beta, while still unfinished, we would like to allow users to experiment with it, because according to our experience analyzing internal C/C++ code at Ericsson, false positives from alpha checkers, even with poor reporting were tolerable, hence the desire to move a portion of alpha checkers to beta. This patch only moves a single checker to beta, `beta.unix.cstring.OutOfBounds`. Repository: rC Clang https://reviews.llvm.org/D62092 Files: include/clang/StaticAnalyzer/Checkers/Checkers.td test/Analysis/bsd-string.c test/Analysis/bstring.c test/Analysis/cstring-plist.c test/Analysis/null-deref-ps-region.c test/Analysis/string.c
Index: test/Analysis/string.c =================================================================== --- test/Analysis/string.c +++ test/Analysis/string.c @@ -1,8 +1,50 @@ -// RUN: %clang_analyze_cc1 -analyzer-checker=core,unix.cstring,unix.Malloc,alpha.unix.cstring,debug.ExprInspection -analyzer-store=region -Wno-null-dereference -verify -analyzer-config eagerly-assume=false %s -// RUN: %clang_analyze_cc1 -DUSE_BUILTINS -analyzer-checker=core,unix.cstring,unix.Malloc,alpha.unix.cstring,debug.ExprInspection -analyzer-store=region -Wno-null-dereference -verify -analyzer-config eagerly-assume=false %s -// RUN: %clang_analyze_cc1 -DVARIANT -analyzer-checker=core,unix.cstring,unix.Malloc,alpha.unix.cstring,debug.ExprInspection -analyzer-store=region -Wno-null-dereference -verify -analyzer-config eagerly-assume=false %s -// RUN: %clang_analyze_cc1 -DUSE_BUILTINS -DVARIANT -analyzer-checker=alpha.security.taint,core,unix.cstring,unix.Malloc,alpha.unix.cstring,debug.ExprInspection -analyzer-store=region -Wno-null-dereference -verify -analyzer-config eagerly-assume=false %s -// RUN: %clang_analyze_cc1 -DSUPPRESS_OUT_OF_BOUND -analyzer-checker=core,unix.cstring,unix.Malloc,alpha.unix.cstring.BufferOverlap,alpha.unix.cstring.NotNullTerminated,debug.ExprInspection -analyzer-store=region -Wno-null-dereference -verify -analyzer-config eagerly-assume=false %s +// RUN: %clang_analyze_cc1 -verify %s -Wno-null-dereference \ +// RUN: -analyzer-checker=core \ +// RUN: -analyzer-checker=unix.cstring \ +// RUN: -analyzer-checker=unix.Malloc \ +// RUN: -analyzer-checker=alpha.unix.cstring \ +// RUN: -analyzer-checker=beta.unix.cstring \ +// RUN: -analyzer-checker=debug.ExprInspection \ +// RUN: -analyzer-config eagerly-assume=false +// +// RUN: %clang_analyze_cc1 -verify %s -Wno-null-dereference -DUSE_BUILTINS \ +// RUN: -analyzer-checker=core \ +// RUN: -analyzer-checker=unix.cstring \ +// RUN: -analyzer-checker=unix.Malloc \ +// RUN: -analyzer-checker=alpha.unix.cstring \ +// RUN: -analyzer-checker=beta.unix.cstring \ +// RUN: -analyzer-checker=debug.ExprInspection \ +// RUN: -analyzer-config eagerly-assume=false +// +// RUN: %clang_analyze_cc1 -verify %s -Wno-null-dereference -DVARIANT \ +// RUN: -analyzer-checker=core \ +// RUN: -analyzer-checker=unix.cstring \ +// RUN: -analyzer-checker=unix.Malloc \ +// RUN: -analyzer-checker=alpha.unix.cstring \ +// RUN: -analyzer-checker=beta.unix.cstring \ +// RUN: -analyzer-checker=debug.ExprInspection \ +// RUN: -analyzer-config eagerly-assume=false +// +// RUN: %clang_analyze_cc1 -verify %s -Wno-null-dereference \ +// RUN: -DUSE_BUILTINS -DVARIANT \ +// RUN: -analyzer-checker=core \ +// RUN: -analyzer-checker=alpha.security.taint \ +// RUN: -analyzer-checker=unix.cstring \ +// RUN: -analyzer-checker=unix.Malloc \ +// RUN: -analyzer-checker=alpha.unix.cstring \ +// RUN: -analyzer-checker=beta.unix.cstring \ +// RUN: -analyzer-checker=debug.ExprInspection \ +// RUN: -analyzer-config eagerly-assume=false +// +// RUN: %clang_analyze_cc1 -verify %s -Wno-null-dereference \ +// RUN: -DSUPPRESS_OUT_OF_BOUND \ +// RUN: -analyzer-checker=core \ +// RUN: -analyzer-checker=unix.cstring \ +// RUN: -analyzer-checker=unix.Malloc \ +// RUN: -analyzer-checker=alpha.unix.cstring.BufferOverlap \ +// RUN: -analyzer-checker=alpha.unix.cstring.NotNullTerminated \ +// RUN: -analyzer-checker=debug.ExprInspection \ +// RUN: -analyzer-config eagerly-assume=false //===----------------------------------------------------------------------=== // Declarations Index: test/Analysis/null-deref-ps-region.c =================================================================== --- test/Analysis/null-deref-ps-region.c +++ test/Analysis/null-deref-ps-region.c @@ -1,4 +1,9 @@ -// RUN: %clang_analyze_cc1 -analyzer-checker=core,alpha.core,unix,alpha.unix -std=gnu99 -analyzer-store=region -verify %s +// RUN: %clang_analyze_cc1 -verify %s -std=gnu99 \ +// RUN: -analyzer-checker=core \ +// RUN: -analyzer-checker=alpha.core \ +// RUN: -analyzer-checker=unix \ +// RUN: -analyzer-checker=alpha.unix \ +// RUN: -analyzer-checker=beta.unix #include "Inputs/system-header-simulator.h" Index: test/Analysis/cstring-plist.c =================================================================== --- test/Analysis/cstring-plist.c +++ test/Analysis/cstring-plist.c @@ -1,7 +1,7 @@ // RUN: rm -f %t // RUN: %clang_analyze_cc1 -fblocks \ // RUN: -analyzer-checker=core,unix.Malloc,unix.cstring.NullArg \ -// RUN: -analyzer-disable-checker=alpha.unix.cstring.OutOfBounds \ +// RUN: -analyzer-disable-checker=beta.unix.cstring.OutOfBounds \ // RUN: -analyzer-output=plist -o %t %s // RUN: FileCheck -input-file %t %s Index: test/Analysis/bstring.c =================================================================== --- test/Analysis/bstring.c +++ test/Analysis/bstring.c @@ -1,7 +1,34 @@ -// RUN: %clang_analyze_cc1 -analyzer-checker=core,unix.cstring,alpha.unix.cstring,debug.ExprInspection -analyzer-store=region -verify -analyzer-config eagerly-assume=false %s -// RUN: %clang_analyze_cc1 -DUSE_BUILTINS -analyzer-checker=core,unix.cstring,alpha.unix.cstring,debug.ExprInspection -analyzer-store=region -verify -analyzer-config eagerly-assume=false %s -// RUN: %clang_analyze_cc1 -DVARIANT -analyzer-checker=core,unix.cstring,alpha.unix.cstring,debug.ExprInspection -analyzer-store=region -verify -analyzer-config eagerly-assume=false %s -// RUN: %clang_analyze_cc1 -DUSE_BUILTINS -DVARIANT -analyzer-checker=core,unix.cstring,alpha.unix.cstring,debug.ExprInspection -analyzer-store=region -verify -analyzer-config eagerly-assume=false %s +// RUN: %clang_analyze_cc1 -verify %s \ +// RUN: -analyzer-checker=core \ +// RUN: -analyzer-checker=unix.cstring \ +// RUN: -analyzer-checker=alpha.unix.cstring \ +// RUN: -analyzer-checker=beta.unix.cstring \ +// RUN: -analyzer-checker=debug.ExprInspection \ +// RUN: -analyzer-config eagerly-assume=false +// +// RUN: %clang_analyze_cc1 -verify %s -DUSE_BUILTINS \ +// RUN: -analyzer-checker=core \ +// RUN: -analyzer-checker=unix.cstring \ +// RUN: -analyzer-checker=alpha.unix.cstring \ +// RUN: -analyzer-checker=beta.unix.cstring \ +// RUN: -analyzer-checker=debug.ExprInspection \ +// RUN: -analyzer-config eagerly-assume=false +// +// RUN: %clang_analyze_cc1 -verify %s -DVARIANT \ +// RUN: -analyzer-checker=core \ +// RUN: -analyzer-checker=unix.cstring \ +// RUN: -analyzer-checker=alpha.unix.cstring \ +// RUN: -analyzer-checker=beta.unix.cstring \ +// RUN: -analyzer-checker=debug.ExprInspection \ +// RUN: -analyzer-config eagerly-assume=false +// +// RUN: %clang_analyze_cc1 -verify %s -DUSE_BUILTINS -DVARIANT \ +// RUN: -analyzer-checker=core \ +// RUN: -analyzer-checker=unix.cstring \ +// RUN: -analyzer-checker=alpha.unix.cstring \ +// RUN: -analyzer-checker=beta.unix.cstring \ +// RUN: -analyzer-checker=debug.ExprInspection \ +// RUN: -analyzer-config eagerly-assume=false //===----------------------------------------------------------------------=== // Declarations Index: test/Analysis/bsd-string.c =================================================================== --- test/Analysis/bsd-string.c +++ test/Analysis/bsd-string.c @@ -1,4 +1,9 @@ -// RUN: %clang_analyze_cc1 -analyzer-checker=core,unix.cstring.NullArg,alpha.unix.cstring,debug.ExprInspection -analyzer-store=region -verify %s +// RUN: %clang_analyze_cc1 -verify %s \ +// RUN: -analyzer-checker=core \ +// RUN: -analyzer-checker=unix.cstring.NullArg \ +// RUN: -analyzer-checker=alpha.unix.cstring \ +// RUN: -analyzer-checker=beta.unix.cstring \ +// RUN: -analyzer-checker=debug.ExprInspection #define NULL ((void *)0) Index: include/clang/StaticAnalyzer/Checkers/Checkers.td =================================================================== --- include/clang/StaticAnalyzer/Checkers/Checkers.td +++ include/clang/StaticAnalyzer/Checkers/Checkers.td @@ -12,13 +12,20 @@ // Packages. //===----------------------------------------------------------------------===// -// The Alpha package is for checkers that have too many false positives to be -// turned on by default. The hierarchy under Alpha should be organized in the -// hierarchy checkers would have had if they were truly at the top level. -// (For example, a Cocoa-specific checker that is alpha should be in -// alpha.osx.cocoa). +// The Alpha package is for checkers that are unstable, incomplet have too many +// false positives to be turned on by default. The hierarchy under Alpha should +// be organized in the hierarchy checkers would have had if they were truly at +// the top level. (For example, a Cocoa-specific checker that is alpha should be +// in alpha.osx.cocoa). def Alpha : Package<"alpha">; +// Beta checkers are considered stable, but need some finishing touches, +// might have a not ideal false positive/false negative ratio, but are +// usable in production. +// Beta checkers checkers at minimum must be able to analyze several large open +// source projects without crashes, and have a reasonable FP/TP ratio. +def Beta : Package<"beta">; + def Core : Package<"core">; def CoreBuiltin : Package<"builtin">, ParentPackage<Core>, Hidden; def CoreUninitialized : Package<"uninitialized">, ParentPackage<Core>; @@ -72,8 +79,10 @@ def Unix : Package<"unix">; def UnixAlpha : Package<"unix">, ParentPackage<Alpha>; +def UnixBeta : Package<"unix">, ParentPackage<Beta>; def CString : Package<"cstring">, ParentPackage<Unix>; def CStringAlpha : Package<"cstring">, ParentPackage<UnixAlpha>; +def CStringBeta : Package<"cstring">, ParentPackage<UnixBeta>; def OSX : Package<"osx">; def OSXAlpha : Package<"osx">, ParentPackage<Alpha>; @@ -358,11 +367,6 @@ let ParentPackage = CStringAlpha in { -def CStringOutOfBounds : Checker<"OutOfBounds">, - HelpText<"Check for out-of-bounds access in string functions">, - Dependencies<[CStringModeling]>, - Documentation<HasAlphaDocumentation>; - def CStringBufferOverlap : Checker<"BufferOverlap">, HelpText<"Checks for overlap in two buffer arguments">, Dependencies<[CStringModeling]>, @@ -375,6 +379,15 @@ } // end "alpha.unix.cstring" +let ParentPackage = CStringBeta in { + +def CStringOutOfBounds : Checker<"OutOfBounds">, + HelpText<"Check for out-of-bounds access in string functions">, + Dependencies<[CStringModeling]>, + Documentation<HasAlphaDocumentation>; + +} // end "beta.unix.cstring" + let ParentPackage = Unix in { def UnixAPIMisuseChecker : Checker<"API">,
_______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits