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

[balazske]

No description provided.

[llvmbot]

@llvm/pr-subscribers-clang

Author: Balázs Kéri (balazske)

Changes

---

Full diff: https://github.com/llvm/llvm-project/pull/127191.diff

5 Files Affected:

• (modified) clang/docs/analyzer/checkers.rst (+38)
• (modified) clang/include/clang/StaticAnalyzer/Checkers/Checkers.td (+15-11)
• (modified) clang/lib/StaticAnalyzer/Checkers/DereferenceChecker.cpp (+57-6)
• (modified) clang/test/Analysis/concrete-address.c (+159-3)
• (modified) clang/test/Analysis/misc-ps.m (+2-2)

diff --git a/clang/docs/analyzer/checkers.rst b/clang/docs/analyzer/checkers.rst
index 707067358fdfe..c5e10f5a8bbf2 100644
--- a/clang/docs/analyzer/checkers.rst
+++ b/clang/docs/analyzer/checkers.rst
@@ -129,6 +129,8 @@ 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``.
+Value of this option is used for checker :ref:`_core-FixedAddressDereference`
+too.
 *Defaults to true*.
 
 .. code-block:: objc
@@ -2919,6 +2921,42 @@ 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 values used as pointers.
+
+Similarly as at :ref:`_core-NullDereference`, the checker specifically does
+not report 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.)
+
+If you want to disable this behavior, set the ``SuppressAddressSpaces`` option
+of checker ``core.NullDereference`` to false, like
+``-analyzer-config core.NullDereference:SuppressAddressSpaces=false``. The value
+of this option is used for both checkers.
+
+.. 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
+ }
+
 .. _alpha-core-PointerArithm:
 
 alpha.core.PointerArithm (C)
diff --git a/clang/include/clang/StaticAnalyzer/Checkers/Checkers.td b/clang/include/clang/StaticAnalyzer/Checkers/Checkers.td
index 9bf491eac1e0e..44ca28c003b06 100644
--- a/clang/include/clang/StaticAnalyzer/Checkers/Checkers.td
+++ b/clang/include/clang/StaticAnalyzer/Checkers/Checkers.td
@@ -211,17 +211,15 @@ def DereferenceModeling : Checker<"DereferenceModeling">,
   Documentation<NotDocumented>,
   Hidden;
 
-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]>;
+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]>;
 
 def NonNullParamChecker : Checker<"NonNullParamChecker">,
   HelpText<"Check for null pointers passed as arguments to a function whose "
@@ -285,6 +283,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 values used as pointers">,
+      Documentation<HasDocumentation>,
+      Dependencies<[DereferenceModeling]>;
+
 def PointerArithChecker : Checker<"PointerArithm">,
   HelpText<"Check for pointer arithmetic on locations other than array "
            "elements">,
diff --git a/clang/lib/StaticAnalyzer/Checkers/DereferenceChecker.cpp b/clang/lib/StaticAnalyzer/Checkers/DereferenceChecker.cpp
index e9e2771c739b6..6a3f70e62e77b 100644
--- a/clang/lib/StaticAnalyzer/Checkers/DereferenceChecker.cpp
+++ b/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;
@@ -52,10 +57,12 @@ class DereferenceChecker
   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
 
@@ -155,30 +162,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 +313,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 +368,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.
@@ -383,3 +421,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;
+}
diff --git a/clang/test/Analysis/concrete-address.c b/clang/test/Analysis/concrete-address.c
index 346f5093e44f7..af99aa43a98a8 100644
--- a/clang/test/Analysis/concrete-address.c
+++ b/clang/test/Analysis/concrete-address.c
@@ -1,7 +1,163 @@
 // RUN: %clang_analyze_cc1 -analyzer-checker=core,alpha.core -verify %s
-// expected-no-diagnostics
 
