[RFC PATCH 10/10] arm: migrate away from sections.c

Sam Edwards
2023-05-21T12:35:40Z

This patch effectively reverts 3ebd1cbc49f0005092d69cf0d9a6e64d7a1c300b.

The approach taken in that commit was to have the section-marking
symbols generated into empty sections by the compiler, for the linker
script to include at the correct location. The rationale was that at
the time, the linker considered linker-assigned symbols to be dynamic
when they were in PIC (PIEs or shared libraries), which meant they were
represented at runtime by a R_ARM_ABS32 relocation (by symbol name)
rather than by M_ARM_RELATIVE.

That commit landed in March 2013, but GNU ld later changed its behavior
on 2016-02-23 to default linker-assigned symbols to dynamic only in
shared libraries (not PIE), so this approach is unnecessary.

I am removing it, because:
1) It required keeping sections.c in sync with multiple linker scripts.
2) It added complexity to the linker scripts, making them less readable.
3) It added unnecessary sections to the output, which can't be merged
because the sections are sometimes of different types.
4) The linker may insert sections not explicitly named in the script
somewhere between explicit sections; having the marker symbols
outside of the sections they were marking meant the markers could
end up with an unintended section inserted within that region.

Signed-off-by: Sam Edwards <CFSworks@gmail.com>
Cc: Albert ARIBAUD <albert.u.boot@aribaud.net>

---

arch/arm/cpu/armv8/spl_data.c | 4 +-
arch/arm/cpu/armv8/u-boot-spl.lds | 26 +++----
arch/arm/cpu/armv8/u-boot.lds | 48 ++++---------
arch/arm/cpu/u-boot.lds | 88 +++++-------------------
arch/arm/lib/Makefile | 2 -
arch/arm/lib/sections.c | 36 ----------
arch/arm/mach-rockchip/u-boot-tpl-v8.lds | 26 ++-----
arch/arm/mach-zynq/u-boot.lds | 73 ++++----------------
8 files changed, 59 insertions(+), 244 deletions(-)
delete mode 100644 arch/arm/lib/sections.c

diff --git a/arch/arm/cpu/armv8/spl_data.c b/arch/arm/cpu/armv8/spl_data.c
index 8f1231c86e..017b117183 100644
--- a/arch/arm/cpu/armv8/spl_data.c
+++ b/arch/arm/cpu/armv8/spl_data.c
@@ -6,8 +6,8 @@
#include <common.h>
#include <spl.h>

-char __data_save_start[0] __section(".__data_save_start");
-char __data_save_end[0] __section(".__data_save_end");
+extern char __data_save_start[0];
+extern char __data_save_end[0];

u32 cold_reboot_flag = 1;

diff --git a/arch/arm/cpu/armv8/u-boot-spl.lds b/arch/arm/cpu/armv8/u-boot-spl.lds
index 7cb9d73124..137113a823 100644
--- a/arch/arm/cpu/armv8/u-boot-spl.lds
+++ b/arch/arm/cpu/armv8/u-boot-spl.lds
@@ -40,9 +40,9 @@ SECTIONS

#ifdef CONFIG_SPL_RECOVER_DATA_SECTION
.data_save : {
- *(.__data_save_start)
+ __data_save_start = .;
. = SIZEOF(.data);
- *(.__data_save_end)
+ __data_save_end = .;
} >.sram
#endif

@@ -51,30 +51,20 @@ SECTIONS
KEEP(*(SORT(__u_boot_list*)));
} >.sram

- .image_copy_end : {
- . = ALIGN(8);
- *(.__image_copy_end)
- } >.sram
+ . = ALIGN(8);

- .end : {
- . = ALIGN(8);
- *(.__end)
- } >.sram
+ __image_copy_end = .;
+ _end = .;

_image_binary_end = .;

- .bss_start (NOLOAD) : {
- . = ALIGN(8);
- KEEP(*(.__bss_start));
- } >.sdram
+ . = ALIGN(8);

