[PATCH] D52615: Handle -fsanitize-address-poison-class-member-array-new-cookie in the driver and propagate it to cc1

2018-11-02 Thread Filipe Cabecinhas via Phabricator via cfe-commits
filcab added a comment.

Thanks for the review.
Unfortunately, I forgot to edit the commit message. Let's hope no one gets too 
confused (phab reviews will be up anyway, so should be easy to figure out).
Filipe


Repository:
  rL LLVM

https://reviews.llvm.org/D52615



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D52615: Handle -fsanitize-address-poison-class-member-array-new-cookie in the driver and propagate it to cc1

2018-11-02 Thread Filipe Cabecinhas via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes.
Closed by commit rL346001: Change 
-fsanitize-address-poison-class-member-array-new-cookie to -fsanitizeā€¦ 
(authored by filcab, committed by ).
Herald added a subscriber: llvm-commits.

Repository:
  rL LLVM

https://reviews.llvm.org/D52615

Files:
  cfe/trunk/docs/ClangCommandLineReference.rst
  cfe/trunk/docs/UsersManual.rst
  cfe/trunk/include/clang/Driver/Options.td
  cfe/trunk/include/clang/Driver/SanitizerArgs.h
  cfe/trunk/include/clang/Frontend/CodeGenOptions.def
  cfe/trunk/lib/CodeGen/ItaniumCXXABI.cpp
  cfe/trunk/lib/Driver/SanitizerArgs.cpp
  cfe/trunk/lib/Frontend/CompilerInvocation.cpp
  cfe/trunk/test/CodeGen/address-sanitizer-and-array-cookie.cpp
  cfe/trunk/test/Driver/fsanitize.c

Index: cfe/trunk/lib/CodeGen/ItaniumCXXABI.cpp
===
--- cfe/trunk/lib/CodeGen/ItaniumCXXABI.cpp
+++ cfe/trunk/lib/CodeGen/ItaniumCXXABI.cpp
@@ -1916,7 +1916,7 @@
   // Handle the array cookie specially in ASan.
   if (CGM.getLangOpts().Sanitize.has(SanitizerKind::Address) && AS == 0 &&
   (expr->getOperatorNew()->isReplaceableGlobalAllocationFunction() ||
-   CGM.getCodeGenOpts().SanitizeAddressPoisonClassMemberArrayNewCookie)) {
+   CGM.getCodeGenOpts().SanitizeAddressPoisonCustomArrayCookie)) {
 // The store to the CookiePtr does not need to be instrumented.
 CGM.getSanitizerMetadata()->disableSanitizerForInstruction(SI);
 llvm::FunctionType *FTy =
Index: cfe/trunk/lib/Driver/SanitizerArgs.cpp
===
--- cfe/trunk/lib/Driver/SanitizerArgs.cpp
+++ cfe/trunk/lib/Driver/SanitizerArgs.cpp
@@ -724,6 +724,11 @@
 options::OPT_fsanitize_address_use_after_scope,
 options::OPT_fno_sanitize_address_use_after_scope, AsanUseAfterScope);
 
+AsanPoisonCustomArrayCookie = Args.hasFlag(
+options::OPT_fsanitize_address_poison_custom_array_cookie,
+options::OPT_fno_sanitize_address_poison_custom_array_cookie,
+AsanPoisonCustomArrayCookie);
+
 // As a workaround for a bug in gold 2.26 and earlier, dead stripping of
 // globals in ASan is disabled by default on ELF targets.
 // See https://sourceware.org/bugzilla/show_bug.cgi?id=19002
@@ -897,6 +902,9 @@
   if (AsanUseAfterScope)
 CmdArgs.push_back("-fsanitize-address-use-after-scope");
 
+  if (AsanPoisonCustomArrayCookie)
+CmdArgs.push_back("-fsanitize-address-poison-custom-array-cookie");
+
   if (AsanGlobalsDeadStripping)
 CmdArgs.push_back("-fsanitize-address-globals-dead-stripping");
 
Index: cfe/trunk/lib/Frontend/CompilerInvocation.cpp
===
--- cfe/trunk/lib/Frontend/CompilerInvocation.cpp
+++ cfe/trunk/lib/Frontend/CompilerInvocation.cpp
@@ -969,11 +969,11 @@
   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 =
