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

2023-08-25 Thread Roland McGrath via Phabricator via cfe-commits
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] D157164: [clang-tidy] Add fix-it support to `llvmlibc-inline-function-decl`

2023-08-07 Thread Roland McGrath via Phabricator via cfe-commits
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

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

2023-08-04 Thread Roland McGrath via Phabricator via cfe-commits
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:

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

2023-07-14 Thread Roland McGrath via Phabricator via cfe-commits
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

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

2023-06-07 Thread Roland McGrath via Phabricator via cfe-commits
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

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

2023-06-07 Thread Roland McGrath via Phabricator via cfe-commits
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

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

2023-05-23 Thread Roland McGrath via Phabricator via cfe-commits
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] D146463: [CodeGen][RISCV] Change Shadow Call Stack Register to X3

2023-04-10 Thread Roland McGrath via Phabricator via cfe-commits
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

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

2023-04-05 Thread Roland McGrath via Phabricator via cfe-commits
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 &

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

2023-03-29 Thread Roland McGrath via Phabricator via cfe-commits
mcgrathr added a comment. In D146463#4232252 , @craig.topper wrote: > In D146463#4232221 , @mcgrathr > wrote: > >> In D146463#4232214 , @craig.topper >> wrote: >> >>>

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

2023-03-29 Thread Roland McGrath via Phabricator via cfe-commits
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

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

2023-02-05 Thread Roland McGrath 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 rGf08d86fc7f44: [RISCV] Default to -fsanitize=shadow-call-stack for Fuchsia (authored by mcgrathr). Repository: rG LLVM Github Monorepo CHANGES

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

2023-02-05 Thread Roland McGrath 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 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

2023-02-05 Thread Roland McGrath via Phabricator via cfe-commits
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:

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

2023-02-05 Thread Roland McGrath via Phabricator via cfe-commits
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:

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

2023-02-05 Thread Roland McGrath via Phabricator via cfe-commits
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

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

2023-02-05 Thread Roland McGrath via Phabricator via cfe-commits
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] D143357: [RISCV] Default to -fsanitize=shadow-call-stack for Fuchsia

2023-02-05 Thread Roland McGrath via Phabricator via cfe-commits
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:

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

2023-02-05 Thread Roland McGrath via Phabricator via cfe-commits
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,

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

2023-01-03 Thread Roland McGrath via Phabricator via cfe-commits
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] D139589: [clang][Fuchsia] Re-enable hwasan global instrumentation on hwasan multilibs

2022-12-07 Thread Roland McGrath via Phabricator via cfe-commits
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] D85802: [clang] Add -fc++-abi= flag for specifying which C++ ABI to use

2022-10-24 Thread Roland McGrath via Phabricator via cfe-commits
mcgrathr added a comment. In D85802#3879950 , @dblaikie wrote: > In D85802#3879888 , @mcgrathr wrote: > >> In D85802#3876106 , @dblaikie wrote: >> The C++ ABI is not

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

2022-10-24 Thread Roland McGrath via Phabricator via cfe-commits
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

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

2022-10-17 Thread Roland McGrath via Phabricator via cfe-commits
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

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

2022-10-17 Thread Roland McGrath via Phabricator via cfe-commits
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

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

2022-10-17 Thread Roland McGrath via Phabricator via cfe-commits
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] D135713: [cmake][Fuchsia] Add -ftrivial-auto-var-init=zero to runtimes build

2022-10-11 Thread Roland McGrath via Phabricator via cfe-commits
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.

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

2022-08-23 Thread Roland McGrath via Phabricator via cfe-commits
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

[PATCH] D129512: [Driver] Don't use frame pointer on Fuchsia when optimizations are enabled

2022-07-11 Thread Roland McGrath via Phabricator via cfe-commits
mcgrathr accepted this revision. mcgrathr added a comment. This revision is now accepted and ready to land. lgtm but be sure that Fuchsia target users get clear release-notes warning about the change. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION

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

2022-06-15 Thread Roland McGrath via Phabricator via cfe-commits
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] D122613: [Driver] Make -moutline-atomics default for aarch64-fuchsia targets

