Page MenuHomePhabricator

mcgrathr (Roland McGrath)
User

Projects

User does not belong to any projects.

User Details

User Since
Jan 31 2017, 4:46 PM (259 w, 17 h)

Recent Activity

Yesterday

mcgrathr added a comment to D116179: [InstrProf][NFC] Do not assume size of counter type.

Please understand that InstrProfData.inc is now a public installed header file in the compiler-rt installation, at <profile/InstrProfData.inc>. So *any* changes to this file have a quite high standard of compatibility concern.

Tue, Jan 18, 5:51 PM · Restricted Project, Restricted Project

Fri, Jan 14

mcgrathr accepted D117359: [scudo] Make Scudo compile for C++20.
Fri, Jan 14, 2:27 PM · Restricted Project

Tue, Jan 11

mcgrathr added inline comments to D116769: [ifs] Allow llvm-ifs to generate text stub from elf stub.
Tue, Jan 11, 5:23 PM · Restricted Project
mcgrathr accepted D117058: [ifs] Added missing DT_STRSZ to the .dynamic section.

lgtm

Tue, Jan 11, 5:20 PM · Restricted Project

Mon, Jan 10

mcgrathr committed rGce167c6fb2ae: [libcxx] Use Fuchsia-native monotonic clock for std::chrono::steady_clock (authored by mcgrathr).
[libcxx] Use Fuchsia-native monotonic clock for std::chrono::steady_clock
Mon, Jan 10, 1:15 PM
mcgrathr closed D116606: [libcxx] Use Fuchsia-native monotonic clock for std::chrono::steady_clock.
Mon, Jan 10, 1:14 PM · Restricted Project
mcgrathr added a comment to D116606: [libcxx] Use Fuchsia-native monotonic clock for std::chrono::steady_clock.

Ping. This is very similar to the other change I landed recently. It needs approval from a libc++ maintainer.
Thanks.

Mon, Jan 10, 9:36 AM · Restricted Project

Fri, Jan 7

mcgrathr added inline comments to D116769: [ifs] Allow llvm-ifs to generate text stub from elf stub.
Fri, Jan 7, 9:19 PM · Restricted Project
mcgrathr added a comment to D116769: [ifs] Allow llvm-ifs to generate text stub from elf stub.

LGTM modulo the fallback behaviors as mentioned below. But I'll leave the approval to those who have done the review for this specific codebase more generally, since I'm not sure on all the points of LLVM style or what library features are available or best to use.

Fri, Jan 7, 4:39 PM · Restricted Project

Thu, Jan 6

mcgrathr added inline comments to D116769: [ifs] Allow llvm-ifs to generate text stub from elf stub.
Thu, Jan 6, 5:57 PM · Restricted Project

Tue, Jan 4

mcgrathr added inline comments to D116606: [libcxx] Use Fuchsia-native monotonic clock for std::chrono::steady_clock.
Tue, Jan 4, 11:07 AM · Restricted Project
mcgrathr requested review of D116606: [libcxx] Use Fuchsia-native monotonic clock for std::chrono::steady_clock.
Tue, Jan 4, 10:26 AM · Restricted Project
mcgrathr committed rG3064dd8ccffc: [libcxx] Use Fuchsia-native CPRNG for std::random_device (authored by mcgrathr).
[libcxx] Use Fuchsia-native CPRNG for std::random_device
Tue, Jan 4, 10:24 AM
mcgrathr closed D116498: [libcxx] Use Fuchsia-native CPRNG for std::random_device.
Tue, Jan 4, 10:24 AM · Restricted Project
mcgrathr added inline comments to D116498: [libcxx] Use Fuchsia-native CPRNG for std::random_device.
Tue, Jan 4, 9:06 AM · Restricted Project

Sun, Jan 2

mcgrathr requested review of D116498: [libcxx] Use Fuchsia-native CPRNG for std::random_device.
Sun, Jan 2, 11:54 AM · Restricted Project

Tue, Dec 28

mcgrathr raised a concern with rG21dbfd430021: [ELF] --gc-sections: Change startwith(".init") (and ".fini") to exact match.