.bss (NOLOAD) : {
+ __bss_start = .;
*(.bss*)
. = ALIGN(8);
- } >.sdram
-
- .bss_end (NOLOAD) : {
- KEEP(*(.__bss_end));
+ __bss_end = .;
} >.sdram

/DISCARD/ : { *(.rela*) }
diff --git a/arch/arm/cpu/armv8/u-boot.lds b/arch/arm/cpu/armv8/u-boot.lds
index fb6a30c922..c14c3365a8 100644
--- a/arch/arm/cpu/armv8/u-boot.lds
+++ b/arch/arm/cpu/armv8/u-boot.lds
@@ -23,7 +23,7 @@ SECTIONS
. = ALIGN(8);
.text :
{
- *(.__image_copy_start)
+ __image_copy_start = .;
CPUDIR/start.o (.text*)
}

@@ -42,13 +42,9 @@ SECTIONS
}

#ifdef CONFIG_ARMV8_PSCI
- .__secure_start :
#ifndef CONFIG_ARMV8_SECURE_BASE
- ALIGN(CONSTANT(COMMONPAGESIZE))
+ . = ALIGN(CONSTANT(COMMONPAGESIZE));
#endif
- {
- KEEP(*(.__secure_start))
- }

#ifndef CONFIG_ARMV8_SECURE_BASE
#define __ARMV8_SECURE_BASE
@@ -57,8 +53,8 @@ SECTIONS
#define __ARMV8_SECURE_BASE CONFIG_ARMV8_SECURE_BASE
#endif
.secure_text __ARMV8_SECURE_BASE :
- AT(ADDR(.__secure_start) + SIZEOF(.__secure_start))
{
+ __secure_start = .;
*(._secure.text)
. = ALIGN(8);
__secure_svc_tbl_start = .;
@@ -79,23 +75,20 @@ SECTIONS
AT(LOADADDR(.secure_data) + SIZEOF(.secure_data))
#endif
{
- KEEP(*(.__secure_stack_start))
+ __secure_stack_start = .;

. = . + CONFIG_ARMV8_PSCI_NR_CPUS * ARM_PSCI_STACK_SIZE;

. = ALIGN(CONSTANT(COMMONPAGESIZE));

- KEEP(*(.__secure_stack_end))
+ __secure_stack_end = .;
}

#ifndef __ARMV8_PSCI_STACK_IN_RAM
. = LOADADDR(.secure_stack);
#endif

- .__secure_end : AT(ADDR(.__secure_end)) {
- KEEP(*(.__secure_end))
- LONG(0x1d1071c); /* Must output something to reset LMA */
- }
+ __secure_end = .;
#endif

. = ALIGN(8);
@@ -126,42 +119,25 @@ SECTIONS

. = ALIGN(8);

- .image_copy_end :
- {
- *(.__image_copy_end)
- }
+ __image_copy_end = .;

. = ALIGN(8);

- .rel_dyn_start :
- {
- *(.__rel_dyn_start)
- }
-
.rela.dyn : {
+ __rel_dyn_start = .;
*(.rela*)
- }
-
- .rel_dyn_end :
- {
- *(.__rel_dyn_end)
+ __rel_dyn_end = .;
}

_end = .;

. = ALIGN(8);

- .bss_start : {
- KEEP(*(.__bss_start));
- }
-
.bss : {
+ __bss_start = .;
*(.bss*)
- . = ALIGN(8);
- }
-
- .bss_end : {
- KEEP(*(.__bss_end));
+ . = ALIGN(8);
+ __bss_end = .;
}

/DISCARD/ : { *(.dynsym) }
diff --git a/arch/arm/cpu/u-boot.lds b/arch/arm/cpu/u-boot.lds
index bd4650bd86..f0e58f4d72 100644
--- a/arch/arm/cpu/u-boot.lds
+++ b/arch/arm/cpu/u-boot.lds
@@ -40,24 +40,18 @@ SECTIONS
. = ALIGN(4);
.text :
{
- *(.__image_copy_start)
+ __image_copy_start = .;
*(.vectors)
CPUDIR/start.o (.text*)
}

/* This needs to come before *(.text*) */
- .__efi_runtime_start : {
- *(.__efi_runtime_start)
- }
-
.efi_runtime : {
+ __efi_runtime_start = .;
*(.text.efi_runtime*)
*(.rodata.efi_runtime*)
*(.data.efi_runtime*)
- }
-
- .__efi_runtime_stop : {
- *(.__efi_runtime_stop)
+ __efi_runtime_stop = .;
}

.text_rest :
@@ -68,13 +62,9 @@ SECTIONS
#ifdef CONFIG_ARMV7_NONSEC

/* Align the secure section only if we're going to use it in situ */
- .__secure_start
#ifndef CONFIG_ARMV7_SECURE_BASE
- ALIGN(CONSTANT(COMMONPAGESIZE))
+ . = ALIGN(CONSTANT(COMMONPAGESIZE));
#endif
- : {
- KEEP(*(.__secure_start))
- }

#ifndef CONFIG_ARMV7_SECURE_BASE
#define __ARMV7_SECURE_BASE
@@ -84,8 +74,8 @@ SECTIONS
#endif

.secure_text __ARMV7_SECURE_BASE :
- AT(ADDR(.__secure_start) + SIZEOF(.__secure_start))
{
+ __secure_start = .;
*(._secure.text)
}

@@ -103,7 +93,7 @@ SECTIONS
AT(LOADADDR(.secure_data) + SIZEOF(.secure_data))
#endif
{
- KEEP(*(.__secure_stack_start))
+ __secure_stack_start = .;

/* Skip addreses for stack */
. = . + CONFIG_ARMV7_PSCI_NR_CPUS * ARM_PSCI_STACK_SIZE;
@@ -111,7 +101,7 @@ SECTIONS
/* Align end of stack section to page boundary */
. = ALIGN(CONSTANT(COMMONPAGESIZE));

- KEEP(*(.__secure_stack_end))
+ __secure_stack_end = .;

#ifdef CONFIG_ARMV7_SECURE_MAX_SIZE
/*
@@ -133,7 +123,7 @@ SECTIONS
#endif

.__secure_end : AT(ADDR(.__secure_end)) {
- *(.__secure_end)
+ __secure_end = .;
LONG(0x1d1071c); /* Must output something to reset LMA */
}
#endif
@@ -146,58 +136,29 @@ SECTIONS
*(.data*)
}

