mcgrathr (Roland McGrath)
User

Projects

User does not belong to any projects.

User Details

User Since
Jan 31 2017, 4:46 PM (63 w, 6 d)

Recent Activity

Thu, Apr 19

mcgrathr accepted D45852: [Fuzzer] Add a missing header in Fuchsia implementation.

We should revisit the implicit dependency on fdio. For other compiler-rt libraries' Fuchsia ports, we've kept it at the pure vDSO and minimal libc ABIs.

Thu, Apr 19, 5:36 PM

Wed, Apr 11

mcgrathr accepted D45525: [sanitizer] Correct name length computation for some Fuchsia vmos.

lgtm

Wed, Apr 11, 11:45 AM

Fri, Mar 30

mcgrathr accepted D45105: [AArch64] Reserve x18 register on Fuchsia.

lgtm

Fri, Mar 30, 1:59 PM

Tue, Mar 27

mcgrathr added a comment to D44953: [sanitizer] Remove empty Symbolizer PrepareForSandboxing.

lgtm but I'm not authoritative.

Tue, Mar 27, 1:43 PM
mcgrathr accepted D44947: [Driver] Add fuzzer-no-link into the list of supported Fuchsia sanitizers.

lgtm

Tue, Mar 27, 1:35 PM

Mar 22 2018

mcgrathr accepted D44770: [sanitizer] zx_vmo_write on Fuchsia takes only 4 arguments now.

Lgtm

Mar 22 2018, 8:37 AM

Mar 20 2018

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

Lgtm with some more comments

Mar 20 2018, 11:09 PM
mcgrathr added a comment to D44724: [Fuchsia] Don't install libc++, libc++abi or libunwind on Darwin.

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 match.
Do these settings accomplish that?

Mar 20 2018, 7:32 PM

Mar 18 2018

mcgrathr added a comment to D44605: [Driver] Default to DWARF 5 for Fuchsia.

We aren't actually using DWARF 5 yet AFAICT. Our builds don't pass -gdwarf-5. So I'm not sure we have yet verified that all the DWARF-consuming tools people are using with Fuchsia binaries can handle all of DWARF 5 (which has several major format changes). I'd certainly like 5 to be the default, but I think we need to establish a set of consumers we care about and verify their format version support before we can be sure about this.

Mar 18 2018, 2:35 AM

Mar 14 2018

mcgrathr added a comment to D44349: [WebAssembly] Verify contents of relocations target before writing it.

So Sam and I had dinner together. :-) I don't know a lot about WebAssembly and Sam doesn't know a lot about traditional (e.g. ELF) linkers, so we talked about the parallels in somewhat general terms while explaining the specifics to each other without distracting from our dessert.
In talking about what WebAssembly wants to do with converting some kinds of references to other kinds, I described the obvious analogy to the various kinds of linker relaxation that have been done in traditional machine code linking such as ELF.
There are two precedents that I can cite in detail off hand. In both cases the reloc type is specified in the ABI as for use with certain instruction patterns and the linker is expected to rewrite instructions with complete knowledge of their meaning, rather than just deliver values into bitfields.
One is x86-64's GOTPCREL and GOTPCRELX relocs, where GNU linkers check for expected opcode bytes immediate prior to the r_offset location and silently forego the instruction-rewriting optimization if the reloc is not used in the expected context.
The other is x86-64's TLS relocs, where GNU linkers check the opcode bytes immediatley prior to the r_offset location and diagnose an error if they don't match specific instruction patterns prescribed in the ABI for each reloc that enables linker relaxation (IIRC for some cases the x86 ABI mandates that linkers perform the relaxation, rather than just describing how the optimization is possible).
I don't know off hand how LLD handles these cases, but I would say that matching the GNU linkers' behavior is right--with the possible caveat that perhaps there should be a warning emitting for using GOTPCRELX relocs with instructions the linker doesn't know how to rewrite.
I know that other machines have many more relocs that are prescribed in their ABIs as for use with specific instruction patterns, but I don't know off hand what the details of those are, nor how existing linkers handle situations where such relocs are used with mismatching instructions.

Mar 14 2018, 2:59 PM

Mar 5 2018

mcgrathr accepted D44126: [sanitizer] Fix the return type for GetTid in Fuchsia implementation.

lgtm

Mar 5 2018, 4:44 PM

Mar 1 2018

mcgrathr accepted D43992: [Frontend] Avoid including default system header paths on Fuchsia.

