[PATCH] D148694: [SanitizerBinaryMetadata] Respect no_sanitize("thread") function attribute

This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rG5f605e254a0f: [SanitizerBinaryMetadata] Respect no_sanitize("thread") function attribute (authored by melver). Repository: rG LLVM Github Monorepo

[PATCH] D148694: [SanitizerBinaryMetadata] Respect no_sanitize("thread") function attribute

melver updated this revision to Diff 514871. melver added a comment. Fix test Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D148694/new/ https://reviews.llvm.org/D148694 Files: clang/lib/CodeGen/CodeGenFunction.cpp clang/test/CodeGen/sanitize-m

[PATCH] D148694: [SanitizerBinaryMetadata] Respect no_sanitize("thread") function attribute

melver created this revision. melver added a reviewer: dvyukov. Herald added subscribers: Enna1, hiraditya. Herald added a project: All. melver requested review of this revision. Herald added projects: clang, LLVM. Herald added subscribers: llvm-commits, cfe-commits. To avoid false positives, resp

[PATCH] D145519: [SanitizerBinaryMetadata] Do not add to GPU code

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 rG61ed64954b97: [SanitizerBinaryMetadata] Do not add to GPU code (authored by melver). Repository: rG LLVM Github Monorep

[PATCH] D145519: [SanitizerBinaryMetadata] Do not add to GPU code

melver updated this revision to Diff 503326. melver marked an inline comment as done. melver added a comment. Simplify assert. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D145519/new/ https://reviews.llvm.org/D145519 Files: clang/lib/Driver/San

[PATCH] D145519: [SanitizerBinaryMetadata] Do not add to GPU code

melver created this revision. melver added a reviewer: dvyukov. Herald added subscribers: Enna1, hiraditya. Herald added a project: All. melver requested review of this revision. Herald added subscribers: llvm-commits, cfe-commits, MaskRay. Herald added projects: clang, LLVM. SanitizerBinaryMetada

[PATCH] D143664: [SanitizerBinaryMetadata] Support ignore list

melver marked an inline comment as done. melver added inline comments. Comment at: clang/test/CodeGen/sanitize-metadata-ignorelist.c:11 +// ALLOW-SAME: () local_unnamed_addr #[[ATTR0:[0-9]+]] !pcsections !5 { +// ALLOW-NEXT: entry: +// ALLOW-NEXT:[[TMP0:%.*]] = atomicrmw add

[PATCH] D143664: [SanitizerBinaryMetadata] Support ignore list

This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rG421215b919d0: [SanitizerBinaryMetadata] Support ignore list (authored by melver). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION

[PATCH] D143664: [SanitizerBinaryMetadata] Support ignore list

melver updated this revision to Diff 496353. melver marked an inline comment as done. melver added a comment. Make Driver test check that cc1 doesn't receive flag if not required. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D143664/new/ https://re

[PATCH] D143664: [SanitizerBinaryMetadata] Support ignore list

melver marked an inline comment as done. melver added inline comments. Comment at: clang/test/Driver/fsanitize-metadata-ignorelist.c:6 +// RUN: %clang -target aarch64-linux-gnu -fexperimental-sanitize-metadata=all -fexperimental-sanitize-metadata-ignorelist=%t.good -fexperiment

[PATCH] D143664: [SanitizerBinaryMetadata] Support ignore list

melver created this revision. melver added reviewers: dvyukov, vitalybuka. Herald added subscribers: Enna1, ormris, hiraditya. Herald added a project: All. melver requested review of this revision. Herald added subscribers: llvm-commits, cfe-commits, MaskRay. Herald added projects: clang, LLVM. Fo

[PATCH] D143634: [ModuleUtils] Assert correct linkage and visibility of structors' COMDAT key

melver added a comment. This is currently more an RFC - there might be other side-effects not yet accounted for, so please review carefully. Although I have been able to reproduce the issue with an LTO and ASan build quite easily. If this is a common usecase for us, we might potentially save a

[PATCH] D143634: [ModuleUtils] Assert correct linkage and visibility of structors' COMDAT key