- . = ALIGN(4);
-
- . = .;
-
. = ALIGN(4);
__u_boot_list : {
KEEP(*(SORT(__u_boot_list*)));
}

. = ALIGN(4);
-
- .efi_runtime_rel_start :
- {
- *(.__efi_runtime_rel_start)
- }
-
.efi_runtime_rel : {
+ __efi_runtime_rel_start = .;
*(.rel*.efi_runtime)
*(.rel*.efi_runtime.*)
- }
-
- .efi_runtime_rel_stop :
- {
- *(.__efi_runtime_rel_stop)
+ __efi_runtime_rel_stop = .;
}

. = ALIGN(4);
-
- .image_copy_end :
- {
- *(.__image_copy_end)
- }
-
- .rel_dyn_start :
- {
- *(.__rel_dyn_start)
- }
+ __image_copy_end = .;

.rel.dyn : {
+ __rel_dyn_start = .;
*(.rel*)
+ __rel_dyn_end = .;
}

- .rel_dyn_end :
- {
- *(.__rel_dyn_end)
- }
-
- .end :
- {
- *(.__end)
- }
-
+ _end = .;
_image_binary_end = .;

/*
@@ -209,24 +170,11 @@ SECTIONS
*(.mmutable)
}

-/*
- * Compiler-generated __bss_start and __bss_end, see arch/arm/lib/bss.c
- * __bss_base and __bss_limit are for linker only (overlay ordering)
- */
-
- .bss_start __rel_dyn_start (OVERLAY) : {
- KEEP(*(.__bss_start));
- __bss_base = .;
- }
-
- .bss __bss_base (OVERLAY) : {
+ .bss : {
+ __bss_start = .;
*(.bss*)
- . = ALIGN(4);
- __bss_limit = .;
- }
-
- .bss_end __bss_limit (OVERLAY) : {
- KEEP(*(.__bss_end));
+ . = ALIGN(4);
+ __bss_end = .;
}

/*
diff --git a/arch/arm/lib/Makefile b/arch/arm/lib/Makefile
index d65dc33f5b..c77e2c1f4b 100644
--- a/arch/arm/lib/Makefile
+++ b/arch/arm/lib/Makefile
@@ -57,8 +57,6 @@ endif

# obj-$(CONFIG_SAVE_PREV_BL_INITRAMFS_START_ADDR) += save_prev_bl_data.o
obj-y += bdinfo.o
-obj-y += sections.o
-CFLAGS_REMOVE_sections.o := $(LTO_CFLAGS)

obj-y += stack.o
ifdef CONFIG_CPU_V7M
diff --git a/arch/arm/lib/sections.c b/arch/arm/lib/sections.c
deleted file mode 100644
index 857879711c..0000000000
--- a/arch/arm/lib/sections.c
+++ /dev/null
@@ -1,36 +0,0 @@
-// SPDX-License-Identifier: GPL-2.0+
-/*
- * Copyright 2013 Albert ARIBAUD <albert.u.boot@aribaud.net>
- */
-#include <linux/compiler.h>
-
-/**
- * These two symbols are declared in a C file so that the linker
- * uses R_ARM_RELATIVE relocation, rather than the R_ARM_ABS32 one
- * it would use if the symbols were defined in the linker file.
- * Using only R_ARM_RELATIVE relocation ensures that references to
- * the symbols are correct after as well as before relocation.
- *
- * We need a 0-byte-size type for these symbols, and the compiler
- * does not allow defining objects of C type 'void'. Using an empty
- * struct is allowed by the compiler, but causes gcc versions 4.4 and
- * below to complain about aliasing. Therefore we use the next best
- * thing: zero-sized arrays, which are both 0-byte-size and exempt from
- * aliasing warnings.
- */
-
-char __bss_start[0] __section(".__bss_start");
-char __bss_end[0] __section(".__bss_end");
-char __image_copy_start[0] __section(".__image_copy_start");
-char __image_copy_end[0] __section(".__image_copy_end");
-char __rel_dyn_start[0] __section(".__rel_dyn_start");
-char __rel_dyn_end[0] __section(".__rel_dyn_end");
-char __secure_start[0] __section(".__secure_start");
-char __secure_end[0] __section(".__secure_end");
-char __secure_stack_start[0] __section(".__secure_stack_start");
-char __secure_stack_end[0] __section(".__secure_stack_end");
-char __efi_runtime_start[0] __section(".__efi_runtime_start");
-char __efi_runtime_stop[0] __section(".__efi_runtime_stop");
-char __efi_runtime_rel_start[0] __section(".__efi_runtime_rel_start");
-char __efi_runtime_rel_stop[0] __section(".__efi_runtime_rel_stop");
-char _end[0] __section(".__end");
diff --git a/arch/arm/mach-rockchip/u-boot-tpl-v8.lds b/arch/arm/mach-rockchip/u-boot-tpl-v8.lds
index 74618eba59..0cd6730533 100644
--- a/arch/arm/mach-rockchip/u-boot-tpl-v8.lds
+++ b/arch/arm/mach-rockchip/u-boot-tpl-v8.lds
@@ -24,7 +24,7 @@ SECTIONS

.text : {
. = ALIGN(8);
- *(.__image_copy_start)
+ __image_copy_start = .;
CPUDIR/start.o (.text*)
*(.text*)
}
@@ -44,30 +44,16 @@ SECTIONS
KEEP(*(SORT(__u_boot_list*)));
}

