Page MenuHomePhabricator
Differential D73255

[ELF][PowerPC] Support R_PPC_COPY and R_PPC64_COPY
ClosedPublic
Actions

Authored by MaskRay on Jan 22 2020, 11:38 PM.

Details

Reviewers
Bdragon28
• espindola
jhenderson
grimar
sfertile
Group Reviewers
Restricted Project
Commits
rG425d15aeb13e: [ELF][PowerPC] Support R_PPC_COPY and R_PPC64_COPY
rGf1dab29908d2: [ELF][PowerPC] Support R_PPC_COPY and R_PPC64_COPY

Diff Detail

Unit TestsFailed
View All

TimeTest
840 mslibc++.std/language_support/cmp/cmp_partialord::Unknown Unit Message ("")
Command: ['/usr/bin/clang++', '-o', '/mnt/disks/ssd0/agent/workspace/amd64_debian_testing_clang8/build/projects/libcxx/test/std/language.support/cmp/cmp.partialord/Output/partialord.pass.cpp.o', '-x', 'c++', '/mnt/disks/ssd0/agent/workspace/amd64_debian_testing_clang8/libcxx/test/std/language.support/cmp/cmp.partialord/partialord.pass.cpp', '-c', '-v', '-ftemplate-depth=270', '-Werror=thread-safety', '-std=c++2a', '-include', '/mnt/disks/ssd0/agent/workspace/amd64_debian_testing_clang8/libcxx/test/support/nasty_macros.h', '-nostdinc++', '-I/mnt/disks/ssd0/agent/workspace/amd64_debian_testing_clang8/libcxx/include', '-I/mnt/disks/ssd0/agent/workspace/amd64_debian_testing_clang8/build/projects/libcxx/include/c++build', '-D__STDC_FORMAT_MACROS', '-D__STDC_LIMIT_MACROS', '-D__STDC_CONSTANT_MACROS', '-I/mnt/disks/ssd0/agent/workspace/amd64_debian_testing_clang8/libcxx/test/support', '-DLIBCXX_FILESYSTEM_STATIC_TEST_ROOT="/mnt/disks/ssd0/agent/workspace/amd64_debian_testing_clang8/libcxx/test/std/input.output/filesystems/Inputs/static_test_env"', '-DLIBCXX_FILESYSTEM_DYNAMIC_TEST_ROOT="/mnt/disks/ssd0/agent/workspace/amd64_debian_testing_clang8/build/projects/libcxx/test/filesystem/Output/dynamic_env"', '-DLIBCXX_FILESYSTEM_DYNAMIC_TEST_HELPER="/usr/bin/python /mnt/disks/ssd0/agent/workspace/amd64_debian_testing_clang8/libcxx/test/support/filesystem_dynamic_test_helper.py"', '-D_LIBCPP_HAS_NO_PRAGMA_SYSTEM_HEADER', '-Wall', '-Wextra', '-Werror', '-Wuser-defined-warnings', '-Wshadow', '-Wno-unused-command-line-argument', '-Wno-attributes', '-Wno-pessimizing-move', '-Wno-c++11-extensions', '-Wno-user-defined-literals', '-Wno-noexcept-type', '-Wsign-compare', '-Wunused-variable', '-Wunused-parameter', '-Wunreachable-code', '-c'] Exit Code: 1 Standard Error:
880 mslibc++.std/language_support/cmp/cmp_strongeq::Unknown Unit Message ("")
Command: ['/usr/bin/clang++', '-o', '/mnt/disks/ssd0/agent/workspace/amd64_debian_testing_clang8/build/projects/libcxx/test/std/language.support/cmp/cmp.strongeq/Output/cmp.strongeq.pass.cpp.o', '-x', 'c++', '/mnt/disks/ssd0/agent/workspace/amd64_debian_testing_clang8/libcxx/test/std/language.support/cmp/cmp.strongeq/cmp.strongeq.pass.cpp', '-c', '-v', '-ftemplate-depth=270', '-Werror=thread-safety', '-std=c++2a', '-include', '/mnt/disks/ssd0/agent/workspace/amd64_debian_testing_clang8/libcxx/test/support/nasty_macros.h', '-nostdinc++', '-I/mnt/disks/ssd0/agent/workspace/amd64_debian_testing_clang8/libcxx/include', '-I/mnt/disks/ssd0/agent/workspace/amd64_debian_testing_clang8/build/projects/libcxx/include/c++build', '-D__STDC_FORMAT_MACROS', '-D__STDC_LIMIT_MACROS', '-D__STDC_CONSTANT_MACROS', '-I/mnt/disks/ssd0/agent/workspace/amd64_debian_testing_clang8/libcxx/test/support', '-DLIBCXX_FILESYSTEM_STATIC_TEST_ROOT="/mnt/disks/ssd0/agent/workspace/amd64_debian_testing_clang8/libcxx/test/std/input.output/filesystems/Inputs/static_test_env"', '-DLIBCXX_FILESYSTEM_DYNAMIC_TEST_ROOT="/mnt/disks/ssd0/agent/workspace/amd64_debian_testing_clang8/build/projects/libcxx/test/filesystem/Output/dynamic_env"', '-DLIBCXX_FILESYSTEM_DYNAMIC_TEST_HELPER="/usr/bin/python /mnt/disks/ssd0/agent/workspace/amd64_debian_testing_clang8/libcxx/test/support/filesystem_dynamic_test_helper.py"', '-D_LIBCPP_HAS_NO_PRAGMA_SYSTEM_HEADER', '-Wall', '-Wextra', '-Werror', '-Wuser-defined-warnings', '-Wshadow', '-Wno-unused-command-line-argument', '-Wno-attributes', '-Wno-pessimizing-move', '-Wno-c++11-extensions', '-Wno-user-defined-literals', '-Wno-noexcept-type', '-Wsign-compare', '-Wunused-variable', '-Wunused-parameter', '-Wunreachable-code', '-c'] Exit Code: 1 Standard Error:
910 mslibc++.std/language_support/cmp/cmp_strongord::Unknown Unit Message ("")
Command: ['/usr/bin/clang++', '-o', '/mnt/disks/ssd0/agent/workspace/amd64_debian_testing_clang8/build/projects/libcxx/test/std/language.support/cmp/cmp.strongord/Output/strongord.pass.cpp.o', '-x', 'c++', '/mnt/disks/ssd0/agent/workspace/amd64_debian_testing_clang8/libcxx/test/std/language.support/cmp/cmp.strongord/strongord.pass.cpp', '-c', '-v', '-ftemplate-depth=270', '-Werror=thread-safety', '-std=c++2a', '-include', '/mnt/disks/ssd0/agent/workspace/amd64_debian_testing_clang8/libcxx/test/support/nasty_macros.h', '-nostdinc++', '-I/mnt/disks/ssd0/agent/workspace/amd64_debian_testing_clang8/libcxx/include', '-I/mnt/disks/ssd0/agent/workspace/amd64_debian_testing_clang8/build/projects/libcxx/include/c++build', '-D__STDC_FORMAT_MACROS', '-D__STDC_LIMIT_MACROS', '-D__STDC_CONSTANT_MACROS', '-I/mnt/disks/ssd0/agent/workspace/amd64_debian_testing_clang8/libcxx/test/support', '-DLIBCXX_FILESYSTEM_STATIC_TEST_ROOT="/mnt/disks/ssd0/agent/workspace/amd64_debian_testing_clang8/libcxx/test/std/input.output/filesystems/Inputs/static_test_env"', '-DLIBCXX_FILESYSTEM_DYNAMIC_TEST_ROOT="/mnt/disks/ssd0/agent/workspace/amd64_debian_testing_clang8/build/projects/libcxx/test/filesystem/Output/dynamic_env"', '-DLIBCXX_FILESYSTEM_DYNAMIC_TEST_HELPER="/usr/bin/python /mnt/disks/ssd0/agent/workspace/amd64_debian_testing_clang8/libcxx/test/support/filesystem_dynamic_test_helper.py"', '-D_LIBCPP_HAS_NO_PRAGMA_SYSTEM_HEADER', '-Wall', '-Wextra', '-Werror', '-Wuser-defined-warnings', '-Wshadow', '-Wno-unused-command-line-argument', '-Wno-attributes', '-Wno-pessimizing-move', '-Wno-c++11-extensions', '-Wno-user-defined-literals', '-Wno-noexcept-type', '-Wsign-compare', '-Wunused-variable', '-Wunused-parameter', '-Wunreachable-code', '-c'] Exit Code: 1 Standard Error:
820 mslibc++.std/language_support/cmp/cmp_weakeq::Unknown Unit Message ("")
Command: ['/usr/bin/clang++', '-o', '/mnt/disks/ssd0/agent/workspace/amd64_debian_testing_clang8/build/projects/libcxx/test/std/language.support/cmp/cmp.weakeq/Output/cmp.weakeq.pass.cpp.o', '-x', 'c++', '/mnt/disks/ssd0/agent/workspace/amd64_debian_testing_clang8/libcxx/test/std/language.support/cmp/cmp.weakeq/cmp.weakeq.pass.cpp', '-c', '-v', '-ftemplate-depth=270', '-Werror=thread-safety', '-std=c++2a', '-include', '/mnt/disks/ssd0/agent/workspace/amd64_debian_testing_clang8/libcxx/test/support/nasty_macros.h', '-nostdinc++', '-I/mnt/disks/ssd0/agent/workspace/amd64_debian_testing_clang8/libcxx/include', '-I/mnt/disks/ssd0/agent/workspace/amd64_debian_testing_clang8/build/projects/libcxx/include/c++build', '-D__STDC_FORMAT_MACROS', '-D__STDC_LIMIT_MACROS', '-D__STDC_CONSTANT_MACROS', '-I/mnt/disks/ssd0/agent/workspace/amd64_debian_testing_clang8/libcxx/test/support', '-DLIBCXX_FILESYSTEM_STATIC_TEST_ROOT="/mnt/disks/ssd0/agent/workspace/amd64_debian_testing_clang8/libcxx/test/std/input.output/filesystems/Inputs/static_test_env"', '-DLIBCXX_FILESYSTEM_DYNAMIC_TEST_ROOT="/mnt/disks/ssd0/agent/workspace/amd64_debian_testing_clang8/build/projects/libcxx/test/filesystem/Output/dynamic_env"', '-DLIBCXX_FILESYSTEM_DYNAMIC_TEST_HELPER="/usr/bin/python /mnt/disks/ssd0/agent/workspace/amd64_debian_testing_clang8/libcxx/test/support/filesystem_dynamic_test_helper.py"', '-D_LIBCPP_HAS_NO_PRAGMA_SYSTEM_HEADER', '-Wall', '-Wextra', '-Werror', '-Wuser-defined-warnings', '-Wshadow', '-Wno-unused-command-line-argument', '-Wno-attributes', '-Wno-pessimizing-move', '-Wno-c++11-extensions', '-Wno-user-defined-literals', '-Wno-noexcept-type', '-Wsign-compare', '-Wunused-variable', '-Wunused-parameter', '-Wunreachable-code', '-c'] Exit Code: 1 Standard Error:
840 mslibc++.std/language_support/cmp/cmp_weakord::Unknown Unit Message ("")
Command: ['/usr/bin/clang++', '-o', '/mnt/disks/ssd0/agent/workspace/amd64_debian_testing_clang8/build/projects/libcxx/test/std/language.support/cmp/cmp.weakord/Output/weakord.pass.cpp.o', '-x', 'c++', '/mnt/disks/ssd0/agent/workspace/amd64_debian_testing_clang8/libcxx/test/std/language.support/cmp/cmp.weakord/weakord.pass.cpp', '-c', '-v', '-ftemplate-depth=270', '-Werror=thread-safety', '-std=c++2a', '-include', '/mnt/disks/ssd0/agent/workspace/amd64_debian_testing_clang8/libcxx/test/support/nasty_macros.h', '-nostdinc++', '-I/mnt/disks/ssd0/agent/workspace/amd64_debian_testing_clang8/libcxx/include', '-I/mnt/disks/ssd0/agent/workspace/amd64_debian_testing_clang8/build/projects/libcxx/include/c++build', '-D__STDC_FORMAT_MACROS', '-D__STDC_LIMIT_MACROS', '-D__STDC_CONSTANT_MACROS', '-I/mnt/disks/ssd0/agent/workspace/amd64_debian_testing_clang8/libcxx/test/support', '-DLIBCXX_FILESYSTEM_STATIC_TEST_ROOT="/mnt/disks/ssd0/agent/workspace/amd64_debian_testing_clang8/libcxx/test/std/input.output/filesystems/Inputs/static_test_env"', '-DLIBCXX_FILESYSTEM_DYNAMIC_TEST_ROOT="/mnt/disks/ssd0/agent/workspace/amd64_debian_testing_clang8/build/projects/libcxx/test/filesystem/Output/dynamic_env"', '-DLIBCXX_FILESYSTEM_DYNAMIC_TEST_HELPER="/usr/bin/python /mnt/disks/ssd0/agent/workspace/amd64_debian_testing_clang8/libcxx/test/support/filesystem_dynamic_test_helper.py"', '-D_LIBCPP_HAS_NO_PRAGMA_SYSTEM_HEADER', '-Wall', '-Wextra', '-Werror', '-Wuser-defined-warnings', '-Wshadow', '-Wno-unused-command-line-argument', '-Wno-attributes', '-Wno-pessimizing-move', '-Wno-c++11-extensions', '-Wno-user-defined-literals', '-Wno-noexcept-type', '-Wsign-compare', '-Wunused-variable', '-Wunused-parameter', '-Wunreachable-code', '-c'] Exit Code: 1 Standard Error:

Event Timeline

MaskRay created this revision.Jan 22 2020, 11:38 PM
Herald added a reviewer: • espindola. · View Herald TranscriptJan 22 2020, 11:38 PM
Herald added a reviewer: jhenderson. · View Herald Transcript
Herald added a project: Restricted Project. · View Herald Transcript
Herald added subscribers: llvm-commits, shchenz, jsji and 5 others. · View Herald Transcript
MaskRay added a reviewer: grimar.Jan 22 2020, 11:39 PM
Herald added a subscriber: • wuzish. · View Herald TranscriptJan 22 2020, 11:39 PM
merge_guards_bot added a subscriber: merge_guards_bot.Jan 22 2020, 11:56 PM

Unit tests: pass. 62087 tests passed, 0 failed and 784 were skipped.

clang-tidy: pass.

clang-format: pass.

Build artifacts: diff.json, clang-tidy.txt, clang-format.patch, CMakeCache.txt, console-log.txt, test-results.xml

Harbormaster completed remote builds in B44671: Diff 239785.Jan 23 2020, 12:04 AM
jhenderson added inline comments.Jan 23 2020, 1:37 AM
lld/test/ELF/ppc64-reloc-copy.s
1 ↗(On Diff #239785)

Does it make sense to fold this and the other test together? They're practically identical...

llvm/include/llvm/BinaryFormat/ELFRelocs/PowerPC64.def
30

I think you need an undef here, right?

MaskRay updated this revision to Diff 239923.Jan 23 2020, 9:18 AM
MaskRay marked 3 inline comments as done.

Address review comments

Bdragon28 added a comment.Jan 23 2020, 9:25 AM

For anyone who is curious about this, copy relocations are especially needed on powerpc32, where they are used to implement global data (such as sharing a pointer between a .so and the main program) in situations where the main program's copy lives in .bss.

Given that FreeBSD libc's API uses this extensively for things such as providing stderr in stdio, this fix is critical on ppc32, as the lack of it prevents anything more complex than /bin/echo from running without crashing due to dereferencing a null pointer.

I am currently running though a powerpc32 buildworld and testing on a G4 so I can see just how much closer that gets us to switching FreeBSD powerpc32 to lld.

On PPC64, the need for copy relocations is much reduced, as generally things go through the TOC in the first place (when using PIC), so it is sufficient to generate an R_PPC64_ADDR64 relocation against the TOC entry, instead of a copy relocation against an offset into .bss.

merge_guards_bot added a comment.Jan 23 2020, 10:23 AM

Unit tests: fail. 62137 tests passed, 5 failed and 811 were skipped.

failed: libc++.std/language_support/cmp/cmp_partialord/partialord.pass.cpp
failed: libc++.std/language_support/cmp/cmp_strongeq/cmp.strongeq.pass.cpp
failed: libc++.std/language_support/cmp/cmp_strongord/strongord.pass.cpp
failed: libc++.std/language_support/cmp/cmp_weakeq/cmp.weakeq.pass.cpp
failed: libc++.std/language_support/cmp/cmp_weakord/weakord.pass.cpp

clang-tidy: pass.

clang-format: pass.

Build artifacts: diff.json, clang-tidy.txt, clang-format.patch, CMakeCache.txt, console-log.txt, test-results.xml

Pre-merge checks is in beta. Report issue. Please join beta or enable it for your project.

Harbormaster failed remote builds in B44734: Diff 239923!Jan 23 2020, 10:28 AM
grimar added a comment.Jan 23 2020, 11:33 PM

Aside from a minor nit I have no more comments, looks fine to me.

lld/test/ELF/ppc-reloc-copy.s
9

It was a bit strange to see t1 again.
I'd suggest to rename t1.o, t1.so, t.o and t to something else.
Perhaps use ppc/ppc64 suffixes somewhere?

MaskRay updated this revision to Diff 240105.Jan 23 2020, 11:49 PM

%t1 -> %t1.32 / %t1.64

MaskRay marked an inline comment as done.Jan 23 2020, 11:49 PM
MaskRay added inline comments.
lld/test/ELF/ppc64-reloc-copy.s
1 ↗(On Diff #239785)

Agree.

Placed in ppc-reloc-copy.s

merge_guards_bot added a comment.Jan 24 2020, 12:05 AM

Unit tests: fail. 62153 tests passed, 5 failed and 811 were skipped.

failed: libc++.std/language_support/cmp/cmp_partialord/partialord.pass.cpp
failed: libc++.std/language_support/cmp/cmp_strongeq/cmp.strongeq.pass.cpp
failed: libc++.std/language_support/cmp/cmp_strongord/strongord.pass.cpp
failed: libc++.std/language_support/cmp/cmp_weakeq/cmp.weakeq.pass.cpp
failed: libc++.std/language_support/cmp/cmp_weakord/weakord.pass.cpp

clang-tidy: fail. clang-tidy found 0 errors and 1 warnings. 0 of them are added as review comments below (why?).

clang-format: pass.

Build artifacts: diff.json, clang-tidy.txt, clang-format.patch, CMakeCache.txt, console-log.txt, test-results.xml

Pre-merge checks is in beta. Report issue. Please join beta or enable it for your project.

Harbormaster failed remote builds in B44811: Diff 240105!Jan 24 2020, 12:05 AM
jhenderson accepted this revision.Jan 24 2020, 1:40 AM

LGTM, but as I'm a bit rusty on some parts of LLD, and know very little about PPC, I can't be particularly confident about the LLD-aspects of this change, so it's probably worth somebody else confirming.

This revision is now accepted and ready to land.Jan 24 2020, 1:40 AM
grimar accepted this revision.Jan 24 2020, 1:44 AM

LGTM. I know nothing about PPC. From LLD code POV this looks fine.

sfertile accepted this revision as: sfertile.Jan 24 2020, 7:15 AM
sfertile added a subscriber: sfertile.

LGTM.

Bdragon28 accepted this revision.Jan 24 2020, 8:26 AM

While I ran into further problems on ppc32, I think it's just because there are further problems on ppc32 (unrelated to this one). At the very least this gets FreeBSD ppc32/lld10 as far as the rescue shell, which is great progress!

Closed by commit rGf1dab29908d2: [ELF][PowerPC] Support R_PPC_COPY and R_PPC64_COPY (authored by MaskRay). · Explain WhyJan 24 2020, 9:08 AM
This revision was automatically updated to reflect the committed changes.
MaskRay added a comment.Jan 24 2020, 9:12 AM

Thanks to everyone for review!

Cherry picked the commit to release/10.x as f1dab29908d25a4044abff6ffc120c48b20f034d

Revision Contents
Changeset List

PathSize
lld/
ELF/
Arch/
1 line
1 line
test/
ELF/
29 lines
llvm/
include/
llvm/
BinaryFormat/
ELFRelocs/
2 lines
test/
tools/
llvm-readobj/
ELF/
3 lines

Diff 239923

View Options

lld/ELF/Arch/PPC.cpp

Loading...
View Options

lld/ELF/Arch/PPC64.cpp

Loading...
View Options

lld/test/ELF/ppc-reloc-copy.s

Loading...
View Options

llvm/include/llvm/BinaryFormat/ELFRelocs/PowerPC64.def

Loading...
View Options

llvm/test/tools/llvm-readobj/ELF/reloc-types-elf-ppc64.test

Loading...