melver created this revision. melver added reviewers: MaskRay, vitalybuka, fmayer, pcc. Herald added subscribers: luke, Enna1, kosarev, frasercrmck, kerbowa, luismarques, apazos, sameer.abuasal, s.egerton, Jim, jocewei, PkmX, the_o, brucehoult, MartinMosbeck, rogfer01, edward-jones, zzheng, jrtc2

[PATCH] D143484: [SanitizerBinaryMetadata] Emit constants as ULEB128

This revision was automatically updated to reflect the committed changes. Closed by commit rGbf9814b70560: [SanitizerBinaryMetadata] Emit constants as ULEB128 (authored by melver). Changed prior to commit: https://reviews.llvm.org/D143484?vs=495442&id=495802#toc Repository: rG LLVM Github Mo

[PATCH] D143482: [SanitizerBinaryMetadata] Optimize used space for features and UAR stack args

This revision was automatically updated to reflect the committed changes. Closed by commit rG3d53b5273003: [SanitizerBinaryMetadata] Optimize used space for features and UAR stack args (authored by melver). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.or

[PATCH] D143484: [SanitizerBinaryMetadata] Emit constants as ULEB128

melver updated this revision to Diff 495442. melver added a comment. Move AsmPrinter LEB128 helpers to AsmPrinter.cpp and use them. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D143484/new/ https://reviews.llvm.org/D143484 Files: clang/test/Code

[PATCH] D143484: [SanitizerBinaryMetadata] Emit constants as ULEB128

melver created this revision. melver added a reviewer: dvyukov. Herald added subscribers: Enna1, pengfei, hiraditya. Herald added a project: All. melver requested review of this revision. Herald added projects: clang, Sanitizers, LLVM. Herald added subscribers: llvm-commits, Sanitizers, cfe-commits

[PATCH] D143482: [SanitizerBinaryMetadata] Optimize used space for features and UAR stack args

melver created this revision. melver added a reviewer: dvyukov. Herald added subscribers: Enna1, hiraditya. Herald added a project: All. melver requested review of this revision. Herald added projects: clang, Sanitizers, LLVM. Herald added subscribers: llvm-commits, Sanitizers, cfe-commits. Optimi

[PATCH] D139276: [SanitizerBinaryMetadata] Use weak __start_/__stop_ instead of dummy empty section

melver accepted this revision. melver added a comment. This revision is now accepted and ready to land. Thanks - yes this looks better. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D139276/new/ https://reviews.llvm.org/D139276

[PATCH] D136078: Use-after-return sanitizer binary metadata

melver added a comment. In D136078#3969061 , @MaskRay wrote: > ` SanitizerBinaryMetadata::createZeroSizedObjectInSection` creates > `__dummy_*`, but why is it needed? (There is no comment.) " // Create a 0-sized object in a section, so that the sectio

[PATCH] D136078: Use-after-return sanitizer binary metadata

melver added a comment. In D136078#3966187 , @melver wrote: >>> Do you have any other ideas on how to dead with this? >> >> It's strange we've not encountered this elsewhere - this must be some >> special (old?) linker. I'll investigate... > > GNU ld <=

[PATCH] D136078: Use-after-return sanitizer binary metadata

melver added a comment. >> Do you have any other ideas on how to dead with this? > > It's strange we've not encountered this elsewhere - this must be some special > (old?) linker. I'll investigate... GNU ld <= 2.35 does not support mixed SHF_LINK_ORDER and non-SHF_LINK_ORDER sections. This i

[PATCH] D136078: Use-after-return sanitizer binary metadata

melver added a comment. In D136078#3964100 , @dvyukov wrote: > @melver Re this failure: > > TEST 'Clang :: > Instrumentation/SanitizerBinaryMetadata/uar.cpp' FAILED > Script: > -- > : 'RUN: at line 4';

[PATCH] D136078: Use-after-return sanitizer binary metadata

melver requested changes to this revision. melver added a comment. This revision now requires changes to proceed. AFAIK Windows bot will break: https://buildkite.com/llvm-project/premerge-checks/builds/124003#0184c872-af5b-47c0-938c-b31ee8808241 Need to restrict test to Linux only. Repository:

[PATCH] D136078: Use-after-return sanitizer binary metadata

melver added inline comments. Comment at: llvm/lib/CodeGen/SanitizerBinaryMetadata.cpp:57 + // Assume it currently only has features. + assert(AuxMDs.size() == 1); + auto *Features = cast(AuxMDs.getOperand(0))->getValue(); Probably should be getNumOperands()?

[PATCH] D136078: Use-after-return sanitizer binary metadata

melver accepted this revision. melver added inline comments. Comment at: llvm/test/Instrumentation/SanitizerBinaryMetadata/uar.cpp:19 +void non_empty_function() { + // Completely empty functions don't get uar metadata. + volatile int x; dvyukov wrote: > melver

[PATCH] D136078: Use-after-return sanitizer binary metadata

melver added inline comments. Comment at: llvm/test/Instrumentation/SanitizerBinaryMetadata/uar.cpp:19 +void non_empty_function() { + // Completely empty functions don't get uar metadata. + volatile int x; Is this comment out of place? Repository: rG LLVM G

[PATCH] D136078: Use-after-return sanitizer binary metadata

melver accepted this revision. melver added a comment. This revision is now accepted and ready to land. LGTM Comment at: llvm/include/llvm/CodeGen/MachinePassRegistry.def:205 DUMMY_MACHINE_FUNCTION_PASS("print-machine-cycles", MachineCycleInfoPrinterPass, ()) +DUMMY_MACHINE_F

[PATCH] D136078: Use-after-return sanitizer binary metadata

melver added inline comments. Comment at: llvm/test/Instrumentation/SanitizerBinaryMetadata/uar.cpp:1 +// We run the compiled binary + sizes of stack arguments depend on the arch. +// REQUIRES: native && target-x86_64 For LLVM IR tests, consider if utils/update_t

[PATCH] D136078: Use-after-return sanitizer binary metadata

melver added inline comments. Comment at: llvm/include/llvm/Transforms/Instrumentation/SanitizerBinaryMetadata.h:38 + +const char *const kSanitizerBinaryMetadataCoveredSection = "sanmd_covered"; +const char *const kSanitizerBinaryMetadataAtomicsSection = "sanmd_atomics"; ---

[PATCH] D136078: [RFC] Use-after-return binary metadata.

melver added a comment. In D136078#3867627 , @dvyukov wrote: > Unrelated, but looking at the metadata format more closely, I think this can > benefit from LEB128-like varlen encoding. > Function size is small. > Features are very small. > Stack args size

[PATCH] D130888: [Clang] Introduce -fexperimental-sanitize-metadata=

This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rGc4842bb2e98e: [Clang] Introduce -fexperimental-sanitize-metadata= (authored by melver). Changed prior to commit: https://reviews.llvm.org/D130888?

[PATCH] D130888: [Clang] Introduce -fexperimental-sanitize-metadata=

melver added inline comments. Comment at: clang/test/Driver/fsanitize-metadata.c:1 +// RUN: %clang --target=x86_64-linux-gnu -fexperimental-sanitize-metadata=all -fno-experimental-sanitize-metadata=all %s -### 2>&1 | FileCheck %s +// CHECK-NOT: -fexperimental-sanitize-metadata -

[PATCH] D130888: [Clang] Introduce -fexperimental-sanitize-metadata=

melver updated this revision to Diff 457779. melver marked 3 inline comments as done. melver added a comment. - Options.td: s/f_clang_Group/f_Group/, remove NoXarchOption - Driver test: also test -fexperimental-sanitize-metadata=all -fno-experimental-metadata= Repository: rG LLVM Github Monor

[PATCH] D130888: [Clang] Introduce -fexperimental-sanitize-metadata=

melver updated this revision to Diff 457637. melver added a comment. Don't split RUN lines. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D130888/new/ https://reviews.llvm.org/D130888 Files: clang/include/clang/Basic/CodeGenOptions.def clang/in

[PATCH] D130888: [Clang] Introduce -fexperimental-sanitize-metadata=

melver updated this revision to Diff 457543. melver added a comment. Add CodeGen test. PTAL. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D130888/new/ https://reviews.llvm.org/D130888 Files: clang/include/clang/Basic/CodeGenOptions.def clang/

[PATCH] D130888: [Clang] Introduce -fexperimental-sanitize-metadata=

melver updated this revision to Diff 456608. melver added a comment. Rebase. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D130888/new/ https://reviews.llvm.org/D130888 Files: clang/include/clang/Basic/CodeGenOptions.def clang/include/clang/Bas

[PATCH] D130888: [Clang] Introduce -fexperimental-sanitize-metadata=

melver updated this revision to Diff 450043. melver added a comment. Rebase. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D130888/new/ https://reviews.llvm.org/D130888 Files: clang/include/clang/Basic/CodeGenOptions.def clang/include/clang/Bas

[PATCH] D130888: [Clang] Introduce -fexperimental-sanitize-metadata=

melver added inline comments. Comment at: clang/lib/Driver/SanitizerArgs.cpp:839 + // Parse -f(no-)?sanitize-metadata. + for (const auto *Arg : Args) { +if (Arg->getOption().matches( MaskRay wrote: > Use `Args.getLastArg(...)` This won't work if someone doe

[PATCH] D130888: [Clang] Introduce -fexperimental-sanitize-metadata=

melver updated this revision to Diff 449671. melver marked 4 inline comments as done. melver added a comment. Address comments. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D130888/new/ https://reviews.llvm.org/D130888 Files: clang/include/clang

[PATCH] D130888: [Clang] Introduce -fexperimental-sanitize-metadata=

melver created this revision. Herald added a subscriber: ormris. Herald added a project: All. melver requested review of this revision. Herald added subscribers: cfe-commits, MaskRay. Herald added a project: clang. Introduces the frontend flag -fexperimental-sanitize-metadata=, which enables Sanit

[PATCH] D119816: [SanitizerBounds] Add support for NoSanitizeBounds function

This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rG17ce89fa8016: [SanitizerBounds] Add support for NoSanitizeBounds function (authored by ztong0001, committed by melver). Changed prior to commit: h

[PATCH] D119816: [SanitizerBounds] Add support for NoSanitizeBounds function

melver added a comment. In D119816#3349076 , @ztong0001 wrote: > Hi Marco, > @melver, Could you please help me landing it? I don't have write permission > to the repo. > Please use Tong Zhang Sure. I had already applied the patch locally to test, but th

[PATCH] D119816: [SanitizerBounds] Add support for NoSanitizeBounds function

melver accepted this revision. melver added a comment. This revision is now accepted and ready to land. LGTM Let me know if you need help landing this. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D119816/new/ https://reviews.llvm.org/D119816 ___

[PATCH] D119816: [SanitizerBounds] Add support for NoSanitizeBounds function

melver added inline comments. Comment at: clang/lib/CodeGen/CodeGenFunction.cpp:757 SanOpts.set(SanitizerKind::HWAddress, false); + if (mask & SanitizerKind::LocalBounds) +Fn->addFnAttr(llvm::Attribute::NoSanitizeBounds); ztong0001 wrote: >

[PATCH] D119816: [SanitizerBounds] Add support for NoSanitizeBounds function

melver added inline comments. Comment at: clang/lib/CodeGen/CodeGenFunction.cpp:757 SanOpts.set(SanitizerKind::HWAddress, false); + if (mask & SanitizerKind::LocalBounds) +Fn->addFnAttr(llvm::Attribute::NoSanitizeBounds); These 2 checks can

[PATCH] D119816: [SanitizerBounds] Add support for NoSanitizeBounds function

melver requested changes to this revision. melver added a comment. This revision now requires changes to proceed. Looks good. Few minor changes. I did some more digging, and it's only fsanitize=local-bounds, so please verify this and also update the commit description. In fact, the Linux kernel

[PATCH] D119816: [SanitizerBounds] Add support for NoSanitizeBounds function

melver requested changes to this revision. melver added a comment. This revision now requires changes to proceed. Thanks - this looks good so far. Comment at: clang/test/CodeGen/sanitize-coverage.c:56 // BOUNDS-NOT: call void @__ubsan_handle_out_of_bounds + // BOUNDS-NOT: c

[PATCH] D119816: Fix not working attribute no_sanitize bounds that affects linux kernel

melver added a comment. Adding a new IR attribute comes with a whole slew of other required changes. Please see https://reviews.llvm.org/D102772 for an example. In addition, please update the patch description to explain what the problem is exactly (remove the old kernel-specific problem, becau

[PATCH] D119816: Fix not working attribute no_sanitize bounds that affects linux kernel

melver added a comment. Right, I was able to repro this. The problem is the trap, which generally sucks that no_sanitize still leaves in the trap. We also have -fno-sanitize-undefined-trap-on-error, which seems to have no effect either (should it?). So I think there are 2 problems: 1. Clang s

[PATCH] D119816: Fix not working attribute no_sanitize bounds that affects linux kernel

melver added a comment. What about this test then: https://github.com/llvm/llvm-project/blob/b0a0df980927ca54a7840a1b0c9766e98c05039b/clang/test/CodeGen/sanitize-coverage.c#L74 Can you show an independent C reproducer where no_sanitize does not work for you? Is there an LKML discussion? I als

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

melver added a comment. I think we dropped the ball on this - was it ever re-reverted? Is it still worth trying to land this? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D114421/new/ https://reviews.llvm.org/D114421 __

[PATCH] D117232: [clang] Improve -Wdeclaration-after-statement

This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rGc65186c89f35: [clang] Improve -Wdeclaration-after-statement (authored by melver). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION

[PATCH] D117232: [clang] Improve -Wdeclaration-after-statement

melver updated this revision to Diff 401689. melver marked an inline comment as done. melver added a comment. Use ``..`` in ReleaseNotes.rst. Thanks! Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D117232/new/ https://reviews.llvm.org/D117232 Files

[PATCH] D117232: [clang] Improve -Wdeclaration-after-statement

melver updated this revision to Diff 401653. melver marked 4 inline comments as done. melver retitled this revision from "[clang] Respect -Wdeclaration-after-statement with C99 or later" to "[clang] Improve -Wdeclaration-after-statement". melver edited the summary of this revision. melver added a

[PATCH] D117232: [clang] Respect -Wdeclaration-after-statement with C99 or later

melver added a comment. Looks like https://reviews.llvm.org/D114787 does the same thing and was submitted a week ago. I'll try to integrate the additional testing you suggested and change this patch to improve the implementation to not do the checking if not required as it doesn't look entirel

[PATCH] D117232: [clang] Respect -Wdeclaration-after-statement with C99 or later

melver added a comment. Hello, A quick review would be much appreciated. I intend to submit this before Feb 1 so it may be included in Clang 14. In case I have not added the appropriate reviewer(s) of the modified code, please let me know. Thank you. Repository: rG LLVM Github Monorepo C

[PATCH] D117232: [clang] Respect -Wdeclaration-after-statement with C99 or later

melver created this revision. melver added reviewers: aaron.ballman, dblaikie, rsmith. melver requested review of this revision. Herald added a project: clang. Herald added a subscriber: cfe-commits. GCC allows using -Wdeclaration-after-statement to warn when mixing code and declarations not just

[PATCH] D115440: Provide __builtin_alloca*_uninitialized variants

This revision was automatically updated to reflect the committed changes. Closed by commit rG732ad8ea62ed: [clang][auto-init] Provide __builtin_alloca*_uninitialized variants (authored by melver). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D115440/

[PATCH] D115440: Provide __builtin_alloca*_uninitialized variants

melver added a comment. Hello, We need a solution for this problem, and because I haven't heard any objections I'll assume the general approach is fine -- @jfb already kindly confirmed that "[...] patch you propose here is a good idea". Review of the implementation would still be much apprecia

[PATCH] D116128: [clang][driver] Warn when '-mno-outline-atomics' is used with a non-AArch64 triple

melver accepted this revision. melver added inline comments. Comment at: clang/test/Driver/x86-outline-atomics.c:1-7 +// RUN: %clang -target x86_64 -moutline-atomics -S %s -### 2>&1 | FileCheck %s -check-prefix=CHECK-OUTLINE-ATOMICS +// CHECK-OUTLINE-ATOMICS: warning: 'x86_64' d

[PATCH] D115440: Provide __builtin_alloca*_uninitialized variants

melver added a subscriber: tstellar. melver added a comment. @tstellar , we were told maybe you have some input. TLDR; - Clang and GCC appear to behave differently wrt `-ftrivial-auto-var-init=` on `__builtin_alloca()` (and VLAs?) - Clang's behavior is (per @jfb above) correct; - How to documen

[PATCH] D115440: Provide __builtin_alloca*_uninitialized variants

melver reclaimed this revision. melver added a comment. > Let's consult libc's own documentation > https://www.gnu.org/software/libc/manual/html_node/Variable-Size-Automatic.html > > It states: > >> Automatic Storage with Variable Size >> The function alloca supports a kind of half-dynamic alloca

[PATCH] D115440: Provide __builtin_alloca*_uninitialized variants

melver abandoned this revision. melver added a comment. GCC devs say that initializing explicit alloca() is a bug, because they aren't "automatic storage": https://lkml.kernel.org/r/20211209201616.gu...@gate.crashing.org .. which is also the reason why GCC's behaviour differs here at the moment.

[PATCH] D115440: [clang][auto-init] Provide __builtin_alloca_uninitialized

melver updated this revision to Diff 393122. melver marked an inline comment as done. melver added a comment. For completeness, also provide __builtin_alloca_with_align_uninitialized(). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D115440/new/ http

[PATCH] D115440: [clang][auto-init] Provide __builtin_alloca_uninitialized

melver added inline comments. Comment at: clang/lib/CodeGen/CGBuiltin.cpp:3436 case Builtin::BI__builtin_alloca_with_align: { Value *Size = EmitScalarExpr(E->getArg(0)); glider wrote: > For the sake of completeness, shall we also implement an uninitiali

[PATCH] D115440: [clang][auto-init] Provide __builtin_alloca_uninitialized

melver created this revision. melver added reviewers: jfb, pcc, glider. melver requested review of this revision. Herald added a project: clang. Herald added a subscriber: cfe-commits. When `-ftrivial-auto-var-init=` is enabled, allocas unconditionally receive auto-initialization since [1]. In ce

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

melver accepted this revision. melver added a comment. This revision is now accepted and ready to land. Thanks! Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D114421/new/ https://reviews.llvm.org/D114421

[PATCH] D108555: [tsan] Do not include from sanitize-thread-disable.c

melver added a comment. In D108555#2960037 , @glider wrote: > In D108555#2960034 , @melver wrote: > >> LGTM, thanks! >> >> Patch title ("...an X86-only test..") also needs adjustment. > > It's strange that Phab do

[PATCH] D108555: [tsan] Make sanitize-thread-disable.c an X86-only test

melver accepted this revision. melver added a comment. This revision is now accepted and ready to land. LGTM, thanks! Patch title ("...an X86-only test..") also needs adjustment. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D108555/new/ https://re

[PATCH] D108555: [tsan] Make sanitize-thread-disable.c an X86-only test

melver added inline comments. Comment at: clang/test/CodeGen/X86/sanitize-thread-disable.c:22 int instrumented1(int *a, _Atomic int *b) { return *a + atomic_load(b); } melver wrote: > I think you do not need to use atomic_load. > > You can just deref b, and

[PATCH] D108555: [tsan] Make sanitize-thread-disable.c an X86-only test

melver added inline comments. Comment at: clang/test/CodeGen/X86/sanitize-thread-disable.c:22 int instrumented1(int *a, _Atomic int *b) { return *a + atomic_load(b); } I think you do not need to use atomic_load. You can just deref b, and because it's _Atomi

[PATCH] D108465: [msan] Hotfix clang/test/CodeGen/sanitize-memory-disable.c

melver accepted this revision. melver added a comment. This revision is now accepted and ready to land. Who reported the issue? Might be worth mentioning in commit message, otherwise it appears to come out of nowhere (although it's semi-obvious given x86-64 is only supported). Repository: rG

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

melver accepted this revision. melver added a comment. LGTM with the LangRef change. Thanks! Comment at: llvm/lib/AsmParser/LLLexer.cpp:646 KEYWORD(dereferenceable_or_null); + KEYWORD(disable_sanitizer_instrumentation); KEYWORD(elementtype); glider wrot

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

melver added a comment. llvm/docs/LangRef.rst also needs a corresponding change. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D108029/new/ https://reviews.llvm.org/D108029 ___ cfe-commits mailing list cf

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

melver added inline comments. Comment at: llvm/lib/AsmParser/LLLexer.cpp:646 KEYWORD(dereferenceable_or_null); + KEYWORD(disable_sanitizer_instrumentation); KEYWORD(elementtype); Do the tests pass? There should also be the code that actually turns the tok

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

melver accepted this revision. melver added inline comments. Comment at: clang/docs/MemorySanitizer.rst:91 + +The ``disable_sanitizer_instrumentation`` attribute can be applied to a certain +function to prevent all kinds of instrumentation. This attribute overrides --

[PATCH] D108202: [tsan] Add support for disable_sanitizer_instrumentation attribute

melver accepted this revision. melver added inline comments. This revision is now accepted and ready to land. Comment at: clang/docs/ThreadSanitizer.rst:106 + +The ``disable_sanitizer_instrumentation`` attribute can be applied to a certain +function to prevent all kinds of instru

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

melver added inline comments. Comment at: clang/test/CodeGen/attr-no-sanitize-coverage.c:1 +// RUN: %clang_cc1 -debug-info-kind=limited %s -emit-llvm -o - | FileCheck %s + This file says it's called attr-no-sanitize-coverage.c, I think that's the wrong name. R

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

melver added inline comments. Comment at: llvm/include/llvm/IR/Attributes.td:90 +/// Do not instrument function with sanitizers. +def DisableSanitizerInstrumentation: EnumAttr<"disable_sanitizer_instrumentation", [FnAttr]>; + There's this long-tail of changes re

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

melver added inline comments. Comment at: clang/include/clang/Basic/AttrDocs.td:2601-2602 + +This is not the same as ``__attribute__((no_sanitize(...)))``, which depending +on the tool may still insert instrumentation to prevent false positive reports. + }]; aar

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

melver accepted this revision. melver added inline comments. This revision is now accepted and ready to land. Comment at: llvm/include/llvm/AsmParser/LLToken.h:222 kw_nosanitize_coverage, + kw_no_sanitizer_instrumentation, kw_null_pointer_is_valid, LLVM se

[PATCH] D104491: [Docs][Clang][Attr] mark no_stack_protector+no_sanitize GCC compatible

melver accepted this revision. melver added a comment. This revision is now accepted and ready to land. There might be subtle inconsistencies between values accepted by our no_sanitize and GCC's no_sanitize, because of internal naming differences, but I couldn't say what those are right now (I t

[PATCH] D104475: [Clang][Codegen] emit noprofile IR Fn Attr for new Fn Attr no_profile

melver added a comment. In D104475#2825772 , @nickdesaulniers wrote: > In D104475#2825711 , @MaskRay wrote: > >> So if we don't want to offer guarantee for IR/C/C++ attributes, we can >> document that users may

[PATCH] D104475: [Clang][Codegen] emit noprofile IR Fn Attr for new Fn Attr no_profile

melver added a comment. In D104475#2825711 , @MaskRay wrote: > In D104475#2825666 , @melver wrote: > >> In D104475#2825297 , @MaskRay >> wrote: >> >>> Should `no_profile`

[PATCH] D104475: [Clang][Codegen] emit noprofile IR Fn Attr for new Fn Attr no_profile

melver added a comment. In D104475#2825297 , @MaskRay wrote: > Should `no_profile` specify the inlining behavior? Though `no_sanitize_*` > don't specify that, either. I think it's somehow implicit, but I can't quite say how. There are some tests that c

[PATCH] D104253: [Clang][Codegen] emit noprofile IR Fn Attr for no_instrument_function Fn Attr

melver added a comment. In D104253#2817673 , @void wrote: > What are your thoughts on adding a `noprofile` function attribute in the FE? > @MaskRay suggested filing a bug with gcov (below) to get their take on it. > > > https://gcc.gnu.org/bugzilla/bu

[PATCH] D103958: [WIP] Support MustControl conditional control attribute

melver added a comment. In D103958#2811246 , @efriedma wrote: >> Defining the value used to establish a control dependency, e.g. the load >> later writes depend on (kernel only defines writes to be ctrl-dependently >> ordered). > > `[[mustcontrol]]` als

[PATCH] D103958: [WIP] Support MustControl conditional control attribute

melver updated this revision to Diff 351229. melver added a comment. As promised, some cleanups, docs, and updated test for the current version (no other major changes yet). While the identical-writes test is quite contrived, the currently failing switch test is a more realistic example. The exam

[PATCH] D103958: [WIP] Support MustControl conditional control attribute

melver added a comment. In D103958#2809353 , @efriedma wrote: > You could break `__builtin_load_with_control_dependency(x)` into something > like `__builtin_control_dependency(READ_ONCE(x))`. I don't think any > transforms will touch that in practice,

[PATCH] D103958: [WIP] Support MustControl conditional control attribute

melver added a comment. In D103958#2809145 , @efriedma wrote: > In D103958#2808966 , @melver wrote: > >> In D103958#2808861 , @efriedma >> wrote: >> >>> I don't like usin

[PATCH] D103958: [WIP] Support MustControl conditional control attribute

melver added a comment. In D103958#2808967 , @jdoerfert wrote: >> Please bear with me, I'm updating examples and documentation. I didn't think >> anybody would look at this while [WIP]. :-) > > People try to help so you have early design feedback ;) Tha

[PATCH] D103958: [WIP] Support MustControl conditional control attribute

melver added a comment. In D103958#2808861 , @efriedma wrote: > I don't like using metadata like this. Dropping metadata should generally > preserve the semantics of the code. Anything better for this without introducing new instructions? Would an arg

[PATCH] D103958: [WIP] Support MustControl conditional control attribute

melver added a comment. In D103958#2807811 , @lebedev.ri wrote: > This is missing langref changes, and a RFC to llvm-dev. We're not there yet, but will send something. Having real code helped me understand what out the myriad of options that were discu

[PATCH] D103958: [WIP] Support MustControl conditional control attribute

melver created this revision. Herald added subscribers: dexonsmith, jfb, hiraditya. Herald added a reviewer: aaron.ballman. melver requested review of this revision. Herald added subscribers: llvm-commits, cfe-commits, jdoerfert. Herald added projects: clang, LLVM. [ WIP, only high-level comments

[PATCH] D103159: [Clang] Enable __has_feature(coverage_sanitizer)

melver added a comment. In D103159#2784926 , @aaron.ballman wrote: > In D103159#2784845 , @melver wrote: > >> Ping. > > FWIW, the usual practice is to ping after no activity on the review for about > a week. > >

[PATCH] D103159: [Clang] Enable __has_feature(coverage_sanitizer)

This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rG4fbc66cd6d90: [Clang] Enable __has_feature(coverage_sanitizer) (authored by melver). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTIO

[PATCH] D103159: [Clang] Enable __has_feature(coverage_sanitizer)

melver marked 3 inline comments as done. melver added a comment. Ping. To reviewers: Do note the `feature` vs. `extension` discussion. Summary: We think to be consistent with other sanitizers and avoid confusion, we must make this a `feature`, too. Thanks. Repository: rG LLVM Github Monorep

[PATCH] D103159: [Clang] Enable __has_feature(coverage_sanitizer)

melver added inline comments. Comment at: clang/include/clang/Basic/Features.def:52 LangOpts.Sanitize.hasOneOf(SanitizerKind::Undefined)) +FEATURE(coverage_sanitizer, LangOpts.SanitizeCoverage) FEATURE(assume_nonnull, true) ojeda wrote: > melver wrote:

[PATCH] D103159: [Clang] Enable __has_feature(coverage_sanitizer)

melver updated this revision to Diff 348021. melver marked an inline comment as done. melver added a comment. s/Since/Because/ Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D103159/new/ https://reviews.llvm.org/D103159 Files: clang/docs/Sanitizer

  1   2   >