- .image_copy_end : {
- . = ALIGN(8);
- *(.__image_copy_end)
- }
-
- .end : {
- . = ALIGN(8);
- *(.__end)
- }
+ __image_copy_end = .;
+ _end = .;

_image_binary_end = .;

- .bss_start (NOLOAD) : {
- . = ALIGN(8);
- KEEP(*(.__bss_start));
- }
-
.bss (NOLOAD) : {
+ __bss_start = .;
*(.bss*)
- . = ALIGN(8);
- }
-
- .bss_end (NOLOAD) : {
- KEEP(*(.__bss_end));
+ . = ALIGN(8);
+ __bss_end = .;
}

/DISCARD/ : { *(.dynsym) }
diff --git a/arch/arm/mach-zynq/u-boot.lds b/arch/arm/mach-zynq/u-boot.lds
index 3b7c9d515f..41d0b9471c 100644
--- a/arch/arm/mach-zynq/u-boot.lds
+++ b/arch/arm/mach-zynq/u-boot.lds
@@ -16,24 +16,18 @@ SECTIONS
. = ALIGN(4);
.text :
{
- *(.__image_copy_start)
+ __image_copy_start = .;
*(.vectors)
CPUDIR/start.o (.text*)
}

/* This needs to come before *(.text*) */
- .__efi_runtime_start : {
- *(.__efi_runtime_start)
- }
-
.efi_runtime : {
+ __efi_runtime_start = .;
*(.text.efi_runtime*)
*(.rodata.efi_runtime*)
*(.data.efi_runtime*)
- }
-
- .__efi_runtime_stop : {
- *(.__efi_runtime_stop)
+ __efi_runtime_stop = .;
}

