diff mbox series

[03/11] s390: vmlinux.lds.S: Explicitly handle '.got' and '.plt' sections

Message ID 20240207-s390-lld-and-orphan-warn-v1-3-8a665b3346ab@kernel.org
State New
Headers
Series [01/11] s390: boot: Add support for CONFIG_LD_ORPHAN_WARN |

Commit Message

Nathan Chancellor Feb. 8, 2024, 12:14 a.m. UTC 
  When building with CONFIG_LD_ORPHAN_WARN after selecting
CONFIG_ARCH_HAS_LD_ORPHAN_WARN, there are a lot of warnings around the
GOT and PLT sections:

  s390-linux-ld: warning: orphan section `.plt' from `arch/s390/kernel/head64.o' being placed in section `.plt'
  s390-linux-ld: warning: orphan section `.got' from `arch/s390/kernel/head64.o' being placed in section `.got'
  s390-linux-ld: warning: orphan section `.got.plt' from `arch/s390/kernel/head64.o' being placed in section `.got.plt'
  s390-linux-ld: warning: orphan section `.iplt' from `arch/s390/kernel/head64.o' being placed in section `.iplt'
  s390-linux-ld: warning: orphan section `.igot.plt' from `arch/s390/kernel/head64.o' being placed in section `.igot.plt'

  s390-linux-ld: warning: orphan section `.iplt' from `arch/s390/boot/head.o' being placed in section `.iplt'
  s390-linux-ld: warning: orphan section `.igot.plt' from `arch/s390/boot/head.o' being placed in section `.igot.plt'
  s390-linux-ld: warning: orphan section `.got' from `arch/s390/boot/head.o' being placed in section `.got'

Currently, only the '.got' section is actually emitted in the final
binary. In a manner similar to other architectures, put the '.got'
section near the '.data' section and coalesce the PLT sections,
checking that the final section is zero sized, which is a safe/tested
approach versus full discard.

Signed-off-by: Nathan Chancellor <nathan@kernel.org>
---
 arch/s390/boot/vmlinux.lds.S   | 19 +++++++++++++++++++
 arch/s390/kernel/vmlinux.lds.S | 19 +++++++++++++++++++
 2 files changed, 38 insertions(+)

Comments

Heiko Carstens Feb. 14, 2024, 12:20 p.m. UTC | #1 
On Mon, Feb 12, 2024 at 09:31:53PM -0800, Fangrui Song wrote:
> On Wed, Feb 7, 2024 at 4:15 PM Nathan Chancellor <nathan@kernel.org> wrote:
> > +       ASSERT(SIZEOF(.got.plt) == 0, "Unexpected GOT/PLT entries detected!")
> > +       .plt : {
> > +               *(.plt)
> > +               *(.plt.*)
> > +               *(.iplt)
> > +               *(.igot .igot.plt)
> > +       }
> > +       ASSERT(SIZEOF(.plt) == 0, "Unexpected run-time procedure linkages detected!")
> > +
> 
> It seems that arches that drop .plt typically place input section
> description on one line. This saves vertical space.
> It's shorter to use one input section description to match multiple
> sections, e.g.
> 
>     *(.plt .iplt)

Yes, I'll change Nathan's patch so it looks like arm64:

	/*
	 * Sections that should stay zero sized, which is safer to
	 * explicitly check instead of blindly discarding.
	 */
	.plt : {
		*(.plt) *(.plt.*) *(.iplt) *(.igot .igot.plt)
	}
