[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 ___

[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] 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] 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

[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] D128958: Add assembler plumbing for sanitize_memtag

eugenis added inline comments. Comment at: llvm/lib/CodeGen/AsmPrinter/AsmPrinter.cpp:713 + if (GV->hasSanitizerMetadata() && GV->isTagged()) { +Triple T = TM.getTargetTriple(); isTagged() already includes a check for hasSanitizerMetadata() Repository:

[PATCH] D133392: [MTE] Add AArch64GlobalsTagging Pass

eugenis added a comment. Change description says that the new pass "marks them as tagged". That's not what is happening. Comment at: llvm/lib/CodeGen/GlobalMerge.cpp:657 +// Tagged global variables shouldn't be merged, as they are assigned unique +// memory tags at

[PATCH] D131438: [clang][dataflow] Analyze constructor bodies

eugenis added a comment. Sorry but I had to revert this because of the conflicts with another revert: https://reviews.llvm.org/D131065 Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D131438/new/ https://reviews.llvm.org/D131438

[PATCH] D131065: [clang][dataflow] Store DeclContext of block being analysed in Environment if available.

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

[PATCH] D131065: [clang][dataflow] Store DeclContext of block being analysed in Environment if available.

eugenis added a comment. MSan is not happy: https://lab.llvm.org/buildbot/#/builders/74/builds/12717 Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D131065/new/ https://reviews.llvm.org/D131065 ___

[PATCH] D129492: Add missing sanitizer metadata plumbing from CFE.

eugenis accepted this revision. eugenis added a comment. This revision is now accepted and ready to land. LGTM Is there an ASan test to update? I guess before this change, for an ignorelisted global, ASan would emit wrongly typed declaration on the undef side - which should not cause any

[PATCH] D128950: Remove 'no_sanitize_memtag'. Add 'sanitize_memtag'.

eugenis accepted this revision. eugenis added a comment. This revision is now accepted and ready to land. LGTM Comment at: llvm/include/llvm/IR/GlobalValue.h:349 +const SanitizerMetadata& Meta = getSanitizerMetadata(); +return Meta.Memtag; + } `return

[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] 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] 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] 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:

[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

[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

[PATCH] D124493: Move Sanitizer metadata to be on-GlobalValue.

eugenis added a comment. I've always viewed the current implementation as a hack, and believed this data should live in debug info. It can be convenient to get symbolized reports for global overflow bug in a stripped binary, but those bugs are quite rare, and "symbolization" only affects the

[PATCH] D118948: [MTE] Add -fsanitize=memtag* and friends.

eugenis accepted this revision. eugenis added a comment. LGTM Comment at: clang/include/clang/Basic/DiagnosticDriverKinds.td:475 def err_stack_tagging_requires_hardware_feature : Error< - "'-fsanitize=memtag' requires hardware support (+memtag)">; +

[PATCH] D122685: [msan] Add link to the lifetime definition

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/D122685/new/ https://reviews.llvm.org/D122685

[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

[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

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

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

[PATCH] D119600: Stricter use-after-dtor detection for trivial members.

eugenis created this revision. eugenis added reviewers: vitalybuka, kda. eugenis requested review of this revision. Herald added a project: clang. Poison trivial class members one-by-one in the reverse order of their construction, instead of all-at-once at the very end. For example, in the

[PATCH] D119300: Use-after-dtor detection for trivial base classes.

eugenis updated this revision to Diff 408071. eugenis added a comment. Fix handling of empty base classes and suppress tail calls. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D119300/new/ https://reviews.llvm.org/D119300 Files:

[PATCH] D119299: [NFC] clang-format one function.

This revision was automatically updated to reflect the committed changes. Closed by commit rGa730b6a41ad7: [NFC] clang-format one function. (authored by eugenis). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D119299/new/

[PATCH] D119367: [HWASan] Allow no_sanitize(..) and change metadata passing.

eugenis added a comment. You might want to update Bitcode writer/reader as well, otherwise nosanitize will be lost after a trip through .bc/.ii. Comment at: compiler-rt/test/hwasan/TestCases/global-with-reduction.c:50 + f()[atoi(argv[1])] = 1; + f()[atoi(argv[1])] = 1; +

[PATCH] D119300: Use-after-dtor detection for trivial base classes.

eugenis created this revision. eugenis added reviewers: kda, vitalybuka. eugenis requested review of this revision. Herald added projects: clang, Sanitizers. Herald added a subscriber: Sanitizers. -fsanitize-memory-use-after-dtor detects memory access after a subobject is destroyed but its memory

[PATCH] D119299: [NFC] clang-format one function.

eugenis created this revision. eugenis added reviewers: kda, vitalybuka. eugenis requested review of this revision. Herald added a project: clang. fix code formatting Repository: rG LLVM Github Monorepo https://reviews.llvm.org/D119299 Files: clang/lib/CodeGen/CGClass.cpp Index:

[PATCH] D118948: [MTE] Add -fsanitize=memtag* and friends.

eugenis added inline comments. Comment at: clang/include/clang/Basic/DiagnosticDriverKinds.td:475 def err_stack_tagging_requires_hardware_feature : Error< - "'-fsanitize=memtag' requires hardware support (+memtag)">; + "'-fsanitize=memtag-stack' requires hardware support

[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] 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 ___

[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

[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

[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

[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/

[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] 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

[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] 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

[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

[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?

[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

[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

[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

[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

[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] 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] 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: > > > >

[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] 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] 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

[PATCH] D99381: [compiler-rt][hwasan] Remove __sanitizer allocation functions from hwasan interface

eugenis added a comment. They are used here: https://cs.android.com/android/platform/superproject/+/master:bionic/libc/bionic/malloc_common.h;l=54;drc=f3968e89cb72400951f93a2a8237ac1428d2627c Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D99381/new/

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

eugenis accepted this revision. eugenis added a comment. In D105703#2887005 , @fmayer wrote: > I removed the stack-safety-analysis-asm.c test because I don't think it > really adds anything and it caused problems. SGTY? Absolutely. LGTM Repository:

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

eugenis added a comment. In D105703#2884056 , @vitalybuka wrote: > In D105703#2883666 , @eugenis wrote: > >> Btw Vitaly, will this work with LTO out of the box? I think we used to add >> pre-LTO StackSafety

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

eugenis added a comment. Btw Vitaly, will this work with LTO out of the box? I think we used to add pre-LTO StackSafety pass explicitly for memtag only, but it looks like that code is gone. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION

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

eugenis accepted this revision. eugenis added a comment. This revision is now accepted and ready to land. LGTM Comment at: clang/lib/CodeGen/BackendUtil.cpp:1174 +CompileKernel, Recover, +/*IsOptNull=*/CodeGenOpts.OptimizationLevel == 0)); }

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

eugenis added inline comments. Comment at: llvm/lib/Transforms/Instrumentation/HWAddressSanitizer.cpp:407 bool Recover; + bool DisableOptimization; }; No need to pass this down, just look at OptimizeNone function attribute. Repository: rG LLVM Github

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

eugenis added inline comments. Comment at: llvm/test/CodeGen/hwasan-stack-safety-analysis-asm.c:1 +// RUN: %clang -fsanitize=hwaddress -target aarch64-linux-gnu -S -mllvm -hwasan-use-stack-safety=true -mllvm -hwasan-generate-tags-with-calls -O2 %s -o - | FileCheck %s

[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] 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

[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] 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

[PATCH] D103564: [NFC][compiler-rt][hwasan] Wrap vfork interceptor with HWASAN_WITH_INTERCEPTORS

eugenis added a comment. s/vfork/fork/ in the description I'm afraid removing the fork interceptor will break Android. This is a pretty uncommon condition, but still. It is interesting to note that ASan does not have this fork protection, see https://github.com/google/sanitizers/issues/774

[PATCH] D102592: [sanitizer] Caught global buffer underflow for first variable

eugenis added a comment. I think it is too hit-and-miss to add even under a flag. It's just not the right approach - but I also don't know what the right approach would be. Perhaps adding a small left redzone for all globals and reducing the right redzone to keep the total size under control?

[PATCH] D102592: [sanitizer] Caught global buffer underflow for first variable

eugenis added a comment. You are adding a new global to every translation unit. - Private linkage would not allow them to be merged, this can have significant binary size and RAM overhead. - There is no guarantee that any of these globals will end up to the left of any sanitized globals. With

[PATCH] D102469: [sanitizer] Reduce redzone size for small size global objects

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

[PATCH] D102288: [HWASan] Add -fsanitize=lam flag and enable HWASan to use it.

eugenis added inline comments. Comment at: clang/include/clang/Driver/Options.td:4172 +// on x86_64 and is subject to change in the future. For example, we may want +// to distinguish between LAM48 and LAM57 at some point. +def mlam : Flag<["-"], "mlam">, Group;

[PATCH] D102288: [HWASan] Add -fsanitize=lam flag and enable HWASan to use it.

eugenis added inline comments. Comment at: clang/include/clang/Basic/Sanitizers.def:55-59 +// Utilize Intel LAM in sanitizers. Currently only used in combination with +// -fsanitize=hwaddress. This is an experimental flag which may be removed in +// the future. +// TODO: Use

[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

[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

[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] 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

[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

[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] 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] 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. +

[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] 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] D85788: [Clang test] Update to allow passing extra default clang arguments in use_clang

eugenis added a comment. In D85788#2449299 , @aqjune wrote: > In D85788#2444136 , @guiand wrote: > >> IMO it's better to just one-and-done programatically add `-Xclang >> -disable-noundef-analysis` to all the

[PATCH] D85788: [Clang test] Update to allow passing extra default clang arguments in use_clang

eugenis requested changes to this revision. eugenis added a comment. This revision now requires changes to proceed. I wonder if we could just disable unused argument warning for both -disable-noundef-analysis -Xclang -disable-noundef-analysis I don't see any precedents for this kind of

[PATCH] D85788: [Clang test] Update to allow passing extra default clang arguments in use_clang

eugenis added a comment. > simply naming it as %clang_noundef would be clearer, Hmm, yeah, that's a little better. > This new substitution is going to be used for the noundef checks in D81678 > only anyway. I'm not sure what you mean by this. The substitution

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

eugenis added a comment. This change looks fine to me, but I'm slightly concerned about https://reviews.llvm.org/D85788 - see my last comment on that revision. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D81678/new/

[PATCH] D92728: [NFC][MSan] Round up OffsetPtr in PoisonMembers

eugenis accepted this revision. eugenis added a comment. This revision is now accepted and ready to land. LGTM Comment at: clang/lib/CodeGen/CGClass.cpp:1742 + Context.toCharUnitsFromBits(Layout.getFieldOffset(layoutStartOffset) + +

[PATCH] D92728: [NFC][MSan] Round up OffsetPtr in PoisonMembersgetFieldOffset(layoutStartOffset) for current calleds is expected topoint to the first trivial field or the one which follows non-trivial

eugenis added a comment. Don't you want to similarly align down PoisonEnd? But if this is something that should never happen, as your comment rightly suggests, wouldn't it be better to add an assert()? The same in the case when PoisonSize < 0 - it should never happen. Repository: rG LLVM

[PATCH] D92727: [CodeGen][MSan] Don't use offsets of zero-sized fields

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

[PATCH] D91258: [clangd] Sanity-check array sizes read from disk before allocating them.

eugenis added a comment. Hi Sam, this patch is failing on the ubsan bot with: [ RUN ] SerializationTest.NoCrashOnBadArraySize /b/sanitizer-x86_64-linux-fast/build/llvm-project/clang-tools-extra/clangd/index/Serialization.cpp:90:26: runtime error: left shift of 127 by 28 places cannot be

[PATCH] D90422: AArch64: Switch to x20 as the shadow base register for outlined HWASan checks.

eugenis accepted this revision. eugenis added a comment. LGTM I was confused about the ABI statement in the description at the first glance - could you reword it to make it clear that HWASan ABI is not affected? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION

[PATCH] D90424: AArch64: Use SBFX instead of UBFX to extract address granule in outlined HWASan checks.

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

[PATCH] D89766: Driver: Add integer sanitizers to trapping group automatically.

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/D89766/new/ https://reviews.llvm.org/D89766

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

eugenis updated this revision to Diff 298692. eugenis added a comment. fix type size check for vscale types Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D81678/new/ https://reviews.llvm.org/D81678 Files:

[PATCH] D85788: [Clang test] Update to allow passing extra default clang arguments in use_clang

eugenis added a comment. I don't like the %clang_bin substitution - imho it's unclear for the test authors when to use it instead of %clang, but I can't come up with a better idea. Is it true that %clang_bin is only necessary when the automatically added -disable-noundef-analysis flag

[PATCH] D85788: [Clang test] Update to allow passing extra default clang arguments in use_clang

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/D85788/new/ https://reviews.llvm.org/D85788

[PATCH] D87717: [docs] Update ControlFlowIntegrity.rst.

This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rG66cf68ed4678: [docs] Update ControlFlowIntegrity.rst. (authored by eugenis). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION

[PATCH] D87717: [docs] Update ControlFlowIntegrity.rst.

eugenis updated this revision to Diff 295888. eugenis added a comment. fix a typo Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D87717/new/ https://reviews.llvm.org/D87717 Files: clang/docs/ControlFlowIntegrity.rst Index:

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

eugenis added a comment. We really need to do something about this. How about a change that adds -fdisable-noundef-analysis to every RUN line with %clang? (-) We are not testing exactly the same mode that is used by the users - but that's already true for many other flags that clang driver

[PATCH] D87717: [docs] Update ControlFlowIntegrity.rst.

eugenis created this revision. eugenis added a reviewer: pcc. Herald added a subscriber: dexonsmith. Herald added a project: clang. eugenis requested review of this revision. Herald added a subscriber: aheejin. Expand the list of targets that support cfi-icall. Add ThinLTO everywhere LTO is

[PATCH] D86000: Add an unsigned shift base sanitizer

eugenis added a comment. In D86000#2219322 , @jfb wrote: > In D86000#2219288 , @vsk wrote: > >> It'd be nice to fold the new check into an existing sanitizer group to bring >> this to a wider audience. Do you

[PATCH] D85559: [MSAN] Reintroduce libatomic load/store instrumentation

eugenis added inline comments. Comment at: llvm/lib/Transforms/Instrumentation/MemorySanitizer.cpp:3573 + case LibFunc_atomic_load: +if (!isa(CB)) { + llvm::errs() << "MSAN -- cannot instrument invoke of libatomic load." guiand wrote: >

  1   2   3   >