[PATCH] D51239: [ubsan] Enable -fsanitize=vptr on Apple devices and simulators

delcypher requested changes to this revision. delcypher added inline comments. This revision now requires changes to proceed. Comment at: clang/lib/Driver/ToolChains/Darwin.cpp:2254 Res |= SanitizerKind::Function; + if (!isTargetMacOS() || !isMacosxVersionLT(10, 9)) +Res

[PATCH] D51239: [ubsan] Enable -fsanitize=vptr on Apple devices and simulators

delcypher accepted this revision. delcypher added a comment. This revision is now accepted and ready to land. LGTM. Comment at: clang/test/Driver/fsanitize.c:426 +// RUN: %clang -target arm-apple-ios4 -fsanitize=vptr %s -### 2>&1 | FileCheck %s --check-prefix=CHECK-VPTR-IOS-

[PATCH] D15225: [Driver] Sanitizer support based on runtime library presence

delcypher added inline comments. Comment at: clang/lib/Driver/ToolChains/Darwin.cpp:2298 SanitizerMask Res = ToolChain::getSupportedSanitizers(); - Res |= SanitizerKind::Address; - Res |= SanitizerKind::Leak; - Res |= SanitizerKind::Fuzzer; - Res |= SanitizerKind::FuzzerNo

[PATCH] D15225: [Driver] Sanitizer support based on runtime library presence

delcypher added inline comments. Comment at: clang/lib/Driver/ToolChains/Darwin.h:425 + /// \return Relative path to the filename for the library + /// containing the sanitizer {@code SanitizerName}. You might want to update that comment to not mention "Relat

[PATCH] D15225: [Driver] Sanitizer support based on runtime library presence

delcypher added a comment. @george.karpenkov Other than the comment that probably needs updating, LGTM. https://reviews.llvm.org/D15225 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commit

[PATCH] D53239: [python] [tests] Disable python binding tests when building with LLVM_USE_SANITIZER=Address

delcypher accepted this revision. delcypher added a comment. This revision is now accepted and ready to land. LGTM. But then I would say that because I wrote the patch 😛... Repository: rC Clang https://reviews.llvm.org/D53239 ___ cfe-commits mail

[PATCH] D54978: Move the SMT API to LLVM

delcypher added a comment. In D54978#1431788 , @ddcc wrote: > The only relevant commit that I can find is > https://github.com/Z3Prover/z3/commit/2cb4223979cc94e2ebc4e49a9e83adbdcd2b6979 > , but it first landed in z3 4.6.0. It looks like it's specific to

[PATCH] D54978: Move the SMT API to LLVM

delcypher added a comment. In D54978#1431430 , @mikhail.ramalho wrote: > Hi all, > > Sorry for the massive delay, but I just updated the `FindZ3` script to > retrieve the version from the lib. I changed it to use `try_run` instead of > `try_compile` so

[PATCH] D54978: Move the SMT API to LLVM

delcypher added a comment. In D54978#1432178 , @ddcc wrote: > In D54978#1431935 , @delcypher wrote: > > > Would one of you be able to file a bug against Z3 to fix this? I am no > > longer in a position to contribut

[PATCH] D15225: [Driver] Sanitizer support based on runtime library presence

delcypher requested changes to this revision. delcypher added a comment. This revision now requires changes to proceed. Seems mostly fine apart from some minor nits. If I'm honest I don't see any reason why this should be Darwin specific. Sure the naming convention and location of the runtime li

[PATCH] D15225: [Driver] Sanitizer support based on runtime library presence

delcypher added inline comments. Comment at: clang/lib/Driver/ToolChains/Darwin.cpp:2298 SanitizerMask Res = ToolChain::getSupportedSanitizers(); - Res |= SanitizerKind::Address; - Res |= SanitizerKind::Leak; - Res |= SanitizerKind::Fuzzer; - Res |= SanitizerKind::FuzzerNo

[PATCH] D121327: Lower `@llvm.global_dtors` using `__cxa_atexit` on MachO

