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

2021-06-09 Thread Marco Elver via Phabricator via cfe-commits
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

2021-06-09 Thread Marco Elver via Phabricator via cfe-commits
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

2021-06-10 Thread Marco Elver via Phabricator via cfe-commits
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

2021-06-10 Thread Marco Elver via Phabricator via cfe-commits
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

2021-06-10 Thread Marco Elver via Phabricator via cfe-commits
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] D104253: [Clang][Codegen] emit noprofile IR Fn Attr for no_instrument_function Fn Attr

2021-06-14 Thread Marco Elver via Phabricator via cfe-commits
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] D104475: [Clang][Codegen] emit noprofile IR Fn Attr for new Fn Attr no_profile

2021-06-17 Thread Marco Elver via Phabricator via cfe-commits
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] D104475: [Clang][Codegen] emit noprofile IR Fn Attr for new Fn Attr no_profile

2021-06-17 Thread Marco Elver via Phabricator via cfe-commits
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

2021-06-17 Thread Marco Elver via Phabricator via cfe-commits
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] D104491: [Docs][Clang][Attr] mark no_stack_protector+no_sanitize GCC compatible

2021-06-18 Thread Marco Elver via Phabricator via cfe-commits
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] D108029: [clang][Codegen] Introduce the no_sanitizer_instrumentation attribute

2021-08-13 Thread Marco Elver via Phabricator via cfe-commits
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] D108029: [clang][Codegen] Introduce the no_sanitizer_instrumentation attribute

2021-08-13 Thread Marco Elver via Phabricator via cfe-commits
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 disable_sanitizer_instrumentation attribute

2021-08-17 Thread Marco Elver via Phabricator via cfe-commits
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 disable_sanitizer_instrumentation attribute

2021-08-17 Thread Marco Elver via Phabricator via cfe-commits
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] D108202: [tsan] Add support for disable_sanitizer_instrumentation attribute

2021-08-17 Thread Marco Elver via Phabricator via cfe-commits
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] D108199: [msan] Add support for disable_sanitizer_instrumentation attribute

2021-08-17 Thread Marco Elver via Phabricator via cfe-commits
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] D108029: [clang][Codegen] Introduce the disable_sanitizer_instrumentation attribute

2021-08-19 Thread Marco Elver via Phabricator via cfe-commits
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] D108029: [clang][Codegen] Introduce the disable_sanitizer_instrumentation attribute

2021-08-19 Thread Marco Elver via Phabricator via cfe-commits
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

2021-08-19 Thread Marco Elver via Phabricator via cfe-commits
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] D114421: [asan] Add support for disable_sanitizer_instrumentation attribute

2021-11-23 Thread Marco Elver via Phabricator via cfe-commits
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] D136078: [RFC] Use-after-return binary metadata.

2022-10-19 Thread Marco Elver via Phabricator via cfe-commits
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] D136078: Use-after-return sanitizer binary metadata

2022-11-28 Thread Marco Elver via Phabricator via cfe-commits
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: Use-after-return sanitizer binary metadata

2022-11-29 Thread Marco Elver via Phabricator via cfe-commits
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

2022-11-29 Thread Marco Elver via Phabricator via cfe-commits
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

2022-11-29 Thread Marco Elver via Phabricator via cfe-commits
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

2022-11-29 Thread Marco Elver via Phabricator via cfe-commits
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

2022-11-29 Thread Marco Elver via Phabricator via cfe-commits
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

2022-11-30 Thread Marco Elver via Phabricator via cfe-commits
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

2022-12-01 Thread Marco Elver via Phabricator via cfe-commits
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

2022-12-02 Thread Marco Elver via Phabricator via cfe-commits
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

2022-12-02 Thread Marco Elver via Phabricator via cfe-commits
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

2022-12-04 Thread Marco Elver via Phabricator via cfe-commits
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] D145519: [SanitizerBinaryMetadata] Do not add to GPU code

2023-03-07 Thread Marco Elver via Phabricator via cfe-commits
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] D145519: [SanitizerBinaryMetadata] Do not add to GPU code

2023-03-08 Thread Marco Elver via Phabricator via cfe-commits
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

2023-03-09 Thread Marco Elver via Phabricator via cfe-commits
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] D143482: [SanitizerBinaryMetadata] Optimize used space for features and UAR stack args

2023-02-07 Thread Marco Elver via Phabricator via cfe-commits
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] D143484: [SanitizerBinaryMetadata] Emit constants as ULEB128

2023-02-07 Thread Marco Elver via Phabricator via cfe-commits
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] D143484: [SanitizerBinaryMetadata] Emit constants as ULEB128

2023-02-07 Thread Marco Elver via Phabricator via cfe-commits
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] D143482: [SanitizerBinaryMetadata] Optimize used space for features and UAR stack args

2023-02-08 Thread Marco Elver via Phabricator via cfe-commits
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

2023-02-08 Thread Marco Elver via Phabricator via cfe-commits
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] D143634: [ModuleUtils] Assert correct linkage and visibility of structors' COMDAT key

2023-02-09 Thread Marco Elver via Phabricator via cfe-commits
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] D143634: [ModuleUtils] Assert correct linkage and visibility of structors' COMDAT key

