[Clang][CodeGen] Reuse isNullPointerArithmeticExtension

Author: dtcxzyw

clang/include/clang/AST/ASTContext.h

@@ -3238,6 +3238,8 @@ class ASTContext : public RefCountedBase<ASTContext> {
   }
 
   bool isSentinelNullExpr(const Expr *E);
+  /// Check whether the underlying base pointer is a constant null.
+  bool isUnderlyingBasePointerConstantNull(const Expr *E);
 
   /// Get the implementation of the ObjCInterfaceDecl \p D, or nullptr if
   /// none exists.

clang/lib/AST/ASTContext.cpp

@@ -3000,6 +3000,13 @@ bool ASTContext::isSentinelNullExpr(const Expr *E) {
   return false;
 }
 
+bool ASTContext::isUnderlyingBasePointerConstantNull(const Expr *E) {
+  const Expr *UnderlyingBaseExpr = E->IgnoreParens();
+  while (auto *BaseMemberExpr = dyn_cast<MemberExpr>(UnderlyingBaseExpr))
+    UnderlyingBaseExpr = BaseMemberExpr->getBase()->IgnoreParens();
+  return isSentinelNullExpr(UnderlyingBaseExpr);
+}
+
 /// Get the implementation of ObjCInterfaceDecl, or nullptr if none
 /// exists.
 ObjCImplementationDecl *ASTContext::getObjCImplementation(ObjCInterfaceDecl *D) {

clang/lib/AST/Expr.cpp

@@ -2248,8 +2248,7 @@ bool BinaryOperator::isNullPointerArithmeticExtension(ASTContext &Ctx,
   }
 
   // Check that the pointer is a nullptr.
-  if (!PExp->IgnoreParenCasts()
-          ->isNullPointerConstant(Ctx, Expr::NPC_ValueDependentIsNotNull))
+  if (!Ctx.isUnderlyingBasePointerConstantNull(PExp))
     return false;
 
   // Check that the pointee type is char-sized.

clang/lib/CodeGen/CGExpr.cpp

@@ -4816,13 +4816,6 @@ EmitExtVectorElementExpr(const ExtVectorElementExpr *E) {
                                   Base.getBaseInfo(), TBAAAccessInfo());
 }
 
