From 9cb00b9ecbe74d19389a5818d61ddee328afe031 Mon Sep 17 00:00:00 2001 From: Jamie Schmeiser Date: Tue, 20 Jul 2021 10:12:20 -0400 Subject: [PATCH] Reland Produce warning for performing pointer arithmetic on a null pointer. Summary: Test and produce warning for subtracting a pointer from null or subtracting null from a pointer. This reland adds the functionality that the warning is no longer reusing an existing warning, it has different wording for C vs C++ to refect the fact that nullptr-nullptr has defined behaviour in C++, it is suppressed when the warning is triggered by a system header and adds -Wnull-pointer-subtraction to allow the warning to be controlled. -Wextra implies -Wnull-pointer-subtraction. Author: Jamie Schmeiser Reviewed By: efriedma (Eli Friedman), nickdesaulniers (Nick Desaulniers) Differential Revision: https://reviews.llvm.org/D98798 --- clang/docs/ReleaseNotes.rst | 4 +++ clang/include/clang/Basic/DiagnosticGroups.td | 2 ++ .../clang/Basic/DiagnosticSemaKinds.td | 3 +++ clang/lib/Sema/SemaExpr.cpp | 27 ++++++++++++++++++- clang/test/Sema/Inputs/pointer-subtraction.h | 1 + clang/test/Sema/pointer-subtraction.c | 19 +++++++++++++ clang/test/Sema/pointer-subtraction.cpp | 19 +++++++++++++ 7 files changed, 74 insertions(+), 1 deletion(-) create mode 100644 clang/test/Sema/Inputs/pointer-subtraction.h create mode 100644 clang/test/Sema/pointer-subtraction.c create mode 100644 clang/test/Sema/pointer-subtraction.cpp diff --git a/clang/docs/ReleaseNotes.rst b/clang/docs/ReleaseNotes.rst index f26c484c5bac..a6f43bcaa4bb 100644 --- a/clang/docs/ReleaseNotes.rst +++ b/clang/docs/ReleaseNotes.rst @@ -76,6 +76,9 @@ New Compiler Flags file contains frame size information for each function defined in the source file. +- ``-Wnull-pointer-subtraction`` emits warning when user code may have + undefined behaviour due to subtraction involving a null pointer. + Deprecated Compiler Flags ------------------------- @@ -92,6 +95,7 @@ Modified Compiler Flags instead. ``-B``'s other GCC-compatible semantics are preserved: ``$prefix/$triple-$file`` and ``$prefix$file`` are searched for executables, libraries, includes, and data files used by the compiler. +- ``-Wextra`` now also implies ``-Wnull-pointer-subtraction.`` Removed Compiler Flags ------------------------- diff --git a/clang/include/clang/Basic/DiagnosticGroups.td b/clang/include/clang/Basic/DiagnosticGroups.td index eb1b5641bfdf..4b4928a7a00e 100644 --- a/clang/include/clang/Basic/DiagnosticGroups.td +++ b/clang/include/clang/Basic/DiagnosticGroups.td @@ -487,6 +487,7 @@ def ClassVarargs : DiagGroup<"class-varargs", [NonPODVarargs]>; def : DiagGroup<"nonportable-cfstrings">; def NonVirtualDtor : DiagGroup<"non-virtual-dtor">; def NullPointerArithmetic : DiagGroup<"null-pointer-arithmetic">; +def NullPointerSubtraction : DiagGroup<"null-pointer-subtraction">; def : DiagGroup<"effc++", [NonVirtualDtor]>; def OveralignedType : DiagGroup<"over-aligned">; def OldStyleCast : DiagGroup<"old-style-cast">; @@ -932,6 +933,7 @@ def Extra : DiagGroup<"extra", [ UnusedParameter, UnusedButSetParameter, NullPointerArithmetic, + NullPointerSubtraction, EmptyInitStatement, StringConcatation, FUseLdPath, diff --git a/clang/include/clang/Basic/DiagnosticSemaKinds.td b/clang/include/clang/Basic/DiagnosticSemaKinds.td index 823a36a597c5..792313fed2a9 100644 --- a/clang/include/clang/Basic/DiagnosticSemaKinds.td +++ b/clang/include/clang/Basic/DiagnosticSemaKinds.td @@ -6390,6 +6390,9 @@ def warn_pointer_arith_null_ptr : Warning< def warn_gnu_null_ptr_arith : Warning< "arithmetic on a null pointer treated as a cast from integer to pointer is a GNU extension">, InGroup, DefaultIgnore; +def warn_pointer_sub_null_ptr : Warning< + "performing pointer subtraction with a null pointer %select{has|may have}0 undefined behavior">, + InGroup, DefaultIgnore; def err_kernel_invalidates_sycl_unique_stable_name : Error<"kernel instantiation changes the result of an evaluated " "'__builtin_sycl_unique_stable_name'">; diff --git a/clang/lib/Sema/SemaExpr.cpp b/clang/lib/Sema/SemaExpr.cpp index f1c49eb082c2..dcb162952d13 100644 --- a/clang/lib/Sema/SemaExpr.cpp +++ b/clang/lib/Sema/SemaExpr.cpp @@ -10432,6 +10432,22 @@ static void diagnoseArithmeticOnNullPointer(Sema &S, SourceLocation Loc, << S.getLangOpts().CPlusPlus << Pointer->getSourceRange(); } +/// Diagnose invalid subraction on a null pointer. +/// +static void diagnoseSubtractionOnNullPointer(Sema &S, SourceLocation Loc, + Expr *Pointer, bool BothNull) { + // Null - null is valid in C++ [expr.add]p7 + if (BothNull && S.getLangOpts().CPlusPlus) + return; + + // Is this s a macro from a system header? + if (S.Diags.getSuppressSystemWarnings() && S.SourceMgr.isInSystemMacro(Loc)) + return; + + S.Diag(Loc, diag::warn_pointer_sub_null_ptr) + << S.getLangOpts().CPlusPlus << Pointer->getSourceRange(); +} + /// Diagnose invalid arithmetic on two function pointers. static void diagnoseArithmeticOnTwoFunctionPointers(Sema &S, SourceLocation Loc, Expr *LHS, Expr *RHS) { @@ -10863,7 +10879,16 @@ QualType Sema::CheckSubtractionOperands(ExprResult &LHS, ExprResult &RHS, LHS.get(), RHS.get())) return QualType(); - // FIXME: Add warnings for nullptr - ptr. + bool LHSIsNullPtr = LHS.get()->IgnoreParenCasts()->isNullPointerConstant( + Context, Expr::NPC_ValueDependentIsNotNull); + bool RHSIsNullPtr = RHS.get()->IgnoreParenCasts()->isNullPointerConstant( + Context, Expr::NPC_ValueDependentIsNotNull); + + // Subtracting nullptr or from nullptr is suspect + if (LHSIsNullPtr) + diagnoseSubtractionOnNullPointer(*this, Loc, LHS.get(), RHSIsNullPtr); + if (RHSIsNullPtr) + diagnoseSubtractionOnNullPointer(*this, Loc, RHS.get(), LHSIsNullPtr); // The pointee type may have zero size. As an extension, a structure or // union may have zero size or an array may have zero length. In this diff --git a/clang/test/Sema/Inputs/pointer-subtraction.h b/clang/test/Sema/Inputs/pointer-subtraction.h new file mode 100644 index 000000000000..d82c88aae33a --- /dev/null +++ b/clang/test/Sema/Inputs/pointer-subtraction.h @@ -0,0 +1 @@ +#define SYSTEM_MACRO(x) (x) = (char *)((char *)0 - (x)) diff --git a/clang/test/Sema/pointer-subtraction.c b/clang/test/Sema/pointer-subtraction.c new file mode 100644 index 000000000000..81e4936ad029 --- /dev/null +++ b/clang/test/Sema/pointer-subtraction.c @@ -0,0 +1,19 @@ +// RUN: %clang_cc1 %s -fsyntax-only -verify -pedantic -Wextra -std=c11 -isystem %S/Inputs +// RUN: %clang_cc1 %s -fsyntax-only -verify -pedantic -Wnull-pointer-subtraction -std=c11 -isystem %S/Inputs +// RUN: %clang_cc1 %s -fsyntax-only -verify -pedantic -Wextra -std=c11 -isystem %S/Inputs -Wsystem-headers -DSYSTEM_WARNINGS +// RUN: %clang_cc1 %s -fsyntax-only -verify -pedantic -Wnull-pointer-subtraction -std=c11 -isystem %S/Inputs -Wsystem-headers -DSYSTEM_WARNINGS + +#include + +void a() { + char *f = (char *)0; + f = (char *)((char *)0 - f); // expected-warning {{performing pointer subtraction with a null pointer has undefined behavior}} + f = (char *)(f - (char *)0); // expected-warning {{performing pointer subtraction with a null pointer has undefined behavior}} + f = (char *)((char *)0 - (char *)0); // expected-warning {{performing pointer subtraction with a null pointer has undefined behavior}} expected-warning {{performing pointer subtraction with a null pointer has undefined behavior}} + +#ifndef SYSTEM_WARNINGS + SYSTEM_MACRO(f); +#else + SYSTEM_MACRO(f); // expected-warning {{performing pointer subtraction with a null pointer has undefined behavior}} +#endif +} diff --git a/clang/test/Sema/pointer-subtraction.cpp b/clang/test/Sema/pointer-subtraction.cpp new file mode 100644 index 000000000000..efbb3255e5d3 --- /dev/null +++ b/clang/test/Sema/pointer-subtraction.cpp @@ -0,0 +1,19 @@ +// RUN: %clang_cc1 %s -fsyntax-only -verify -pedantic -Wextra -std=c++11 -isystem %S/Inputs +// RUN: %clang_cc1 %s -fsyntax-only -verify -pedantic -Wnull-pointer-subtraction -std=c++11 -isystem %S/Inputs +// RUN: %clang_cc1 %s -fsyntax-only -verify -pedantic -Wextra -std=c++11 -isystem %S/Inputs -Wsystem-headers -DSYSTEM_WARNINGS +// RUN: %clang_cc1 %s -fsyntax-only -verify -pedantic -Wnull-pointer-subtraction -std=c++11 -isystem %S/Inputs -Wsystem-headers -DSYSTEM_WARNINGS + +#include + +void a() { + char *f = (char *)0; + f = (char *)((char *)0 - f); // expected-warning {{performing pointer subtraction with a null pointer may have undefined behavior}} + f = (char *)(f - (char *)0); // expected-warning {{performing pointer subtraction with a null pointer may have undefined behavior}} + f = (char *)((char *)0 - (char *)0); // valid in C++ + +#ifndef SYSTEM_WARNINGS + SYSTEM_MACRO(f); +#else + SYSTEM_MACRO(f); // expected-warning {{performing pointer subtraction with a null pointer may have undefined behavior}} +#endif +}