Page MenuHomePhabricator

pcc (Peter Collingbourne)
User

Projects

User does not belong to any projects.

User Details

User Since
Dec 28 2012, 2:34 PM (441 w, 6 d)

Recent Activity

Today

pcc accepted D79822: [AArch64] Emit CFI instruction for updating x18 when using ShadowCallStack with exception unwinding.

I believe that Android used to use these older versions of libgcc, however Android has now moved to libunwind on aarch64 and although old libgcc versions may still be linked into applications, I do not believe that Android platform code compiled with SCS will run on the same thread as application code, so the old libgcc versions should not be exposed to this unwind info.

Thu, Jun 17, 2:42 PM · Restricted Project
pcc committed rGac35ed5d3487: [compiler-rt][hwasan]: undefine new/delete operators with alignment on Android. (authored by yabinc).
[compiler-rt][hwasan]: undefine new/delete operators with alignment on Android.
Thu, Jun 17, 9:28 AM
pcc closed D104313: [compiler-rt][hwasan]: undefine new/delete operators with alignment on Android..
Thu, Jun 17, 9:28 AM · Restricted Project
pcc accepted D104058: ThinLTO: Fix inline assembly references to static functions with CFI.

LGTM

Thu, Jun 17, 9:16 AM · Restricted Project, Restricted Project

Yesterday

pcc added a comment to D103626: [lldb][AArch64] Remove non address bits from memory read arguments.

The side effect here is that you do "memory read <tagged ptr>" and you see untagged addresses for the lines. It's not really that confusing but maybe something we should make a general decision about.

Same thing applies to the previous "memory region" patch.

(lldb) p ptr1
(char *) $1 = 0x344dfffffffffcb0 ""
(lldb) memory read ptr1
0xfffffffffcb0: f0 05 40 00 00 00 00 00 00 00 00 00 00 00 00 00  ..@.............
0xfffffffffcc0: e0 fc ff ff ff ff 00 00 54 f0 e7 f7 ff ff 00 00  ........T.......
(lldb) memory region ptr1
[0x0000fffffffdf000-0x0001000000000000) rw- [stack]

Personally it doesn't bother me, but then again I know what all the upper bits are doing anyway.

Wed, Jun 16, 4:57 PM · Restricted Project
pcc accepted D104313: [compiler-rt][hwasan]: undefine new/delete operators with alignment on Android..

LGTM

Wed, Jun 16, 10:42 AM · Restricted Project

Mon, Jun 14

pcc accepted D103845: [compiler-rt][hwasan] Add newline between record_addr lines on frame record dumps.

LGTM

Mon, Jun 14, 2:20 PM · Restricted Project
pcc added a comment to D104094: Add missing AArch64 data synchronization barrier (dsb) to __clear_cache.

Do we still need the DSB on line 119?

Mon, Jun 14, 10:20 AM · Restricted Project

Wed, Jun 9

pcc added a comment to D103845: [compiler-rt][hwasan] Add newline between record_addr lines on frame record dumps.

Thanks for tracking this down. It seems best to change the Printf call to add the newline, since otherwise it looks like you'd be adding a spurious newline to the other callers of RenderFrame on Fuchsia.

Wed, Jun 9, 10:45 AM · Restricted Project

Tue, Jun 8

pcc accepted D103120: LTO: Export functions referenced by non-canonical CFI jump tables.

LGTM

Tue, Jun 8, 2:28 PM · Restricted Project
pcc added a comment to D103845: [compiler-rt][hwasan] Add newline between record_addr lines on frame record dumps.

This isn't how the output looks on Android. Are you sure this isn't a Fuchsia-specific bug in the output formatting?

I think the newline gets added to the frame description only if Symbolizer::GetOrInit()->SymbolizePC(pc) is nonnull, so if it fails then no newline is added.

Tue, Jun 8, 12:15 PM · Restricted Project

Mon, Jun 7

pcc requested changes to D103845: [compiler-rt][hwasan] Add newline between record_addr lines on frame record dumps.

This isn't how the output looks on Android. Are you sure this isn't a Fuchsia-specific bug in the output formatting?

Mon, Jun 7, 8:14 PM · Restricted Project

