[PATCH] ld - Add SYMBOL_ABI_ALIGNMENT variable and apply to __bss_start

[PATCH] ld - Add SYMBOL_ABI_ALIGNMENT variable and apply to __bss_start

[Andreas Krebbel]

Currently not specific alignment is applied to the __bss_start
symbol. However, on s390x we require every symbol to be on an even
address.  The patch introduces a new variable for the elf.sc template
which allows to specify such an symbol alignment requirement.

ld/
	* scripttempl/elf.sc: New variable SYMBOL_ABI_ALIGNMENT. Apply
	to __bss_start.
	* emulparams/elf_s390.sh: Set SYMBOL_ABI_ALIGNMENT to 2.
	* emulparams/elf64_s390.sh: Likewise.
---
 ld/emulparams/elf64_s390.sh | 1 +
 ld/emulparams/elf_s390.sh   | 1 +
 ld/scripttempl/elf.sc       | 5 ++++-
 3 files changed, 6 insertions(+), 1 deletion(-)

diff --git a/ld/emulparams/elf64_s390.sh b/ld/emulparams/elf64_s390.sh
index 899efd7f532..d0a2a59a092 100644
--- a/ld/emulparams/elf64_s390.sh
+++ b/ld/emulparams/elf64_s390.sh
@@ -17,6 +17,7 @@ EXTRA_EM_FILE=s390
 IREL_IN_PLT=
 SEPARATE_GOTPLT=0
 test -z "$RELRO" && unset SEPARATE_GOTPLT
+SYMBOL_ABI_ALIGNMENT=2
 
 # Treat a host that matches the target with the possible exception of "x"
 # in the name as if it were native.
diff --git a/ld/emulparams/elf_s390.sh b/ld/emulparams/elf_s390.sh
index cb1c6b5b04d..1b2fca3f937 100644
--- a/ld/emulparams/elf_s390.sh
+++ b/ld/emulparams/elf_s390.sh
@@ -12,3 +12,4 @@ GENERATE_SHLIB_SCRIPT=yes
 GENERATE_PIE_SCRIPT=yes
 NO_SMALL_DATA=yes
 IREL_IN_PLT=
+SYMBOL_ABI_ALIGNMENT=2
diff --git a/ld/scripttempl/elf.sc b/ld/scripttempl/elf.sc
index 1e3c5aa8504..59a851fb47d 100644
--- a/ld/scripttempl/elf.sc
+++ b/ld/scripttempl/elf.sc
@@ -78,6 +78,8 @@
 #	USER_LABEL_PREFIX - prefix to add to user-visible symbols.
 #	RODATA_NAME, SDATA_NAME, SBSS_NAME, BSS_NAME - base parts of names
 #		for standard sections, without initial "." or suffixes.
+#       SYMBOL_ABI_ALIGNMENT - minimum alignment in bytes which needs to be
+#               applied to every symbol definition
 #
 # When adding sections, do note that the names of some sections are used
 # when specifying the start address of the next.
@@ -165,6 +167,7 @@ if test -z "$GOT"; then
     GOTPLT=".got.plt      ${RELOCATING-0} : { *(.got.plt)${RELOCATING+ *(.igot.plt)} }"
   fi
 fi
+test -z "${SYMBOL_ABI_ALIGNMENT}" && SYMBOL_ABI_ALIGNMENT=1
 REL_IFUNC=".rel.ifunc    ${RELOCATING-0} : { *(.rel.ifunc) }"
 RELA_IFUNC=".rela.ifunc   ${RELOCATING-0} : { *(.rela.ifunc) }"
 REL_IPLT=".rel.iplt     ${RELOCATING-0} :
@@ -678,7 +681,7 @@ cat <<EOF
   ${DATA_SDATA-${OTHER_SDATA_SECTIONS}}
   ${RELOCATING+${DATA_END_SYMBOLS-${CREATE_SHLIB+PROVIDE (}${USER_LABEL_PREFIX}_edata = .${CREATE_SHLIB+)}; PROVIDE (${USER_LABEL_PREFIX}edata = .);}}
   ${PERSISTENT}