2023-02-09 Thread Marco Elver via Phabricator via cfe-commits
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] D143664: [SanitizerBinaryMetadata] Support ignore list

2023-02-09 Thread Marco Elver via Phabricator via cfe-commits
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] D143664: [SanitizerBinaryMetadata] Support ignore list

2023-02-10 Thread Marco Elver via Phabricator via cfe-commits
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

2023-02-10 Thread Marco Elver via Phabricator via cfe-commits
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

2023-02-10 Thread Marco Elver via Phabricator via cfe-commits
This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rG421215b919d0: [SanitizerBinaryMetadata] Support ignore list (authored by melver). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION

[PATCH] D143664: [SanitizerBinaryMetadata] Support ignore list

2023-02-10 Thread Marco Elver via Phabricator via cfe-commits
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] D148694: [SanitizerBinaryMetadata] Respect no_sanitize("thread") function attribute

2023-04-19 Thread Marco Elver via Phabricator via cfe-commits
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] D148694: [SanitizerBinaryMetadata] Respect no_sanitize("thread") function attribute

2023-04-19 Thread Marco Elver via Phabricator via cfe-commits
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

2023-04-19 Thread Marco Elver via Phabricator via cfe-commits
This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rG5f605e254a0f: [SanitizerBinaryMetadata] Respect no_sanitize("thread") function attribute (authored by melver). Repository: rG LLVM Github Monorepo

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

2022-12-04 Thread Marco Elver via Phabricator via cfe-commits
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] D117232: [clang] Respect -Wdeclaration-after-statement with C99 or later

2022-01-20 Thread Marco Elver via Phabricator via cfe-commits
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

2022-01-20 Thread Marco Elver via Phabricator via cfe-commits
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] Improve -Wdeclaration-after-statement

2022-01-20 Thread Marco Elver via Phabricator via cfe-commits
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] Improve -Wdeclaration-after-statement

2022-01-20 Thread Marco Elver via Phabricator via cfe-commits
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

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

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

2022-02-18 Thread Marco Elver via Phabricator via cfe-commits
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] D119816: Fix not working attribute no_sanitize bounds that affects linux kernel

2022-02-18 Thread Marco Elver via Phabricator via cfe-commits
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

2022-02-22 Thread Marco Elver via Phabricator via cfe-commits
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: [SanitizerBounds] Add support for NoSanitizeBounds function

2022-02-24 Thread Marco Elver via Phabricator via cfe-commits
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: [SanitizerBounds] Add support for NoSanitizeBounds function

2022-02-25 Thread Marco Elver via Phabricator via cfe-commits
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

2022-02-25 Thread Marco Elver via Phabricator via cfe-commits
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

2022-02-25 Thread Marco Elver via Phabricator via cfe-commits
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

2022-02-28 Thread Marco Elver via Phabricator via cfe-commits
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

2022-03-01 Thread Marco Elver via Phabricator via cfe-commits
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

2022-03-01 Thread Marco Elver via Phabricator via cfe-commits
This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rG17ce89fa8016: [SanitizerBounds] Add support for NoSanitizeBounds function (authored by ztong0001, committed by melver). Changed prior to commit: h

[PATCH] D115440: Provide __builtin_alloca*_uninitialized variants

2022-01-10 Thread Marco Elver via Phabricator via cfe-commits
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] D115440: Provide __builtin_alloca*_uninitialized variants

2022-01-12 Thread Marco Elver via Phabricator via cfe-commits
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] D117232: [clang] Respect -Wdeclaration-after-statement with C99 or later

2022-01-13 Thread Marco Elver via Phabricator via cfe-commits
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: [clang][auto-init] Provide __builtin_alloca_uninitialized

2021-12-09 Thread Marco Elver via Phabricator via cfe-commits
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] D115440: [clang][auto-init] Provide __builtin_alloca_uninitialized

2021-12-09 Thread Marco Elver via Phabricator via cfe-commits
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

2021-12-09 Thread Marco Elver via Phabricator via cfe-commits
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: Provide __builtin_alloca*_uninitialized variants

2021-12-09 Thread Marco Elver via Phabricator via cfe-commits
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: Provide __builtin_alloca*_uninitialized variants

2021-12-14 Thread Marco Elver via Phabricator via cfe-commits
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

2021-12-14 Thread Marco Elver via Phabricator via cfe-commits
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] D116128: [clang][driver] Warn when '-mno-outline-atomics' is used with a non-AArch64 triple

2021-12-22 Thread Marco Elver via Phabricator via cfe-commits
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] D108465: [msan] Hotfix clang/test/CodeGen/sanitize-memory-disable.c

2021-08-20 Thread Marco Elver via Phabricator via cfe-commits
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] D108555: [tsan] Make sanitize-thread-disable.c an X86-only test

2021-08-23 Thread Marco Elver via Phabricator via cfe-commits
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] D108555: [tsan] Make sanitize-thread-disable.c an X86-only test

2021-08-23 Thread Marco Elver via Phabricator via cfe-commits
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

2021-08-23 Thread Marco Elver via Phabricator via cfe-commits
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] Do not include from sanitize-thread-disable.c

2021-08-23 Thread Marco Elver via Phabricator via cfe-commits
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

<    1   2