Fri, Jun 4

pcc accepted D103708: [scudo] Remove disableMemoryTagChecksTestOnly.

LGTM

Fri, Jun 4, 11:33 AM · Restricted Project
pcc accepted D103074: [scudo] Add memtag_test.

LGTM

Fri, Jun 4, 11:32 AM · Restricted Project
pcc accepted D103496: [scudo] Untag pointer in iterateOverChunks.

LGTM

Fri, Jun 4, 11:30 AM · Restricted Project

Thu, Jun 3

pcc added inline comments to D103496: [scudo] Untag pointer in iterateOverChunks.
Thu, Jun 3, 8:50 PM · Restricted Project
pcc accepted D103134: [scudo] Always exclude Tag 0.

LGTM

Thu, Jun 3, 8:15 PM · Restricted Project
pcc requested changes to D103074: [scudo] Add memtag_test.
Thu, Jun 3, 8:14 PM · Restricted Project
pcc added inline comments to D103578: [scudo] Add Scudo support for Trusty OS.
Thu, Jun 3, 6:26 PM · Restricted Project

Tue, Jun 1

pcc accepted D103305: [scudo] Enabled MTE in tests.

LGTM

Tue, Jun 1, 5:12 PM · Restricted Project

Wed, May 26

pcc added inline comments to D102912: [libunwind] AARCH64 use inline assembly for pointer authentication.
Wed, May 26, 10:36 AM · Restricted Project, Restricted Project, Restricted Project

Tue, May 25

pcc added a comment to D103134: [scudo] Always exclude Tag 0.

There are probably going to be more callers of selectRandomTag and I don't think we want to have to deal with tag 0 in any of them. Could we just or 1 into the mask in selectRandomTag?

Tue, May 25, 6:53 PM · Restricted Project
pcc added inline comments to D95601: [lldb][AArch64] Add memory tag reading to lldb-server.
Tue, May 25, 5:04 PM · Restricted Project
pcc added inline comments to D95602: [lldb][AArch64] Add MTE memory tag reading to lldb.
Tue, May 25, 5:03 PM · Restricted Project
pcc requested review of D103127: lldb: Don't check for CMAKE_SYSTEM_NAME==Android..
Tue, May 25, 4:57 PM · Restricted Project
pcc added inline comments to D103074: [scudo] Add memtag_test.
Tue, May 25, 10:19 AM · Restricted Project

Mon, May 24

pcc added inline comments to D103042: [NFC][scudo] Add paramenters DCHECKs.
Mon, May 24, 10:35 PM · Restricted Project
pcc added inline comments to D103042: [NFC][scudo] Add paramenters DCHECKs.
Mon, May 24, 8:33 PM · Restricted Project
pcc added inline comments to D102979: [scudo] Use MAP_NORESERVE to release pages.
Mon, May 24, 10:04 AM · Restricted Project
pcc added a comment to D102979: [scudo] Use MAP_NORESERVE to release pages.

TBH, I'm not sure it's worth going to this much effort to work around this QEMU bug. It seems better to figure out a way to use QEMU in system mode instead.

Mon, May 24, 10:00 AM · Restricted Project

Fri, May 21

pcc added a comment to D102943: Hashing: use a 64-bit storage type on all platforms..

Isn't the bug here that module hashing is using hash_code? So shouldn't the correct fix be to use a specific hashing algorithm for module hashes?

Fri, May 21, 5:54 PM · Restricted Project, Restricted Project, Restricted Project
pcc added a comment to D102912: [libunwind] AARCH64 use inline assembly for pointer authentication.

Which version of Clang are you using? I'm reasonably sure that we're building libunwind on Android with Clang, and the pointer authentication code works there.

Fri, May 21, 4:44 PM · Restricted Project, Restricted Project, Restricted Project

Wed, May 19

pcc requested changes to D102472: [HWASAN] Update the tag info for X86_64.
Wed, May 19, 8:59 PM · Restricted Project, Restricted Project
pcc reopened D102472: [HWASAN] Update the tag info for X86_64.

It doesn't look like this was reviewed by anyone. Could you revert this please until it gets reviewed?

