Page MenuHomePhabricator

pcc (Peter Collingbourne)
User

Projects

User does not belong to any projects.

User Details

User Since
Dec 28 2012, 2:34 PM (466 w, 5 d)

Recent Activity

Thu, Dec 2

pcc requested review of D115015: CodeGen: Strip exception specifications from function types in CFI type names..
Thu, Dec 2, 5:46 PM · Restricted Project

Sat, Nov 20

pcc added a comment to D104830: AST: Create __va_list in the std namespace even in C..

Agreed that this should probably be done in the mangler, I'll try to take a look next week.

Sat, Nov 20, 5:54 PM · Restricted Project

Tue, Nov 16

pcc accepted D114022: [scudo] Fix MTE crash in storeEndMarker..

LGTM

Tue, Nov 16, 12:48 PM · Restricted Project
pcc added inline comments to D114022: [scudo] Fix MTE crash in storeEndMarker..
Tue, Nov 16, 12:35 PM · Restricted Project
pcc added inline comments to D114022: [scudo] Fix MTE crash in storeEndMarker..
Tue, Nov 16, 12:29 PM · Restricted Project

Mon, Nov 15

pcc added inline comments to D113951: [scudo] Handle mallinfo2.
Mon, Nov 15, 3:46 PM · Restricted Project
pcc added a comment to D113951: [scudo] Handle mallinfo2.

Doesn't this require a wrapper for mallinfo2() as well that returns a struct mallinfo2?

Mon, Nov 15, 3:44 PM · Restricted Project
pcc added a comment to D108479: [Clang] Add __builtin_addressof_nocfi.

For CHERI there's the added complication that descriptors and trampolines can exist for security reasons when crossing security domains, and you absolutely should not let one compartment get pointers to the entry point of another compartment's function. You can hand it out if sealed or the permissions are cleared, as then you can't really do anything with it other than look at the integer address, but that seems a bit odd.

That would be consistent with getting an unsigned pointer under pointer authentication or an address with the THUMB bit potentially stripped: it's just a raw address that you can't safely call. In any case, I suspect it would be fine for us to say that we just don't support this builtin when the target is using weird function pointers unless we have some way to bypass the special treatment in LLVM.

I agree that "symbol" address is probably the wrong name. Maybe __builtin_function_start or something like that? But before we go much further on this, we should get confirmation from Peter that we're targeting the right design.

Mon, Nov 15, 11:07 AM · Restricted Project

Fri, Nov 12

pcc added a comment to D113738: [LTO] Allow passing -Os/-Oz as the optimization level.

Please see D63976 where we rejected a similar change in favor of just letting this be controllable at compile time.

Fri, Nov 12, 10:46 AM · Restricted Project, Restricted Project, Restricted Project

Thu, Nov 11

pcc committed rZORG30d287a65f72: Switch from master to main in Chromium URL. (authored by pcc).
Switch from master to main in Chromium URL.
Thu, Nov 11, 3:49 PM
pcc accepted D113606: [gn build] Fix Android compiler-rt targets.

LGTM

Thu, Nov 11, 3:19 PM · Restricted Project
pcc accepted D113220: [X86] Selective relocation relaxation for +tagged-globals.

LGTM

Thu, Nov 11, 3:16 PM · Restricted Project

Wed, Nov 10

pcc added a comment to D113220: [X86] Selective relocation relaxation for +tagged-globals.

I think it ought to be possible to write these non-relaxable relocations from assembly, so that the behavior with -c is equivalent to the behavior with -S and then assembling the output. For example, you should be able to assemble

mov foo@gotpcrel_norelax(%rip), %rax

and the -S output from the compiler should look like that as well.

Won't this cause incompatibility with all other assemblers?

Wed, Nov 10, 2:59 PM · Restricted Project
pcc added a comment to D113220: [X86] Selective relocation relaxation for +tagged-globals.

I think it ought to be possible to write these non-relaxable relocations from assembly, so that the behavior with -c is equivalent to the behavior with -S and then assembling the output. For example, you should be able to assemble

mov foo@gotpcrel_norelax(%rip), %rax

and the -S output from the compiler should look like that as well.

Wed, Nov 10, 1:55 PM · Restricted Project
pcc added inline comments to D113606: [gn build] Fix Android compiler-rt targets.
Wed, Nov 10, 1:28 PM · Restricted Project
pcc added a comment to D113606: [gn build] Fix Android compiler-rt targets.

