From 3fb90d137ea16e5c3a4580b9db5fd18d93df1a90 Mon Sep 17 00:00:00 2001 From: Kristof Umann Date: Tue, 7 Aug 2018 12:55:26 +0000 Subject: [PATCH] [analyzer][UninitializedObjectChecker] New flag to turn off dereferencing Even for a checker being in alpha, some reports about pointees held so little value to the user that it's safer to disable pointer/reference chasing for now. It can be enabled with a new flag, in which case checker should function as it has always been. This can be set with `CheckPointeeInitialization`. Differential Revision: https://reviews.llvm.org/D49438 git-svn-id: https://llvm.org/svn/llvm-project/cfe/trunk@339135 91177308-0d34-0410-b5e6-96231b3b80d8 --- .../Checkers/UninitializedObjectChecker.cpp | 43 +++++++++++++++---- .../cxx-uninitialized-object-inheritance.cpp | 5 ++- ...xx-uninitialized-object-no-dereference.cpp | 27 ++++++++++++ ...uninitialized-object-notes-as-warnings.cpp | 5 ++- .../cxx-uninitialized-object-ptr-ref.cpp | 11 +++-- test/Analysis/cxx-uninitialized-object.cpp | 11 +++-- 6 files changed, 86 insertions(+), 16 deletions(-) create mode 100644 test/Analysis/cxx-uninitialized-object-no-dereference.cpp diff --git a/lib/StaticAnalyzer/Checkers/UninitializedObjectChecker.cpp b/lib/StaticAnalyzer/Checkers/UninitializedObjectChecker.cpp index 398228a9d8..2383ecff22 100644 --- a/lib/StaticAnalyzer/Checkers/UninitializedObjectChecker.cpp +++ b/lib/StaticAnalyzer/Checkers/UninitializedObjectChecker.cpp @@ -10,19 +10,33 @@ // This file defines a checker that reports uninitialized fields in objects // created after a constructor call. // -// This checker has two options: +// This checker has several options: // - "Pedantic" (boolean). If its not set or is set to false, the checker // won't emit warnings for objects that don't have at least one initialized // field. This may be set with // -// `-analyzer-config alpha.cplusplus.UninitializedObject:Pedantic=true`. +// `-analyzer-config alpha.cplusplus.UninitializedObject:Pedantic=true`. // // - "NotesAsWarnings" (boolean). If set to true, the checker will emit a // warning for each uninitalized field, as opposed to emitting one warning // per constructor call, and listing the uninitialized fields that belongs // to it in notes. Defaults to false. // -// `-analyzer-config alpha.cplusplus.UninitializedObject:NotesAsWarnings=true`. +// `-analyzer-config \ +// alpha.cplusplus.UninitializedObject:NotesAsWarnings=true`. +// +// - "CheckPointeeInitialization" (boolean). If set to false, the checker will +// not analyze the pointee of pointer/reference fields, and will only check +// whether the object itself is initialized. Defaults to false. +// +// `-analyzer-config \ +// alpha.cplusplus.UninitializedObject:CheckPointeeInitialization=true`. +// +// TODO: With some clever heuristics, some pointers should be dereferenced +// by default. For example, if the pointee is constructed within the +// constructor call, it's reasonable to say that no external object +// references it, and we wouldn't generate multiple report on the same +// pointee. // //===----------------------------------------------------------------------===// @@ -44,6 +58,7 @@ public: // These fields will be initialized when registering the checker. bool IsPedantic; bool ShouldConvertNotesToWarnings; + bool CheckPointeeInitialization; UninitializedObjectChecker() : BT_uninitField(new BuiltinBug(this, "Uninitialized fields")) {} @@ -109,13 +124,16 @@ class FindUninitializedFields { const TypedValueRegion *const ObjectR; const bool IsPedantic; + const bool CheckPointeeInitialization; + bool IsAnyFieldInitialized = false; UninitFieldSet UninitFields; public: FindUninitializedFields(ProgramStateRef State, - const TypedValueRegion *const R, bool IsPedantic); + const TypedValueRegion *const R, bool IsPedantic, + bool CheckPointeeInitialization); const UninitFieldSet &getUninitFields(); private: @@ -262,8 +280,8 @@ void UninitializedObjectChecker::checkEndFunction( if (!Object) return; - FindUninitializedFields F(Context.getState(), Object->getRegion(), - IsPedantic); + FindUninitializedFields F(Context.getState(), Object->getRegion(), IsPedantic, + CheckPointeeInitialization); const UninitFieldSet &UninitFields = F.getUninitFields(); @@ -327,8 +345,10 @@ void UninitializedObjectChecker::checkEndFunction( //===----------------------------------------------------------------------===// FindUninitializedFields::FindUninitializedFields( - ProgramStateRef State, const TypedValueRegion *const R, bool IsPedantic) - : State(State), ObjectR(R), IsPedantic(IsPedantic) {} + ProgramStateRef State, const TypedValueRegion *const R, bool IsPedantic, + bool CheckPointeeInitialization) + : State(State), ObjectR(R), IsPedantic(IsPedantic), + CheckPointeeInitialization(CheckPointeeInitialization) {} const UninitFieldSet &FindUninitializedFields::getUninitFields() { isNonUnionUninit(ObjectR, FieldChainInfo()); @@ -468,6 +488,11 @@ bool FindUninitializedFields::isPointerOrReferenceUninit( return addFieldToUninits({LocalChain, FR}); } + if (!CheckPointeeInitialization) { + IsAnyFieldInitialized = true; + return false; + } + const FieldDecl *FD = FR->getDecl(); // TODO: The dynamic type of a void pointer may be retrieved with @@ -685,4 +710,6 @@ void ento::registerUninitializedObjectChecker(CheckerManager &Mgr) { "Pedantic", /*DefaultVal*/ false, Chk); Chk->ShouldConvertNotesToWarnings = Mgr.getAnalyzerOptions().getBooleanOption( "NotesAsWarnings", /*DefaultVal*/ false, Chk); + Chk->CheckPointeeInitialization = Mgr.getAnalyzerOptions().getBooleanOption( + "CheckPointeeInitialization", /*DefaultVal*/ false, Chk); } diff --git a/test/Analysis/cxx-uninitialized-object-inheritance.cpp b/test/Analysis/cxx-uninitialized-object-inheritance.cpp index 3b048b759e..9f6d2a0e71 100644 --- a/test/Analysis/cxx-uninitialized-object-inheritance.cpp +++ b/test/Analysis/cxx-uninitialized-object-inheritance.cpp @@ -1,4 +1,7 @@ -// RUN: %clang_analyze_cc1 -analyzer-checker=core,alpha.cplusplus.UninitializedObject -analyzer-config alpha.cplusplus.UninitializedObject:Pedantic=true -std=c++11 -verify %s +// RUN: %clang_analyze_cc1 -analyzer-checker=core,alpha.cplusplus.UninitializedObject \ +// RUN: -analyzer-config alpha.cplusplus.UninitializedObject:Pedantic=true -DPEDANTIC \ +// RUN: -analyzer-config alpha.cplusplus.UninitializedObject:CheckPointeeInitialization=true \ +// RUN: -std=c++11 -verify %s //===----------------------------------------------------------------------===// // Non-polymorphic inheritance tests diff --git a/test/Analysis/cxx-uninitialized-object-no-dereference.cpp b/test/Analysis/cxx-uninitialized-object-no-dereference.cpp new file mode 100644 index 0000000000..0309c28b3e --- /dev/null +++ b/test/Analysis/cxx-uninitialized-object-no-dereference.cpp @@ -0,0 +1,27 @@ +// RUN: %clang_analyze_cc1 -analyzer-checker=core,alpha.cplusplus.UninitializedObject \ +// RUN: -std=c++11 -DPEDANTIC -verify %s + +class UninitPointerTest { + int *ptr; // expected-note{{uninitialized pointer 'this->ptr'}} + int dontGetFilteredByNonPedanticMode = 0; + +public: + UninitPointerTest() {} // expected-warning{{1 uninitialized field}} +}; + +void fUninitPointerTest() { + UninitPointerTest(); +} + +class UninitPointeeTest { + int *ptr; // no-note + int dontGetFilteredByNonPedanticMode = 0; + +public: + UninitPointeeTest(int *ptr) : ptr(ptr) {} // no-warning +}; + +void fUninitPointeeTest() { + int a; + UninitPointeeTest t(&a); +} diff --git a/test/Analysis/cxx-uninitialized-object-notes-as-warnings.cpp b/test/Analysis/cxx-uninitialized-object-notes-as-warnings.cpp index a49507a1aa..2a5fcbc923 100644 --- a/test/Analysis/cxx-uninitialized-object-notes-as-warnings.cpp +++ b/test/Analysis/cxx-uninitialized-object-notes-as-warnings.cpp @@ -1,4 +1,7 @@ -// RUN: %clang_analyze_cc1 -analyzer-checker=core,alpha.cplusplus.UninitializedObject -analyzer-config alpha.cplusplus.UninitializedObject:NotesAsWarnings=true -std=c++11 -verify %s +// RUN: %clang_analyze_cc1 -analyzer-checker=core,alpha.cplusplus.UninitializedObject \ +// RUN: -analyzer-config alpha.cplusplus.UninitializedObject:NotesAsWarnings=true \ +// RUN: -analyzer-config alpha.cplusplus.UninitializedObject:CheckPointeeInitialization=true \ +// RUN: -std=c++11 -verify %s class NotesAsWarningsTest { int a; diff --git a/test/Analysis/cxx-uninitialized-object-ptr-ref.cpp b/test/Analysis/cxx-uninitialized-object-ptr-ref.cpp index 1507098c5e..bfffc800bc 100644 --- a/test/Analysis/cxx-uninitialized-object-ptr-ref.cpp +++ b/test/Analysis/cxx-uninitialized-object-ptr-ref.cpp @@ -1,6 +1,11 @@ -// RUN: %clang_analyze_cc1 -analyzer-checker=core,alpha.cplusplus.UninitializedObject -analyzer-config alpha.cplusplus.UninitializedObject:Pedantic=true -std=c++11 -DPEDANTIC -verify %s - -// RUN: %clang_analyze_cc1 -analyzer-checker=core,alpha.cplusplus.UninitializedObject -std=c++11 -verify %s +// RUN: %clang_analyze_cc1 -analyzer-checker=core,alpha.cplusplus.UninitializedObject \ +// RUN: -analyzer-config alpha.cplusplus.UninitializedObject:Pedantic=true -DPEDANTIC \ +// RUN: -analyzer-config alpha.cplusplus.UninitializedObject:CheckPointeeInitialization=true \ +// RUN: -std=c++11 -verify %s + +// RUN: %clang_analyze_cc1 -analyzer-checker=core,alpha.cplusplus.UninitializedObject \ +// RUN: -analyzer-config alpha.cplusplus.UninitializedObject:CheckPointeeInitialization=true \ +// RUN: -std=c++11 -verify %s //===----------------------------------------------------------------------===// // Concrete location tests. diff --git a/test/Analysis/cxx-uninitialized-object.cpp b/test/Analysis/cxx-uninitialized-object.cpp index 39d4a7f801..0c5c1c246c 100644 --- a/test/Analysis/cxx-uninitialized-object.cpp +++ b/test/Analysis/cxx-uninitialized-object.cpp @@ -1,6 +1,11 @@ -// RUN: %clang_analyze_cc1 -analyzer-checker=core,alpha.cplusplus.UninitializedObject -analyzer-config alpha.cplusplus.UninitializedObject:Pedantic=true -std=c++11 -DPEDANTIC -verify %s - -// RUN: %clang_analyze_cc1 -analyzer-checker=core,alpha.cplusplus.UninitializedObject -std=c++11 -verify %s +// RUN: %clang_analyze_cc1 -analyzer-checker=core,alpha.cplusplus.UninitializedObject \ +// RUN: -analyzer-config alpha.cplusplus.UninitializedObject:Pedantic=true -DPEDANTIC \ +// RUN: -analyzer-config alpha.cplusplus.UninitializedObject:CheckPointeeInitialization=true \ +// RUN: -std=c++11 -verify %s + +// RUN: %clang_analyze_cc1 -analyzer-checker=core,alpha.cplusplus.UninitializedObject \ +// RUN: -analyzer-config alpha.cplusplus.UninitializedObject:CheckPointeeInitialization=true \ +// RUN: -std=c++11 -verify %s //===----------------------------------------------------------------------===// // Default constructor test. -- 2.50.1