lgtm

Mar 1 2018, 10:31 PM

Jan 12 2018

mcgrathr added a comment to D42022: Reland "Make TracePcGuardController linker-initialized".

I wonder if cpplint.py is worthwhile any more. Perhaps it should be replaced by a check that clang-format doesn't decide to change anything.

Jan 12 2018, 5:13 PM · Restricted Project
mcgrathr added a comment to D42022: Reland "Make TracePcGuardController linker-initialized".

clang-format and lib/sanitizer_common/scripts/cpplint.py disagree about whitespace before braces.

Jan 12 2018, 5:09 PM · Restricted Project
mcgrathr added a project to D42022: Reland "Make TracePcGuardController linker-initialized": Restricted Project.
Jan 12 2018, 5:09 PM · Restricted Project
mcgrathr added inline comments to D42019: [Driver] Set default sysroot for Fuchsia if none is specified.
Jan 12 2018, 4:58 PM
mcgrathr added a comment to D39348: Implement --just-symbols..

ping. Will this or D41277 land?

Jan 12 2018, 2:51 PM

Dec 21 2017

mcgrathr created D41513: [SanitizerCoverage][Fuchsia] Make TracePcGuardController linker-initialized.
Dec 21 2017, 1:54 PM · Restricted Project

Dec 20 2017

mcgrathr created D41475: [sanitizer] Make function declarations C-compatible.
Dec 20 2017, 6:31 PM · Restricted Project
mcgrathr created D41471: [CMake][Fuchsia] Enable assertions.
Dec 20 2017, 4:47 PM
mcgrathr added a comment to D41277: Implement --just-symbols.

This works fine for me. Perhaps Rui should land D39348 first and then you can rebase this in terms of that to discuss the remaining features (-R alias, no input files case).

Dec 20 2017, 4:38 PM

Dec 19 2017

mcgrathr accepted D41413: [scudo] Add -fsanitize=scudo option to Fuchsia..

Mutual exclusion is not target-specific and is already handled in SanitizerArgs. The test coverage for that looks fine.

Dec 19 2017, 5:12 PM

Dec 18 2017

mcgrathr accepted D39348: Implement --just-symbols..

Works for me. D41277 works just as well. I don't know how you plan to reconcile the two.

Dec 18 2017, 3:42 PM

Dec 1 2017

mcgrathr added a comment to D36493: [MC] Handle unknown literal register numbers in .cfi_* directives.

Great! Please land it for me.

Dec 1 2017, 1:21 PM

Nov 30 2017

mcgrathr added a comment to D40652: [ELF] - Produce relocation section name consistent with output section name when --emit-reloc used with linker script..

This change as is fixes my real-world case.
Thanks! Now I'm just blocked on --just-symbols...

Nov 30 2017, 3:18 PM
mcgrathr added a comment to D40657: [sanitizer] Introduce a vDSO aware time function, and use it in the allocator.

Fuchsia bit LGTM. Linux bit highly suspect.

Nov 30 2017, 11:47 AM

Nov 27 2017

mcgrathr accepted D40329: [CMake][Fuchsia] Disable terminfo database in Fuchsia toolchain.

lgtm

Nov 27 2017, 11:36 AM

Nov 21 2017

mcgrathr accepted D40319: [libcxx] Support getentropy as a source of randomness for std::random_device.

LGTM

Nov 21 2017, 1:34 PM
mcgrathr accepted D39831: [Driver] Make the use of relax relocations a per target option.

LGTM

Nov 21 2017, 12:57 PM
mcgrathr added a comment to D39831: [Driver] Make the use of relax relocations a per target option.

test case?

Nov 21 2017, 12:02 PM

Nov 20 2017

mcgrathr added inline comments to D38595: [fuchsia] Update Fuchsia with a new mmap implementation..
Nov 20 2017, 1:13 PM

Nov 17 2017

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

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.

Nov 17 2017, 11:31 PM
mcgrathr accepted D40201: [llvm-objcopy] Add --strip-all-gnu and change --strip-all.

LGTM. I wonder if it's really worth keeping --strip-non-alloc.

Nov 17 2017, 4:01 PM

Nov 10 2017

mcgrathr added a comment to D39930: [CMake] Use libc++ and compiler-rt as default libraries in Fuchsia toolchain.

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.
I don't think we want that. Perhaps we could make -static-libstdc++ default in our host targets. But that's not really enough, since compiler-rt has some too. Probably better to just disable building the shared libraries altogether for the host targets.