I don't think we can use llvm_code for this because we do cross-compile other parts of LLVM for Android (llvm-symbolizer, lldb-server).

Wed, Nov 10, 1:16 PM · Restricted Project
pcc added inline comments to D113606: [gn build] Fix Android compiler-rt targets.
Wed, Nov 10, 1:15 PM · Restricted Project

Nov 5 2021

pcc updated subscribers of D108479: [Clang] Add __builtin_addressof_nocfi.

(Adding back @rsmith, @rjmccall.)

Nov 5 2021, 4:05 PM · Restricted Project
pcc added a comment to D108479: [Clang] Add __builtin_addressof_nocfi.

Maybe it should even be semantically restricted to require a constant decl reference as its operand?

Nov 5 2021, 12:10 PM · Restricted Project

Nov 4 2021

pcc requested changes to D108479: [Clang] Add __builtin_addressof_nocfi.

I would argue that the existing __builtin_addressof() should absorb this behavior, rather than adding a special builtin related to CFI.

As is documented for __builtin_addressof(), its intended use is in cases where the & operator may return something other than the address of the object, which is exactly the case we are dealing with here.

Nov 4 2021, 2:02 PM · Restricted Project

Oct 30 2021

pcc added a comment to D112870: [MergeFunctions] Extend FunctionComparator to account for metadata arguments in intrinsics, and only ignore debug info metadata.

Shouldn't this be fixed in FunctionComparator?

So that's certainly the right question here. Should it? To me it looks like FunctionComparator is ignoring metadata attachments *on purpose*, and that fits the contract of "determine whether 2 functions will generate machine code with the same behavior".

Oct 30 2021, 8:50 AM · Restricted Project

Oct 28 2021

pcc requested changes to D112761: cfi-icall: Add -fsanitize-cfi-promotion-aliases.

I asked @samitolvanen out-of-band to check whether this really needs a flag since it seems like there could be some underlying issue that needs to be resolved so that we can do this unconditionally.

Oct 28 2021, 5:28 PM · Restricted Project, Restricted Project

Oct 27 2021

pcc accepted D108479: [Clang] Add __builtin_addressof_nocfi.

LGTM

Oct 27 2021, 5:21 PM · Restricted Project
pcc added inline comments to D108478: [llvm][IR] Add no_cfi constant.
Oct 27 2021, 5:16 PM · Restricted Project
pcc added a comment to D108741: [IR][GlobalDCE] Extend !vcall_visibility to be able to specify an offset range that it applies to.

Thanks, something like this seems reasonable to me and I don't have any major issues with it.

Oct 27 2021, 4:38 PM · Restricted Project
pcc accepted D107934: [LowerTypeTests] Emit cfi_jt aliases regardless of function export.

LGTM

Oct 27 2021, 11:33 AM · Restricted Project, Restricted Project

Oct 26 2021

pcc added inline comments to D107934: [LowerTypeTests] Emit cfi_jt aliases regardless of function export.
Oct 26 2021, 12:53 PM · Restricted Project, Restricted Project

Oct 8 2021

pcc accepted D111097: [DFSan] Remove -dfsan-args-abi support in favor of TLS..

LGTM

Oct 8 2021, 10:53 AM · Restricted Project, Restricted Project

Oct 4 2021

pcc accepted D110888: [compiler-rt][scudo] Check for failing prctl call.

s/TBI/the tagged address ABI/ in the commit message.

Oct 4 2021, 12:00 PM · Restricted Project
pcc accepted D111080: [compiler-rt][scudo] Simplify TBI checks.

LGTM

Oct 4 2021, 11:59 AM · Restricted Project

Sep 30 2021

pcc added a comment to D110888: [compiler-rt][scudo] Check for failing prctl call.

Please remember to upload patches with context: https://llvm.org/docs/Phabricator.html#requesting-a-review-via-the-web-interface

The tests in MemtagTests are meant to be skipped if the hardware doesn't support MTE (see the top of MemtagTest::SetUp()). Is that not working for you?

I don't think you should normally need to be defining SCUDO_DISABLE_TBI because it's controlled by the operating system, and all arm64 Linux systems that we normally care about support TBI. I figured that you were maybe trying to run these tests on Fuchsia, but then I noticed that memtag_test.cpp is wrapped in an #if SCUDO_LINUX, so I'm not sure why you're running into an issue. Are you trying to run these tests on arm64 non-Android Linux? Maybe the GTEST_SKIP() isn't working correctly for you for some reason?