-void foo(void) {
+extern void __assert_fail (__const char *__assertion, __const char *__file,
+    unsigned int __line, __const char *__function)
+     __attribute__ ((__noreturn__));
+
+#define assert(expr) \
+  ((expr)  ? (void)(0)  : __assert_fail (#expr, __FILE__, __LINE__, __func__))
+
+typedef unsigned long uintptr_t;
+
+void f0(void) {
   int *p = (int*) 0x10000; // Should not crash here.
-  *p = 3;
+  *p = 3; // expected-warning{{Dereference of a fixed address}}
+}
+
+void f1(int *p) {
+  if (p != (int *)-1)
+    *p = 1;
+  else
+    *p = 0; // expected-warning{{Dereference of a fixed address}}
+}
+
+struct f2_struct {
+  int x;
+};
+
+int f2(struct f2_struct* p) {
+
+  if (p != (struct f2_struct *)1)
+    p->x = 1;
+
+  return p->x++; // expected-warning{{Access to field 'x' results in a dereference of a fixed address (loaded from variable 'p')}}
+}
+
+int f3_1(char* x) {
+  int i = 2;
+
+  if (x != (char *)1)
+    return x[i - 1];
+
+  return x[i+1]; // expected-warning{{Array access (from variable 'x') results in a dereference of a fixed address}}
+}
+
+int f3_2(char* x) {
+  int i = 2;
+
+  if (x != (char *)1)
+    return x[i - 1];
+
+  return x[i+1]++; // expected-warning{{Array access (from variable 'x') results in a dereference of a fixed address}}
+}
+
+int f4_1(int *p) {
+  uintptr_t x = (uintptr_t) p;
+
+  if (x != (uintptr_t)1)
+    return 1;
+
+  int *q = (int*) x;
+  return *q; // expected-warning{{Dereference of a fixed address (loaded from variable 'q')}}
+}
+
+int f4_2(void) {
+  short array[2];
+  uintptr_t x = (uintptr_t)array;
+  short *p = (short *)x;
+
+  // The following branch should be infeasible.
+  if (!(p == &array[0])) {
+    p = (short *)1;
+    *p = 1; // no-warning
+  }
+
+  if (p != (short *)1) {
+    *p = 5; // no-warning
+    p = (short *)1; // expected-warning {{Using a fixed address is not portable}}
+  }
+  else return 1;
+
+  *p += 10; // expected-warning{{Dereference of a fixed}}
+  return 0;
+}
+
+int f5(void) {
+  char *s = "hello world";
+  return s[0]; // no-warning
+}
+
+void f6(int *p, int *q) {
+  if (p != (int *)1)
+    if (p == (int *)1)
+      *p = 1; // no-warning
+
+  if (q == (int *)1)
+    if (q != (int *)1)
+      *q = 1; // no-warning
+}
+
+int* qux(int);
+
+int f7_1(unsigned len) {
+  assert (len != 0);
+  int *p = (int *)1;
+  unsigned i;
+
+  for (i = 0; i < len; ++i)
+   p = qux(i);
+
+  return *p++; // no-warning
+}
+
+int f7_2(unsigned len) {
+  assert (len > 0);  // note use of '>'
+  int *p = (int *)1;
+  unsigned i;
+
+  for (i = 0; i < len; ++i)
+   p = qux(i);
+
+  return *p++; // no-warning
+}
+
+struct f8_s {
+  int x;
+  int y[2];
+};
+
+void f8(struct f8_s *s, int coin) {
+  if (s != (struct f8_s *)7)
+    return;
+
+  if (coin)
+    s->x = 5; // expected-warning{{Access to field 'x' results in a dereference of a fixed address (loaded from variable 's')}}
+  else
+    s->y[1] = 6; // expected-warning{{Array access (via field 'y') results in a dereference of a fixed address}}
+}
+
+void f9() {
+  int (*p_function) (char, char) = (int (*)(char, char))0x04040; // FIXME: warn at this initialization
+  p_function = (int (*)(char, char))0x04080; // expected-warning {{Using a fixed address is not portable}}
+  // FIXME: there should be a warning from calling the function pointer with fixed address
+  int x = (*p_function) ('x', 'y');
+}
+
+#define AS_ATTRIBUTE volatile __attribute__((address_space(256)))
+#define _get_base() ((void * AS_ATTRIBUTE *)0x10)
+
+void* test_address_space_array(unsigned long slot) {
+  return _get_base()[slot]; // no-warning
+}
+void test_address_space_condition(int AS_ATTRIBUTE *cpu_data) {
+   if (cpu_data == (int *)0x10) {
+    *cpu_data = 3; // no-warning
+  }
+}
+struct X { int member; };
+int test_address_space_member(void) {
+  struct X AS_ATTRIBUTE *data = (struct X AS_ATTRIBUTE *)0x10UL;
+  int ret;
+  ret = data->member; // no-warning
+  return ret;
 }
diff --git a/clang/test/Analysis/misc-ps.m b/clang/test/Analysis/misc-ps.m
index 0a8a30cb6175c..841f618fbdfab 100644
--- a/clang/test/Analysis/misc-ps.m
+++ b/clang/test/Analysis/misc-ps.m
@@ -1,6 +1,6 @@
 // NOTE: Use '-fobjc-gc' to test the analysis being run twice, and multiple reports are not issued.
-// RUN: %clang_analyze_cc1 -triple i386-apple-darwin10 -analyzer-checker=core,alpha.core,osx.cocoa.AtSync -Wno-strict-prototypes -Wno-pointer-to-int-cast -verify -fblocks -Wno-unreachable-code -Wno-null-dereference -Wno-objc-root-class %s
-// RUN: %clang_analyze_cc1 -triple x86_64-apple-darwin10 -analyzer-checker=core,alpha.core,osx.cocoa.AtSync -Wno-strict-prototypes -Wno-pointer-to-int-cast -verify -fblocks -Wno-unreachable-code -Wno-null-dereference -Wno-objc-root-class %s
+// RUN: %clang_analyze_cc1 -triple i386-apple-darwin10 -analyzer-checker=core,alpha.core,osx.cocoa.AtSync -analyzer-disable-checker=alpha.core.FixedAddressDereference -Wno-strict-prototypes -Wno-pointer-to-int-cast -verify -fblocks -Wno-unreachable-code -Wno-null-dereference -Wno-objc-root-class %s
+// RUN: %clang_analyze_cc1 -triple x86_64-apple-darwin10 -analyzer-checker=core,alpha.core,osx.cocoa.AtSync -analyzer-disable-checker=alpha.core.FixedAddressDereference -Wno-strict-prototypes -Wno-pointer-to-int-cast -verify -fblocks -Wno-unreachable-code -Wno-null-dereference -Wno-objc-root-class %s
 
 #ifndef __clang_analyzer__
 #error __clang_analyzer__ not defined

[llvmbot]

@llvm/pr-subscribers-clang-static-analyzer-1

Author: Balázs Kéri (balazske)

Changes

---

Full diff: https://github.com/llvm/llvm-project/pull/127191.diff

5 Files Affected:

• (modified) clang/docs/analyzer/checkers.rst (+38)
• (modified) clang/include/clang/StaticAnalyzer/Checkers/Checkers.td (+15-11)
• (modified) clang/lib/StaticAnalyzer/Checkers/DereferenceChecker.cpp (+57-6)
• (modified) clang/test/Analysis/concrete-address.c (+159-3)
• (modified) clang/test/Analysis/misc-ps.m (+2-2)

diff --git a/clang/docs/analyzer/checkers.rst b/clang/docs/analyzer/checkers.rst
index 707067358fdfe..c5e10f5a8bbf2 100644
--- a/clang/docs/analyzer/checkers.rst
+++ b/clang/docs/analyzer/checkers.rst
@@ -129,6 +129,8 @@ 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``.
+Value of this option is used for checker :ref:`_core-FixedAddressDereference`
+too.
 *Defaults to true*.
 
 .. code-block:: objc
@@ -2919,6 +2921,42 @@ 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 values used as pointers.
+
+Similarly as at :ref:`_core-NullDereference`, the checker specifically does
+not report 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.)
+
+If you want to disable this behavior, set the ``SuppressAddressSpaces`` option
+of checker ``core.NullDereference`` to false, like
+``-analyzer-config core.NullDereference:SuppressAddressSpaces=false``. The value
+of this option is used for both checkers.
+
+.. 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
+ }
+
 .. _alpha-core-PointerArithm:
 
 alpha.core.PointerArithm (C)
diff --git a/clang/include/clang/StaticAnalyzer/Checkers/Checkers.td b/clang/include/clang/StaticAnalyzer/Checkers/Checkers.td
index 9bf491eac1e0e..44ca28c003b06 100644
--- a/clang/include/clang/StaticAnalyzer/Checkers/Checkers.td
+++ b/clang/include/clang/StaticAnalyzer/Checkers/Checkers.td
@@ -211,17 +211,15 @@ def DereferenceModeling : Checker<"DereferenceModeling">,
   Documentation<NotDocumented>,
   Hidden;
 
-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]>;
+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]>;
 
 def NonNullParamChecker : Checker<"NonNullParamChecker">,
   HelpText<"Check for null pointers passed as arguments to a function whose "
