[PATCH] D124493: [symbolizer] Parse DW_TAG_variable DIs to show line info for globals

[Filipe Cabecinhas via Phabricator via cfe-commits]

filcab added a comment.

Hi @hctim, thanks for the patch.
I have one question, though. Do you really need to remove the information you 
removed?
Some people might be testing ASan binaries without access to debug info 
(because it's been stripped or it's not been loaded and is not available for 
the sanitizers to get symbols from, etc), and having the full global 
information would be useful for those reports.

Can this be done by having a flag to make clang not emit the source information 
+ the sanitizer patch to get the line and column?
That way we could keep the current behaviour of emitting more info, and you'd 
still get your savings + use of debuginfo.

Thank you,
Filipe


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D124493/new/

https://reviews.llvm.org/D124493

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

[PATCH] D67985: CFI: wrong type passed to llvm.type.test with multiple inheritance devirtualization

[Filipe Cabecinhas via Phabricator via cfe-commits]

filcab added subscribers: pcc, filcab.
filcab added a comment.

It seems there's a FIXME anticipating this problem.

@pcc: Can you double-check, please?

Thank you,
Filipe


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D67985/new/

https://reviews.llvm.org/D67985



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

[PATCH] D32950: Support C++1z features in `__has_extension`

[Filipe Cabecinhas via Phabricator via cfe-commits]

filcab added a comment.

Hi Eric,

I know this is old, but are you interested in reviving this patch? I don't know 
enough about clang's extensions to LGTM such a patch (updated for the current 
code), but would really like to have a way to know if extensions are supported.
We just now had people ask how to detect if `if constexpr` is available, and 
not having a feature check makes it much harder to figure out (and makes our 
test based on current clang behaviour).

Thank you,
Filipe


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D32950/new/

https://reviews.llvm.org/D32950



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

[PATCH] D46403: [CFI] Force LLVM to die if the implicit blacklist files cannot be found.

[Filipe Cabecinhas via Phabricator via cfe-commits]

filcab added a comment.

Sorry to ressurect this review, but I have a few questions:

- What kind of functions fail?
- Are there bugzillas to track these?
- How can a compiler expect to have blacklists for "all" its CFI clients? (I'm 
ok with having a default for "most-used, well-known, problematic functions", 
e.g: functions in libstdc++ or libc++)
- clang/compiler-rt don't seem to have a default blacklist, what should the 
contents be? This patch seems to be saying "There are some well-known functions 
that should never be instrumented with CFI, I'll provide a list of names", 
which doesn't really seem possible to do in general, for "most" CFI-toolchain 
clients. As I see it, each client will need to have their own blacklist, and 
provide it as an argument if needed. Which brings us to:
- If I pass `-fsanitize-blacklist=my_blacklist.txt` I still get an error. This 
is not ideal, as I might be working on the blacklist to include in my 
toolchain/program.

I don't think we should have an error that is default enabled for this issue. 
At most a warning (+ `-Werror`) if there was no blacklist passed in nor found 
in the toolchain resource directory.

Thank you,
Filipe

P.S: Sorry for only noticing this so much later.


Repository:
  rC Clang

https://reviews.llvm.org/D46403



___
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

