[PATCH] D96328: [Clang, NewPM] Add KMSan support

eugenis accepted this revision. eugenis 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/D96328/new/ https://reviews.llvm.org/D96328 ___

[PATCH] D96406: [Msan, NewPM] Reduce size of msan binaries

eugenis accepted this revision. eugenis added a comment. This revision is now accepted and ready to land. LGTM, please add a TODO for more passes Comment at: clang/lib/CodeGen/BackendUtil.cpp:1291 + // general purpose optimization passes. + FPM.addPass

[PATCH] D81678: Introduce noundef attribute at call sites for stricter poison analysis

eugenis accepted this revision. eugenis 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/D81678/new/ https://reviews.llvm.org/D81678 ___

[PATCH] D103845: [compiler-rt][hwasan] Add newline between record_addr lines on frame record dumps

eugenis added a comment. In D103845#2806211 , @leonardchan wrote: > I think the newline gets added to the frame description only if > `Symbolizer::GetOrInit()->SymbolizePC(pc)` is nonnull, so if it fails then no > newline is added. This makes sense to

[PATCH] D104553: [compiler-rt][hwasan] Add InitState options to thread initialization

eugenis accepted this revision. eugenis 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/D104553/new/ https://reviews.llvm.org/D104553 _

[PATCH] D104830: AST: Create __va_list in the std namespace even in C.

eugenis added a comment. LGTM Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D104830/new/ https://reviews.llvm.org/D104830 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi

[PATCH] D108029: [clang][Codegen] Introduce the no_sanitizer_instrumentation attribute

eugenis added a comment. The new attribute should be very rare, and not something that a typical end user should even know about. That's why I'd prefer if the name reflected how the attribute affects code generation rather than user-visible behavior: disable_sanitizer_instrumentation sounds per

[PATCH] D108199: [msan] Add support for disable_sanitizer_instrumentation attribute

eugenis accepted this revision. eugenis 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/D108199/new/ https://reviews.llvm.org/D108199 _

[PATCH] D107850: [asan] Implemented custom calling convention similar to the one used by HWASan for X86.

eugenis added inline comments. Comment at: llvm/include/llvm/IR/Intrinsics.td:1640 +def int_asan_check_memaccess : + Intrinsic<[],[llvm_ptr_ty, llvm_i8_ty, llvm_i8_ty], vitalybuka wrote: > kstoimenov wrote: > > vitalybuka wrote: > > > vitalybuka wrote: > > > >

[PATCH] D107850: [asan] Implemented intrinsic for the custom calling convention similar used by HWASan for X86.

eugenis added inline comments. Comment at: llvm/include/llvm/IR/Intrinsics.td:1640 +def int_asan_check_memaccess : + Intrinsic<[],[llvm_ptr_ty, llvm_i8_ty, llvm_i8_ty], vitalybuka wrote: > pcc wrote: > > eugenis wrote: > > > vitalybuka wrote: > > > > kstoimeno

[PATCH] D105703: [hwasan] Use stack safety analysis.

eugenis added a comment. The analysis should not run at -O0. Otherwise, LGTM. What kind of code size improvement do you see? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D105703/new/ https://reviews.llvm.org/D105703 ___

[PATCH] D112098: [ASan] Added stack safety support in address sanitizer.

eugenis added inline comments. Comment at: llvm/include/llvm/Analysis/StackSafetyAnalysis.h:93 + bool invalidate(Module &, const PreservedAnalyses &, + ModuleAnalysisManager::Invalidator &) { +return false; vitalybuka wrote: > vitalybuka wro

[PATCH] D105169: [Clang/Test]: Rename enable_noundef_analysis to disable-noundef-analysis and turn it off by default

eugenis added a comment. You are absolutely right. X86 variant uses an "=a" constraint (rax register), others pin the output variable to a specific register with __asm__ declaration. It appears we've missed it in Aarch64. Could you check if __asm__("x0") in the declaration of res helps? Repos

[PATCH] D111443: [Driver] Fix ToolChain::getSanitizerArgs

eugenis added a comment. The approach looks reasonable to me. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D111443/new/ https://reviews.llvm.org/D111443 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/ma

[PATCH] D105169: [Clang/Test]: Rename enable_noundef_analysis to disable-noundef-analysis and turn it off by default

eugenis added a comment. In D105169#3111935 , @hyeongyukim wrote: > - long long res; > + register long long res __asm__("x0"); > > Is it okay to commit this change by myself? Yes, go ahead! Repository: rG LLVM Github Monorepo CHANGES SINCE LA

[PATCH] D111443: [Driver] Fix ToolChain::getSanitizerArgs

eugenis added inline comments. Comment at: clang/lib/Driver/ToolChain.cpp:124 + } + return SanitizerArgs(*this, JobArgs, /*DiagnoseErrors=*/false); } SanitizerArgs SanArgs(*this, JobArgs, !SanitizerArgsChecked); SanitizerArgsChecked = true; return SanArgs;

