Author: akaylor Date: Tue Sep 19 13:26:40 2017 New Revision: 313666 URL: http://llvm.org/viewvc/llvm-project?rev=313666&view=rev Log: Teach clang to tolerate the 'p = nullptr + n' idiom used by glibc
Differential Revision: https://reviews.llvm.org/D37042 Added: cfe/trunk/test/CodeGen/nullptr-arithmetic.c cfe/trunk/test/SemaCXX/nullptr-arithmetic.cpp Modified: cfe/trunk/include/clang/AST/Expr.h cfe/trunk/include/clang/Basic/DiagnosticGroups.td cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td cfe/trunk/lib/AST/Expr.cpp cfe/trunk/lib/CodeGen/CGExprScalar.cpp cfe/trunk/lib/Sema/SemaExpr.cpp cfe/trunk/test/Sema/pointer-addition.c Modified: cfe/trunk/include/clang/AST/Expr.h URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/AST/Expr.h?rev=313666&r1=313665&r2=313666&view=diff ============================================================================== --- cfe/trunk/include/clang/AST/Expr.h (original) +++ cfe/trunk/include/clang/AST/Expr.h Tue Sep 19 13:26:40 2017 @@ -3126,6 +3126,12 @@ public: return isShiftAssignOp(getOpcode()); } + // Return true if a binary operator using the specified opcode and operands + // would match the 'p = (i8*)nullptr + n' idiom for casting a pointer-sized + // integer to a pointer. + static bool isNullPointerArithmeticExtension(ASTContext &Ctx, Opcode Opc, + Expr *LHS, Expr *RHS); + static bool classof(const Stmt *S) { return S->getStmtClass() >= firstBinaryOperatorConstant && S->getStmtClass() <= lastBinaryOperatorConstant; Modified: cfe/trunk/include/clang/Basic/DiagnosticGroups.td URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/DiagnosticGroups.td?rev=313666&r1=313665&r2=313666&view=diff ============================================================================== --- cfe/trunk/include/clang/Basic/DiagnosticGroups.td (original) +++ cfe/trunk/include/clang/Basic/DiagnosticGroups.td Tue Sep 19 13:26:40 2017 @@ -338,6 +338,7 @@ def NonPODVarargs : DiagGroup<"non-pod-v 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">; @@ -706,7 +707,8 @@ def Extra : DiagGroup<"extra", [ SemiBeforeMethodBody, MissingMethodReturnType, SignCompare, - UnusedParameter + UnusedParameter, + NullPointerArithmetic ]>; def Most : DiagGroup<"most", [ Modified: cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td?rev=313666&r1=313665&r2=313666&view=diff ============================================================================== --- cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td (original) +++ cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td Tue Sep 19 13:26:40 2017 @@ -5501,6 +5501,12 @@ def err_offsetof_field_of_virtual_base : 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%select{| if the offset is nonzero}0">, + InGroup<NullPointerArithmetic>, DefaultIgnore; +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_floatingpoint_eq : Warning< "comparing floating point with == or != is unsafe">, Modified: cfe/trunk/lib/AST/Expr.cpp URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/AST/Expr.cpp?rev=313666&r1=313665&r2=313666&view=diff ============================================================================== --- cfe/trunk/lib/AST/Expr.cpp (original) +++ cfe/trunk/lib/AST/Expr.cpp Tue Sep 19 13:26:40 2017 @@ -1829,6 +1829,45 @@ OverloadedOperatorKind BinaryOperator::g return OverOps[Opc]; } +bool BinaryOperator::isNullPointerArithmeticExtension(ASTContext &Ctx, + Opcode Opc, + Expr *LHS, Expr *RHS) { + if (Opc != BO_Add) + return false; + + // Check that we have one pointer and one integer operand. + Expr *PExp; + Expr *IExp; + if (LHS->getType()->isPointerType()) { + if (!RHS->getType()->isIntegerType()) + return false; + PExp = LHS; + IExp = RHS; + } else if (RHS->getType()->isPointerType()) { + if (!LHS->getType()->isIntegerType()) + return false; + PExp = RHS; + IExp = LHS; + } else { + return false; + } + + // Check that the pointer is a nullptr. + if (!PExp->IgnoreParenCasts() + ->isNullPointerConstant(Ctx, Expr::NPC_ValueDependentIsNotNull)) + return false; + + // Check that the pointee type is char-sized. + const PointerType *PTy = PExp->getType()->getAs<PointerType>(); + if (!PTy || !PTy->getPointeeType()->isCharType()) + return false; + + // Check that the integer type is pointer-sized. + if (Ctx.getTypeSize(IExp->getType()) != Ctx.getTypeSize(PExp->getType())) + return false; + + return true; +} InitListExpr::InitListExpr(const ASTContext &C, SourceLocation lbraceloc, ArrayRef<Expr*> initExprs, SourceLocation rbraceloc) : Expr(InitListExprClass, QualType(), VK_RValue, OK_Ordinary, false, false, Modified: cfe/trunk/lib/CodeGen/CGExprScalar.cpp URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/CGExprScalar.cpp?rev=313666&r1=313665&r2=313666&view=diff ============================================================================== --- cfe/trunk/lib/CodeGen/CGExprScalar.cpp (original) +++ cfe/trunk/lib/CodeGen/CGExprScalar.cpp Tue Sep 19 13:26:40 2017 @@ -2678,6 +2678,30 @@ static Value *emitPointerArithmetic(Code 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. + // + if (BinaryOperator::isNullPointerArithmeticExtension(CGF.getContext(), + op.Opcode, + expr->getLHS(), + expr->getRHS())) + 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. Modified: cfe/trunk/lib/Sema/SemaExpr.cpp URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaExpr.cpp?rev=313666&r1=313665&r2=313666&view=diff ============================================================================== --- cfe/trunk/lib/Sema/SemaExpr.cpp (original) +++ cfe/trunk/lib/Sema/SemaExpr.cpp Tue Sep 19 13:26:40 2017 @@ -8572,6 +8572,21 @@ static void diagnoseArithmeticOnVoidPoin << 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) { + if (IsGNUIdiom) + S.Diag(Loc, diag::warn_gnu_null_ptr_arith) + << Pointer->getSourceRange(); + else + S.Diag(Loc, diag::warn_pointer_arith_null_ptr) + << S.getLangOpts().CPlusPlus << Pointer->getSourceRange(); +} + /// \brief Diagnose invalid arithmetic on two function pointers. static void diagnoseArithmeticOnTwoFunctionPointers(Sema &S, SourceLocation Loc, Expr *LHS, Expr *RHS) { @@ -8866,6 +8881,21 @@ QualType Sema::CheckAdditionOperands(Exp if (!IExp->getType()->isIntegerType()) return InvalidOperands(Loc, LHS, RHS); + // Adding to a null pointer results in undefined behavior. + if (PExp->IgnoreParenCasts()->isNullPointerConstant( + Context, Expr::NPC_ValueDependentIsNotNull)) { + // In C++ adding zero to a null pointer is defined. + llvm::APSInt KnownVal; + if (!getLangOpts().CPlusPlus || + (!IExp->isValueDependent() && + (!IExp->EvaluateAsInt(KnownVal, Context) || KnownVal != 0))) { + // Check the conditions to see if this is the 'p = nullptr + n' idiom. + bool IsGNUIdiom = BinaryOperator::isNullPointerArithmeticExtension( + Context, BO_Add, PExp, IExp); + diagnoseArithmeticOnNullPointer(*this, Loc, PExp, IsGNUIdiom); + } + } + if (!checkArithmeticOpPointerOperand(*this, Loc, PExp)) return QualType(); @@ -8927,6 +8957,20 @@ QualType Sema::CheckSubtractionOperands( // 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_ValueDependentIsNotNull)) { + // In C++ adding zero to a null pointer is defined. + llvm::APSInt KnownVal; + if (!getLangOpts().CPlusPlus || + (!RHS.get()->isValueDependent() && + (!RHS.get()->EvaluateAsInt(KnownVal, Context) || KnownVal != 0))) { + diagnoseArithmeticOnNullPointer(*this, Loc, LHS.get(), false); + } + } + if (!checkArithmeticOpPointerOperand(*this, Loc, LHS.get())) return QualType(); @@ -8962,6 +9006,8 @@ QualType Sema::CheckSubtractionOperands( LHS.get(), RHS.get())) return QualType(); + // FIXME: Add warnings for nullptr - ptr. + // 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 // case subtraction does not make sense. Added: cfe/trunk/test/CodeGen/nullptr-arithmetic.c URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/CodeGen/nullptr-arithmetic.c?rev=313666&view=auto ============================================================================== --- cfe/trunk/test/CodeGen/nullptr-arithmetic.c (added) +++ cfe/trunk/test/CodeGen/nullptr-arithmetic.c Tue Sep 19 13:26:40 2017 @@ -0,0 +1,42 @@ +// 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(int8_t n) { + return NULLPTRI8 + n; +} +// CHECK-LABEL: test2 +// CHECK: getelementptr +// CHECK-NOT: inttoptr + +// This doesn't meet the idiom because the element type is larger than a byte. +int16_t *test3(intptr_t n) { + return (int16_t*)0 + n; +} +// CHECK-LABEL: test3 +// CHECK: getelementptr +// CHECK-NOT: inttoptr + +// This doesn't meet the idiom because the offset is subtracted. +int8_t* test4(intptr_t n) { + return NULLPTRI8 - n; +} +// CHECK-LABEL: test4 +// CHECK: getelementptr +// CHECK-NOT: inttoptr Modified: cfe/trunk/test/Sema/pointer-addition.c URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Sema/pointer-addition.c?rev=313666&r1=313665&r2=313666&view=diff ============================================================================== --- cfe/trunk/test/Sema/pointer-addition.c (original) +++ cfe/trunk/test/Sema/pointer-addition.c Tue Sep 19 13:26:40 2017 @@ -1,4 +1,6 @@ -// RUN: %clang_cc1 %s -fsyntax-only -verify -pedantic -std=c11 +// RUN: %clang_cc1 %s -fsyntax-only -verify -pedantic -Wextra -std=c11 + +#include <stdint.h> typedef struct S S; // expected-note 4 {{forward declaration of 'struct S'}} extern _Atomic(S*) e; @@ -20,4 +22,11 @@ void a(S* b, void* c) { 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}} + intptr_t i = (intptr_t)b; + char *f = (char*)0 + i; // expected-warning {{arithmetic on a null pointer treated as a cast from integer to 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}} + 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}} } Added: cfe/trunk/test/SemaCXX/nullptr-arithmetic.cpp URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/SemaCXX/nullptr-arithmetic.cpp?rev=313666&view=auto ============================================================================== --- cfe/trunk/test/SemaCXX/nullptr-arithmetic.cpp (added) +++ cfe/trunk/test/SemaCXX/nullptr-arithmetic.cpp Tue Sep 19 13:26:40 2017 @@ -0,0 +1,29 @@ +// RUN: %clang_cc1 %s -fsyntax-only -verify -pedantic -Wextra -std=c++11 + +#include <stdint.h> + +void f(intptr_t offset, int8_t b) { + // A zero offset from a nullptr is OK. + char *f = (char*)nullptr + 0; + int *g = (int*)0 + 0; + f = (char*)nullptr - 0; + g = (int*)nullptr - 0; + // adding other values is undefined. + f = (char*)nullptr + offset; // expected-warning {{arithmetic on a null pointer treated as a cast from integer to pointer is a GNU extension}} + // Cases that don't match the GNU inttoptr idiom get a different warning. + f = (char*)0 - offset; // expected-warning {{performing pointer arithmetic on a null pointer has undefined behavior if the offset is nonzero}} + g = (int*)0 + offset; // expected-warning {{performing pointer arithmetic on a null pointer has undefined behavior if the offset is nonzero}} + f = (char*)0 + b; // expected-warning {{performing pointer arithmetic on a null pointer has undefined behavior if the offset is nonzero}} +} + +// Value-dependent pointer arithmetic should not produce a nullptr warning. +template<char *P> +char* g(intptr_t offset) { + return P + offset; +} + +// Value-dependent offsets should not produce a nullptr warning. +template<intptr_t N> +char *h() { + return (char*)nullptr + N; +} _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