Just the fact that any follow-up changes were required is clear testament to the fact that this was never a change that was appropriate to commit without community review and approval as all LLVM changes require. Regardless of the technical details, nontrivial changes, especially ones affecting the user-visible semantics, simply should not be committed without proper review. Resistance to reverting unreviewed changes is simply hostile to the community's established standards and code of conduct and frankly does not merit further technical discussion. The appropriate forum for technical discussion is in review of changes such as these *before* they are committed. The appropriate action right now is to revert all the changes that were not properly reviewed and changed behavior in ways that reviewers have not yet had the opportunity to discuss. Once the damage done has been recovered, then it's appropriate to propose the changes you wanted made and allow proper review and discussion about the semantics being changed.

Tue, Dec 28, 3:24 PM

Dec 3 2021

mcgrathr added a comment to D115024: [ifs] Patch llvm-ifs to allow output multiple types of stub file at the same time.

This feature seems like a good addition. I'm wondering about the interactions with other switches and if it might make sense to rearchitect some of the CLI API here for the more arcane corners. Maybe it's already orthogonal enough, but I'm not sure. I'm thinking in particular about the various --strip-* options. Perhaps it's the case that all such variations apply only to text IFS output? I'm also wondering if there might be use cases for multiple different kinds of IFS output (i.e. different sets of stripping / arch options). Come to think of it, we could even have uses for multiple ELF stub outputs from the same IFS input with different switches (e.g. arch). So perhaps it makes sense to go in a direction where each output file is separately described with both its format (IFS vs ELF) and its format options (stripping, arch, etc), and there can be any number of such output files in whatever combination. I'm not sure exactly what a CLI syntax for that would look like. Another possibility that might fit well for build system integration is to accept a complex output specification in JSON either as a command-line switch value or from a file.

Dec 3 2021, 9:24 AM · Restricted Project

Nov 30 2021

mcgrathr accepted D114023: [Driver] Pass --fix-cortex-a53-843419 automatically on Fuchsia.
Nov 30 2021, 10:52 AM · Restricted Project
mcgrathr accepted D114115: [Driver] Support for compressed debug info on Fuchsia.

lgtm, but we should be sure to advise users that their debugging tools may need build adjustments to ensure they support the compressed formats.

Nov 30 2021, 10:47 AM · Restricted Project
mcgrathr accepted D44605: [Driver] Default to DWARF 5 for Fuchsia.

lgtm, but we should be sure to advise users who may need to use -gdwarf-4 explicitly if their debugging tools become unhappy.

Nov 30 2021, 10:45 AM
mcgrathr updated the summary of D114383: [InstrProfiling] Add -runtime-counter-relocation=function mode.
Nov 30 2021, 10:43 AM · Restricted Project, Restricted Project
mcgrathr updated the diff for D114383: [InstrProfiling] Add -runtime-counter-relocation=function mode.

renamed callback

Nov 30 2021, 10:43 AM · Restricted Project, Restricted Project
mcgrathr added a comment to D114383: [InstrProfiling] Add -runtime-counter-relocation=function mode.

An alternative I thought of would be to always use a call and generate a (weak) implementation of __llvm_profile_counter_dynamic_bias inside each translation unit. The default generated function would simply return the variable. I'm not sure what the impact on performance would be though.

Nov 30 2021, 10:43 AM · Restricted Project, Restricted Project

Nov 29 2021

mcgrathr added inline comments to D40319: [libcxx] Support getentropy as a source of randomness for std::random_device.
Nov 29 2021, 11:14 AM

Nov 22 2021

mcgrathr updated the diff for D114383: [InstrProfiling] Add -runtime-counter-relocation=function mode.

second copy of InstrProfData.inc

Nov 22 2021, 6:07 PM · Restricted Project, Restricted Project
mcgrathr updated the diff for D114383: [InstrProfiling] Add -runtime-counter-relocation=function mode.

Use a DenseMap rather than instruction name to match bias load.