Wed, May 19, 8:58 PM · Restricted Project, Restricted Project

May 18 2021

pcc committed rG8e93d10633d7: scudo: Test realloc on increasing size buffers. (authored by pcc).
scudo: Test realloc on increasing size buffers.
May 18 2021, 3:00 PM
pcc closed D102716: scudo: Test realloc on increasing size buffers..
May 18 2021, 2:59 PM · Restricted Project
pcc added a comment to D102716: scudo: Test realloc on increasing size buffers..

It's funny you mention this, I think this is one of the problems that I saw in https://reviews.llvm.org/D102543#change-dE7v8cbuK8l4 (but there are other bugs also in realloc.cpp that need to be addressed as well). Is the fix already merged into the tree or yet to come?

May 18 2021, 2:58 PM · Restricted Project
pcc requested review of D102716: scudo: Test realloc on increasing size buffers..
May 18 2021, 1:02 PM · Restricted Project

May 17 2021

pcc added a comment to D100581: [Clang] -Wunused-but-set-parameter and -Wunused-but-set-variable.

This warning seems to have a lot of false positives on things like reference arguments that are used as output parameters. For example here is a small sample of output from a stage2 build of part of LLVM:

In file included from ../llvm/lib/BinaryFormat/Minidump.cpp:9:
In file included from ../llvm/include/llvm/BinaryFormat/Minidump.h:21:
In file included from ../llvm/include/llvm/ADT/BitmaskEnum.h:16:
../llvm/include/llvm/Support/MathExtras.h:822:9: warning: variable 'Overflowed' set but not used [-Wunused-but-set-variable]
  bool &Overflowed = ResultOverflowed ? *ResultOverflowed : Dummy;
        ^
