Skip to content
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
43 changes: 17 additions & 26 deletions clang/docs/analyzer/checkers.rst
Original file line number Diff line number Diff line change
Expand Up @@ -118,21 +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``.
Value of this option is also used for checker
:ref:`_alpha-core-FixedAddressDereference`.
*Defaults to true*.

.. code-block:: objc

// C
Expand Down Expand Up @@ -172,6 +157,16 @@ Value of this option is also used for checker
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.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.


.. _core-StackAddressEscape:

core.StackAddressEscape (C)
Expand Down Expand Up @@ -2926,6 +2921,9 @@ Check for assignment of a fixed address to a pointer.
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

Expand All @@ -2945,17 +2943,10 @@ Check for dereferences of fixed addresses.
int x = (*p_function)('x', 'y'); // NO warning yet at functon pointer calls
}

Similarly to :ref:`_core-NullDereference`, the checker intentionally 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.
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.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.


.. _alpha-core-PointerArithm:

Expand Down
7 changes: 0 additions & 7 deletions clang/include/clang/StaticAnalyzer/Checkers/Checkers.td
Original file line number Diff line number Diff line change
Expand Up @@ -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]>;

Expand Down
13 changes: 13 additions & 0 deletions clang/include/clang/StaticAnalyzer/Core/AnalyzerOptions.def
Original file line number Diff line number Diff line change
Expand Up @@ -395,6 +395,19 @@ ANALYZER_OPTION(
"flex\" won't be analyzed.",
true)

ANALYZER_OPTION(
bool, ShouldSuppressAddressSpaces, "suppress-all-address-spaces",
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

"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.
//===----------------------------------------------------------------------===//
Expand Down
6 changes: 1 addition & 5 deletions clang/lib/StaticAnalyzer/Checkers/DereferenceChecker.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -54,8 +54,6 @@ class DereferenceChecker
const LocationContext *LCtx,
bool loadedFrom = false);

bool SuppressAddressSpaces = false;

bool CheckNullDereference = false;
bool CheckFixedDereference = false;

Expand Down Expand Up @@ -137,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 =
Expand Down Expand Up @@ -405,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));
Expand Down
2 changes: 1 addition & 1 deletion clang/test/Analysis/analyzer-config.c
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand Down
12 changes: 6 additions & 6 deletions clang/test/Analysis/cast-value-notes.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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 \
Expand All @@ -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 \
Expand All @@ -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"
Expand Down
19 changes: 0 additions & 19 deletions clang/test/Analysis/concrete-address.c
Original file line number Diff line number Diff line change
Expand Up @@ -142,22 +142,3 @@ void f9() {
// 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;
}
77 changes: 77 additions & 0 deletions clang/test/Analysis/suppress-all-address-spaces.c
Original file line number Diff line number Diff line change
@@ -0,0 +1,77 @@
// 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
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.


#define AS_ATTRIBUTE(_X) volatile __attribute__((address_space(_X)))

#define _get_base() ((void * AS_ATTRIBUTE(256) *)0)

void* test_address_space_array(unsigned long slot) {
return _get_base()[slot]; // other-nosuppress-warning{{Dereference}}
}

void test_address_space_condition(int AS_ATTRIBUTE(257) *cpu_data) {
if (cpu_data == 0) {
*cpu_data = 3; // other-nosuppress-warning{{Dereference}}
}
}

struct X { int member; };
int test_address_space_member(void) {
struct X AS_ATTRIBUTE(258) *data = (struct X AS_ATTRIBUTE(258) *)0UL;
int ret;
ret = data->member; // other-nosuppress-warning{{Dereference}}
return ret;
}

void test_other_address_space_condition(int AS_ATTRIBUTE(259) *cpu_data) {
if (cpu_data == 0) {
*cpu_data = 3; // other-nosuppress-warning{{Dereference}} \
// x86-nosuppress-warning{{Dereference}}
}
}

void test_no_address_space_condition(int *cpu_data) {
if (cpu_data == 0) {
*cpu_data = 3; // other-nosuppress-warning{{Dereference}} \
// x86-nosuppress-warning{{Dereference}} \
// other-suppress-warning{{Dereference}} \
// x86-suppress-warning{{Dereference}}
}
}

#define _fixed_get_base() ((void * AS_ATTRIBUTE(256) *)2)

void* fixed_test_address_space_array(unsigned long slot) {
return _fixed_get_base()[slot]; // other-nosuppress-warning{{Dereference}}
}

void fixed_test_address_space_condition(int AS_ATTRIBUTE(257) *cpu_data) {
if (cpu_data == (int AS_ATTRIBUTE(257) *)2) {
*cpu_data = 3; // other-nosuppress-warning{{Dereference}}
}
}

int fixed_test_address_space_member(void) {
struct X AS_ATTRIBUTE(258) *data = (struct X AS_ATTRIBUTE(258) *)2UL;
int ret;
ret = data->member; // other-nosuppress-warning{{Dereference}}
return ret;
}

void fixed_test_other_address_space_condition(int AS_ATTRIBUTE(259) *cpu_data) {
if (cpu_data == (int AS_ATTRIBUTE(259) *)2) {
*cpu_data = 3; // other-nosuppress-warning{{Dereference}} \
// x86-nosuppress-warning{{Dereference}}
}
}

void fixed_test_no_address_space_condition(int *cpu_data) {
if (cpu_data == (int *)2) {
*cpu_data = 3; // other-nosuppress-warning{{Dereference}} \
// x86-nosuppress-warning{{Dereference}} \
// other-suppress-warning{{Dereference}} \
// x86-suppress-warning{{Dereference}}
}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
}
}

}
Loading