Skip to content

[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

Merged
merged 3 commits into from
Feb 5, 2025

Conversation

smithp35
Copy link
Collaborator

@smithp35 smithp35 commented Feb 5, 2025

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.
@llvmbot
Copy link
Member

llvmbot commented Feb 5, 2025

@llvm/pr-subscribers-lld

@llvm/pr-subscribers-lld-elf

Author: Peter Smith (smithp35)

Changes

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.


Full diff: https://github.com/llvm/llvm-project/pull/125838.diff

2 Files Affected:

  • (modified) lld/ELF/InputFiles.cpp (+7)
  • (added) lld/test/ELF/aarch64-build-attributes-output.s (+26)
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
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

typo 'When'

Copy link
Contributor

@nico nico left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks!

Copy link
Collaborator

@zmodem zmodem left a 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 :)

@smithp35
Copy link
Collaborator Author

smithp35 commented Feb 5, 2025

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.
https://github.com/bminor/binutils-gdb/blob/master/bfd/elfnn-aarch64.c#L8383

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.
@smithp35
Copy link
Collaborator Author

smithp35 commented Feb 5, 2025

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).

@MaskRay
Copy link
Member

MaskRay commented Feb 5, 2025

LLVM has started to emit AArch64 build attributes sections called .ARM.attributes.

Add a link #123990 ?

@smithp35
Copy link
Collaborator Author

smithp35 commented Feb 5, 2025

LLVM has started to emit AArch64 build attributes sections called .ARM.attributes.

Add a link #123990 ?

Will do, thanks for the suggestion.

@@ -0,0 +1,26 @@
// REQUIRES: aarch64
Copy link
Member

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.

Copy link
Collaborator Author

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
@nico nico merged commit ba476d0 into llvm:main Feb 5, 2025
6 of 7 checks passed
@smithp35
Copy link
Collaborator Author

smithp35 commented Feb 5, 2025

Thanks, was waiting for the buildkite to come back, but has been waiting for Linux for over an hour

@zmodem zmodem added this to the LLVM 20.X Release milestone Feb 6, 2025
@zmodem
Copy link
Collaborator

zmodem commented Feb 6, 2025

/cherry-pick ba476d0

@llvmbot
Copy link
Member

llvmbot commented Feb 6, 2025

/pull-request #126065

swift-ci pushed a commit to swiftlang/llvm-project that referenced this pull request Feb 8, 2025
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)
Icohedron pushed a commit to Icohedron/llvm-project that referenced this pull request Feb 11, 2025
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.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Development

Successfully merging this pull request may close these issues.

6 participants