I'm attempting to run host-linux runtimes tests. We weren't running them before and this is the first time we're trying it. We just happen to run into these test failures when turning them on before. But I think I found out the underlying issue: prctl(PR_GET_TAGGED_ADDR_CTRL, 0, 0, 0, 0) (in systemDetectsMemoryTagFaultsTestOnly) can return -1 if TBI isn't supported according to https://man7.org/linux/man-pages/man2/prctl.2.html, and this seems to be the case for us. The -1 is cast to an unsigned long and masked, then systemDetectsMemoryTagFaultsTestOnly ends up returning true when comparing a non-zero value after masking. I think the proper fix might be to just account for the -1 result. I'll update the patch later.

Sep 30 2021, 6:06 PM · Restricted Project
pcc added a comment to D110888: [compiler-rt][scudo] Check for failing prctl call.

Please remember to upload patches with context: https://llvm.org/docs/Phabricator.html#requesting-a-review-via-the-web-interface

Sep 30 2021, 4:08 PM · Restricted Project
pcc added inline comments to D103074: [scudo] Add memtag_test.
Sep 30 2021, 3:58 PM · Restricted Project

Sep 27 2021

pcc added a comment to D108741: [IR][GlobalDCE] Extend !vcall_visibility to be able to specify an offset range that it applies to.

Sorry, I think I managed to confuse myself when I was trying to figure out what VFE actually relies on. It seems that VFE doesn't rely on what I thought it did. Although VFE relies on type metadata, it only relies on it to determine the locations of the address points, and not the locations of the VFE-eligible function slots.

@pcc, I apologize I think I got confused the same way. I think I had the wrong understanding that if any slot in a vtable does not have a matching !type annotation (on the vtable global) with the right offset, it's basically trivially removable by VFE, but apparently, as you're pointing out, that's not the case -- it could still be used by a non-zero offset load via llvm.type.checked.load. In such a case, the slot at (vcall offset + vtable annotation offset) is actually being matched. So even if a vtable slot at offset X is never mentioned in any !type annotation, it could still be matched by any preceding !type type id with a non-zero offset at the call site.

Could you confirm that this understanding I just mentioned is actually correct now? Thanks :)

Here's one alternative proposal: can we make it so that any vtable slots that are eligible for VFE are represented using a new type of Constant (similar to dso_local_equivalent)? This also has the advantage that frontends could also use it for global variables as well as functions.

Just to make sure I understand this proposal, would we *require* frontends to mark all VFE eligible slots with this? I assume the only existing (publicly known) user of VFE is Clang, so as part of the change we'd fix Clang too? And the way this would look on a simple example would be something like the following?

@vtable = internal unnamed_addr constant ... {[
  i8* bitcast (void ()* vfe_eligible @vfunc1 to i8*),
  i8* bitcast (void ()* vfe_eligible @vfunc2 to i8*),
  i8* bitcast (void ()* @non_eligible to i8*)
]}, align 8, !type ...

And in the case of "relative pointers" that Swift uses in its data structures, the Constant would be present at the inner-most reference, like this?

@vtable = internal unnamed_addr constant { [2 x i32] } { [2 x i32] [
  i32 trunc (i64 sub (i64 ptrtoint (void ()* vfe_eligible @vfunc1 to i64), i64 ptrtoint ({ [2 x i32] }* @vtable to i64)) to i32),
  i32 trunc (i64 sub (i64 ptrtoint (void ()* vfe_eligible @vfunc2 to i64), i64 ptrtoint ({ [2 x i32] }* @vtable to i64)) to i32)
]}, align 8, !type ...
Sep 27 2021, 8:54 PM · Restricted Project
pcc added a comment to D108741: [IR][GlobalDCE] Extend !vcall_visibility to be able to specify an offset range that it applies to.

@pcc, could I ask for some details about your last comment? I'm specifically wondering whether you're against proceeding with the change from this review.

Sorry, but I think I'm against it because the mechanism that you've chosen seems like an abuse of the !type metadata, since the metadata is meant to be used as a reference point for accesses relative to it, and not as a way of marking up specific fields by itself. As a more practical concern, it has no way of representing the situation where the pointer at the address point is not eligible for VFE.

