andrew.w.kaylor updated this revision to Diff 113134.
andrew.w.kaylor added a comment.
Added warnings for null pointer arithmetic.
https://reviews.llvm.org/D37042
Files:
include/clang/Basic/DiagnosticGroups.td
include/clang/Basic/DiagnosticSemaKinds.td
lib/CodeGen/CGExprScalar.cpp
lib/Sema/SemaExpr.cpp
test/CodeGen/nullptr-arithmetic.c
test/Sema/pointer-addition.c
Index: lib/CodeGen/CGExprScalar.cpp
===================================================================
--- lib/CodeGen/CGExprScalar.cpp
+++ lib/CodeGen/CGExprScalar.cpp
@@ -2671,6 +2671,37 @@
unsigned width = cast<llvm::IntegerType>(index->getType())->getBitWidth();
auto &DL = CGF.CGM.getDataLayout();
auto PtrTy = cast<llvm::PointerType>(pointer->getType());
+
+ // Some versions of glibc and gcc use idioms (particularly in their malloc
+ // routines) that add a pointer-sized integer (known to be a pointer value)
+ // to a null pointer in order to cast the value back to an integer or as
+ // part of a pointer alignment algorithm. This is undefined behavior, but
+ // we'd like to be able to compile programs that use it.
+ //
+ // Normally, we'd generate a GEP with a null-pointer base here in response
+ // to that code, but it's also UB to dereference a pointer created that
+ // way. Instead (as an acknowledged hack to tolerate the idiom) we will
+ // generate a direct cast of the integer value to a pointer.
+ //
+ // The idiom (p = nullptr + N) is not met if any of the following are true:
+ //
+ // The operation is subtraction.
+ // The index is not pointer-sized.
+ // The pointer type is not byte-sized.
+ // The index operand is a constant.
+ //
+ if (isa<llvm::ConstantPointerNull>(pointer) && !isSubtraction &&
+ (width == DL.getTypeSizeInBits(PtrTy)) &&
+ !isa<llvm::Constant>(index)) {
+ // The pointer type might come back as null, so it's deferred until here.
+ const PointerType *pointerType
+ = pointerOperand->getType()->getAs<PointerType>();
+ if (pointerType && pointerType->getPointeeType()->isCharType()) {
+ // (nullptr + N) -> inttoptr N to <PtrTy>
+ return CGF.Builder.CreateIntToPtr(index, pointer->getType());
+ }
+ }
+
if (width != DL.getTypeSizeInBits(PtrTy)) {
// Zero-extend or sign-extend the pointer value according to
// whether the index is signed or not.
Index: lib/Sema/SemaExpr.cpp
===================================================================
--- lib/Sema/SemaExpr.cpp
+++ lib/Sema/SemaExpr.cpp
@@ -8490,6 +8490,18 @@
<< 0 /* one pointer */ << Pointer->getSourceRange();
}
+/// \brief Diagnose invalid arithmetic on a null pointer.
+///
+/// If \p IsGNUIdiom is true, the operation is using the 'p = (i8*)nullptr + n'
+/// idiom, which we recognize as a GNU extension.
+///
+static void diagnoseArithmeticOnNullPointer(Sema &S, SourceLocation Loc,
+ Expr *Pointer, bool IsGNUIdiom) {
+ S.Diag(Loc, IsGNUIdiom ? diag::ext_gnu_null_ptr_arith
+ : diag::warn_pointer_arith_null_ptr)
+ << Pointer->getSourceRange();
+}
+
/// \brief Diagnose invalid arithmetic on two function pointers.
static void diagnoseArithmeticOnTwoFunctionPointers(Sema &S, SourceLocation Loc,
Expr *LHS, Expr *RHS) {
@@ -8784,6 +8796,21 @@
if (!IExp->getType()->isIntegerType())
return InvalidOperands(Loc, LHS, RHS);
+ // Adding to a null pointer is never an error, but should warn.
+ if (PExp->IgnoreParenCasts()->isNullPointerConstant(Context,
+ Expr::NPC_ValueDependentIsNull)) {
+ // Check the conditions to see if this is the 'p = (i8*)nullptr + n' idiom.
+ bool IsGNUIdiom = false;
+ const PointerType *pointerType = PExp->getType()->getAs<PointerType>();
+ if (!IExp->isIntegerConstantExpr(Context) &&
+ pointerType->getPointeeType()->isCharType() &&
+ Context.getTypeSize(pointerType) ==
+ Context.getTypeSize(IExp->getType()))
+ IsGNUIdiom = true;
+
+ diagnoseArithmeticOnNullPointer(*this, Loc, PExp, IsGNUIdiom);
+ }
+
if (!checkArithmeticOpPointerOperand(*this, Loc, PExp))
return QualType();
@@ -8845,6 +8872,13 @@
// The result type of a pointer-int computation is the pointer type.
if (RHS.get()->getType()->isIntegerType()) {
+ // Subtracting from a null pointer should produce a warning.
+ // The last argument to the diagnose call says this doesn't match the
+ // GNU int-to-pointer idiom.
+ if (LHS.get()->IgnoreParenCasts()->isNullPointerConstant(Context,
+ Expr::NPC_ValueDependentIsNull))
+ diagnoseArithmeticOnNullPointer(*this, Loc, LHS.get(), false);
+
if (!checkArithmeticOpPointerOperand(*this, Loc, LHS.get()))
return QualType();
Index: include/clang/Basic/DiagnosticSemaKinds.td
===================================================================
--- include/clang/Basic/DiagnosticSemaKinds.td
+++ include/clang/Basic/DiagnosticSemaKinds.td
@@ -5483,6 +5483,9 @@
def warn_sub_ptr_zero_size_types : Warning<
"subtraction of pointers to type %0 of zero size has undefined behavior">,
InGroup<PointerArith>;
+def warn_pointer_arith_null_ptr : Warning<
+ "performing pointer arithmetic on a null pointer has undefined behavior">,
+ InGroup<NullPointerArithmetic>, DefaultIgnore;
def warn_floatingpoint_eq : Warning<
"comparing floating point with == or != is unsafe">,
@@ -6025,6 +6028,9 @@
"arithmetic on%select{ a|}0 pointer%select{|s}0 to%select{ the|}2 function "
"type%select{|s}2 %1%select{| and %3}2 is a GNU extension">,
InGroup<PointerArith>;
+def ext_gnu_null_ptr_arith : Extension<
+ "inttoptr casting using arithmetic on a null pointer is a GNU extension">,
+ InGroup<PointerArith>;
def err_readonly_message_assignment : Error<
"assigning to 'readonly' return result of an Objective-C message not allowed">;
def ext_integer_increment_complex : Extension<
Index: include/clang/Basic/DiagnosticGroups.td
===================================================================
--- include/clang/Basic/DiagnosticGroups.td
+++ include/clang/Basic/DiagnosticGroups.td
@@ -323,6 +323,7 @@
def ClassVarargs : DiagGroup<"class-varargs", [NonPODVarargs]>;
def : DiagGroup<"nonportable-cfstrings">;
def NonVirtualDtor : DiagGroup<"non-virtual-dtor">;
+def NullPointerArithmetic : DiagGroup<"null-pointer-arithmetic">;
def : DiagGroup<"effc++", [NonVirtualDtor]>;
def OveralignedType : DiagGroup<"over-aligned">;
def AlignedAllocationUnavailable : DiagGroup<"aligned-allocation-unavailable">;
@@ -689,7 +690,8 @@
SemiBeforeMethodBody,
MissingMethodReturnType,
SignCompare,
- UnusedParameter
+ UnusedParameter,
+ NullPointerArithmetic
]>;
def Most : DiagGroup<"most", [
Index: test/CodeGen/nullptr-arithmetic.c
===================================================================
--- test/CodeGen/nullptr-arithmetic.c
+++ test/CodeGen/nullptr-arithmetic.c
@@ -0,0 +1,50 @@
+// RUN: %clang_cc1 -S %s -emit-llvm -o - | FileCheck %s
+
+#include <stdint.h>
+
+// This test is meant to verify code that handles the 'p = nullptr + n' idiom
+// used by some versions of glibc and gcc. This is undefined behavior but
+// it is intended there to act like a conversion from a pointer-sized integer
+// to a pointer, and we would like to tolerate that.
+
+#define NULLPTRI8 ((int8_t*)0)
+
+// This should get the inttoptr instruction.
+int8_t *test1(intptr_t n) {
+ return NULLPTRI8 + n;
+}
+// CHECK-LABEL: test1
+// CHECK: inttoptr
+// CHECK-NOT: getelementptr
+
+// This doesn't meet the idiom because the offset type isn't pointer-sized.
+int8_t *test2(int16_t n) {
+ return NULLPTRI8 + n;
+}
+// CHECK-LABEL: test2
+// CHECK: getelementptr
+// CHECK-NOT: inttoptr
+
+// This doesn't meet the idiom because the offset is constant.
+int8_t *test3() {
+ return NULLPTRI8 + 16;
+}
+// CHECK-LABEL: test3
+// CHECK: getelementptr
+// CHECK-NOT: inttoptr
+
+// This doesn't meet the idiom because the element type is larger than a byte.
+int16_t *test4(intptr_t n) {
+ return (int16_t*)0 + n;
+}
+// CHECK-LABEL: test4
+// CHECK: getelementptr
+// CHECK-NOT: inttoptr
+
+// This doesn't meet the idiom because the offset is subtracted.
+int8_t* test5(intptr_t n) {
+ return NULLPTRI8 - n;
+}
+// CHECK-LABEL: test5
+// CHECK: getelementptr
+// CHECK-NOT: inttoptr
Index: test/Sema/pointer-addition.c
===================================================================
--- test/Sema/pointer-addition.c
+++ test/Sema/pointer-addition.c
@@ -1,4 +1,4 @@
-// RUN: %clang_cc1 %s -fsyntax-only -verify -pedantic -std=c11
+// RUN: %clang_cc1 %s -fsyntax-only -verify -pedantic -Wextra -std=c11
typedef struct S S; // expected-note 4 {{forward declaration of 'struct S'}}
extern _Atomic(S*) e;
@@ -20,4 +20,12 @@
d -= 1; // expected-warning {{arithmetic on a pointer to the function type 'void (S *, void *)' (aka 'void (struct S *, void *)') is a GNU extension}}
(void)(1 + d); // expected-warning {{arithmetic on a pointer to the function type 'void (S *, void *)' (aka 'void (struct S *, void *)') is a GNU extension}}
e++; // expected-error {{arithmetic on a pointer to an incomplete type}}
+ long i = (long)b;
+ char *f = (char*)0 + i; // expected-warning {{inttoptr casting using arithmetic on a null pointer is a GNU extension}}
+ // Cases that don't match the GNU inttoptr idiom get a different warning.
+ f = (char*)0 - i; // expected-warning {{performing pointer arithmetic on a null pointer has undefined behavior}}
+ f = (char*)0 + (long)0x100000; // expected-warning {{performing pointer arithmetic on a null pointer has undefined behavior}}
+ int *g = (int*)0 + i; // expected-warning {{performing pointer arithmetic on a null pointer has undefined behavior}}
+ unsigned char j = (unsigned char)b;
+ f = (char*)0 + j; // expected-warning {{performing pointer arithmetic on a null pointer has undefined behavior}}
}
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
