[PATCH] D103564: [NFC][compiler-rt][hwasan] Move allocation functions into their own file

mcgrathr added inline comments. Comment at: compiler-rt/lib/hwasan/hwasan_interceptors.cpp:171 } // namespace __hwasan +#else // #if !SANITIZER_FUCHSIA +namespace __hwasan { blank lines around `#else` and `#endif` lines. But here I think you just omit this.

[PATCH] D103936: [compiler-rt][hwasan] Define fuchsia implementations of required hwasan functions

mcgrathr added inline comments. Comment at: compiler-rt/lib/hwasan/hwasan_fuchsia.cpp:29 + +uptr kHighMemEnd; +uptr kHighMemBeg; These need comments about what they are and why they need to exist as runtime variables at all. Comment at: compil

[PATCH] D104085: [compiler-rt][hwasan] Setup hwasan thread handling on Fuchsia

mcgrathr added a comment. Usually we like to split changes up into separate small changes for the pure refactoring, and then changes that purely add new Fuchsia-specific code. I'll do an initial review round of the Fuchsia code here since you've sent it. But then I think this review should be t

[PATCH] D103936: [compiler-rt][hwasan] Define fuchsia implementations of required hwasan functions

mcgrathr added inline comments. Comment at: compiler-rt/lib/hwasan/hwasan_fuchsia.cpp:29 + +uptr kHighMemEnd; +uptr kHighMemBeg; leonardchan wrote: > mcgrathr wrote: > > These need comments about what they are and why they need to exist as > > runtime variables

[PATCH] D104248: [compiler-rt][hwasan] Move Thread::Init into hwasan_linux.cpp