@@ -285,6 +283,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 values used as pointers">,
+      Documentation<HasDocumentation>,
+      Dependencies<[DereferenceModeling]>;
+
 def PointerArithChecker : Checker<"PointerArithm">,
   HelpText<"Check for pointer arithmetic on locations other than array "
            "elements">,
diff --git a/clang/lib/StaticAnalyzer/Checkers/DereferenceChecker.cpp b/clang/lib/StaticAnalyzer/Checkers/DereferenceChecker.cpp
index e9e2771c739b6..6a3f70e62e77b 100644
--- a/clang/lib/StaticAnalyzer/Checkers/DereferenceChecker.cpp
+++ b/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;
@@ -52,10 +57,12 @@ class DereferenceChecker
   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
 
@@ -155,30 +162,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 +313,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 +368,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.
@@ -383,3 +421,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;
+}
diff --git a/clang/test/Analysis/concrete-address.c b/clang/test/Analysis/concrete-address.c
index 346f5093e44f7..af99aa43a98a8 100644
--- a/clang/test/Analysis/concrete-address.c
+++ b/clang/test/Analysis/concrete-address.c
@@ -1,7 +1,163 @@
 // RUN: %clang_analyze_cc1 -analyzer-checker=core,alpha.core -verify %s
-// expected-no-diagnostics
 
