llvmbot wrote:
<!--LLVM PR SUMMARY COMMENT--> @llvm/pr-subscribers-clang-static-analyzer-1 Author: Balázs Kéri (balazske) <details> <summary>Changes</summary> --- 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 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 `````````` </details> https://github.com/llvm/llvm-project/pull/127191 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
