Page MenuHomePhabricator

pcc (Peter Collingbourne)
User

Projects

User does not belong to any projects.

User Details

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

Recent Activity

Fri, Jul 19

pcc updated subscribers of D65029: [Driver] Support for disabling sanitizer runtime linking.

It's also worth asking whether -nodefaultlibs would work for this use case as @filcab suggested on D64547.

Fri, Jul 19, 9:49 PM · Restricted Project
pcc accepted D65032: [ELF] Support explicitly overriding relocation model in LTO.

LGTM

Fri, Jul 19, 5:59 PM · Restricted Project
pcc created D65033: LowerTypeTests: Teach the pass to respect global alignments..
Fri, Jul 19, 5:55 PM · Restricted Project
pcc created D65031: WholeProgramDevirt: Teach the pass to respect the global's alignment..
Fri, Jul 19, 5:04 PM · Restricted Project
pcc accepted D65009: [LTO] Always mark regular LTO units with EnableSplitLTOUnit=1.

LGTM

Fri, Jul 19, 3:56 PM · Restricted Project, Restricted Project
pcc added a comment to D65009: [LTO] Always mark regular LTO units with EnableSplitLTOUnit=1.

Sorry, just realized this. If I do

clang++ -c -flto a.cpp # "split"
clang++ -c -flto=thin b.cpp -fwhole-program-vtables # non-split
clang++ a.o b.o

this should fail, right? If I'm not mistaken, this patch series will cause this to succeed. I think we need to change this patch so that it always sets EnableSplitLTOUnit to 1 for regular LTO, then you can drop the other patch.

Fri, Jul 19, 1:26 PM · Restricted Project, Restricted Project

Thu, Jul 18

pcc added a comment to rL366497: Fix typo in r366494. Spotted by Yuanfang Chen..

Unfortunately it looks like this part of the condition had no test coverage before I refactored it in r366494, which is how the mistake slipped through.