mcgrathr added inline comments. Comment at: compiler-rt/lib/hwasan/hwasan_linux.cpp:430 +void Thread::Init(uptr stack_buffer_start, uptr stack_buffer_size) { + CHECK_EQ(0, unique_id_); // try to catch bad stack reuse Most of this code can actually be reused f

[PATCH] D104248: [compiler-rt][hwasan] Refactor Thread::Init

mcgrathr added inline comments. Comment at: compiler-rt/lib/hwasan/hwasan_linux.cpp:430 +void Thread::Init(uptr stack_buffer_start, uptr stack_buffer_size) { + CHECK_EQ(0, unique_id_); // try to catch bad stack reuse vitalybuka wrote: > leonardchan wrote: > >

[PATCH] D104275: [compiler-rt][hwasan] Add GetShadowOffset function

mcgrathr added inline comments. Comment at: compiler-rt/lib/hwasan/hwasan_mapping.h:54 +inline uptr GetShadowOffset() { + return SANITIZER_FUCHSIA ? 0 : __hwasan_shadow_memory_dynamic_address; +} I think you'll eventually need to make this an `#if` or `if conste

[PATCH] D104275: [compiler-rt][hwasan] Add GetShadowOffset function

mcgrathr added inline comments. Comment at: compiler-rt/lib/hwasan/hwasan_mapping.h:54 +inline uptr GetShadowOffset() { + return SANITIZER_FUCHSIA ? 0 : __hwasan_shadow_memory_dynamic_address; +} vitalybuka wrote: > mcgrathr wrote: > > I think you'll eventually

[PATCH] D99381: [compiler-rt][hwasan] Add Fuchsia-specific sanitizer platform limits

mcgrathr added a comment. Frankly I don't think it makes sense to have __sanitizer_mallinfo and those other allocator functions declared in hwasan_interface_internal.h at all. Those are neither APIs that the compiler emits nor ones that anyone else should use. They are just implementation detai

[PATCH] D103544: [compiler-rt][Fuchsia] Disable interceptors while enabling new/delete replacements

mcgrathr accepted this revision. mcgrathr 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/D103544/new/ https://reviews.llvm.org/D103544 ___

[PATCH] D103936: [compiler-rt][hwasan] Define fuchsia implementations of required hwasan functions

mcgrathr accepted this revision. mcgrathr added a comment. This revision is now accepted and ready to land. lgtm with minor changes + clang-format. Comment at: compiler-rt/lib/hwasan/hwasan_fuchsia.cpp:29 + // now. Otherwise, ShadowBounds will be a zero-initialized global. +

[PATCH] D104085: [compiler-rt][hwasan] Setup hwasan thread handling on Fuchsia

mcgrathr accepted this revision. mcgrathr 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/D104085/new/ https://reviews.llvm.org/D104085 ___

[PATCH] D113136: [Clang] Pass -z rel to linker for Fuchsia

mcgrathr created this revision. mcgrathr added a reviewer: phosek. Herald added a subscriber: abrachet. mcgrathr requested review of this revision. Herald added a project: clang. Herald added a subscriber: cfe-commits. Fuchsia already supports the more compact relocation format. Make it the defaul

[PATCH] D113136: [Clang] Pass -z rel to linker for Fuchsia

mcgrathr updated this revision to Diff 386262. mcgrathr added a comment. rebased Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D113136/new/ https://reviews.llvm.org/D113136 Files: clang/lib/Driver/ToolChains/Fuchsia.cpp clang/test/Driver/fuchsi

[PATCH] D113136: [Clang] Pass -z rel to linker for Fuchsia

mcgrathr updated this revision to Diff 386293. mcgrathr marked an inline comment as done. mcgrathr added a comment. reordered switches Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D113136/new/ https://reviews.llvm.org/D113136 Files: clang/lib/Dr

[PATCH] D113136: [Clang] Pass -z rel to linker for Fuchsia

mcgrathr added a comment. Reordered the switches. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D113136/new/ https://reviews.llvm.org/D113136 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https:

[PATCH] D113136: [Clang] Pass -z rel to linker for Fuchsia

This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rGff11f0aa5de1: [Clang] Pass -z rel to linker for Fuchsia (authored by mcgrathr). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION h

[PATCH] D98572: [Fuchsia] Add check-polly to CLANG_BOOTSTRAP_TARGETS

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

[PATCH] D114023: [Driver] Pass --fix-cortex-a53-843419 automatically on Fuchsia

mcgrathr added inline comments. Comment at: clang/lib/Driver/ToolChains/Fuchsia.cpp:91 +std::string CPU = getCPUName(D, Args, Triple); +if (CPU.empty() || CPU == "generic" || CPU == "cortex-a53") + CmdArgs.push_back("--fix-cortex-a53-843419"); How do

[PATCH] D40319: [libcxx] Support getentropy as a source of randomness for std::random_device

mcgrathr added inline comments. Comment at: libcxx/trunk/src/random.cpp:29 +#if defined(_LIBCPP_USING_GETENTROPY) +#include +#elif defined(_LIBCPP_USING_DEV_RANDOM) jwakely wrote: > jwakely wrote: > > musl only declares `getentropy` in `` not ``. Glibc > > decl

[PATCH] D44605: [Driver] Default to DWARF 5 for Fuchsia

mcgrathr accepted this revision. mcgrathr added a comment. This revision is now accepted and ready to land. lgtm, but we should be sure to advise users who may need to use `-gdwarf-4` explicitly if their debugging tools become unhappy. Repository: rC Clang CHANGES SINCE LAST ACTION https:/

[PATCH] D114115: [Driver] Support for compressed debug info on Fuchsia

mcgrathr accepted this revision. mcgrathr added a comment. This revision is now accepted and ready to land. lgtm, but we should be sure to advise users that their debugging tools may need build adjustments to ensure they support the compressed formats. Repository: rG LLVM Github Monorepo CHA

[PATCH] D114023: [Driver] Pass --fix-cortex-a53-843419 automatically on Fuchsia

mcgrathr accepted this revision. mcgrathr added inline comments. This revision is now accepted and ready to land. Comment at: clang/lib/Driver/ToolChains/Fuchsia.cpp:91 +std::string CPU = getCPUName(D, Args, Triple); +if (CPU.empty() || CPU == "generic" || CPU == "cortex-

[PATCH] D85802: [clang] Add -fc++-abi= flag for specifying which C++ ABI to use

mcgrathr added a comment. In D85802#3876106 , @dblaikie wrote: >> The C++ ABI is not part of the Fuchsia system ABI, nor what we call the >> "Fuchsia compiler ABI". Different users of C++ are free to use whatever C++ >> ABI they like. Only the backend AB

[PATCH] D85802: [clang] Add -fc++-abi= flag for specifying which C++ ABI to use

mcgrathr added a comment. In D85802#3879950 , @dblaikie wrote: > In D85802#3879888 , @mcgrathr wrote: > >> In D85802#3876106 , @dblaikie wrote: >> The C++ ABI is not par

[PATCH] D132425: [clang] Do not instrument relative vtables under hwasan

mcgrathr accepted this revision. mcgrathr added a comment. This revision is now accepted and ready to land. lgtm. You might add some comments in CGVTables.cpp about why this is done and what the alternatives might be in the future. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTIO

[PATCH] D135713: [cmake][Fuchsia] Add -ftrivial-auto-var-init=zero to runtimes build

mcgrathr added a comment. I had imagined making lsan always do this since there's a specific lsan-related rationale for it that applies to everybody. I'm not clear on how cmake structures things and whether sanitizer_common would have to do it unconditionally since it's used by lsan code. Doing

[PATCH] D85802: [clang] Add -fc++-abi= flag for specifying which C++ ABI to use

mcgrathr added a comment. The C++ ABI is not part of the Fuchsia system ABI, nor what we call the "Fuchsia compiler ABI". Different users of C++ are free to use whatever C++ ABI they like. Only the backend ABI independent of language-specific issues is necessary to interoperate with other code

[PATCH] D85802: [clang] Add -fc++-abi= flag for specifying which C++ ABI to use

mcgrathr added a comment. It's certainly correct that we envision each target having an explicit list of viable C++ ABIs to select from. AIUI Clang has no generic multilib support but it's very patchily supported differently in different per-target drivers. The Fuchsia target support in the d

[PATCH] D85802: [clang] Add -fc++-abi= flag for specifying which C++ ABI to use

mcgrathr added a comment. I'm quite sure that we will always want a means to select the original Itanium ABI. It's also quite likely that there will be future innovations in the Fuchsia C++ ABI and we'll go through migration periods of supporting additional variants and changing the default for

[PATCH] D140857: Fix Fuchsia dyld in asan-ubsan variant

mcgrathr added a comment. I don't think we want this in the generic compiler behavior. It would only make sense if the toolchain runtimes have extra versions, and I don't think we want that many extra versions. It may be worthwhile to change the asan multilib builds to enable ubsan checks too,

[PATCH] D143355: [RISCV] Default to -ffixed-x18 for Fuchsia

mcgrathr created this revision. mcgrathr added reviewers: phosek, paulkirth, leonardchan. Herald added subscribers: luke, VincentWu, abrachet, vkmr, frasercrmck, evandro, luismarques, apazos, sameer.abuasal, s.egerton, Jim, benna, psnobl, jocewei, PkmX, the_o, brucehoult, MartinMosbeck, rogfer01,

[PATCH] D143357: [RISCV] Default to -fsanitize=shadow-call-stack for Fuchsia

mcgrathr created this revision. mcgrathr added reviewers: phosek, paulkirth, leonardchan. Herald added subscribers: VincentWu, abrachet, vkmr, evandro, luismarques, sameer.abuasal, s.egerton, Jim, benna, psnobl, rogfer01, shiva0217, kito-cheng, simoncook, arichardson. Herald added a project: All.

[PATCH] D143355: [RISCV] Default to -ffixed-x18 for Fuchsia

mcgrathr added inline comments. Comment at: llvm/lib/Target/RISCV/RISCVSubtarget.cpp:16 +#include "GISel/RISCVLegalizerInfo.h" +#include "GISel/RISCVRegisterBankInfo.h" #include "RISCV.h" jrtc27 wrote: > Unrelated change arcanist requested it via clang-format

[PATCH] D143355: [RISCV] Default to -ffixed-x18 for Fuchsia

mcgrathr updated this revision to Diff 494970. mcgrathr added a comment. rebased Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D143355/new/ https://reviews.llvm.org/D143355 Files: clang/lib/Driver/SanitizerArgs.cpp llvm/include/llvm/TargetParse

[PATCH] D143355: [RISCV] Default to -ffixed-x18 for Fuchsia

mcgrathr updated this revision to Diff 494972. mcgrathr added a comment. rebased, added clang driver test vs -fsanitize=shadow-call-stack Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D143355/new/ https://reviews.llvm.org/D143355 Files: clang/lib

[PATCH] D143357: [RISCV] Default to -fsanitize=shadow-call-stack for Fuchsia

mcgrathr updated this revision to Diff 494973. mcgrathr marked an inline comment as done. mcgrathr added a comment. remove TODO Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D143357/new/ https://reviews.llvm.org/D143357 Files: clang/lib/Driver/To

[PATCH] D143355: [RISCV] Default to -ffixed-x18 for Fuchsia

This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rG03ff435da540: [RISCV] Default to -ffixed-x18 for Fuchsia (authored by mcgrathr). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION

[PATCH] D143357: [RISCV] Default to -fsanitize=shadow-call-stack for Fuchsia

This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rGf08d86fc7f44: [RISCV] Default to -fsanitize=shadow-call-stack for Fuchsia (authored by mcgrathr). Repository: rG LLVM Github Monorepo CHANGES SIN

[PATCH] D127887: [CMake][Fuchsia] Use libunwind as the default unwinder

mcgrathr accepted this revision. mcgrathr 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/D127887/new/ https://reviews.llvm.org/D127887 ___

[PATCH] D157164: [clang-tidy] Add fix-it support to `llvmlibc-inline-function-decl`

mcgrathr created this revision. mcgrathr added reviewers: abrachet, Caslyn, sivachandra, michaelrj. Herald added subscribers: PiotrZSL, carlosgalvezp, xazax.hun. Herald added a reviewer: njames93. Herald added a project: All. mcgrathr requested review of this revision. Herald added a project: clang

[PATCH] D157164: [clang-tidy] Add fix-it support to `llvmlibc-inline-function-decl`

This revision was automatically updated to reflect the committed changes. Closed by commit rG9d4162ff28b4: [clang-tidy] Add fix-it support to `llvmlibc-inline-function-decl` (authored by mcgrathr). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D157164

[PATCH] D155337: [CMake] Include riscv32-unknown-elf target in Fuchsia toolchain

mcgrathr accepted this revision. mcgrathr added a comment. This revision is now accepted and ready to land. It would be a better summary to say "include ... runtimes ...". Comment at: clang/cmake/caches/Fuchsia-stage2.cmake:10 set(_FUCHSIA_ENABLE_PROJECTS "bolt;clang;clang-too

[PATCH] D158822: [Fuchsia] Make __start_* / __stop_* symbols hidden

mcgrathr accepted this revision. mcgrathr 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/D158822/new/ https://reviews.llvm.org/D158822 ___

[PATCH] D72959: Relative VTables ABI on Fuchsia

mcgrathr added inline comments. Comment at: libcxxabi/src/private_typeinfo.cpp:617 // Get (dynamic_ptr, dynamic_type) from static_ptr +#ifndef __Fuchsia__ void **vtable = *static_cast(static_ptr); ldionne wrote: > Please introduce a macro that generical

[PATCH] D75002: [AArch64] Predefine __AARCH64_CMODEL_*__ as GCC does

mcgrathr created this revision. Herald added subscribers: cfe-commits, kristof.beyls. Herald added a project: clang. mcgrathr updated this revision to Diff 246043. mcgrathr added a comment. mcgrathr edited the summary of this revision. update log Make Clang on aarch64 targets predefine `__AARCH6

[PATCH] D75002: [AArch64] Predefine __AARCH64_CMODEL_*__ as GCC does

mcgrathr updated this revision to Diff 246043. mcgrathr added a comment. update log Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D75002/new/ https://reviews.llvm.org/D75002 Files: clang/lib/Basic/Targets/AArch64.cpp clang/test/Preprocessor/ini

[PATCH] D75003: [X86] Fix __code_model_*__ predefine names

mcgrathr created this revision. mcgrathr added a reviewer: tamur. Herald added a project: clang. Herald added a subscriber: cfe-commits. GCC defines __code_model_*__ (two trailing underscores), not __code_model_*_ (one trailing underscore). Repository: rG LLVM Github Monorepo https://reviews.

[PATCH] D75003: [X86] Fix __code_model_*__ predefine names

mcgrathr added a comment. Thanks! Can you land it for me? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D75003/new/ https://reviews.llvm.org/D75003 ___ cfe-commits mailing list cfe-commits@lists.llvm.or

[PATCH] D75002: [AArch64] Predefine __AARCH64_CMODEL_*__ as GCC does

mcgrathr added a comment. Thanks! Can you land it for me? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D75002/new/ https://reviews.llvm.org/D75002 ___ cfe-commits mailing list cfe-commits@lists.llvm.or

[PATCH] D73397: [Clang] Enable -fsanitize=leak on Fuchsia targets

mcgrathr created this revision. mcgrathr added reviewers: phosek, aarongreen, cryptoad, vitalybuka. Herald added a project: clang. Herald added a subscriber: cfe-commits. This required some fixes to the generic code for two issues: 1. -fsanitize=safe-stack is default on x86_64-fuchsia and is *not

[PATCH] D73397: [Clang] Enable -fsanitize=leak on Fuchsia targets

mcgrathr updated this revision to Diff 240346. mcgrathr added a comment. Fixed Android SafeStack case, now passing check-clang. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D73397/new/ https://reviews.llvm.org/D73397 Files: clang/lib/Driver/Sani

[PATCH] D73734: [Fuchsia] Never link in implicit "system dependencies" of sanitizer runtimes

mcgrathr created this revision. mcgrathr added a reviewer: phosek. Herald added a project: clang. Herald added a subscriber: cfe-commits. This is never appropriate on Fuchsia and any future needs for system library dependencies of compiler-supplied runtimes will be addressed via `.deplibs` instead

[PATCH] D73734: [Fuchsia] Never link in implicit "system dependencies" of sanitizer runtimes

mcgrathr updated this revision to Diff 241560. mcgrathr added a comment. builds now. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D73734/new/ https://reviews.llvm.org/D73734 Files: clang/lib/Driver/ToolChains/CommonArgs.cpp Index: clang/lib/Dr

[PATCH] D73734: [Fuchsia] Never link in implicit "system dependencies" of sanitizer runtimes

mcgrathr updated this revision to Diff 241561. mcgrathr added a comment. style nit Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D73734/new/ https://reviews.llvm.org/D73734 Files: clang/lib/Driver/ToolChains/CommonArgs.cpp Index: clang/lib/Driv

[PATCH] D44724: [Fuchsia] Avoid using static ABI library with libc++ on Darwin

mcgrathr added a comment. When targetting Darwin we should really not be even compiling libc++/libc++abi and certainly should not have anything else compiling against their headers since at runtime the system libraries will be used and so things better compile against the system headers that ma

[PATCH] D44724: [Fuchsia] Don't install libc++, libc++abi or libunwind on Darwin

mcgrathr accepted this revision. mcgrathr added a comment. This revision is now accepted and ready to land. Lgtm with some more comments Repository: rC Clang https://reviews.llvm.org/D44724 ___ cfe-commits mailing list cfe-commits@lists.llvm.org

[PATCH] D44947: [Driver] Add fuzzer-no-link into the list of supported Fuchsia sanitizers

mcgrathr accepted this revision. mcgrathr added a comment. This revision is now accepted and ready to land. lgtm Repository: rC Clang https://reviews.llvm.org/D44947 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cg

[PATCH] D39930: [CMake] Use libc++ and compiler-rt as default libraries in Fuchsia toolchain

mcgrathr added a comment. In principle this should be OK since we're unconditionally building the runtimes for `default`. However, this means that any dynamically-linked host builds will assume our toolchain-provided shared libraries are available on the runtime host in standard library places.

[PATCH] D39930: [CMake] Use libc++ and compiler-rt as default libraries in Fuchsia toolchain

mcgrathr accepted this revision. mcgrathr added a comment. This revision is now accepted and ready to land. LGTM with comment added but if a different approach will be required for multiple Linux targets (which we all treat as "hostish") then it's worth at least thinking about that now. =

[PATCH] D39831: [Driver] Make the use of relax relocations a per target option

mcgrathr added a comment. test case? Repository: rL LLVM https://reviews.llvm.org/D39831 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D39831: [Driver] Make the use of relax relocations a per target option

mcgrathr accepted this revision. mcgrathr added a comment. This revision is now accepted and ready to land. LGTM Repository: rL LLVM https://reviews.llvm.org/D39831 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi

[PATCH] D40319: [libcxx] Support getentropy as a source of randomness for std::random_device

mcgrathr accepted this revision. mcgrathr added a comment. This revision is now accepted and ready to land. LGTM Repository: rL LLVM https://reviews.llvm.org/D40319 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi

[PATCH] D146463: [CodeGen][RISCV] Change Shadow Call Stack Register to X3

mcgrathr added a comment. In D146463#4232214 , @craig.topper wrote: > Does the kernel populate the initial value of the register for the shadow > stack or is that done by a runtime inside the application? This is a change to code generation and is orth

[PATCH] D146463: [CodeGen][RISCV] Change Shadow Call Stack Register to X3

mcgrathr added a comment. In D146463#4232252 , @craig.topper wrote: > In D146463#4232221 , @mcgrathr > wrote: > >> In D146463#4232214 , @craig.topper >> wrote: >> >>> D

[PATCH] D146463: [CodeGen][RISCV] Change Shadow Call Stack Register to X3

mcgrathr added a comment. I don't see changes here to make the Fuchsia & Android targets stop doing implicit `-ffixed-x18` and to make them start doing implicit `-msmall-data-limit=0` for non-PIC modes. Comment at: clang/lib/Driver/SanitizerArgs.cpp:546 - if ((Kinds & Sani

[PATCH] D146463: [CodeGen][RISCV] Change Shadow Call Stack Register to X3

mcgrathr accepted this revision. mcgrathr added a comment. lgtm with some nits and a few more test cases. Comment at: clang/docs/ShadowCallStack.rst:61 +The instrumentation makes use of the platform register ``x18`` on AArch64 and +``x3`` on RISC-V. For simplicity we will refer

[PATCH] D151186: [Driver] Properly handle -pie and -nopie on Fuchsia

mcgrathr added a comment. In GCC the spelling is `-no-pie`. `-nopie` is not documented as an alias, and I don't think it works. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D151186/new/ https://reviews.llvm.org/D151186 ___

[PATCH] D152405: [WIP][clang] Add experimental option to omit the RTTI component from the vtable when -fno-rtti is used

mcgrathr added a comment. It's not clear from the change description if this can be enabled orthogonally to relative-vtables. I think it should be possible to choose each switch independently, thus generating 4 variants of the vtable layout ABI. Does any runtime code (libc++abi) ever need to kn

[PATCH] D152405: [WIP][clang] Add experimental option to omit the RTTI component from the vtable when -fno-rtti is used

mcgrathr added a comment. In D152405#4404616 , @leonardchan wrote: > Oh this is completely independent from relative vtables. I'll update the > wording. Great. I'd like to see us try some experiments with enabling both together in places like the Fuch

[PATCH] D139589: [clang][Fuchsia] Re-enable hwasan global instrumentation on hwasan multilibs

mcgrathr accepted this revision. mcgrathr 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/D139589/new/ https://reviews.llvm.org/D139589 ___

[PATCH] D35582: [Driver] Always use -z rodynamic for Fuchsia

mcgrathr requested changes to this revision. mcgrathr added a comment. This revision now requires changes to proceed. Looks almost right. Might be overkill to add a test case that -z rodynamic is omitted with -fuse-ld=gold or something. Comment at: lib/Driver/ToolChains/Fuchs

[PATCH] D35582: [Driver] Always use -z rodynamic for Fuchsia

mcgrathr accepted this revision. mcgrathr added a comment. This revision is now accepted and ready to land. LGTM Repository: rL LLVM https://reviews.llvm.org/D35582 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi

[PATCH] D35603: [Driver] Consider -fno-sanitize=... state when filtering out -fsanitize-coverage=...

mcgrathr created this revision. The driver ignores -fsanitize-coverage=... flags when also given -fsanitize=... flags for sanitizer flavors that don't support the coverage runtime. This logic failed to account for subsequent -fno-sanitize=... flags that disable the sanitizer flavors that conflict

[PATCH] D35705: [CMake] Use ABI version 2 for C++ library

mcgrathr accepted this revision. mcgrathr added a comment. This revision is now accepted and ready to land. LGTM Repository: rL LLVM https://reviews.llvm.org/D35705 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi

[PATCH] D35747: [Driver] Fuchsia defaults to -fno-math-errno

mcgrathr created this revision. Repository: rL LLVM https://reviews.llvm.org/D35747 Files: lib/Driver/ToolChains/Fuchsia.h Index: lib/Driver/ToolChains/Fuchsia.h === --- lib/Driver/ToolChains/Fuchsia.h +++ lib/Driver/ToolChain

[PATCH] D35747: [Driver] Fuchsia defaults to -fno-math-errno

mcgrathr added a comment. Please land it for me when approved. https://reviews.llvm.org/D35747 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D35747: [Driver] Fuchsia defaults to -fno-math-errno

mcgrathr updated this revision to Diff 107752. mcgrathr added a comment. test case https://reviews.llvm.org/D35747 Files: lib/Driver/ToolChains/Fuchsia.h test/Driver/fast-math.c Index: test/Driver/fast-math.c === --- test/Dri

[PATCH] D35820: [Driver] Search compiler-rt paths in -print-file-name=

mcgrathr accepted this revision. mcgrathr added a comment. This revision is now accepted and ready to land. LGTM. Perhaps there should be a test. Repository: rL LLVM https://reviews.llvm.org/D35820 ___ cfe-commits mailing list cfe-commits@lists.

[PATCH] D35922: [Driver] Enable AddressSanitizer for Fuchsia targets

mcgrathr created this revision. mcgrathr added a project: Sanitizers. Herald added subscribers: mgorny, srhines. Repository: rL LLVM https://reviews.llvm.org/D35922 Files: cmake/caches/Fuchsia-stage2.cmake lib/Driver/SanitizerArgs.cpp lib/Driver/ToolChains/Fuchsia.cpp Index: lib/Driver

[PATCH] D35930: [CMake] Include sancov tool in Fuchsia toolchain

mcgrathr accepted this revision. mcgrathr added a comment. This revision is now accepted and ready to land. LGTM Repository: rL LLVM https://reviews.llvm.org/D35930 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi

[PATCH] D42019: [Driver] Set default sysroot for Fuchsia if none is specified

mcgrathr added inline comments. Comment at: lib/Driver/ToolChains/Fuchsia.cpp:152 + llvm::sys::path::append(P, normalizeTriple(Triple)); + return P.str(); +} The GCC behavior (if --with-sysroot is given to configure) is tooldir/sys-root, not tooldir itself.

[PATCH] D29843: [CodeGen] Treat auto-generated __dso_handle symbol as HiddenVisibility

mcgrathr updated this revision to Diff 88064. https://reviews.llvm.org/D29843 Files: lib/CodeGen/CodeGenModule.cpp lib/CodeGen/CodeGenModule.h lib/CodeGen/ItaniumCXXABI.cpp Index: lib/CodeGen/ItaniumCXXABI.cpp === --- lib/Cod

[PATCH] D29843: [CodeGen] Treat auto-generated __dso_handle symbol as HiddenVisibility

mcgrathr added a comment. This was just the first crack, my first attempt at any change in LLVMland. I don't know how to run the tests yet. https://reviews.llvm.org/D29843 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.o

[PATCH] D29843: [CodeGen] Treat auto-generated __dso_handle symbol as HiddenVisibility

mcgrathr updated this revision to Diff 88067. https://reviews.llvm.org/D29843 Files: lib/CodeGen/CodeGenModule.cpp lib/CodeGen/CodeGenModule.h lib/CodeGen/ItaniumCXXABI.cpp test/CodeGenCXX/global-init.cpp test/OpenMP/threadprivate_codegen.cpp Index: test/OpenMP/threadprivate_codegen.c

[PATCH] D29843: [CodeGen] Treat auto-generated __dso_handle symbol as HiddenVisibility

mcgrathr updated this revision to Diff 88068. mcgrathr added a comment. This now passes check-clang. https://reviews.llvm.org/D29843 Files: lib/CodeGen/CodeGenModule.cpp lib/CodeGen/CodeGenModule.h lib/CodeGen/ItaniumCXXABI.cpp test/CodeGenCXX/global-init.cpp test/OpenMP/threadprivate

[PATCH] D30238: [Driver] Enable SafeStack for Fuchsia targets

mcgrathr created this revision. The runtime support is provided directly by the Fuchsia system C library. Repository: rL LLVM https://reviews.llvm.org/D30238 Files: lib/Driver/ToolChains.cpp lib/Driver/ToolChains.h Index: lib/Driver/ToolChains.h

[PATCH] D30238: [Driver] Enable SafeStack for Fuchsia targets

mcgrathr added a comment. This assumes https://reviews.llvm.org/D30237 Repository: rL LLVM https://reviews.llvm.org/D30238 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D30238: [Driver] Enable SafeStack for Fuchsia targets

mcgrathr updated this revision to Diff 89393. mcgrathr added a comment. Added test case. Repository: rL LLVM https://reviews.llvm.org/D30238 Files: lib/Driver/ToolChains.cpp lib/Driver/ToolChains.h test/Driver/fuchsia.c Index: test/Driver/fuchsia.c ===

[PATCH] D32231: [CMake] Enable ARM target in Fuchsia toolchain

mcgrathr accepted this revision. mcgrathr added a comment. This revision is now accepted and ready to land. lgtm Repository: rL LLVM https://reviews.llvm.org/D32231 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi

[PATCH] D32613: [Driver] Update Fuchsia driver path handling

mcgrathr added a comment. Perhaps use llvm::sys::path::parent_path instead of explicit "..". Perhaps put the getDriver.Dir() + .. + getTriple().str() logic into a shared subroutine. Use default rather that CST_Libstdcxx in the switches on GetCXXStdlibType. Repository: rL LLVM https://review

[PATCH] D32613: [Driver] Update Fuchsia driver path handling

mcgrathr accepted this revision. mcgrathr added a comment. This revision is now accepted and ready to land. lgtm Repository: rL LLVM https://reviews.llvm.org/D32613 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi

[PATCH] D118306: [CMake][Fuchsia] Drop 32-bit ios runtimes

mcgrathr accepted this revision. mcgrathr 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/D118306/new/ https://reviews.llvm.org/D118306 ___

[PATCH] D119996: [safestack] Support safestack in stack size diagnostics

mcgrathr added inline comments. Comment at: llvm/lib/CodeGen/AsmPrinter/AsmPrinter.cpp:1211 const MachineFrameInfo &FrameInfo = MF.getFrameInfo(); - uint64_t StackSize = FrameInfo.getStackSize(); + uint64_t StackSize = + FrameInfo.getStackSize() + FrameInfo.getUnsafeSta

[PATCH] D101967: [Driver] Use x86-64 libc++ headers with -m32

mcgrathr accepted this revision. mcgrathr added a comment. This revision is now accepted and ready to land. lgtm. I'm not sure what better to do that's especially cleaner, except actually just installing i386-fuchsia header subdirs. The easy way to do that would be to just build it all for the

[PATCH] D99381: [compiler-rt][hwasan] Add Fuchsia-specific sanitizer platform limits

mcgrathr added a comment. I don't think this is warranted. There should not be any references to such things when building for Fuchsia. This looks like it's intended to satisfy one reference in dead code in hwasan_interceptors.cpp. I think the right solution is just to refactor the hwasan code

[PATCH] D93668: [clang] Add -ffuchsia-c++-abi flag to explicitly use the Fuchsia C++ ABI

mcgrathr added a comment. It's fine to have a target tuple translate to a default C++ ABI. But the C++ ABI selection is fundamentally not a target flavor thing. It's just a C++ ABI thing. So using the target tuple as the sole mechanism to determine C++ ABI is fundamentally wrong. Repository:

[PATCH] D93668: [clang] Add -ffuchsia-c++-abi flag to explicitly use the Fuchsia C++ ABI

mcgrathr added a comment. You've said you want to avoid expressing the C++ ABI as a generic switch orthogonal to target because people can use it when it's not what they actually want or isn't actually useful to attempt. This is true of all switches affecting ABI and they haven't all been reje

[PATCH] D93668: [clang] Override the Fuchsia platform ABI using the triple environment

mcgrathr added a comment. The Fuchsia platform ABI includes the SS & SCS ABI. There is no compatibility issue with those. PIC-friendly optimizations are purely an optimization and correctness issue for any PIC/PIE code, and is not an ABI issue. The only thing where we have an incompatible diff

<    1   2