Nov 10 2017, 5:19 PM

Oct 26 2017

mcgrathr added inline comments to D39348: Implement --just-symbols..
Oct 26 2017, 5:36 PM
mcgrathr added a comment to D39348: Implement --just-symbols..

This doesn't implement the -R switch name, which is the traditional interface from time immemorial. I can use --just-symbols, but you really should handle -R too.

Oct 26 2017, 5:23 PM

Oct 24 2017

mcgrathr accepted D39273: [CMake] Build host builtins in Fuchsia toolchain even on Darwin.

lgtm

Oct 24 2017, 6:22 PM · Restricted Project
mcgrathr accepted D39270: [CMake] Include clang-refactor in Fuchsia toolchain.

lgtm

Oct 24 2017, 6:13 PM · Restricted Project

Oct 23 2017

mcgrathr accepted D39176: [Driver] Use ld.lld directly for Fuchsia rather than passing flavor.

This seems like how it always should have been, since -fuse-ld=lld sets it to ld.lld, not lld.
Looks to me like the existing MipsLinux and WebAssembly implementations are wrong too and only BareMetal is right.

Oct 23 2017, 1:13 PM · Restricted Project

Oct 21 2017

mcgrathr added a comment to D38724: [ELF] - Do not collect SHT_REL[A] sections unconditionally when --gc-sections and --emit-relocs used together..

This change works on my case, but it names the reloc sections it emits based on the first input section in the output section being relocated rather than on the output section itself.
e.g. in my -ffunction-sections --gc-sections --emit-relocs build, the SHT_RELA section with sh_info pointing to the .text section is called .rela.text.first_function_in_text rather than .rela.text.

Oct 21 2017, 3:06 AM

Oct 17 2017

mcgrathr accepted D39017: [CMake] Build Fuchsia toolchain as -O3 .

LGTM
Since we use lld, -Wl,-O2 also helps make the binaries smaller.

Oct 17 2017, 12:41 PM

Oct 15 2017

mcgrathr added a comment to D38907: Give .note.gnu.build-id section alignment 4.

http://www.sco.com/developers/gabi/latest/ch5.pheader.html#note_section describes alignment of the contents of notes. Maintaining that alignment is obviously impossible if the section is not so aligned.

Oct 15 2017, 5:43 PM · lld

Oct 13 2017

mcgrathr created D38907: Give .note.gnu.build-id section alignment 4.
Oct 13 2017, 4:06 PM · lld

Oct 11 2017

mcgrathr added inline comments to D38759: [fuchsia] Add loop to _zx_cprng_draw..
Oct 11 2017, 11:48 AM

Oct 10 2017

mcgrathr accepted D38759: [fuchsia] Add loop to _zx_cprng_draw..

LGTM

Oct 10 2017, 1:46 PM

Oct 9 2017

mcgrathr added inline comments to D38437: Introduce ReservedAddressRange to sanitizer_common..
Oct 9 2017, 2:46 PM

Oct 8 2017

mcgrathr added a comment to D38669: [sanitizer] Don't intercept signal and sigaction on Fuchsia.

I wonder why my builds have never had an undefined symbol HandleDeadlySignal.

Oct 8 2017, 1:13 AM

Oct 7 2017

mcgrathr requested changes to D38669: [sanitizer] Don't intercept signal and sigaction on Fuchsia.

What effect does this have?
Nothing built for Fuchsia ought to be using sanitizer_signal_interceptors.inc or HandleDeadlySignal in the first place.

Oct 7 2017, 8:24 PM

Oct 5 2017

mcgrathr added inline comments to D38437: Introduce ReservedAddressRange to sanitizer_common..
Oct 5 2017, 3:27 PM
mcgrathr added inline comments to D38437: Introduce ReservedAddressRange to sanitizer_common..
Oct 5 2017, 3:26 PM
mcgrathr added inline comments to D38595: [fuchsia] Update Fuchsia with a new mmap implementation..
Oct 5 2017, 3:25 PM
mcgrathr added a comment to D38592: Update sanitizer_allocator to use new API..

Mostly LGTM

Oct 5 2017, 3:19 PM

Sep 12 2017

mcgrathr created D37785: [Fuchsia] Set ENABLE_X86_RELAX_RELOCATIONS for Fuchsia builds.
Sep 12 2017, 7:01 PM
mcgrathr accepted D37767: [llvm-objcopy] Add e_machine validity check for reserved section indexes.