[PATCH] D111443: [Driver] Fix ToolChain::getSanitizerArgs

eugenis accepted this revision. eugenis added a comment. This revision is now accepted and ready to land. LGTM CHANGES SINCE LAST ACTION https://reviews.llvm.org/D111443/new/ https://reviews.llvm.org/D111443 ___ cfe-commits mailing list cfe-commits

[PATCH] D81678: Introduce noundef attribute at call sites for stricter poison analysis

eugenis added a comment. Any more comments/opinions? Gui, would you like to push it to main if no one objects? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D81678/new/ https://reviews.llvm.org/D81678 ___

[PATCH] D114421: [asan] Add support for disable_sanitizer_instrumentation attribute

eugenis accepted this revision. eugenis added a comment. LGTM Comment at: clang/test/CodeGen/asan-globals.cpp:62 // CHECK: ![[ATTR_GLOBAL]] = !{{{.*}}, null, null, i1 false, i1 true} +// CHECK: ![[DISABLE_INSTR_GLOBAL]] = !{{{.*}}, null, null, i1 false, i1 true} // CHECK: ![[

[PATCH] D133392: [MTE] Add AArch64GlobalsTagging Pass

eugenis accepted this revision. eugenis added a comment. LGTM Comment at: llvm/lib/CodeGen/GlobalMerge.cpp:655 +// Don't merged tagged globals, as each global should have its own unique +// memory tag at runtime. TODO(hctim): This can be relaxed: constant globals ---

[PATCH] D127163: [clang] Add -fsanitize=memtag-globals (no-op).

eugenis accepted this revision. eugenis 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/D127163/new/ https://reviews.llvm.org/D127163 _

[PATCH] D127860: [msan] Allow KMSAN to use -fsanitize-memory-param-retval

eugenis accepted this revision. eugenis 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/D127860/new/ https://reviews.llvm.org/D127860 _

[PATCH] D127145: [Driver] add -lresolv for all but Android.

eugenis added a comment. The interceptor (dn_expand) is only defined on linux && !android, while this code links libresolv on *bsd and others, too. I think it's ok, as long as all platforms with the interceptor are covered. It would be great to mention the actual reason we do this in the commit

[PATCH] D127145: [Driver] add -lresolv for all but Android.

eugenis accepted this revision. eugenis added a comment. Typo: "runttime" LGTM Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D127145/new/ https://reviews.llvm.org/D127145 ___ cfe-commits mailing list cfe

[PATCH] D127163: [clang] Add -fsanitize=memtag-globals (no-op).

eugenis added inline comments. Comment at: clang/lib/Driver/ToolChains/Linux.cpp:767 + if (IsAArch64) { +Res |= SanitizerKind::MemtagGlobals; + } Hmm why are all the other memtag* not here? Comment at: clang/lib/Sema/SemaDeclAttr.cpp:7892

[PATCH] D127163: [clang] Add -fsanitize=memtag-globals (no-op).

eugenis added a comment. I don't have a lot of arguments either way, do you? Ignorelist support is one for the new sanitizer type. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D127163/new/ https://reviews.llvm.org/D127163 _

[PATCH] D70692: IRGen: Call SetLLVMFunctionAttributes{, ForDefinition} on __cfi_check_fail.

eugenis accepted this revision. eugenis 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/D70692/new/ https://reviews.llvm.org/D70692 _

[PATCH] D70958: [compiler-rt] [test] Disable ASLR on ASAN/MSAN/TSAN tests on NetBSD