delcypher added inline comments. Comment at: llvm/lib/CodeGen/TargetLoweringObjectFileImpl.cpp:1181 +unsigned Priority, const MCSymbol *KeySym) const { + // TODO(yln): Remove -lower-global-dtors-via-cxa-atexit fallback flag + // (LowerGlobalDtorsViaCxaAtExit) and issue a fa

[PATCH] D116635: Add warning to detect when calls passing arguments are made to functions without prototypes.

delcypher abandoned this revision. delcypher added inline comments. Herald added a project: All. Comment at: clang/include/clang/Basic/DiagnosticSemaKinds.td:5529 +def warn_call_function_without_prototype : Warning< + "calling function %0 with arguments when function has no prot

[PATCH] D124054: [NFC] Avoid unnecessary duplication of code generating diagnostic.

delcypher created this revision. delcypher added reviewers: rapidsna, fcloutier, aaron.ballman. Herald added a project: All. delcypher requested review of this revision. Herald added a project: clang. The previous code unneccessarily duplicated the creation of a diagnostic where the only differenc

[PATCH] D124054: [NFC] Avoid unnecessary duplication of code generating diagnostic.

This revision was automatically updated to reflect the committed changes. Closed by commit rG3d612a930dce: [NFC] Avoid unnecessary duplication of code generating diagnostic. (authored by delcypher). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D12405

[PATCH] D124054: [NFC] Avoid unnecessary duplication of code generating diagnostic.

delcypher added a comment. @aaron.ballman Thanks. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D124054/new/ https://reviews.llvm.org/D124054 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https:

[PATCH] D118855: [modules] Add a flag for TagDecl if it was a definition demoted to a declaration.

delcypher added a comment. Change seems reasonable but I don't have expertise on this code. I've left a few minor nits. Comment at: clang/include/clang/AST/Decl.h:3494 + /// symbol and not interested in the final AST with deduplicated definitions. + bool isThisDeclarationADe

[PATCH] D124474: Honor COMPILER_RT_INCLUDE_TESTS when using LLVM_BUILD_EXTERNAL_COMPILER_RT=ON

delcypher added a comment. > I'd definitely prefer moving towards LLVM_ENABLE_RUNTIMES. We already require > LLVM_ENABLE_RUNTIMES for libc++, libc++abi and libunwind and I'm going to > propose doing the same for compiler-rt as well. +1 on this. Compiler-RT's CMake code is extremely and unnecess

[PATCH] D124474: Honor COMPILER_RT_INCLUDE_TESTS when using LLVM_BUILD_EXTERNAL_COMPILER_RT=ON

delcypher accepted this revision. delcypher added a comment. This revision is now accepted and ready to land. Change LGTM. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D124474/new/ https://reviews.llvm.org/D124474 _

[PATCH] D124489: Deprecate LLVM_BUILD_EXTERNAL_COMPILER_RT

delcypher added a comment. > Making it an ERROR and providing an option to downgrade it to a WARNING seems > reasonable to me. Thoughts? One nice property this would have is it would (eventually) allow us to collect a list of all the places that depend on this old code. It is potentially quite

[PATCH] D124489: Deprecate LLVM_BUILD_EXTERNAL_COMPILER_RT

delcypher added a comment. I spoke to a few other engineers in Apple. They are 1. Happy for us to proceed with marking `LLVM_BUILD_EXTERNAL_COMPILER_RT` as deprecated. 2. Okay with using `LLVM_BUILD_EXTERNAL_COMPILER_RT` being an error by default that can be downgraded to a warning using a CMak

[PATCH] D125195: [asan][ARMCXXABI] Added missing asan poison array cookie hooks.

delcypher added inline comments. Comment at: clang/lib/CodeGen/ItaniumCXXABI.cpp:2443 + // Handle poisoning the array cookie in asan + if (CGM.getLangOpts().Sanitize.has(SanitizerKind::Address) && AS == 0 && + (expr->getOperatorNew()->isReplaceableGlobalAllocationFunction(

[PATCH] D124866: [CUDA][HIP] support __noinline__ as keyword

delcypher added inline comments. Comment at: clang/lib/Basic/IdentifierTable.cpp:111 KEYSYCL = 0x100, +KEYCUDA = 0x200, KEYALLCXX = KEYCXX | KEYCXX11 | KEYCXX20, @yaxunl Is it intentional that you didn't update `KEYALL` here? Tha

[PATCH] D124866: [CUDA][HIP] support __noinline__ as keyword

delcypher added inline comments. Comment at: clang/lib/Basic/IdentifierTable.cpp:111 KEYSYCL = 0x100, +KEYCUDA = 0x200, KEYALLCXX = KEYCXX | KEYCXX11 | KEYCXX20, yaxunl wrote: > delcypher wrote: > > @yaxunl Is it intentional that

[PATCH] D124866: [CUDA][HIP] support __noinline__ as keyword

delcypher added inline comments. Comment at: clang/lib/Basic/IdentifierTable.cpp:111 KEYSYCL = 0x100, +KEYCUDA = 0x200, KEYALLCXX = KEYCXX | KEYCXX11 | KEYCXX20, yaxunl wrote: > yaxunl wrote: > > delcypher wrote: > > > yaxunl wro

[PATCH] D125396: [clang] Fix KEYALL

delcypher accepted this revision. delcypher added a comment. This revision is now accepted and ready to land. @yaxunl Thanks for addressing my feedback so quickly. I think the commit message should also mention that `KEYCUDA` is now included in `KEYALL`. Other than that LGTM. CHANGES SINCE LAS

[PATCH] D96572: [Clang][ASan] Introduce `-fsanitize-address-destructor-kind=` driver & frontend option.

delcypher created this revision. delcypher added reviewers: arphaman, kubamracek, yln, aralisza, kcc, vitalybuka. Herald added subscribers: dexonsmith, dang. Herald added a reviewer: jansvoboda11. delcypher requested review of this revision. Herald added a project: clang. Herald added a subscriber:

[PATCH] D96572: [Clang][ASan] Introduce `-fsanitize-address-destructor-kind=` driver & frontend option.

delcypher updated this revision to Diff 323213. delcypher added a comment. clang-format fix Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D96572/new/ https://reviews.llvm.org/D96572 Files: clang/include/clang/Basic/CodeGenOptions.def clang/incl

[PATCH] D96573: [Clang][ASan] Teach Clang to not emit ASan module destructors when compiling with `-mkernel` or `-fapple-kext`.

delcypher created this revision. delcypher added reviewers: arphaman, kubamracek, yln, aralisza, kcc, vitalybuka. delcypher requested review of this revision. Herald added a project: clang. Herald added a subscriber: cfe-commits. rdar://71609176 Repository: rG LLVM Github Monorepo https://rev

[PATCH] D96572: [Clang][ASan] Introduce `-fsanitize-address-destructor-kind=` driver & frontend option.

delcypher added a comment. Note this patch depends on https://reviews.llvm.org/D96571 Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D96572/new/ https://reviews.llvm.org/D96572 ___ cfe-commits mailing list

[PATCH] D96572: [Clang][ASan] Introduce `-fsanitize-address-destructor-kind=` driver & frontend option.

delcypher added inline comments. Comment at: clang/include/clang/Driver/Options.td:1485 +def sanitize_address_destructor_kind_EQ : Joined<["-"], "fsanitize-address-destructor-kind=">, + MetaVarName<"">, + Flags<[CC1Option]>, vitalybuka wrote: > What is the dif

[PATCH] D96572: [Clang][ASan] Introduce `-fsanitize-address-destructor-kind=` driver & frontend option.

delcypher updated this revision to Diff 323455. delcypher added a comment. Add documentation for `-fsanitize-address-destructor-kind=` option. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D96572/new/ https://reviews.llvm.org/D96572 Files: clang/

[PATCH] D96572: [Clang][ASan] Introduce `-fsanitize-address-destructor-kind=` driver & frontend option.

delcypher added inline comments. Comment at: clang/include/clang/Driver/Options.td:1485 +def sanitize_address_destructor_kind_EQ : Joined<["-"], "fsanitize-address-destructor-kind=">, + MetaVarName<"">, + Flags<[CC1Option]>, delcypher wrote: > vitalybuka wrote

[PATCH] D96572: [Clang][ASan] Introduce `-fsanitize-address-destructor-kind=` driver & frontend option.

delcypher added a comment. @jansvoboda11 I noticed that the `Options.td` file is making use of some fancy mixins that mean the option gets automatically marshalled by the frontend into the codegen options :). I tried this out using the patch to this patch and all my tests seem to pass. Should I