-bool CodeGenFunction::isUnderlyingBasePointerConstantNull(const Expr *E) {
-  const Expr *UnderlyingBaseExpr = E->IgnoreParens();
-  while (auto *BaseMemberExpr = dyn_cast<MemberExpr>(UnderlyingBaseExpr))
-    UnderlyingBaseExpr = BaseMemberExpr->getBase()->IgnoreParens();
-  return getContext().isSentinelNullExpr(UnderlyingBaseExpr);
-}
-
 LValue CodeGenFunction::EmitMemberExpr(const MemberExpr *E) {
   if (DeclRefExpr *DRE = tryToConvertMemberExprToDeclRefExpr(*this, E)) {
     EmitIgnoredExpr(E->getBase());
@@ -4834,7 +4827,7 @@ LValue CodeGenFunction::EmitMemberExpr(const MemberExpr *E) {
   // If so, we do not set inbounds flag for GEP to avoid breaking some
   // old-style offsetof idioms.
   bool IsInBounds = !getLangOpts().PointerOverflowDefined &&
-                    !isUnderlyingBasePointerConstantNull(BaseExpr);
+                    !getContext().isUnderlyingBasePointerConstantNull(BaseExpr);
   // If this is s.x, emit s as an lvalue.  If it is s->x, emit s as a scalar.
   LValue BaseLV;
   if (E->isArrow()) {
@@ -6373,8 +6366,9 @@ EmitPointerToDataMemberBinaryExpr(const BinaryOperator *E) {
 
   LValueBaseInfo BaseInfo;
   TBAAAccessInfo TBAAInfo;
-  bool IsInBounds = !getLangOpts().PointerOverflowDefined &&
-                    !isUnderlyingBasePointerConstantNull(E->getLHS());
+  bool IsInBounds =
+      !getLangOpts().PointerOverflowDefined &&
+      !getContext().isUnderlyingBasePointerConstantNull(E->getLHS());
   Address MemberAddr = EmitCXXMemberDataPointerAddress(
       E, BaseAddr, OffsetV, MPT, IsInBounds, &BaseInfo, &TBAAInfo);
 

clang/lib/CodeGen/CGExprScalar.cpp

@@ -4169,11 +4169,16 @@ static Value *emitPointerArithmetic(CodeGenFunction &CGF,
   //   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());
+  // Note that we do not suppress the pointer overflow check in this case.
+  if (!CGF.SanOpts.has(SanitizerKind::PointerOverflow) &&
+      BinaryOperator::isNullPointerArithmeticExtension(
+          CGF.getContext(), op.Opcode, expr->getLHS(), expr->getRHS())) {
+    // isUnderlyingBasePointerConstantNull returns true does not indicate that
+    // the base pointer is null.
+    Value *Base = CGF.Builder.CreatePtrToInt(pointer, index->getType());
+    Value *PtrAdd = CGF.Builder.CreateAdd(Base, index, "ptr.add");
+    return CGF.Builder.CreateIntToPtr(PtrAdd, pointer->getType());
+  }
 
   if (width != DL.getIndexTypeSizeInBits(PtrTy)) {
     // Zero-extend or sign-extend the pointer value according to
@@ -4238,16 +4243,7 @@ static Value *emitPointerArithmetic(CodeGenFunction &CGF,
   else
     elemTy = CGF.ConvertTypeForMem(elementType);
 
-  // Some versions of glibc __PTR_ALIGN macro use idioms that add an index to a
-  // null pointer in order to cast the aligned integer value back to a pointer.
-  // This is undefined behavior, but we have to add a workaround for it.
-  //
-  // Unlike the above check `BinaryOperator::isNullPointerArithmeticExtension`,
-  // we still generate a GEP with a null-pointer base instead of inttoptr.
-  // This means that it's also UB to dereference a pointer created that way.
-  if (CGF.getLangOpts().PointerOverflowDefined ||
-      (!CGF.SanOpts.has(SanitizerKind::PointerOverflow) &&
-       CGF.isUnderlyingBasePointerConstantNull(pointerOperand)))
+  if (CGF.getLangOpts().PointerOverflowDefined)
     return CGF.Builder.CreateGEP(elemTy, pointer, index, "add.ptr");
 
   return CGF.EmitCheckedInBoundsGEP(

clang/lib/CodeGen/CodeGenFunction.h

@@ -4559,8 +4559,6 @@ class CodeGenFunction : public CodeGenTypeCache {
                                                const CXXRecordDecl *RD);
 
   bool isPointerKnownNonNull(const Expr *E);
-  /// Check whether the underlying base pointer is a constant null.
-  bool isUnderlyingBasePointerConstantNull(const Expr *E);
 
   /// Create the discriminator from the storage address and the entity hash.
   llvm::Value *EmitPointerAuthBlendDiscriminator(llvm::Value *StorageAddress,

clang/test/CodeGen/catch-nullptr-and-nonzero-offset.c

@@ -32,7 +32,7 @@
 // CHECK-SANITIZE-ANYRECOVER-DAG: @[[LINE_1500:.*]] = {{.*}}, i32 1500, i32 15 } }
 // CHECK-SANITIZE-ANYRECOVER-DAG: @[[LINE_1600:.*]] = {{.*}}, i32 1600, i32 15 } }
 // CHECK-SANITIZE-ANYRECOVER-DAG: @[[LINE_1700:.*]] = {{.*}}, i32 1700, i32 15 } }
-// CHECK-SANITIZE-ANYRECOVER-DAG: @[[LINE_1800:.*]] = {{.*}}, i32 1800, i32 19 } }
+// CHECK-SANITIZE-ANYRECOVER-DAG: @[[LINE_1800:.*]] = {{.*}}, i32 1800, i32 20 } }
 
 #ifdef __cplusplus
 extern "C" {
@@ -432,15 +432,16 @@ char *void_ptr(void *base, unsigned long offset) {
   return base + offset;
 }
 
-int *constant_null_add(unsigned long offset) {
+char *constant_null_add(unsigned long offset) {
   // CHECK: define{{.*}} ptr @constant_null_add(i64 noundef %[[OFFSET:.*]])
   // CHECK-NEXT:                      [[ENTRY:.*]]:
   // CHECK-NEXT:                        %[[OFFSET_ADDR:.*]] = alloca i64, align 8
   // CHECK-NEXT:                        store i64 %[[OFFSET]], ptr %[[OFFSET_ADDR]], align 8
   // CHECK-NEXT:                        %[[OFFSET_RELOADED:.*]] = load i64, ptr %[[OFFSET_ADDR]], align 8
-  // CHECK-NOSANITIZE-NEXT:             %[[ADD_PTR:.*]] = getelementptr i32, ptr null, i64 %[[OFFSET_RELOADED]]
-  // CHECK-SANITIZE-NEXT:               %[[ADD_PTR:.*]] = getelementptr inbounds nuw i32, ptr null, i64 %[[OFFSET_RELOADED]]
-  // CHECK-SANITIZE-NEXT:               %[[COMPUTED_OFFSET_AGGREGATE:.*]] = call { i64, i1 } @llvm.smul.with.overflow.i64(i64 4, i64 %[[OFFSET_RELOADED]]), !nosanitize
+  // CHECK-NOSANITIZE-NEXT:             %[[PTR_ADD:.*]] = add i64 0, %[[OFFSET_RELOADED]]
+  // CHECK-NOSANITIZE-NEXT:             %[[ADD_PTR:.*]] = inttoptr i64 %[[PTR_ADD]] to ptr
+  // CHECK-SANITIZE-NEXT:               %[[ADD_PTR:.*]] = getelementptr inbounds nuw i8, ptr null, i64 %[[OFFSET_RELOADED]]
+  // CHECK-SANITIZE-NEXT:               %[[COMPUTED_OFFSET_AGGREGATE:.*]] = call { i64, i1 } @llvm.smul.with.overflow.i64(i64 1, i64 %[[OFFSET_RELOADED]]), !nosanitize
   // CHECK-SANITIZE-NEXT:               %[[COMPUTED_OFFSET_OVERFLOWED:.*]] = extractvalue { i64, i1 } %[[COMPUTED_OFFSET_AGGREGATE]], 1, !nosanitize
   // CHECK-SANITIZE-NEXT:               %[[OR_OV:.+]] = or i1 %[[COMPUTED_OFFSET_OVERFLOWED]], false, !nosanitize
   // CHECK-SANITIZE-NEXT:               %[[COMPUTED_OFFSET:.*]] = extractvalue { i64, i1 } %[[COMPUTED_OFFSET_AGGREGATE]], 0, !nosanitize
@@ -460,7 +461,7 @@ int *constant_null_add(unsigned long offset) {
   // CHECK-SANITIZE:                  [[CONT]]:
   // CHECK-NEXT:                        ret ptr %[[ADD_PTR]]
 #line 1800
-  return (int *)0 + offset;
+  return (char *)0 + offset;
 }
 
 #ifdef __cplusplus