eugenis added a comment. Could this go into the common lit config in compiler-rt/test? What is the problem with ASLR in NetBSD? Is this about fixed shadow location conflicts with the binary & library mappings? CHANGES SINCE LAST ACTION https://reviews.llvm.org/D70958/new/ https://reviews.llv

[PATCH] D70958: [compiler-rt] [test] Disable ASLR on ASAN/MSAN/TSAN tests on NetBSD

eugenis added a comment. I meant you could define this path in the common lit config as something like "netbsd_noaslr_prefix" and then set up the %run substitution in subdirectories. > PaX ASLR on NetBSD is too aggressive with the layout randomization and we > cannot predict ranges of the heap

[PATCH] D70958: [compiler-rt] [test] Disable ASLR on ASAN/MSAN/TSAN tests on NetBSD

eugenis accepted this revision. eugenis added a comment. This revision is now accepted and ready to land. LGTM Comment at: compiler-rt/test/sanitizer_common/netbsd_commands/run_noaslr.sh:4 +PATH=${PATH}:/usr/sbin +paxctl +a "${1}" +exec "${@}" krytarowski wrote

[PATCH] D71408: [lit] Remove lit's REQUIRES-ANY directive

eugenis added a comment. Nice cleanup! Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D71408/new/ https://reviews.llvm.org/D71408 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llv

[PATCH] D73513: [memtag] Plug in stack safety analysis.

eugenis updated this revision to Diff 249175. eugenis added a comment. Herald added a subscriber: danielkiss. + a test Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D73513/new/ https://reviews.llvm.org/D73513 Files: clang/lib/CodeGen/BackendUtil.

[PATCH] D73513: [memtag] Plug in stack safety analysis.

This revision was automatically updated to reflect the committed changes. Closed by commit rG2a3723ef114d: [memtag] Plug in stack safety analysis. (authored by eugenis). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D73513/new/ https://reviews.llvm.o

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

eugenis accepted this revision. eugenis added a comment. This revision is now accepted and ready to land. LGTM Comment at: clang/docs/HardwareAssistedAddressSanitizerDesign.rst:87 + bl __hwasan_check_x0_2_short // call outlined tag check +

[PATCH] D73513: [memtag] Plug in stack safety analysis.

eugenis created this revision. eugenis added reviewers: pcc, vitalybuka, ostannard. Herald added subscribers: cfe-commits, hiraditya, kristof.beyls. Herald added projects: clang, LLVM. Run StackSafetyAnalysis at the end of the IR pipeline and annotate proven safe allocas with !stack-safe metadata.

[PATCH] D73513: [memtag] Plug in stack safety analysis.

eugenis added a comment. To explain some design decisions: The analysis is added to the IR pipeline and not to the AArch64 codegen pipeline mainly because we plan to extend it with ThinLTO support in the future. To do that, module summary builder will need to depend on the (function-)local sta

[PATCH] D44645: [test] Fix Cross-DSO CFI Android sanitizer test for -rtlib=compiler-rt

eugenis added a comment. This was fixed in https://reviews.llvm.org/D44655 https://reviews.llvm.org/D44645 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D44745: [HWASan] Port HWASan to Linux x86-64 (clang)

eugenis added inline comments. Comment at: lib/Driver/SanitizerArgs.cpp:33 NotAllowedWithMinimalRuntime = Vptr, - RequiresPIE = DataFlow | Scudo, + RequiresPIE = DataFlow | HWAddress | Scudo, NeedsUnwindTables = Address | HWAddress | Thread | Memory | DataFlow, --

[PATCH] D44745: [HWASan] Port HWASan to Linux x86-64 (clang)

eugenis added inline comments. Comment at: test/Driver/asan.c:12 -// RUN: %clang -O2 -target aarch64-unknown-linux -fsanitize=hwaddress %s -S -emit-llvm -o - | FileCheck %s --check-prefix=CHECK-HWASAN -// RUN: %clang -O3 -target aarch64-unknown-linux -fsanitize=hwaddress %s -S

[PATCH] D44745: [HWASan] Port HWASan to Linux x86-64 (clang)

eugenis accepted this revision. eugenis added a comment. This revision is now accepted and ready to land. OK, sure, If you feel so strongly about this. Repository: rC Clang https://reviews.llvm.org/D44745 ___ cfe-commits mailing list cfe-commits@