../llvm/include/llvm/Support/MathExtras.h:936:72: warning: parameter 'Result' set but not used [-Wunused-but-set-parameter]
std::enable_if_t<std::is_signed<T>::value, T> MulOverflow(T X, T Y, T &Result) {
                                                                       ^
In file included from ../llvm/lib/BinaryFormat/Minidump.cpp:9:
In file included from ../llvm/include/llvm/BinaryFormat/Minidump.h:22:
In file included from ../llvm/include/llvm/ADT/DenseMapInfo.h:20:
../llvm/include/llvm/ADT/StringRef.h:511:37: warning: parameter 'Result' set but not used [-Wunused-but-set-parameter]
    getAsInteger(unsigned Radix, T &Result) const {
                                    ^
../llvm/include/llvm/ADT/StringRef.h:522:37: warning: parameter 'Result' set but not used [-Wunused-but-set-parameter]
    getAsInteger(unsigned Radix, T &Result) const {
                                    ^
../llvm/include/llvm/ADT/StringRef.h:545:39: warning: parameter 'Result' set but not used [-Wunused-but-set-parameter]
    consumeInteger(unsigned Radix, T &Result) {
                                      ^
../llvm/include/llvm/ADT/StringRef.h:556:39: warning: parameter 'Result' set but not used [-Wunused-but-set-parameter]
    consumeInteger(unsigned Radix, T &Result) {
                                      ^
6 warnings generated.

Could you please take a look?

May 17 2021, 12:08 PM · Restricted Project, Restricted Project, Restricted Project
pcc committed rGc870e36be1b2: gn build: Only build the hwasan runtime in aliasing mode on x86. (authored by pcc).
gn build: Only build the hwasan runtime in aliasing mode on x86.
May 17 2021, 11:49 AM

May 13 2021

pcc committed rGf79929aceae9: scudo: Fix MTE error reporting for zero-sized allocations. (authored by pcc).
scudo: Fix MTE error reporting for zero-sized allocations.
May 13 2021, 6:14 PM
pcc committed rG9567131d0365: scudo: Check for UAF in ring buffer before OOB in more distant blocks. (authored by pcc).
scudo: Check for UAF in ring buffer before OOB in more distant blocks.
May 13 2021, 6:14 PM
pcc closed D102442: scudo: Fix MTE error reporting for zero-sized allocations..
May 13 2021, 6:14 PM · Restricted Project
pcc closed D102379: scudo: Check for UAF in ring buffer before OOB in more distant blocks..
May 13 2021, 6:14 PM · Restricted Project
pcc added inline comments to D102379: scudo: Check for UAF in ring buffer before OOB in more distant blocks..
May 13 2021, 5:54 PM · Restricted Project
pcc requested review of D102442: scudo: Fix MTE error reporting for zero-sized allocations..
May 13 2021, 2:07 PM · Restricted Project

May 12 2021

pcc committed rG6732a5328cf0: scudo: Require fault address to be in bounds for UAF. (authored by pcc).
scudo: Require fault address to be in bounds for UAF.
May 12 2021, 6:03 PM
pcc closed D102376: scudo: Require fault address to be in bounds for UAF..
May 12 2021, 6:03 PM · Restricted Project
pcc requested review of D102379: scudo: Check for UAF in ring buffer before OOB in more distant blocks..
May 12 2021, 5:08 PM · Restricted Project
pcc updated the diff for D102376: scudo: Require fault address to be in bounds for UAF..
  • Add comments
May 12 2021, 5:03 PM · Restricted Project
pcc added inline comments to D102376: scudo: Require fault address to be in bounds for UAF..
May 12 2021, 4:47 PM · Restricted Project
pcc requested review of D102376: scudo: Require fault address to be in bounds for UAF..
May 12 2021, 3:53 PM · Restricted Project

May 11 2021

pcc accepted D101704: [IR] Introduce the opaque pointer type.

This looks like a reasonable starting point for switching to opaque pointers in LLVM.

May 11 2021, 12:03 AM · Restricted Project

May 4 2021

pcc added a comment to D101874: [scudo] Align objects with alignas.

I'm not sure that we can do this until all of our minimum compiler versions support C++17. E.g. minimum gcc version is 5.1.0 which was released in 2015, so it presumably doesn't have C++17 support.

May 4 2021, 5:09 PM · Restricted Project

Apr 23 2021

pcc committed rGf2819ee6cc46: scudo: Work around gcc 8 conversion warning. (authored by pcc).
scudo: Work around gcc 8 conversion warning.
Apr 23 2021, 11:26 AM
pcc committed rG0a5576ecf05f: scudo: Store header on deallocation before retagging memory. (authored by pcc).
scudo: Store header on deallocation before retagging memory.
Apr 23 2021, 9:32 AM
pcc closed D101137: scudo: Store header on deallocation before retagging memory..
Apr 23 2021, 9:32 AM · Restricted Project

Apr 22 2021

pcc requested review of D101137: scudo: Store header on deallocation before retagging memory..
Apr 22 2021, 11:22 PM · Restricted Project
pcc added a comment to D101031: [scudo] Check if MADV_DONTNEED zeroes memory.

Have you reported this bug to QEMU? Unfortunately the comment you linked to is inaccurate about madvise being a hint on Linux [1].

Apr 22 2021, 5:03 PM · Restricted Project
pcc committed rG484b6648fdd4: scudo: Only static_assert for compressed LSB format with clang. (authored by pcc).
scudo: Only static_assert for compressed LSB format with clang.
Apr 22 2021, 4:12 PM
pcc committed rGa6500b013a25: scudo: Optimize getSizeLSBByClassId() by compressing the table into an integer… (authored by pcc).
scudo: Optimize getSizeLSBByClassId() by compressing the table into an integer…
Apr 22 2021, 3:36 PM
pcc closed D101105: scudo: Optimize getSizeLSBByClassId() by compressing the table into an integer if possible. NFCI..
Apr 22 2021, 3:35 PM · Restricted Project
pcc added a comment to D101105: scudo: Optimize getSizeLSBByClassId() by compressing the table into an integer if possible. NFCI..

Nice.
If this is a significant optimization, we might want to request it through the allocator config and fail if it is impossible - in case we change the size class map in the future and break this by accident.

Apr 22 2021, 3:35 PM · Restricted Project
pcc requested review of D101105: scudo: Optimize getSizeLSBByClassId() by compressing the table into an integer if possible. NFCI..
Apr 22 2021, 1:20 PM · Restricted Project
pcc committed rG4e88e5877c9f: scudo: Use a table to look up the LSB for computing the odd/even mask. NFCI. (authored by pcc).
scudo: Use a table to look up the LSB for computing the odd/even mask. NFCI.
Apr 22 2021, 11:20 AM
pcc closed D101018: scudo: Use a table to look up the LSB for computing the odd/even mask. NFCI..
Apr 22 2021, 11:20 AM · Restricted Project

Apr 21 2021

pcc updated the diff for D101018: scudo: Use a table to look up the LSB for computing the odd/even mask. NFCI..

Fix build

Apr 21 2021, 9:54 PM · Restricted Project
pcc requested review of D101018: scudo: Use a table to look up the LSB for computing the odd/even mask. NFCI..
Apr 21 2021, 9:51 PM · Restricted Project
pcc committed rGe4fa0b307f7f: scudo: Obtain tag from pointer instead of loading it from memory. NFCI. (authored by pcc).
scudo: Obtain tag from pointer instead of loading it from memory. NFCI.
Apr 21 2021, 9:48 PM
pcc closed D101014: scudo: Obtain tag from pointer instead of loading it from memory. NFCI..
Apr 21 2021, 9:48 PM · Restricted Project
pcc requested review of D101014: scudo: Obtain tag from pointer instead of loading it from memory. NFCI..
Apr 21 2021, 7:44 PM · Restricted Project
pcc committed rG3d47e003e922: scudo: Make prepareTaggedChunk() and resizeTaggedChunk() generic. (authored by pcc).
scudo: Make prepareTaggedChunk() and resizeTaggedChunk() generic.
Apr 21 2021, 1:54 PM
pcc committed rG46c59d91dc7a: scudo: Use DC GZVA instruction in storeTags(). (authored by pcc).
scudo: Use DC GZVA instruction in storeTags().
Apr 21 2021, 1:54 PM
pcc closed D100911: scudo: Make prepareTaggedChunk() and resizeTaggedChunk() generic..
Apr 21 2021, 1:54 PM · Restricted Project
pcc closed D100910: scudo: Use DC GZVA instruction in storeTags()..
Apr 21 2021, 1:54 PM · Restricted Project
pcc added a comment to D100994: [MC] Set addrsig symbol as weak/weakExternal.

Wouldn't this make all address significant symbols weak external?

Apr 21 2021, 1:24 PM · Restricted Project
pcc added inline comments to D100911: scudo: Make prepareTaggedChunk() and resizeTaggedChunk() generic..
Apr 21 2021, 11:35 AM · Restricted Project
pcc added inline comments to D100911: scudo: Make prepareTaggedChunk() and resizeTaggedChunk() generic..
Apr 21 2021, 11:12 AM · Restricted Project

Apr 20 2021

pcc added inline comments to D100911: scudo: Make prepareTaggedChunk() and resizeTaggedChunk() generic..
Apr 20 2021, 4:47 PM · Restricted Project
pcc requested review of D100911: scudo: Make prepareTaggedChunk() and resizeTaggedChunk() generic..
Apr 20 2021, 4:27 PM · Restricted Project
pcc requested review of D100910: scudo: Use DC GZVA instruction in storeTags()..
Apr 20 2021, 4:26 PM · Restricted Project

Apr 16 2021

pcc added inline comments to D100515: [lldb] Add GetCodeAddressMask and GetDataAddressMask to Process.
Apr 16 2021, 8:19 AM · Restricted Project
pcc added inline comments to D100515: [lldb] Add GetCodeAddressMask and GetDataAddressMask to Process.
Apr 16 2021, 12:07 AM · Restricted Project

Apr 15 2021

pcc added inline comments to D100515: [lldb] Add GetCodeAddressMask and GetDataAddressMask to Process.
Apr 15 2021, 10:08 AM · Restricted Project

Apr 13 2021

pcc accepted D100413: [scudo] Make MTE inline asm compatible with GNU assembler.

It seems that even with the LLVM assembler the ; character isn't understood everywhere. For example at one point I noticed that ; is treated as a comment character on Mach-O targets, which caused the following instructions to silently disappear. I think we can reasonably count on newline being understood as a delimiter everywhere.

Apr 13 2021, 4:59 PM · Restricted Project

Apr 12 2021

pcc added a comment to D99240: [ConstantFolding] Look through local aliases when simplify GEPs.
In D99240#2684236, @rnk wrote:

I think Arthur is right, it should be safe to look through local aliases to local linkage globals. I think we can actually do this in the "for alias analysis" variant of strip pointer casts, and we should consider that.

Apr 12 2021, 2:56 PM · Restricted Project

Mar 30 2021

pcc added inline comments to D98886: Pass pointer authentication code mask from minidump and use to strip pac from pc..
Mar 30 2021, 3:42 PM · Restricted Project, Restricted Project

Mar 25 2021

pcc added inline comments to D99240: [ConstantFolding] Look through local aliases when simplify GEPs.
Mar 25 2021, 1:25 PM · Restricted Project
pcc added inline comments to D99240: [ConstantFolding] Look through local aliases when simplify GEPs.
Mar 25 2021, 1:02 PM · Restricted Project
pcc added a comment to D99240: [ConstantFolding] Look through local aliases when simplify GEPs.

Ah I see, https://github.com/llvm/llvm-project/blob/61a55c8812e790842799ba1de5bd81fe8afb3b16/llvm/lib/Transforms/IPO/GlobalOpt.cpp#L2953. It only works if the aliasee is a global value. Extending globalopt to handle aliasees that may be a constant expression containing a global value, rather than just a global value, would probably work as well. Either way is fine by me.

globalopt specifically deletes aliases when possible, so it seems fair to do that here as well, even if that may remove entries from the symbol table.

Mar 25 2021, 12:30 PM · Restricted Project
pcc added a comment to D99240: [ConstantFolding] Look through local aliases when simplify GEPs.
In D99240#2649567, @pcc wrote:

Can you make it so that the load ends up being replaced with the loaded value, without replacing the GEP itself? That would seem to address the MSVC issue.

What's the benefit of that over this approach?

Mar 25 2021, 10:53 AM · Restricted Project

Mar 24 2021

pcc requested changes to D99240: [ConstantFolding] Look through local aliases when simplify GEPs.

Can you make it so that the load ends up being replaced with the loaded value, without replacing the GEP itself? That would seem to address the MSVC issue.

Mar 24 2021, 8:28 PM · Restricted Project

Mar 23 2021

pcc committed rGe702fd4f1be0: scudo: Preserve no-memtag attribute on cached secondary allocations. (authored by pcc).
scudo: Preserve no-memtag attribute on cached secondary allocations.
Mar 23 2021, 11:15 AM
pcc closed D99103: scudo: Preserve no-memtag attribute on cached secondary allocations..
Mar 23 2021, 11:15 AM · Restricted Project

Mar 22 2021

pcc requested review of D99103: scudo: Preserve no-memtag attribute on cached secondary allocations..
Mar 22 2021, 12:33 PM · Restricted Project

Mar 19 2021

pcc committed rGeef8b74ef5ef: gn build: Unbreak Android cross-compilation. (authored by pcc).
gn build: Unbreak Android cross-compilation.
Mar 19 2021, 4:29 PM

Mar 17 2021

pcc added a comment to D98529: [lldb] Strip pointer authentication codes from aarch64 pc..

TCR_ELx begins at 1 (see D13.2.123 TCR_EL1, Translation Control Register (EL1) in the armarm) and covers EL0 and 1. Looking at the pseudocode access to this is undefined at EL0.

So the OS would have to provide you some other way to read that, I know this is missing on Linux at the moment. A user setting might be good for us supporting Linux too. (though I realise your immediate concern is MacOS)

Mar 17 2021, 11:42 AM · Restricted Project

Mar 16 2021

pcc committed rGdb36d882ed18: scudo: Allow TBI to be disabled on Linux with a macro. (authored by pcc).
scudo: Allow TBI to be disabled on Linux with a macro.
Mar 16 2021, 12:57 PM
pcc closed D98732: scudo: Allow TBI to be disabled on Linux with a macro..
Mar 16 2021, 12:56 PM · Restricted Project
pcc requested review of D98732: scudo: Allow TBI to be disabled on Linux with a macro..
Mar 16 2021, 12:04 PM · Restricted Project