Nov 22 2021, 1:09 PM · Restricted Project, Restricted Project
mcgrathr updated the summary of D114383: [InstrProfiling] Add -runtime-counter-relocation=function mode.
Nov 22 2021, 11:13 AM · Restricted Project, Restricted Project
mcgrathr requested review of D114383: [InstrProfiling] Add -runtime-counter-relocation=function mode.
Nov 22 2021, 10:59 AM · Restricted Project, Restricted Project

Nov 21 2021

mcgrathr committed rGb72b56016a6b: NFC: clang-format lib/Transforms/Instrumentation/InstrProfiling.cpp (authored by mcgrathr).
NFC: clang-format lib/Transforms/Instrumentation/InstrProfiling.cpp
Nov 21 2021, 6:16 PM
mcgrathr closed D114343: NFC: clang-format lib/Transforms/Instrumentation/InstrProfiling.cpp.
Nov 21 2021, 6:16 PM · Restricted Project
mcgrathr requested review of D114343: NFC: clang-format lib/Transforms/Instrumentation/InstrProfiling.cpp.
Nov 21 2021, 6:15 PM · Restricted Project

Nov 16 2021

mcgrathr added inline comments to D114023: [Driver] Pass --fix-cortex-a53-843419 automatically on Fuchsia.
Nov 16 2021, 1:19 PM · Restricted Project

Nov 10 2021

mcgrathr committed rGff11f0aa5de1: [Clang] Pass -z rel to linker for Fuchsia (authored by mcgrathr).
[Clang] Pass -z rel to linker for Fuchsia
Nov 10 2021, 1:31 PM
mcgrathr closed D113136: [Clang] Pass -z rel to linker for Fuchsia.
Nov 10 2021, 1:31 PM · Restricted Project
mcgrathr added a comment to D113136: [Clang] Pass -z rel to linker for Fuchsia.

Reordered the switches.

Nov 10 2021, 1:02 PM · Restricted Project
mcgrathr updated the diff for D113136: [Clang] Pass -z rel to linker for Fuchsia.

reordered switches

Nov 10 2021, 1:02 PM · Restricted Project
mcgrathr updated the diff for D113136: [Clang] Pass -z rel to linker for Fuchsia.

rebased

Nov 10 2021, 11:25 AM · Restricted Project

Nov 3 2021

mcgrathr requested review of D113136: [Clang] Pass -z rel to linker for Fuchsia.
Nov 3 2021, 12:10 PM · Restricted Project

Sep 23 2021

mcgrathr added inline comments to D110365: [llvm][profile] Add padding after binary IDs.
Sep 23 2021, 6:37 PM · Restricted Project, Restricted Project
mcgrathr added inline comments to D110365: [llvm][profile] Add padding after binary IDs.
Sep 23 2021, 3:23 PM · Restricted Project, Restricted Project
mcgrathr committed rG80b92db02c5a: [profile][fuchsia] Don't include extra NUL in log messages (authored by mcgrathr).
[profile][fuchsia] Don't include extra NUL in log messages
Sep 23 2021, 3:17 PM
mcgrathr closed D110361: [profile][fuchsia] Don't include extra NUL in log messages.
Sep 23 2021, 3:16 PM · Restricted Project
mcgrathr requested review of D110361: [profile][fuchsia] Don't include extra NUL in log messages.
Sep 23 2021, 1:37 PM · Restricted Project

Aug 27 2021

mcgrathr committed rG225eb8a22d41: [libc][NFC] Fix onre more -Wconversion warning in strtoul test code. (authored by mcgrathr).
[libc][NFC] Fix onre more -Wconversion warning in strtoul test code.
Aug 27 2021, 2:13 PM
mcgrathr closed D108845: [libc][NFC] Fix onre more -Wconversion warning in strtoul test code..
Aug 27 2021, 2:13 PM · Restricted Project
mcgrathr requested review of D108845: [libc][NFC] Fix onre more -Wconversion warning in strtoul test code..
Aug 27 2021, 2:12 PM · Restricted Project
mcgrathr committed rG4e1a164d7bd5: [libc] Fix various -Wconversion warnings in strto*l test code. (authored by mcgrathr).
[libc] Fix various -Wconversion warnings in strto*l test code.
Aug 27 2021, 2:04 PM
mcgrathr closed D108800: [libc] Fix various -Wconversion warnings in strto*l test code..
Aug 27 2021, 2:04 PM · Restricted Project
mcgrathr updated the diff for D108800: [libc] Fix various -Wconversion warnings in strto*l test code..