LGTM

Sep 12 2017, 4:21 PM
mcgrathr added inline comments to D37767: [llvm-objcopy] Add e_machine validity check for reserved section indexes.
Sep 12 2017, 3:56 PM
mcgrathr created D37770: [Fuchsia] Magenta -> Zircon.
Sep 12 2017, 1:54 PM · Restricted Project
mcgrathr created D37763: [Fuchsia] Magenta -> Zircon.
Sep 12 2017, 1:36 PM

Sep 11 2017

mcgrathr created D37723: [Driver] Fuchsia targets default to -fasynchronous-unwind-tables.
Sep 11 2017, 4:56 PM

Sep 2 2017

mcgrathr accepted D37391: [yaml2obj][ELF] Add support for symbol indexes greater than SHN_LORESERVE.

Not being expert in the yaml2obj code base, this looks good to me with the caveats below.

Sep 2 2017, 1:21 AM
mcgrathr accepted D37393: [llvm-objcopy] Add support for special section indexes in symbol table greater than SHN_LORESERVE.

I'm not closely familiar with this code base, so it should probably still get some review from someone who has reviewed the objcopy implementation so far.
But just looking at the apparent logic in this change, it looks OK to me with the caveats noted below.

Sep 2 2017, 1:04 AM

Sep 1 2017

mcgrathr added inline comments to D37391: [yaml2obj][ELF] Add support for symbol indexes greater than SHN_LORESERVE.
Sep 1 2017, 2:05 PM
mcgrathr added inline comments to D37391: [yaml2obj][ELF] Add support for symbol indexes greater than SHN_LORESERVE.
Sep 1 2017, 1:05 PM
mcgrathr requested changes to D37393: [llvm-objcopy] Add support for special section indexes in symbol table greater than SHN_LORESERVE.
Sep 1 2017, 12:57 PM
mcgrathr requested changes to D37391: [yaml2obj][ELF] Add support for symbol indexes greater than SHN_LORESERVE.
Sep 1 2017, 12:39 PM

Aug 29 2017

mcgrathr updated subscribers of D37229: When built with ASan, __cxa_throw calls __asan_handle_no_return.
Aug 29 2017, 1:47 PM · Restricted Project
mcgrathr updated subscribers of D37273: [sanitizer_common][Fuchsia] Update Fuchsia sanitizer markup.
Aug 29 2017, 1:47 PM · Restricted Project
mcgrathr created D37273: [sanitizer_common][Fuchsia] Update Fuchsia sanitizer markup.
Aug 29 2017, 1:46 PM · Restricted Project

Aug 28 2017

mcgrathr added a comment to D37229: When built with ASan, __cxa_throw calls __asan_handle_no_return.

Re-land of https://reviews.llvm.org/D36599
Actually builds now that compiler-rt has been updated.

Aug 28 2017, 1:11 PM · Restricted Project
mcgrathr created D37229: When built with ASan, __cxa_throw calls __asan_handle_no_return.
Aug 28 2017, 1:10 PM · Restricted Project
mcgrathr accepted D37218: Enable GetRandom for Fuchsia sanitizer..

LGTM

Aug 28 2017, 1:04 PM
mcgrathr requested changes to D37218: Enable GetRandom for Fuchsia sanitizer..
Aug 28 2017, 12:16 PM

Aug 27 2017

mcgrathr added a comment to D36811: [asan] Move __asan_handle_no_return to public header.

As now it's going to be apart of API, please add a test

Aug 27 2017, 1:34 AM · Restricted Project
mcgrathr updated the diff for D36811: [asan] Move __asan_handle_no_return to public header.

Added test case.

Aug 27 2017, 1:34 AM · Restricted Project

Aug 16 2017

mcgrathr added a comment to D36812: Add support for writing 64-bit symbol tables for archives when offsets become too large for 32-bit.

You have a bunch of changes here to support individual members >=4G, but I'm not sure there is any need for that.
The bug is just about the whole archive being >=4G so that the member offsets need to be 64 bits.
If you want to support members >=4G I think that can be a separate change since it's not related to the bug.

Aug 16 2017, 5:44 PM
mcgrathr updated the diff for D36811: [asan] Move __asan_handle_no_return to public header.
Aug 16 2017, 5:43 PM · Restricted Project
mcgrathr added a project to D36811: [asan] Move __asan_handle_no_return to public header: Restricted Project.
Aug 16 2017, 5:38 PM · Restricted Project
mcgrathr added a comment to D36811: [asan] Move __asan_handle_no_return to public header.