2022-03-28 Thread Roland McGrath 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 rG1a963d3278c2: [Driver] Make -moutline-atomics default for aarch64-fuchsia targets (authored by mcgrathr). Repository: rG LLVM Github Monorepo

[PATCH] D122613: [Driver] Make -moutline-atomics default for aarch64-fuchsia targets

2022-03-28 Thread Roland McGrath via Phabricator via cfe-commits
mcgrathr created this revision. mcgrathr added reviewers: phosek, abrachet. Herald added a subscriber: kristof.beyls. Herald added a project: All. mcgrathr requested review of this revision. Herald added subscribers: cfe-commits, MaskRay. Herald added a project: clang. This makes Fuchsia

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

2022-03-04 Thread Roland McGrath via Phabricator via cfe-commits
mcgrathr added inline comments. Comment at: llvm/lib/CodeGen/AsmPrinter/AsmPrinter.cpp:1211 const MachineFrameInfo = MF.getFrameInfo(); - uint64_t StackSize = FrameInfo.getStackSize(); + uint64_t StackSize = + FrameInfo.getStackSize() + FrameInfo.getUnsafeStackSize();

[PATCH] D119201: [clang][Fuchsia] Ensure static sanitizer libs are only linked in after the -nostdlib check

2022-02-07 Thread Roland McGrath via Phabricator via cfe-commits
mcgrathr added inline comments. Comment at: clang/lib/Driver/ToolChains/Fuchsia.cpp:157 if (NeedsSanitizerDeps) linkSanitizerRuntimeDeps(ToolChain, CmdArgs); This function is a no-op because it tests for fuchsia targets. So it probably makes more

[PATCH] D119201: [clang][Fuchsia] Ensure static sanitizer libs are only linked in after the -nostdlib check

2022-02-07 Thread Roland McGrath via Phabricator via cfe-commits
mcgrathr added inline comments. Comment at: clang/lib/Driver/ToolChains/Fuchsia.cpp:130 - bool NeedsSanitizerDeps = addSanitizerRuntimes(ToolChain, Args, CmdArgs); bool NeedsXRayDeps = addXRayRuntime(ToolChain, Args, CmdArgs); AddLinkerInputs(ToolChain, Inputs, Args,

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

2022-01-26 Thread Roland McGrath via Phabricator via cfe-commits
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] D114023: [Driver] Pass --fix-cortex-a53-843419 automatically on Fuchsia

2021-11-30 Thread Roland McGrath via Phabricator via cfe-commits
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 ==

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

2021-11-30 Thread Roland McGrath via Phabricator via cfe-commits
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

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

2021-11-30 Thread Roland McGrath via Phabricator via cfe-commits
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

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

2021-11-29 Thread Roland McGrath via Phabricator via cfe-commits
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 > >

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

2021-11-16 Thread Roland McGrath via Phabricator via cfe-commits
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

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

2021-11-10 Thread Roland McGrath 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 rGff11f0aa5de1: [Clang] Pass -z rel to linker for Fuchsia (authored by mcgrathr). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION

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

2021-11-10 Thread Roland McGrath via Phabricator via cfe-commits
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

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

2021-11-10 Thread Roland McGrath via Phabricator via cfe-commits
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:

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

2021-11-10 Thread Roland McGrath via Phabricator via cfe-commits
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

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

2021-11-03 Thread Roland McGrath via Phabricator via cfe-commits
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

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

2021-07-05 Thread Roland McGrath via Phabricator via cfe-commits
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] D103936: [compiler-rt][hwasan] Define fuchsia implementations of required hwasan functions

2021-07-05 Thread Roland McGrath via Phabricator via cfe-commits
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] D103544: [compiler-rt][Fuchsia] Disable interceptors while enabling new/delete replacements

2021-07-05 Thread Roland McGrath via Phabricator via cfe-commits
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] D99381: [compiler-rt][hwasan] Add Fuchsia-specific sanitizer platform limits

2021-07-05 Thread Roland McGrath via Phabricator via cfe-commits
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

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

2021-06-16 Thread Roland McGrath via Phabricator via cfe-commits
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] D104275: [compiler-rt][hwasan] Add GetShadowOffset function