add more casts

Aug 27 2021, 11:51 AM · Restricted Project

Aug 26 2021

mcgrathr requested review of D108800: [libc] Fix various -Wconversion warnings in strto*l test code..
Aug 26 2021, 7:06 PM · Restricted Project
mcgrathr added a comment to D108747: [llvm][ProfileData] Add a warning to indicate potentially corrupted data.

The intent is to sanity-check for garbage header fields. The uncompressed size field is one that you can only warn about heuristically like this I guess. But you can do a definitive test on the CompressedSize field to ensure that it isn't larger than the actual size of the data available. There's no need to even worry about whether UncompressedSize is valid if CompressedSize is clearly invalid.

Aug 26 2021, 9:48 AM · Restricted Project

Aug 25 2021

mcgrathr accepted D107201: [libfuzzer] Use crash stack for fuchsia.
Aug 25 2021, 3:03 PM · Restricted Project
mcgrathr accepted D107023: [libfuzzer] Handle more than one exception in Fuchsia..

lgtm

Aug 25 2021, 2:57 PM · Restricted Project

Aug 23 2021

mcgrathr added inline comments to D108428: [ifs] Add option to hide undefined symbols.
Aug 23 2021, 1:38 PM · Restricted Project

Aug 12 2021

mcgrathr accepted D108000: [IFS] Fix copy constructor warnings in IFSStub.cpp.
Aug 12 2021, 6:45 PM · Restricted Project

Aug 6 2021

mcgrathr committed rG5a2a17969583: [profile][Fuchsia] Add missing system header #include (authored by mcgrathr).
[profile][Fuchsia] Add missing system header #include
Aug 6 2021, 6:00 PM
mcgrathr closed D107616: [profile][Fuchsia] Add missing system header #include.
Aug 6 2021, 5:59 PM · Restricted Project

Aug 5 2021

mcgrathr requested review of D107616: [profile][Fuchsia] Add missing system header #include.
Aug 5 2021, 7:53 PM · Restricted Project

Jul 27 2021

mcgrathr accepted D106725: [libFuzzer] Fix CFI Directives for fuchsia.

lgtm

Jul 27 2021, 4:38 PM · Restricted Project
mcgrathr accepted D106835: [asan][fuchsia] Implement PlatformUnpoisonStacks.

lgtm

Jul 27 2021, 4:34 PM · Restricted Project
mcgrathr accepted D105735: [compiler-rt][hwasan][Fuchsia] Do not emit FindDynamicShadowStart for Fuchsia.

lgtm

Jul 27 2021, 1:15 PM · Restricted Project

Jul 23 2021

mcgrathr accepted D106690: [GWP-ASan] Add version header..

lgtm

Jul 23 2021, 1:37 PM · Restricted Project

Jul 20 2021

mcgrathr accepted D106086: [libc] Add a new test matcher for tests raising floating point exceptions..

lgtm

Jul 20 2021, 10:23 AM · Restricted Project

Jul 19 2021

mcgrathr added a comment to D106086: [libc] Add a new test matcher for tests raising floating point exceptions..

For Fuchsia you should just define the new macro to continue to use zxtest's ASSERT_DEATH macro.

Jul 19 2021, 2:55 PM · Restricted Project
mcgrathr accepted D105745: [compiler-rt][hwasan][fuchsia] Define shadow bound globals.

lgtm modulo typo

Jul 19 2021, 1:31 PM · Restricted Project
mcgrathr added a comment to D105735: [compiler-rt][hwasan][Fuchsia] Do not emit FindDynamicShadowStart for Fuchsia.