This is necessary for D36599 to (re-)land.

Aug 16 2017, 5:38 PM · Restricted Project
mcgrathr created D36811: [asan] Move __asan_handle_no_return to public header.
Aug 16 2017, 5:38 PM · Restricted Project
mcgrathr added a project to D36803: Create new VMARs on calls to MmapNoAccess.: Restricted Project.
Aug 16 2017, 3:01 PM · Restricted Project
mcgrathr accepted D36803: Create new VMARs on calls to MmapNoAccess..

LGTM

Aug 16 2017, 3:01 PM · Restricted Project

Aug 15 2017

mcgrathr created D36779: [Driver] SafeStack does not need a runtime library on Fuchsia.
Aug 15 2017, 7:56 PM · Restricted Project

Aug 10 2017

mcgrathr created D36599: When built with ASan, __cxa_throw calls __asan_handle_no_return .
Aug 10 2017, 5:12 PM

Aug 9 2017

mcgrathr added inline comments to D36385: [asan] Refactor thread creation bookkeeping.
Aug 9 2017, 11:31 AM · Restricted Project

Aug 8 2017

mcgrathr updated the diff for D35865: [asan] Complete the Fuchsia port.

Rebased and ready to land once D36385 has landed.
Please land it for me!

Aug 8 2017, 5:07 PM · Restricted Project
mcgrathr added a comment to D36385: [asan] Refactor thread creation bookkeeping.

I think I've done everything requested in review.
If you are both happy with it now, please land it for me!

Aug 8 2017, 5:06 PM · Restricted Project
mcgrathr updated the diff for D36385: [asan] Refactor thread creation bookkeeping.

Options struct for Init.
Remove excess reformatting.
Don't change tctx->detached.

Aug 8 2017, 5:06 PM · Restricted Project
mcgrathr created D36493: [MC] Handle unknown literal register numbers in .cfi_* directives.
Aug 8 2017, 3:55 PM
mcgrathr added inline comments to D36385: [asan] Refactor thread creation bookkeeping.
Aug 8 2017, 11:58 AM · Restricted Project
mcgrathr updated the diff for D35865: [asan] Complete the Fuchsia port.

Rebased

Aug 8 2017, 11:58 AM · Restricted Project
mcgrathr updated the diff for D36385: [asan] Refactor thread creation bookkeeping.

Updated stale comment. Changed FinishThread logic as suggested in review.

Aug 8 2017, 11:58 AM · Restricted Project

Aug 7 2017

mcgrathr added inline comments to D36385: [asan] Refactor thread creation bookkeeping.
Aug 7 2017, 5:10 PM · Restricted Project
mcgrathr added a comment to D36430: [asan] Restore dead-code-elimination optimization for Fuchsia.

Thanks! Please land it for me.

Aug 7 2017, 4:55 PM · Restricted Project
mcgrathr added inline comments to D36430: [asan] Restore dead-code-elimination optimization for Fuchsia.
Aug 7 2017, 3:46 PM · Restricted Project
mcgrathr added a dependent revision for D36385: [asan] Refactor thread creation bookkeeping: D35865: [asan] Complete the Fuchsia port.
Aug 7 2017, 3:25 PM · Restricted Project
mcgrathr added a dependency for D35865: [asan] Complete the Fuchsia port: D36385: [asan] Refactor thread creation bookkeeping.
Aug 7 2017, 3:25 PM · Restricted Project
mcgrathr updated the diff for D35865: [asan] Complete the Fuchsia port.

Rebased

Aug 7 2017, 3:25 PM · Restricted Project
mcgrathr updated the diff for D36385: [asan] Refactor thread creation bookkeeping.

Avoided new argument to FinishThread.

Aug 7 2017, 3:25 PM · Restricted Project
mcgrathr created D36430: [asan] Restore dead-code-elimination optimization for Fuchsia.
Aug 7 2017, 2:43 PM · Restricted Project
mcgrathr added inline comments to D36190: [asan] Allocator support for Fuchsia.
Aug 7 2017, 2:22 PM · Restricted Project

Aug 6 2017

mcgrathr updated the diff for D35865: [asan] Complete the Fuchsia port.

Split out D36385. Reduced number of #if's. Moved Fuchsia-specific implementations into asan_fuchsia.cc.

Aug 6 2017, 5:46 PM · Restricted Project