-  ${RELOCATING+. = .;}
+  ${RELOCATING+. = ALIGN(${SYMBOL_ABI_ALIGNMENT});}
   ${RELOCATING+${CREATE_SHLIB+PROVIDE (}${USER_LABEL_PREFIX}__bss_start = .${CREATE_SHLIB+)};}
   ${RELOCATING+${OTHER_BSS_SYMBOLS}}
   ${DATA_SDATA-${SBSS}}
-- 
2.40.1

Re: [PATCH] ld - Add SYMBOL_ABI_ALIGNMENT variable and apply to __bss_start

[Nick Clifton]

Hi Andreas,

> Currently not specific alignment is applied to the __bss_start
> symbol. However, on s390x we require every symbol to be on an even
> address.  The patch introduces a new variable for the elf.sc template
> which allows to specify such an symbol alignment requirement.

This patch looks good, but I see one issue - there are lot more symbols
provided by the elf.sc template than just __bss_start.  So if an ABI
requires symbols to be aligned, then that should apply to all symbols,
right ?

Actually there is one other thing.  Rather than doing:

    ${RELOCATING+. = ALIGN(${SYMBOL_ABI_ALIGNMENT});}
    ${RELOCATING+${CREATE_SHLIB+PROVIDE (}${USER_LABEL_PREFIX}__bss_start = .${CREATE_SHLIB+)};}

