[Clang][CodeGen] Add memory(read) if the check value is passed by ref

Author: dtcxzyw

clang/lib/CodeGen/CGExpr.cpp

@@ -1972,7 +1972,7 @@ bool CodeGenFunction::EmitScalarRangeCheck(llvm::Value *Value, QualType Ty,
   SanitizerKind::SanitizerOrdinal Kind =
       NeedsEnumCheck ? SanitizerKind::SO_Enum : SanitizerKind::SO_Bool;
   EmitCheck(std::make_pair(Check, Kind), SanitizerHandler::LoadInvalidValue,
-            StaticArgs, EmitCheckValue(Value));
+            StaticArgs, Value);
   return true;
 }
 
@@ -3462,7 +3462,8 @@ llvm::Constant *CodeGenFunction::EmitCheckTypeDescriptor(QualType T) {
   return GV;
 }
 
-llvm::Value *CodeGenFunction::EmitCheckValue(llvm::Value *V) {
+llvm::Value *CodeGenFunction::EmitCheckValue(llvm::Value *V,
+                                             bool &MayReadFromPtrToInt) {
   llvm::Type *TargetTy = IntPtrTy;
 
   if (V->getType() == TargetTy)
@@ -3488,6 +3489,7 @@ llvm::Value *CodeGenFunction::EmitCheckValue(llvm::Value *V) {
     Builder.CreateStore(V, Ptr);
     V = Ptr.getPointer();
   }
+  MayReadFromPtrToInt = true;
   return Builder.CreatePtrToInt(V, TargetTy);
 }
 
@@ -3593,7 +3595,8 @@ static void emitCheckHandlerCall(CodeGenFunction &CGF,
                                  ArrayRef<llvm::Value *> FnArgs,
                                  SanitizerHandler CheckHandler,
                                  CheckRecoverableKind RecoverKind, bool IsFatal,
-                                 llvm::BasicBlock *ContBB, bool NoMerge) {
+                                 llvm::BasicBlock *ContBB, bool NoMerge,
+                                 bool MayReadFromPtrToInt) {
   assert(IsFatal || RecoverKind != CheckRecoverableKind::Unrecoverable);
   std::optional<ApplyDebugLocation> DL;
   if (!CGF.Builder.getCurrentDebugLocation()) {
@@ -3626,9 +3629,14 @@ static void emitCheckHandlerCall(CodeGenFunction &CGF,
   if (CGF.CGM.getCodeGenOpts().OptimizationLevel > 0 && MayReturn) {
     // __ubsan_handle_dynamic_type_cache_miss reads the vtable, which is also
     // accessible by the current module.
-    if (CheckHandler != SanitizerHandler::DynamicTypeCacheMiss)
-      B.addMemoryAttr(llvm::MemoryEffects::argMemOnly(llvm::ModRefInfo::Ref) |
-                      llvm::MemoryEffects::inaccessibleMemOnly());
+    if (CheckHandler != SanitizerHandler::DynamicTypeCacheMiss) {
+      llvm::MemoryEffects ME =
+          llvm::MemoryEffects::argMemOnly(llvm::ModRefInfo::Ref) |
+          llvm::MemoryEffects::inaccessibleMemOnly();
+      if (MayReadFromPtrToInt)
+        ME |= llvm::MemoryEffects::readOnly();
+      B.addMemoryAttr(ME);
+    }
     // If the handler does not return, it must interact with the environment in
     // an observable way.
     B.addAttribute(llvm::Attribute::MustProgress);
@@ -3729,6 +3737,7 @@ void CodeGenFunction::EmitCheck(
   // representing operand values.
   SmallVector<llvm::Value *, 4> Args;
   SmallVector<llvm::Type *, 4> ArgTypes;
+  bool MayReadFromPtrToInt = false;
   if (!CGM.getCodeGenOpts().SanitizeMinimalRuntime) {
     Args.reserve(DynamicArgs.size() + 1);
     ArgTypes.reserve(DynamicArgs.size() + 1);
@@ -3748,7 +3757,7 @@ void CodeGenFunction::EmitCheck(
     }
 
     for (size_t i = 0, n = DynamicArgs.size(); i != n; ++i) {
-      Args.push_back(EmitCheckValue(DynamicArgs[i]));
+      Args.push_back(EmitCheckValue(DynamicArgs[i], MayReadFromPtrToInt));
       ArgTypes.push_back(IntPtrTy);
     }
   }
@@ -3760,7 +3769,8 @@ void CodeGenFunction::EmitCheck(
     // Simple case: we need to generate a single handler call, either
     // fatal, or non-fatal.
     emitCheckHandlerCall(*this, FnType, Args, CheckHandler, RecoverKind,
-                         (FatalCond != nullptr), Cont, NoMerge);
+                         (FatalCond != nullptr), Cont, NoMerge,
+                         MayReadFromPtrToInt);
   } else {
     // Emit two handler calls: first one for set of unrecoverable checks,
     // another one for recoverable.
@@ -3770,10 +3780,10 @@ void CodeGenFunction::EmitCheck(
     Builder.CreateCondBr(FatalCond, NonFatalHandlerBB, FatalHandlerBB);
     EmitBlock(FatalHandlerBB);
     emitCheckHandlerCall(*this, FnType, Args, CheckHandler, RecoverKind, true,
-                         NonFatalHandlerBB, NoMerge);
+                         NonFatalHandlerBB, NoMerge, MayReadFromPtrToInt);
     EmitBlock(NonFatalHandlerBB);
     emitCheckHandlerCall(*this, FnType, Args, CheckHandler, RecoverKind, false,
-                         Cont, NoMerge);
+                         Cont, NoMerge, MayReadFromPtrToInt);
   }
 
   EmitBlock(Cont);

clang/lib/CodeGen/CodeGenFunction.cpp

@@ -3197,9 +3197,7 @@ void CodeGenFunction::emitAlignmentAssumptionCheck(
     llvm::Constant *StaticData[] = {EmitCheckSourceLocation(Loc),
                                     EmitCheckSourceLocation(SecondaryLoc),
                                     EmitCheckTypeDescriptor(Ty)};
-    llvm::Value *DynamicData[] = {EmitCheckValue(Ptr),
-                                  EmitCheckValue(Alignment),
-                                  EmitCheckValue(OffsetValue)};
+    llvm::Value *DynamicData[] = {Ptr, Alignment, OffsetValue};
     EmitCheck({std::make_pair(TheCheck, SanitizerKind::SO_Alignment)},
               SanitizerHandler::AlignmentAssumption, StaticData, DynamicData);
   }

clang/lib/CodeGen/CodeGenFunction.h

@@ -5219,7 +5219,9 @@ class CodeGenFunction : public CodeGenTypeCache {
 
   /// Convert a value into a format suitable for passing to a runtime
   /// sanitizer handler.
-  llvm::Value *EmitCheckValue(llvm::Value *V);
+  /// If the check value is a pointer or passed by reference, set \p
+  /// MayReadFromPtrToInt to true.
+  llvm::Value *EmitCheckValue(llvm::Value *V, bool &MayReadFromPtrToInt);
 
   /// Emit a description of a source location in a format suitable for
   /// passing to a runtime sanitizer handler.

clang/test/CodeGen/AArch64/ubsan-handler-pass-by-ref.c

@@ -8,7 +8,8 @@
 // RECOVER-SAME: ) local_unnamed_addr #[[ATTR0:[0-9]+]] {
 // RECOVER-NEXT:  [[ENTRY:.*:]]
 // RECOVER-NEXT:    [[TMP:%.*]] = alloca i64, align 8
-// RECOVER-NEXT:    [[TMP0:%.*]] = ptrtoint ptr [[TMP]] to i32, !nosanitize [[META3:![0-9]+]]
+// RECOVER-NEXT:    store i64 -9223372036854775808, ptr [[TMP]], align 8, !nosanitize [[META3:![0-9]+]]
+// RECOVER-NEXT:    [[TMP0:%.*]] = ptrtoint ptr [[TMP]] to i32, !nosanitize [[META3]]
 // RECOVER-NEXT:    call void @__ubsan_handle_negate_overflow(ptr nonnull @[[GLOB1:[0-9]+]], i32 [[TMP0]]) #[[ATTR2:[0-9]+]], !nosanitize [[META3]]
 // RECOVER-NEXT:    ret i32 0
 //