[PATCH] D44927: Enable msan unconditionally on Linux

eugenis added a subscriber: kcc. eugenis added a comment. I too find these checks arbitrary and pointless. @kcc ? Repository: rC Clang https://reviews.llvm.org/D44927 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/c

[PATCH] D45054: Set dso_local on cfi_slowpath

eugenis added a comment. This looks wrong. cfi_slowpath is defined in libclang_rt.cfi, which is linked to the main executable. It is not always dso-local. On Android it is defined in libdl.so and is never dso-local. https://reviews.llvm.org/D45054 ___

[PATCH] D45054: Set dso_local on cfi_slowpath

eugenis accepted this revision. eugenis added a comment. This revision is now accepted and ready to land. LGTM then. This naming is confusing. https://reviews.llvm.org/D45054 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.

[PATCH] D45239: AArch64: Implement support for the shadowcallstack attribute.

eugenis added inline comments. Comment at: clang/docs/ShadowCallStack.rst:12 ShadowCallStack is an **experimental** instrumentation pass, currently only implemented for x86_64, that protects programs against return address overwrites (e.g. stack buffer overflows.) It works by

[PATCH] D44927: Enable msan unconditionally on Linux

This revision was not accepted when it landed; it landed in state "Needs Review". This revision was automatically updated to reflect the committed changes. Closed by commit rL329241: Enable msan unconditionally on Linux. (authored by eugenis, committed by ). Herald added a subscriber: llvm-commit

[PATCH] D44927: Enable msan unconditionally on Linux

eugenis added a comment. Merged. Please don't forget to update tests next time. Repository: rL LLVM https://reviews.llvm.org/D44927 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D38430: Enable -pie and --enable-new-dtags by default on Android.

This revision was automatically updated to reflect the committed changes. Closed by commit rL316606: Enable -pie and --enable-new-dtags by default on Android. (authored by eugenis). Changed prior to commit: https://reviews.llvm.org/D38430?vs=117229&id=120306#toc Repository: rL LLVM https://

[PATCH] D147121: [hwasan] remove requirment for PIE

eugenis added a comment. Sorry, I do not remember why this requirement is there. Must be related to shadow / allocator placement and kernel mapping conflicts, but hwasan is using dynamic shadow so that should not be an issue... LGTM as long as it works. Repository: rG LLVM Github Monorepo C

[PATCH] D148596: [KMSAN] Enable on SystemZ

eugenis accepted this revision. eugenis 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/D148596/new/ https://reviews.llvm.org/D148596 _

[PATCH] D143675: Discussion: Darwin Sanitizers Stable ABI

eugenis added a comment. I'm fine with it in general. Is asan_abi.cpp meant as a temporary stub? It's not even link anywhere in the current version. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D143675/new/ https://reviews.llvm.org/D143675 __

[PATCH] D152604: [Driver] Default -fsanitize-address-globals-dead-stripping to true for ELF

eugenis accepted this revision. eugenis added a comment. LGTM This is long overdue, I think. Thank you! Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D152604/new/ https://reviews.llvm.org/D152604 ___ cfe

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

eugenis created this revision. eugenis added a reviewer: kcc. https://reviews.llvm.org/D42874 Files: clang/docs/HardwareAssistedAddressSanitizerDesign.rst Index: clang/docs/HardwareAssistedAddressSanitizerDesign.rst === --- clang

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

This revision was automatically updated to reflect the committed changes. Closed by commit rL324163: [hwasan] Add a paragraph on stack instrumentation. (authored by eugenis, committed by ). Herald added a subscriber: llvm-commits. Changed prior to commit: https://reviews.llvm.org/D42874?vs=1327

[PATCH] D32064: [asan] Disable ASan global-GC depending on the target and compiler flags

eugenis closed this revision. eugenis added a comment. r301225 Repository: rL LLVM https://reviews.llvm.org/D32064 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D32886: [asan] A clang flag to enable ELF globals-gc

eugenis created this revision. This feature is subtly broken when the linker is gold 2.26 or earlier. See the following bug for details: https://sourceware.org/bugzilla/show_bug.cgi?id=19002 Since the decision needs to be made at compilation time, we can not test the linker version. The flag i

[PATCH] D32886: [asan] A clang flag to enable ELF globals-gc