Wouldn't it make more sense to do:

    ${RELOCATING+${CREATE_SHLIB+PROVIDE (}${USER_LABEL_PREFIX}__bss_start = ALIGN (.${CREATE_SHLIB+), ${SYMBOL_ABI_ALIGNMENT});}

Ie making it clear that the alignment is being applied to the symbol.

It might even be possible to create a macro to do all of this
USER_LABEL_PREFIX and ALIGN munging, so that it does not have
to be repeated countless times in the elf.sc file.  I have not
actually checked to see if this is possible, but it would be
nice if it could be done.

Cheers
   Nick

Re: [PATCH] ld - Add SYMBOL_ABI_ALIGNMENT variable and apply to __bss_start

[Andreas Krebbel]

On 6/27/23 16:14, Nick Clifton wrote:
> Hi Andreas,
> 
>> Currently not specific alignment is applied to the __bss_start
>> symbol. However, on s390x we require every symbol to be on an even
>> address.  The patch introduces a new variable for the elf.sc template
>> which allows to specify such an symbol alignment requirement.
> 
> This patch looks good, but I see one issue - there are lot more symbols
> provided by the elf.sc template than just __bss_start.  So if an ABI
> requires symbols to be aligned, then that should apply to all symbols,
> right ?

I think most of the other symbols are implicitly aligned because of other effects. The same way it
used to be for __bss_start actually. GCC for Z pads the .data section size to a multiple of 8 but
LLVM doesn't. But you are right. In order to make it more robust we should apply it to the other
symbols as well. I'll have a look.

> Actually there is one other thing.  Rather than doing:
> 
>     ${RELOCATING+. = ALIGN(${SYMBOL_ABI_ALIGNMENT});}
>     ${RELOCATING+${CREATE_SHLIB+PROVIDE (}${USER_LABEL_PREFIX}__bss_start = .${CREATE_SHLIB+)};}
> 
> Wouldn't it make more sense to do:
> 
>     ${RELOCATING+${CREATE_SHLIB+PROVIDE (}${USER_LABEL_PREFIX}__bss_start = ALIGN (.${CREATE_SHLIB+), ${SYMBOL_ABI_ALIGNMENT});}

Yes, that's better.

> Ie making it clear that the alignment is being applied to the symbol.
> 
> It might even be possible to create a macro to do all of this
> USER_LABEL_PREFIX and ALIGN munging, so that it does not have
> to be repeated countless times in the elf.sc file.  I have not
> actually checked to see if this is possible, but it would be
> nice if it could be done.

Agreed. I'm not that familiar with what is possible in these files but I'll try to come up with
something.

Thanks for having a look!

Bye,

Andreas

> 
> Cheers
>    Nick
> 

[PATCH] Align linkerscript symbols according to ABI

[Andreas Krebbel]

Apply ABI specific alignment to symbols generated in the default
linker script.

No regressions on x86 and s390x. Linkerscript on x86 does not change
with that.

Ok for mainline?

Bye,

Andreas


--- ld/emulparams/elf64_s390.sh | 1 +
ld/emulparams/elf_s390.sh | 1 + ld/scripttempl/elf.sc | 50
+++++++++++++++++++++++-------------- 3 files changed, 33
insertions(+), 19 deletions(-)

diff --git a/ld/emulparams/elf64_s390.sh b/ld/emulparams/elf64_s390.sh
index 899efd7f532..d0a2a59a092 100644
--- a/ld/emulparams/elf64_s390.sh
+++ b/ld/emulparams/elf64_s390.sh
@@ -17,6 +17,7 @@ EXTRA_EM_FILE=s390
 IREL_IN_PLT=
 SEPARATE_GOTPLT=0
 test -z "$RELRO" && unset SEPARATE_GOTPLT
+SYMBOL_ABI_ALIGNMENT=2
 
 # Treat a host that matches the target with the possible exception of "x"
 # in the name as if it were native.
diff --git a/ld/emulparams/elf_s390.sh b/ld/emulparams/elf_s390.sh
index cb1c6b5b04d..1b2fca3f937 100644
--- a/ld/emulparams/elf_s390.sh
+++ b/ld/emulparams/elf_s390.sh
@@ -12,3 +12,4 @@ GENERATE_SHLIB_SCRIPT=yes
 GENERATE_PIE_SCRIPT=yes
 NO_SMALL_DATA=yes
 IREL_IN_PLT=
+SYMBOL_ABI_ALIGNMENT=2
diff --git a/ld/scripttempl/elf.sc b/ld/scripttempl/elf.sc
index 1e3c5aa8504..9e95e6b4162 100644
--- a/ld/scripttempl/elf.sc
+++ b/ld/scripttempl/elf.sc
@@ -78,6 +78,8 @@
 #	USER_LABEL_PREFIX - prefix to add to user-visible symbols.
 #	RODATA_NAME, SDATA_NAME, SBSS_NAME, BSS_NAME - base parts of names
 #		for standard sections, without initial "." or suffixes.
+#       SYMBOL_ABI_ALIGNMENT - minimum alignment in bytes which needs to be
+#               applied to every symbol definition
 #
 # When adding sections, do note that the names of some sections are used
 # when specifying the start address of the next.
@@ -165,19 +167,29 @@ if test -z "$GOT"; then
     GOTPLT=".got.plt      ${RELOCATING-0} : { *(.got.plt)${RELOCATING+ *(.igot.plt)} }"
   fi
 fi
+
+def_symbol()
+{
+    if [ -z "${SYMBOL_ABI_ALIGNMENT}" ]; then
+	echo "${USER_LABEL_PREFIX}$1 =  . "
+    else
+	echo "${USER_LABEL_PREFIX}$1 = ALIGN(${SYMBOL_ABI_ALIGNMENT})"
+    fi
+}
+
 REL_IFUNC=".rel.ifunc    ${RELOCATING-0} : { *(.rel.ifunc) }"
 RELA_IFUNC=".rela.ifunc   ${RELOCATING-0} : { *(.rela.ifunc) }"
 REL_IPLT=".rel.iplt     ${RELOCATING-0} :
     {
-      ${RELOCATING+${CREATE_PIC-PROVIDE_HIDDEN (${USER_LABEL_PREFIX}__rel_iplt_start = .);}}
+      ${RELOCATING+${CREATE_PIC-PROVIDE_HIDDEN ($(def_symbol "__rel_iplt_start"));}}
       *(.rel.iplt)
-      ${RELOCATING+${CREATE_PIC-PROVIDE_HIDDEN (${USER_LABEL_PREFIX}__rel_iplt_end = .);}}
+      ${RELOCATING+${CREATE_PIC-PROVIDE_HIDDEN ($(def_symbol "__rel_iplt_end"));}}
     }"
 RELA_IPLT=".rela.iplt    ${RELOCATING-0} :
     {
-      ${RELOCATING+${CREATE_PIC-PROVIDE_HIDDEN (${USER_LABEL_PREFIX}__rela_iplt_start = .);}}
+      ${RELOCATING+${CREATE_PIC-PROVIDE_HIDDEN ($(def_symbol "__rela_iplt_start"));}}
       *(.rela.iplt)
-      ${RELOCATING+${CREATE_PIC-PROVIDE_HIDDEN (${USER_LABEL_PREFIX}__rela_iplt_end = .);}}
+      ${RELOCATING+${CREATE_PIC-PROVIDE_HIDDEN ($(def_symbol "__rela_iplt_end"));}}
     }"
 DYNAMIC=".dynamic      ${RELOCATING-0} : { *(.dynamic) }"
 RODATA=".${RODATA_NAME}       ${RELOCATING-0} : { *(.${RODATA_NAME}${RELOCATING+ .${RODATA_NAME}.* .gnu.linkonce.r.*}) }"
@@ -267,23 +279,23 @@ else
 fi
 PREINIT_ARRAY=".preinit_array    :
   {
-    ${CREATE_SHLIB-PROVIDE_HIDDEN (${USER_LABEL_PREFIX}__preinit_array_start = .);}
+    ${CREATE_SHLIB-PROVIDE_HIDDEN ($(def_symbol "__preinit_array_start"));}
     KEEP (*(.preinit_array))
-    ${CREATE_SHLIB-PROVIDE_HIDDEN (${USER_LABEL_PREFIX}__preinit_array_end = .);}
+    ${CREATE_SHLIB-PROVIDE_HIDDEN ($(def_symbol "__preinit_array_end"));}
   }"
 INIT_ARRAY=".init_array    :
   {
-    ${CREATE_SHLIB-PROVIDE_HIDDEN (${USER_LABEL_PREFIX}__init_array_start = .);}
+    ${CREATE_SHLIB-PROVIDE_HIDDEN ($(def_symbol "__init_array_start"));}
     ${SORT_INIT_ARRAY}
     KEEP (*(.init_array ${CTORS_IN_INIT_ARRAY}))
-    ${CREATE_SHLIB-PROVIDE_HIDDEN (${USER_LABEL_PREFIX}__init_array_end = .);}
+    ${CREATE_SHLIB-PROVIDE_HIDDEN ($(def_symbol "__init_array_end"));}
   }"
 FINI_ARRAY=".fini_array    :
   {
-    ${CREATE_SHLIB-PROVIDE_HIDDEN (${USER_LABEL_PREFIX}__fini_array_start = .);}
+    ${CREATE_SHLIB-PROVIDE_HIDDEN ($(def_symbol "__fini_array_start"));}
     ${SORT_FINI_ARRAY}
     KEEP (*(.fini_array ${DTORS_IN_FINI_ARRAY}))
-    ${CREATE_SHLIB-PROVIDE_HIDDEN (${USER_LABEL_PREFIX}__fini_array_end = .);}
+    ${CREATE_SHLIB-PROVIDE_HIDDEN ($(def_symbol "__fini_array_end"));}
   }"
 CTOR=".ctors        ${CONSTRUCTING-0} :
   {
@@ -323,7 +335,7 @@ DTOR=".dtors        ${CONSTRUCTING-0} :
   }"
 STACK=".stack        ${RELOCATING-0}${RELOCATING+${STACK_ADDR}} :
   {
-    ${RELOCATING+${USER_LABEL_PREFIX}_stack = .;}
+    ${RELOCATING+$(def_symbol "_stack");}
     *(.stack)
     ${RELOCATING+${STACK_SENTINEL}}
   }"
@@ -494,16 +506,16 @@ cat >> ldscripts/dyntmp.$$ <<EOF
   .rel.plt      ${RELOCATING-0} :
     {
       *(.rel.plt)
-      ${IREL_IN_PLT+${RELOCATING+${CREATE_PIC-PROVIDE_HIDDEN (${USER_LABEL_PREFIX}__rel_iplt_start = .);}}}
+      ${IREL_IN_PLT+${RELOCATING+${CREATE_PIC-PROVIDE_HIDDEN ($(def_symbol "__rel_iplt_start"));}}}
       ${IREL_IN_PLT+${RELOCATING+*(.rel.iplt)}}
-      ${IREL_IN_PLT+${RELOCATING+${CREATE_PIC-PROVIDE_HIDDEN (${USER_LABEL_PREFIX}__rel_iplt_end = .);}}}
+      ${IREL_IN_PLT+${RELOCATING+${CREATE_PIC-PROVIDE_HIDDEN ($(def_symbol "__rel_iplt_end"));}}}
     }
   .rela.plt     ${RELOCATING-0} :
     {
       *(.rela.plt)
-      ${IREL_IN_PLT+${RELOCATING+${CREATE_PIC-PROVIDE_HIDDEN (${USER_LABEL_PREFIX}__rela_iplt_start = .);}}}
+      ${IREL_IN_PLT+${RELOCATING+${CREATE_PIC-PROVIDE_HIDDEN ($(def_symbol "__rela_iplt_start"));}}}
       ${IREL_IN_PLT+${RELOCATING+*(.rela.iplt)}}
-      ${IREL_IN_PLT+${RELOCATING+${CREATE_PIC-PROVIDE_HIDDEN (${USER_LABEL_PREFIX}__rela_iplt_end = .);}}}
+      ${IREL_IN_PLT+${RELOCATING+${CREATE_PIC-PROVIDE_HIDDEN ($(def_symbol "__rela_iplt_end"));}}}
     }
   ${OTHER_PLT_RELOC_SECTIONS}
 EOF
@@ -628,7 +640,7 @@ cat <<EOF
   /* Thread Local Storage sections  */
   .tdata	${RELOCATING-0} :
    {
-     ${RELOCATING+${CREATE_SHLIB-PROVIDE_HIDDEN (${USER_LABEL_PREFIX}__tdata_start = .);}}
+     ${RELOCATING+${CREATE_SHLIB-PROVIDE_HIDDEN ($(def_symbol "__tdata_start"));}}
      *(.tdata${RELOCATING+ .tdata.* .gnu.linkonce.td.*})
    }
   .tbss		${RELOCATING-0} : { *(.tbss${RELOCATING+ .tbss.* .gnu.linkonce.tb.*})${RELOCATING+ *(.tcommon)} }
@@ -676,10 +688,10 @@ cat <<EOF
   ${SDATA_GOT+${OTHER_GOT_SECTIONS}}
   ${DATA_SDATA-${SDATA}}
   ${DATA_SDATA-${OTHER_SDATA_SECTIONS}}
-  ${RELOCATING+${DATA_END_SYMBOLS-${CREATE_SHLIB+PROVIDE (}${USER_LABEL_PREFIX}_edata = .${CREATE_SHLIB+)}; PROVIDE (${USER_LABEL_PREFIX}edata = .);}}
+  ${RELOCATING+${DATA_END_SYMBOLS-${CREATE_SHLIB+PROVIDE (}$(def_symbol "_edata")${CREATE_SHLIB+)}; PROVIDE ($(def_symbol "edata"));}}
   ${PERSISTENT}
   ${RELOCATING+. = .;}
-  ${RELOCATING+${CREATE_SHLIB+PROVIDE (}${USER_LABEL_PREFIX}__bss_start = .${CREATE_SHLIB+)};}
+  ${RELOCATING+${CREATE_SHLIB+PROVIDE (}$(def_symbol "__bss_start")${CREATE_SHLIB+)};}
   ${RELOCATING+${OTHER_BSS_SYMBOLS}}
   ${DATA_SDATA-${SBSS}}
   ${BSS_PLT+${PLT}}
@@ -713,7 +725,7 @@ cat <<EOF
   ${LARGE_BSS_AFTER_BSS-${LARGE_BSS}}
   ${RELOCATING+. = ALIGN(${ALIGNMENT});}
   ${RELOCATING+${OTHER_END_SYMBOLS}}
-  ${RELOCATING+${END_SYMBOLS-${CREATE_SHLIB+PROVIDE (}${USER_LABEL_PREFIX}_end = .${CREATE_SHLIB+)}; PROVIDE (${USER_LABEL_PREFIX}end = .);}}
+  ${RELOCATING+${END_SYMBOLS-${CREATE_SHLIB+PROVIDE (}$(def_symbol "_end")${CREATE_SHLIB+)}; PROVIDE ($(def_symbol "end"));}}
   ${RELOCATING+${DATA_SEGMENT_END}}
   ${TINY_DATA_SECTION}
   ${TINY_BSS_SECTION}
-- 
2.41.0

Re: [PATCH] Align linkerscript symbols according to ABI

[Nick Clifton]

Hi Andreas,

> Apply ABI specific alignment to symbols generated in the default
> linker script.
> 
> No regressions on x86 and s390x. Linkerscript on x86 does not change
> with that.
> 
> Ok for mainline?

Approved - please apply.

I suspect that we will get some complaints about using a user defined
function in a shell script.  Presumably from people where /bin/sh is a
shell interpreter that does not support user functions.  Maybe I am
being pessimistic however.  Surely these days everyone is using a modern
interpreter ?

Cheers
   Nick

Re: [PATCH] Align linkerscript symbols according to ABI

[Andreas Krebbel]

On 7/5/23 11:44, Nick Clifton wrote:
> Hi Andreas,
> 
>> Apply ABI specific alignment to symbols generated in the default
>> linker script.
>>
>> No regressions on x86 and s390x. Linkerscript on x86 does not change
>> with that.
>>
>> Ok for mainline?
> 
> Approved - please apply.

Thanks!

> I suspect that we will get some complaints about using a user defined
> function in a shell script.  Presumably from people where /bin/sh is a
> shell interpreter that does not support user functions.  Maybe I am
> being pessimistic however.  Surely these days everyone is using a modern
> interpreter ?

ld/genscripts.sh already uses a couple of shell functions. So I hope it doesn't create new issues.

Bye,

Andreas

Re: [PATCH] Align linkerscript symbols according to ABI

[Alan Modra]

On Wed, Jul 05, 2023 at 11:53:35AM +0200, Andreas Krebbel via Binutils wrote:
> >> Apply ABI specific alignment to symbols generated in the default
> >> linker script.

I think this change is potentially wrong for symbols defined inside an
output section.  Do you really want symbols marking the start and end
of a section to disagree with the actual section, if the section
itself is wrongly aligned?

-- 
Alan Modra
Australia Development Lab, IBM

Re: [PATCH] Align linkerscript symbols according to ABI

[Andreas Krebbel]

On 7/7/23 05:03, Alan Modra wrote:
> On Wed, Jul 05, 2023 at 11:53:35AM +0200, Andreas Krebbel via Binutils wrote:
>>>> Apply ABI specific alignment to symbols generated in the default
>>>> linker script.
> 
> I think this change is potentially wrong for symbols defined inside an
> output section.  Do you really want symbols marking the start and end
> of a section to disagree with the actual section, if the section
> itself is wrongly aligned?

Hopefully all our output sections are at least aligned to our symbol ABI alignment otherwise that's
what I would fix I guess. You are right that having a gap there would be bad.

Andreas

Re: [PATCH] Align linkerscript symbols according to ABI

[Alan Modra]

Align dot before symbols defined outside of output sections.  Before _end
is already aligned.

I'm inclined to think that with this patch the alignment in def_symbol
ought to disappear, but for now I'm leaving it in.

	* scripttempl/elf.sc (def_symbol): Tidy excess space.
	(_edata): Align before emitting symbol when SYMBOL_ABI_ALIGNMENT.

diff --git a/ld/scripttempl/elf.sc b/ld/scripttempl/elf.sc
index 9e95e6b4162..bfd8b5ed4b3 100644
--- a/ld/scripttempl/elf.sc
+++ b/ld/scripttempl/elf.sc
@@ -171,7 +171,7 @@ fi
 def_symbol()
 {
     if [ -z "${SYMBOL_ABI_ALIGNMENT}" ]; then
-	echo "${USER_LABEL_PREFIX}$1 =  . "
+	echo "${USER_LABEL_PREFIX}$1 = ."
     else
 	echo "${USER_LABEL_PREFIX}$1 = ALIGN(${SYMBOL_ABI_ALIGNMENT})"
     fi
@@ -688,6 +688,7 @@ cat <<EOF
   ${SDATA_GOT+${OTHER_GOT_SECTIONS}}
   ${DATA_SDATA-${SDATA}}
   ${DATA_SDATA-${OTHER_SDATA_SECTIONS}}
+  ${RELOCATING+${SYMBOL_ABI_ALIGNMENT+. = ALIGN(${SYMBOL_ABI_ALIGNMENT});}}
   ${RELOCATING+${DATA_END_SYMBOLS-${CREATE_SHLIB+PROVIDE (}$(def_symbol "_edata")${CREATE_SHLIB+)}; PROVIDE ($(def_symbol "edata"));}}
   ${PERSISTENT}
   ${RELOCATING+. = .;}

-- 
Alan Modra
Australia Development Lab, IBM