[PATCH] D56109: [sanitizer_common] Define __sanitizer_FILE on NetBSD

eugenis added a comment. In D56109#1341967 , @mgorny wrote: > We've been discussing this, and I think we're doing this the wrong way. Could > you help me a little understand this? > > In particular, what is the purpose of unpoisoning file? Is it in order

[PATCH] D45290: [Driver] Use the per-API level Android library directories.

eugenis accepted this revision. eugenis added a comment. This revision is now accepted and ready to land. LGTM (did not check the tests). Comment at: lib/Driver/ToolChains/Linux.cpp:25 #include "llvm/Support/Path.h" +#include "llvm/Support/ScopedPrinter.h" #include -

[PATCH] D45046: hwasan: add -fsanitize=kernel-hwaddress

eugenis added a comment. LGTM (+missing *-commits MLs) https://reviews.llvm.org/D45046 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D48570: [Driver] Do not add -lpthread & -lrt with -static-libsan on Android

eugenis accepted this revision. eugenis added a comment. This revision is now accepted and ready to land. LGTM This has always been the case. Repository: rC Clang https://reviews.llvm.org/D48570 ___ cfe-commits mailing list cfe-commits@lists.llvm

[PATCH] D49492: Run bounds checking sanitizer earlier to make it easier to optimize away its checks.

eugenis added a comment. ObjectSizeOffsetEvaluator may fail to trace the address back to the underlying object, and we will end up not inserting the check. Does ChecksUnable statistic go up with this change? Repository: rC Clang https://reviews.llvm.org/D49492

[PATCH] D49492: Run bounds checking sanitizer earlier to make it easier to optimize away its checks.

eugenis added a comment. Some checks can be removed by asking SCEV for offset bounds, see SafeStack::IsAccessSafe for an example. Repository: rC Clang https://reviews.llvm.org/D49492 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http:/

[PATCH] D69574: Remove lazy thread-initialisation

eugenis added a comment. LGTM keeping the flag looks like the right thing to do. I'll leave this for @pcc to accept. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D69574/new/ https://reviews.llvm.org/D69574 ___

[PATCH] D80039: [NFC, StackSafety] LTO tests for MTE and StackSafety

eugenis accepted this revision. eugenis added a comment. This revision is now accepted and ready to land. LGTM, but the comments at the start of each run are confusing - they describe the desired, not the current state. Maybe move FIXME on the same line? Repository: rG LLVM Github Monorepo C

[PATCH] D80771: [MTE] Convert StackSafety into analysisThis lets us to remove !stack-safe metadata andbetter controll when to perform StackSafetyanalysis.

eugenis added inline comments. Comment at: clang/test/Driver/memtag_lto.c:126 + // XUNSAFE: [4]: full-set + // XSAFE: [4]: [0,4) int x; Alloca order can easily change in the future. Not sure how to make this better. Perhaps simply remove the numbers and tes

[PATCH] D80771: [MTE] Convert StackSafety into analysis

eugenis added inline comments. Comment at: clang/test/Driver/memtag_lto.c:126 + // XUNSAFE: [4]: full-set + // XSAFE: [4]: [0,4) int x; vitalybuka wrote: > eugenis wrote: > > Alloca order can easily change in the future. Not sure how to make this > > better

[PATCH] D80771: [MTE] Convert StackSafety into 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/D80771/new/ https://reviews.llvm.org/D80771 _

[PATCH] D80954: [NFC,MTE] Drop unneeded attribute from test

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

[PATCH] D81422: Change filecheck default to dump input on failure

eugenis accepted this revision. eugenis added a comment. This would help debugging sanitizer failures on the bots a lot. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D81422/new/ https://reviews.llvm.org/D81422 ___

[PATCH] D81244: [StackSafety] Control paramer access summary from frontend

eugenis added a comment. I like this in principle. Makes a lot more sense to control this directly from clang than to rely on function attributes. Comment at: llvm/lib/Bitcode/Writer/BitcodeWriterPass.cpp:31 + EmitModuleHash); + } return PreservedAnal

[PATCH] D81242: [StackSafety] Run ThinLTO

eugenis added inline comments. Comment at: llvm/lib/Analysis/StackSafetyAnalysis.cpp:618 +ConstantRange Access = Found->sextOrTrunc(Use.Range.getBitWidth()); +if (Access.signedAddMayOverflow(C.Offset) != +ConstantRange::OverflowResult::NeverOverflows)

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

eugenis added a comment. In D81678#2088284 , @efriedma wrote: > I usually like to start reading this sort of patch with the proposed LangRef > change, but I'm not seeing one. > > There are a couple of related issues here in the existing representation of

[PATCH] D81242: [StackSafety] Run ThinLTO