Thu, Jul 18, 3:22 PM
pcc committed rG50057f328870: CodeGen: Allow !associated metadata to point to aliases. (authored by pcc).
CodeGen: Allow !associated metadata to point to aliases.
Thu, Jul 18, 2:40 PM
pcc committed rL366502: CodeGen: Allow !associated metadata to point to aliases..
CodeGen: Allow !associated metadata to point to aliases.
Thu, Jul 18, 2:40 PM
pcc closed D64951: CodeGen: Allow !associated metadata to point to aliases..
Thu, Jul 18, 2:40 PM · Restricted Project
pcc created D64951: CodeGen: Allow !associated metadata to point to aliases..
Thu, Jul 18, 2:20 PM · Restricted Project
pcc committed rG68f3fc2d9166: Fix typo in r366494. Spotted by Yuanfang Chen. (authored by pcc).
Fix typo in r366494. Spotted by Yuanfang Chen.
Thu, Jul 18, 2:05 PM
pcc committed rL366497: Fix typo in r366494. Spotted by Yuanfang Chen..
Fix typo in r366494. Spotted by Yuanfang Chen.
Thu, Jul 18, 2:04 PM
pcc added inline comments to D64948: IR: Teach Constant::needsRelocation() that relative pointers don't need to be relocated..
Thu, Jul 18, 2:04 PM · Restricted Project
pcc committed rGd1ec8eb84f7f: IR: Teach Constant::needsRelocation() that relative pointers don't need to be… (authored by pcc).
IR: Teach Constant::needsRelocation() that relative pointers don't need to be…
Thu, Jul 18, 1:58 PM
pcc committed rL366494: IR: Teach Constant::needsRelocation() that relative pointers don't need to be….
IR: Teach Constant::needsRelocation() that relative pointers don't need to be…
Thu, Jul 18, 1:57 PM
pcc closed D64948: IR: Teach Constant::needsRelocation() that relative pointers don't need to be relocated..
Thu, Jul 18, 1:57 PM · Restricted Project
pcc created D64948: IR: Teach Constant::needsRelocation() that relative pointers don't need to be relocated..
Thu, Jul 18, 1:46 PM · Restricted Project
pcc committed rG468f34d75f1e: gn build: Merge r366458. (authored by pcc).
gn build: Merge r366458.
Thu, Jul 18, 1:16 PM
pcc committed rL366487: gn build: Merge r366458..
gn build: Merge r366458.
Thu, Jul 18, 1:15 PM
pcc committed rGc2ccf4ccba29: ELF: Add support for remaining R_AARCH64_MOVW* relocations. (authored by pcc).
ELF: Add support for remaining R_AARCH64_MOVW* relocations.
Thu, Jul 18, 10:15 AM
pcc committed rL366466: ELF: Add support for remaining R_AARCH64_MOVW* relocations..
ELF: Add support for remaining R_AARCH64_MOVW* relocations.
Thu, Jul 18, 10:13 AM
pcc closed D64685: ELF: Add support for remaining R_AARCH64_MOVW* relocations..
Thu, Jul 18, 10:13 AM · Restricted Project
pcc added inline comments to D64685: ELF: Add support for remaining R_AARCH64_MOVW* relocations..
Thu, Jul 18, 10:10 AM · Restricted Project
pcc committed rG311131dafc0e: ELF: Simplify test. NFCI. (authored by pcc).
ELF: Simplify test. NFCI.
Thu, Jul 18, 9:57 AM
pcc committed rL366463: ELF: Simplify test. NFCI..
ELF: Simplify test. NFCI.
Thu, Jul 18, 9:57 AM
pcc closed D64684: ELF: Simplify test. NFCI..
Thu, Jul 18, 9:57 AM · Restricted Project
pcc committed rGaa6a7df64a46: MC: AArch64: Add support for prel_g* relocation specifiers. (authored by pcc).
MC: AArch64: Add support for prel_g* relocation specifiers.
Thu, Jul 18, 9:57 AM
pcc committed rL366462: MC: AArch64: Add support for prel_g* relocation specifiers..
MC: AArch64: Add support for prel_g* relocation specifiers.
Thu, Jul 18, 9:56 AM
pcc closed D64683: MC: AArch64: Add support for prel_g* relocation specifiers..
Thu, Jul 18, 9:56 AM · Restricted Project
pcc committed rG76427f849fc9: AArch64: Unify relocation restrictions between MOVK/MOVN/MOVZ. (authored by pcc).
AArch64: Unify relocation restrictions between MOVK/MOVN/MOVZ.
Thu, Jul 18, 9:53 AM
pcc committed rL366461: AArch64: Unify relocation restrictions between MOVK/MOVN/MOVZ..
AArch64: Unify relocation restrictions between MOVK/MOVN/MOVZ.
Thu, Jul 18, 9:53 AM
pcc closed D64466: AArch64: Unify relocation restrictions between MOVK/MOVN/MOVZ..
Thu, Jul 18, 9:53 AM · Restricted Project
pcc committed rGcb2d8e912512: ELF: Allow forward references to linked sections. (authored by pcc).
ELF: Allow forward references to linked sections.
Thu, Jul 18, 9:48 AM
pcc committed rL366460: ELF: Allow forward references to linked sections..
ELF: Allow forward references to linked sections.
Thu, Jul 18, 9:48 AM
pcc closed D64880: ELF: Allow forward references to linked sections..
Thu, Jul 18, 9:48 AM · Restricted Project

Wed, Jul 17

pcc updated the diff for D64880: ELF: Allow forward references to linked sections..
  • Add comment
Wed, Jul 17, 7:30 PM · Restricted Project
pcc added inline comments to D64880: ELF: Allow forward references to linked sections..
Wed, Jul 17, 7:30 PM · Restricted Project
pcc committed rG749f556bbd14: hwasan: Use C++ driver for cfi.cc test. (authored by pcc).
hwasan: Use C++ driver for cfi.cc test.
Wed, Jul 17, 4:39 PM
pcc committed rL366389: hwasan: Use C++ driver for cfi.cc test..
hwasan: Use C++ driver for cfi.cc test.
Wed, Jul 17, 4:39 PM
pcc closed D64890: hwasan: Use C++ driver for cfi.cc test..
Wed, Jul 17, 4:39 PM · Restricted Project, Restricted Project
pcc accepted D64896: Make DT a transitive dependency of LI..