I think it would be more in keeping with local style to make this a third #elif SANITIZER_FUCHSIA leg in the middle with the InitShadowGOT stub. FindDynamicShadowStart is only called from Linux-specific code and does not need to be defined.

Jul 19 2021, 1:28 PM · Restricted Project
mcgrathr accepted D105668: [compiler-rt][hwasan][fuchsia] Implement InitializeOsSupport.

lgtm with fxbug.dev/NNN url ref added.

Jul 19 2021, 1:25 PM · Restricted Project
mcgrathr accepted D105664: [compiler-rt][hwasan][fuchsia] Implement TagMemoryAligned for fuchsia.

lgtm

Jul 19 2021, 1:23 PM · Restricted Project

Jul 15 2021

mcgrathr committed rG8f053eadbe27: [libc] Fix typos in x86_64/FEnv.h (authored by mcgrathr).
[libc] Fix typos in x86_64/FEnv.h
Jul 15 2021, 3:21 PM
mcgrathr closed D106105: [libc] Fix typos in x86_64/FEnv.h.
Jul 15 2021, 3:21 PM · Restricted Project
mcgrathr requested review of D106105: [libc] Fix typos in x86_64/FEnv.h.
Jul 15 2021, 3:08 PM · Restricted Project

Jul 7 2021

mcgrathr added inline comments to D88248: [lsan] On Fuchsia, don't use atexit hook for leak checks.
Jul 7 2021, 1:44 PM · Restricted Project

Jul 5 2021

mcgrathr accepted D104085: [compiler-rt][hwasan] Setup hwasan thread handling on Fuchsia.

lgtm

Jul 5 2021, 2:14 PM · Restricted Project
mcgrathr added a comment to D105176: [InstrProfiling] Use external weak reference for bias variable.

MaskRay's code generation example shows the optimal code possible for implementing this feature and how that code is generated is unaffected by this change, so I don't understand why that subject is being raised here.

Jul 5 2021, 2:08 PM · Restricted Project, Restricted Project
mcgrathr accepted D103936: [compiler-rt][hwasan] Define fuchsia implementations of required hwasan functions.

lgtm with minor changes + clang-format.

Jul 5 2021, 1:48 PM · Restricted Project
mcgrathr accepted D103544: [compiler-rt][Fuchsia] Disable interceptors while enabling new/delete replacements.

lgtm

Jul 5 2021, 1:40 PM · Restricted Project
mcgrathr added a comment to D99381: [compiler-rt][hwasan] Remove __sanitizer allocation functions from hwasan interface.

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 details of the allocator interceptors.
I think a better cleanup is to move those declarations out of that file. I don't see why they need to be in any header file rather than just in hwasan_allocation_functions.cpp.

Jul 5 2021, 1:40 PM · Restricted Project
mcgrathr added a comment to D96450: [libFuzzer] Use crash stack for fuchsia..

The stated goal is very sensible. But this change is doing too many things at once to get there. Please separate the refactorings into smaller changes that come with some discussion of why that refactoring is needed.

Jul 5 2021, 1:34 PM · Restricted Project
mcgrathr updated the diff for D88248: [lsan] On Fuchsia, don't use atexit hook for leak checks.

rebased

Jul 5 2021, 12:29 PM · Restricted Project
mcgrathr accepted D104839: [profile] Decommit memory after counter relocation.

Factoring out the arithmetic from the OS-specific code and landing only the Fuchsia version of the OS-specific implementation LGTM.

Jul 5 2021, 12:24 PM · Restricted Project

Jul 1 2021

mcgrathr accepted D105176: [InstrProfiling] Use external weak reference for bias variable.

lgtm modulo some comment cleanups

Jul 1 2021, 3:01 PM · Restricted Project, Restricted Project

Jun 30 2021

mcgrathr added a comment to D105176: [InstrProfiling] Use external weak reference for bias variable.

It makes sense that the contract be that the runtime must define the variable.

Jun 30 2021, 11:13 AM · Restricted Project, Restricted Project

Jun 22 2021

