[PATCH] D133157: Add -fsanitizer-coverage=control-flow

2022-09-14 Thread Kostya Serebryany via Phabricator via cfe-commits
kcc accepted this revision. kcc added a comment. LGTM Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D133157/new/ https://reviews.llvm.org/D133157 ___ cfe-commits mailing list cfe-commits@lists.llvm.org ht

[PATCH] D133157: Add -fsanitizer-coverage=control-flow

2022-09-14 Thread Kostya Serebryany via Phabricator via cfe-commits
kcc added inline comments. Comment at: compiler-rt/test/sanitizer_common/TestCases/sanitizer_coverage_control_flow.cpp:24 +int main() { + int (*main_ptr)() = &main; + void (*foo_ptr)(int) = &foo; syntax nit: auto main_ptr = &main Comment a

[PATCH] D133157: Add -fsanitizer-coverage=control-flow

2022-09-13 Thread Kostya Serebryany via Phabricator via cfe-commits
kcc added inline comments. Comment at: compiler-rt/test/sanitizer_common/TestCases/sanitizer_coverage_control_flow.cpp:1 +// Tests -fsanitize-coverage=control-flow. + I suggest to make this test smaller: * foo() can by empty (but avoid inlining) * main() can hav

[PATCH] D133157: Add -fsanitizer-coverage=control-flow

2022-09-13 Thread Kostya Serebryany via Phabricator via cfe-commits
kcc added inline comments. Comment at: clang/docs/SanitizerCoverage.rst:382 + void __sanitizer_cov_cfs_init(const uintptr_t *cfs_beg, +const uintptr_t *cfs_end) { +// [cfs_beg,cfs_end) is the array of ptr-sized integers representing --

[PATCH] D113447: [sancov] add tracing for loads and store

2021-11-09 Thread Kostya Serebryany via Phabricator via cfe-commits
This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rGb7f3a4f4fa14: [sancov] add tracing for loads and store (authored by kcc). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https:/

[PATCH] D113447: [sancov] add tracing for loads and store

2021-11-09 Thread Kostya Serebryany via Phabricator via cfe-commits
kcc marked an inline comment as done. kcc added inline comments. Comment at: clang/test/Driver/autocomplete.c:73 // FNOSANICOVERALL-NEXT: trace-pc-guard +// FNOSANICOVERALL-NEXT: trace-loads +// FNOSANICOVERALL-NEXT: trace-stores morehouse wrote: > This check is

[PATCH] D113447: [sancov] add tracing for loads and store

2021-11-09 Thread Kostya Serebryany via Phabricator via cfe-commits
kcc updated this revision to Diff 385883. kcc added a comment. addressed review comments Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D113447/new/ https://reviews.llvm.org/D113447 Files: clang/docs/SanitizerCoverage.rst clang/include/clang/Bas

[PATCH] D113447: [sancov] add tracing for loads and store

2021-11-08 Thread Kostya Serebryany via Phabricator via cfe-commits
kcc created this revision. kcc added a reviewer: morehouse. Herald added subscribers: ormris, dexonsmith, dang, hiraditya. kcc requested review of this revision. Herald added projects: clang, Sanitizers, LLVM. Herald added subscribers: llvm-commits, Sanitizers, cfe-commits. add tracing for loads a

[PATCH] D108323: [asan] Added -inline-small-callbacks LLVM flag, which would force inline code for 8 and 16 byte data types when otherwise a callback would have been used.

2021-08-18 Thread Kostya Serebryany via Phabricator via cfe-commits
kcc added a comment. What's the code size implications? Comment at: clang/test/CodeGen/asan-use-callbacks.cpp:15 -int deref(int *p) { +long deref(long *p) { return *p; As we introduce a difference in behavior for small and large accesses, I would extend t

[PATCH] D106101: [asan] Slightly modified the documentation.

2021-07-15 Thread Kostya Serebryany via Phabricator via cfe-commits
kcc accepted this revision. kcc added a comment. This revision is now accepted and ready to land. LGTM Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D106101/new/ https://reviews.llvm.org/D106101 ___ cfe-c

[PATCH] D104494: [dfsan] Replace dfs$ prefix with .dfsan suffix

2021-06-17 Thread Kostya Serebryany via Phabricator via cfe-commits
kcc added a comment. Yey, great idea! :) (I am not reviewing the code; but the change looks straightforward) Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D104494/new/ https://reviews.llvm.org/D104494 __

[PATCH] D96120: [scudo] Port scudo sanitizer to Windows

2021-03-02 Thread Kostya Serebryany via Phabricator via cfe-commits
kcc added reviewers: kcc, pcc, vitalybuka. kcc added a comment. We can't possibly maintain two variants of scudo. All effort is currently spent on the newer (standalone) version. I am afraid we will have to delete the older (non-standalone) variant entirely. (And the sooner the better) CHANGE

[PATCH] D89959: UBSAN: emit distinctive traps in trapping mode

2020-11-03 Thread Kostya Serebryany via Phabricator via cfe-commits
kcc added a reviewer: morehouse. kcc added a comment. did you consider approaches where the emitted code doesn't change, but the binary contains a debug-like metadata that corresponds to the trap instructions? Matt (CC-ed) has a patch if this kind (for a different purpose) in the works . CHANGE

[PATCH] D84371: [DFSan] Add efficient fast16labels instrumentation mode.

2020-07-23 Thread Kostya Serebryany via Phabricator via cfe-commits
kcc added a comment. Yep, cool. LGTM from me, but please get another pair if eyes (Vitaly?) Comment at: clang/docs/DataFlowSanitizer.rst:182 +less CPU and code size overhead. To use fast16labels instrumentation, you'll +need to specify `-fsanitize=dataflow -mllvm -fast-16-labe

[PATCH] D77477: tsan: don't instrument __attribute__((naked)) functions

2020-04-08 Thread Kostya Serebryany via Phabricator via cfe-commits
kcc added a comment. The code is ok, but I'd like to see an ACK from Dmitry. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D77477/new/ https://reviews.llvm.org/D77477 ___ cfe-commits mailing list cfe-co

[PATCH] D74150: Update hwasan docs to cover outlined checks and globals.

2020-02-06 Thread Kostya Serebryany via Phabricator via cfe-commits
kcc accepted this revision. kcc added a comment. thanks! Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D74150/new/ https://reviews.llvm.org/D74150 ___ cfe-commits mailing list cfe-commits@lists.llvm.org

[PATCH] D58195: [HWASAN] Updated HWASAN design document to better portray the chance of missing a bug.

2019-02-13 Thread Kostya Serebryany via Phabricator via cfe-commits
kcc accepted this revision. kcc added a comment. This revision is now accepted and ready to land. LGTM and welcome back to the project! :) Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D58195/new/ https://reviews.llvm.org/D58195 _