eugenis updated this revision to Diff 97901. Repository: rL LLVM https://reviews.llvm.org/D32886 Files: include/clang/Driver/Options.td include/clang/Driver/SanitizerArgs.h include/clang/Frontend/CodeGenOptions.def lib/CodeGen/BackendUtil.cpp lib/Driver/SanitizerArgs.cpp lib/Fronte

[PATCH] D32886: [asan] A clang flag to enable ELF globals-gc

eugenis added inline comments. Comment at: include/clang/Driver/Options.td:830 HelpText<"Disable use-after-scope detection in AddressSanitizer">; +def fsanitize_address_globals_live_support : Flag<["-"], "fsanitize-address-globals-liv

[PATCH] D32886: [asan] A clang flag to enable ELF globals-gc

eugenis updated this revision to Diff 98200. Herald added a subscriber: krytarowski. Repository: rL LLVM https://reviews.llvm.org/D32886 Files: include/clang/Driver/Options.td include/clang/Driver/SanitizerArgs.h include/clang/Frontend/CodeGenOptions.def lib/CodeGen/BackendUtil.cpp l

[PATCH] D32886: [asan] A clang flag to enable ELF globals-gc

eugenis updated this revision to Diff 98354. eugenis marked an inline comment as done. Repository: rL LLVM https://reviews.llvm.org/D32886 Files: include/clang/Driver/Options.td include/clang/Driver/SanitizerArgs.h include/clang/Frontend/CodeGenOptions.def lib/CodeGen/BackendUtil.cpp

[PATCH] D32886: [asan] A clang flag to enable ELF globals-gc

eugenis added a comment. I'll fix the other unnecessary MakeArgString calls in a separate commit Repository: rL LLVM https://reviews.llvm.org/D32886 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/lis

[PATCH] D32886: [asan] A clang flag to enable ELF globals-gc

eugenis closed this revision. eugenis added a comment. r302591, thanks for the review! Repository: rL LLVM https://reviews.llvm.org/D32886 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-

[PATCH] D118171: [HWASan] Add __hwasan_init to .preinit_array.

eugenis added a subscriber: pcc. eugenis added a comment. LGTM, but I'd like @pcc to take a look. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D118171/new/ https://reviews.llvm.org/D118171 ___ cfe-commit

[PATCH] D118171: [HWASan] Add __hwasan_init to .preinit_array.

eugenis added a comment. The driver seems to always add sanitizer runtimes first on the linker command line. Comment at: clang/lib/Driver/ToolChains/CommonArgs.cpp:842 + if (!Args.hasArg(options::OPT_shared)) +SharedRuntimes.push_back("hwasan-preinit"); } ---

[PATCH] D120437: [HWASAN] erase lifetime intrinsics if tag is outside.

eugenis accepted this revision. eugenis added a comment. This revision is now accepted and ready to land. LGTM Comment at: clang/test/CodeGen/lifetime-sanitizer.c:9 +// RUN: %clang -target aarch64-linux-gnu -S -emit-llvm -o /dev/null -O0 \ +// RUN: -fsanitize=hwaddress -mll

[PATCH] D120437: [HWASAN] erase lifetime intrinsics if tag is outside.

eugenis added inline comments. Comment at: clang/test/CodeGen/lifetime-sanitizer.c:9 +// RUN: %clang -target aarch64-linux-gnu -S -emit-llvm -o /dev/null -O0 \ +// RUN: -fsanitize=hwaddress -mllvm -print-before=hwasan %s 2>&1 | \ // RUN: FileCheck %s -check-prefix=LIFETI

[PATCH] D100581: [Clang] -Wunused-but-set-parameter and -Wunused-but-set-variable

eugenis added a comment. I fixed another one in https://reviews.llvm.org/rG561f4b9087457e9ea3cf4aeb57dcd507e2fa6258 Please watch the bots after you land changes like this one and don't let them stay broken for an entire day! Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION ht

[PATCH] D100581: [Clang] -Wunused-but-set-parameter and -Wunused-but-set-variable

eugenis added a comment. Right, our commit process is a bit strange. Ideally, the change author should initiate commit after they have the necessary approvals, and then be responsible to deal with any fallout in a timely manner. As things are, the author does not control the timing of the commi

[PATCH] D116633: Add -fsanitize-address-param-retval to clang.