2021-06-15 Thread Roland McGrath via Phabricator via cfe-commits
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

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

2021-06-15 Thread Roland McGrath via Phabricator via cfe-commits
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] D104248: [compiler-rt][hwasan] Move Thread::Init into hwasan_linux.cpp

2021-06-14 Thread Roland McGrath via Phabricator via cfe-commits
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

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

2021-06-11 Thread Roland McGrath via Phabricator via cfe-commits
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] D104085: [compiler-rt][hwasan] Setup hwasan thread handling on Fuchsia

2021-06-11 Thread Roland McGrath via Phabricator via cfe-commits
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

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

2021-06-08 Thread Roland McGrath via Phabricator via cfe-commits
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:

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

2021-06-07 Thread Roland McGrath via Phabricator via cfe-commits
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] D103564: [NFC][compiler-rt][hwasan] Wrap fork interceptor with HWASAN_WITH_INTERCEPTORS

2021-06-03 Thread Roland McGrath via Phabricator via cfe-commits
mcgrathr added a comment. In D103564#2797657 , @leonardchan wrote: > I don't think we can move the `__sanitizer_*` allocation functions since > they're used for aliases for their libc equivalents which require definitions > in the same TU: What I

[PATCH] D103564: [NFC][compiler-rt][hwasan] Wrap vfork interceptor with HWASAN_WITH_INTERCEPTORS

2021-06-02 Thread Roland McGrath via Phabricator via cfe-commits
mcgrathr added a comment. I think probably the better refactoring here would be to split this file in two: one containing just the allocation functions; and a second with everything else. On Fuchsia, we'll use the allocation functions but none of the rest of it. So the second whole file could

[PATCH] D103555: [Fuchsia] Use libc++abi on Windows in Fuchsia toolchain

2021-06-02 Thread Roland McGrath via Phabricator via cfe-commits
mcgrathr accepted this revision. mcgrathr added a comment. lgtm Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D103555/new/ https://reviews.llvm.org/D103555 ___ cfe-commits mailing list

[PATCH] D103543: [compiler-rt][Fuchsia] Support HWASan on Fuchsia

2021-06-02 Thread Roland McGrath via Phabricator via cfe-commits
mcgrathr accepted this revision. mcgrathr added a comment. This revision is now accepted and ready to land. I think this may need to be about the last thing to actually land. It shouldn't land before building the runtime reliably works without error, and I think we should land any refactoring

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

2021-06-02 Thread Roland McGrath via Phabricator via cfe-commits
mcgrathr added inline comments. Comment at: compiler-rt/lib/hwasan/CMakeLists.txt:45 +if (NOT FUCHSIA) + append_list_if(COMPILER_RT_HWASAN_WITH_INTERCEPTORS HWASAN_WITH_INTERCEPTORS=1 HWASAN_DEFINITIONS) +else() It might be better to force the value of

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

2021-05-06 Thread Roland McGrath via Phabricator via cfe-commits
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

2021-03-25 Thread Roland McGrath via Phabricator via cfe-commits
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] D98572: [Fuchsia] Add check-polly to CLANG_BOOTSTRAP_TARGETS

2021-03-12 Thread Roland McGrath via Phabricator via cfe-commits
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] D93668: [clang] Override the Fuchsia platform ABI using the triple environment

2021-01-21 Thread Roland McGrath via Phabricator via cfe-commits
mcgrathr added a comment. I am also a C++ programmer working on C++ ABI support. I'm glad to be working on a platform that made many intentional design decisions to enable keeping the C++ ABI out of the platform ABI. It makes it vastly easier to do interesting work on C++ ABIs such as what

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

2021-01-20 Thread Roland McGrath via Phabricator via cfe-commits
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

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

2021-01-08 Thread Roland McGrath via Phabricator via cfe-commits
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

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

2021-01-07 Thread Roland McGrath via Phabricator via cfe-commits
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] D92444: [CMake][Fuchsia] Install llvm-elfabi

2020-12-02 Thread Roland McGrath via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes. Closed by commit rG70764c02e474: [CMake][Fuchsia] Install llvm-elfabi (authored by mcgrathr). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D92444/new/

[PATCH] D92444: [CMake][Fuchsia] Install llvm-elfabi