[PATCH] D54905: [AddressSanitizer] Add flag to disable linking with CXX runtime

2018-11-29 Thread Kostya Serebryany via Phabricator via cfe-commits
kcc added a comment. weak new/delete may solve this particular problem, but may cause confusion to lots of other users, e.g. in cases when a user has overridden new/delete for non-essential reasons (e.g. debuging) and can disable the overrides in asan mode. With weak symbols such a user will n

[PATCH] D54905: [AddressSanitizer] Add flag to disable linking with CXX runtime

2018-11-29 Thread Kostya Serebryany via Phabricator via cfe-commits
kcc added a comment. In D54905#1312386 , @evgeny777 wrote: > Unfortunately, this is not an option. I have very custom heap implementation > (in fact multiple heap types sharing contiguous memory block), so ASAN heap > can't be a drop-in replacement. I'm

[PATCH] D54905: [AddressSanitizer] Add flag to disable linking with CXX runtime

2018-11-28 Thread Kostya Serebryany via Phabricator via cfe-commits
kcc added a comment. I am reluctant to add any new flags if there is any possibility to avoid doing so. Is there any other solution for your problem? E.g. you can disable the override of new/delete when building with asan. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D54905/new/ http

[PATCH] D54604: Automatic variable initialization

2018-11-16 Thread Kostya Serebryany via Phabricator via cfe-commits
kcc added a comment. Would it be possible to extend test/Sema/uninit-variables.c to verify that the new option doesn't break -Wuninitialized? Repository: rC Clang https://reviews.llvm.org/D54604 ___ cfe-commits mailing list cfe-commits@lists.llv

[PATCH] D54473: [sanitizers] Initial implementation for -fsanitize=init-locals

2018-11-15 Thread Kostya Serebryany via Phabricator via cfe-commits
kcc added a comment. I think you can safely abandon this change in favor of https://reviews.llvm.org/D54604 -- please join the review there. Repository: rC Clang https://reviews.llvm.org/D54473 ___ cfe-commits mailing list cfe-commits@lists.llvm

[PATCH] D54604: Automatic variable initialization

2018-11-15 Thread Kostya Serebryany via Phabricator via cfe-commits
kcc added reviewers: glider, vitalybuka, pcc, eugenis, vlad.tsyrklevich. kcc added a comment. exciting. adding more folks FYI and to help review Repository: rC Clang https://reviews.llvm.org/D54604 ___ cfe-commits mailing list cfe-commits@lists.l

[PATCH] D54473: [sanitizers] Initial implementation for -fsanitize=init-locals

2018-11-13 Thread Kostya Serebryany via Phabricator via cfe-commits
kcc added a comment. This new flag inhibits the warnings from -Wuninitialized, right? While this is fine for experimenting (and I want to have this in ToT to enable wide experimentation) we should clearly state (in the comments) that the final intent is to make the feature work together with -W