[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

[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

[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

[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

[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

[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

[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

[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

[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

[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

[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

[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

[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] D50901: [clang][ubsan] Split Implicit Integer Truncation Sanitizer into unsigned and signed checks

[Filipe Cabecinhas via Phabricator via cfe-commits]

filcab added a comment.

LGTM on the clang side too.

Thank you,
Filipe


Repository:
  rC Clang

https://reviews.llvm.org/D50901



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

[PATCH] D50901: [clang][ubsan] Split Implicit Integer Truncation Sanitizer into unsigned and signed checks

[Filipe Cabecinhas via Phabricator via cfe-commits]

filcab added a subscriber: aizatsky.
filcab added a comment.

Sorry about that. I’m away today but I don’t think you’ve answered my
questions about “why not support standalone UBSan in tests”. Sorry if I
missed the answers if you did. Will review the latest tomorrow.

Thank you,
Filipe


Repository:
  rC Clang

https://reviews.llvm.org/D50901



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

[PATCH] D50901: [clang][ubsan] Split Implicit Integer Truncation Sanitizer into unsigned and signed checks

[Filipe Cabecinhas via Phabricator via cfe-commits]

filcab added inline comments.



Comment at: lib/CodeGen/CGExprScalar.cpp:305
   enum ImplicitConversionCheckKind : unsigned char {
-ICCK_IntegerTruncation = 0,
+ICCK_IntegerTruncation = 0, // Legacy, no longer used.
+ICCK_UnsignedIntegerTruncation = 1,

vitalybuka wrote:
> lebedev.ri wrote:
> > vitalybuka wrote:
> > > why do you need to keep it?
> > *Here* - for consistency with the compiler-rt part.
> > 
> > There - what about mismatch in the used clang version
> > (which still only produced the `(ImplicitConversionCheckKind)0`), and 
> > compiler-rt version
> > (which has `(ImplicitConversionCheckKind)1` and 
> > `(ImplicitConversionCheckKind)2`)?
> > Is it 100.00% guaranteed not to happen? I'm not so sure.
> I don't think we try support mismatched versions of clang and compiler-rt
We don't make a big effort in open source. But we try to at least make it work 
(check previous work on type_mismatch handler, before versions were 
introduced), or error "loudly" when linking (versioned checks).

I think having keeping the older check number free is a good thing and allows 
us to have binary compatibility with older objects (and keep supporting old 
object files (we *did* release llvm 7.0 with the old-type check)).

If we hadn't had a release in between, I'd be all for reusing.


Repository:
  rC Clang

https://reviews.llvm.org/D50901



___
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

[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 

[PATCH] D47375: [Driver] Add flag "--dependent-lib=..." when enabling asan or ubsan on PS4.

[Filipe Cabecinhas via Phabricator via cfe-commits]

filcab added a comment.

I have a minor nit + what Paul mentioned (missing a `-NOT` check). Otherwise 
LGTM.




Comment at: lib/Driver/ToolChains/Clang.cpp:3690
 
-  // Add runtime flag for PS4 when PGO or Coverage are enabled.
-  if (RawTriple.isPS4CPU())
+  // Add runtime flag for PS4 when PGO or Coverage or sanitizers are enabled.
+  if (RawTriple.isPS4CPU()) {

Nit: `PGO, coverage, or sanitizers`


Repository:
  rC Clang

https://reviews.llvm.org/D47375



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

[PATCH] D47291: Proposal to make rtti errors more generic.

[Filipe Cabecinhas via Phabricator via cfe-commits]

filcab added inline comments.



Comment at: include/clang/Basic/DiagnosticSemaKinds.td:6729
 def err_no_dynamic_cast_with_fno_rtti : Error<
-  "cannot use dynamic_cast with -fno-rtti">;
+  "use of dynamic_cast requires enabling RTTI">;
 

I'd prefer to have the way to enable RTTI mentioned in the message. Could we 
have something like `ToolChain::getRTTIMode()` and have a "RTTI was on/of" or 
"RTTI defaulted to on/off"? That way we'd be able to have a message similar to 
the current one (mentioning "you passed -fno-rtti") on platforms that default 
to RTTI=on, and have your updated message (possibly with a mention of "use 
-frtti") on platforms that default to RTTI=off.

(This is a minor usability comment about this patch, I don't consider it a 
blocker or anything)


Repository:
  rC Clang

https://reviews.llvm.org/D47291



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

[PATCH] D46836: Fix some rtti-options tests

[Filipe Cabecinhas via Phabricator via cfe-commits]

filcab accepted this revision.
filcab added a comment.
This revision is now accepted and ready to land.

LGTM




Comment at: test/Driver/rtti-options.cpp:13
 // RUN: %clang -### -c -fno-rtti -frtti %s 2>&1 | FileCheck 
-check-prefix=CHECK-RTTI %s
-// RUN: %clang -### -c -frtti -fno-rtti %s 2>&1 | FileCheck 
-check-prefix=CHECK-NO-RTTI %s
+// RUN: %clang -### -c -frtti -fno-rtti %s 2>&1 | FileCheck 
-check-prefix=CHECK-RTTI-NOT %s
 

Minor nit: I like saving space like that, but I think it might be slightly 
confusing for someone reading this, as the `-NOT` suffix is special in 
`FileCheck`. I don't think this is a huge problem as we have the single CHECK 
line, and we don't end up using `...-NOT-NOT` anyway.
Feel free to keep or change as you want.


Repository:
  rC Clang

https://reviews.llvm.org/D46836



___
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

[Filipe Cabecinhas via Phabricator via cfe-commits]

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

[Filipe Cabecinhas via Phabricator via cfe-commits]

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

[Filipe Cabecinhas via Phabricator via cfe-commits]

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

[Filipe Cabecinhas via Phabricator via cfe-commits]

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 @@
   

[PATCH] D41301: ASan+operator new[]: Fix operator new[] cookie poisoning

[Filipe Cabecinhas via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rL321645: ASan+operator new[]: Fix operator new[] cookie 
poisoning (authored by filcab, committed by ).

Repository:
  rL LLVM

https://reviews.llvm.org/D41301

Files:
  cfe/trunk/lib/CodeGen/ItaniumCXXABI.cpp
  cfe/trunk/test/CodeGen/address-sanitizer-and-array-cookie.cpp


Index: cfe/trunk/test/CodeGen/address-sanitizer-and-array-cookie.cpp
===
--- cfe/trunk/test/CodeGen/address-sanitizer-and-array-cookie.cpp
+++ cfe/trunk/test/CodeGen/address-sanitizer-and-array-cookie.cpp
@@ -7,7 +7,7 @@
   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, void *);
 
 struct C {
   int x;
@@ -53,3 +53,11 @@
 }
 // ASAN-LABEL: CallPlacementNew
 // ASAN-NOT: __asan_poison_cxx_array_cookie
+
+void *operator new[](size_t n, int);
+
+C *CallNewWithArgs() {
+// ASAN-LABEL: CallNewWithArgs
+// ASAN: call void @__asan_poison_cxx_array_cookie
+  return new (123) C[20];
+}
Index: cfe/trunk/lib/CodeGen/ItaniumCXXABI.cpp
===
--- cfe/trunk/lib/CodeGen/ItaniumCXXABI.cpp
+++ cfe/trunk/lib/CodeGen/ItaniumCXXABI.cpp
@@ -1847,8 +1847,7 @@
   llvm::Instruction *SI = CGF.Builder.CreateStore(NumElements, NumElementsPtr);
 
   // Handle the array cookie specially in ASan.
-  if (CGM.getLangOpts().Sanitize.has(SanitizerKind::Address) && AS == 0 &&
-  expr->getOperatorNew()->isReplaceableGlobalAllocationFunction()) {
+  if (CGM.getLangOpts().Sanitize.has(SanitizerKind::Address) && AS == 0) {
 // The store to the CookiePtr does not need to be instrumented.
 CGM.getSanitizerMetadata()->disableSanitizerForInstruction(SI);
 llvm::FunctionType *FTy =


Index: cfe/trunk/test/CodeGen/address-sanitizer-and-array-cookie.cpp
===
--- cfe/trunk/test/CodeGen/address-sanitizer-and-array-cookie.cpp
+++ cfe/trunk/test/CodeGen/address-sanitizer-and-array-cookie.cpp
@@ -7,7 +7,7 @@
   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, void *);
 
 struct C {
   int x;
@@ -53,3 +53,11 @@
 }
 // ASAN-LABEL: CallPlacementNew
 // ASAN-NOT: __asan_poison_cxx_array_cookie
+
+void *operator new[](size_t n, int);
+
+C *CallNewWithArgs() {
+// ASAN-LABEL: CallNewWithArgs
+// ASAN: call void @__asan_poison_cxx_array_cookie
+  return new (123) C[20];
+}
Index: cfe/trunk/lib/CodeGen/ItaniumCXXABI.cpp
===
--- cfe/trunk/lib/CodeGen/ItaniumCXXABI.cpp
+++ cfe/trunk/lib/CodeGen/ItaniumCXXABI.cpp
@@ -1847,8 +1847,7 @@
   llvm::Instruction *SI = CGF.Builder.CreateStore(NumElements, NumElementsPtr);
 
   // Handle the array cookie specially in ASan.
-  if (CGM.getLangOpts().Sanitize.has(SanitizerKind::Address) && AS == 0 &&
-  expr->getOperatorNew()->isReplaceableGlobalAllocationFunction()) {
+  if (CGM.getLangOpts().Sanitize.has(SanitizerKind::Address) && AS == 0) {
 // The store to the CookiePtr does not need to be instrumented.
 CGM.getSanitizerMetadata()->disableSanitizerForInstruction(SI);
 llvm::FunctionType *FTy =
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D41301: ASan+operator new[]: Fix operator new[] cookie poisoning

[Filipe Cabecinhas via Phabricator via cfe-commits]

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*).

We should only avoid poisoning the cookie if we're calling this
operator, not others. This is dealt with before the call to
InitializeArrayCookie.


Repository:
  rC Clang

https://reviews.llvm.org/D41301

Files:
  lib/CodeGen/ItaniumCXXABI.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
@@ -7,7 +7,7 @@
   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, void *);
 
 struct C {
   int x;
@@ -53,3 +53,11 @@
 }
 // ASAN-LABEL: CallPlacementNew
 // ASAN-NOT: __asan_poison_cxx_array_cookie
+
+void *operator new[](size_t n, int);
+
+C *CallNewWithArgs() {
+// ASAN-LABEL: CallNewWithArgs
+// ASAN: call void @__asan_poison_cxx_array_cookie
+  return new (123) C[20];
+}
Index: lib/CodeGen/ItaniumCXXABI.cpp
===
--- lib/CodeGen/ItaniumCXXABI.cpp
+++ lib/CodeGen/ItaniumCXXABI.cpp
@@ -1843,8 +1843,7 @@
   llvm::Instruction *SI = CGF.Builder.CreateStore(NumElements, NumElementsPtr);
 
   // Handle the array cookie specially in ASan.
-  if (CGM.getLangOpts().Sanitize.has(SanitizerKind::Address) && AS == 0 &&
-  expr->getOperatorNew()->isReplaceableGlobalAllocationFunction()) {
+  if (CGM.getLangOpts().Sanitize.has(SanitizerKind::Address) && AS == 0) {
 // The store to the CookiePtr does not need to be instrumented.
 CGM.getSanitizerMetadata()->disableSanitizerForInstruction(SI);
 llvm::FunctionType *FTy =


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
@@ -7,7 +7,7 @@
   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, void *);
 
 struct C {
   int x;
@@ -53,3 +53,11 @@
 }
 // ASAN-LABEL: CallPlacementNew
 // ASAN-NOT: __asan_poison_cxx_array_cookie
+
+void *operator new[](size_t n, int);
+
+C *CallNewWithArgs() {
+// ASAN-LABEL: CallNewWithArgs
+// ASAN: call void @__asan_poison_cxx_array_cookie
+  return new (123) C[20];
+}
Index: lib/CodeGen/ItaniumCXXABI.cpp
===
--- lib/CodeGen/ItaniumCXXABI.cpp
+++ lib/CodeGen/ItaniumCXXABI.cpp
@@ -1843,8 +1843,7 @@
   llvm::Instruction *SI = CGF.Builder.CreateStore(NumElements, NumElementsPtr);
 
   // Handle the array cookie specially in ASan.
-  if (CGM.getLangOpts().Sanitize.has(SanitizerKind::Address) && AS == 0 &&
-  expr->getOperatorNew()->isReplaceableGlobalAllocationFunction()) {
+  if (CGM.getLangOpts().Sanitize.has(SanitizerKind::Address) && AS == 0) {
 // The store to the CookiePtr does not need to be instrumented.
 CGM.getSanitizerMetadata()->disableSanitizerForInstruction(SI);
 llvm::FunctionType *FTy =
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D40948: Switch Clang's default C++ language target to C++14

[Filipe Cabecinhas via Phabricator via cfe-commits]

filcab added inline comments.



Comment at: clang/lib/Frontend/CompilerInvocation.cpp:1733
   // The PS4 uses C++11 as the default C++ standard.
-  if (T.isPS4())
-LangStd = LangStandard::lang_gnucxx11;
-  else
-LangStd = LangStandard::lang_gnucxx98;
+  LangStd = LangStandard::lang_gnucxx14;
   break;

t.p.northover wrote:
> filcab wrote:
> > Why are you changing the PS4 default too?
> Paul Robinson indicated that it was feasible back in March: 
> http://lists.llvm.org/pipermail/cfe-dev/2017-March/052986.html. If that's 
> changed I'd be happy to put it back to C++11, but he's one of the main people 
> (rightly) bugging me about this so I'd be slightly surprised.
> 
> I also notice the comment crept back in. Bother.
Sounds good, then. If Paul is happy with that change, I don't see a problem 
there (assuming you do get rid of the comment for good :-) ).

Thank you,

 Filipe


Repository:
  rC Clang

https://reviews.llvm.org/D40948



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

[PATCH] D40948: Switch Clang's default C++ language target to C++14

[Filipe Cabecinhas via Phabricator via cfe-commits]

filcab added inline comments.



Comment at: clang/lib/Frontend/CompilerInvocation.cpp:1733
   // The PS4 uses C++11 as the default C++ standard.
-  if (T.isPS4())
-LangStd = LangStandard::lang_gnucxx11;
-  else
-LangStd = LangStandard::lang_gnucxx98;
+  LangStd = LangStandard::lang_gnucxx14;
   break;

Why are you changing the PS4 default too?


Repository:
  rC Clang

https://reviews.llvm.org/D40948



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

[PATCH] D38354: Fix test by using -target instead of cc1 arguments (c-index-test goes through the driver)

[Filipe Cabecinhas via Phabricator via cfe-commits]

filcab added a comment.

Ping!


https://reviews.llvm.org/D38354



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

[PATCH] D38364: Fix Modules/builtin-import.mm to be able to handle non-Darwin targets

[Filipe Cabecinhas via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rL314524: Fix 
Modules/{builtin-import.mm,umbrella-header-include-builtin.mm} to be able… 
(authored by filcab).

Repository:
  rL LLVM

https://reviews.llvm.org/D38364

Files:
  cfe/trunk/test/Modules/builtin-import.mm
  cfe/trunk/test/Modules/umbrella-header-include-builtin.mm


Index: cfe/trunk/test/Modules/builtin-import.mm
===
--- cfe/trunk/test/Modules/builtin-import.mm
+++ cfe/trunk/test/Modules/builtin-import.mm
@@ -1,7 +1,5 @@
-// REQUIRES: system-darwin
-
 // RUN: rm -rf %t
-// RUN: %clang -cc1 -fsyntax-only -nostdinc++ -isysroot 
%S/Inputs/libc-libcxx/sysroot -isystem 
%S/Inputs/libc-libcxx/sysroot/usr/include/c++/v1 -std=c++11 -fmodules 
-fimplicit-module-maps -fmodules-cache-path=%t -x objective-c++ 
-fmodules-local-submodule-visibility %s
+// RUN: %clang -cc1 -fsyntax-only -nobuiltininc -nostdinc++ -isysroot 
%S/Inputs/libc-libcxx/sysroot -isystem 
%S/Inputs/libc-libcxx/sysroot/usr/include/c++/v1 -isystem 
%S/Inputs/libc-libcxx/sysroot/usr/include -std=c++11 -fmodules 
-fimplicit-module-maps -fmodules-cache-path=%t -x objective-c++ 
-fmodules-local-submodule-visibility %s
 
 #include 
 #include 
Index: cfe/trunk/test/Modules/umbrella-header-include-builtin.mm
===
--- cfe/trunk/test/Modules/umbrella-header-include-builtin.mm
+++ cfe/trunk/test/Modules/umbrella-header-include-builtin.mm
@@ -1,7 +1,5 @@
-// REQUIRES: system-darwin
-
 // RUN: rm -rf %t
-// RUN: %clang -cc1 -fsyntax-only -nostdinc++ -isysroot 
%S/Inputs/libc-libcxx/sysroot -isystem 
%S/Inputs/libc-libcxx/sysroot/usr/include/c++/v1 
-F%S/Inputs/libc-libcxx/sysroot/Frameworks -std=c++11 -fmodules 
-fimplicit-module-maps -fmodules-cache-path=%t -x objective-c++ %s
+// RUN: %clang -cc1 -fsyntax-only -nobuiltininc -nostdinc++ -isysroot 
%S/Inputs/libc-libcxx/sysroot -isystem 
%S/Inputs/libc-libcxx/sysroot/usr/include/c++/v1 -isystem 
%S/Inputs/libc-libcxx/sysroot/usr/include 
-F%S/Inputs/libc-libcxx/sysroot/Frameworks -std=c++11 -fmodules 
-fimplicit-module-maps -fmodules-cache-path=%t -x objective-c++ %s
 
 #include 
 


Index: cfe/trunk/test/Modules/builtin-import.mm
===
--- cfe/trunk/test/Modules/builtin-import.mm
+++ cfe/trunk/test/Modules/builtin-import.mm
@@ -1,7 +1,5 @@
-// REQUIRES: system-darwin
-
 // RUN: rm -rf %t
-// RUN: %clang -cc1 -fsyntax-only -nostdinc++ -isysroot %S/Inputs/libc-libcxx/sysroot -isystem %S/Inputs/libc-libcxx/sysroot/usr/include/c++/v1 -std=c++11 -fmodules -fimplicit-module-maps -fmodules-cache-path=%t -x objective-c++ -fmodules-local-submodule-visibility %s
+// RUN: %clang -cc1 -fsyntax-only -nobuiltininc -nostdinc++ -isysroot %S/Inputs/libc-libcxx/sysroot -isystem %S/Inputs/libc-libcxx/sysroot/usr/include/c++/v1 -isystem %S/Inputs/libc-libcxx/sysroot/usr/include -std=c++11 -fmodules -fimplicit-module-maps -fmodules-cache-path=%t -x objective-c++ -fmodules-local-submodule-visibility %s
 
 #include 
 #include 
Index: cfe/trunk/test/Modules/umbrella-header-include-builtin.mm
===
--- cfe/trunk/test/Modules/umbrella-header-include-builtin.mm
+++ cfe/trunk/test/Modules/umbrella-header-include-builtin.mm
@@ -1,7 +1,5 @@
-// REQUIRES: system-darwin
-
 // RUN: rm -rf %t
-// RUN: %clang -cc1 -fsyntax-only -nostdinc++ -isysroot %S/Inputs/libc-libcxx/sysroot -isystem %S/Inputs/libc-libcxx/sysroot/usr/include/c++/v1 -F%S/Inputs/libc-libcxx/sysroot/Frameworks -std=c++11 -fmodules -fimplicit-module-maps -fmodules-cache-path=%t -x objective-c++ %s
+// RUN: %clang -cc1 -fsyntax-only -nobuiltininc -nostdinc++ -isysroot %S/Inputs/libc-libcxx/sysroot -isystem %S/Inputs/libc-libcxx/sysroot/usr/include/c++/v1 -isystem %S/Inputs/libc-libcxx/sysroot/usr/include -F%S/Inputs/libc-libcxx/sysroot/Frameworks -std=c++11 -fmodules -fimplicit-module-maps -fmodules-cache-path=%t -x objective-c++ %s
 
 #include 
 
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D38364: Fix Modules/builtin-import.mm to be able to handle non-Darwin targets

[Filipe Cabecinhas via Phabricator via cfe-commits]

filcab updated this revision to Diff 117002.
filcab added a comment.

Add umbrella-header-include-builtin.mm too


https://reviews.llvm.org/D38364

Files:
  test/Modules/builtin-import.mm
  test/Modules/umbrella-header-include-builtin.mm


Index: test/Modules/umbrella-header-include-builtin.mm
===
--- test/Modules/umbrella-header-include-builtin.mm
+++ test/Modules/umbrella-header-include-builtin.mm
@@ -1,7 +1,5 @@
-// REQUIRES: system-darwin
-
 // RUN: rm -rf %t
-// RUN: %clang -cc1 -fsyntax-only -nostdinc++ -isysroot 
%S/Inputs/libc-libcxx/sysroot -isystem 
%S/Inputs/libc-libcxx/sysroot/usr/include/c++/v1 
-F%S/Inputs/libc-libcxx/sysroot/Frameworks -std=c++11 -fmodules 
-fimplicit-module-maps -fmodules-cache-path=%t -x objective-c++ %s
+// RUN: %clang -cc1 -fsyntax-only -nobuiltininc -nostdinc++ -isysroot 
%S/Inputs/libc-libcxx/sysroot -isystem 
%S/Inputs/libc-libcxx/sysroot/usr/include/c++/v1 -isystem 
%S/Inputs/libc-libcxx/sysroot/usr/include 
-F%S/Inputs/libc-libcxx/sysroot/Frameworks -std=c++11 -fmodules 
-fimplicit-module-maps -fmodules-cache-path=%t -x objective-c++ %s
 
 #include 
 
Index: test/Modules/builtin-import.mm
===
--- test/Modules/builtin-import.mm
+++ test/Modules/builtin-import.mm
@@ -1,7 +1,5 @@
-// REQUIRES: system-darwin
-
 // RUN: rm -rf %t
-// RUN: %clang -cc1 -fsyntax-only -nostdinc++ -isysroot 
%S/Inputs/libc-libcxx/sysroot -isystem 
%S/Inputs/libc-libcxx/sysroot/usr/include/c++/v1 -std=c++11 -fmodules 
-fimplicit-module-maps -fmodules-cache-path=%t -x objective-c++ 
-fmodules-local-submodule-visibility %s
+// RUN: %clang -cc1 -fsyntax-only -nobuiltininc -nostdinc++ -isysroot 
%S/Inputs/libc-libcxx/sysroot -isystem 
%S/Inputs/libc-libcxx/sysroot/usr/include/c++/v1 -isystem 
%S/Inputs/libc-libcxx/sysroot/usr/include -std=c++11 -fmodules 
-fimplicit-module-maps -fmodules-cache-path=%t -x objective-c++ 
-fmodules-local-submodule-visibility %s
 
 #include 
 #include 


Index: test/Modules/umbrella-header-include-builtin.mm
===
--- test/Modules/umbrella-header-include-builtin.mm
+++ test/Modules/umbrella-header-include-builtin.mm
@@ -1,7 +1,5 @@
-// REQUIRES: system-darwin
-
 // RUN: rm -rf %t
-// RUN: %clang -cc1 -fsyntax-only -nostdinc++ -isysroot %S/Inputs/libc-libcxx/sysroot -isystem %S/Inputs/libc-libcxx/sysroot/usr/include/c++/v1 -F%S/Inputs/libc-libcxx/sysroot/Frameworks -std=c++11 -fmodules -fimplicit-module-maps -fmodules-cache-path=%t -x objective-c++ %s
+// RUN: %clang -cc1 -fsyntax-only -nobuiltininc -nostdinc++ -isysroot %S/Inputs/libc-libcxx/sysroot -isystem %S/Inputs/libc-libcxx/sysroot/usr/include/c++/v1 -isystem %S/Inputs/libc-libcxx/sysroot/usr/include -F%S/Inputs/libc-libcxx/sysroot/Frameworks -std=c++11 -fmodules -fimplicit-module-maps -fmodules-cache-path=%t -x objective-c++ %s
 
 #include 
 
Index: test/Modules/builtin-import.mm
===
--- test/Modules/builtin-import.mm
+++ test/Modules/builtin-import.mm
@@ -1,7 +1,5 @@
-// REQUIRES: system-darwin
-
 // RUN: rm -rf %t
-// RUN: %clang -cc1 -fsyntax-only -nostdinc++ -isysroot %S/Inputs/libc-libcxx/sysroot -isystem %S/Inputs/libc-libcxx/sysroot/usr/include/c++/v1 -std=c++11 -fmodules -fimplicit-module-maps -fmodules-cache-path=%t -x objective-c++ -fmodules-local-submodule-visibility %s
+// RUN: %clang -cc1 -fsyntax-only -nobuiltininc -nostdinc++ -isysroot %S/Inputs/libc-libcxx/sysroot -isystem %S/Inputs/libc-libcxx/sysroot/usr/include/c++/v1 -isystem %S/Inputs/libc-libcxx/sysroot/usr/include -std=c++11 -fmodules -fimplicit-module-maps -fmodules-cache-path=%t -x objective-c++ -fmodules-local-submodule-visibility %s
 
 #include 
 #include 
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D38364: Fix Modules/builtin-import.mm to be able to handle non-Darwin targets

[Filipe Cabecinhas via Phabricator via cfe-commits]

filcab created this revision.

Also makes it pass on Darwin, if the default target triple is a Linux triple.


https://reviews.llvm.org/D38364

Files:
  test/Modules/builtin-import.mm


Index: test/Modules/builtin-import.mm
===
--- test/Modules/builtin-import.mm
+++ test/Modules/builtin-import.mm
@@ -1,7 +1,5 @@
-// REQUIRES: system-darwin
-
 // RUN: rm -rf %t
-// RUN: %clang -cc1 -fsyntax-only -nostdinc++ -isysroot 
%S/Inputs/libc-libcxx/sysroot -isystem 
%S/Inputs/libc-libcxx/sysroot/usr/include/c++/v1 -std=c++11 -fmodules 
-fimplicit-module-maps -fmodules-cache-path=%t -x objective-c++ 
-fmodules-local-submodule-visibility %s
+// RUN: %clang -cc1 -fsyntax-only -nobuiltininc -nostdinc++ -isysroot 
%S/Inputs/libc-libcxx/sysroot -isystem 
%S/Inputs/libc-libcxx/sysroot/usr/include/c++/v1 -isystem 
%S/Inputs/libc-libcxx/sysroot/usr/include -std=c++11 -fmodules 
-fimplicit-module-maps -fmodules-cache-path=%t -x objective-c++ 
-fmodules-local-submodule-visibility %s
 
 #include 
 #include 


Index: test/Modules/builtin-import.mm
===
--- test/Modules/builtin-import.mm
+++ test/Modules/builtin-import.mm
@@ -1,7 +1,5 @@
-// REQUIRES: system-darwin
-
 // RUN: rm -rf %t
-// RUN: %clang -cc1 -fsyntax-only -nostdinc++ -isysroot %S/Inputs/libc-libcxx/sysroot -isystem %S/Inputs/libc-libcxx/sysroot/usr/include/c++/v1 -std=c++11 -fmodules -fimplicit-module-maps -fmodules-cache-path=%t -x objective-c++ -fmodules-local-submodule-visibility %s
+// RUN: %clang -cc1 -fsyntax-only -nobuiltininc -nostdinc++ -isysroot %S/Inputs/libc-libcxx/sysroot -isystem %S/Inputs/libc-libcxx/sysroot/usr/include/c++/v1 -isystem %S/Inputs/libc-libcxx/sysroot/usr/include -std=c++11 -fmodules -fimplicit-module-maps -fmodules-cache-path=%t -x objective-c++ -fmodules-local-submodule-visibility %s
 
 #include 
 #include 
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D38354: Fix test by using -target instead of cc1 arguments (c-index-test goes through the driver)

[Filipe Cabecinhas via Phabricator via cfe-commits]

filcab created this revision.

If LLVM_DEFAULT_TARGET_TRIPLE is a non-darwin triple, the file might
fail. This might allow us to take out the FIXME and REQUIRES lines, but I'd
rather let the bots double-check that the test is ok first.


https://reviews.llvm.org/D38354

Files:
  test/Index/pch-from-libclang.c


Index: test/Index/pch-from-libclang.c
===
--- test/Index/pch-from-libclang.c
+++ test/Index/pch-from-libclang.c
@@ -4,10 +4,10 @@
 // REQUIRES: system-darwin
 
 // RUN: %clang_cc1 -fsyntax-only %s -verify
-// RUN: c-index-test -write-pch %t.h.pch %s -fmodules 
-fmodules-cache-path=%t.mcp -Xclang -triple -Xclang x86_64-apple-darwin
-// RUN: %clang -fsyntax-only -include %t.h %s -Xclang -verify -fmodules 
-fmodules-cache-path=%t.mcp -Xclang -detailed-preprocessing-record -Xclang 
-triple -Xclang x86_64-apple-darwin -Xclang -fallow-pch-with-compiler-errors
-// RUN: %clang -x c-header %s -o %t.clang.h.pch -fmodules 
-fmodules-cache-path=%t.mcp -Xclang -detailed-preprocessing-record -Xclang 
-triple -Xclang x86_64-apple-darwin -Xclang -fallow-pch-with-compiler-errors 
-Xclang -verify
-// RUN: c-index-test -test-load-source local %s -include %t.clang.h -fmodules 
-fmodules-cache-path=%t.mcp -Xclang -triple -Xclang x86_64-apple-darwin | 
FileCheck %s
+// RUN: c-index-test -write-pch %t.h.pch %s -fmodules 
-fmodules-cache-path=%t.mcp -target x86_64-apple-darwin
+// RUN: %clang -fsyntax-only -include %t.h %s -Xclang -verify -fmodules 
-fmodules-cache-path=%t.mcp -Xclang -detailed-preprocessing-record -target 
x86_64-apple-darwin -Xclang -fallow-pch-with-compiler-errors
+// RUN: %clang -x c-header %s -o %t.clang.h.pch -fmodules 
-fmodules-cache-path=%t.mcp -Xclang -detailed-preprocessing-record -target 
x86_64-apple-darwin -Xclang -fallow-pch-with-compiler-errors -Xclang -verify
+// RUN: c-index-test -test-load-source local %s -include %t.clang.h -fmodules 
-fmodules-cache-path=%t.mcp -target x86_64-apple-darwin | FileCheck %s
 
 #ifndef HEADER
 #define HEADER


Index: test/Index/pch-from-libclang.c
===
--- test/Index/pch-from-libclang.c
+++ test/Index/pch-from-libclang.c
@@ -4,10 +4,10 @@
 // REQUIRES: system-darwin
 
 // RUN: %clang_cc1 -fsyntax-only %s -verify
-// RUN: c-index-test -write-pch %t.h.pch %s -fmodules -fmodules-cache-path=%t.mcp -Xclang -triple -Xclang x86_64-apple-darwin
-// RUN: %clang -fsyntax-only -include %t.h %s -Xclang -verify -fmodules -fmodules-cache-path=%t.mcp -Xclang -detailed-preprocessing-record -Xclang -triple -Xclang x86_64-apple-darwin -Xclang -fallow-pch-with-compiler-errors
-// RUN: %clang -x c-header %s -o %t.clang.h.pch -fmodules -fmodules-cache-path=%t.mcp -Xclang -detailed-preprocessing-record -Xclang -triple -Xclang x86_64-apple-darwin -Xclang -fallow-pch-with-compiler-errors -Xclang -verify
-// RUN: c-index-test -test-load-source local %s -include %t.clang.h -fmodules -fmodules-cache-path=%t.mcp -Xclang -triple -Xclang x86_64-apple-darwin | FileCheck %s
+// RUN: c-index-test -write-pch %t.h.pch %s -fmodules -fmodules-cache-path=%t.mcp -target x86_64-apple-darwin
+// RUN: %clang -fsyntax-only -include %t.h %s -Xclang -verify -fmodules -fmodules-cache-path=%t.mcp -Xclang -detailed-preprocessing-record -target x86_64-apple-darwin -Xclang -fallow-pch-with-compiler-errors
+// RUN: %clang -x c-header %s -o %t.clang.h.pch -fmodules -fmodules-cache-path=%t.mcp -Xclang -detailed-preprocessing-record -target x86_64-apple-darwin -Xclang -fallow-pch-with-compiler-errors -Xclang -verify
+// RUN: c-index-test -test-load-source local %s -include %t.clang.h -fmodules -fmodules-cache-path=%t.mcp -target x86_64-apple-darwin | FileCheck %s
 
 #ifndef HEADER
 #define HEADER
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D32047: [Driver] Add support for default UBSan blacklists

[Filipe Cabecinhas via Phabricator via cfe-commits]

filcab added a comment.

Should we simply not have `ubsan_blacklist.txt` if it's empty?

Otherwise LGTM


https://reviews.llvm.org/D32047



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

[PATCH] D34299: [ubsan] Improve diagnostics for return value checks (clang)

[Filipe Cabecinhas via Phabricator via cfe-commits]

filcab added a comment.

In https://reviews.llvm.org/D34299#788427, @vsk wrote:

> I hope I've cleared this up, but: we need to store the source location 
> constant _somewhere_, before we emit the return value check. That's because 
> we can't infer which return location to use at compile time.


Yes, sorry about that. I was thinking about the code the wrong way around. I'm 
ok with this patch and how it got in. Sorry for the delay in replying.


Repository:
  rL LLVM

https://reviews.llvm.org/D34299



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

[PATCH] D34299: [ubsan] Improve diagnostics for return value checks (clang)

[Filipe Cabecinhas via Phabricator via cfe-commits]

filcab added a comment.

In https://reviews.llvm.org/D34299#788152, @vsk wrote:

> The source locations aren't constants. The ubsan runtime uses a bit inside of 
> source location structures as a flag. When an issue is diagnosed at a 
> particular source location, that bit is atomically set. This is how ubsan 
> implements issue deduplication.


It's still an `llvm::Constant`. Just like in StaticData, in line 2966.
Basically, I don't see why we need to add the store/load and an additional 
indirection, since the pointer is constant, and we can just emit the static 
data as before.
We're already doing `Data->Loc.acquire();` for the current version (and all the 
other checks).

>> I'd also like to have some asserts and explicit resets to nullptr after use 
>> on the ReturnLocation variable, if possible.
> 
> Resetting Address fields in CodeGenFunction doesn't appear to be an 
> established practice. Could you explain what this would be in aid of?

It would be a sanity check and help with code reading/keeping in mind the 
lifetime of the information. I'm ok with that happening only on `!NDEBUG` 
builds.

Reading the code, I don't know if a `ReturnLocation` might end up being used 
for more than one checks. If it's supposed to be a different one per check, etc.
If it's only supposed to be used once, I'd rather set it to `nullptr` right 
after use (at least when `!NDEBUG`), and `assert(!ReturnLocation)` before 
setting. It's not a big deal, but would help with making sense of the flow of 
information when debugging.


https://reviews.llvm.org/D34299



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

[PATCH] D34299: [ubsan] Improve diagnostics for return value checks (clang)

[Filipe Cabecinhas via Phabricator via cfe-commits]

filcab added a comment.

Splitting the attrloc from the useloc might make sense since we would be able 
to emit attrloc just once. But I don't see why we need to store/load those 
pointers in runtime instead of just caching the `Constant*` in 
`CodeGenFunction`.
I'd also like to have some asserts and explicit resets to `nullptr` after use 
on the `ReturnLocation` variable, if possible.




Comment at: lib/CodeGen/CGStmt.cpp:1035
+assert(ReturnLocation.isValid() && "No valid return location");
+Builder.CreateStore(Builder.CreateBitCast(SLocPtr, Int8PtrTy),
+ReturnLocation);

Can't you just keep the `Constant*` around and use it later for the static 
data? Instead of creating a global var and have runtime store/load?



Comment at: lib/CodeGen/CodeGenFunction.h:1412
+  /// source location for diagnostics.
+  Address ReturnLocation = Address::invalid();
+

Maybe `CurrentReturnLocation`?


https://reviews.llvm.org/D34299



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

[PATCH] D33305: [ubsan] Add a check for pointer overflow UB

[Filipe Cabecinhas via Phabricator via cfe-commits]

filcab added inline comments.



Comment at: lib/CodeGen/CGExprScalar.cpp:3854
+   const Twine ) {
+  Value *GEPVal = Builder.CreateInBoundsGEP(Ptr, IdxList, Name);
+

You're creating the GEP first (possibly triggering UB), and then checking 
("after" UB). Shouldn't you put the checking instructions before the GEP?



Comment at: lib/CodeGen/CGExprScalar.cpp:3948
+return GEPVal;
+
+  // Now that we've computed the total offset, add it to the base pointer (with

Do we want an extra test for `TotalOffset` being a constant + not overflowing? 
(Not strictly need, but you've been working on avoiding __ubsan() calls :-) )


https://reviews.llvm.org/D33305



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

[PATCH] D32199: [TBAASan] A TBAA Sanitizer (Clang)

[Filipe Cabecinhas via Phabricator via cfe-commits]

filcab added a comment.

Missing a `sanitizer-ld.c` test for freebsd.




Comment at: include/clang/Basic/Attr.td:1849
+   GNU<"no_sanitize_memory">,
+   GNU<"no_sanitize_tbaa">];
   let Subjects = SubjectList<[Function, GlobalVar], ErrorDiag,

Shouldn't you be extending the no_sanitize attribute instead, as it says in the 
comment?



Comment at: lib/CodeGen/CodeGenModule.cpp:130
   if (LangOpts.Sanitize.has(SanitizerKind::Thread) ||
+  LangOpts.Sanitize.has(SanitizerKind::TBAA) ||
   (!CodeGenOpts.RelaxedAliasing && CodeGenOpts.OptimizationLevel > 0))

`hasOneOf(SanitizerKind::Thread | SanitizerKind::TBAA)`?



Comment at: lib/CodeGen/CodeGenTBAA.cpp:96
+  if (!Features.Sanitize.has(SanitizerKind::TBAA) &&
+  (CodeGenOpts.OptimizationLevel == 0 || CodeGenOpts.RelaxedAliasing))
 return nullptr;

Interesting that TSan needs TBAA (per the comment in the chunk above), but is 
not in this condition.


https://reviews.llvm.org/D32199



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

[PATCH] D30762: [ubsan] Add a nullability sanitizer

[Filipe Cabecinhas via Phabricator via cfe-commits]

filcab added a comment.

Please make the tests tighter using `CHECK-NEXT` when possible. Much easier if 
later anyone needs to debug differences in IR.




Comment at: docs/UndefinedBehaviorSanitizer.rst:102
+ violating nullability rules does not result in undefined behavior, it
+ is often unintentional, so UBSan offers to catch it.
   -  ``-fsanitize=object-size``: An attempt to potentially use bytes which

This is weird. Please just document each of the `nullability-*` in isolation. 
None of the other flags is documenting more than one flag per bullet point, and 
I think splitting them up here is easy. It also makes it easier to read.
You already added the `nullability` group below, which mentions it adds the 
three checks.




Comment at: docs/UndefinedBehaviorSanitizer.rst:141
   -  ``-fsanitize=undefined``: All of the checks listed above other than
- ``unsigned-integer-overflow``.
+ ``unsigned-integer-overflow`` and ``nullability``.
   -  ``-fsanitize=undefined-trap``: Deprecated alias of

```... and the ``nullability`` group.```



Comment at: test/CodeGenObjC/ubsan-nullability.m:114
+  // CHECK: [[NONULL]]:
+  // CHECK: ret i32*
+}

`CHECK-NEXT`?


https://reviews.llvm.org/D30762



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

[PATCH] D30423: [ubsan] Detect UB loads from bitfields

[Filipe Cabecinhas via Phabricator via cfe-commits]

filcab added a comment.

LGTM since my issue is only an issue on ObjC platforms and it seems those are 
the semantics it should have there.




Comment at: test/CodeGenObjC/ubsan-bool.m:26
+  // OBJC:  [[ICMP:%.*]] = icmp ule i8 [[ASHR]], 1, !nosanitize
+  // OBJC: call void @__ubsan_handle_load_invalid_value
+

vsk wrote:
> filcab wrote:
> > jroelofs wrote:
> > > vsk wrote:
> > > > jroelofs wrote:
> > > > > vsk wrote:
> > > > > > arphaman wrote:
> > > > > > > Is it possible to avoid the check here since the bitfield is just 
> > > > > > > one bit wide?
> > > > > > No, a user could do:
> > > > > > 
> > > > > > ```
> > > > > > struct S1 s;
> > > > > > s.b1 = 1;
> > > > > > if (s.b1 == 1) // evaluates to false, s.b1 is negative
> > > > > > ```
> > > > > > 
> > > > > > With this patch we get:
> > > > > > ```
> > > > > > runtime error: load of value -1, which is not a valid value for 
> > > > > > type 'BOOL' (aka 'signed char')
> > > > > > ```
> > > > > What if BOOL is an `unsigned char`, or a `char` that's not signed?
> > > > Good point, we don't need to emit the range check if the bitfield is an 
> > > > unsigned, 1-bit wide BOOL. Would you be OK with me tackling that in a 
> > > > follow-up patch?
> > > No problem, sounds good to me.
> > I don't like generating an error here.
> > -1 is a perfectly valid BOOL (signed char) value and it's a perfectly valid 
> > value to have in a (signed) 1-wide bitfield.
> True/false should be the only values loaded from a boolean. Since we made 
> ubsan stricter about the range of BOOL (r289290), the check has caught a lot 
> of bugs in our software. Specifically, it's helped root out buggy BOOL 
> initialization and portability problems (the signedness of BOOL is not 
> consistent on Apple platforms). There have been no false positives so far.
> 
> Is this an issue you think needs to be addressed in this patch?
It looks weird that we would disallow using a 1-bit (signed) bitfield for 
storing `BOOL`. But since this will only be a problem on obj-c, I don't think 
it will be a problem in general. If you're saying those are the semantics, then 
I'm ok with it.
P.S: If it documented that the only possible values for `BOOL` are `YES` and 
`NO` (assuming they are `1` and `0`)?
If so, I wonder if it's planned to document that you can't store a `YES` on a 
`BOOL` bitfield with a width of 1, since it looks like a weird case.


https://reviews.llvm.org/D30423



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

[PATCH] D30729: [ubsan] Skip range checks for width-limited values

[Filipe Cabecinhas via Phabricator via cfe-commits]

filcab added inline comments.



Comment at: test/CodeGenCXX/ubsan-bitfields.cpp:39
+  // CHECK-NOT: !nosanitize
+  return s->e3;
+}

Add checks/check-nots to make sure the thing you don't want isn't emitted, not 
just `!nosanitize`
The checks help document what you're trying to get in each test case.
Here I'd prefer to have a `CHECK-NOT: __ubsan_handle_load_invalid_value` than a 
check-not for `!nosanitize`, since checking for the handler call is more 
explicit.



https://reviews.llvm.org/D30729



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

[PATCH] D30423: [ubsan] Detect UB loads from bitfields

[Filipe Cabecinhas via Phabricator via cfe-commits]

filcab added inline comments.



Comment at: test/CodeGenObjC/ubsan-bool.m:26
+  // OBJC:  [[ICMP:%.*]] = icmp ule i8 [[ASHR]], 1, !nosanitize
+  // OBJC: call void @__ubsan_handle_load_invalid_value
+

jroelofs wrote:
> vsk wrote:
> > jroelofs wrote:
> > > vsk wrote:
> > > > arphaman wrote:
> > > > > Is it possible to avoid the check here since the bitfield is just one 
> > > > > bit wide?
> > > > No, a user could do:
> > > > 
> > > > ```
> > > > struct S1 s;
> > > > s.b1 = 1;
> > > > if (s.b1 == 1) // evaluates to false, s.b1 is negative
> > > > ```
> > > > 
> > > > With this patch we get:
> > > > ```
> > > > runtime error: load of value -1, which is not a valid value for type 
> > > > 'BOOL' (aka 'signed char')
> > > > ```
> > > What if BOOL is an `unsigned char`, or a `char` that's not signed?
> > Good point, we don't need to emit the range check if the bitfield is an 
> > unsigned, 1-bit wide BOOL. Would you be OK with me tackling that in a 
> > follow-up patch?
> No problem, sounds good to me.
I don't like generating an error here.
-1 is a perfectly valid BOOL (signed char) value and it's a perfectly valid 
value to have in a (signed) 1-wide bitfield.



Comment at: test/CodeGenObjC/ubsan-bool.m:50
+// OBJC: [[SHL:%.*]] = shl i8 [[LOAD]], 7
+// OBJC: [[ASHR:%.*]] = ashr i8 [[SHL]], 7
+// OBJC: icmp ule i8 [[ASHR]], 1, !nosanitize

jroelofs wrote:
> hmmm. just occurred to me that this value is always going to be 0xff or 0x0, 
> so it might be useful if there were also a frontend warning that complains 
> about signed bitfield bools (maybe something useful for another patch).
I'm more ok with this, assuming it makes sense in ObjC.


https://reviews.llvm.org/D30423



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

[PATCH] D30285: [ubsan] Don't check alignment if the alignment is 1

[Filipe Cabecinhas via Phabricator via cfe-commits]

filcab accepted this revision.
filcab added a comment.
This revision is now accepted and ready to land.

LGTM


https://reviews.llvm.org/D30285



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

[PATCH] D29369: [ubsan] Omit superflous overflow checks for promoted arithmetic (PR20193)

[Filipe Cabecinhas via Phabricator via cfe-commits]

filcab added a comment.

Minor nits, now.
LGTM, but having someone more familiar with clang chime in would be great.




Comment at: lib/CodeGen/CGExprScalar.cpp:1700
   case LangOptions::SOB_Trapping:
+if (getUnwidenedIntegerType(CGF.getContext(), E->getSubExpr()).hasValue())
+  return Builder.CreateNSWAdd(InVal, Amount, Name);

Maybe a helper `IsWidenedIntegerOp(...)` (or `IsOpWiderThanBaseType`or 
something) would make this (and others, like the first return of 
`getUnwidenedIntegerType`) easier to read?



Comment at: test/CodeGen/ubsan-promoted-arith.cpp:90
+
+// Note: -INT_MIN / -1 can overflow.
+//

Extra `-`. `INT_MIN/-1` is what you want. You already have a test above for 
`-INT_MIN` (which would overflow before the division.


https://reviews.llvm.org/D29369



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

[PATCH] D29369: [ubsan] Omit superflous overflow checks for promoted arithmetic (PR20193)

[Filipe Cabecinhas via Phabricator via cfe-commits]

filcab added a comment.

Why the switch to `if` instead of a fully-covered switch/case?

In https://reviews.llvm.org/D29369#664426, @vsk wrote:

> In https://reviews.llvm.org/D29369#664366, @regehr wrote:
>
> > Out of curiosity, how many of these superfluous checks are not subsequently 
> > eliminated by InstCombine?
>
>
> I don't have numbers from a benchmark prepped. Here's what we get with the 
> 'ubsan-promoted-arith.cpp' test case from this patch:
>
> | Setup| # of overflow checks |
> | unpatched, -O0   | 22   |
> | unpatched, -O0 + instcombine | 7|
> | patched, -O0 | 8|
> | patched, -O0 + instcombine   | 7|
>
> (There's a difference between the "patched, -O0" setup and the "patched, -O0 
> + instcombine" setup because llvm figures out that the symbol 'a' is 0, and 
> gets rid of an addition that way.)
>
> At least for us, this patch is still worthwhile, because our use case is `-O0 
> -fsanitized=undefined`. Also, this makes less work for instcombine, but I 
> haven't measured the compile-time effect.


Probably running mem2reg and others before instcombine would make it elide more 
checks. But if you're using -O0 anyway, I guess this would help anyway.




Comment at: test/CodeGen/ubsan-promoted-arith.cpp:56
+
+// Note: -SHRT_MIN * -SHRT_MIN can overflow.
+//

Nit: Maybe `USHRT_MAX * USHRT_MAX` is more understandable?



Comment at: test/CodeGen/ubsan-promoted-arith.cpp:99
+// CHECK-LABEL: define signext i8 @_Z4rem3
+// rdar30301609: ubsan_handle_divrem_overflow
+char rem3(int i, char c) { return i % c; }

Maybe put the rdar ID next to the FIXME? Like this it looks like you might have 
written that instead of `CHECK` by mistake.


https://reviews.llvm.org/D29369



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

[PATCH] D28242: [ubsan] Minimize size of data for type_mismatch (Redo of D19667)

[Filipe Cabecinhas via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rL291236: [ubsan] Minimize size of data for type_mismatch 
(Redo of D19667) (authored by filcab).

Changed prior to commit:
  https://reviews.llvm.org/D28242?vs=82913=83362#toc

Repository:
  rL LLVM

https://reviews.llvm.org/D28242

Files:
  cfe/trunk/lib/CodeGen/CGExpr.cpp
  cfe/trunk/lib/CodeGen/CodeGenFunction.h
  cfe/trunk/test/CodeGen/catch-undef-behavior.c
  cfe/trunk/test/CodeGen/sanitize-recover.c
  cfe/trunk/test/CodeGenCXX/ubsan-vtable-checks.cpp

Index: cfe/trunk/test/CodeGenCXX/ubsan-vtable-checks.cpp
===
--- cfe/trunk/test/CodeGenCXX/ubsan-vtable-checks.cpp
+++ cfe/trunk/test/CodeGenCXX/ubsan-vtable-checks.cpp
@@ -21,7 +21,7 @@
   // CHECK-NULL-NOT: load {{.*}} (%struct.T*{{.*}})**, {{.*}} (%struct.T*{{.*}})***
   // CHECK-NULL: [[UBSAN_CMP_RES:%[0-9]+]] = icmp ne %struct.T* %{{[_a-z0-9]+}}, null
   // CHECK-NULL-NEXT: br i1 [[UBSAN_CMP_RES]], label %{{.*}}, label %{{.*}}
-  // CHECK-NULL: call void @__ubsan_handle_type_mismatch_abort
+  // CHECK-NULL: call void @__ubsan_handle_type_mismatch_v1_abort
   // Second, we check that vtable is actually loaded once the type check is done.
   // CHECK-NULL: load {{.*}} (%struct.T*{{.*}})**, {{.*}} (%struct.T*{{.*}})***
   return t->v();
Index: cfe/trunk/test/CodeGen/sanitize-recover.c
===
--- cfe/trunk/test/CodeGen/sanitize-recover.c
+++ cfe/trunk/test/CodeGen/sanitize-recover.c
@@ -33,7 +33,7 @@
   // PARTIAL:  br i1 %[[CHECK012]], {{.*}} !prof ![[WEIGHT_MD:.*]], !nosanitize
 
   // PARTIAL:  br i1 %[[CHECK02]], {{.*}}
-  // PARTIAL:  call void @__ubsan_handle_type_mismatch_abort(
+  // PARTIAL:  call void @__ubsan_handle_type_mismatch_v1_abort(
   // PARTIAL-NEXT: unreachable
-  // PARTIAL:  call void @__ubsan_handle_type_mismatch(
+  // PARTIAL:  call void @__ubsan_handle_type_mismatch_v1(
 }
Index: cfe/trunk/test/CodeGen/catch-undef-behavior.c
===
--- cfe/trunk/test/CodeGen/catch-undef-behavior.c
+++ cfe/trunk/test/CodeGen/catch-undef-behavior.c
@@ -6,16 +6,16 @@
 // CHECK-UBSAN: @[[INT:.*]] = private unnamed_addr constant { i16, i16, [6 x i8] } { i16 0, i16 11, [6 x i8] c"'int'\00" }
 
 // FIXME: When we only emit each type once, use [[INT]] more below.
-// CHECK-UBSAN: @[[LINE_100:.*]] = private unnamed_addr global {{.*}}, i32 100, i32 5 {{.*}} @[[INT]], i64 4, i8 1
-// CHECK-UBSAN: @[[LINE_200:.*]] = {{.*}}, i32 200, i32 10 {{.*}}, i64 4, i8 0
+// CHECK-UBSAN: @[[LINE_100:.*]] = private unnamed_addr global {{.*}}, i32 100, i32 5 {{.*}} @[[INT]], i8 2, i8 1
+// CHECK-UBSAN: @[[LINE_200:.*]] = {{.*}}, i32 200, i32 10 {{.*}}, i8 2, i8 0
 // CHECK-UBSAN: @[[LINE_300:.*]] = {{.*}}, i32 300, i32 12 {{.*}} @{{.*}}, {{.*}} @{{.*}}
 // CHECK-UBSAN: @[[LINE_400:.*]] = {{.*}}, i32 400, i32 12 {{.*}} @{{.*}}, {{.*}} @{{.*}}
-// CHECK-UBSAN: @[[LINE_500:.*]] = {{.*}}, i32 500, i32 10 {{.*}} @{{.*}}, i64 4, i8 0 }
-// CHECK-UBSAN: @[[LINE_600:.*]] = {{.*}}, i32 600, i32 3 {{.*}} @{{.*}}, i64 4, i8 1 }
+// CHECK-UBSAN: @[[LINE_500:.*]] = {{.*}}, i32 500, i32 10 {{.*}} @{{.*}}, i8 2, i8 0 }
+// CHECK-UBSAN: @[[LINE_600:.*]] = {{.*}}, i32 600, i32 3 {{.*}} @{{.*}}, i8 2, i8 1 }
 
 // CHECK-UBSAN: @[[STRUCT_S:.*]] = private unnamed_addr constant { i16, i16, [11 x i8] } { i16 -1, i16 0, [11 x i8] c"'struct S'\00" }
 
-// CHECK-UBSAN: @[[LINE_700:.*]] = {{.*}}, i32 700, i32 14 {{.*}} @[[STRUCT_S]], i64 4, i8 3 }
+// CHECK-UBSAN: @[[LINE_700:.*]] = {{.*}}, i32 700, i32 14 {{.*}} @[[STRUCT_S]], i8 2, i8 3 }
 // CHECK-UBSAN: @[[LINE_800:.*]] = {{.*}}, i32 800, i32 12 {{.*}} @{{.*}} }
 // CHECK-UBSAN: @[[LINE_900:.*]] = {{.*}}, i32 900, i32 11 {{.*}} @{{.*}} }
 // CHECK-UBSAN: @[[LINE_1000:.*]] = {{.*}}, i32 1000, i32 10 {{.*}} @{{.*}} }
@@ -54,15 +54,15 @@
   // CHECK-TRAP:  br i1 %[[OK]], {{.*}}
 
   // CHECK-UBSAN:  %[[ARG:.*]] = ptrtoint {{.*}} %[[PTR]] to i64
-  // CHECK-UBSAN-NEXT: call void @__ubsan_handle_type_mismatch(i8* bitcast ({{.*}} @[[LINE_100]] to i8*), i64 %[[ARG]])
+  // CHECK-UBSAN-NEXT: call void @__ubsan_handle_type_mismatch_v1(i8* bitcast ({{.*}} @[[LINE_100]] to i8*), i64 %[[ARG]])
 
   // CHECK-TRAP:  call void @llvm.trap() [[NR_NUW:#[0-9]+]]
   // CHECK-TRAP-NEXT: unreachable
 
   // With -fsanitize=null, only perform the null check.
   // CHECK-NULL: %[[NULL:.*]] = icmp ne {{.*}}, null
   // CHECK-NULL: br i1 %[[NULL]]
-  // CHECK-NULL: call void @__ubsan_handle_type_mismatch(i8* bitcast ({{.*}} @[[LINE_100]] to i8*), i64 %{{.*}})
+  // CHECK-NULL: call void @__ubsan_handle_type_mismatch_v1(i8* bitcast ({{.*}} @[[LINE_100]] to i8*), i64 %{{.*}})
 #line 100
   u.i=1;
 }
@@ -77,7 +77,7 @@
   // CHECK-COMMON-NEXT: icmp eq i64 %[[MISALIGN]], 0
 
   // CHECK-UBSAN:  %[[ARG:.*]] = ptrtoint
-  // CHECK-UBSAN-NEXT: call void 

[PATCH] D28242: [ubsan] Minimize size of data for type_mismatch (Redo of D19667)

[Filipe Cabecinhas via Phabricator via cfe-commits]

filcab accepted this revision.
filcab added a reviewer: filcab.
filcab added a comment.
This revision is now accepted and ready to land.

Since Richard has already LGTMed the previous version, and this is a trivial 
change, I'll go ahead with committing.


https://reviews.llvm.org/D28242



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

[PATCH] D19667: [ubsan] Minimize size of data for type_mismatch

[Filipe Cabecinhas via Phabricator via cfe-commits]

filcab abandoned this revision.
filcab added a comment.

I will post a new patch, using the features from 
https://reviews.llvm.org/rL289444.


https://reviews.llvm.org/D19667



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

[PATCH] D21695: [clang] Version support for UBSan handlers

[Filipe Cabecinhas via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rL289444: [clang] Version support for UBSan handlers (authored 
by filcab).

Changed prior to commit:
  https://reviews.llvm.org/D21695?vs=61817=81089#toc

Repository:
  rL LLVM

https://reviews.llvm.org/D21695

Files:
  cfe/trunk/lib/CodeGen/CGBuiltin.cpp
  cfe/trunk/lib/CodeGen/CGCall.cpp
  cfe/trunk/lib/CodeGen/CGClass.cpp
  cfe/trunk/lib/CodeGen/CGExpr.cpp
  cfe/trunk/lib/CodeGen/CGExprScalar.cpp
  cfe/trunk/lib/CodeGen/CodeGenFunction.cpp
  cfe/trunk/lib/CodeGen/CodeGenFunction.h

Index: cfe/trunk/lib/CodeGen/CGExpr.cpp
===
--- cfe/trunk/lib/CodeGen/CGExpr.cpp
+++ cfe/trunk/lib/CodeGen/CGExpr.cpp
@@ -608,7 +608,7 @@
   llvm::ConstantInt::get(SizeTy, AlignVal),
   llvm::ConstantInt::get(Int8Ty, TCK)
 };
-EmitCheck(Checks, "type_mismatch", StaticData, Ptr);
+EmitCheck(Checks, SanitizerHandler::TypeMismatch, StaticData, Ptr);
   }
 
   // If possible, check that the vptr indicates that there is a subobject of
@@ -676,7 +676,8 @@
   };
   llvm::Value *DynamicData[] = { Ptr, Hash };
   EmitCheck(std::make_pair(EqualHash, SanitizerKind::Vptr),
-"dynamic_type_cache_miss", StaticData, DynamicData);
+SanitizerHandler::DynamicTypeCacheMiss, StaticData,
+DynamicData);
 }
   }
 
@@ -766,8 +767,8 @@
   };
   llvm::Value *Check = Accessed ? Builder.CreateICmpULT(IndexVal, BoundVal)
 : Builder.CreateICmpULE(IndexVal, BoundVal);
-  EmitCheck(std::make_pair(Check, SanitizerKind::ArrayBounds), "out_of_bounds",
-StaticData, Index);
+  EmitCheck(std::make_pair(Check, SanitizerKind::ArrayBounds),
+SanitizerHandler::OutOfBounds, StaticData, Index);
 }
 
 
@@ -1339,8 +1340,8 @@
 EmitCheckTypeDescriptor(Ty)
   };
   SanitizerMask Kind = NeedsEnumCheck ? SanitizerKind::Enum : SanitizerKind::Bool;
-  EmitCheck(std::make_pair(Check, Kind), "load_invalid_value", StaticArgs,
-EmitCheckValue(Load));
+  EmitCheck(std::make_pair(Check, Kind), SanitizerHandler::LoadInvalidValue,
+StaticArgs, EmitCheckValue(Load));
 }
   } else if (CGM.getCodeGenOpts().OptimizationLevel > 0)
 if (llvm::MDNode *RangeInfo = getRangeForLoadFromType(Ty))
@@ -2500,17 +2501,35 @@
   }
 }
 
+namespace {
+struct SanitizerHandlerInfo {
+  char const *const Name;
+  unsigned Version;
+};
+};
+
+const SanitizerHandlerInfo SanitizerHandlers[] = {
+#define SANITIZER_CHECK(Enum, Name, Version) {#Name, Version},
+LIST_SANITIZER_CHECKS
+#undef SANITIZER_CHECK
+};
+
 static void emitCheckHandlerCall(CodeGenFunction ,
  llvm::FunctionType *FnType,
  ArrayRef FnArgs,
- StringRef CheckName,
+ SanitizerHandler CheckHandler,
  CheckRecoverableKind RecoverKind, bool IsFatal,
  llvm::BasicBlock *ContBB) {
   assert(IsFatal || RecoverKind != CheckRecoverableKind::Unrecoverable);
   bool NeedsAbortSuffix =
   IsFatal && RecoverKind != CheckRecoverableKind::Unrecoverable;
-  std::string FnName = ("__ubsan_handle_" + CheckName +
-(NeedsAbortSuffix ? "_abort" : "")).str();
+  const SanitizerHandlerInfo  = SanitizerHandlers[CheckHandler];
+  const StringRef CheckName = CheckInfo.Name;
+  std::string FnName =
+  ("__ubsan_handle_" + CheckName +
+   (CheckInfo.Version ? "_v" + std::to_string(CheckInfo.Version) : "") +
+   (NeedsAbortSuffix ? "_abort" : ""))
+  .str();
   bool MayReturn =
   !IsFatal || RecoverKind == CheckRecoverableKind::AlwaysRecoverable;
 
@@ -2536,10 +2555,13 @@
 
 void CodeGenFunction::EmitCheck(
 ArrayRef Checked,
-StringRef CheckName, ArrayRef StaticArgs,
+SanitizerHandler CheckHandler, ArrayRef StaticArgs,
 ArrayRef DynamicArgs) {
   assert(IsSanitizerScope);
   assert(Checked.size() > 0);
+  assert(CheckHandler >= 0 &&
+ CheckHandler < sizeof(SanitizerHandlers) / sizeof(*SanitizerHandlers));
+  const StringRef CheckName = SanitizerHandlers[CheckHandler].Name;
 
   llvm::Value *FatalCond = nullptr;
   llvm::Value *RecoverableCond = nullptr;
@@ -2619,7 +2641,7 @@
   if (!FatalCond || !RecoverableCond) {
 // Simple case: we need to generate a single handler call, either
 // fatal, or non-fatal.
-emitCheckHandlerCall(*this, FnType, Args, CheckName, RecoverKind,
+emitCheckHandlerCall(*this, FnType, Args, CheckHandler, RecoverKind,
  (FatalCond != nullptr), Cont);
   } else {
 // Emit two handler calls: first one for set of unrecoverable checks,
@@ -2629,10 +2651,10 @@
 llvm::BasicBlock *FatalHandlerBB = createBasicBlock("fatal." + CheckName);
 Builder.CreateCondBr(FatalCond, 

[PATCH] D21695: [clang] Version support for UBSan handlers

[Filipe Cabecinhas via Phabricator via cfe-commits]

filcab added a comment.

Ping!


https://reviews.llvm.org/D21695



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