LGTM

Wed, Jul 17, 4:30 PM · Restricted Project
pcc added a comment to D64843: hwasan: Initialize the pass only once..

The instrumentation for globals is coming in a separate patch.

Wed, Jul 17, 3:36 PM · Restricted Project, Restricted Project
pcc created D64890: hwasan: Use C++ driver for cfi.cc test..
Wed, Jul 17, 3:25 PM · Restricted Project, Restricted Project
pcc committed rG0dd40a7d9f2f: gn build: Merge r366361. (authored by pcc).
gn build: Merge r366361.
Wed, Jul 17, 2:47 PM
pcc committed rG3b82b92c6b90: hwasan: Initialize the pass only once. (authored by pcc).
hwasan: Initialize the pass only once.
Wed, Jul 17, 2:46 PM
pcc committed rL366380: gn build: Merge r366361..
gn build: Merge r366361.
Wed, Jul 17, 2:45 PM
pcc committed rL366379: hwasan: Initialize the pass only once..
hwasan: Initialize the pass only once.
Wed, Jul 17, 2:45 PM
pcc closed D64843: hwasan: Initialize the pass only once..
Wed, Jul 17, 2:45 PM · Restricted Project, Restricted Project
pcc added a comment to D64843: hwasan: Initialize the pass only once..

IIUC the reason was to work around the lack of analysis availability from module passes, which is no longer a concern with the new PM. I would personally prefer to see the other sanitizers re-architected this way.

Wed, Jul 17, 12:45 PM · Restricted Project, Restricted Project
pcc created D64880: ELF: Allow forward references to linked sections..
Wed, Jul 17, 12:43 PM · Restricted Project
pcc added a comment to D57400: Add a .gitignore file to the root that ignores any files outside of the project directories..

