| Message ID | 20230627074703.99608-5-thuth@redhat.com |
|---|---|
| State | New |
| Headers | show |
| Series | pc-bios/s390-ccw: Fixes and improvements for start.S | expand |
On Tue, 27 Jun 2023 09:47:03 +0200 Thomas Huth <thuth@redhat.com> wrote: > start.S currently cannot be compiled with Clang 16 and binutils 2.40: > > ld: start.o(.text+0x8): misaligned symbol `__bss_start' (0xc1e5) for > relocation R_390_PC32DBL > > According to the built-in linker script of ld, the symbol __bss_start > can actually point *before* the .bss section and does not need to have > any alignment, so in certain situations (like when using the internal > assembler of Clang), the __bss_start symbol can indeed be unaligned > and thus it is not suitable for being used with the "larl" instruction > that needs an address that is at least aligned to halfwords. > The problem went unnoticed so far since binutils <= 2.39 did not > check the alignment, but starting with binutils 2.40, such unaligned > addresses are now refused. > > Fix it by using the real start address of the .bss section instead. > > Buglink: https://bugzilla.redhat.com/show_bug.cgi?id=2216662 > Reported-by: Miroslav Rezanina <mrezanin@redhat.com> > Suggested-by: Nick Clifton <nickc@redhat.com> > Signed-off-by: Thomas Huth <thuth@redhat.com> > --- > pc-bios/s390-ccw/start.S | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/pc-bios/s390-ccw/start.S b/pc-bios/s390-ccw/start.S > index 111dea261b..a63c4e3ff2 100644 > --- a/pc-bios/s390-ccw/start.S > +++ b/pc-bios/s390-ccw/start.S > @@ -18,7 +18,7 @@ _start: > larl %r15,stack + STACK_SIZE - 160 /* Set up stack */ > > /* clear bss */ > - larl %r2,__bss_start > + larl %r2,.bss > larl %r3,_end since we are here, do you have guarantees that _end is always correctly aligned? if so: Reviewed-by: Claudio Imbrenda <imbrenda@linux.ibm.com> > slgr %r3,%r2 /* get sizeof bss */ > ltgr %r3,%r3 /* bss empty? */
On Tue, 2023-06-27 at 09:47 +0200, Thomas Huth wrote: > start.S currently cannot be compiled with Clang 16 and binutils 2.40: > > ld: start.o(.text+0x8): misaligned symbol `__bss_start' (0xc1e5) for > relocation R_390_PC32DBL > > According to the built-in linker script of ld, the symbol __bss_start > can actually point *before* the .bss section and does not need to > have > any alignment, so in certain situations (like when using the internal > assembler of Clang), the __bss_start symbol can indeed be unaligned > and thus it is not suitable for being used with the "larl" > instruction > that needs an address that is at least aligned to halfwords. > The problem went unnoticed so far since binutils <= 2.39 did not > check the alignment, but starting with binutils 2.40, such unaligned > addresses are now refused. > > Fix it by using the real start address of the .bss section instead. > > Buglink: https://bugzilla.redhat.com/show_bug.cgi?id=2216662 > Reported-by: Miroslav Rezanina <mrezanin@redhat.com> > Suggested-by: Nick Clifton <nickc@redhat.com> > Signed-off-by: Thomas Huth <thuth@redhat.com> > --- > pc-bios/s390-ccw/start.S | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) Reviewed-by: Eric Farman <farman@linux.ibm.com>
On 27/06/2023 11.29, Claudio Imbrenda wrote: > On Tue, 27 Jun 2023 09:47:03 +0200 > Thomas Huth <thuth@redhat.com> wrote: > >> start.S currently cannot be compiled with Clang 16 and binutils 2.40: >> >> ld: start.o(.text+0x8): misaligned symbol `__bss_start' (0xc1e5) for >> relocation R_390_PC32DBL >> >> According to the built-in linker script of ld, the symbol __bss_start >> can actually point *before* the .bss section and does not need to have >> any alignment, so in certain situations (like when using the internal >> assembler of Clang), the __bss_start symbol can indeed be unaligned >> and thus it is not suitable for being used with the "larl" instruction >> that needs an address that is at least aligned to halfwords. >> The problem went unnoticed so far since binutils <= 2.39 did not >> check the alignment, but starting with binutils 2.40, such unaligned >> addresses are now refused. >> >> Fix it by using the real start address of the .bss section instead. >> >> Buglink: https://bugzilla.redhat.com/show_bug.cgi?id=2216662 >> Reported-by: Miroslav Rezanina <mrezanin@redhat.com> >> Suggested-by: Nick Clifton <nickc@redhat.com> >> Signed-off-by: Thomas Huth <thuth@redhat.com> >> --- >> pc-bios/s390-ccw/start.S | 2 +- >> 1 file changed, 1 insertion(+), 1 deletion(-) >> >> diff --git a/pc-bios/s390-ccw/start.S b/pc-bios/s390-ccw/start.S >> index 111dea261b..a63c4e3ff2 100644 >> --- a/pc-bios/s390-ccw/start.S >> +++ b/pc-bios/s390-ccw/start.S >> @@ -18,7 +18,7 @@ _start: >> larl %r15,stack + STACK_SIZE - 160 /* Set up stack */ >> >> /* clear bss */ >> - larl %r2,__bss_start >> + larl %r2,.bss >> larl %r3,_end > > since we are here, do you have guarantees that _end is always correctly > aligned? Yes, you can see the internal linker script by running "ld --verbose", and there I get: ... .data1 : { *(.data1) } _edata = .; PROVIDE (edata = .); . = .; __bss_start = .; .bss : { *(.dynbss) *(.bss .bss.* .gnu.linkonce.b.*) *(COMMON) /* Align here to ensure that the .bss section occupies space up to _end. Align after .bss to ensure correct alignment even if the .bss section disappears because there are no input sections. FIXME: Why do we need it? When there is no .bss section, we do not pad the .data section. */ . = ALIGN(. != 0 ? 64 / 8 : 1); } . = ALIGN(64 / 8); . = SEGMENT_START("ldata-segment", .); . = ALIGN(64 / 8); _end = .; PROVIDE (end = .); . = DATA_SEGMENT_END (.); ... As you can see, there is no alignment in front of __bss_start, but there is alignment in front of __end. > if so: > > Reviewed-by: Claudio Imbrenda <imbrenda@linux.ibm.com> Thanks! Thomas
diff --git a/pc-bios/s390-ccw/start.S b/pc-bios/s390-ccw/start.S index 111dea261b..a63c4e3ff2 100644 --- a/pc-bios/s390-ccw/start.S +++ b/pc-bios/s390-ccw/start.S @@ -18,7 +18,7 @@ _start: larl %r15,stack + STACK_SIZE - 160 /* Set up stack */ /* clear bss */ - larl %r2,__bss_start + larl %r2,.bss larl %r3,_end slgr %r3,%r2 /* get sizeof bss */ ltgr %r3,%r3 /* bss empty? */
start.S currently cannot be compiled with Clang 16 and binutils 2.40: ld: start.o(.text+0x8): misaligned symbol `__bss_start' (0xc1e5) for relocation R_390_PC32DBL According to the built-in linker script of ld, the symbol __bss_start can actually point *before* the .bss section and does not need to have any alignment, so in certain situations (like when using the internal assembler of Clang), the __bss_start symbol can indeed be unaligned and thus it is not suitable for being used with the "larl" instruction that needs an address that is at least aligned to halfwords. The problem went unnoticed so far since binutils <= 2.39 did not check the alignment, but starting with binutils 2.40, such unaligned addresses are now refused. Fix it by using the real start address of the .bss section instead. Buglink: https://bugzilla.redhat.com/show_bug.cgi?id=2216662 Reported-by: Miroslav Rezanina <mrezanin@redhat.com> Suggested-by: Nick Clifton <nickc@redhat.com> Signed-off-by: Thomas Huth <thuth@redhat.com> --- pc-bios/s390-ccw/start.S | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)