jamieschmeiser updated this revision to Diff 346515.
jamieschmeiser edited the summary of this revision.
jamieschmeiser added a comment.
As requested, I have added a new warning option -Wnull-pointer-subtraction (and
added it to -Wextra) that does not trigger on system headers. In addition, I
changed the message used such that it states that it is undefined behaviour for
C but may be undefined behaviour in C++ to address the concerns that performing
the subtraction with a pointer that dynamically becomes null is not undefined
behaviour. Expanded the testing to also test that the warning does not fire on
system headers. Also, updated the release notes to indicate the new option.
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D98798/new/
https://reviews.llvm.org/D98798
Files:
clang/docs/ReleaseNotes.rst
clang/include/clang/Basic/DiagnosticGroups.td
clang/include/clang/Basic/DiagnosticSemaKinds.td
clang/lib/Sema/SemaExpr.cpp
clang/test/Sema/Inputs/pointer-subtraction.h
clang/test/Sema/pointer-subtraction.c
clang/test/Sema/pointer-subtraction.cpp
Index: clang/test/Sema/pointer-subtraction.cpp
===================================================================
--- /dev/null
+++ 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 <pointer-subtraction.h>
+
+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
+}
Index: clang/test/Sema/pointer-subtraction.c
===================================================================
--- /dev/null
+++ 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 <pointer-subtraction.h>
+
+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
+}
Index: clang/test/Sema/Inputs/pointer-subtraction.h
===================================================================
--- /dev/null
+++ clang/test/Sema/Inputs/pointer-subtraction.h
@@ -0,0 +1 @@
+#define SYSTEM_MACRO(x) (x) = (char*)((char*)0 - (x))
Index: clang/lib/Sema/SemaExpr.cpp
===================================================================
--- clang/lib/Sema/SemaExpr.cpp
+++ clang/lib/Sema/SemaExpr.cpp
@@ -10366,6 +10366,22 @@
<< 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) {
@@ -10797,7 +10813,16 @@
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
Index: clang/include/clang/Basic/DiagnosticSemaKinds.td
===================================================================
--- clang/include/clang/Basic/DiagnosticSemaKinds.td
+++ clang/include/clang/Basic/DiagnosticSemaKinds.td
@@ -6351,6 +6351,9 @@
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<NullPointerArithmetic>, DefaultIgnore;
+def warn_pointer_sub_null_ptr : Warning<
+ "performing pointer subtraction with a null pointer %select{has|may have}0 undefined behavior">,
+ InGroup<NullPointerSubtraction>, DefaultIgnore;
def warn_floatingpoint_eq : Warning<
"comparing floating point with == or != is unsafe">,
Index: clang/include/clang/Basic/DiagnosticGroups.td
===================================================================
--- clang/include/clang/Basic/DiagnosticGroups.td
+++ clang/include/clang/Basic/DiagnosticGroups.td
@@ -487,6 +487,7 @@
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">;
@@ -928,6 +929,7 @@
SignCompare,
UnusedParameter,
NullPointerArithmetic,
+ NullPointerSubtraction,
EmptyInitStatement,
StringConcatation,
FUseLdPath,
Index: clang/docs/ReleaseNotes.rst
===================================================================
--- clang/docs/ReleaseNotes.rst
+++ clang/docs/ReleaseNotes.rst
@@ -74,6 +74,9 @@
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
-------------------------
@@ -90,6 +93,7 @@
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
-------------------------
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