-void foo(void) {
+extern void __assert_fail (__const char *__assertion, __const char *__file,
+    unsigned int __line, __const char *__function)
+     __attribute__ ((__noreturn__));
+
+#define assert(expr) \
+  ((expr)  ? (void)(0)  : __assert_fail (#expr, __FILE__, __LINE__, __func__))
+
+typedef unsigned long uintptr_t;
+
+void f0(void) {
   int *p = (int*) 0x10000; // Should not crash here.
-  *p = 3;
+  *p = 3; // expected-warning{{Dereference of a fixed address}}
+}
+
+void f1(int *p) {
+  if (p != (int *)-1)
+    *p = 1;
+  else
+    *p = 0; // expected-warning{{Dereference of a fixed address}}
+}
+
+struct f2_struct {
+  int x;
+};
+
+int f2(struct f2_struct* p) {
+
+  if (p != (struct f2_struct *)1)
+    p->x = 1;
+
+  return p->x++; // expected-warning{{Access to field 'x' results in a dereference of a fixed address (loaded from variable 'p')}}
+}
+
+int f3_1(char* x) {
+  int i = 2;
+
+  if (x != (char *)1)
+    return x[i - 1];
+
+  return x[i+1]; // expected-warning{{Array access (from variable 'x') results in a dereference of a fixed address}}
+}
+
+int f3_2(char* x) {
+  int i = 2;
+
+  if (x != (char *)1)
+    return x[i - 1];
+
+  return x[i+1]++; // expected-warning{{Array access (from variable 'x') results in a dereference of a fixed address}}
+}
+
+int f4_1(int *p) {
+  uintptr_t x = (uintptr_t) p;
+
+  if (x != (uintptr_t)1)
+    return 1;
+
+  int *q = (int*) x;
+  return *q; // expected-warning{{Dereference of a fixed address (loaded from variable 'q')}}
+}
+
+int f4_2(void) {
+  short array[2];
+  uintptr_t x = (uintptr_t)array;
+  short *p = (short *)x;
+
+  // The following branch should be infeasible.
+  if (!(p == &array[0])) {
+    p = (short *)1;
+    *p = 1; // no-warning
+  }
+
+  if (p != (short *)1) {
+    *p = 5; // no-warning
+    p = (short *)1; // expected-warning {{Using a fixed address is not portable}}
+  }
+  else return 1;
+
+  *p += 10; // expected-warning{{Dereference of a fixed}}
+  return 0;
+}
+
+int f5(void) {
+  char *s = "hello world";
+  return s[0]; // no-warning
+}
+
+void f6(int *p, int *q) {
+  if (p != (int *)1)
+    if (p == (int *)1)
+      *p = 1; // no-warning
+
+  if (q == (int *)1)
+    if (q != (int *)1)
+      *q = 1; // no-warning
+}
+
+int* qux(int);
+
+int f7_1(unsigned len) {
+  assert (len != 0);
+  int *p = (int *)1;
+  unsigned i;
+
+  for (i = 0; i < len; ++i)
+   p = qux(i);
+
+  return *p++; // no-warning
+}
+
+int f7_2(unsigned len) {
+  assert (len > 0);  // note use of '>'
+  int *p = (int *)1;
+  unsigned i;
+
+  for (i = 0; i < len; ++i)
+   p = qux(i);
+
+  return *p++; // no-warning
+}
+
+struct f8_s {
+  int x;
+  int y[2];
+};
+
+void f8(struct f8_s *s, int coin) {
+  if (s != (struct f8_s *)7)
+    return;
+
+  if (coin)
+    s->x = 5; // expected-warning{{Access to field 'x' results in a dereference of a fixed address (loaded from variable 's')}}
+  else
+    s->y[1] = 6; // expected-warning{{Array access (via field 'y') results in a dereference of a fixed address}}
+}
+
+void f9() {
+  int (*p_function) (char, char) = (int (*)(char, char))0x04040; // FIXME: warn at this initialization
+  p_function = (int (*)(char, char))0x04080; // expected-warning {{Using a fixed address is not portable}}
+  // FIXME: there should be a warning from calling the function pointer with fixed address
+  int x = (*p_function) ('x', 'y');
+}
+
+#define AS_ATTRIBUTE volatile __attribute__((address_space(256)))
+#define _get_base() ((void * AS_ATTRIBUTE *)0x10)
+
+void* test_address_space_array(unsigned long slot) {
+  return _get_base()[slot]; // no-warning
+}
+void test_address_space_condition(int AS_ATTRIBUTE *cpu_data) {
+   if (cpu_data == (int *)0x10) {
+    *cpu_data = 3; // no-warning
+  }
+}
+struct X { int member; };
+int test_address_space_member(void) {
+  struct X AS_ATTRIBUTE *data = (struct X AS_ATTRIBUTE *)0x10UL;
+  int ret;
+  ret = data->member; // no-warning
+  return ret;
 }
diff --git a/clang/test/Analysis/misc-ps.m b/clang/test/Analysis/misc-ps.m
index 0a8a30cb6175c..841f618fbdfab 100644
--- a/clang/test/Analysis/misc-ps.m
+++ b/clang/test/Analysis/misc-ps.m
@@ -1,6 +1,6 @@
 // NOTE: Use '-fobjc-gc' to test the analysis being run twice, and multiple reports are not issued.
-// RUN: %clang_analyze_cc1 -triple i386-apple-darwin10 -analyzer-checker=core,alpha.core,osx.cocoa.AtSync -Wno-strict-prototypes -Wno-pointer-to-int-cast -verify -fblocks -Wno-unreachable-code -Wno-null-dereference -Wno-objc-root-class %s
-// RUN: %clang_analyze_cc1 -triple x86_64-apple-darwin10 -analyzer-checker=core,alpha.core,osx.cocoa.AtSync -Wno-strict-prototypes -Wno-pointer-to-int-cast -verify -fblocks -Wno-unreachable-code -Wno-null-dereference -Wno-objc-root-class %s
+// RUN: %clang_analyze_cc1 -triple i386-apple-darwin10 -analyzer-checker=core,alpha.core,osx.cocoa.AtSync -analyzer-disable-checker=alpha.core.FixedAddressDereference -Wno-strict-prototypes -Wno-pointer-to-int-cast -verify -fblocks -Wno-unreachable-code -Wno-null-dereference -Wno-objc-root-class %s
+// RUN: %clang_analyze_cc1 -triple x86_64-apple-darwin10 -analyzer-checker=core,alpha.core,osx.cocoa.AtSync -analyzer-disable-checker=alpha.core.FixedAddressDereference -Wno-strict-prototypes -Wno-pointer-to-int-cast -verify -fblocks -Wno-unreachable-code -Wno-null-dereference -Wno-objc-root-class %s
 
 #ifndef __clang_analyzer__
 #error __clang_analyzer__ not defined

[balazske]

The checker is alpha because there are known problems with it which I plan to fix later.
A problem is with the bugpath messages where a constant pointer was assumed to be a null pointer but after this checker (or even before it?) this is not true.

[steakhal]

I had the impression that the core dereference checker already checks this.

[balazske]

It may cause confusion that "NullDereference" checker checks not only null dereference but undefined pointer and label address too. Probably these checks (specially label address) can be moved into this checker. (Or add the new check to NullDereference without a new checker?)

[steakhal]

It may cause confusion that "NullDereference" checker checks not only null dereference but undefined pointer and label address too. Probably these checks (specially label address) can be moved into this checker. (Or add the new check to NullDereference without a new checker?)

I see. Thanks.
I dont have a firm opinion on the architecture. Usually we should go with what is more natural. If there would be signifficat semantic sharing, then a single checker works usually better. Its up to you.

[steakhal]

Looks pretty good. Thank you!

[steakhal]

I've marked the resolved comments. There are a few unresolved conversations left.

[steakhal]

I'm happy with your PR. I think it looks great.

[steakhal]

Suggested change

	}
	}

[NagyDonat]

Overall I like the change, but I feel that the analyzer option name suppress-all-address-spaces is too misleading (see inline comment for more detailed suggestion).

[NagyDonat]

Suggested change

	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.
	for reference.
	If the analyzer option ``suppress-all-address-spaces`` is set to true (the
	default value), then this checker never reports dereference of pointers with a
	specified address space. If the option is set to false, then reports from the
	specific x86 address spaces 256, 257 and 258 are still suppressed, but null
	dereferences from other address spaces are reported.

I think it is better to mention "(the default value)" immediately when you describe it.

[NagyDonat]

Suggested change

	// RUN: %clang_analyze_cc1 -triple x86_64-pc-linux-gnu -analyzer-checker=core,alpha.core -std=gnu99 -analyzer-config suppress-all-address-spaces=false -verify=x86-nosuppress %s
	// RUN: %clang_analyze_cc1 -triple x86_64-pc-linux-gnu -analyzer-checker=core,alpha.core -std=gnu99 -verify=x86-suppress %s
	// RUN: %clang_analyze_cc1 -triple arm-pc-linux-gnu -analyzer-checker=core,alpha.core -std=gnu99 -analyzer-config suppress-all-address-spaces=false -verify=other-nosuppress %s
	// RUN: %clang_analyze_cc1 -triple arm-pc-linux-gnu -analyzer-checker=core,alpha.core -std=gnu99 -verify=other-suppress %s
	// RUN: %clang_analyze_cc1 -triple x86_64-pc-linux-gnu -analyzer-checker=core,alpha.core -std=gnu99 -analyzer-config suppress-all-address-spaces=false -verify=common,x86-nosuppress %s
	// RUN: %clang_analyze_cc1 -triple x86_64-pc-linux-gnu -analyzer-checker=core,alpha.core -std=gnu99 -verify=common,x86-suppress %s
	// RUN: %clang_analyze_cc1 -triple arm-pc-linux-gnu -analyzer-checker=core,alpha.core -std=gnu99 -analyzer-config suppress-all-address-spaces=false -verify=common,other-nosuppress %s
	// RUN: %clang_analyze_cc1 -triple arm-pc-linux-gnu -analyzer-checker=core,alpha.core -std=gnu99 -verify=common,other-suppress %s

The -verify argument can accept multiple comma-separated labels and this can be used to simplify the code in cases where the same warning is emitted in many run lines. For example if you add the same string (e.g. common as in the suggested edit), you can write common-warning {{....}} instead of separately listing each of the four different labels.

[NagyDonat]

Suggested change

	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.
	If the analyzer option ``suppress-all-address-spaces`` is set to true (the
	default value), then this checker never reports dereference of pointers with a
	specified address space. If the option is set to false, then reports from the
	specific x86 address spaces 256, 257 and 258 are still suppressed, but fixed
	address dereferences from other address spaces are reported.

This is the same paragraph that I suggested for the NullDereference checker.

[NagyDonat]

The name of this analyzer option is misleading: the natural guess is that it makes the analyzer ignore the address space annotations -- and it's completely counter-intuitive that it suppresses certain results that come from two particular checkers. Of course the attached documentation describes its meaning, but I don't think that's enough (especially considering that these option docs are currently very obscure -- e.g. they don't appear on the webpage.

Note that this name was already misleading when it belonged to just a checker option, but moving it into the global namespace exacerbates the problem.

I know that it's difficult to find a more accurate name, but perhaps something like ignore-deref-from-all-address-spaces could work.

@steakhal What do you think about this?

[balazske]

I think that "ignore-all-address-spaces" would be really problematic but "suppress" is somehow related to eliminate reporting of specific errors (or path suppression). Probably "suppress-dereferences-from-any-address-space" is better.