[PATCH] D43013: ASan+operator new[]: Fix operator new[] cookie poisoning
This revision was automatically updated to reflect the committed changes.
Closed by commit rC324884: ASan+operator new[]: Add an option for more thorough
operator new[] cookie… (authored by filcab, committed by ).
Changed prior to commit:
https://reviews.llvm.org/D43013?vs=133389=133830#toc
Repository:
rC Clang
https://reviews.llvm.org/D43013
Files:
include/clang/Driver/Options.td
include/clang/Frontend/CodeGenOptions.def
lib/CodeGen/ItaniumCXXABI.cpp
lib/Frontend/CompilerInvocation.cpp
test/CodeGen/address-sanitizer-and-array-cookie.cpp
Index: include/clang/Driver/Options.td
===
--- include/clang/Driver/Options.td
+++ include/clang/Driver/Options.td
@@ -898,6 +898,14 @@
Group,
Flags<[CoreOption, DriverOption]>,
HelpText<"Disable use-after-scope
detection in AddressSanitizer">;
+def fsanitize_address_poison_class_member_array_new_cookie
+: Flag<[ "-" ], "fsanitize-address-poison-class-member-array-new-cookie">,
+ Group,
+ HelpText<"Enable poisoning array cookies when using class member
operator new[] in AddressSanitizer">;
+def fno_sanitize_address_poison_class_member_array_new_cookie
+: Flag<[ "-" ],
"fno-sanitize-address-poison-class-member-array-new-cookie">,
+ Group,
+ HelpText<"Disable poisoning array cookies when using class member
operator new[] in AddressSanitizer">;
def fsanitize_address_globals_dead_stripping : Flag<["-"],
"fsanitize-address-globals-dead-stripping">,
Group,
HelpText<"Enable linker dead stripping
of globals in AddressSanitizer">;
Index: include/clang/Frontend/CodeGenOptions.def
===
--- include/clang/Frontend/CodeGenOptions.def
+++ include/clang/Frontend/CodeGenOptions.def
@@ -159,6 +159,9 @@
CODEGENOPT(SaveTempLabels, 1, 0) ///< Save temporary labels.
CODEGENOPT(SanitizeAddressUseAfterScope , 1, 0) ///< Enable use-after-scope
detection
///< in AddressSanitizer
+CODEGENOPT(SanitizeAddressPoisonClassMemberArrayNewCookie, 1,
+ 0) ///< Enable poisoning operator new[] which is not a replaceable
+ ///< global allocation function in AddressSanitizer
CODEGENOPT(SanitizeAddressGlobalsDeadStripping, 1, 0) ///< Enable linker dead
stripping
///< of globals in
AddressSanitizer
CODEGENOPT(SanitizeMemoryTrackOrigins, 2, 0) ///< Enable tracking origins in
Index: test/CodeGen/address-sanitizer-and-array-cookie.cpp
===
--- test/CodeGen/address-sanitizer-and-array-cookie.cpp
+++ test/CodeGen/address-sanitizer-and-array-cookie.cpp
@@ -1,13 +1,15 @@
// RUN: %clang_cc1 -triple x86_64-gnu-linux -emit-llvm -o - %s | FileCheck %s
-check-prefix=PLAIN
// RUN: %clang_cc1 -triple x86_64-gnu-linux -emit-llvm -o - -fsanitize=address
%s | FileCheck %s -check-prefix=ASAN
+// RUN: %clang_cc1 -triple x86_64-gnu-linux -emit-llvm -o - -fsanitize=address
-fsanitize-address-poison-class-member-array-new-cookie %s | FileCheck %s
-check-prefix=ASAN-POISON-ALL-NEW-ARRAY
typedef __typeof__(sizeof(0)) size_t;
namespace std {
struct nothrow_t {};
std::nothrow_t nothrow;
}
void *operator new[](size_t, const std::nothrow_t &) throw();
void *operator new[](size_t, char *);
+void *operator new[](size_t, int, int);
struct C {
int x;
@@ -53,3 +55,10 @@
}
// ASAN-LABEL: CallPlacementNew
// ASAN-NOT: __asan_poison_cxx_array_cookie
+
+C *CallNewWithArgs() {
+// ASAN-LABEL: CallNewWithArgs
+// ASAN-NOT: call void @__asan_poison_cxx_array_cookie
+// ASAN-POISON-ALL-NEW-ARRAY: call void @__asan_poison_cxx_array_cookie
+ return new (123, 456) C[20];
+}
Index: lib/CodeGen/ItaniumCXXABI.cpp
===
--- lib/CodeGen/ItaniumCXXABI.cpp
+++ lib/CodeGen/ItaniumCXXABI.cpp
@@ -1848,7 +1848,8 @@
// Handle the array cookie specially in ASan.
if (CGM.getLangOpts().Sanitize.has(SanitizerKind::Address) && AS == 0 &&
- expr->getOperatorNew()->isReplaceableGlobalAllocationFunction()) {
+ (expr->getOperatorNew()->isReplaceableGlobalAllocationFunction() ||
+ CGM.getCodeGenOpts().SanitizeAddressPoisonClassMemberArrayNewCookie)) {
// The store to the CookiePtr does not need to be instrumented.
CGM.getSanitizerMetadata()->disableSanitizerForInstruction(SI);
llvm::FunctionType *FTy =
Index: lib/Frontend/CompilerInvocation.cpp
===
--- lib/Frontend/CompilerInvocation.cpp
+++ lib/Frontend/CompilerInvocation.cpp
@@ -890,6 +890,13 @@
Opts.SanitizeCfiICallGeneralizePointers =
[PATCH] D43013: ASan+operator new[]: Fix operator new[] cookie poisoning
rjmccall accepted this revision. rjmccall added a comment. This revision is now accepted and ready to land. Okay, thanks. LGTM. Repository: rC Clang https://reviews.llvm.org/D43013 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D43013: ASan+operator new[]: Fix operator new[] cookie poisoning
filcab updated this revision to Diff 133389.
filcab added a comment.
Update commit message.
Repository:
rC Clang
https://reviews.llvm.org/D43013
Files:
include/clang/Driver/Options.td
include/clang/Frontend/CodeGenOptions.def
lib/CodeGen/ItaniumCXXABI.cpp
lib/Frontend/CompilerInvocation.cpp
test/CodeGen/address-sanitizer-and-array-cookie.cpp
Index: test/CodeGen/address-sanitizer-and-array-cookie.cpp
===
--- test/CodeGen/address-sanitizer-and-array-cookie.cpp
+++ test/CodeGen/address-sanitizer-and-array-cookie.cpp
@@ -1,13 +1,15 @@
// RUN: %clang_cc1 -triple x86_64-gnu-linux -emit-llvm -o - %s | FileCheck %s
-check-prefix=PLAIN
// RUN: %clang_cc1 -triple x86_64-gnu-linux -emit-llvm -o - -fsanitize=address
%s | FileCheck %s -check-prefix=ASAN
+// RUN: %clang_cc1 -triple x86_64-gnu-linux -emit-llvm -o - -fsanitize=address
-fsanitize-address-poison-class-member-array-new-cookie %s | FileCheck %s
-check-prefix=ASAN-POISON-ALL-NEW-ARRAY
typedef __typeof__(sizeof(0)) size_t;
namespace std {
struct nothrow_t {};
std::nothrow_t nothrow;
}
void *operator new[](size_t, const std::nothrow_t &) throw();
void *operator new[](size_t, char *);
+void *operator new[](size_t, int, int);
struct C {
int x;
@@ -53,3 +55,10 @@
}
// ASAN-LABEL: CallPlacementNew
// ASAN-NOT: __asan_poison_cxx_array_cookie
+
+C *CallNewWithArgs() {
+// ASAN-LABEL: CallNewWithArgs
+// ASAN-NOT: call void @__asan_poison_cxx_array_cookie
+// ASAN-POISON-ALL-NEW-ARRAY: call void @__asan_poison_cxx_array_cookie
+ return new (123, 456) C[20];
+}
Index: lib/Frontend/CompilerInvocation.cpp
===
--- lib/Frontend/CompilerInvocation.cpp
+++ lib/Frontend/CompilerInvocation.cpp
@@ -890,6 +890,13 @@
Opts.SanitizeCfiICallGeneralizePointers =
Args.hasArg(OPT_fsanitize_cfi_icall_generalize_pointers);
Opts.SanitizeStats = Args.hasArg(OPT_fsanitize_stats);
+ if (Arg *A = Args.getLastArg(
+ OPT_fsanitize_address_poison_class_member_array_new_cookie,
+ OPT_fno_sanitize_address_poison_class_member_array_new_cookie)) {
+Opts.SanitizeAddressPoisonClassMemberArrayNewCookie =
+A->getOption().getID() ==
+OPT_fsanitize_address_poison_class_member_array_new_cookie;
+ }
if (Arg *A = Args.getLastArg(OPT_fsanitize_address_use_after_scope,
OPT_fno_sanitize_address_use_after_scope)) {
Opts.SanitizeAddressUseAfterScope =
Index: lib/CodeGen/ItaniumCXXABI.cpp
===
--- lib/CodeGen/ItaniumCXXABI.cpp
+++ lib/CodeGen/ItaniumCXXABI.cpp
@@ -1848,7 +1848,8 @@
// Handle the array cookie specially in ASan.
if (CGM.getLangOpts().Sanitize.has(SanitizerKind::Address) && AS == 0 &&
- expr->getOperatorNew()->isReplaceableGlobalAllocationFunction()) {
+ (expr->getOperatorNew()->isReplaceableGlobalAllocationFunction() ||
+ CGM.getCodeGenOpts().SanitizeAddressPoisonClassMemberArrayNewCookie)) {
// The store to the CookiePtr does not need to be instrumented.
CGM.getSanitizerMetadata()->disableSanitizerForInstruction(SI);
llvm::FunctionType *FTy =
Index: include/clang/Frontend/CodeGenOptions.def
===
--- include/clang/Frontend/CodeGenOptions.def
+++ include/clang/Frontend/CodeGenOptions.def
@@ -159,6 +159,9 @@
CODEGENOPT(SaveTempLabels, 1, 0) ///< Save temporary labels.
CODEGENOPT(SanitizeAddressUseAfterScope , 1, 0) ///< Enable use-after-scope
detection
///< in AddressSanitizer
+CODEGENOPT(SanitizeAddressPoisonClassMemberArrayNewCookie, 1,
+ 0) ///< Enable poisoning operator new[] which is not a replaceable
+ ///< global allocation function in AddressSanitizer
CODEGENOPT(SanitizeAddressGlobalsDeadStripping, 1, 0) ///< Enable linker dead
stripping
///< of globals in
AddressSanitizer
CODEGENOPT(SanitizeMemoryTrackOrigins, 2, 0) ///< Enable tracking origins in
Index: include/clang/Driver/Options.td
===
--- include/clang/Driver/Options.td
+++ include/clang/Driver/Options.td
@@ -895,6 +895,14 @@
Group,
Flags<[CoreOption, DriverOption]>,
HelpText<"Disable use-after-scope
detection in AddressSanitizer">;
+def fsanitize_address_poison_class_member_array_new_cookie
+: Flag<[ "-" ], "fsanitize-address-poison-class-member-array-new-cookie">,
+ Group,
+ HelpText<"Enable poisoning array cookies when using class member
operator new[] in AddressSanitizer">;
+def fno_sanitize_address_poison_class_member_array_new_cookie
+: Flag<[ "-" ],
[PATCH] D43013: ASan+operator new[]: Fix operator new[] cookie poisoning
filcab added a comment. In https://reviews.llvm.org/D43013#1001006, @rjmccall wrote: > I don't understand why your description of this patch mentions the void* > placement new[] operator. There's no cookie to poison for that operator. Hah, sorry. In writing this commit log I used parts of the old patch one. I'll update with a better commit log. Repository: rC Clang https://reviews.llvm.org/D43013 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D43013: ASan+operator new[]: Fix operator new[] cookie poisoning
rjmccall added a comment. I don't understand why your description of this patch mentions the void* placement new[] operator. There's no cookie to poison for that operator. Repository: rC Clang https://reviews.llvm.org/D43013 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D43013: ASan+operator new[]: Fix operator new[] cookie poisoning
filcab created this revision.
filcab added reviewers: rjmccall, kcc, rsmith.
The C++ Itanium ABI says:
No cookie is required if the new operator being used is ::operator
new[](size_t, void*).
This commit adds a flag to tell clang to poison all operator new[]
cookies.
A previous review was poisoning unconditionally, but there is an edge
case which would stop working under ASan (a custom operator new[] saves
whatever pointer it returned, and then accesses it).
This newer revision adds a command line argument to toggle this feature.
Original revision: https://reviews.llvm.org/D41301
Compiler-rt test revision with an explanation of the edge case:
https://reviews.llvm.org/D41664
Repository:
rC Clang
https://reviews.llvm.org/D43013
Files:
include/clang/Driver/Options.td
include/clang/Frontend/CodeGenOptions.def
lib/CodeGen/ItaniumCXXABI.cpp
lib/Frontend/CompilerInvocation.cpp
test/CodeGen/address-sanitizer-and-array-cookie.cpp
Index: test/CodeGen/address-sanitizer-and-array-cookie.cpp
===
--- test/CodeGen/address-sanitizer-and-array-cookie.cpp
+++ test/CodeGen/address-sanitizer-and-array-cookie.cpp
@@ -1,13 +1,15 @@
// RUN: %clang_cc1 -triple x86_64-gnu-linux -emit-llvm -o - %s | FileCheck %s
-check-prefix=PLAIN
// RUN: %clang_cc1 -triple x86_64-gnu-linux -emit-llvm -o - -fsanitize=address
%s | FileCheck %s -check-prefix=ASAN
+// RUN: %clang_cc1 -triple x86_64-gnu-linux -emit-llvm -o - -fsanitize=address
-fsanitize-address-poison-class-member-array-new-cookie %s | FileCheck %s
-check-prefix=ASAN-POISON-ALL-NEW-ARRAY
typedef __typeof__(sizeof(0)) size_t;
namespace std {
struct nothrow_t {};
std::nothrow_t nothrow;
}
void *operator new[](size_t, const std::nothrow_t &) throw();
void *operator new[](size_t, char *);
+void *operator new[](size_t, int, int);
struct C {
int x;
@@ -53,3 +55,10 @@
}
// ASAN-LABEL: CallPlacementNew
// ASAN-NOT: __asan_poison_cxx_array_cookie
+
+C *CallNewWithArgs() {
+// ASAN-LABEL: CallNewWithArgs
+// ASAN-NOT: call void @__asan_poison_cxx_array_cookie
+// ASAN-POISON-ALL-NEW-ARRAY: call void @__asan_poison_cxx_array_cookie
+ return new (123, 456) C[20];
+}
Index: lib/Frontend/CompilerInvocation.cpp
===
--- lib/Frontend/CompilerInvocation.cpp
+++ lib/Frontend/CompilerInvocation.cpp
@@ -890,6 +890,13 @@
Opts.SanitizeCfiICallGeneralizePointers =
Args.hasArg(OPT_fsanitize_cfi_icall_generalize_pointers);
Opts.SanitizeStats = Args.hasArg(OPT_fsanitize_stats);
+ if (Arg *A = Args.getLastArg(
+ OPT_fsanitize_address_poison_class_member_array_new_cookie,
+ OPT_fno_sanitize_address_poison_class_member_array_new_cookie)) {
+Opts.SanitizeAddressPoisonClassMemberArrayNewCookie =
+A->getOption().getID() ==
+OPT_fsanitize_address_poison_class_member_array_new_cookie;
+ }
if (Arg *A = Args.getLastArg(OPT_fsanitize_address_use_after_scope,
OPT_fno_sanitize_address_use_after_scope)) {
Opts.SanitizeAddressUseAfterScope =
Index: lib/CodeGen/ItaniumCXXABI.cpp
===
--- lib/CodeGen/ItaniumCXXABI.cpp
+++ lib/CodeGen/ItaniumCXXABI.cpp
@@ -1848,7 +1848,8 @@
// Handle the array cookie specially in ASan.
if (CGM.getLangOpts().Sanitize.has(SanitizerKind::Address) && AS == 0 &&
- expr->getOperatorNew()->isReplaceableGlobalAllocationFunction()) {
+ (expr->getOperatorNew()->isReplaceableGlobalAllocationFunction() ||
+ CGM.getCodeGenOpts().SanitizeAddressPoisonClassMemberArrayNewCookie)) {
// The store to the CookiePtr does not need to be instrumented.
CGM.getSanitizerMetadata()->disableSanitizerForInstruction(SI);
llvm::FunctionType *FTy =
Index: include/clang/Frontend/CodeGenOptions.def
===
--- include/clang/Frontend/CodeGenOptions.def
+++ include/clang/Frontend/CodeGenOptions.def
@@ -159,6 +159,9 @@
CODEGENOPT(SaveTempLabels, 1, 0) ///< Save temporary labels.
CODEGENOPT(SanitizeAddressUseAfterScope , 1, 0) ///< Enable use-after-scope
detection
///< in AddressSanitizer
+CODEGENOPT(SanitizeAddressPoisonClassMemberArrayNewCookie, 1,
+ 0) ///< Enable poisoning operator new[] which is not a replaceable
+ ///< global allocation function in AddressSanitizer
CODEGENOPT(SanitizeAddressGlobalsDeadStripping, 1, 0) ///< Enable linker dead
stripping
///< of globals in
AddressSanitizer
CODEGENOPT(SanitizeMemoryTrackOrigins, 2, 0) ///< Enable tracking origins in
Index: include/clang/Driver/Options.td
===
--- include/clang/Driver/Options.td
+++ include/clang/Driver/Options.td
@@ -889,6 +889,14 @@
