-
Notifications
You must be signed in to change notification settings - Fork 14.3k
[LLD][ELF][AArch64] Discard .ARM.attributes sections #125838
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
LLVM has started to emit AArch64 build attributes sections called .ARM.attributes. LLD does not yet have support for these so they are accumulating in the ELF output. As the first part of that support discard all the .ARM.attributes sections. This can be built upon by the full implementation in LLD. The build attributes specification only defines build attributes for relocatable objects. The intention for LLD is that files of type ET_EXEC and ET_SHARED will not have a build attributes in the output. A relocatable link with -r will need a merged build attributes, but until the merge is implemented it is better to discard.
@llvm/pr-subscribers-lld @llvm/pr-subscribers-lld-elf Author: Peter Smith (smithp35) ChangesLLVM has started to emit AArch64 build attributes sections called .ARM.attributes. LLD does not yet have support for these so they are accumulating in the ELF output. As the first part of that support discard all the .ARM.attributes sections. This can be built upon by the full implementation in LLD. The build attributes specification only defines build attributes for relocatable objects. The intention for LLD is that files of type ET_EXEC and ET_SHARED will not have a build attributes in the output. A relocatable link with -r will need a merged build attributes, but until the merge is implemented it is better to discard. Full diff: https://github.com/llvm/llvm-project/pull/125838.diff 2 Files Affected:
diff --git a/lld/ELF/InputFiles.cpp b/lld/ELF/InputFiles.cpp
index 16943c484d96bc6..d43de8ce6dfef83 100644
--- a/lld/ELF/InputFiles.cpp
+++ b/lld/ELF/InputFiles.cpp
@@ -639,6 +639,13 @@ template <class ELFT> void ObjFile<ELFT>::parse(bool ignoreComdats) {
}
break;
case EM_AARCH64:
+ // FIXME: BuildAttributes have been implemented in llvm, but not yet in
+ // lld. Remove the section so that it does not accumulate in the output
+ // file. When support is implemented we expect not to output a build
+ // attributes section in files of type ET_EXEC or ET_SHARED, but ld -r
+ // ouptut will need a single merged attributes section.
+ if (sec.sh_type == SHT_AARCH64_ATTRIBUTES)
+ sections[i] = &InputSection::discarded;
// Producing a static binary with MTE globals is not currently supported,
// remove all SHT_AARCH64_MEMTAG_GLOBALS_STATIC sections as they're unused
// medatada, and we don't want them to end up in the output file for
diff --git a/lld/test/ELF/aarch64-build-attributes-output.s b/lld/test/ELF/aarch64-build-attributes-output.s
new file mode 100644
index 000000000000000..0834343dd43a986
--- /dev/null
+++ b/lld/test/ELF/aarch64-build-attributes-output.s
@@ -0,0 +1,26 @@
+// REQUIRES: aarch64
+// RUN: llvm-mc -triple=aarch64 %s -filetype=obj -o %t.o
+// RUN: ld.lld %t.o --shared -o %t.so
+// RUN: llvm-readelf --sections %t.so | FileCheck %s
+// RUN: ld.lld %t.o -o %t
+// RUN: llvm-readelf --sections %t | FileCheck %s
+// RUN: ld.lld -r %t.o -o %t2.o
+// RUN: llvm-readelf --sections %t2.o | FileCheck %s
+
+/// File has a Build attributes section. This should not appear in
+/// ET_EXEC or ET_SHARED files as there is no requirement for it to
+/// do so. FIXME, the ld -r (relocatable link) should output a single
+/// merged build attributes section. WHen full support is added in
+/// ld.lld this test should be updated.
+
+// CHECK-NOT: .ARM.attributes
+
+.aeabi_subsection aeabi_feature_and_bits, optional, uleb128
+.aeabi_attribute Tag_Feature_BTI, 1
+.aeabi_attribute Tag_Feature_PAC, 1
+.aeabi_attribute Tag_Feature_GCS, 1
+
+.global _start
+.type _start, %function
+_start:
+ret
|
/// File has a Build attributes section. This should not appear in | ||
/// ET_EXEC or ET_SHARED files as there is no requirement for it to | ||
/// do so. FIXME, the ld -r (relocatable link) should output a single | ||
/// merged build attributes section. WHen full support is added in |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
typo 'When'
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
My preference would be to revert the breaking change and find the proper solution (fix all the linkers, apply shf_exclude, put behind a flag..) without time pressure. Especially since it landed before the branch point.
But if you think this is better, please land it so at least lld users get fixed :)
FWIW I've just checked GNU ld. It does already have some code for SHT_AARCH64_ATTRIBUTES as the section name has been reserved since the initial AArch64 ABI. This has the same effect as the lld patch above. So I think that leaves us with people building clang alone, and then using an older lld. I'm hopeful that I can add SHF_EXCLUDE to the spec so that we can make sure old LLD does not accumulate the sections. |
WHen should be When.
I'll wait to the end of the UK day to see if @MaskRay has any input. Otherwise I'll land it and fix up any suggestions post-commit (with apologies). |
Add a link #123990 ? |
Will do, thanks for the suggestion. |
@@ -0,0 +1,26 @@ | |||
// REQUIRES: aarch64 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Perhaps omit -output
from the name.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ACK.
Rename test/ELF/aarch64-build-attributes-output.s as test/ELF/aarch64-build-attributes.s
Thanks, was waiting for the buildkite to come back, but has been waiting for Linux for over an hour |
/cherry-pick ba476d0 |
/pull-request #126065 |
LLVM has started to emit AArch64 build attributes sections called .ARM.attributes. LLD does not yet have support for these so they are accumulating in the ELF output. As the first part of that support discard all the .ARM.attributes sections. This can be built upon by the full implementation in LLD. The build attributes specification only defines build attributes for relocatable objects. The intention for LLD is that files of type ET_EXEC and ET_SHARED will not have a build attributes in the output. A relocatable link with -r will need a merged build attributes, but until the merge is implemented it is better to discard. (cherry picked from commit ba476d0)
LLVM has started to emit AArch64 build attributes sections called .ARM.attributes. LLD does not yet have support for these so they are accumulating in the ELF output. As the first part of that support discard all the .ARM.attributes sections. This can be built upon by the full implementation in LLD. The build attributes specification only defines build attributes for relocatable objects. The intention for LLD is that files of type ET_EXEC and ET_SHARED will not have a build attributes in the output. A relocatable link with -r will need a merged build attributes, but until the merge is implemented it is better to discard.
LLVM has started to emit AArch64 build attributes sections called .ARM.attributes. LLD does not yet have support for these so they are accumulating in the ELF output. As the first part of that support discard all the .ARM.attributes sections. This can be built upon by the full implementation in LLD.
The build attributes specification only defines build attributes for relocatable objects. The intention for LLD is that files of type ET_EXEC and ET_SHARED will not have a build attributes in the output. A relocatable link with -r will need a merged build attributes, but until the merge is implemented it is better to discard.