[clang][analyzer] Add checker 'alpha.core.FixedAddressDereference'

clang/docs/analyzer/checkers.rst

@@ -118,19 +118,6 @@ core.NullDereference (C, C++, ObjC)
 """""""""""""""""""""""""""""""""""
 Check for dereferences of null pointers.
 
-This checker specifically does
-not report null pointer dereferences for x86 and x86-64 targets when the
-address space is 256 (x86 GS Segment), 257 (x86 FS Segment), or 258 (x86 SS
-segment). See `X86/X86-64 Language Extensions
-<https://clang.llvm.org/docs/LanguageExtensions.html#memory-references-to-specified-segments>`__
-for reference.
-
-The ``SuppressAddressSpaces`` option suppresses
-warnings for null dereferences of all pointers with address spaces. You can
-disable this behavior with the option
-``-analyzer-config core.NullDereference:SuppressAddressSpaces=false``.
-*Defaults to true*.
-
 .. code-block:: objc
 
  // C
@@ -170,6 +157,16 @@ disable this behavior with the option
    obj->x = 1; // warn
  }
 
+Null pointer dereferences of pointers with address spaces are not always defined
+as error. Specifically on x86/x86-64 target if the pointer address space is
+256 (x86 GS Segment), 257 (x86 FS Segment), or 258 (x86 SS Segment), a null
+dereference is not defined as error. See `X86/X86-64 Language Extensions
+<https://clang.llvm.org/docs/LanguageExtensions.html#memory-references-to-specified-segments>`__
+for reference. The ``suppress-all-address-spaces`` configuration option can be
+used to control if null dereferences with any address space or only with the
+specific x86 address spaces 256, 257, 258 are excluded from reporting as error.
+The default is all address spaces.
+
 .. _core-StackAddressEscape:
 
 core.StackAddressEscape (C)
@@ -2919,6 +2916,38 @@ Check for assignment of a fixed address to a pointer.
    p = (int *) 0x10000; // warn
  }
 
+.. _alpha-core-FixedAddressDereference:
+
+alpha.core.FixedAddressDereference (C, C++, ObjC)
+"""""""""""""""""""""""""""""""""""""""""""""""""
+Check for dereferences of fixed addresses.
+A pointer contains a fixed address if it was set to a hard-coded value or it
+becomes otherwise obvious that at that point it can have only a single specific
+value.
+
+.. code-block:: c
+
+ void test1() {
+   int *p = (int *)0x020;
+   int x = p[0]; // warn
+ }
+
+ void test2(int *p) {
+   if (p == (int *)-1)
+     *p = 0; // warn
+ }
+
+ void test3() {
+   int (*p_function)(char, char);
+   p_function = (int (*)(char, char))0x04080;
+   int x = (*p_function)('x', 'y'); // NO warning yet at functon pointer calls
+ }
+
+The analyzer option ``suppress-all-address-spaces`` affects this checker. If it
+is set to true pointer dereferences with any address space are not reported as
+error. Otherwise only address spaces 256, 257, 258 on target x86/x86-64 are
+excluded from reporting as error. The default is all address spaces.
+
 .. _alpha-core-PointerArithm:
 
 alpha.core.PointerArithm (C)

clang/include/clang/StaticAnalyzer/Checkers/Checkers.td

@@ -213,13 +213,6 @@ def DereferenceModeling : Checker<"DereferenceModeling">,
 
 def NullDereferenceChecker : Checker<"NullDereference">,
   HelpText<"Check for dereferences of null pointers">,
-  CheckerOptions<[
-    CmdLineOption<Boolean,
-                  "SuppressAddressSpaces",
-                  "Suppresses warning when pointer dereferences an address space",
-                  "true",
-                  Released>
-  ]>,
   Documentation<HasDocumentation>,
   Dependencies<[DereferenceModeling]>;
 
@@ -285,6 +278,12 @@ def FixedAddressChecker : Checker<"FixedAddr">,
   HelpText<"Check for assignment of a fixed address to a pointer">,
   Documentation<HasDocumentation>;
 
+def FixedAddressDereferenceChecker
+    : Checker<"FixedAddressDereference">,
+      HelpText<"Check for dereferences of fixed addresses">,
+      Documentation<HasDocumentation>,
+      Dependencies<[DereferenceModeling]>;
+
 def PointerArithChecker : Checker<"PointerArithm">,
   HelpText<"Check for pointer arithmetic on locations other than array "
            "elements">,

clang/include/clang/StaticAnalyzer/Core/AnalyzerOptions.def

@@ -395,6 +395,19 @@ ANALYZER_OPTION(
     "flex\" won't be analyzed.",
     true)
 
+ANALYZER_OPTION(
+    bool, ShouldSuppressAddressSpaces, "suppress-all-address-spaces",
+    "The analyzer does not report dereferences on memory that use "
+    "address space #256, #257, and #258. Those address spaces are used when "
+    "dereferencing address spaces relative to the GS, FS, and SS segments on "
+    "x86/x86-64 targets. Dereferencing a null pointer in these address spaces "
+    "is not defined as an error. All other null dereferences in other address "
+    "spaces are defined as an error unless explicitly defined. "
+    "When this option is turned on, the special behavior of address spaces "
+    "#256, #257, #258 is extended to all pointers with address spaces and on "
+    "any target.",
+    true)
+
 //===----------------------------------------------------------------------===//
 // Unsigned analyzer options.
 //===----------------------------------------------------------------------===//

clang/lib/StaticAnalyzer/Checkers/DereferenceChecker.cpp

@@ -31,7 +31,12 @@ class DereferenceChecker
     : public Checker< check::Location,
                       check::Bind,
                       EventDispatcher<ImplicitNullDerefEvent> > {
-  enum DerefKind { NullPointer, UndefinedPointerValue, AddressOfLabel };
+  enum DerefKind {
+    NullPointer,
+    UndefinedPointerValue,
+    AddressOfLabel,
+    FixedAddress,
+  };
 
   void reportBug(DerefKind K, ProgramStateRef State, const Stmt *S,
                  CheckerContext &C) const;
@@ -49,13 +54,13 @@ class DereferenceChecker
                              const LocationContext *LCtx,
                              bool loadedFrom = false);
 
-  bool SuppressAddressSpaces = false;
-
   bool CheckNullDereference = false;
+  bool CheckFixedDereference = false;
 
   std::unique_ptr<BugType> BT_Null;
   std::unique_ptr<BugType> BT_Undef;
   std::unique_ptr<BugType> BT_Label;
+  std::unique_ptr<BugType> BT_FixedAddress;
 };
 } // end anonymous namespace
 
@@ -130,7 +135,7 @@ bool DereferenceChecker::suppressReport(CheckerContext &C,
   QualType Ty = E->getType();
   if (!Ty.hasAddressSpace())
     return false;
-  if (SuppressAddressSpaces)
+  if (C.getAnalysisManager().getAnalyzerOptions().ShouldSuppressAddressSpaces)
     return true;
 
   const llvm::Triple::ArchType Arch =
@@ -155,30 +160,47 @@ static bool isDeclRefExprToReference(const Expr *E) {
 
 void DereferenceChecker::reportBug(DerefKind K, ProgramStateRef State,
                                    const Stmt *S, CheckerContext &C) const {
-  if (!CheckNullDereference) {
-    C.addSink();
-    return;
-  }
-
   const BugType *BT = nullptr;
   llvm::StringRef DerefStr1;
   llvm::StringRef DerefStr2;
   switch (K) {
   case DerefKind::NullPointer:
+    if (!CheckNullDereference) {
+      C.addSink();
+      return;
+    }
     BT = BT_Null.get();
     DerefStr1 = " results in a null pointer dereference";
     DerefStr2 = " results in a dereference of a null pointer";
     break;
   case DerefKind::UndefinedPointerValue:
+    if (!CheckNullDereference) {
+      C.addSink();
+      return;
+    }
     BT = BT_Undef.get();
     DerefStr1 = " results in an undefined pointer dereference";
     DerefStr2 = " results in a dereference of an undefined pointer value";
     break;
   case DerefKind::AddressOfLabel:
+    if (!CheckNullDereference) {
+      C.addSink();
+      return;
+    }
     BT = BT_Label.get();
     DerefStr1 = " results in an undefined pointer dereference";
     DerefStr2 = " results in a dereference of an address of a label";
     break;
+  case DerefKind::FixedAddress:
+    // Deliberately don't add a sink node if check is disabled.
+    // This situation may be valid in special cases.
+    if (!CheckFixedDereference)
+      return;
+
+    BT = BT_FixedAddress.get();
+    DerefStr1 = " results in a dereference of a fixed address";
+    DerefStr2 = " results in a dereference of a fixed address";
+    break;
   };
 
   // Generate an error node.
@@ -289,6 +311,13 @@ void DereferenceChecker::checkLocation(SVal l, bool isLoad, const Stmt* S,
     }
   }
 
+  if (location.isConstant()) {
+    const Expr *DerefExpr = getDereferenceExpr(S, isLoad);
+    if (!suppressReport(C, DerefExpr))
+      reportBug(DerefKind::FixedAddress, notNullState, DerefExpr, C);
+    return;
+  }
+
   // From this point forward, we know that the location is not null.
   C.addTransition(notNullState);
 }
@@ -337,6 +366,13 @@ void DereferenceChecker::checkBind(SVal L, SVal V, const Stmt *S,
     }
   }
 
+  if (V.isConstant()) {
+    const Expr *DerefExpr = getDereferenceExpr(S, true);
+    if (!suppressReport(C, DerefExpr))
+      reportBug(DerefKind::FixedAddress, State, DerefExpr, C);
+    return;
+  }
+
   // Unlike a regular null dereference, initializing a reference with a
   // dereferenced null pointer does not actually cause a runtime exception in
   // Clang's implementation of references.
@@ -367,8 +403,6 @@ bool ento::shouldRegisterDereferenceModeling(const CheckerManager &) {
 void ento::registerNullDereferenceChecker(CheckerManager &Mgr) {
   auto *Chk = Mgr.getChecker<DereferenceChecker>();
   Chk->CheckNullDereference = true;
-  Chk->SuppressAddressSpaces = Mgr.getAnalyzerOptions().getCheckerBooleanOption(
-      Mgr.getCurrentCheckerName(), "SuppressAddressSpaces");
   Chk->BT_Null.reset(new BugType(Mgr.getCurrentCheckerName(),
                                  "Dereference of null pointer",
                                  categories::LogicError));
@@ -383,3 +417,16 @@ void ento::registerNullDereferenceChecker(CheckerManager &Mgr) {
 bool ento::shouldRegisterNullDereferenceChecker(const CheckerManager &) {
   return true;
 }
+
+void ento::registerFixedAddressDereferenceChecker(CheckerManager &Mgr) {
+  auto *Chk = Mgr.getChecker<DereferenceChecker>();
+  Chk->CheckFixedDereference = true;
+  Chk->BT_FixedAddress.reset(new BugType(Mgr.getCurrentCheckerName(),
+                                         "Dereference of a fixed address",
+                                         categories::LogicError));
+}
+
+bool ento::shouldRegisterFixedAddressDereferenceChecker(
+    const CheckerManager &) {
+  return true;
+}

clang/test/Analysis/analyzer-config.c

@@ -37,7 +37,6 @@
 // CHECK-NEXT: core.CallAndMessage:NilReceiver = true
 // CHECK-NEXT: core.CallAndMessage:ParameterCount = true
 // CHECK-NEXT: core.CallAndMessage:UndefReceiver = true
-// CHECK-NEXT: core.NullDereference:SuppressAddressSpaces = true
 // CHECK-NEXT: cplusplus.Move:WarnOn = KnownsAndLocals
 // CHECK-NEXT: cplusplus.SmartPtrModeling:ModelSmartPtrDereference = false
 // CHECK-NEXT: crosscheck-with-z3 = false
@@ -124,6 +123,7 @@
 // CHECK-NEXT: silence-checkers = ""
 // CHECK-NEXT: stable-report-filename = false
 // CHECK-NEXT: support-symbolic-integer-casts = false
+// CHECK-NEXT: suppress-all-address-spaces = true
 // CHECK-NEXT: suppress-c++-stdlib = true
 // CHECK-NEXT: suppress-inlined-defensive-checks = true
 // CHECK-NEXT: suppress-null-return-paths = true

clang/test/Analysis/cast-value-notes.cpp

@@ -4,12 +4,12 @@
 //
 // RUN: %clang_analyze_cc1 -std=c++14 -triple amdgcn-unknown-unknown \
 // RUN: -analyzer-checker=core,apiModeling.llvm.CastValue,debug.ExprInspection\
-// RUN:  -analyzer-config core.NullDereference:SuppressAddressSpaces=false\
+// RUN:  -analyzer-config suppress-all-address-spaces=false\
 // RUN:  -analyzer-output=text -verify -DX86 -DNOT_SUPPRESSED %s 2>&1 | FileCheck %s -check-prefix=X86-CHECK
 //
 // RUN: %clang_analyze_cc1 -std=c++14 -triple amdgcn-unknown-unknown \
 // RUN: -analyzer-checker=core,apiModeling.llvm.CastValue,debug.ExprInspection\
-// RUN:  -analyzer-config core.NullDereference:SuppressAddressSpaces=true\
+// RUN:  -analyzer-config suppress-all-address-spaces=true\
 // RUN:  -analyzer-output=text -verify -DX86 -DSUPPRESSED %s 2>&1 | FileCheck %s -check-prefix=X86-CHECK
 //
 // RUN: %clang_analyze_cc1 -std=c++14 -triple x86_64-unknown-unknown \
@@ -18,12 +18,12 @@
 //
 // RUN: %clang_analyze_cc1 -std=c++14 -triple x86_64-unknown-unknown \
 // RUN:  -analyzer-checker=core,apiModeling.llvm.CastValue,debug.ExprInspection\
-// RUN:  -analyzer-config core.NullDereference:SuppressAddressSpaces=true\
+// RUN:  -analyzer-config suppress-all-address-spaces=true\
 // RUN:  -analyzer-output=text -verify -DX86 -DSUPPRESSED %s 2>&1 | FileCheck %s --check-prefix=X86-CHECK-SUPPRESSED
 //
 // RUN: %clang_analyze_cc1 -std=c++14 -triple x86_64-unknown-unknown \
 // RUN:  -analyzer-checker=core,apiModeling.llvm.CastValue,debug.ExprInspection\
-// RUN:  -analyzer-config core.NullDereference:SuppressAddressSpaces=false\
+// RUN:  -analyzer-config suppress-all-address-spaces=false\
 // RUN:  -analyzer-output=text -verify -DX86 -DNOT_SUPPRESSED %s 2>&1 | FileCheck %s --check-prefix=X86-CHECK
 //
 // RUN: %clang_analyze_cc1 -std=c++14 -triple mips-unknown-unknown \
@@ -32,12 +32,12 @@
 //
 // RUN: %clang_analyze_cc1 -std=c++14 -triple mips-unknown-unknown \
 // RUN: -analyzer-checker=core,apiModeling.llvm.CastValue,debug.ExprInspection\
-// RUN: -analyzer-config core.NullDereference:SuppressAddressSpaces=false\
+// RUN: -analyzer-config suppress-all-address-spaces=false\
 // RUN: -analyzer-output=text -verify -DMIPS %s 2>&1
 //
 // RUN: %clang_analyze_cc1 -std=c++14 -triple mips-unknown-unknown \
 // RUN: -analyzer-checker=core,apiModeling.llvm.CastValue,debug.ExprInspection\
-// RUN: -analyzer-config core.NullDereference:SuppressAddressSpaces=true\
+// RUN: -analyzer-config suppress-all-address-spaces=true\
 // RUN: -analyzer-output=text -verify -DMIPS_SUPPRESSED %s
 
 #include "Inputs/llvm.h"