[PATCH] D50194: LLVM Proto Fuzzer - Run Functions on Suite of Inputs

2018-08-03 Thread Kostya Serebryany via Phabricator via cfe-commits
kcc added inline comments. Comment at: clang/tools/clang-fuzzer/handle-llvm/handle_llvm.cpp:128 +void RunFuncOnInputs(LLVMFunc f, int x) { + if (x) { +for (int i = 0; i < NumArrays; i++) looks like code duplication, also strange name for a variable: 'x'. Ca

[PATCH] D49793: [AArch64] - return address signing

2018-07-25 Thread Kostya Serebryany via Phabricator via cfe-commits
kcc added inline comments. Comment at: include/clang/Frontend/CodeGenOptions.h:114 +Partial,// Sign the return address of functions that spill LR +All // Sign the return address of all functions + }; what's the purpose of signing LR i

[PATCH] D47666: Refactored clang-fuzzer and added new (copy) files

2018-06-07 Thread Kostya Serebryany via Phabricator via cfe-commits
kcc added a comment. Some feedback on the generated code: while (1){ let's not have the while loops inside the for loops for now. If the initial goal is to stress the loop optimizations (e.g. vectorizer), loops likes this are just a distraction for (int loop_ctr = 0 too verbose. Use 'i'm

[PATCH] D45996: [HWASan] Update HWASan assembly snippet in the docs

2018-04-23 Thread Kostya Serebryany via Phabricator via cfe-commits
kcc accepted this revision. kcc added a comment. This revision is now accepted and ready to land. LGTM Repository: rC Clang https://reviews.llvm.org/D45996 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mail

[PATCH] D44801: Add the -fsanitize=shadow-call-stack flag

2018-03-27 Thread Kostya Serebryany via Phabricator via cfe-commits
kcc accepted this revision. kcc added a comment. LGTM modulo prolog vs prlogue and epilog vs epilogue https://en.wiktionary.org/wiki/epilog says these are alternative spellings, so up to you. Comment at: docs/ShadowCallStack.rst:14 +buffer overflows. It works by saving a func

[PATCH] D44801: Add the -fsanitize=shadow-call-stack flag

2018-03-22 Thread Kostya Serebryany via Phabricator via cfe-commits
kcc added a comment. please also add a short comparison with Intel CET. Repository: rC Clang https://reviews.llvm.org/D44801 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D44801: Add the -fsanitize=shadow-call-stack flag

2018-03-22 Thread Kostya Serebryany via Phabricator via cfe-commits
kcc added a comment. [didn't look at the code yet, just at the docs] Please add a docs section describing how to handle leaf functions. If they are not handled yet, no need to change the implementation in these pathches -- ok to do it later. Comment at: docs/ShadowCallStack.

[PATCH] D43423: [SimplifyCFG] Create flag to disable simplifyCFG.

2018-02-20 Thread Kostya Serebryany via Phabricator via cfe-commits
kcc added a comment. We use function attributes in similar situations for asan/tsan/msan. Similar, but not equivalent, so I don't know if we must follow the same pattern here. The difference here is that we should keep the ability to turn optimization on and off from command line, regardless o

[PATCH] D42874: [hwasan] Add a paragraph on stack instrumentation.

2018-02-02 Thread Kostya Serebryany via Phabricator via cfe-commits
kcc accepted this revision. kcc added a comment. This revision is now accepted and ready to land. LGTM++ https://reviews.llvm.org/D42874 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commi

[PATCH] D41417: [hwasan] Implement -fsanitize-recover=hwaddress.

2017-12-19 Thread Kostya Serebryany via Phabricator via cfe-commits
kcc accepted this revision. kcc added a comment. This revision is now accepted and ready to land. LGTM with a nit Comment at: compiler-rt/lib/hwasan/hwasan.cc:255 -template +template __attribute__((always_inline, nodebug)) I'd prefer enums to booleans, for b

[PATCH] D40938: update hwasan docs

2017-12-07 Thread Kostya Serebryany via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes. Closed by commit rC320075: update hwasan docs (authored by kcc). Changed prior to commit: https://reviews.llvm.org/D40938?vs=126001&id=126005#toc Repository: rC Clang https://reviews.llvm.org/D40938 Files: docs/Hard

[PATCH] D40938: update hwasan docs

2017-12-07 Thread Kostya Serebryany via Phabricator via cfe-commits
kcc updated this revision to Diff 126001. kcc added a comment. mention https://www.kernel.org/doc/Documentation/arm64/tagged-pointers.txt Repository: rC Clang https://reviews.llvm.org/D40938 Files: docs/HardwareAssistedAddressSanitizerDesign.rst Index: docs/HardwareAssistedAddressSanitiz

[PATCH] D40936: Hardware-assisted AddressSanitizer (clang part).

2017-12-06 Thread Kostya Serebryany via Phabricator via cfe-commits
kcc accepted this revision. kcc added a comment. This revision is now accepted and ready to land. LGTM please give at least Aleksey a chance to review as well. https://reviews.llvm.org/D40936 ___ cfe-commits mailing list cfe-commits@lists.llvm.org h

[PATCH] D40938: update hwasan docs

2017-12-06 Thread Kostya Serebryany via Phabricator via cfe-commits
kcc created this revision. Herald added a subscriber: cfe-commits. - use more readable name - document the hwasan attribute Repository: rC Clang https://reviews.llvm.org/D40938 Files: docs/HardwareAssistedAddressSanitizerDesign.rst Index: docs/HardwareAssistedAddressSanitizerDesign.rst =

[PATCH] D40568: design document for a hardware-assisted memory safety (HWAMS) tool, similar to AddressSanitizer

2017-12-04 Thread Kostya Serebryany via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes. Closed by commit rC319684: design document for a hardware-assisted memory safety (HWAMS) tool, similar to… (authored by kcc). Changed prior to commit: https://reviews.llvm.org/D40568?vs=125045&id=125398#toc Repository:

[PATCH] D40568: design document for a hardware-assisted memory safety (HWAMS) tool, similar to AddressSanitizer

2017-11-30 Thread Kostya Serebryany via Phabricator via cfe-commits
kcc updated this revision to Diff 125045. kcc added a comment. Rename the new tool to HWASAN Repository: rC Clang https://reviews.llvm.org/D40568 Files: docs/HardwareAssistedAddressSanitizerDesign.rst docs/index.rst Index: docs/index.rst =

[PATCH] D40568: design document for a hardware-assisted memory safety (HWAMS) tool, similar to AddressSanitizer

2017-11-29 Thread Kostya Serebryany via Phabricator via cfe-commits
kcc updated this revision to Diff 124797. kcc added a comment. rephrase the sources of asan overhead Repository: rC Clang https://reviews.llvm.org/D40568 Files: docs/TaggedAddressSanitizerDesign.rst Index: docs/TaggedAddressSanitizerDesign.rst =

[PATCH] D40568: design document for a hardware-assisted memory safety (HWAMS) tool, similar to AddressSanitizer

2017-11-28 Thread Kostya Serebryany via Phabricator via cfe-commits
kcc updated this revision to Diff 124691. kcc added a comment. mention alternatives for memory access instrumentation Repository: rC Clang https://reviews.llvm.org/D40568 Files: docs/TaggedAddressSanitizerDesign.rst Index: docs/TaggedAddressSanitizerDesign.rst

[PATCH] D40568: design document for a hardware-assisted memory safety (HWAMS) tool, similar to AddressSanitizer

2017-11-28 Thread Kostya Serebryany via Phabricator via cfe-commits
kcc added inline comments. Comment at: docs/TaggedAddressSanitizerDesign.rst:22 +*quarantine* to find use-after-free. +The shadow, the redzones, and the quarantine are the +major sources of AddressSanitizer's memory overhead. davidxl wrote: > What is the overhead

[PATCH] D40568: design document for a hardware-assisted memory safety (HWAMS) tool, similar to AddressSanitizer

2017-11-28 Thread Kostya Serebryany via Phabricator via cfe-commits
kcc marked an inline comment as done. kcc added inline comments. Comment at: docs/TaggedAddressSanitizerDesign.rst:2 +=== +TaggedAddressSanitizer Design Documentation +=== eugenis wro

[PATCH] D40568: design document for a hardware-assisted memory safety (HWAMS) tool, similar to AddressSanitizer

2017-11-28 Thread Kostya Serebryany via Phabricator via cfe-commits
kcc updated this revision to Diff 124637. kcc added a comment. addressed comments Repository: rC Clang https://reviews.llvm.org/D40568 Files: docs/TaggedAddressSanitizerDesign.rst Index: docs/TaggedAddressSanitizerDesign.rst

[PATCH] D40568: design document for a hardware-assisted memory safety (HWAMS) tool, similar to AddressSanitizer

2017-11-28 Thread Kostya Serebryany via Phabricator via cfe-commits
kcc updated this revision to Diff 124604. kcc added a comment. minor edit (explained shadow) Repository: rC Clang https://reviews.llvm.org/D40568 Files: docs/TaggedAddressSanitizerDesign.rst Index: docs/TaggedAddressSanitizerDesign.rst =

[PATCH] D40568: design document for a hardware-assisted memory safety (HWAMS) tool, similar to AddressSanitizer

2017-11-28 Thread Kostya Serebryany via Phabricator via cfe-commits
kcc created this revision. Herald added subscribers: cfe-commits, fedor.sergeev. preliminary design document for a hardware-assisted memory safety (HWAMS) tool, similar to AddressSanitizer The name TaggedAddressSanitizer and the rest of the document, are early draft, suggestions are welcome. Th

[PATCH] D38853: [clang-format] Allow building fuzzer with OSS-Fuzz flags.

2017-10-12 Thread Kostya Serebryany via Phabricator via cfe-commits
kcc added a reviewer: bogner. kcc accepted this revision. kcc added a comment. This revision is now accepted and ready to land. LGTM +Justin FYI Please also take a look at llvm-isel-fuzzer, which I've just added to oss-fuzz https://reviews.llvm.org/D38853 ___

[PATCH] D38812: [clang-fuzzer] Allow linking with any fuzzing engine.

2017-10-11 Thread Kostya Serebryany via Phabricator via cfe-commits
kcc accepted this revision. kcc added a comment. This revision is now accepted and ready to land. LGTM https://reviews.llvm.org/D38812 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D38642: [clang-fuzzer] Allow building without coverage instrumentation.

2017-10-06 Thread Kostya Serebryany via Phabricator via cfe-commits
kcc added a comment. In https://reviews.llvm.org/D38642#891125, @morehouse wrote: > In https://reviews.llvm.org/D38642#891074, @kcc wrote: > > > If you can *easily* share main() with the one in LLVM -- do it, otherwise > > don't bother. > > > Does the fuzzer main come from LLVM or compiler-rt no

[PATCH] D38642: [clang-fuzzer] Allow building without coverage instrumentation.

2017-10-06 Thread Kostya Serebryany via Phabricator via cfe-commits
kcc added a reviewer: vitalybuka. kcc added a comment. conceptually ok, but please let Vitaly review the cmake part. https://reviews.llvm.org/D38642 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listin

[PATCH] D38642: [clang-fuzzer] Allow building without coverage instrumentation.

2017-10-06 Thread Kostya Serebryany via Phabricator via cfe-commits
kcc added a comment. grrr. I am sorry, I've just contradicted myself. :( The goal here is to build the fuzz targets always and use them as tests, which includes building with any toolchain, including toolchains that don't support -fsanitize=fuzzer your original change actually solved this.

[PATCH] D38642: [clang-fuzzer] Allow building without coverage instrumentation.

2017-10-06 Thread Kostya Serebryany via Phabricator via cfe-commits
kcc added a comment. We often suggest to code owners to implement their own dummy main to run fuzz targets as regression tests. But for ourselves (LLVM) this recommendations makes less sense since libFuzzer is part of LLVM and we can use it's main directly. https://reviews.llvm.org/D38642

[PATCH] D38642: [clang-fuzzer] Allow building without coverage instrumentation.

2017-10-06 Thread Kostya Serebryany via Phabricator via cfe-commits
kcc added a comment. >> Will we be able to reuse some of Justin's code instead of creating one more >> main() function? > > This reuses the code that Justin moved to FuzzMutate/FuzzerCLI. That's why > the main is so short. But perhaps we could move the main itself into > FuzzerCLI? Yes, hav

[PATCH] D38642: [clang-fuzzer] Allow building without coverage instrumentation.

2017-10-06 Thread Kostya Serebryany via Phabricator via cfe-commits
kcc added a comment. It's not about coverage instrumentation (not) being present, but about libFuzzer's main() being present, right? Will we be able to reuse some of Justin's code instead of creating one more main() function? Or, why not link with libFuzzer (-fsanitize=fuzzer at link time) eve

[PATCH] D37156: [SanitizeCoverage] Enable stack-depth coverage for -fsanitize=fuzzer

2017-08-29 Thread Kostya Serebryany via Phabricator via cfe-commits
kcc accepted this revision. kcc added a comment. LGTM https://reviews.llvm.org/D37156 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D37156: [SanitizeCoverage] Enable stack-depth coverage for -fsanitize=fuzzer

2017-08-29 Thread Kostya Serebryany via Phabricator via cfe-commits
kcc added inline comments. Comment at: clang/lib/Driver/SanitizerArgs.cpp:297 CoverageTraceCmp | CoveragePCTable; +#if !defined(__APPLE__) +// Due to TLS differences, stack depth tracking is disabled on Mac. please use if(Some

[PATCH] D37156: [SanitizeCoverage] Enable stack-depth coverage for -fsanitize=fuzzer

2017-08-28 Thread Kostya Serebryany via Phabricator via cfe-commits
kcc added a reviewer: george.karpenkov. kcc added a comment. +George, in case he knows about __attribute__((tls_model("initial-exec"))) on Mac Comment at: compiler-rt/lib/sanitizer_common/sanitizer_coverage_libcdep_new.cc:218 +SANITIZER_INTERFACE_ATTRIBUTE SANITIZER_WEAK_ATTR

[PATCH] D37156: [SanitizeCoverage] Enable stack-depth coverage for -fsanitize=fuzzer

2017-08-25 Thread Kostya Serebryany via Phabricator via cfe-commits
kcc accepted this revision. kcc added a comment. This revision is now accepted and ready to land. LGTM https://reviews.llvm.org/D37156 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D37156: [SanitizeCoverage] Enable stack-depth coverage for -fsanitize=fuzzer

2017-08-25 Thread Kostya Serebryany via Phabricator via cfe-commits
kcc added a comment. Did you check this on something other than the unit tests? E.g. a couple of benchmarks from fuzzer-test-suite? Comment at: llvm/lib/Transforms/Instrumentation/SanitizerCoverage.cpp:177 +bool IsLeafFunc(const Function &F) { + for (const BasicBlock &BB :

[PATCH] D36839: [SanitizerCoverage] Add stack depth tracing instrumentation.

2017-08-19 Thread Kostya Serebryany via Phabricator via cfe-commits
kcc added a comment. Please also write a lit test for test/DeepRecursionTest.cpp (e.g. test/deep-recursion.test) Repository: rL LLVM https://reviews.llvm.org/D36839 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cg

[PATCH] D36882: [clang-proto-fuzzer] Allow user-specified compiler arguments.

2017-08-19 Thread Kostya Serebryany via Phabricator via cfe-commits
kcc added a comment. Update the README file please (with a text similar to this commit message) Comment at: cfe/trunk/tools/clang-fuzzer/ExampleClangProtoFuzzer.cpp:32 + for (int I = 1; I < *argc; I++) { +if (strcmp((*argv)[I], "-ignore_remaining_args=1") == 0) { + fo

[PATCH] D36635: Add a Dockerfile for clang-proto-fuzzer

2017-08-11 Thread Kostya Serebryany via Phabricator via cfe-commits
kcc added a comment. I'd avoid such extra complexity, after all this is just an example. One can force the full rebuild with --no-cache. It'll take just a bit more time since most of the time is consumed by the compiler builds. BTW, my old svn (1.8.8, Ubuntu 14.04) does't have "info --show-item

[PATCH] D36635: Add a Dockerfile for clang-proto-fuzzer

2017-08-11 Thread Kostya Serebryany via Phabricator via cfe-commits
kcc added inline comments. Comment at: tools/clang-fuzzer/Dockerfile:22 +# Get LLVM +RUN svn co http://llvm.org/svn/llvm-project/llvm/trunk llvm +RUN cd llvm/tools && svn co http://llvm.org/svn/llvm-project/cfe/trunk clang -r $(cd ../ && svn info | grep Revision | awk '{print $2

[PATCH] D36635: Add a Dockerfile for clang-proto-fuzzer

2017-08-11 Thread Kostya Serebryany via Phabricator via cfe-commits
kcc updated this revision to Diff 110810. kcc added a comment. fix 'svn co' command (apparently it did not matter though) https://reviews.llvm.org/D36635 Files: tools/clang-fuzzer/Dockerfile tools/clang-fuzzer/README.txt Index: tools/clang-fuzzer/README.txt ===

[PATCH] D36635: Add a Dockerfile for clang-proto-fuzzer

2017-08-11 Thread Kostya Serebryany via Phabricator via cfe-commits
kcc created this revision. Add a Dockerfile for clang-proto-fuzzer https://reviews.llvm.org/D36635 Files: tools/clang-fuzzer/Dockerfile tools/clang-fuzzer/README.txt Index: tools/clang-fuzzer/README.txt === --- tools/clang-fu

[PATCH] D36324: Integrate Kostya's clang-proto-fuzzer with LLVM.

2017-08-08 Thread Kostya Serebryany via Phabricator via cfe-commits
kcc accepted this revision. kcc added a comment. LGTM with a couple if nits in the README Thanks! Comment at: clang/tools/clang-fuzzer/README.txt:11 +class, producing valid C++ programs in the process. As a result, +clang-proto-fuzzer is better at stressing deeper layers of C

[PATCH] D36324: Integrate Kostya's clang-proto-fuzzer with LLVM.

2017-08-08 Thread Kostya Serebryany via Phabricator via cfe-commits
kcc added a comment. Looks good! Now, please add a clang/tools/clang-fuzzer/README.txt describing how to build the fuzzers (both the old one and the new one) and how to run them. For the new one explain how to install the deps https://reviews.llvm.org/D36324 _

[PATCH] D36324: Integrate Kostya's clang-proto-fuzzer with LLVM.

2017-08-08 Thread Kostya Serebryany via Phabricator via cfe-commits
kcc added a comment. In https://reviews.llvm.org/D36324#835415, @morehouse wrote: > In https://reviews.llvm.org/D36324#834660, @kcc wrote: > > > Why do we need LLVM_ENABLE_RTTI=ON here? > > > Attempting to build without it yields all kinds of protobuf errors. For > example: > F4944099: image.p

[PATCH] D36324: Integrate Kostya's clang-proto-fuzzer with LLVM.

2017-08-07 Thread Kostya Serebryany via Phabricator via cfe-commits
kcc added a comment. Why do we need LLVM_ENABLE_RTTI=ON here? https://reviews.llvm.org/D36324 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D36324: Integrate Kostya's clang-proto-fuzzer with LLVM.

2017-08-07 Thread Kostya Serebryany via Phabricator via cfe-commits
kcc added a reviewer: bogner. kcc added a comment. +bogner@ FYI Comment at: clang/tools/clang-fuzzer/ExampleClangProtoFuzzer.cpp:25 + +static void MaybePrint(const std::string &S) { + static const char *env = getenv("CXXFUZZ_PRINT"); this is debug code, not wo

[PATCH] D36324: Integrate Kostya's clang-proto-fuzzer with LLVM.

2017-08-04 Thread Kostya Serebryany via Phabricator via cfe-commits
kcc added a comment. In https://reviews.llvm.org/D36324#832271, @thakis wrote: > Why should this be part of llvm? This seems to come with very heavy > dependencies (protobuf), and LLVM has historically tried to minimize the > number of things it depends on. This fuzzer has already uncovered a

[PATCH] D33277: [Clang][x86][Inline Asm] - Enum support for MS syntax

2017-07-25 Thread Kostya Serebryany via Phabricator via cfe-commits
kcc added a comment. Hi. This patch causes the msan bots to complain. Please fix or revert ASAP. http://lab.llvm.org:8011/builders/sanitizer-x86_64-linux-fast/builds/6702/steps/check-clang%20msan/logs/stdio Testing: 0 .. 10.. FAIL: Clang :: CodeGen/ms_this.cpp (2142 of 11057) - TEST 'Clang :: C

[PATCH] D34430: [Clang][TypoCorrection] Clang hangs in typo correction

2017-07-17 Thread Kostya Serebryany via Phabricator via cfe-commits
kcc added a comment. Looks good, but I don't know this code well enough to stamp LGTM. Richard? Repository: rL LLVM https://reviews.llvm.org/D34430 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/list

[PATCH] D34267: do more processing in clang-fuzzer (use EmitAssemblyAction)

2017-07-13 Thread Kostya Serebryany via Phabricator via cfe-commits
kcc updated this revision to Diff 106573. kcc added a comment. Use EmitObjAction; also link and initialize the targets so that we actually call the optimizer. https://reviews.llvm.org/D34267 Files: tools/clang-fuzzer/CMakeLists.txt tools/clang-fuzzer/ClangFuzzer.cpp Index: tools/clang-fuz

[PATCH] D34210: Add __has_feature(leak_sanitizer)

2017-07-10 Thread Kostya Serebryany via Phabricator via cfe-commits
kcc added a comment. In https://reviews.llvm.org/D34210#785828, @rnk wrote: > If LSan is a runtime thing, why not use weak hooks or something to detect > LSan at runtime instead of using a macro? +1 For a run-time-only feature the checking should also be run-time-only (not compile-time) ht

[PATCH] D34267: do more processing in clang-fuzzer (use EmitAssemblyAction)

2017-06-16 Thread Kostya Serebryany via Phabricator via cfe-commits
kcc added a comment. ignore this for now. I've found how to make it even more interesting (by using llvm::InitializeAllTargets, etc), will send an update later. https://reviews.llvm.org/D34267 ___ cfe-commits mailing list cfe-commits@lists.llvm.org

[PATCH] D34267: do more processing in clang-fuzzer (use EmitAssemblyAction)

2017-06-15 Thread Kostya Serebryany via Phabricator via cfe-commits
kcc created this revision. Herald added a subscriber: mgorny. use EmitAssemblyAction in clang-fuzzer https://reviews.llvm.org/D34267 Files: tools/clang-fuzzer/CMakeLists.txt tools/clang-fuzzer/ClangFuzzer.cpp Index: tools/clang-fuzzer/ClangFuzzer.cpp ==

[PATCH] D32046: [Preprocessor]Correct Macro-Arg allocation of StringifiedArguments, correct getNumArguments

2017-06-15 Thread Kostya Serebryany via Phabricator via cfe-commits
kcc added a comment. the bots complain about a leak in the new test code. Please fix/revert ASAP. http://lab.llvm.org:8011/builders/sanitizer-x86_64-linux-fast/builds/5691/steps/check-clang%20asan/logs/stdio 28905==ERROR: LeakSanitizer: detected memory leaks

[PATCH] D34210: Add __has_feature(leak_sanitizer)

2017-06-14 Thread Kostya Serebryany via Phabricator via cfe-commits
kcc added a comment. I'm not sure about this one. standalone lsan is a link-time feature only, it doesn't change the compilation, so the argument of consistency doesn't apply. https://reviews.llvm.org/D34210 ___ cfe-commits mailing list cfe-commit

[PATCH] D33368: [libcxxabi][demangler] Fix a crash in the demangler

2017-05-24 Thread Kostya Serebryany via Phabricator via cfe-commits
kcc added a comment. Done (see https://github.com/google/oss-fuzz/blob/master/projects/llvm_libcxxabi/project.yaml) Repository: rL LLVM https://reviews.llvm.org/D33368 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org

[PATCH] D33368: [libcxxabi][demangler] Fix a crash in the demangler

2017-05-24 Thread Kostya Serebryany via Phabricator via cfe-commits
kcc added a comment. Also, are you now maintaining this code? I am trying to find someone who wants to be CC-ed to other demangler bugs automatically reported by oss-fuzz. Repository: rL LLVM https://reviews.llvm.org/D33368 ___ cfe-commits mail

[PATCH] D33368: [libcxxabi][demangler] Fix a crash in the demangler

2017-05-24 Thread Kostya Serebryany via Phabricator via cfe-commits
kcc added a comment. I also encourage you to run the fuzzer on every change in this code. Repository: rL LLVM https://reviews.llvm.org/D33368 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/c

[PATCH] D33368: [libcxxabi][demangler] Fix a crash in the demangler

2017-05-24 Thread Kostya Serebryany via Phabricator via cfe-commits
kcc added a comment. oss-fuzz finds the assertion failure in this new code: https://bugs.chromium.org/p/oss-fuzz/issues/detail?id=1834 Repository: rL LLVM https://reviews.llvm.org/D33368 ___ cfe-commits mailing list cfe-commits@lists.llvm.org ht

[PATCH] D33393: [PATCH] Libcxxabi Demangler PR32890

2017-05-23 Thread Kostya Serebryany via Phabricator via cfe-commits
kcc added a comment. I don't know this code and can't properly comment, but having a constant like this ("7") sounds wrong. Why not 6, or 8, or 42? https://reviews.llvm.org/D33393 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lis

[PATCH] D32243: Fix a leak in tools/driver/cc1as_main.cpp

2017-04-19 Thread Kostya Serebryany via Phabricator via cfe-commits
kcc created this revision. For some reason, the asan bot has recently started reporting this leak even though it existed for ages. https://reviews.llvm.org/D32243 Files: tools/driver/cc1as_main.cpp Index: tools/driver/cc1as_main.cpp =

[PATCH] D29628: [compiler-rt] [test] Enable the strace_test only if strace is installed

2017-02-07 Thread Kostya Serebryany via Phabricator via cfe-commits
kcc accepted this revision. kcc added a comment. This revision is now accepted and ready to land. LGTM, but please manually verify that the test runs if strace *is* available. Repository: rL LLVM https://reviews.llvm.org/D29628 ___ cfe-commits m

[PATCH] D29077: [lsan] Enable LSan for x86 Linux.

2017-01-24 Thread Kostya Serebryany via Phabricator via cfe-commits
kcc accepted this revision. kcc added a comment. This revision is now accepted and ready to land. LGTM Repository: rL LLVM https://reviews.llvm.org/D29077 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailm

[PATCH] D28133: add cxa_demangle_fuzzer

2016-12-27 Thread Kostya Serebryany via Phabricator via cfe-commits
kcc updated this revision to Diff 82571. kcc added a comment. remove unneeded set(LLVM_LINK_COMPONENTS support) https://reviews.llvm.org/D28133 Files: CMakeLists.txt fuzz/ fuzz/CMakeLists.txt fuzz/cxa_demangle_fuzzer.cpp Index: fuzz/cxa_demangle_fuzzer.cpp ===

[PATCH] D28133: add cxa_demangle_fuzzer

2016-12-27 Thread Kostya Serebryany via Phabricator via cfe-commits
kcc marked an inline comment as done. kcc added a comment. yes, removed. https://reviews.llvm.org/D28133 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D28133: add cxa_demangle_fuzzer

2016-12-27 Thread Kostya Serebryany via Phabricator via cfe-commits
kcc created this revision. kcc added reviewers: compnerd, mehdi_amini, mclow.lists. kcc added a subscriber: cfe-commits. Herald added a subscriber: mgorny. All easy-to-find bugs in cxa_demangle where fixed now (https://bugs.chromium.org/p/chromium/issues/detail?id=606626) except for one (https://l

[PATCH] D27316: [CUDA] Filter out dirver sanitizer args for NVPTX

2016-12-01 Thread Kostya Serebryany via Phabricator via cfe-commits
kcc added a comment. I am not sure this is going to work. You essentially break on the first iteration in the beginning of the loop. Why not exit the function before the loop? Also, the loop "claims" the args and by breaking early you leave the args unclaimed. I don't remember this code well

[PATCH] D27316: [CUDA] Filter out dirver sanitizer args for NVPTX

2016-12-01 Thread Kostya Serebryany via Phabricator via cfe-commits
kcc added a comment. at the very least this requires a test... Let me thing about the logic a bit more... https://reviews.llvm.org/D27316 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-com