Using /*/ as the ignore pattern would work for me. My main concern was that it would conflict with my use of /* in the exclude file to ignore files in the root directory, but it appears that they don't conflict. If @jyknight is happy with this then I'll update this review as suggested.

Wed, Jul 17, 10:22 AM

Tue, Jul 16

pcc added a comment to D64711: [LTO] Don't override relocation model stored in the module.

As I see it it's not too different from e.g. internalizing or hiding symbols or propagating DSO-local, which is, in the end, also overriding a compile-time decision. I think that taking this sort of stance in general makes it hard to say where to draw the line on what kind of optimizations are permitted. If we do switch to the compile-time relocation model, should the backend then be prevented from generating jump tables with absolute addresses when linking without -pie or -shared because it could cause problems if the binary happens to end up being loaded at the wrong address? That doesn't seem like a good idea to me. It seems more practical to say, at least in this case, that the binary that you get from a non-PIC link is, in fact, position-dependent, and it is an error to use it position-independently.

Tue, Jul 16, 8:40 PM · Restricted Project
pcc created D64843: hwasan: Initialize the pass only once..
Tue, Jul 16, 5:40 PM · Restricted Project, Restricted Project
pcc added a comment to D48680: Add missing visibility annotation for __base.

@pcc In your reproducer, what is ~/l3/ra-cmake/bin/clang++?

Tue, Jul 16, 11:09 AM
pcc added a comment to D48680: Add missing visibility annotation for __base.

I think it would be up to the libc++ maintainers to approve the patch.

Tue, Jul 16, 10:53 AM

Mon, Jul 15

pcc committed rGe5c4b468f063: hwasan: Pad arrays with non-1 size correctly. (authored by pcc).
hwasan: Pad arrays with non-1 size correctly.
Mon, Jul 15, 8:30 PM
pcc committed rL366171: hwasan: Pad arrays with non-1 size correctly..
hwasan: Pad arrays with non-1 size correctly.
Mon, Jul 15, 8:30 PM
pcc closed D64783: hwasan: Pad arrays with non-1 size correctly..
Mon, Jul 15, 8:30 PM · Restricted Project
pcc added a comment to D64711: [LTO] Don't override relocation model stored in the module.
In D64711#1586738, @pcc wrote:
In D64711#1584916, @pcc wrote:

I'd like to understand the motivation behind this change a little better because the relaxation of the code model from PIC to static is technically an "optimization" that the linker should have license to perform and in fact is already performing in some ways (e.g. emitting addresses into the binary instead of RELATIVE/RELR relocations for statically known addresses).

In the case of our kernel, we need the generated code to be position independent for our KASLR scheme, but we're not doing traditional PIE link. When compiling individual translation units, we use -fpie, but when linking we use --no-pie. Changing the code model from PIC to static in this case results in an invalid kernel image because the generated code is not relocatable. I think it's a matter of principle that it shouldn't be the linker's responsibility to decide how to combine our code-generation choices with our linking choices. I'm open to suggestions for other ways to solve this issue though as we'd really like to be able to use ThinLTO for our kernel.

Have you considered linking the kernel as a PIE image when KASLR is enabled? This will fix not only this problem but also others (e.g. initializers containing global pointers, which might not just be explicit in the source code but could also be implicitly generated by the compiler, are broken with this scheme).

This is what the Linux kernel does (it uses -shared -Bsymbolic instead of -pie in KASLR mode, but they are almost equivalent):
https://github.com/torvalds/linux/blob/fec88ab0af9706b2201e5daf377c5031c62d11f7/arch/arm64/Makefile#L21

It uses assembly to self relocate to avoid depending on relative relocations during early startup:
https://github.com/torvalds/linux/blob/fec88ab0af9706b2201e5daf377c5031c62d11f7/arch/arm64/kernel/head.S#L822

We're considering it but it requires some non-trivial changes and so won't be ready anytime soon.

Mon, Jul 15, 7:20 PM · Restricted Project
pcc updated the diff for D64783: hwasan: Pad arrays with non-1 size correctly..
  • Add test
Mon, Jul 15, 6:02 PM · Restricted Project
pcc created D64783: hwasan: Pad arrays with non-1 size correctly..
Mon, Jul 15, 5:57 PM · Restricted Project
pcc added a comment to D64711: [LTO] Don't override relocation model stored in the module.
In D64711#1584916, @pcc wrote:

I'd like to understand the motivation behind this change a little better because the relaxation of the code model from PIC to static is technically an "optimization" that the linker should have license to perform and in fact is already performing in some ways (e.g. emitting addresses into the binary instead of RELATIVE/RELR relocations for statically known addresses).

In the case of our kernel, we need the generated code to be position independent for our KASLR scheme, but we're not doing traditional PIE link. When compiling individual translation units, we use -fpie, but when linking we use --no-pie. Changing the code model from PIC to static in this case results in an invalid kernel image because the generated code is not relocatable. I think it's a matter of principle that it shouldn't be the linker's responsibility to decide how to combine our code-generation choices with our linking choices. I'm open to suggestions for other ways to solve this issue though as we'd really like to be able to use ThinLTO for our kernel.

Mon, Jul 15, 4:55 PM · Restricted Project
pcc added a comment to D64458: add -fthinlto-index= option to clang-cl.

I think it's a little unfortunate that we're continuing to go down the road of letting users pass more flags to the ThinLTO backend action, but I won't stand in the way here.

Mon, Jul 15, 3:16 PM · Restricted Project, Restricted Project

Sun, Jul 14

pcc added a reviewer for D64711: [LTO] Don't override relocation model stored in the module: pcc.

I'd like to understand the motivation behind this change a little better because the relaxation of the code model from PIC to static is technically an "optimization" that the linker should have license to perform and in fact is already performing in some ways (e.g. emitting addresses into the binary instead of RELATIVE/RELR relocations for statically known addresses).

Sun, Jul 14, 9:37 PM · Restricted Project

Fri, Jul 12

pcc created D64685: ELF: Add support for remaining R_AARCH64_MOVW* relocations..
Fri, Jul 12, 6:42 PM · Restricted Project
pcc added a child revision for D64683: MC: AArch64: Add support for prel_g* relocation specifiers.: D64685: ELF: Add support for remaining R_AARCH64_MOVW* relocations..
Fri, Jul 12, 6:42 PM · Restricted Project
pcc added a child revision for D64684: ELF: Simplify test. NFCI.: D64685: ELF: Add support for remaining R_AARCH64_MOVW* relocations..
Fri, Jul 12, 6:42 PM · Restricted Project
pcc created D64684: ELF: Simplify test. NFCI..
Fri, Jul 12, 6:40 PM · Restricted Project
pcc created D64683: MC: AArch64: Add support for prel_g* relocation specifiers..
Fri, Jul 12, 6:38 PM · Restricted Project
pcc added a child revision for D64466: AArch64: Unify relocation restrictions between MOVK/MOVN/MOVZ.: D64683: MC: AArch64: Add support for prel_g* relocation specifiers..
Fri, Jul 12, 6:38 PM · Restricted Project
pcc added inline comments to D64679: Display codeview type record values in hex representation instead of decimal.
Fri, Jul 12, 5:42 PM · Restricted Project
pcc accepted D64663: Extend function attributes bitset size from 64 to 96..

Ah, I missed that.

Fri, Jul 12, 2:49 PM · Restricted Project
pcc added a comment to D64663: Extend function attributes bitset size from 64 to 96..

I would prefer to keep this the way it is with an array so that it is obvious what the size/alignment are.

Fri, Jul 12, 2:42 PM · Restricted Project
pcc updated subscribers of rL365847: Attempt to override broken buildbot config for libc++abi..

This change broke the fuzzer libc++ build on multiple sanitizer buildbots, e.g.:
http://lab.llvm.org:8011/builders/sanitizer-x86_64-linux/builds/22287
http://lab.llvm.org:8011/builders/sanitizer-x86_64-linux-bootstrap/builds/13081

Fri, Jul 12, 2:07 PM
pcc added a comment to D64597: CodeGet: Init 32bit pointers with 0xFFFFFFFF.
In D64597#1581605, @pcc wrote:

The problem with 0xaaaaaaaa on 32-bit is that it is likely to be a valid address.

When I discussed this with JF I proposed a pointer initialization of 0xffffffff which he agreed to. This value is very likely to trap when accessed (due to accesses likely wrapping to zero) and also has the benefit of being the same pattern as for floats.

The value is very likely not to trap on some systems (due to accesses likely wrapping to zero). I am not sure I have a better magic value to offer though.

Fri, Jul 12, 9:42 AM · Restricted Project, Restricted Project

Thu, Jul 11

pcc added a comment to D64597: CodeGet: Init 32bit pointers with 0xFFFFFFFF.

The problem with 0xaaaaaaaa on 32-bit is that it is likely to be a valid address.

Thu, Jul 11, 2:32 PM · Restricted Project, Restricted Project
pcc added inline comments to D64427: [Test-Suite] Support Cross-Compilation and Cross-execution targeting arm64-linux-android.
Thu, Jul 11, 12:40 PM · Restricted Project
pcc added a comment to D64427: [Test-Suite] Support Cross-Compilation and Cross-execution targeting arm64-linux-android.

FWIW, I did something similar: https://github.com/pcc/llvm-test-suite/tree/android
but never got time to clean it up and send it for review.

Thu, Jul 11, 12:37 PM · Restricted Project

Wed, Jul 10

pcc committed rGd37edd0c7946: ELF: Add support for R_AARCH64_ADR_PREL_PG_HI21_NC relocation. (authored by pcc).
ELF: Add support for R_AARCH64_ADR_PREL_PG_HI21_NC relocation.
Wed, Jul 10, 9:43 AM
pcc committed rL365662: ELF: Add support for R_AARCH64_ADR_PREL_PG_HI21_NC relocation..
ELF: Add support for R_AARCH64_ADR_PREL_PG_HI21_NC relocation.
Wed, Jul 10, 9:43 AM
pcc closed D64456: ELF: Add support for R_AARCH64_ADR_PREL_PG_HI21_NC relocation..
Wed, Jul 10, 9:43 AM · Restricted Project
pcc committed rG893f8d719c0e: MC: AArch64: Add support for pg_hi21_nc relocation specifier. (authored by pcc).
MC: AArch64: Add support for pg_hi21_nc relocation specifier.
Wed, Jul 10, 9:38 AM
pcc committed rL365661: MC: AArch64: Add support for pg_hi21_nc relocation specifier..
MC: AArch64: Add support for pg_hi21_nc relocation specifier.
Wed, Jul 10, 9:38 AM
pcc closed D64455: MC: AArch64: Add support for pg_hi21_nc relocation specifier..
Wed, Jul 10, 9:37 AM · Restricted Project
pcc added a comment to D63932: [GlobalDCE] Dead Virtual Function Elimination.

Hi Oliver, thanks for working on this. This is something that I've always wanted to do with the type metadata but never had time to work on. I'll try to take a more in depth look later this week.

Wed, Jul 10, 8:52 AM · Restricted Project, Restricted Project

Tue, Jul 9

pcc added a comment to D64456: ELF: Add support for R_AARCH64_ADR_PREL_PG_HI21_NC relocation..

Might be worth adding another test to aarch64-fpic-adr_prel_pg_hi21.s

Tue, Jul 9, 7:35 PM · Restricted Project
pcc updated the diff for D64456: ELF: Add support for R_AARCH64_ADR_PREL_PG_HI21_NC relocation..
  • Address review comments
Tue, Jul 9, 7:35 PM · Restricted Project
pcc created D64466: AArch64: Unify relocation restrictions between MOVK/MOVN/MOVZ..
Tue, Jul 9, 6:55 PM · Restricted Project
pcc added a comment to D64458: add -fthinlto-index= option to clang-cl.

Do we really need to support clang-cl syntax here? Can't the user of -fthinlto-index= use the regular clang driver?

Tue, Jul 9, 5:35 PM · Restricted Project, Restricted Project
pcc created D64456: ELF: Add support for R_AARCH64_ADR_PREL_PG_HI21_NC relocation..
Tue, Jul 9, 5:09 PM · Restricted Project
pcc added a child revision for D64455: MC: AArch64: Add support for pg_hi21_nc relocation specifier.: D64456: ELF: Add support for R_AARCH64_ADR_PREL_PG_HI21_NC relocation..
Tue, Jul 9, 5:09 PM · Restricted Project
pcc created D64455: MC: AArch64: Add support for pg_hi21_nc relocation specifier..
Tue, Jul 9, 5:08 PM · Restricted Project
pcc committed rG67a06d949547: gn build: Merge r365536. (authored by pcc).
gn build: Merge r365536.
Tue, Jul 9, 3:40 PM
pcc committed rG3ea053ad505d: gn build: Merge r365532. (authored by pcc).
gn build: Merge r365532.
Tue, Jul 9, 3:40 PM
pcc committed rGfa7eea9e4ec8: gn build: Merge r365531. (authored by pcc).
gn build: Merge r365531.
Tue, Jul 9, 3:40 PM
pcc committed rGd9f7162d4bf1: gn build: Merge r365541. (authored by pcc).
gn build: Merge r365541.
Tue, Jul 9, 3:40 PM
pcc committed rL365572: gn build: Merge r365536..
gn build: Merge r365536.
Tue, Jul 9, 3:40 PM
pcc committed rL365571: gn build: Merge r365532..
gn build: Merge r365532.
Tue, Jul 9, 3:40 PM
pcc committed rL365570: gn build: Merge r365541..
gn build: Merge r365541.
Tue, Jul 9, 3:40 PM