[PATCH] D96572: [Clang][ASan] Introduce `-fsanitize-address-destructor-kind=` driver & frontend option.

delcypher added inline comments. Comment at: clang/include/clang/Driver/Options.td:1485 +def sanitize_address_destructor_kind_EQ : Joined<["-"], "fsanitize-address-destructor-kind=">, + MetaVarName<"">, + Flags<[CC1Option]>, vitalybuka wrote: > delcypher wrote

[PATCH] D96572: [Clang][ASan] Introduce `-fsanitize-address-destructor-kind=` driver & frontend option.

delcypher updated this revision to Diff 325872. delcypher added a comment. Rebase and fix merge conflict caused by f2f59d2a060788f17040ad924ee2d11da0e215c8 Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION h

[PATCH] D97327: [NFC] Switch to auto marshalling infrastructure for `-fsanitize-address-destructor-kind=` flag.

delcypher created this revision. delcypher added reviewers: jansvoboda11, arphaman, kubamracek, yln, aralisza. Herald added a subscriber: dang. delcypher requested review of this revision. Herald added a project: clang. Herald added a subscriber: cfe-commits. This change simplifies `clang/lib/Fron

[PATCH] D96572: [Clang][ASan] Introduce `-fsanitize-address-destructor-kind=` driver & frontend option.

delcypher added a comment. In D96572#2563707 , @jansvoboda11 wrote: > In D96572#2561216 , @delcypher wrote: > >> @jansvoboda11 I noticed that the `Options.td` file is making use of some >> fancy mixins that mean t

[PATCH] D96572: [Clang][ASan] Introduce `-fsanitize-address-destructor-kind=` driver & frontend option.

delcypher added a comment. @arphaman @jansvoboda11 ping. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D96572/new/ https://reviews.llvm.org/D96572 ___ cfe-commits mailing list cfe-commits@lists.llvm.org h

[PATCH] D97327: [NFC] Switch to auto marshalling infrastructure for `-fsanitize-address-destructor-kind=` flag.

delcypher added a comment. In D97327#2585410 , @jansvoboda11 wrote: > LGTM. > > If you don't get around committing this today, please rebase this on top of > D97375 that I'm going to commit tomorrow. @jansvoboda11 This

[PATCH] D97327: [NFC] Switch to auto marshalling infrastructure for `-fsanitize-address-destructor-kind=` flag.

delcypher added a comment. In D97327#2582981 , @yln wrote: > LGTM, cleanup only, right? Yes. This is just a small clean up. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D97327/new/ https://reviews.llvm.org

[PATCH] D105489: [compiler-rt] [test] Fix asan symbolize tests on py3.10

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

[PATCH] D96572: [Clang][ASan] Introduce `-fsanitize-address-destructor-kind=` driver & frontend option.

This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rG5d64dd8e3c22: [Clang][ASan] Introduce `-fsanitize-address-destructor-kind=` driver & frontend… (authored by delcypher). Repository: rG LLVM Githu

[PATCH] D96573: [Clang][ASan] Teach Clang to not emit ASan module destructors when compiling with `-mkernel` or `-fapple-kext`.

This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rGfdce098b49cb: [Clang][ASan] Teach Clang to not emit ASan module destructors when compiling… (authored by delcypher). Repository: rG LLVM Github Mo

[PATCH] D97327: [NFC] Switch to auto marshalling infrastructure for `-fsanitize-address-destructor-kind=` flag.

delcypher updated this revision to Diff 326474. delcypher added a comment. - Fix build. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D97327/new/ https://reviews.llvm.org/D97327 Files: clang/include/clang/Driver/Options.td clang/lib/Frontend/Co

[PATCH] D97327: [NFC] Switch to auto marshalling infrastructure for `-fsanitize-address-destructor-kind=` flag.

This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rG7b1d2a2891d8: [NFC] Switch to auto marshalling infrastructure for `-fsanitize-address… (authored by delcypher). Repository: rG LLVM Github Monorep