.text_rest :
@@ -49,77 +43,36 @@ SECTIONS
*(.data*)
}

- . = ALIGN(4);
-
- . = .;
-
. = ALIGN(4);
__u_boot_list : {
KEEP(*(SORT(__u_boot_list*)));
}

. = ALIGN(4);
-
- .efi_runtime_rel_start :
- {
- *(.__efi_runtime_rel_start)
- }
-
.efi_runtime_rel : {
+ __efi_runtime_rel_start = .;
*(.rel*.efi_runtime)
*(.rel*.efi_runtime.*)
- }
-
- .efi_runtime_rel_stop :
- {
- *(.__efi_runtime_rel_stop)
+ __efi_runtime_rel_stop = .;
}

. = ALIGN(8);
- .image_copy_end :
- {
- *(.__image_copy_end)
- }
-
- .rel_dyn_start :
- {
- *(.__rel_dyn_start)
- }
+ __image_copy_end = .;

.rel.dyn : {
+ __rel_dyn_start = .;
*(.rel*)
+ __rel_dyn_end = .;
}

- .rel_dyn_end :
- {
- *(.__rel_dyn_end)
- }
-
- .end :
- {
- *(.__end)
- }
-
+ _end = .;
_image_binary_end = .;

-/*
- * Compiler-generated __bss_start and __bss_end, see arch/arm/lib/bss.c
- * __bss_base and __bss_limit are for linker only (overlay ordering)
- */
-
- .bss_start __rel_dyn_start (OVERLAY) : {
- KEEP(*(.__bss_start));
- __bss_base = .;
- }
-
- .bss __bss_base (OVERLAY) : {
+ .bss : {
+ __bss_start = .;
*(.bss*)
- . = ALIGN(8);
- __bss_limit = .;
- }
-
- .bss_end __bss_limit (OVERLAY) : {
- KEEP(*(.__bss_end));
+ . = ALIGN(8);
+ __bss_end = .;
}

/*
--
2.39.2