eugenis added inline comments. Comment at: clang/lib/CodeGen/CGCall.cpp:2384 +if ((CodeGenOpts.EnableNoundefAttrs || + CodeGenOpts.SanitizeMemoryParamRetval) && +ArgNoUndef) vitalybuka wrote: > vitalybuka wrote: > > @eugenis Would be better to

[PATCH] D116633: Add -fsanitize-address-param-retval to clang.

eugenis added inline comments. Comment at: clang/lib/CodeGen/CGCall.cpp:2384 +if ((CodeGenOpts.EnableNoundefAttrs || + CodeGenOpts.SanitizeMemoryParamRetval) && +ArgNoUndef) vitalybuka wrote: > vitalybuka wrote: > > kda wrote: > > > eugenis wr

[PATCH] D116182: [ASan] Moved optimized callbacks into a separate library.

eugenis added a comment. I don't like the name "asan_dso". DSO means "dynamic shared object", and this is the very opposite of that. Maybe "asan_private" or "asan_helper"? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D116182/new/ https://reviews.l

[PATCH] D116182: [ASan] Moved optimized callbacks into a separate library.

eugenis added a comment. Maybe `asan_client` (like `stats_client` above), or `asan_inline`? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D116182/new/ https://reviews.llvm.org/D116182 ___ cfe-commits mail

[PATCH] D98892: [HWASan] Mention x86_64 aliasing mode in design doc.

eugenis accepted this revision. eugenis added a comment. This revision is now accepted and ready to land. LGTM Comment at: clang/docs/HardwareAssistedAddressSanitizerDesign.rst:279 + +HWASAN's approach is not applicable to 32-bit architectures. This statement

[PATCH] D82317: [Clang/Test]: Update tests where `noundef` attribute is necessary

eugenis added a comment. I'm not working on this currently, but I would appreciate and support such change. It's a big diff, but IMHO it is the cleanest, most maintainable approach. Gui, WDYT? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D82317/n

[PATCH] D99368: [compiler-rt][hwasan] Add C++17 new/delete operators with alignment

eugenis accepted this revision. eugenis added a comment. LGTM Comment at: compiler-rt/lib/hwasan/hwasan_new_delete.cpp:104 +INTERCEPTOR_ATTRIBUTE SANITIZER_WEAK_ATTRIBUTE void operator delete( +void *ptr, std::align_val_t align)NOEXCEPT { + OPERATOR_DELETE_BODY; --

[PATCH] D81678: Introduce noundef attribute at call sites for stricter poison analysis

eugenis added a comment. In D81678#2503931 , @nikic wrote: > As the discussion is spread out across multiple threads, do I understand > correctly that the current consensus is to introduce the > `-disable-noundef-analysis` flag, and explicitly add it to

[PATCH] D105169: [Clang/Test]: Rename enable_noundef_analysis to disable-noundef-analysis and turn it off by default

eugenis added a comment. @hyeongyukim, thank you for the summary. This looks like a great change, and a net positive to me. The test churn in the other patch is unfortunate, but IMHO unavoidable. If no one has any further objections, LGTM Repository: rG LLVM Github Monorepo CHANGES SINCE

[PATCH] D111344: [HWASan] Use tagged-globals feature on x86.

eugenis accepted this revision. eugenis 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/D111344/new/ https://reviews.llvm.org/D111344 _

[PATCH] D111443: [Driver] Fix ToolChain::getSanitizerArgs

eugenis added a comment. Don't we want to diagnose the problems in the job's command line? What kind of changes can the driver do there that would affect SanitizerArgs? I wonder if diagnostics should still be performed on the job args, but presented to the user differently, mainly because we ca

[PATCH] D111443: [Driver] Fix ToolChain::getSanitizerArgs

eugenis added a comment. Right, but a cache for SanitizerArgs is not enough to avoid repeated diagnostics, is it? Ex. if I request a non-existing sanitizer, I think I would get errors from host arg parsing, as well as from each of device1 and device2, because each device will have a unique ArgL

[PATCH] D108453: [Clang/Test]: Rename enable_noundef_analysis to disable-noundef-analysis and turn it off by default (2)

eugenis accepted this revision. eugenis 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/D108453/new/ https://reviews.llvm.org/D108453 _

<    1   2   3