[PATCH] D97496: [Clang][ASan] Correct AsanDtorKindToString to return non-void in default case

delcypher added a comment. @cryptoad Thanks. LGTM Originally this code had a `default` case in the switch but clang complained about it being unnecessary because it was exhaustive, so I removed it... It seems this is causing problems with your compiler :( Repository: rG LLVM Github Monorep

[PATCH] D98291: [compiler-rt] Fix stale incremental builds when using `LLVM_BUILD_EXTERNAL_COMPILER_RT=ON`.

delcypher created this revision. delcypher added reviewers: kubamracek, yln, aralisza, beanz, samsonov. Herald added subscribers: mgorny, dberris. delcypher requested review of this revision. Herald added a project: clang. When building with `LLVM_BUILD_EXTERNAL_COMPILER_RT=ON` (e.g. Swift does th

[PATCH] D98291: [compiler-rt] Fix stale incremental builds when using `LLVM_BUILD_EXTERNAL_COMPILER_RT=ON`.

delcypher updated this revision to Diff 329469. delcypher added a comment. - update description Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D98291/new/ https://reviews.llvm.org/D98291 Files: clang/runtime/CMakeLists.txt Index: clang/runtime/C

[PATCH] D98291: [compiler-rt] Fix stale incremental builds when using `LLVM_BUILD_EXTERNAL_COMPILER_RT=ON`.

delcypher added a comment. In D98291#2615590 , @kubamracek wrote: > When using Ninja, does this mean running a null build is no longer a null > build, but it at least always invokes this sub-build? Not saying that's bad, > just want to understand the cha

[PATCH] D98291: [compiler-rt] Fix stale incremental builds when using `LLVM_BUILD_EXTERNAL_COMPILER_RT=ON`.

delcypher added a comment. In D98291#2615593 , @beanz wrote: > So... this patch is fine. (marked as approved). That said, we should really > be working to remove the `LLVM_BUILD_EXTERNAL_COMPILER_RT` option entirely in > favor of just using `LLVM_ENABLE_

[PATCH] D98291: [compiler-rt] Fix stale incremental builds when using `LLVM_BUILD_EXTERNAL_COMPILER_RT=ON`.

This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rGa159f91c8d06: [compiler-rt] Fix stale incremental builds when using… (authored by delcypher). Repository: rG LLVM Github Monorepo CHANGES SINCE L

[PATCH] D98291: [compiler-rt] Fix stale incremental builds when using `LLVM_BUILD_EXTERNAL_COMPILER_RT=ON`.

delcypher added a comment. In D98291#2616865 , @kubamracek wrote: > Looks good! > > By the way, Apple Clang releases also build compiler-rt this way, so it would > be worth checking with @arphaman that he's okay with the change. @kubamracek @arphaman Ah

[PATCH] D137714: Do not merge traps in functions annotated optnone

delcypher added a comment. Other than minor issue in the test this LGTM Comment at: clang/test/CodeGen/ubsan-trap-debugloc.c:11 +void bar(volatile int a) __attribute__((optnone)) { + // CHECK: call void @llvm.ubsantrap(i8 0){{.*}} !dbg [[LOC2:![0-9]+]] + // CHECK: call void @

[PATCH] D137714: Do not merge traps in functions annotated optnone

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

[PATCH] D137024: [compiler-rt] Switch from llvm-config to find_package(LLVM)

delcypher added a comment. In D137024#3931167 , @mgorny wrote: > In D137024#3931126 , @thetruestblue > wrote: > >> This patch removes the logic that sets the binary tools dir using >> llvm-config. We don't have

[PATCH] D137024: [compiler-rt] Switch from llvm-config to find_package(LLVM)

delcypher added a comment. In D137024#3931488 , @mgorny wrote: > Well, I'm certainly not opposed to making all the paths configurable. > However, I'm not sure if having CMake file accessible one way or another > wouldn't eventually be a necessity. For o

[PATCH] D141550: [CompilerRT] Remove ubsan static runtime on Apple

delcypher added a comment. Overall approach LGTM. I just have some very minor nits. Comment at: clang/include/clang/Basic/DiagnosticDriverKinds.td:219 +def err_drv_unsupported_static_ubsan_darwin : Error< + "Static UBSan runtime is not supported on darwin">; def err_drv_dupli

[PATCH] D141550: [CompilerRT] Remove ubsan static runtime on Apple

delcypher added inline comments. Comment at: compiler-rt/lib/ubsan/CMakeLists.txt:118 +if (NOT APPLE) + add_compiler_rt_runtime(clang_rt.ubsan +STATIC usama54321 wrote: > delcypher wrote: > > I think you may have accidentally added tabs here when

[PATCH] D141550: [CompilerRT] Remove ubsan static runtime on Apple

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

[PATCH] D125919: Drop qualifiers from return types in C (DR423)

delcypher added a comment. @aaron.ballman Hey I just saw this change and had questions about it. For others looking I think the resolution to DR423 is in https://www.open-std.org/jtc1/sc22/wg14/www/docs/n1863.pdf, I found https://www.open-std.org/jtc1/sc22/wg14/www/docs/n2148.htm#dr_423 hard to

[PATCH] D125919: Drop qualifiers from return types in C (DR423)

delcypher added a comment. @aaron.ballman >> Sorry if these are silly questions and if I've misunderstood something, I >> saw n1863 say "functions return unqualified types" and I was very surprised. > > These are not at all silly questions, so thank you for asking them! Thanks for taking the ti

[PATCH] D125919: Drop qualifiers from return types in C (DR423)

delcypher added a comment. @rjmccall >> Sorry if these are silly questions and if I've misunderstood something, I >> saw n1863 say "functions return unqualified types" and I was very surprised. > > Just to be clear, you understand that this is only about top-level qualifiers > on the return typ

[PATCH] D71491: [ubsan] Check implicit casts in ObjC for-in statements

delcypher added inline comments. Comment at: compiler-rt/lib/ubsan/ubsan_value.cpp:29 +const char *__ubsan::getObjCClassName(ValueHandle Pointer) { +#if defined(__APPLE__) + // We need to query the ObjC runtime for some information, but do not want The compiler-

[PATCH] D71491: [ubsan] Check implicit casts in ObjC for-in statements

delcypher added inline comments. Comment at: clang/lib/Driver/ToolChain.cpp:953 + SanitizerKind::ImplicitConversion | SanitizerKind::Nullability | + SanitizerKind::LocalBounds | SanitizerKind::ObjCCast; if (getTriple().getArch() == llvm::Triple::x86 || --

[PATCH] D71491: [ubsan] Check implicit casts in ObjC for-in statements

delcypher added inline comments. Comment at: compiler-rt/lib/ubsan/ubsan_value.cpp:29 +const char *__ubsan::getObjCClassName(ValueHandle Pointer) { +#if defined(__APPLE__) + // We need to query the ObjC runtime for some information, but do not want vsk wrote: >

[PATCH] D71491: [ubsan] Check implicit casts in ObjC for-in statements

delcypher added inline comments. Comment at: compiler-rt/lib/ubsan/ubsan_value.cpp:29 +const char *__ubsan::getObjCClassName(ValueHandle Pointer) { +#if defined(__APPLE__) + // We need to query the ObjC runtime for some information, but do not want vsk wrote: >

[PATCH] D72875: [clang][cmake] Include generated rst files in html built by docs-clang-html target

delcypher accepted this revision. delcypher added a comment. This revision is now accepted and ready to land. LGTM. Thanks for addressing all the issues I raised. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D72875/new/ https://reviews.llvm.org/D72

[PATCH] D71491: [ubsan] Check implicit casts in ObjC for-in statements

delcypher added a comment. @vsk The compiler-rt side seems fine to me but I'm not very familiar with the Clang side of things. @arphaman @jfb @rjmccall any thoughts? CHANGES SINCE LAST ACTION https://reviews.llvm.org/D71491/new/ https://reviews.llvm.org/D71491 ___

[PATCH] D72875: [clang][cmake] Include generated rst files in html built by docs-clang-html target

delcypher requested changes to this revision. delcypher added inline comments. This revision now requires changes to proceed. Comment at: clang/docs/CMakeLists.txt:93 +function (gen_rst_file output_file td_option source) + get_filename_component(TABLEGEN_INCLUDE_DIR "${CMAKE_

[PATCH] D72875: [clang][cmake] Include generated rst files in html built by docs-clang-html target

delcypher added inline comments. Comment at: llvm/cmake/modules/AddSphinxTarget.cmake:33 + if (NOT ARG_SOURCE_DIR) +set(ARG_SOURCE_DIR ${CMAKE_CURRENT_SOURCE_DIR}) + endif() @tstellar I'm not 100% sure about this but I think you probably want `${CMAKE_CURR

[PATCH] D148851: Disable llvm-symbolizer on some of the driver tests that are timing out

delcypher added a comment. @ahatanak Thanks for working on this. I think that annotating these particular tests so that's clear that they are supposed to crash (and therefore symbolication is of no use) is a good change. If we want to make a more global change for tests I think that should be h

[PATCH] D148216: Add support for annotations in UpdateTestChecks (NFC)

delcypher added inline comments. Comment at: clang/test/utils/update_cc_test_checks/Inputs/annotations.c.expected:12 +// CHECK-NEXT:[[TMP1:%.*]] = load i32, ptr [[X]], align 4 +// CHECK-NEXT:ret i32 [[TMP1]] +// @hnrklssn I just noticed we don't have a `

[PATCH] D28952: [analyzer] Add new Z3 constraint manager backend

delcypher added a comment. I'm a little late to the review but hopefully I'll have useful comments. Nice work @ddc > I looked around for a generic smt-lib interface earlier, but couldn't find > much available, and since I wanted floating-point arithmetic support, I ended > up just directly usi

[PATCH] D28952: [analyzer] Add new Z3 constraint manager backend

delcypher added a comment. In https://reviews.llvm.org/D28952#655337, @dcoughlin wrote: > In https://reviews.llvm.org/D28952#655278, @ddcc wrote: > > > > - That said, I think there are still significant optimization > > > opportunities. It looks like when performing a query you create a new > >

[PATCH] D28952: [analyzer] Add new Z3 constraint manager backend

delcypher added inline comments. Comment at: CMakeLists.txt:188 +find_package(Z3 4.5) + @ddcc It is of course up to you but I recently [[ added support for using `libz3` directly | added support for using `libz3` directly ]] from CMake. via it's own CMake con

[PATCH] D28952: [analyzer] Add new Z3 constraint manager backend

delcypher added inline comments. Comment at: CMakeLists.txt:188 +find_package(Z3 4.5) + delcypher wrote: > @ddcc It is of course up to you but I recently [[ added support for using > `libz3` directly | added support for using `libz3` directly ]] from CMake. >

[PATCH] D28952: [analyzer] Add new Z3 constraint manager backend

delcypher added inline comments. Comment at: lib/StaticAnalyzer/Core/Z3ConstraintManager.cpp:559 + Float.toString(Chars, 0, 0); + AST = Z3_mk_numeral(Z3Context::ZC, Chars.c_str(), Sort); + break; ddcc wrote: > delcypher wrote: > > @ddcc I'm not con

[PATCH] D28952: [analyzer] Add new Z3 constraint manager backend

delcypher added inline comments. Comment at: cmake/modules/FindZ3.cmake:3 +# in the find_path() and find_library() calls +find_package(PkgConfig QUIET) +PKG_CHECK_MODULES(PC_Z3 QUIET libz3) @ddcc Seeing as you don't want to use the new upstream Z3 CMake package c

[PATCH] D28952: [analyzer] Add new Z3 constraint manager backend

delcypher added inline comments. Comment at: CMakeLists.txt:188 +find_package(Z3 4.5) + ddcc wrote: > delcypher wrote: > > delcypher wrote: > > > @ddcc It is of course up to you but I recently [[ added support for using > > > `libz3` directly | added support f

[PATCH] D28952: [analyzer] Add new Z3 constraint manager backend

delcypher added inline comments. Comment at: lib/StaticAnalyzer/Core/Z3ConstraintManager.cpp:619 + +llvm::APSInt Int = llvm::APSInt(Float.bitcastToAPInt(), true); +Z3Expr Z3Int = Z3Expr::fromAPSInt(Int); @ddcc Why use APSInt here? Why not APInt, we are lo

[PATCH] D28952: [analyzer] Add new Z3 constraint manager backend

delcypher added inline comments. Comment at: lib/StaticAnalyzer/Core/Z3ConstraintManager.cpp:674 + // are the same size. + Z3_get_numeral_uint64(Z3Context::ZC, AST, +reinterpret_cast<__uint64 *>(&Value)); ddcc wrote: > The on

[PATCH] D28952: [analyzer] Add new Z3 constraint manager backend

delcypher added inline comments. Comment at: cmake/modules/FindZ3.cmake:5 +PKG_CHECK_MODULES(PC_Z3 QUIET libz3) +set(Z3_DEFINITIONS ${PC_LIBZ3_CFLAGS_OTHER}) + ddcc wrote: > delcypher wrote: > > @ddcc This CMake variable is set but never used. Also based on the n

[PATCH] D117924: [compiler_rt] Add a seperate runtime for Mac Catalyst

delcypher added a comment. @bc-lee Thanks for the patch. While I get what you're trying to do I have doubts about being able to accept the patch in its current form. Apple's ASan catalyst doesn't work by building a separate dylib, instead it builds the macosx dylib in a different way to make it

[PATCH] D116635: Add warning to detect when calls passing arguments are made to functions without prototypes.

delcypher added inline comments. Comment at: clang/include/clang/Basic/DiagnosticSemaKinds.td:5529 +def warn_call_function_without_prototype : Warning< + "calling function %0 with arguments when function has no prototype">, InGroup< + DiagGroup<"strict-calls-without-prototype"

[PATCH] D101122: introduce flag -fsanitize-address-detect-stack-use-after-return-mode. No functional change.

delcypher added inline comments. Comment at: clang/include/clang/Basic/CodeGenOptions.def:225-228 +ENUM_CODEGENOPT(SanitizeAddressDetectStackUseAfterReturnMode, +llvm::AsanDetectStackUseAfterReturnMode, 2, +llvm::AsanDetectStackUseAfterReturnMode::

[PATCH] D101490: [NFC] Rename SanitizeAddressDtorKind codegen opt to not have `Kind` suffix.

delcypher created this revision. delcypher added a reviewer: vitalybuka. Herald added subscribers: dexonsmith, dang. delcypher requested review of this revision. Herald added a project: clang. This is post commit follow up based on discussions in https://reviews.llvm.org/D101122. Repository: r

[PATCH] D101491: [ASan] Rename `-fsanitize-address-destructor-kind=` to drop the `-kind` suffix.

delcypher created this revision. delcypher added reviewers: vitalybuka, jansvoboda11. Herald added a subscriber: dang. delcypher requested review of this revision. Herald added a project: clang. Renaming the option is based on discussions in https://reviews.llvm.org/D101122. It is normally not a

[PATCH] D101122: introduce flag -fsanitize-address-detect-stack-use-after-return-mode. No functional change.

delcypher added inline comments. Comment at: clang/include/clang/Basic/CodeGenOptions.def:225-228 +ENUM_CODEGENOPT(SanitizeAddressDetectStackUseAfterReturnMode, +llvm::AsanDetectStackUseAfterReturnMode, 2, +llvm::AsanDetectStackUseAfterReturnMode::

[PATCH] D101491: [ASan] Rename `-fsanitize-address-destructor-kind=` to drop the `-kind` suffix.

delcypher added a comment. @jansvoboda11 Should I use the the alias feature so that the old `-fsanitize-address-destructor-kind` argument is still parsed? I'm not sure if it's worth it but if doing so is very simple and has a low maintenance burden we should probably do it. Repository: rG L

[PATCH] D101490: [NFC] Rename SanitizeAddressDtorKind codegen opt to not have `Kind` suffix.

This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rG1bbbcff99de8: [NFC] Rename SanitizeAddressDtorKind codegen opt to not have `Kind` suffix. (authored by delcypher). Repository: rG LLVM Github Mono

[PATCH] D101491: [ASan] Rename `-fsanitize-address-destructor-kind=` to drop the `-kind` suffix.

delcypher updated this revision to Diff 341384. delcypher added a comment. Remove metavar Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D101491/new/ https://reviews.llvm.org/D101491 Files: clang/docs/ClangCommandLineReference.rst clang/include/

[PATCH] D101491: [ASan] Rename `-fsanitize-address-destructor-kind=` to drop the `-kind` suffix.

delcypher added a comment. In D101491#2725348 , @jansvoboda11 wrote: > In D101491#2724025 , @delcypher > wrote: > >> @jansvoboda11 Should I use the the alias feature so that the old >> `-fsanitize-address-destr

[PATCH] D101491: [ASan] Rename `-fsanitize-address-destructor-kind=` to drop the `-kind` suffix.

delcypher updated this revision to Diff 341596. delcypher added a comment. Rename def too. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D101491/new/ https://reviews.llvm.org/D101491 Files: clang/docs/ClangCommandLineReference.rst clang/include

[PATCH] D101491: [ASan] Rename `-fsanitize-address-destructor-kind=` to drop the `-kind` suffix.

delcypher added a comment. @jansvoboda11 I'm going to land this patch as is (with your nit fixed). If you would like me to add an alias please follow up with me and I can put up a patch to do that. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D101

[PATCH] D101491: [ASan] Rename `-fsanitize-address-destructor-kind=` to drop the `-kind` suffix.

This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rG2d42b2ee7baf: [ASan] Rename `-fsanitize-address-destructor-kind=` to drop the `-kind` suffix. (authored by delcypher). Repository: rG LLVM Github

[PATCH] D101682: [Driver] Fix `ToolChain::getCompilerRTPath()` to return the correct path on Darwin.

delcypher created this revision. delcypher added reviewers: arphaman, dexonsmith, kubamracek, aralisza, yln. delcypher requested review of this revision. Herald added a project: clang. When the Darwin target triple included the OS version number this would cause `ToolChain::getCompilerRTPath()` to

[PATCH] D101682: [Driver] Fix `ToolChain::getCompilerRTPath()` to return the correct path on Darwin.

delcypher updated this revision to Diff 342152. delcypher added a comment. - Support other Apple target triples. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D101682/new/ https://reviews.llvm.org/D101682 Files: clang/lib/Driver/ToolChain.cpp c

[PATCH] D101682: [Driver] Fix `ToolChain::getCompilerRTPath()` to return the correct path on Darwin.

delcypher marked an inline comment as done. delcypher added inline comments. Comment at: clang/lib/Driver/ToolChain.cpp:401 return "aix"; + case llvm::Triple::Darwin: +return "darwin"; arphaman wrote: > Should this also apply to `llvm::Triple::MacOSX` ,

[PATCH] D101682: [Driver] Fix `ToolChain::getCompilerRTPath()` to return the correct path on Apple platforms.

delcypher updated this revision to Diff 342809. delcypher edited the summary of this revision. delcypher added a comment. Use `isOSDarwin()` instead of explicitly checking Darwin OS enum values. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D101682/n

  1   2   >