mcgrathr accepted D104611: [compiler-rt][CMake] Drop flags that are set by default for Fuchsia.

lgtm

Jun 22 2021, 11:39 AM · Restricted Project
mcgrathr accepted D104728: [compiler-rt] Make use of undefined symbols configurable.

Since those only affects sanitizers and not other compiler-rt libraries, perhaps it should be called SANITIZER_* instead of COMPILER_RT_*?

Jun 22 2021, 11:29 AM · Restricted Project

Jun 16 2021

mcgrathr added inline comments to D104275: [compiler-rt][hwasan] Add GetShadowOffset function.
Jun 16 2021, 1:07 AM · Restricted Project

Jun 15 2021

mcgrathr added inline comments to D104248: [compiler-rt][hwasan] Refactor Thread::Init.
Jun 15 2021, 3:57 PM · Restricted Project
mcgrathr added inline comments to D104275: [compiler-rt][hwasan] Add GetShadowOffset function.
Jun 15 2021, 3:50 PM · Restricted Project

Jun 14 2021

mcgrathr added inline comments to D104248: [compiler-rt][hwasan] Refactor Thread::Init.
Jun 14 2021, 11:28 AM · Restricted Project

Jun 11 2021

mcgrathr added inline comments to D103936: [compiler-rt][hwasan] Define fuchsia implementations of required hwasan functions.
Jun 11 2021, 4:07 PM · Restricted Project
mcgrathr added a comment to D104085: [compiler-rt][hwasan] Setup hwasan thread handling on Fuchsia.

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 tightened to just the pure refactoring, and we should have a separate review thread for the Fuchsia parts.

Jun 11 2021, 3:58 PM · Restricted Project

Jun 8 2021

mcgrathr added inline comments to D103936: [compiler-rt][hwasan] Define fuchsia implementations of required hwasan functions.
Jun 8 2021, 6:11 PM · Restricted Project

Jun 7 2021

mcgrathr added inline comments to D103564: [NFC][compiler-rt][hwasan] Move allocation functions into their own file.
Jun 7 2021, 10:15 PM · Restricted Project
mcgrathr added a comment to D103841: [llvm][hwasan] Decouple use of the TLS global for getting the shadow base and using the frame record feature .

lgtm

Jun 7 2021, 10:13 PM · Restricted Project, Restricted Project

Jun 4 2021

mcgrathr added a comment to D103303: [ELF] Add OVERWRITE_SECTIONS command.

Thanks for the updates.

One idea that might work better for the linker script fragment is to use a new keyword. For example instead of using --overwrite-section-script and SECTIONS. Use a new keyword like OVERWRITE_SECTIONS this could essentially be mixed in with any other linker script without needing --overwrite-section-script.

Jun 4 2021, 11:35 AM · Restricted Project

Jun 3 2021

mcgrathr added a comment to D103564: [NFC][compiler-rt][hwasan] Move allocation functions into their own file.

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:

Jun 3 2021, 6:21 PM · Restricted Project
mcgrathr added a comment to D103303: [ELF] Add OVERWRITE_SECTIONS command.

This would add some complexity to readLinkerScript as we will need to check the first token. Do we really need it?
This is just a matter between $file and -Wl,--overwrite-script=$file in the build system.

Jun 3 2021, 5:39 PM · Restricted Project

Jun 2 2021

mcgrathr added a comment to D103564: [NFC][compiler-rt][hwasan] Move allocation functions into their own file.

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 just be under #if !SANITIZER_FUCHSIA.

Jun 2 2021, 6:39 PM · Restricted Project
mcgrathr accepted D103555: [Fuchsia] Use libc++abi on Windows in Fuchsia toolchain.

lgtm

Jun 2 2021, 2:28 PM · Restricted Project
mcgrathr accepted D103543: [compiler-rt][Fuchsia] Support HWASan on Fuchsia.

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 of the runtime we needed in preparation for Fuchsia-specific changes before landing anything Fuchsia-specific to make it compile.

Jun 2 2021, 12:07 PM · Restricted Project