+  OPT_fsanitize_address_poison_custom_array_cookie,
+  OPT_fno_sanitize_address_poison_custom_array_cookie)) {
+Opts.SanitizeAddressPoisonCustomArrayCookie =
 A->getOption().getID() ==
-OPT_fsanitize_address_poison_class_member_array_new_cookie;
+OPT_fsanitize_address_poison_custom_array_cookie;
   }
   if (Arg *A = Args.getLastArg(OPT_fsanitize_address_use_after_scope,
OPT_fno_sanitize_address_use_after_scope)) {
Index: cfe/trunk/docs/UsersManual.rst
===
--- cfe/trunk/docs/UsersManual.rst
+++ cfe/trunk/docs/UsersManual.rst
@@ -3000,8 +3000,8 @@
   -fno-debug-macroDo not emit macro debug information
   -fno-delayed-template-parsing
   Disable delayed template parsing
-  -fno-sanitize-address-poison-class-member-array-new-cookie
-  Disable poisoning array cookies when using class member operator new[] in AddressSanitizer
+  -fno-sanitize-address-poison-custom-array-cookie
+  Disable poisoning array cookies when using custom operator new[] in AddressSanitizer
   -fno-sanitize-address-use-after-scope
   Disable use-after-scope detection in AddressSanitizer
   -fno-sanitize-blacklist Don't use blacklist file for sanitizers
@@ -3037,8 +3037,8 @@
   Level of field padding for AddressSanitizer
   -fsanitize-address-globals-dead-stripping
   Enable linker dead stripping of globals in AddressSanitizer
-  -fsanitize-address-poison-class-member-array-new-cookie
- 

[PATCH] D52615: Handle -fsanitize-address-poison-class-member-array-new-cookie in the driver and propagate it to cc1

2018-11-02 Thread Filipe Cabecinhas via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes.
Closed by commit rC346001: Change 
-fsanitize-address-poison-class-member-array-new-cookie to -fsanitizeā€¦ 
(authored by filcab, committed by ).

Changed prior to commit:
  https://reviews.llvm.org/D52615?vs=172376=172392#toc

Repository:
  rL LLVM

https://reviews.llvm.org/D52615

Files:
  docs/ClangCommandLineReference.rst
  docs/UsersManual.rst
  include/clang/Driver/Options.td
  include/clang/Driver/SanitizerArgs.h
  include/clang/Frontend/CodeGenOptions.def
  lib/CodeGen/ItaniumCXXABI.cpp
  lib/Driver/SanitizerArgs.cpp
  lib/Frontend/CompilerInvocation.cpp
  test/CodeGen/address-sanitizer-and-array-cookie.cpp
  test/Driver/fsanitize.c

Index: docs/ClangCommandLineReference.rst
===
--- docs/ClangCommandLineReference.rst
+++ docs/ClangCommandLineReference.rst
@@ -800,9 +800,11 @@
 
 Enable linker dead stripping of globals in AddressSanitizer
 
-.. option:: -fsanitize-address-poison-class-member-array-new-cookie, -fno-sanitize-address-poison-class-member-array-new-cookie
+.. option:: -fsanitize-address-poison-custom-array-cookie, -fno-sanitize-address-poison-custom-array-cookie
 
-Enable poisoning array cookies when using class member operator new\[\] in AddressSanitizer
+Enable "poisoning" array cookies when allocating arrays with a custom operator new\[\] in Address Sanitizer, preventing accesses to the cookies from user code. An array cookie is a small implementation-defined header added to certain array allocations to record metadata such as the length of the array. Accesses to array cookies from user code are technically allowed by the standard but are more likely to be the result of an out-of-bounds array access.
+
+An operator new\[\] is "custom" if it is not one of the allocation functions provided by the C++ standard library. Array cookies from non-custom allocation functions are always poisoned.
 
 .. option:: -fsanitize-address-use-after-scope, -fno-sanitize-address-use-after-scope
 
Index: docs/UsersManual.rst
===
--- docs/UsersManual.rst
+++ docs/UsersManual.rst
@@ -3000,8 +3000,8 @@
   -fno-debug-macroDo not emit macro debug information
   -fno-delayed-template-parsing
   Disable delayed template parsing
-  -fno-sanitize-address-poison-class-member-array-new-cookie
-  Disable poisoning array cookies when using class member operator new[] in AddressSanitizer
+  -fno-sanitize-address-poison-custom-array-cookie
+  Disable poisoning array cookies when using custom operator new[] in AddressSanitizer
   -fno-sanitize-address-use-after-scope
   Disable use-after-scope detection in AddressSanitizer
   -fno-sanitize-blacklist Don't use blacklist file for sanitizers
@@ -3037,8 +3037,8 @@
   Level of field padding for AddressSanitizer
   -fsanitize-address-globals-dead-stripping
   Enable linker dead stripping of globals in AddressSanitizer
-  -fsanitize-address-poison-class-member-array-new-cookie
-  Enable poisoning array cookies when using class member operator new[] in AddressSanitizer
+  -fsanitize-address-poison-custom-array-cookie
+  Enable poisoning array cookies when using custom operator new[] in AddressSanitizer
   -fsanitize-address-use-after-scope
   Enable use-after-scope detection in AddressSanitizer
   -fsanitize-blacklist=
Index: include/clang/Frontend/CodeGenOptions.def
===
--- include/clang/Frontend/CodeGenOptions.def
+++ include/clang/Frontend/CodeGenOptions.def
@@ -174,7 +174,7 @@
 CODEGENOPT(SaveTempLabels, 1, 0) ///< Save temporary labels.
 CODEGENOPT(SanitizeAddressUseAfterScope , 1, 0) ///< Enable use-after-scope detection
 ///< in AddressSanitizer
-CODEGENOPT(SanitizeAddressPoisonClassMemberArrayNewCookie, 1,
+CODEGENOPT(SanitizeAddressPoisonCustomArrayCookie, 1,
0) ///< Enable poisoning operator new[] which is not a replaceable
   ///< global allocation function in AddressSanitizer
 CODEGENOPT(SanitizeAddressGlobalsDeadStripping, 1, 0) ///< Enable linker dead stripping
Index: include/clang/Driver/SanitizerArgs.h
===
--- include/clang/Driver/SanitizerArgs.h
+++ include/clang/Driver/SanitizerArgs.h
@@ -36,6 +36,7 @@
   int AsanFieldPadding = 0;
   bool SharedRuntime = false;
   bool AsanUseAfterScope = true;
+  bool AsanPoisonCustomArrayCookie = false;
   bool AsanGlobalsDeadStripping = false;
   bool LinkCXXRuntimes = false;
   bool NeedPIE = false;
Index: 

[PATCH] D52615: Handle -fsanitize-address-poison-class-member-array-new-cookie in the driver and propagate it to cc1

2018-11-02 Thread John McCall via Phabricator via cfe-commits
rjmccall accepted this revision.
rjmccall added a comment.
This revision is now accepted and ready to land.

Thanks, LGTM.


Repository:
  rC Clang

https://reviews.llvm.org/D52615



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D52615: Handle -fsanitize-address-poison-class-member-array-new-cookie in the driver and propagate it to cc1

2018-11-02 Thread Filipe Cabecinhas via Phabricator via cfe-commits
filcab updated this revision to Diff 172376.
filcab added a comment.

Reworded with feedback from the review.


Repository:
  rC Clang

https://reviews.llvm.org/D52615

Files:
  docs/ClangCommandLineReference.rst
  docs/UsersManual.rst
  include/clang/Driver/Options.td
  include/clang/Driver/SanitizerArgs.h
  include/clang/Frontend/CodeGenOptions.def
  lib/CodeGen/ItaniumCXXABI.cpp
  lib/Driver/SanitizerArgs.cpp
  lib/Frontend/CompilerInvocation.cpp
  test/CodeGen/address-sanitizer-and-array-cookie.cpp
  test/Driver/fsanitize.c

Index: test/Driver/fsanitize.c
===
--- test/Driver/fsanitize.c
+++ test/Driver/fsanitize.c
@@ -208,6 +208,24 @@
 // RUN: %clang -target x86_64-linux-gnu -fsanitize=address %s -### 2>&1 | FileCheck %s --check-prefix=CHECK-ASAN-WITHOUT-USE-AFTER-SCOPE
 // CHECK-ASAN-WITHOUT-USE-AFTER-SCOPE: -cc1{{.*}}address-use-after-scope
 
+// RUN: %clang -target x86_64-linux-gnu -fsanitize=address -fsanitize-address-poison-custom-array-cookie %s -### 2>&1 | FileCheck %s --check-prefix=CHECK-POISON-CUSTOM-ARRAY-NEW-COOKIE
+// RUN: %clang_cl --target=x86_64-windows -fsanitize=address -fsanitize-address-poison-custom-array-cookie -### -- %s 2>&1 | FileCheck %s --check-prefix=CHECK-POISON-CUSTOM-ARRAY-NEW-COOKIE
+// CHECK-POISON-CUSTOM-ARRAY-NEW-COOKIE: -cc1{{.*}}-fsanitize-address-poison-custom-array-cookie
+
+// RUN: %clang -target x86_64-linux-gnu -fsanitize=address -fno-sanitize-address-poison-custom-array-cookie %s -### 2>&1 | FileCheck %s --check-prefix=CHECK-POISON-CUSTOM-ARRAY-NEW-COOKIE-OFF
+// RUN: %clang_cl --target=x86_64-windows -fsanitize=address -fno-sanitize-address-poison-custom-array-cookie -### -- %s 2>&1 | FileCheck %s --check-prefix=CHECK-POISON-CUSTOM-ARRAY-NEW-COOKIE-OFF
+// CHECK-POISON-CUSTOM-ARRAY-NEW-COOKIE-OFF-NOT: -cc1{{.*}}address-poison-custom-array-cookie
+
+// RUN: %clang -target x86_64-linux-gnu -fsanitize=address -fno-sanitize-address-poison-custom-array-cookie -fsanitize-address-poison-custom-array-cookie %s -### 2>&1 | FileCheck %s --check-prefix=CHECK-POISON-CUSTOM-ARRAY-NEW-COOKIE-BOTH
+// RUN: %clang_cl --target=x86_64-windows -fsanitize=address -fno-sanitize-address-poison-custom-array-cookie -fsanitize-address-poison-custom-array-cookie -### -- %s 2>&1 | FileCheck %s --check-prefix=CHECK-POISON-CUSTOM-ARRAY-NEW-COOKIE-BOTH
+// CHECK-POISON-CUSTOM-ARRAY-NEW-COOKIE-BOTH: -cc1{{.*}}-fsanitize-address-poison-custom-array-cookie
+
+// RUN: %clang -target x86_64-linux-gnu -fsanitize=address -fsanitize-address-poison-custom-array-cookie -fno-sanitize-address-poison-custom-array-cookie %s -### 2>&1 | FileCheck %s --check-prefix=CHECK-POISON-CUSTOM-ARRAY-NEW-COOKIE-BOTH-OFF
+// CHECK-POISON-CUSTOM-ARRAY-NEW-COOKIE-BOTH-OFF-NOT: -cc1{{.*}}address-poison-custom-array-cookie
+
+// RUN: %clang -target x86_64-linux-gnu -fsanitize=address %s -### 2>&1 | FileCheck %s --check-prefix=CHECK-ASAN-WITHOUT-POISON-CUSTOM-ARRAY-NEW-COOKIE
+// CHECK-ASAN-WITHOUT-POISON-CUSTOM-ARRAY-NEW-COOKIE-NOT: -cc1{{.*}}address-poison-custom-array-cookie
+
 // RUN: %clang -target x86_64-linux-gnu -fsanitize=address -fsanitize-address-globals-dead-stripping %s -### 2>&1 | FileCheck %s --check-prefix=CHECK-ASAN-GLOBALS
 // RUN: %clang -target x86_64-linux-gnu -fsanitize=address %s -### 2>&1 | FileCheck %s --check-prefix=CHECK-NO-ASAN-GLOBALS
 // RUN: %clang_cl --target=x86_64-windows-msvc -fsanitize=address -fsanitize-address-globals-dead-stripping -### -- %s 2>&1 | FileCheck %s --check-prefix=CHECK-ASAN-GLOBALS
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,6 +1,6 @@
 // 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
+// RUN: %clang_cc1 -triple x86_64-gnu-linux -emit-llvm -o - -fsanitize=address -fsanitize-address-poison-custom-array-cookie %s | FileCheck %s -check-prefix=ASAN-POISON-ALL-NEW-ARRAY
 
 typedef __typeof__(sizeof(0)) size_t;
 namespace std {
Index: lib/Frontend/CompilerInvocation.cpp
===
--- lib/Frontend/CompilerInvocation.cpp
+++ lib/Frontend/CompilerInvocation.cpp
@@ -968,11 +968,11 @@
   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)) {
-

[PATCH] D52615: Handle -fsanitize-address-poison-class-member-array-new-cookie in the driver and propagate it to cc1

2018-11-01 Thread John McCall via Phabricator via cfe-commits
rjmccall added inline comments.



Comment at: docs/ClangCommandLineReference.rst:805
 
-Enable poisoning array cookies when using class member operator new\[\] in 
AddressSanitizer
+Enable poisoning array cookies when using custom operator new\[\] in 
AddressSanitizer
 

rjmccall wrote:
> This is user documentation, so it would be good to explain here what exactly 
> this does and why you might enable or disable it.  I know the surrounding 
> documentation is even more barebones, but that's a problem, and it's a 
> problem we won't fix by making it worse.
Thanks.  Grammar/style clean-up:

> Enable "poisoning" array cookies when allocating arrays with a custom 
> `operator new[]` in Address Sanitizer, preventing accesses to the cookies 
> from user code.  An array cookie is a small implementation-defined header 
> added to certain array allocations to record metadata such as the length of 
> the array.  Accesses to array cookies from user code are technically allowed 
> by the standard but are more likely to be the result of an out-of-bounds 
> array access.

> An `operator new[]` is "custom" if it is not one of the allocation functions 
> provided by the C++ standard library.  Array cookies from non-custom 
> allocation functions are always poisoned.


Repository:
  rC Clang

https://reviews.llvm.org/D52615



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D52615: Handle -fsanitize-address-poison-class-member-array-new-cookie in the driver and propagate it to cc1

2018-11-01 Thread Filipe Cabecinhas via Phabricator via cfe-commits
filcab updated this revision to Diff 172179.
filcab added a comment.

Expand yet a bit more.


Repository:
  rC Clang

https://reviews.llvm.org/D52615

Files:
  docs/ClangCommandLineReference.rst
  docs/UsersManual.rst
  include/clang/Driver/Options.td
  include/clang/Driver/SanitizerArgs.h
  include/clang/Frontend/CodeGenOptions.def
  lib/CodeGen/ItaniumCXXABI.cpp
  lib/Driver/SanitizerArgs.cpp
  lib/Frontend/CompilerInvocation.cpp
  test/CodeGen/address-sanitizer-and-array-cookie.cpp
  test/Driver/fsanitize.c

Index: test/Driver/fsanitize.c
===
--- test/Driver/fsanitize.c
+++ test/Driver/fsanitize.c
@@ -208,6 +208,24 @@
 // RUN: %clang -target x86_64-linux-gnu -fsanitize=address %s -### 2>&1 | FileCheck %s --check-prefix=CHECK-ASAN-WITHOUT-USE-AFTER-SCOPE
 // CHECK-ASAN-WITHOUT-USE-AFTER-SCOPE: -cc1{{.*}}address-use-after-scope
 
+// RUN: %clang -target x86_64-linux-gnu -fsanitize=address -fsanitize-address-poison-custom-array-cookie %s -### 2>&1 | FileCheck %s --check-prefix=CHECK-POISON-CUSTOM-ARRAY-NEW-COOKIE
+// RUN: %clang_cl --target=x86_64-windows -fsanitize=address -fsanitize-address-poison-custom-array-cookie -### -- %s 2>&1 | FileCheck %s --check-prefix=CHECK-POISON-CUSTOM-ARRAY-NEW-COOKIE
+// CHECK-POISON-CUSTOM-ARRAY-NEW-COOKIE: -cc1{{.*}}-fsanitize-address-poison-custom-array-cookie
+
+// RUN: %clang -target x86_64-linux-gnu -fsanitize=address -fno-sanitize-address-poison-custom-array-cookie %s -### 2>&1 | FileCheck %s --check-prefix=CHECK-POISON-CUSTOM-ARRAY-NEW-COOKIE-OFF
+// RUN: %clang_cl --target=x86_64-windows -fsanitize=address -fno-sanitize-address-poison-custom-array-cookie -### -- %s 2>&1 | FileCheck %s --check-prefix=CHECK-POISON-CUSTOM-ARRAY-NEW-COOKIE-OFF
+// CHECK-POISON-CUSTOM-ARRAY-NEW-COOKIE-OFF-NOT: -cc1{{.*}}address-poison-custom-array-cookie
+
+// RUN: %clang -target x86_64-linux-gnu -fsanitize=address -fno-sanitize-address-poison-custom-array-cookie -fsanitize-address-poison-custom-array-cookie %s -### 2>&1 | FileCheck %s --check-prefix=CHECK-POISON-CUSTOM-ARRAY-NEW-COOKIE-BOTH
+// RUN: %clang_cl --target=x86_64-windows -fsanitize=address -fno-sanitize-address-poison-custom-array-cookie -fsanitize-address-poison-custom-array-cookie -### -- %s 2>&1 | FileCheck %s --check-prefix=CHECK-POISON-CUSTOM-ARRAY-NEW-COOKIE-BOTH
+// CHECK-POISON-CUSTOM-ARRAY-NEW-COOKIE-BOTH: -cc1{{.*}}-fsanitize-address-poison-custom-array-cookie
+
+// RUN: %clang -target x86_64-linux-gnu -fsanitize=address -fsanitize-address-poison-custom-array-cookie -fno-sanitize-address-poison-custom-array-cookie %s -### 2>&1 | FileCheck %s --check-prefix=CHECK-POISON-CUSTOM-ARRAY-NEW-COOKIE-BOTH-OFF
+// CHECK-POISON-CUSTOM-ARRAY-NEW-COOKIE-BOTH-OFF-NOT: -cc1{{.*}}address-poison-custom-array-cookie
+
+// RUN: %clang -target x86_64-linux-gnu -fsanitize=address %s -### 2>&1 | FileCheck %s --check-prefix=CHECK-ASAN-WITHOUT-POISON-CUSTOM-ARRAY-NEW-COOKIE
+// CHECK-ASAN-WITHOUT-POISON-CUSTOM-ARRAY-NEW-COOKIE-NOT: -cc1{{.*}}address-poison-custom-array-cookie
+
 // RUN: %clang -target x86_64-linux-gnu -fsanitize=address -fsanitize-address-globals-dead-stripping %s -### 2>&1 | FileCheck %s --check-prefix=CHECK-ASAN-GLOBALS
 // RUN: %clang -target x86_64-linux-gnu -fsanitize=address %s -### 2>&1 | FileCheck %s --check-prefix=CHECK-NO-ASAN-GLOBALS
 // RUN: %clang_cl --target=x86_64-windows-msvc -fsanitize=address -fsanitize-address-globals-dead-stripping -### -- %s 2>&1 | FileCheck %s --check-prefix=CHECK-ASAN-GLOBALS
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,6 +1,6 @@
 // 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
+// RUN: %clang_cc1 -triple x86_64-gnu-linux -emit-llvm -o - -fsanitize=address -fsanitize-address-poison-custom-array-cookie %s | FileCheck %s -check-prefix=ASAN-POISON-ALL-NEW-ARRAY
 
 typedef __typeof__(sizeof(0)) size_t;
 namespace std {
Index: lib/Frontend/CompilerInvocation.cpp
===
--- lib/Frontend/CompilerInvocation.cpp
+++ lib/Frontend/CompilerInvocation.cpp
@@ -968,11 +968,11 @@
   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)) {
-

[PATCH] D52615: Handle -fsanitize-address-poison-class-member-array-new-cookie in the driver and propagate it to cc1

2018-11-01 Thread Filipe Cabecinhas via Phabricator via cfe-commits
filcab updated this revision to Diff 172149.
filcab added a comment.

Expanded a bit more on the documentation, mentioning that user code might be 
technically allowed to access those array cookies, but that users might not 
want to allow it.


Repository:
  rC Clang

https://reviews.llvm.org/D52615

Files:
  docs/ClangCommandLineReference.rst
  docs/UsersManual.rst
  include/clang/Driver/Options.td
  include/clang/Driver/SanitizerArgs.h
  include/clang/Frontend/CodeGenOptions.def
  lib/CodeGen/ItaniumCXXABI.cpp
  lib/Driver/SanitizerArgs.cpp
  lib/Frontend/CompilerInvocation.cpp
  test/CodeGen/address-sanitizer-and-array-cookie.cpp
  test/Driver/fsanitize.c

Index: test/Driver/fsanitize.c
===
--- test/Driver/fsanitize.c
+++ test/Driver/fsanitize.c
@@ -208,6 +208,24 @@
 // RUN: %clang -target x86_64-linux-gnu -fsanitize=address %s -### 2>&1 | FileCheck %s --check-prefix=CHECK-ASAN-WITHOUT-USE-AFTER-SCOPE
 // CHECK-ASAN-WITHOUT-USE-AFTER-SCOPE: -cc1{{.*}}address-use-after-scope
 
+// RUN: %clang -target x86_64-linux-gnu -fsanitize=address -fsanitize-address-poison-custom-array-cookie %s -### 2>&1 | FileCheck %s --check-prefix=CHECK-POISON-CUSTOM-ARRAY-NEW-COOKIE
+// RUN: %clang_cl --target=x86_64-windows -fsanitize=address -fsanitize-address-poison-custom-array-cookie -### -- %s 2>&1 | FileCheck %s --check-prefix=CHECK-POISON-CUSTOM-ARRAY-NEW-COOKIE
+// CHECK-POISON-CUSTOM-ARRAY-NEW-COOKIE: -cc1{{.*}}-fsanitize-address-poison-custom-array-cookie
+
+// RUN: %clang -target x86_64-linux-gnu -fsanitize=address -fno-sanitize-address-poison-custom-array-cookie %s -### 2>&1 | FileCheck %s --check-prefix=CHECK-POISON-CUSTOM-ARRAY-NEW-COOKIE-OFF
+// RUN: %clang_cl --target=x86_64-windows -fsanitize=address -fno-sanitize-address-poison-custom-array-cookie -### -- %s 2>&1 | FileCheck %s --check-prefix=CHECK-POISON-CUSTOM-ARRAY-NEW-COOKIE-OFF
+// CHECK-POISON-CUSTOM-ARRAY-NEW-COOKIE-OFF-NOT: -cc1{{.*}}address-poison-custom-array-cookie
+
+// RUN: %clang -target x86_64-linux-gnu -fsanitize=address -fno-sanitize-address-poison-custom-array-cookie -fsanitize-address-poison-custom-array-cookie %s -### 2>&1 | FileCheck %s --check-prefix=CHECK-POISON-CUSTOM-ARRAY-NEW-COOKIE-BOTH
+// RUN: %clang_cl --target=x86_64-windows -fsanitize=address -fno-sanitize-address-poison-custom-array-cookie -fsanitize-address-poison-custom-array-cookie -### -- %s 2>&1 | FileCheck %s --check-prefix=CHECK-POISON-CUSTOM-ARRAY-NEW-COOKIE-BOTH
+// CHECK-POISON-CUSTOM-ARRAY-NEW-COOKIE-BOTH: -cc1{{.*}}-fsanitize-address-poison-custom-array-cookie
+
+// RUN: %clang -target x86_64-linux-gnu -fsanitize=address -fsanitize-address-poison-custom-array-cookie -fno-sanitize-address-poison-custom-array-cookie %s -### 2>&1 | FileCheck %s --check-prefix=CHECK-POISON-CUSTOM-ARRAY-NEW-COOKIE-BOTH-OFF
+// CHECK-POISON-CUSTOM-ARRAY-NEW-COOKIE-BOTH-OFF-NOT: -cc1{{.*}}address-poison-custom-array-cookie
+
+// RUN: %clang -target x86_64-linux-gnu -fsanitize=address %s -### 2>&1 | FileCheck %s --check-prefix=CHECK-ASAN-WITHOUT-POISON-CUSTOM-ARRAY-NEW-COOKIE
+// CHECK-ASAN-WITHOUT-POISON-CUSTOM-ARRAY-NEW-COOKIE-NOT: -cc1{{.*}}address-poison-custom-array-cookie
+
 // RUN: %clang -target x86_64-linux-gnu -fsanitize=address -fsanitize-address-globals-dead-stripping %s -### 2>&1 | FileCheck %s --check-prefix=CHECK-ASAN-GLOBALS
 // RUN: %clang -target x86_64-linux-gnu -fsanitize=address %s -### 2>&1 | FileCheck %s --check-prefix=CHECK-NO-ASAN-GLOBALS
 // RUN: %clang_cl --target=x86_64-windows-msvc -fsanitize=address -fsanitize-address-globals-dead-stripping -### -- %s 2>&1 | FileCheck %s --check-prefix=CHECK-ASAN-GLOBALS
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,6 +1,6 @@
 // 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
+// RUN: %clang_cc1 -triple x86_64-gnu-linux -emit-llvm -o - -fsanitize=address -fsanitize-address-poison-custom-array-cookie %s | FileCheck %s -check-prefix=ASAN-POISON-ALL-NEW-ARRAY
 
 typedef __typeof__(sizeof(0)) size_t;
 namespace std {
Index: lib/Frontend/CompilerInvocation.cpp
===
--- lib/Frontend/CompilerInvocation.cpp
+++ lib/Frontend/CompilerInvocation.cpp
@@ -968,11 +968,11 @@
   Args.hasArg(OPT_fsanitize_cfi_icall_generalize_pointers);
   Opts.SanitizeStats = Args.hasArg(OPT_fsanitize_stats);
   if (Arg *A = Args.getLastArg(
-  

[PATCH] D52615: Handle -fsanitize-address-poison-class-member-array-new-cookie in the driver and propagate it to cc1

2018-10-30 Thread John McCall via Phabricator via cfe-commits
rjmccall added a comment.

Thanks, and I agree with renaming the existing option.




Comment at: docs/ClangCommandLineReference.rst:805
 
-Enable poisoning array cookies when using class member operator new\[\] in 
AddressSanitizer
+Enable poisoning array cookies when using custom operator new\[\] in 
AddressSanitizer
 

This is user documentation, so it would be good to explain here what exactly 
this does and why you might enable or disable it.  I know the surrounding 
documentation is even more barebones, but that's a problem, and it's a problem 
we won't fix by making it worse.


Repository:
  rC Clang

https://reviews.llvm.org/D52615



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D52615: Handle -fsanitize-address-poison-class-member-array-new-cookie in the driver and propagate it to cc1

2018-10-30 Thread Filipe Cabecinhas via Phabricator via cfe-commits
filcab added a comment.

In https://reviews.llvm.org/D52615#1278000, @rjmccall wrote:

> So, three points:
>
> - That's not class-member-specific; you can have a placement `operator new[]` 
> at global scope that isn't the special `void*` placement operator and 
> therefore still has a cookie, and it would have just as much flexibility as a 
> class-member override would.  So the split you're trying to describe is the 
> standard operators vs. custom ones.
> - Anyone can provide their own definition of the standard operators; there 
> are some semantic restrictions on those definitions, but I'm not sure what 
> about those restrictions would forbid this kind of capture.
> - Even with the standard implementations of the standard replaceable 
> operators, I'm not sure what rule would actually outlaw the client from going 
> from the result of `new[]` back to the cookie.
>
>   At any rate, taking the feature as a given, the first point suggests 
> `-fsanitize-address-poison-custom-array-cookie` or something along those 
> lines.  If we want a more standard-ese term than "custom", the standard 
> refers to its operators collectively as "library allocation functions", so 
> maybe "non-library".


Thank you. I went with your suggestion, as I think it's close enough to be 
understandable. I've also changed the -cc1 argument in this patch, as it looks 
weird to have two different arguments (unless needed). Let me know if you'd 
like to keep the different names.

Thank you,
Filipe


Repository:
  rC Clang

https://reviews.llvm.org/D52615



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D52615: Handle -fsanitize-address-poison-class-member-array-new-cookie in the driver and propagate it to cc1

2018-10-30 Thread Filipe Cabecinhas via Phabricator via cfe-commits
filcab updated this revision to Diff 171661.
filcab added a comment.

Missed the change in some places


Repository:
  rC Clang

https://reviews.llvm.org/D52615

Files:
  docs/ClangCommandLineReference.rst
  docs/UsersManual.rst
  include/clang/Driver/Options.td
  include/clang/Driver/SanitizerArgs.h
  include/clang/Frontend/CodeGenOptions.def
  lib/CodeGen/ItaniumCXXABI.cpp
  lib/Driver/SanitizerArgs.cpp
  lib/Frontend/CompilerInvocation.cpp
  test/CodeGen/address-sanitizer-and-array-cookie.cpp
  test/Driver/fsanitize.c

Index: test/Driver/fsanitize.c
===
--- test/Driver/fsanitize.c
+++ test/Driver/fsanitize.c
@@ -208,6 +208,24 @@
 // RUN: %clang -target x86_64-linux-gnu -fsanitize=address %s -### 2>&1 | FileCheck %s --check-prefix=CHECK-ASAN-WITHOUT-USE-AFTER-SCOPE
 // CHECK-ASAN-WITHOUT-USE-AFTER-SCOPE: -cc1{{.*}}address-use-after-scope
 
+// RUN: %clang -target x86_64-linux-gnu -fsanitize=address -fsanitize-address-poison-custom-array-cookie %s -### 2>&1 | FileCheck %s --check-prefix=CHECK-POISON-CUSTOM-ARRAY-NEW-COOKIE
+// RUN: %clang_cl --target=x86_64-windows -fsanitize=address -fsanitize-address-poison-custom-array-cookie -### -- %s 2>&1 | FileCheck %s --check-prefix=CHECK-POISON-CUSTOM-ARRAY-NEW-COOKIE
+// CHECK-POISON-CUSTOM-ARRAY-NEW-COOKIE: -cc1{{.*}}-fsanitize-address-poison-custom-array-cookie
+
+// RUN: %clang -target x86_64-linux-gnu -fsanitize=address -fno-sanitize-address-poison-custom-array-cookie %s -### 2>&1 | FileCheck %s --check-prefix=CHECK-POISON-CUSTOM-ARRAY-NEW-COOKIE-OFF
+// RUN: %clang_cl --target=x86_64-windows -fsanitize=address -fno-sanitize-address-poison-custom-array-cookie -### -- %s 2>&1 | FileCheck %s --check-prefix=CHECK-POISON-CUSTOM-ARRAY-NEW-COOKIE-OFF
+// CHECK-POISON-CUSTOM-ARRAY-NEW-COOKIE-OFF-NOT: -cc1{{.*}}address-poison-custom-array-cookie
+
+// RUN: %clang -target x86_64-linux-gnu -fsanitize=address -fno-sanitize-address-poison-custom-array-cookie -fsanitize-address-poison-custom-array-cookie %s -### 2>&1 | FileCheck %s --check-prefix=CHECK-POISON-CUSTOM-ARRAY-NEW-COOKIE-BOTH
+// RUN: %clang_cl --target=x86_64-windows -fsanitize=address -fno-sanitize-address-poison-custom-array-cookie -fsanitize-address-poison-custom-array-cookie -### -- %s 2>&1 | FileCheck %s --check-prefix=CHECK-POISON-CUSTOM-ARRAY-NEW-COOKIE-BOTH
+// CHECK-POISON-CUSTOM-ARRAY-NEW-COOKIE-BOTH: -cc1{{.*}}-fsanitize-address-poison-custom-array-cookie
+
+// RUN: %clang -target x86_64-linux-gnu -fsanitize=address -fsanitize-address-poison-custom-array-cookie -fno-sanitize-address-poison-custom-array-cookie %s -### 2>&1 | FileCheck %s --check-prefix=CHECK-POISON-CUSTOM-ARRAY-NEW-COOKIE-BOTH-OFF
+// CHECK-POISON-CUSTOM-ARRAY-NEW-COOKIE-BOTH-OFF-NOT: -cc1{{.*}}address-poison-custom-array-cookie
+
+// RUN: %clang -target x86_64-linux-gnu -fsanitize=address %s -### 2>&1 | FileCheck %s --check-prefix=CHECK-ASAN-WITHOUT-POISON-CUSTOM-ARRAY-NEW-COOKIE
+// CHECK-ASAN-WITHOUT-POISON-CUSTOM-ARRAY-NEW-COOKIE-NOT: -cc1{{.*}}address-poison-custom-array-cookie
+
 // RUN: %clang -target x86_64-linux-gnu -fsanitize=address -fsanitize-address-globals-dead-stripping %s -### 2>&1 | FileCheck %s --check-prefix=CHECK-ASAN-GLOBALS
 // RUN: %clang -target x86_64-linux-gnu -fsanitize=address %s -### 2>&1 | FileCheck %s --check-prefix=CHECK-NO-ASAN-GLOBALS
 // RUN: %clang_cl --target=x86_64-windows-msvc -fsanitize=address -fsanitize-address-globals-dead-stripping -### -- %s 2>&1 | FileCheck %s --check-prefix=CHECK-ASAN-GLOBALS
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,6 +1,6 @@
 // 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
+// RUN: %clang_cc1 -triple x86_64-gnu-linux -emit-llvm -o - -fsanitize=address -fsanitize-address-poison-custom-array-cookie %s | FileCheck %s -check-prefix=ASAN-POISON-ALL-NEW-ARRAY
 
 typedef __typeof__(sizeof(0)) size_t;
 namespace std {
Index: lib/Frontend/CompilerInvocation.cpp
===
--- lib/Frontend/CompilerInvocation.cpp
+++ lib/Frontend/CompilerInvocation.cpp
@@ -968,11 +968,11 @@
   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)) {
-

[PATCH] D52615: Handle -fsanitize-address-poison-class-member-array-new-cookie in the driver and propagate it to cc1

2018-10-30 Thread Filipe Cabecinhas via Phabricator via cfe-commits
filcab updated this revision to Diff 171660.
filcab added a comment.

Update with name change, using rjmccall's suggestion


Repository:
  rC Clang

https://reviews.llvm.org/D52615

Files:
  include/clang/Driver/Options.td
  include/clang/Driver/SanitizerArgs.h
  include/clang/Frontend/CodeGenOptions.def
  lib/CodeGen/ItaniumCXXABI.cpp
  lib/Driver/SanitizerArgs.cpp
  lib/Frontend/CompilerInvocation.cpp
  test/Driver/fsanitize.c

Index: test/Driver/fsanitize.c
===
--- test/Driver/fsanitize.c
+++ test/Driver/fsanitize.c
@@ -208,6 +208,24 @@
 // RUN: %clang -target x86_64-linux-gnu -fsanitize=address %s -### 2>&1 | FileCheck %s --check-prefix=CHECK-ASAN-WITHOUT-USE-AFTER-SCOPE
 // CHECK-ASAN-WITHOUT-USE-AFTER-SCOPE: -cc1{{.*}}address-use-after-scope
 
+// RUN: %clang -target x86_64-linux-gnu -fsanitize=address -fsanitize-address-poison-custom-array-cookie %s -### 2>&1 | FileCheck %s --check-prefix=CHECK-POISON-CUSTOM-ARRAY-NEW-COOKIE
+// RUN: %clang_cl --target=x86_64-windows -fsanitize=address -fsanitize-address-poison-custom-array-cookie -### -- %s 2>&1 | FileCheck %s --check-prefix=CHECK-POISON-CUSTOM-ARRAY-NEW-COOKIE
+// CHECK-POISON-CUSTOM-ARRAY-NEW-COOKIE: -cc1{{.*}}-fsanitize-address-poison-custom-array-cookie
+
+// RUN: %clang -target x86_64-linux-gnu -fsanitize=address -fno-sanitize-address-poison-custom-array-cookie %s -### 2>&1 | FileCheck %s --check-prefix=CHECK-POISON-CUSTOM-ARRAY-NEW-COOKIE-OFF
+// RUN: %clang_cl --target=x86_64-windows -fsanitize=address -fno-sanitize-address-poison-custom-array-cookie -### -- %s 2>&1 | FileCheck %s --check-prefix=CHECK-POISON-CUSTOM-ARRAY-NEW-COOKIE-OFF
+// CHECK-POISON-CUSTOM-ARRAY-NEW-COOKIE-OFF-NOT: -cc1{{.*}}address-poison-custom-array-cookie
+
+// RUN: %clang -target x86_64-linux-gnu -fsanitize=address -fno-sanitize-address-poison-custom-array-cookie -fsanitize-address-poison-custom-array-cookie %s -### 2>&1 | FileCheck %s --check-prefix=CHECK-POISON-CUSTOM-ARRAY-NEW-COOKIE-BOTH
+// RUN: %clang_cl --target=x86_64-windows -fsanitize=address -fno-sanitize-address-poison-custom-array-cookie -fsanitize-address-poison-custom-array-cookie -### -- %s 2>&1 | FileCheck %s --check-prefix=CHECK-POISON-CUSTOM-ARRAY-NEW-COOKIE-BOTH
+// CHECK-POISON-CUSTOM-ARRAY-NEW-COOKIE-BOTH: -cc1{{.*}}-fsanitize-address-poison-custom-array-cookie
+
+// RUN: %clang -target x86_64-linux-gnu -fsanitize=address -fsanitize-address-poison-custom-array-cookie -fno-sanitize-address-poison-custom-array-cookie %s -### 2>&1 | FileCheck %s --check-prefix=CHECK-POISON-CUSTOM-ARRAY-NEW-COOKIE-BOTH-OFF
+// CHECK-POISON-CUSTOM-ARRAY-NEW-COOKIE-BOTH-OFF-NOT: -cc1{{.*}}address-poison-custom-array-cookie
+
+// RUN: %clang -target x86_64-linux-gnu -fsanitize=address %s -### 2>&1 | FileCheck %s --check-prefix=CHECK-ASAN-WITHOUT-POISON-CUSTOM-ARRAY-NEW-COOKIE
+// CHECK-ASAN-WITHOUT-POISON-CUSTOM-ARRAY-NEW-COOKIE-NOT: -cc1{{.*}}address-poison-custom-array-cookie
+
 // RUN: %clang -target x86_64-linux-gnu -fsanitize=address -fsanitize-address-globals-dead-stripping %s -### 2>&1 | FileCheck %s --check-prefix=CHECK-ASAN-GLOBALS
 // RUN: %clang -target x86_64-linux-gnu -fsanitize=address %s -### 2>&1 | FileCheck %s --check-prefix=CHECK-NO-ASAN-GLOBALS
 // RUN: %clang_cl --target=x86_64-windows-msvc -fsanitize=address -fsanitize-address-globals-dead-stripping -### -- %s 2>&1 | FileCheck %s --check-prefix=CHECK-ASAN-GLOBALS
Index: lib/Frontend/CompilerInvocation.cpp
===
--- lib/Frontend/CompilerInvocation.cpp
+++ lib/Frontend/CompilerInvocation.cpp
@@ -968,11 +968,11 @@
   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 =
+  OPT_fsanitize_address_poison_custom_array_cookie,
+  OPT_fno_sanitize_address_poison_custom_array_cookie)) {
+Opts.SanitizeAddressPoisonCustomArrayCookie =
 A->getOption().getID() ==
-OPT_fsanitize_address_poison_class_member_array_new_cookie;
+OPT_fsanitize_address_poison_custom_array_cookie;
   }
   if (Arg *A = Args.getLastArg(OPT_fsanitize_address_use_after_scope,
OPT_fno_sanitize_address_use_after_scope)) {
Index: lib/Driver/SanitizerArgs.cpp
===
--- lib/Driver/SanitizerArgs.cpp
+++ lib/Driver/SanitizerArgs.cpp
@@ -724,6 +724,11 @@
 options::OPT_fsanitize_address_use_after_scope,
 options::OPT_fno_sanitize_address_use_after_scope, AsanUseAfterScope);
 
+AsanPoisonCustomArrayCookie = Args.hasFlag(
+options::OPT_fsanitize_address_poison_custom_array_cookie,
+

[PATCH] D52615: Handle -fsanitize-address-poison-class-member-array-new-cookie in the driver and propagate it to cc1

2018-10-26 Thread John McCall via Phabricator via cfe-commits
rjmccall added a comment.

So, three points:

- That's not class-member-specific; you can have a placement `operator new[]` 
at global scope that isn't the special `void*` placement operator and therefore 
still has a cookie, and it would have just as much flexibility as a 
class-member override would.  So the split you're trying to describe is the 
standard operators vs. custom ones.

- Anyone can provide their own definition of the standard operators; there are 
some semantic restrictions on those definitions, but I'm not sure what about 
those restrictions would forbid this kind of capture.

- Even with the standard implementations of the standard replaceable operators, 
I'm not sure what rule would actually outlaw the client from going from the 
result of `new[]` back to the cookie.

At any rate, taking the feature as a given, the first point suggests 
`-fsanitize-address-poison-custom-array-cookie` or something along those lines. 
 If we want a more standard-ese term than "custom", the standard refers to its 
operators collectively as "library allocation functions", so maybe 
"non-library".


Repository:
  rC Clang

https://reviews.llvm.org/D52615



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D52615: Handle -fsanitize-address-poison-class-member-array-new-cookie in the driver and propagate it to cc1

2018-10-26 Thread Filipe Cabecinhas via Phabricator via cfe-commits
filcab added a comment.

In https://reviews.llvm.org/D52615#1276938, @rjmccall wrote:

> In https://reviews.llvm.org/D52615#1275946, @filcab wrote:
>
> > In https://reviews.llvm.org/D52615#1272725, @rjmccall wrote:
> >
> > > In https://reviews.llvm.org/D52615#1266320, @filcab wrote:
> > >
> > > > In https://reviews.llvm.org/D52615#1254567, @rsmith wrote:
> > > >
> > > > > This seems like an unreasonably long flag name. Can you try to find a 
> > > > > shorter name for it?
> > > >
> > > >
> > > > `-fsanitize-poison-extra-operator-new`?
> > > >  Not as explicit, but maybe ok if documented?
> > >
> > >
> > > `-fsanitize-address-poison-array-cookie`?
> >
> >
> > I strongly dislike this one because "poison array cookie", in general, is 
> > always enabled (it's actually triggered by a runtime flag). This flag is 
> > about poisoning it in more cases (cases where the standard doesn't 
> > completely disallow accesses to the cookie, so we have to have a flag and 
> > can't enable it all the time).
>
>
> Hmm.  This naming discussion might be a proxy for another debate.  I 
> understand the argument that programs are allowed to assume a particular C++ 
> ABI and therefore access the array cookie.  And I can understand having an 
> option that makes ASan stricter and disallow accessing the array cookie 
> anyway.  But I don't understand why this analysis is in any way 
> case-specific, and the discussion you've linked doesn't seem particularly 
> clarifying about that point.  Can you explain this a little?


If you don't have a class-member operator new[], then you don't expect to be 
able to have access to the memory occupied by the array cookie, since you never 
"see" a pointer that points to it.
But if you have a class-member operator new[], then you can keep a copy of all 
the pointers you've returned, and you're allowed to read from them, even if 
they contain an array cookie (you're just reading from memory you've returned 
to users).
With this flag, we disallow this second case, at the expense of having ASan 
trigger an error on code that is standards compliant. We're making it more 
strict, therefore we start emitting ASan errors on the test I was removing in 
https://reviews.llvm.org/D41664. Kostya's comment on me removing that test was 
that the code is valid, so ASan shouldn't reject it by default (but ok to have 
flags that make it more strict).

Thank you,
Filipe

P.S: We still have "placement new" (`::operator new[](size_t, void*)`as a 
special case, which will not have an array cookie, so we don't poison it. This 
flag only applies to user-defined, class-specific allocation functions.

P.P.S: Sorry if I misunderstood your question. Please let me know.


Repository:
  rC Clang

https://reviews.llvm.org/D52615



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D52615: Handle -fsanitize-address-poison-class-member-array-new-cookie in the driver and propagate it to cc1

2018-10-25 Thread John McCall via Phabricator via cfe-commits
rjmccall added a comment.

Oops, sorry for the unedited comment; it's fixed on Phab.


Repository:
  rC Clang

https://reviews.llvm.org/D52615



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D52615: Handle -fsanitize-address-poison-class-member-array-new-cookie in the driver and propagate it to cc1

2018-10-25 Thread John McCall via Phabricator via cfe-commits
rjmccall added a comment.

In https://reviews.llvm.org/D52615#1275946, @filcab wrote:

> In https://reviews.llvm.org/D52615#1272725, @rjmccall wrote:
>
> > In https://reviews.llvm.org/D52615#1266320, @filcab wrote:
> >
> > > In https://reviews.llvm.org/D52615#1254567, @rsmith wrote:
> > >
> > > > This seems like an unreasonably long flag name. Can you try to find a 
> > > > shorter name for it?
> > >
> > >
> > > `-fsanitize-poison-extra-operator-new`?
> > >  Not as explicit, but maybe ok if documented?
> >
> >
> > `-fsanitize-address-poison-array-cookie`?
>
>
> I strongly dislike this one because "poison array cookie", in general, is 
> always enabled (it's actually triggered by a runtime flag). This flag is 
> about poisoning it in more cases (cases where the standard doesn't completely 
> disallow accesses to the cookie, so we have to have a flag and can't enable 
> it all the time).


Hmm.  This naming discussion might be a proxy for another debate.  I understand 
the argument that programs are allowed to assume a particular C++ ABI and 
therefore access the array cookie.  And I can understand having an option that 
makes ASan stricter and disallow accessing the array cookie anyway.  But I 
don't understand why this analysis is in any way case-specific, and the 
discussion you've linked doesn't seem particularly clarifying about that point. 
 Can you explain this a little?

The discussion you've linked doesn't explain why this differs from case to 
case.  Can you please provide both a positive and a negative example of cases 
where there actually is an array cookie butI don't understand the 
case-by-case distinction from the discussion there.  I get that the cookie

In what situations is there an actual cookie in which it is differently legal 
to assume information about the C++ ABI and


Repository:
  rC Clang

https://reviews.llvm.org/D52615



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D52615: Handle -fsanitize-address-poison-class-member-array-new-cookie in the driver and propagate it to cc1

2018-10-25 Thread Filipe Cabecinhas via Phabricator via cfe-commits
filcab added a comment.

In https://reviews.llvm.org/D52615#1272725, @rjmccall wrote:

> In https://reviews.llvm.org/D52615#1266320, @filcab wrote:
>
> > In https://reviews.llvm.org/D52615#1254567, @rsmith wrote:
> >
> > > This seems like an unreasonably long flag name. Can you try to find a 
> > > shorter name for it?
> >
> >
> > `-fsanitize-poison-extra-operator-new`?
> >  Not as explicit, but maybe ok if documented?
>
>
> `-fsanitize-address-poison-array-cookie`?


I strongly dislike this one because "poison array cookie", in general, is 
always enabled (it's actually triggered by a runtime flag). This flag is about 
poisoning it in more cases (cases where the standard doesn't completely 
disallow accesses to the cookie, so we have to have a flag and can't enable it 
all the time).

Thank you,
Filipe

P.S: Some additional discussion is at https://reviews.llvm.org/D41664, from 
when this flag was first implemented.


Repository:
  rC Clang

https://reviews.llvm.org/D52615



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D52615: Handle -fsanitize-address-poison-class-member-array-new-cookie in the driver and propagate it to cc1

2018-10-23 Thread John McCall via Phabricator via cfe-commits
rjmccall added a comment.

In https://reviews.llvm.org/D52615#1266320, @filcab wrote:

> In https://reviews.llvm.org/D52615#1254567, @rsmith wrote:
>
> > This seems like an unreasonably long flag name. Can you try to find a 
> > shorter name for it?
>
>
> `-fsanitize-poison-extra-operator-new`?
>  Not as explicit, but maybe ok if documented?


`-fsanitize-address-poison-array-cookie`?


Repository:
  rC Clang

https://reviews.llvm.org/D52615



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D52615: Handle -fsanitize-address-poison-class-member-array-new-cookie in the driver and propagate it to cc1

2018-10-23 Thread Filipe Cabecinhas via Phabricator via cfe-commits
filcab added a comment.

Ping!

Thank you,
Filipe


Repository:
  rC Clang

https://reviews.llvm.org/D52615



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D52615: Handle -fsanitize-address-poison-class-member-array-new-cookie in the driver and propagate it to cc1

2018-10-16 Thread Filipe Cabecinhas via Phabricator via cfe-commits
filcab added a comment.

In https://reviews.llvm.org/D52615#1254567, @rsmith wrote:

> This seems like an unreasonably long flag name. Can you try to find a shorter 
> name for it?


`-fsanitize-poison-extra-operator-new`?
Not as explicit, but maybe ok if documented?

Thank you,
Filipe


Repository:
  rC Clang

https://reviews.llvm.org/D52615



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D52615: Handle -fsanitize-address-poison-class-member-array-new-cookie in the driver and propagate it to cc1

2018-10-03 Thread Richard Smith - zygoloid via Phabricator via cfe-commits
rsmith added a comment.

This seems like an unreasonably long flag name. Can you try to find a shorter 
name for it?


Repository:
  rC Clang

https://reviews.llvm.org/D52615



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D52615: Handle -fsanitize-address-poison-class-member-array-new-cookie in the driver and propagate it to cc1

2018-09-27 Thread Filipe Cabecinhas via Phabricator via cfe-commits
filcab created this revision.
filcab added reviewers: rjmccall, kcc, rsmith.

Repository:
  rC Clang

https://reviews.llvm.org/D52615

Files:
  include/clang/Driver/SanitizerArgs.h
  lib/Driver/SanitizerArgs.cpp
  test/Driver/fsanitize.c


Index: test/Driver/fsanitize.c
===
--- test/Driver/fsanitize.c
+++ test/Driver/fsanitize.c
@@ -191,6 +191,24 @@
 // RUN: %clang -target x86_64-linux-gnu -fsanitize=address %s -### 2>&1 | 
FileCheck %s --check-prefix=CHECK-ASAN-WITHOUT-USE-AFTER-SCOPE
 // CHECK-ASAN-WITHOUT-USE-AFTER-SCOPE: -cc1{{.*}}address-use-after-scope
 
+// RUN: %clang -target x86_64-linux-gnu -fsanitize=address 
-fsanitize-address-poison-class-member-array-new-cookie %s -### 2>&1 | 
FileCheck %s --check-prefix=CHECK-POISON-CLASS-MEMBER-ARRAY-NEW-COOKIE
+// RUN: %clang_cl --target=x86_64-windows -fsanitize=address 
-fsanitize-address-poison-class-member-array-new-cookie -### -- %s 2>&1 | 
FileCheck %s --check-prefix=CHECK-POISON-CLASS-MEMBER-ARRAY-NEW-COOKIE
+// CHECK-POISON-CLASS-MEMBER-ARRAY-NEW-COOKIE: 
-cc1{{.*}}-fsanitize-address-poison-class-member-array-new-cookie
+
+// RUN: %clang -target x86_64-linux-gnu -fsanitize=address 
-fno-sanitize-address-poison-class-member-array-new-cookie %s -### 2>&1 | 
FileCheck %s --check-prefix=CHECK-POISON-CLASS-MEMBER-ARRAY-NEW-COOKIE-OFF
+// RUN: %clang_cl --target=x86_64-windows -fsanitize=address 
-fno-sanitize-address-poison-class-member-array-new-cookie -### -- %s 2>&1 | 
FileCheck %s --check-prefix=CHECK-POISON-CLASS-MEMBER-ARRAY-NEW-COOKIE-OFF
+// CHECK-POISON-CLASS-MEMBER-ARRAY-NEW-COOKIE-OFF-NOT: 
-cc1{{.*}}address-poison-class-member-array-new-cookie
+
+// RUN: %clang -target x86_64-linux-gnu -fsanitize=address 
-fno-sanitize-address-poison-class-member-array-new-cookie 
-fsanitize-address-poison-class-member-array-new-cookie %s -### 2>&1 | 
FileCheck %s --check-prefix=CHECK-POISON-CLASS-MEMBER-ARRAY-NEW-COOKIE-BOTH
+// RUN: %clang_cl --target=x86_64-windows -fsanitize=address 
-fno-sanitize-address-poison-class-member-array-new-cookie 
-fsanitize-address-poison-class-member-array-new-cookie -### -- %s 2>&1 | 
FileCheck %s --check-prefix=CHECK-POISON-CLASS-MEMBER-ARRAY-NEW-COOKIE-BOTH
+// CHECK-POISON-CLASS-MEMBER-ARRAY-NEW-COOKIE-BOTH: 
-cc1{{.*}}-fsanitize-address-poison-class-member-array-new-cookie
+
+// RUN: %clang -target x86_64-linux-gnu -fsanitize=address 
-fsanitize-address-poison-class-member-array-new-cookie 
-fno-sanitize-address-poison-class-member-array-new-cookie %s -### 2>&1 | 
FileCheck %s --check-prefix=CHECK-POISON-CLASS-MEMBER-ARRAY-NEW-COOKIE-BOTH-OFF
+// CHECK-POISON-CLASS-MEMBER-ARRAY-NEW-COOKIE-BOTH-OFF-NOT: 
-cc1{{.*}}address-poison-class-member-array-new-cookie
+
+// RUN: %clang -target x86_64-linux-gnu -fsanitize=address %s -### 2>&1 | 
FileCheck %s 
--check-prefix=CHECK-ASAN-WITHOUT-POISON-CLASS-MEMBER-ARRAY-NEW-COOKIE
+// CHECK-ASAN-WITHOUT-POISON-CLASS-MEMBER-ARRAY-NEW-COOKIE-NOT: 
-cc1{{.*}}address-poison-class-member-array-new-cookie
+
 // RUN: %clang -target x86_64-linux-gnu -fsanitize=address 
-fsanitize-address-globals-dead-stripping %s -### 2>&1 | FileCheck %s 
--check-prefix=CHECK-ASAN-GLOBALS
 // RUN: %clang -target x86_64-linux-gnu -fsanitize=address %s -### 2>&1 | 
FileCheck %s --check-prefix=CHECK-NO-ASAN-GLOBALS
 // RUN: %clang_cl --target=x86_64-windows-msvc -fsanitize=address 
-fsanitize-address-globals-dead-stripping -### -- %s 2>&1 | FileCheck %s 
--check-prefix=CHECK-ASAN-GLOBALS
Index: lib/Driver/SanitizerArgs.cpp
===
--- lib/Driver/SanitizerArgs.cpp
+++ lib/Driver/SanitizerArgs.cpp
@@ -724,6 +724,11 @@
 options::OPT_fsanitize_address_use_after_scope,
 options::OPT_fno_sanitize_address_use_after_scope, AsanUseAfterScope);
 
+AsanPoisonClassMemberOperatorNew = Args.hasFlag(
+options::OPT_fsanitize_address_poison_class_member_array_new_cookie,
+options::OPT_fno_sanitize_address_poison_class_member_array_new_cookie,
+AsanPoisonClassMemberOperatorNew);
+
 // As a workaround for a bug in gold 2.26 and earlier, dead stripping of
 // globals in ASan is disabled by default on ELF targets.
 // See https://sourceware.org/bugzilla/show_bug.cgi?id=19002
@@ -897,6 +902,10 @@
   if (AsanUseAfterScope)
 CmdArgs.push_back("-fsanitize-address-use-after-scope");
 
+  if (AsanPoisonClassMemberOperatorNew)
+CmdArgs.push_back(
+"-fsanitize-address-poison-class-member-array-new-cookie");
+
   if (AsanGlobalsDeadStripping)
 CmdArgs.push_back("-fsanitize-address-globals-dead-stripping");
 
Index: include/clang/Driver/SanitizerArgs.h
===
--- include/clang/Driver/SanitizerArgs.h
+++ include/clang/Driver/SanitizerArgs.h
@@ -36,6 +36,7 @@
   int AsanFieldPadding = 0;
   bool SharedRuntime = false;
   bool AsanUseAfterScope = true;
+  bool