Nathan Chancellor Feb. 14, 2024, 7:48 p.m. UTC | #2 
On Wed, Feb 14, 2024 at 01:20:55PM +0100, Heiko Carstens wrote:
> On Mon, Feb 12, 2024 at 09:31:53PM -0800, Fangrui Song wrote:
> > On Wed, Feb 7, 2024 at 4:15 PM Nathan Chancellor <nathan@kernel.org> wrote:
> > > +       ASSERT(SIZEOF(.got.plt) == 0, "Unexpected GOT/PLT entries detected!")
> > > +       .plt : {
> > > +               *(.plt)
> > > +               *(.plt.*)
> > > +               *(.iplt)
> > > +               *(.igot .igot.plt)
> > > +       }
> > > +       ASSERT(SIZEOF(.plt) == 0, "Unexpected run-time procedure linkages detected!")
> > > +
> > 
> > It seems that arches that drop .plt typically place input section
> > description on one line. This saves vertical space.
> > It's shorter to use one input section description to match multiple
> > sections, e.g.
> > 
> >     *(.plt .iplt)
> 
> Yes, I'll change Nathan's patch so it looks like arm64:
> 
> 	/*
> 	 * Sections that should stay zero sized, which is safer to
> 	 * explicitly check instead of blindly discarding.
> 	 */
> 	.plt : {
> 		*(.plt) *(.plt.*) *(.iplt) *(.igot .igot.plt)
> 	}

Thanks a lot for changing this. I tried to be consistent with the rest
of the linker script but I guess there were not really any sections that
had this many lines to make it one lining it worth it prior to this
point. Stylistic expectations are hard to account for :)

Cheers,
Nathan
diff mbox series

Patch

diff --git a/arch/s390/boot/vmlinux.lds.S b/arch/s390/boot/vmlinux.lds.S
index 389df0e0d9e5..4aa2f340c8d9 100644
--- a/arch/s390/boot/vmlinux.lds.S
+++ b/arch/s390/boot/vmlinux.lds.S
@@ -39,6 +39,9 @@  SECTIONS
 		*(.rodata.*)
 		_erodata = . ;
 	}
+	.got : {
+		*(.got)
+	}
 	NOTES
 	.data :	{
 		_data = . ;
@@ -118,6 +121,22 @@  SECTIONS
 	}
 	_end = .;
 
+	/*
+	 * Sections that should stay zero sized, which is safer to
+	 * explicitly check instead of blindly discarding.
+	 */
+	.got.plt : {
+		*(.got.plt)
+	}
+	ASSERT(SIZEOF(.got.plt) == 0, "Unexpected GOT/PLT entries detected!")
+	.plt : {
+		*(.plt)
+		*(.plt.*)
+		*(.iplt)
+		*(.igot .igot.plt)
+	}
+	ASSERT(SIZEOF(.plt) == 0, "Unexpected run-time procedure linkages detected!")
+
 	/* Sections to be discarded */
 	/DISCARD/ : {
 		*(.eh_frame)
diff --git a/arch/s390/kernel/vmlinux.lds.S b/arch/s390/kernel/vmlinux.lds.S
index d231a3faf981..661a487a3048 100644
--- a/arch/s390/kernel/vmlinux.lds.S
+++ b/arch/s390/kernel/vmlinux.lds.S
@@ -62,6 +62,9 @@  SECTIONS
 	.data.rel.ro : {
 		*(.data.rel.ro .data.rel.ro.*)
 	}
+	.got : {
+		*(.got)
+	}
 
 	. = ALIGN(PAGE_SIZE);
 	_sdata = .;		/* Start of data section */
@@ -241,6 +244,22 @@  SECTIONS
 	DWARF_DEBUG
 	ELF_DETAILS
 
+	/*
+	 * Sections that should stay zero sized, which is safer to
+	 * explicitly check instead of blindly discarding.
+	 */
+	.got.plt : {
+		*(.got.plt)
+	}
+	ASSERT(SIZEOF(.got.plt) == 0, "Unexpected GOT/PLT entries detected!")
+	.plt : {
+		*(.plt)
+		*(.plt.*)
+		*(.iplt)
+		*(.igot .igot.plt)
+	}
+	ASSERT(SIZEOF(.plt) == 0, "Unexpected run-time procedure linkages detected!")
+
 	/* Sections to be discarded */
 	DISCARDS
 	/DISCARD/ : {