eugenis accepted this revision. eugenis added a comment. This revision is now accepted and ready to land. LGTM with 2 notes Comment at: llvm/lib/Analysis/StackSafetyAnalysis.cpp:618 +ConstantRange Access = Found->sextOrTrunc(Use.Range.getBitWidth()); +if (Access.signedA

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

eugenis added a comment. Positive attribute sounds good to me (frozen is not a bad name), but the tests update change is going to be huge. Any concerns about IR size bloat? The attribute will apply to the majority of function arguments, 8 bytes per instance as far as I can see. Good point abou

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

eugenis added a comment. In D81678#2099444 , @efriedma wrote: > So I guess we've discussed the following alternatives so far: > > 1. Attach the "frozen" attribute everywhere; this makes the textual IR > generated by clang messy, and likely bloats memory u

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

eugenis added a comment. > Could you explicitly state that if it is aggregate or vector type all > elements and paddings should be frozen too? If an aggregate is passed as an aggregate at IR level, we should not care about the padding. Unless it's coerced to an integer. Repository: rG LLVM

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

eugenis added inline comments. Comment at: clang/lib/CodeGen/CGCall.cpp:4095 +} + } call->setCallingConv(getRuntimeCC()); guiand wrote: > jdoerfert wrote: > > Why would we do this? Function attributes are valid at the call site, no > > need to copy them.

[PATCH] D82398: [MSAN] Handle x86 {round,min,max}sd intrinsics

