jfb created this revision.
Herald added subscribers: cfe-commits, ributzka, dexonsmith, jkorous,
JDevlieghere.
Herald added a project: clang.
These builtins are often used (or should be used) in places where time-of-check
time-of-use security issues are important (e.g. copying from untrusted buffers).
They don't accept volatile pointee parameters in C++, and merely warn about such
parameters in C, which leads to confusion. In these settings, it's useful to
overload the builtin and permit volatile pointee parameters. The code generation
then directly emits the existing volatile variant of the mem* builtin function
call, which ensures that the affected memory location is only accessed once
(thereby preventing double-reads under an adversatial memory mapping).
Repository:
rG LLVM Github Monorepo
https://reviews.llvm.org/D79279
Files:
clang/include/clang/Basic/Builtins.def
clang/lib/CodeGen/CGBuiltin.cpp
clang/test/CodeGen/builtin-memfns.c
clang/test/CodeGen/builtin-volatile-memfns.c
Index: clang/test/CodeGen/builtin-volatile-memfns.c
===================================================================
--- /dev/null
+++ clang/test/CodeGen/builtin-volatile-memfns.c
@@ -0,0 +1,68 @@
+// RUN: %clang_cc1 -triple arm64-unknown-unknown -emit-llvm < %s| FileCheck %s
+
+typedef __SIZE_TYPE__ size_t;
+
+// Regular char* parameters
+
+// CHECK-LABEL: dst_cpy(
+// CHECK: call void @llvm.memcpy.p0i8.p0i8.i64({{.*}}, i1 true)
+void dst_cpy(volatile char *dst, const char *src, size_t size) { __builtin_memcpy(dst, src, size); }
+
+// CHECK-LABEL: src_cpy(
+// CHECK: call void @llvm.memcpy.p0i8.p0i8.i64({{.*}}, i1 true)
+void src_cpy(char *dst, volatile const char *src, size_t size) { __builtin_memcpy(dst, src, size); }
+
+// CHECK-LABEL: dstsrc_cpy(
+// CHECK: call void @llvm.memcpy.p0i8.p0i8.i64({{.*}}, i1 true)
+void dstsrc_cpy(volatile char *dst, volatile const char *src, size_t size) { __builtin_memcpy(dst, src, size); }
+
+// CHECK-LABEL: dst_move(
+// CHECK: call void @llvm.memmove.p0i8.p0i8.i64({{.*}}, i1 true)
+void dst_move(volatile char *dst, const char *src, size_t size) { __builtin_memmove(dst, src, size); }
+
+// CHECK-LABEL: src_move(
+// CHECK: call void @llvm.memmove.p0i8.p0i8.i64({{.*}}, i1 true)
+void src_move(char *dst, volatile const char *src, size_t size) { __builtin_memmove(dst, src, size); }
+
+// CHECK-LABEL: dstsrc_move(
+// CHECK: call void @llvm.memmove.p0i8.p0i8.i64({{.*}}, i1 true)
+void dstsrc_move(volatile char *dst, volatile const char *src, size_t size) { __builtin_memmove(dst, src, size); }
+
+// CHECK-LABEL: set(
+// CHECK: call void @llvm.memset.p0i8.i64({{.*}}, i1 true)
+void set(volatile char *dst, char value, size_t size) { __builtin_memset(dst, value, size); }
+
+// Arrays
+
+extern const char asrc[512];
+extern const volatile char avsrc[512];
+extern char adst[512];
+extern volatile char avdst[512];
+
+// CHECK-LABEL: array_dst_cpy(
+// CHECK: call void @llvm.memcpy.p0i8.p0i8.i64({{.*}}, i1 true)
+void array_dst_cpy(size_t size) { __builtin_memcpy(avdst, asrc, size); }
+
+// CHECK-LABEL: array_src_cpy(
+// CHECK: call void @llvm.memcpy.p0i8.p0i8.i64({{.*}}, i1 true)
+void array_src_cpy(size_t size) { __builtin_memcpy(adst, avsrc, size); }
+
+// CHECK-LABEL: array_dstsrc_cpy(
+// CHECK: call void @llvm.memcpy.p0i8.p0i8.i64({{.*}}, i1 true)
+void array_dstsrc_cpy(size_t size) { __builtin_memcpy(avdst, avsrc, size); }
+
+// CHECK-LABEL: array_dst_move(
+// CHECK: call void @llvm.memmove.p0i8.p0i8.i64({{.*}}, i1 true)
+void array_dst_move(size_t size) { __builtin_memmove(avdst, asrc, size); }
+
+// CHECK-LABEL: array_src_move(
+// CHECK: call void @llvm.memmove.p0i8.p0i8.i64({{.*}}, i1 true)
+void array_src_move(size_t size) { __builtin_memmove(adst, avsrc, size); }
+
+// CHECK-LABEL: array_dstsrc_move(
+// CHECK: call void @llvm.memmove.p0i8.p0i8.i64({{.*}}, i1 true)
+void array_dstsrc_move(size_t size) { __builtin_memmove(avdst, avsrc, size); }
+
+// CHECK-LABEL: array_set(
+// CHECK: call void @llvm.memset.p0i8.i64({{.*}}, i1 true)
+void array_set(char value, size_t size) { __builtin_memset(avdst, value, size); }
Index: clang/test/CodeGen/builtin-memfns.c
===================================================================
--- clang/test/CodeGen/builtin-memfns.c
+++ clang/test/CodeGen/builtin-memfns.c
@@ -7,10 +7,10 @@
void *memccpy(void *, void const *, int, size_t);
// CHECK: @test1
-// CHECK: call void @llvm.memset.p0i8.i32
-// CHECK: call void @llvm.memset.p0i8.i32
-// CHECK: call void @llvm.memcpy.p0i8.p0i8.i32
-// CHECK: call void @llvm.memmove.p0i8.p0i8.i32
+// CHECK: call void @llvm.memset.p0i8.i32({{.*}}, i1 false)
+// CHECK: call void @llvm.memset.p0i8.i32({{.*}}, i1 false)
+// CHECK: call void @llvm.memcpy.p0i8.p0i8.i32({{.*}}, i1 false)
+// CHECK: call void @llvm.memmove.p0i8.p0i8.i32({{.*}}, i1 false)
// CHECK-NOT: __builtin
// CHECK: ret
int test1(int argc, char **argv) {
@@ -26,31 +26,31 @@
// rdar://9289468
// CHECK: @test2
-// CHECK: call void @llvm.memcpy.p0i8.p0i8.i32
+// CHECK: call void @llvm.memcpy.p0i8.p0i8.i32({{.*}}, i1 false)
char* test2(char* a, char* b) {
return __builtin_memcpy(a, b, 4);
}
// CHECK: @test3
-// CHECK: call void @llvm.memset
+// CHECK: call void @llvm.memset{{.*}}, i1 false)
void test3(char *P) {
__builtin___memset_chk(P, 42, 128, 128);
}
// CHECK: @test4
-// CHECK: call void @llvm.memcpy
+// CHECK: call void @llvm.memcpy{{.*}}, i1 false)
void test4(char *P, char *Q) {
__builtin___memcpy_chk(P, Q, 128, 128);
}
// CHECK: @test5
-// CHECK: call void @llvm.memmove
+// CHECK: call void @llvm.memmove{{.*}}, i1 false)
void test5(char *P, char *Q) {
__builtin___memmove_chk(P, Q, 128, 128);
}
// CHECK: @test6
-// CHECK: call void @llvm.memcpy
+// CHECK: call void @llvm.memcpy{{.*}}, i1 false)
int test6(char *X) {
return __builtin___memcpy_chk(X, X, 42, 42) != 0;
}
@@ -108,7 +108,7 @@
typedef struct { int a; } b;
int d;
b e;
- // CHECK: call void @llvm.memcpy{{.*}}(
+ // CHECK: call void @llvm.memcpy{{.*}}, i1 false)
memcpy(&d, (char *)&e.a, sizeof(e));
}
@@ -116,7 +116,7 @@
extern char dest_array[];
extern char src_array[];
void test12() {
- // CHECK: call void @llvm.memcpy{{.*}}(
+ // CHECK: call void @llvm.memcpy{{.*}}, i1 false)
memcpy(&dest_array, &dest_array, 2);
}
Index: clang/lib/CodeGen/CGBuiltin.cpp
===================================================================
--- clang/lib/CodeGen/CGBuiltin.cpp
+++ clang/lib/CodeGen/CGBuiltin.cpp
@@ -619,6 +619,19 @@
return {Width, Signed};
}
+static bool isPtrArgVolatile(CodeGenModule &CGM, const CallExpr *E,
+ unsigned ArgNo) {
+ QualType ArgTy = E->getArg(ArgNo)->IgnoreImpCasts()->getType();
+ if (ArgTy->isArrayType())
+ return CGM.getContext()
+ .getAsArrayType(ArgTy)
+ ->getElementType()
+ .isVolatileQualified();
+ return ArgTy->castAs<clang::PointerType>()
+ ->getPointeeType()
+ .isVolatileQualified();
+}
+
Value *CodeGenFunction::EmitVAStartEnd(Value *ArgValue, bool IsStart) {
llvm::Type *DestType = Int8PtrTy;
if (ArgValue->getType() != DestType)
@@ -2529,12 +2542,14 @@
case Builtin::BI__builtin_mempcpy: {
Address Dest = EmitPointerWithAlignment(E->getArg(0));
Address Src = EmitPointerWithAlignment(E->getArg(1));
+ bool isVolatile =
+ isPtrArgVolatile(CGM, E, 0) || isPtrArgVolatile(CGM, E, 1);
Value *SizeVal = EmitScalarExpr(E->getArg(2));
EmitNonNullArgCheck(RValue::get(Dest.getPointer()), E->getArg(0)->getType(),
E->getArg(0)->getExprLoc(), FD, 0);
EmitNonNullArgCheck(RValue::get(Src.getPointer()), E->getArg(1)->getType(),
E->getArg(1)->getExprLoc(), FD, 1);
- Builder.CreateMemCpy(Dest, Src, SizeVal, false);
+ Builder.CreateMemCpy(Dest, Src, SizeVal, isVolatile);
if (BuiltinID == Builtin::BImempcpy ||
BuiltinID == Builtin::BI__builtin_mempcpy)
return RValue::get(Builder.CreateInBoundsGEP(Dest.getPointer(), SizeVal));
@@ -2571,8 +2586,10 @@
break;
Address Dest = EmitPointerWithAlignment(E->getArg(0));
Address Src = EmitPointerWithAlignment(E->getArg(1));
+ bool isVolatile =
+ isPtrArgVolatile(CGM, E, 0) || isPtrArgVolatile(CGM, E, 1);
Value *SizeVal = llvm::ConstantInt::get(Builder.getContext(), Size);
- Builder.CreateMemCpy(Dest, Src, SizeVal, false);
+ Builder.CreateMemCpy(Dest, Src, SizeVal, isVolatile);
return RValue::get(Dest.getPointer());
}
@@ -2597,8 +2614,10 @@
break;
Address Dest = EmitPointerWithAlignment(E->getArg(0));
Address Src = EmitPointerWithAlignment(E->getArg(1));
+ bool isVolatile =
+ isPtrArgVolatile(CGM, E, 0) || isPtrArgVolatile(CGM, E, 1);
Value *SizeVal = llvm::ConstantInt::get(Builder.getContext(), Size);
- Builder.CreateMemMove(Dest, Src, SizeVal, false);
+ Builder.CreateMemMove(Dest, Src, SizeVal, isVolatile);
return RValue::get(Dest.getPointer());
}
@@ -2606,23 +2625,26 @@
case Builtin::BI__builtin_memmove: {
Address Dest = EmitPointerWithAlignment(E->getArg(0));
Address Src = EmitPointerWithAlignment(E->getArg(1));
+ bool isVolatile =
+ isPtrArgVolatile(CGM, E, 0) || isPtrArgVolatile(CGM, E, 1);
Value *SizeVal = EmitScalarExpr(E->getArg(2));
EmitNonNullArgCheck(RValue::get(Dest.getPointer()), E->getArg(0)->getType(),
E->getArg(0)->getExprLoc(), FD, 0);
EmitNonNullArgCheck(RValue::get(Src.getPointer()), E->getArg(1)->getType(),
E->getArg(1)->getExprLoc(), FD, 1);
- Builder.CreateMemMove(Dest, Src, SizeVal, false);
+ Builder.CreateMemMove(Dest, Src, SizeVal, isVolatile);
return RValue::get(Dest.getPointer());
}
case Builtin::BImemset:
case Builtin::BI__builtin_memset: {
Address Dest = EmitPointerWithAlignment(E->getArg(0));
+ bool isVolatile = isPtrArgVolatile(CGM, E, 0);
Value *ByteVal = Builder.CreateTrunc(EmitScalarExpr(E->getArg(1)),
Builder.getInt8Ty());
Value *SizeVal = EmitScalarExpr(E->getArg(2));
EmitNonNullArgCheck(RValue::get(Dest.getPointer()), E->getArg(0)->getType(),
E->getArg(0)->getExprLoc(), FD, 0);
- Builder.CreateMemSet(Dest, ByteVal, SizeVal, false);
+ Builder.CreateMemSet(Dest, ByteVal, SizeVal, isVolatile);
return RValue::get(Dest.getPointer());
}
case Builtin::BI__builtin___memset_chk: {
@@ -2636,10 +2658,11 @@
if (Size.ugt(DstSize))
break;
Address Dest = EmitPointerWithAlignment(E->getArg(0));
+ bool isVolatile = isPtrArgVolatile(CGM, E, 0);
Value *ByteVal = Builder.CreateTrunc(EmitScalarExpr(E->getArg(1)),
Builder.getInt8Ty());
Value *SizeVal = llvm::ConstantInt::get(Builder.getContext(), Size);
- Builder.CreateMemSet(Dest, ByteVal, SizeVal, false);
+ Builder.CreateMemSet(Dest, ByteVal, SizeVal, isVolatile);
return RValue::get(Dest.getPointer());
}
case Builtin::BI__builtin_wmemcmp: {
Index: clang/include/clang/Basic/Builtins.def
===================================================================
--- clang/include/clang/Basic/Builtins.def
+++ clang/include/clang/Basic/Builtins.def
@@ -480,11 +480,11 @@
BUILTIN(__builtin_fprintf, "iP*cC*.", "Fp:1:")
BUILTIN(__builtin_memchr, "v*vC*iz", "nF")
BUILTIN(__builtin_memcmp, "ivC*vC*z", "nF")
-BUILTIN(__builtin_memcpy, "v*v*vC*z", "nF")
+BUILTIN(__builtin_memcpy, "v*vD*vCD*z", "nF")
BUILTIN(__builtin_memcpy_inline, "vv*vC*Iz", "nt")
-BUILTIN(__builtin_memmove, "v*v*vC*z", "nF")
+BUILTIN(__builtin_memmove, "v*vD*vCD*z", "nF")
BUILTIN(__builtin_mempcpy, "v*v*vC*z", "nF")
-BUILTIN(__builtin_memset, "v*v*iz", "nF")
+BUILTIN(__builtin_memset, "v*vD*iz", "nF")
BUILTIN(__builtin_printf, "icC*.", "Fp:0:")
BUILTIN(__builtin_stpcpy, "c*c*cC*", "nF")
BUILTIN(__builtin_stpncpy, "c*c*cC*z", "nF")
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