2020-12-01 Thread Roland McGrath via Phabricator via cfe-commits
mcgrathr created this revision. mcgrathr added reviewers: leonardchan, haowei. Herald added subscribers: cfe-commits, mgorny. Herald added a project: clang. mcgrathr requested review of this revision. The canonical Fuchsia toolchain configuration installs llvm-elfabi. Repository: rG LLVM

[PATCH] D91387: [Driver] Support UBSan multilib

2020-11-13 Thread Roland McGrath via Phabricator via cfe-commits
mcgrathr added inline comments. Comment at: clang/cmake/caches/Fuchsia-stage2.cmake:214-215 + set(RUNTIMES_${target}-unknown-fuchsia+ubsan+noexcept_LLVM_BUILD_COMPILER_RT OFF CACHE BOOL "") +set(RUNTIMES_${target}-unknown-fuchsia+ubsan+noexcept_LLVM_USE_SANITIZER

[PATCH] D91226: [clang] Add missing header guard in

2020-11-10 Thread Roland McGrath 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 rGcf36142d342a: [clang] Add missing header guard in cpuid.h (authored by mcgrathr). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION

[PATCH] D91226: [clang] Add missing header guard in

2020-11-10 Thread Roland McGrath via Phabricator via cfe-commits
mcgrathr created this revision. mcgrathr added reviewers: phosek, leonardchan. Herald added a project: clang. Herald added a subscriber: cfe-commits. mcgrathr requested review of this revision. This header has long lacked a standard multiple inclusion guard like other headers have, for no

[PATCH] D90275: [clang][IR] Add support for leaf attribute

2020-11-03 Thread Roland McGrath via Phabricator via cfe-commits
mcgrathr added a comment. The GCC documentation specifically gives the example of the standard C function `qsort` as one that does not qualify as `__attribute__((leaf))` because it uses a callback function (that presumably might be from the caller's own TU). AIUI the claim the attribute makes

[PATCH] D90275: [clang][IR] Add support for leaf attribute

2020-10-27 Thread Roland McGrath via Phabricator via cfe-commits
mcgrathr added a comment. There is an RFC going out with this prototype as reference. When there is consensus on the RFC, this will get in shape for landing with complete tests and all. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D90275/new/

[PATCH] D86822: [clang] Enable -fsanitize=thread on Fuchsia.

2020-08-28 Thread Roland McGrath via Phabricator via cfe-commits
mcgrathr accepted this revision. mcgrathr added a comment. I'm not 100% sure we don't need more fiddles in the driver, but we can iterate from here. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D86822/new/ https://reviews.llvm.org/D86822

[PATCH] D85924: [clang][feature] Add cxx_abi_relative_vtable feature

2020-08-14 Thread Roland McGrath via Phabricator via cfe-commits
mcgrathr accepted this revision. mcgrathr added a comment. This revision is now accepted and ready to land. LGTM but I'm not sure who should sign off on new `__has_feature` symbols. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D85924/new/

[PATCH] D85924: [clang] Add __RELATIVE_CXX_ABI_VTABLES predefine macro

2020-08-13 Thread Roland McGrath via Phabricator via cfe-commits
mcgrathr added inline comments. Comment at: clang/lib/Frontend/InitPreprocessor.cpp:1127 + if (LangOpts.RelativeCXXABIVTables) +Builder.defineMacro("__RELATIVE_CXX_ABI_VTABLES"); ldionne wrote: > mcgrathr wrote: > > This should also test

[PATCH] D85924: [clang] Add __RELATIVE_CXX_ABI_VTABLES predefine macro

2020-08-13 Thread Roland McGrath via Phabricator via cfe-commits
mcgrathr added inline comments. Comment at: clang/lib/Frontend/InitPreprocessor.cpp:1127 + if (LangOpts.RelativeCXXABIVTables) +Builder.defineMacro("__RELATIVE_CXX_ABI_VTABLES"); This should also test `LangOpts.CplusPlus` so it's never defined in C

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

2020-08-12 Thread Roland McGrath via Phabricator via cfe-commits
mcgrathr added inline comments. Comment at: clang/include/clang/Basic/TargetCXXABI.h:39 + static const auto () { +static llvm::StringMap ABIMap = { The option UI should use lowercase values by default, or else just do case-insensitive matching.

[PATCH] D85362: [CMake] Print the autodetected host linker version

2020-08-05 Thread Roland McGrath via Phabricator via cfe-commits
mcgrathr accepted this revision. mcgrathr added a comment. This revision is now accepted and ready to land. That's a great start. IMHO we should give the driver a way to print out its embedded value too. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION

[PATCH] D84482: [Fuchsia] Use -z dead-reloc-in-nonalloc=*=0 for Fuchsia targets

2020-07-23 Thread Roland McGrath via Phabricator via cfe-commits
mcgrathr created this revision. mcgrathr added reviewers: phosek, leonardchan. Herald added a project: clang. Herald added a subscriber: cfe-commits. Repository: rG LLVM Github Monorepo https://reviews.llvm.org/D84482 Files: clang/lib/Driver/ToolChains/Fuchsia.cpp Index:

[PATCH] D79835: [Fuchsia] Rely on linker switch rather than dead code ref for profile runtime

2020-05-29 Thread Roland McGrath via Phabricator via cfe-commits
mcgrathr updated this revision to Diff 267336. mcgrathr added a comment. comment update Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D79835/new/ https://reviews.llvm.org/D79835 Files: clang/lib/Driver/ToolChains/Fuchsia.cpp

[PATCH] D79835: [Fuchsia] Rely on linker switch rather than dead code ref for profile runtime

2020-05-12 Thread Roland McGrath via Phabricator via cfe-commits
mcgrathr created this revision. mcgrathr added a reviewer: phosek. Herald added subscribers: llvm-commits, cfe-commits, hiraditya. Herald added projects: clang, LLVM. Follow the model used on Linux, where the clang driver passes the linker a `-u` switch to force the profile runtime to be linked

[PATCH] D79667: [Clang] Pass -z max-page-size to linker for Fuchsia

2020-05-09 Thread Roland McGrath via Phabricator via cfe-commits
mcgrathr created this revision. mcgrathr added a reviewer: phosek. Herald added a project: clang. Herald added a subscriber: cfe-commits. mcgrathr added a parent revision: D79665: [Clang] Pass --pack-dyn-relocs=relr to lld for Fuchsia. Currently all Fuchsia ABIs use a 4k page size, departing

[PATCH] D79665: [Clang] Pass --pack-dyn-relocs=relr to lld for Fuchsia

2020-05-09 Thread Roland McGrath via Phabricator via cfe-commits
mcgrathr created this revision. mcgrathr added a reviewer: phosek. Herald added a project: clang. Herald added a subscriber: cfe-commits. The compact format is fully supported on Fuchsia and is the preferred default. Repository: rG LLVM Github Monorepo https://reviews.llvm.org/D79665 Files:

[PATCH] D78687: [Fuchsia] Build compiler-rt builtins for 32-bit x86

2020-04-22 Thread Roland McGrath via Phabricator via cfe-commits
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/D78687/new/ https://reviews.llvm.org/D78687

[PATCH] D72959: Relative VTables ABI on Fuchsia

2020-04-02 Thread Roland McGrath via Phabricator via cfe-commits
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

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

2020-02-21 Thread Roland McGrath via Phabricator via cfe-commits
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

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

2020-02-21 Thread Roland McGrath via Phabricator via cfe-commits
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

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

2020-02-21 Thread Roland McGrath via Phabricator via cfe-commits
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

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

2020-02-21 Thread Roland McGrath via Phabricator via cfe-commits
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

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

2020-02-21 Thread Roland McGrath via Phabricator via cfe-commits
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

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

2020-01-30 Thread Roland McGrath via Phabricator via cfe-commits
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:

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

2020-01-30 Thread Roland McGrath via Phabricator via cfe-commits
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:

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

2020-01-30 Thread Roland McGrath via Phabricator via cfe-commits
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`

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

2020-01-24 Thread Roland McGrath via Phabricator via cfe-commits
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:

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

2020-01-24 Thread Roland McGrath via Phabricator via cfe-commits
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

  1   2   >