eugenis added a comment. The test should be in LLVM, under test/Instrumentation/MemorySanitizer Comment at: llvm/lib/Transforms/Instrumentation/MemorySanitizer.cpp:3077 +Value *LowShadow = IRB.CreateOr(LowA, LowB); +Value *Shadow = IRB.CreateInsertElement(Second, LowSh

[PATCH] D82249: [HWASan] Disable GlobalISel/FastISel for HWASan Globals.

eugenis added a comment. LGTM I'm OK with this as a workaround, but it would be more natural to detect the unsupported IR pattern in globalisel and fall back instead of disabling it entirely. Is it difficult to do for some reason? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTI

[PATCH] D82249: [HWASan] Disable GlobalISel/FastISel for HWASan Globals.

eugenis added a comment. In D82249#2110054 , @hctim wrote: > In D82249#2109920 , @eugenis wrote: > > > I'm OK with this as a workaround, but it would be more natural to detect > > the unsupported IR pattern in glob

[PATCH] D82398: [MSAN] Handle x86 {round,min,max}sd intrinsics

eugenis added inline comments. Comment at: llvm/lib/Transforms/Instrumentation/MemorySanitizer.cpp:3077 +Value *LowShadow = IRB.CreateOr(LowA, LowB); +Value *Shadow = IRB.CreateInsertElement(Second, LowShadow, IRB.getInt32(0)); + guiand wrote: > eugenis

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

eugenis added a comment. >> Also, what's the plan to detect these cases in ubsan? > > I don't think this has any practical impact on our goals with sanitizers. We > should detect undefined behavior before it gets to the point of actually > passing or returning an undef or poison value. MSan w

[PATCH] D82398: [MSAN] Handle x86 {round,min,max}sd intrinsics

eugenis accepted this revision. eugenis added a comment. This revision is now accepted and ready to land. LGTM Comment at: llvm/lib/Transforms/Instrumentation/MemorySanitizer.cpp:3077 +Value *LowShadow = IRB.CreateOr(LowA, LowB); +Value *Shadow = IRB.CreateInsertElement

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

eugenis added a comment. In D82317#2200789 , @rjmccall wrote: > Are you seriously adding an attribute to literally every argument and return > value? Why is this the right representation? We've discussed quite a few options in D81678

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

eugenis added inline comments. Comment at: clang/lib/CodeGen/CGAtomic.cpp:315 + llvm::FunctionCallee fn = + CGF.CGM.CreateRuntimeFunction(fnTy, fnName, fnAttrs); auto callee = CGCallee::forDirect(fn); This needs a clang test, and better move it to a sepa

[PATCH] D85573: [CGAtomic] Mark atomic libcall functions `nounwind`

eugenis added a comment. While we are here, how about setting a few more attributes? argmemonly, readonly/writeonly, willreturn come to mind. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D85573/new/ https://reviews.llvm.org/D85573

[PATCH] D85573: [CGAtomic] Mark atomic libcall functions `nounwind`

eugenis added a comment. nocapture on the pointer argument Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D85573/new/ https://reviews.llvm.org/D85573 ___ cfe-commits mailing list cfe-commits@lists.llvm.org

[PATCH] D85573: [CGAtomic] Mark atomic libcall functions `nounwind`

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

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

eugenis added inline comments. Comment at: llvm/lib/Transforms/Instrumentation/MemorySanitizer.cpp:3513 +assert(InsPoint); +if (!InsPoint) { + return; guiand wrote: > TODO: Remove this redudant call you've already checked it with assert(isa) - which

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

eugenis added a comment. In D82317#2212145 , @jdoerfert wrote: > In D82317#2211643 , @guiand wrote: > >> After discussing with @eugenis, for the meantime it might be best to do the >> following: >> >> - Change the

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

eugenis accepted this revision. eugenis added a comment. This revision is now accepted and ready to land. LGTM with 1 comment Comment at: llvm/lib/Transforms/Instrumentation/MemorySanitizer.cpp:3573 + case LibFunc_atomic_load: +if (!isa(CB)) { + llvm::errs

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

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

[PATCH] D79522: Allow -fsanitize-minimal-runtime with memtag sanitizer.

eugenis created this revision. eugenis added a reviewer: pcc. Herald added a subscriber: cryptoad. Herald added a project: clang. MemTag does not have any runtime at the moment, it's strictly code instrumentation. Repository: rG LLVM Github Monorepo https://reviews.llvm.org/D79522 Files: c

[PATCH] D79522: Allow -fsanitize-minimal-runtime with memtag sanitizer.

This revision was automatically updated to reflect the committed changes. Closed by commit rGb4aa71e1bd9a: Allow -fsanitize-minimal-runtime with memtag sanitizer. (authored by eugenis). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D79522/new/ https:

[PATCH] D80046: [StackSafety] Make full LTO to attach metadata if MTE is enabled

eugenis added a comment. I think this is an OK approach. Perhaps a module flag would be even better, then it can be decoupled from "sanitize_memtag", and it would not require iterating over the entire module. You can check how it is used in CFI: !"CFI Canonical Jump Tables". It could also be be

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

eugenis added inline comments. Comment at: docs/TaggedAddressSanitizerDesign.rst:2 +=== +TaggedAddressSanitizer Design Documentation +=== I vote for HardwareAssistedAddressSanitizer,

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

eugenis 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. kcc wrote: > davidxl wrote: > > W

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

eugenis accepted this revision. eugenis added a comment. This revision is now accepted and ready to land. Looks great! Repository: rC Clang https://reviews.llvm.org/D40568 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.

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

eugenis created this revision. Herald added subscribers: javed.absar, srhines. Driver, frontend and LLVM codegen for HWASan. A clone of ASan, basically. https://reviews.llvm.org/D40936 Files: clang/include/clang/Basic/Sanitizers.def clang/include/clang/Driver/SanitizerArgs.h clang/lib/Cod

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

eugenis added inline comments. Comment at: clang/include/clang/Basic/Sanitizers.def:47 +SANITIZER("hwaddress", HWAddress) + alekseyshl wrote: > Why didn't we follow KASan style and name it "hw-address"? A matter of taste, I guess. I don't want the attribute to

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

eugenis updated this revision to Diff 126060. eugenis added a comment. clang-format https://reviews.llvm.org/D40936 Files: clang/include/clang/Basic/Sanitizers.def clang/include/clang/Driver/SanitizerArgs.h clang/lib/CodeGen/BackendUtil.cpp clang/lib/CodeGen/CGDeclCXX.cpp clang/lib/Co

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

This revision was automatically updated to reflect the committed changes. Closed by commit rC320232: Hardware-assisted AddressSanitizer (clang part). (authored by eugenis). Changed prior to commit: https://reviews.llvm.org/D40936?vs=126060&id=126251#toc Repository: rC Clang https://reviews.

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

This revision was automatically updated to reflect the committed changes. Closed by commit rL320232: Hardware-assisted AddressSanitizer (clang part). (authored by eugenis). Changed prior to commit: https://reviews.llvm.org/D40936?vs=126060&id=126252#toc Repository: rL LLVM https://reviews.l

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

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

[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 https://lists.llvm.or

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

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

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

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

[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] 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] 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)">; + "'-fsanitize=memtag-stack

[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 (+mem

[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: clang/li

[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] 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; + re

[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/ https://reviews.llvm.org/D11

[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: clang/lib/CodeGe

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

[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 cfe-commits@lists.llv

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

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

[PATCH] D66695: msan, codegen, instcombine: Keep more lifetime markers used for msan

eugenis accepted this revision. eugenis added a comment. This revision is now accepted and ready to land. LGTM Comment at: llvm/test/Transforms/InstCombine/lifetime-sanitizer.ll:37-50 +define void @msan() sanitize_memory { +entry: + ; CHECK-LABEL: @msan( + %text = alloca i8,

[PATCH] D66697: hwasan, codegen: Keep more lifetime markers used for hwasan

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

[PATCH] D34590: [ubsan] Diagnose invalid uses of builtins (clang)

eugenis accepted this revision. eugenis added a comment. This revision is now accepted and ready to land. Looks fine to me. https://reviews.llvm.org/D34590 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman

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

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

[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/ https://reviews.llvm.org/D8167

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

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

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

[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: clang/docs/ControlFlow

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

[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 cfe-commits@lists.llvm.

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

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

[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: clang/include/clang/Basic/CodeGenOptions.

[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] 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 cfe-commits@lists.llvm.

[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 https://reviews.

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

[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] 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 https://lists.llvm.org/cgi

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

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

<    1   2   3   >