Here's one alternative proposal: can we make it so that any vtable slots that are eligible for VFE are represented using a new type of Constant (similar to dso_local_equivalent)? This also has the advantage that frontends could also use it for global variables as well as functions.

A potentially more lightweight solution may be providing a way to opt-in to the stricter interpretation. That way, we ensure no existing users are disturbed while not having to add more invasive changes. Do you think the additional complexity a new type of Constant carries its weight if it is only used to support the swift VFE case?

Sep 27 2021, 1:11 PM · Restricted Project

Sep 24 2021

pcc requested changes to D108741: [IR][GlobalDCE] Extend !vcall_visibility to be able to specify an offset range that it applies to.

It isn't great that VFE apparently relies on the type metadata that we added to support member function pointer CFI in C++. I was unaware that this was the case, and it shouldn't be necessary for frontends that don't have something like C++'s peculiar member function pointer ABI (I assume that Swift doesn't?) It would be better if we designed an independent way of marking these entries as subject to VFE.

As far as I can tell, VFE was added via https://reviews.llvm.org/D63932 and it has since always used type metadata and the !type annotations on vtables. Are you asking for a reversal on how VFE is done at the IR level, including how it's done for C++ in Clang? I'd love to see more thought on that (and what exactly do you think the design should aim to do better), but it seems to me that that's out of scope for this particular small tweak that I'm pursuing.

Sep 24 2021, 4:04 PM · Restricted Project

Sep 20 2021

pcc added a comment to D108741: [IR][GlobalDCE] Extend !vcall_visibility to be able to specify an offset range that it applies to.

As part of codesize optimization work for Swift, we'd like to add Virtual Function Elimination to Swift, very similarly to how GlobalDCE supports C++ VFE.

I think it would be good to drop this part from the description, as the change is not really related to swift; it just changes the code to ignore entries at slots without !type.

It seems sensible to me to ignore entries without corresponding !type metadata and this should be in line with the specification in langref. @pcc, @tejohnson Are there any issues you could think of?

Is this specified in the langref? I looked through the type metadata documentation but don't see it mentioned there that all vfuncs will have type metadata. Looks like that was added a bit later than the original type metadata, in D47567. I think what is specified in langref is that it has type metadata for the address point. Probably the documentation needs updating. But from what I can see, it looks like this should always be emitted now. @pcc ?

Thanks for checking! I think the update to the langref in the latest version should spell out things clearly.

Sep 20 2021, 3:42 PM · Restricted Project

Sep 16 2021

pcc committed rGfc08cfb8884d: CodeView: static_cast result of getOffset() to size_t. (authored by pcc).
CodeView: static_cast result of getOffset() to size_t.
Sep 16 2021, 11:41 PM
pcc added a comment to D108782: [MergeICmps] Ignore clobbering instructions before the loads.

Looks like this change caused this crash, can you please take a look?

$ cat test.ii
class a {
public:
  enum b {};
  struct c {
    b j;
    int *d;
    void *e;
  };
};
class f {
  bool operator==(const f &) const;
  a::c g() const;
};
bool f::operator==(const f &) const {
  a::c h = g(), i = g();
  return h.e == i.e && h.d == i.d && h.j == i.j;
}
$ clang++ -c  -o test.o test.ii -O2
Instruction does not dominate all uses!
  %h = alloca %"struct.a::c", align 8
  %1 = bitcast %"struct.a::c"* %h to i32*
Instruction does not dominate all uses!
  %i = alloca %"struct.a::c", align 8
  %2 = bitcast %"struct.a::c"* %i to i32*
Instruction does not dominate all uses!
  %i = alloca %"struct.a::c", align 8
  %18 = bitcast %"struct.a::c"* %i to i8*
Instruction does not dominate all uses!
  %h = alloca %"struct.a::c", align 8
  %19 = bitcast %"struct.a::c"* %h to i8*
in function _ZNK1feqERKS_
fatal error: error in backend: Broken function found, compilation aborted!
Sep 16 2021, 11:15 PM · Restricted Project

Sep 13 2021

pcc added inline comments to D109698: [hwasan] print globals in symbolizer-friendly format..
Sep 13 2021, 12:24 PM · Restricted Project

Sep 10 2021

pcc added a comment to D109572: [ELF] Add --why-extract= to query why archive members/lazy object files are extracted.

I'm not sure this should be considered an alternative to D69607. Maybe it is if you're not using --gc-sections, but with that flag we really need something like a breadth-first search like in that patch.

Sep 10 2021, 9:33 AM · Restricted Project

Sep 9 2021

pcc added inline comments to D107934: [LowerTypeTests] Emit cfi_jt aliases regardless of function export.
Sep 9 2021, 1:57 PM · Restricted Project, Restricted Project

Sep 8 2021

pcc accepted D109478: [gn build] Make lldb build on Windows.

LGTM

Sep 8 2021, 7:37 PM · Restricted Project
pcc committed rG883e93cb280e: gn build: Add support for building lldb-server on Android. (authored by pcc).
gn build: Add support for building lldb-server on Android.
Sep 8 2021, 7:35 PM
pcc committed rG9449f441fc9b: gn build: Add support for building LLDB on Linux. (authored by pcc).
gn build: Add support for building LLDB on Linux.
Sep 8 2021, 7:35 PM
pcc closed D109464: gn build: Add support for building lldb-server on Android..
Sep 8 2021, 7:35 PM · Restricted Project
pcc closed D109463: gn build: Add support for building LLDB on Linux..
Sep 8 2021, 7:35 PM · Restricted Project
pcc added inline comments to D109463: gn build: Add support for building LLDB on Linux..
Sep 8 2021, 5:18 PM · Restricted Project
pcc updated the diff for D109464: gn build: Add support for building lldb-server on Android..

Move to data_deps

Sep 8 2021, 3:47 PM · Restricted Project
pcc updated the diff for D109463: gn build: Add support for building LLDB on Linux..

Move to data_deps

Sep 8 2021, 3:47 PM · Restricted Project
pcc added inline comments to D109463: gn build: Add support for building LLDB on Linux..
Sep 8 2021, 3:28 PM · Restricted Project
pcc requested review of D109464: gn build: Add support for building lldb-server on Android..
Sep 8 2021, 1:54 PM · Restricted Project
pcc requested review of D109463: gn build: Add support for building LLDB on Linux..
Sep 8 2021, 1:53 PM · Restricted Project
pcc added a comment to D103127: lldb: Don't check for CMAKE_SYSTEM_NAME==Android..

How exactly are you building this? CMAKE_SYSTEM_NAME is set in the official android cmake toolchain file (https://android.googlesource.com/platform/ndk/+/refs/heads/ndk-release-r23/build/cmake/android.toolchain.cmake#213)

Sep 8 2021, 1:12 PM · Restricted Project

Sep 7 2021

pcc added a comment to D109393: [asan] Changed the label for error reporting code to use GOT instead of PLT..

Have you verified that things work at runtime with this change? Because I believe that your change will result in branching to the GOT entry for the runtime functions. Since GOT entries are just addresses and are not executable, I believe that this would result in a segfault.

Sep 7 2021, 5:35 PM · Restricted Project
pcc added a comment to D109393: [asan] Changed the label for error reporting code to use GOT instead of PLT..

What's the effect on the relocation types in the object file with this change? I don't think it's valid to have a branch with an @GOT modifier.

Moving from PTL to GOT fixed this errors like this: "error: 32 bit reloc applied to a field with a different size". What do you thing should be the right type for this? Not specifying PTL also had problems with linking so living in blank is not an option.

Sep 7 2021, 2:57 PM · Restricted Project
pcc added a comment to D109393: [asan] Changed the label for error reporting code to use GOT instead of PLT..

What's the effect on the relocation types in the object file with this change? I don't think it's valid to have a branch with an @GOT modifier.

Sep 7 2021, 2:40 PM · Restricted Project
pcc accepted D109185: [gn build] Add build files for LLDB.
  • rebase
  • add -sectcreate flags for embedding Info.plist files into lldb and lldb-vscode

This is probably in shape enough to iterate in in-tree. I didn't squash in your linux change; let's land that separately :)

Sep 7 2021, 11:59 AM · Restricted Project, Restricted Project

Sep 6 2021

hafixo awarded rGc336557f0238: hwasan: Compatibility fixes for short granules. a Party Time token.
Sep 6 2021, 12:45 AM
hafixo awarded rCRT373035: hwasan: Compatibility fixes for short granules. a Party Time token.
Sep 6 2021, 12:42 AM

Sep 2 2021

pcc added a comment to D109185: [gn build] Add build files for LLDB.

Thanks for working on this!

Sep 2 2021, 3:12 PM · Restricted Project, Restricted Project
pcc added a comment to D109114: [GlobalDCE] In VFE, replace the whole 'sub' expression of unused relative-pointer-based vtable slots.

Oh right, I missed that you were emitting the function locally. So then replace "saves a PLT slot" with "saves emitting a trap function".

Sep 2 2021, 12:47 PM · Restricted Project
pcc added a comment to D109114: [GlobalDCE] In VFE, replace the whole 'sub' expression of unused relative-pointer-based vtable slots.

Maybe you can set the relative offset to 0 in this case? That way, the crashing PC will point to the vtable, which should make it easier to track down the source of any problems. This would also require no runtime support (so doesn't need to be behind a flag) and also saves a PLT slot if the trap function would be in another DSO.

Sep 2 2021, 11:12 AM · Restricted Project

Aug 27 2021

pcc added a comment to D108006: [lld][ELF] Add --no-search-static-libs-for-shlib-undefined flag.

Could we perhaps put something in the binary if it was linked with --no-undefined (e.g. a dynamic table entry)? Then if we're linking against such a binary, ignore any undefined symbols.

Aug 27 2021, 10:30 AM · Restricted Project

Aug 24 2021

pcc added inline comments to D108452: [hwasan] prevent out of range memory accesses for invalid free..
Aug 24 2021, 3:23 PM · Restricted Project
pcc updated subscribers of D42225: libcxx: Provide overloads for basic_filebuf::open() et al that take wchar_t* filenames on Windows..

I believe that Chromium uses it (or at least it did at the time that I added this, and Chromium has since switched to using libc++ on Windows).

Aug 24 2021, 11:20 AM

Aug 20 2021

pcc added a comment to D106653: [LoopVectorize][AArch64] Enable ordered reductions by default for AArch64.

Hi David, it looks like this change caused an assertion failure. Can you please take a look?

> cat test.i
int a;
double b, c;
void d() {
  for (; a; a++) {
    b += c;
    c = a;
  }
}
> clang -O2 test.i --target=aarch64-linux
clang: ../llvm/lib/Transforms/Vectorize/VPlanValue.h:186: llvm::Value *llvm::VPValue::getLiveInIRValue(): Assertion `!getDef() && "VPValue is not a live-in; it is defined by a VPDef inside a VPlan"' failed.
Aug 20 2021, 10:07 AM · Restricted Project

Aug 19 2021

pcc added inline comments to D107850: [asan] Implemented intrinsic for the custom calling convention similar used by HWASan for X86..
Aug 19 2021, 2:09 PM · Restricted Project, Restricted Project
pcc added inline comments to D107850: [asan] Implemented intrinsic for the custom calling convention similar used by HWASan for X86..
Aug 19 2021, 1:06 PM · Restricted Project, Restricted Project

Aug 18 2021

pcc committed rG6f85225ef379: StackLifetime: Remove asserts for multiple lifetime intrinsics. (authored by pcc).
StackLifetime: Remove asserts for multiple lifetime intrinsics.
Aug 18 2021, 6:46 PM
pcc closed D108337: StackLifetime: Remove asserts for multiple lifetime intrinsics..
Aug 18 2021, 6:46 PM · Restricted Project
pcc updated the diff for D108337: StackLifetime: Remove asserts for multiple lifetime intrinsics..

Test this in test/Analysis/StackSafetyAnalysis/lifetime.ll instead

Aug 18 2021, 5:50 PM · Restricted Project
pcc requested review of D108337: StackLifetime: Remove asserts for multiple lifetime intrinsics..
Aug 18 2021, 3:14 PM · Restricted Project
pcc committed rGb2e77cd095a6: gn build: Build libclang.so and libLTO.so on ELF platforms. (authored by pcc).
gn build: Build libclang.so and libLTO.so on ELF platforms.
Aug 18 2021, 1:50 PM
pcc closed D108223: gn build: Build libclang.so on ELF platforms..
Aug 18 2021, 1:49 PM · Restricted Project

Aug 17 2021

pcc added inline comments to D108223: gn build: Build libclang.so on ELF platforms..
Aug 17 2021, 11:34 AM · Restricted Project
pcc updated the diff for D108223: gn build: Build libclang.so on ELF platforms..

Also libLTO.so

Aug 17 2021, 11:34 AM · Restricted Project
pcc requested review of D108223: gn build: Build libclang.so on ELF platforms..
Aug 17 2021, 10:55 AM · Restricted Project
pcc committed rGaffb132ab89f: hwasan: Move stack ring buffer initialization before InitStackAndTls. (authored by pcc).
hwasan: Move stack ring buffer initialization before InitStackAndTls.
Aug 17 2021, 10:18 AM
pcc closed D108184: hwasan: Move stack ring buffer initialization before InitStackAndTls..
Aug 17 2021, 10:18 AM · Restricted Project

Aug 16 2021

pcc requested review of D108184: hwasan: Move stack ring buffer initialization before InitStackAndTls..
Aug 16 2021, 8:42 PM · Restricted Project

Aug 13 2021

pcc accepted D108064: [hwasan] Ignore lit config.enable_aliases on non-x86..

LGTM

Aug 13 2021, 7:00 PM · Restricted Project
pcc accepted D108063: [hwasan] Support malloc in atfork..

LGTM

Aug 13 2021, 7:00 PM · Restricted Project

Aug 12 2021

pcc added a comment to D107978: scudo: fix __attribute__((format)).

Note that this is the legacy copy of Scudo. Looks like scudo/standalone has the same problem (see ScopedString::Append and Printf in compiler-rt/lib/scudo/standalone/string_utils.cpp).

Aug 12 2021, 2:43 PM · Restricted Project

Aug 9 2021

pcc added a comment to D104496: [GlobalDCE] Support of conditionally used global variables.

0 means when any GV in the third element is alive, mark the first element as used.

Aug 9 2021, 5:33 PM · Restricted Project

Aug 4 2021

pcc accepted D107391: [hwasan] Add __hwasan_init constructor to runtime lib..

LGTM

Aug 4 2021, 10:49 AM · Restricted Project

Jul 16 2021

pcc added inline comments to D106046: tsan: make obtaining current PC faster.
Jul 16 2021, 11:29 AM · Restricted Project
pcc accepted D104058: ThinLTO: Fix inline assembly references to static functions with CFI.

LGTM

Jul 16 2021, 11:15 AM · Restricted Project, Restricted Project

Jul 14 2021

pcc accepted D105954: [scudo] Don't enabled MTE for small alignment.

LGTM

Jul 14 2021, 12:02 PM · Restricted Project

Jul 12 2021

pcc added a comment to D105178: [lldb][AArch64] Annotate synchronous tag faults.

Programs must enable the tagged address ABI to
receive these signals and are also opting into the
presence of these tag bits.

Jul 12 2021, 6:14 PM · Restricted Project

Jul 9 2021

pcc accepted D105722: [scudo] Check if we use __clang_major__ >= 12.

LGTM

Jul 9 2021, 4:33 PM · Restricted Project
pcc added inline comments to D105722: [scudo] Check if we use __clang_major__ >= 12.
Jul 9 2021, 12:33 PM · Restricted Project
pcc added a comment to D103726: [scudo] Enabled MTE before the first allocator.

Yeah, that should be good enough for now. It won't cover users who have the integrated assembler disabled, but it's better than nothing.

Jul 9 2021, 11:10 AM · Restricted Project

Jul 7 2021

pcc added a comment to D103726: [scudo] Enabled MTE before the first allocator.

This is a known issue with the use of MTE instructions in memtag.h -- they should really be conditional on whether the assembler supports them. For now, please upgrade your assembler.

Jul 7 2021, 10:36 AM · Restricted Project

Jul 1 2021

pcc accepted D105261: [scudo] Untag BlockEnd in reallocate.

LGTM

Jul 1 2021, 12:55 PM · Restricted Project
pcc added inline comments to D105261: [scudo] Untag BlockEnd in reallocate.
Jul 1 2021, 12:36 PM · Restricted Project
pcc added a comment to D105261: [scudo] Untag BlockEnd in reallocate.

Can we untag BlockEnd in reallocate instead?

Yes, but it looks a little bit inconsistent when untag End here

Jul 1 2021, 12:15 PM · Restricted Project
pcc added a comment to D105261: [scudo] Untag BlockEnd in reallocate.

Can we untag BlockEnd in reallocate instead?

Jul 1 2021, 12:04 PM · Restricted Project
pcc accepted D105266: [scudo] Remove false DCHECK.

LGTM

Jul 1 2021, 11:32 AM · Restricted Project

Jun 30 2021

pcc accepted D105232: [scudo] GWP_ASAN runs on untagged pointers.

LGTM

Jun 30 2021, 2:48 PM · Restricted Project