[PATCH] D46836: Fix some rtti-options tests
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] D47291: Proposal to make rtti errors more generic.
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] D38354: Fix test by using -target instead of cc1 arguments (c-index-test goes through the driver)
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] D38364: Fix Modules/builtin-import.mm to be able to handle non-Darwin targets
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] D38364: Fix Modules/builtin-import.mm to be able to handle non-Darwin targets
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
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] D38354: Fix test by using -target instead of cc1 arguments (c-index-test goes through the driver)
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] D43013: ASan+operator new[]: Fix operator new[] cookie poisoning
filcab created this revision.
filcab added reviewers: rjmccall, kcc, rsmith.
The C++ Itanium ABI says:
No cookie is required if the new operator being used is ::operator
new[](size_t, void*).
This commit adds a flag to tell clang to poison all operator new[]
cookies.
A previous review was poisoning unconditionally, but there is an edge
case which would stop working under ASan (a custom operator new[] saves
whatever pointer it returned, and then accesses it).
This newer revision adds a command line argument to toggle this feature.
Original revision: https://reviews.llvm.org/D41301
Compiler-rt test revision with an explanation of the edge case:
https://reviews.llvm.org/D41664
Repository:
rC Clang
https://reviews.llvm.org/D43013
Files:
include/clang/Driver/Options.td
include/clang/Frontend/CodeGenOptions.def
lib/CodeGen/ItaniumCXXABI.cpp
lib/Frontend/CompilerInvocation.cpp
test/CodeGen/address-sanitizer-and-array-cookie.cpp
Index: test/CodeGen/address-sanitizer-and-array-cookie.cpp
===
--- test/CodeGen/address-sanitizer-and-array-cookie.cpp
+++ test/CodeGen/address-sanitizer-and-array-cookie.cpp
@@ -1,13 +1,15 @@
// RUN: %clang_cc1 -triple x86_64-gnu-linux -emit-llvm -o - %s | FileCheck %s
-check-prefix=PLAIN
// RUN: %clang_cc1 -triple x86_64-gnu-linux -emit-llvm -o - -fsanitize=address
%s | FileCheck %s -check-prefix=ASAN
+// RUN: %clang_cc1 -triple x86_64-gnu-linux -emit-llvm -o - -fsanitize=address
-fsanitize-address-poison-class-member-array-new-cookie %s | FileCheck %s
-check-prefix=ASAN-POISON-ALL-NEW-ARRAY
typedef __typeof__(sizeof(0)) size_t;
namespace std {
struct nothrow_t {};
std::nothrow_t nothrow;
}
void *operator new[](size_t, const std::nothrow_t &) throw();
void *operator new[](size_t, char *);
+void *operator new[](size_t, int, int);
struct C {
int x;
@@ -53,3 +55,10 @@
}
// ASAN-LABEL: CallPlacementNew
// ASAN-NOT: __asan_poison_cxx_array_cookie
+
+C *CallNewWithArgs() {
+// ASAN-LABEL: CallNewWithArgs
+// ASAN-NOT: call void @__asan_poison_cxx_array_cookie
+// ASAN-POISON-ALL-NEW-ARRAY: call void @__asan_poison_cxx_array_cookie
+ return new (123, 456) C[20];
+}
Index: lib/Frontend/CompilerInvocation.cpp
===
--- lib/Frontend/CompilerInvocation.cpp
+++ lib/Frontend/CompilerInvocation.cpp
@@ -890,6 +890,13 @@
Opts.SanitizeCfiICallGeneralizePointers =
Args.hasArg(OPT_fsanitize_cfi_icall_generalize_pointers);
Opts.SanitizeStats = Args.hasArg(OPT_fsanitize_stats);
+ if (Arg *A = Args.getLastArg(
+ OPT_fsanitize_address_poison_class_member_array_new_cookie,
+ OPT_fno_sanitize_address_poison_class_member_array_new_cookie)) {
+Opts.SanitizeAddressPoisonClassMemberArrayNewCookie =
+A->getOption().getID() ==
+OPT_fsanitize_address_poison_class_member_array_new_cookie;
+ }
if (Arg *A = Args.getLastArg(OPT_fsanitize_address_use_after_scope,
OPT_fno_sanitize_address_use_after_scope)) {
Opts.SanitizeAddressUseAfterScope =
Index: lib/CodeGen/ItaniumCXXABI.cpp
===
--- lib/CodeGen/ItaniumCXXABI.cpp
+++ lib/CodeGen/ItaniumCXXABI.cpp
@@ -1848,7 +1848,8 @@
// Handle the array cookie specially in ASan.
if (CGM.getLangOpts().Sanitize.has(SanitizerKind::Address) && AS == 0 &&
- expr->getOperatorNew()->isReplaceableGlobalAllocationFunction()) {
+ (expr->getOperatorNew()->isReplaceableGlobalAllocationFunction() ||
+ CGM.getCodeGenOpts().SanitizeAddressPoisonClassMemberArrayNewCookie)) {
// The store to the CookiePtr does not need to be instrumented.
CGM.getSanitizerMetadata()->disableSanitizerForInstruction(SI);
llvm::FunctionType *FTy =
Index: include/clang/Frontend/CodeGenOptions.def
===
--- include/clang/Frontend/CodeGenOptions.def
+++ include/clang/Frontend/CodeGenOptions.def
@@ -159,6 +159,9 @@
CODEGENOPT(SaveTempLabels, 1, 0) ///< Save temporary labels.
CODEGENOPT(SanitizeAddressUseAfterScope , 1, 0) ///< Enable use-after-scope
detection
///< in AddressSanitizer
+CODEGENOPT(SanitizeAddressPoisonClassMemberArrayNewCookie, 1,
+ 0) ///< Enable poisoning operator new[] which is not a replaceable
+ ///< global allocation function in AddressSanitizer
CODEGENOPT(SanitizeAddressGlobalsDeadStripping, 1, 0) ///< Enable linker dead
stripping
///< of globals in
AddressSanitizer
CODEGENOPT(SanitizeMemoryTrackOrigins, 2, 0) ///< Enable tracking origins in
Index: include/clang/Driver/Options.td
===
--- include/clang/Driver/Options.td
+++ include/clang/Driver/Options.td
@@ -889,6 +889,14 @@
[PATCH] D43013: ASan+operator new[]: Fix operator new[] cookie poisoning
filcab added a comment. In https://reviews.llvm.org/D43013#1001006, @rjmccall wrote: > I don't understand why your description of this patch mentions the void* > placement new[] operator. There's no cookie to poison for that operator. Hah, sorry. In writing this commit log I used parts of the old patch one. I'll update with a better commit log. Repository: rC Clang https://reviews.llvm.org/D43013 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D43013: ASan+operator new[]: Fix operator new[] cookie poisoning
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<[ "-" ],
"fno-san
[PATCH] D43013: ASan+operator new[]: Fix operator new[] cookie poisoning
This revision was automatically updated to reflect the committed changes.
Closed by commit rC324884: ASan+operator new[]: Add an option for more thorough
operator new[] cookie… (authored by filcab, committed by ).
Changed prior to commit:
https://reviews.llvm.org/D43013?vs=133389&id=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] D34299: [ubsan] Improve diagnostics for return value checks (clang)
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] D34299: [ubsan] Improve diagnostics for return value checks (clang)
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)
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] D41301: ASan+operator new[]: Fix operator new[] cookie poisoning
filcab created this revision.
filcab added reviewers: rjmccall, kcc, rsmith.
The C++ Itanium ABI says:
No cookie is required if the new operator being used is ::operator
new[](size_t, void*).
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] D41301: ASan+operator new[]: Fix operator new[] cookie poisoning
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] D47375: [Driver] Add flag "--dependent-lib=..." when enabling asan or ubsan on PS4.
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] D50901: [clang][ubsan] Split Implicit Integer Truncation Sanitizer into unsigned and signed checks
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] D52615: Handle -fsanitize-address-poison-class-member-array-new-cookie in the driver and propagate it to cc1
filcab added a comment. In https://reviews.llvm.org/D52615#1254567, @rsmith wrote: > This seems like an unreasonably long flag name. Can you try to find a shorter > name for it? `-fsanitize-poison-extra-operator-new`? Not as explicit, but maybe ok if documented? Thank you, Filipe Repository: rC Clang https://reviews.llvm.org/D52615 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D52615: Handle -fsanitize-address-poison-class-member-array-new-cookie in the driver and propagate it to cc1
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
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
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
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,
+options::OPT_fno_sanitize
[PATCH] D52615: Handle -fsanitize-address-poison-class-member-array-new-cookie in the driver and propagate it to cc1
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)) {
-Opts.SanitizeAddressPoisonCla
[PATCH] D52615: Handle -fsanitize-address-poison-class-member-array-new-cookie in the driver and propagate it to cc1
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
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(
- OPT_fsanitize_address_poison_cl
[PATCH] D52615: Handle -fsanitize-address-poison-class-member-array-new-cookie in the driver and propagate it to cc1
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)) {
-Opts.SanitizeAddressPoisonClassMemberAr
[PATCH] D52615: Handle -fsanitize-address-poison-class-member-array-new-cookie in the driver and propagate it to cc1
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)) {
-Opts.SanitizeAddressPo
[PATCH] D52615: Handle -fsanitize-address-poison-class-member-array-new-cookie in the driver and propagate it to cc1
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&id=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: include/clang/Driver/
[PATCH] D52615: Handle -fsanitize-address-poison-class-member-array-new-cookie in the driver and propagate it to cc1
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
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] D46403: [CFI] Force LLVM to die if the implicit blacklist files cannot be found.
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] D32950: Support C++1z features in `__has_extension`
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] D52615: Handle -fsanitize-address-poison-class-member-array-new-cookie in the driver and propagate it to cc1
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 AsanPoisonClassMemberOperat
[PATCH] D50901: [clang][ubsan] Split Implicit Integer Truncation Sanitizer into unsigned and signed checks
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] D50901: [clang][ubsan] Split Implicit Integer Truncation Sanitizer into unsigned and signed checks
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] D67985: CFI: wrong type passed to llvm.type.test with multiple inheritance devirtualization
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] D40948: Switch Clang's default C++ language target to C++14
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] D40948: Switch Clang's default C++ language target to C++14
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] D124493: [symbolizer] Parse DW_TAG_variable DIs to show line info for globals
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] D19667: [ubsan] Minimize size of data for type_mismatch
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] D28242: [ubsan] Minimize size of data for type_mismatch (Redo of D19667)
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] D28242: [ubsan] Minimize size of data for type_mismatch (Redo of D19667)
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&id=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 @__ubsan
[PATCH] D29369: [ubsan] Omit superflous overflow checks for promoted arithmetic (PR20193)
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] D29369: [ubsan] Omit superflous overflow checks for promoted arithmetic (PR20193)
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] D32047: [Driver] Add support for default UBSan blacklists
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] D30285: [ubsan] Don't check alignment if the alignment is 1
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] D30423: [ubsan] Detect UB loads from bitfields
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] D30729: [ubsan] Skip range checks for width-limited values
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
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] D30762: [ubsan] Add a nullability sanitizer
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] D21695: [clang] Version support for UBSan handlers
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
[PATCH] D21695: [clang] Version support for UBSan handlers
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&id=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 &CGF,
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 &CheckInfo = 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] D32199: [TBAASan] A TBAA Sanitizer (Clang)
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] D33305: [ubsan] Add a check for pointer overflow UB
filcab added inline comments.
Comment at: lib/CodeGen/CGExprScalar.cpp:3854
+ const Twine &Name) {
+ 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
