| Message ID | 20190619110250.18881-2-roger.pau@citrix.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | x86: build with llvm 8 linker | expand |
On 19/06/2019 12:02, Roger Pau Monne wrote: > Or else clang adds a .init.rodata.cst8 section to the resulting object > file, which is not handled by the Xen linker script and can end up > before the text section which contains the headers, thus resulting in > a not usable binary. > > Signed-off-by: Roger Pau Monné <roger.pau@citrix.com> I think Clang is actually adding a .rodata.cst8 section, and the bulk objcopy turns it into .init.rodata.cst8. This is a good change so Reviewed-by: Andrew Cooper <andrew.cooper3@citrix.com> (subject to some clarity over the exact section), but I also think this is needed as well: diff --git a/xen/arch/x86/xen.lds.S b/xen/arch/x86/xen.lds.S index cb42dc8..4f23059 100644 --- a/xen/arch/x86/xen.lds.S +++ b/xen/arch/x86/xen.lds.S @@ -206,8 +206,7 @@ SECTIONS #endif *(.init.rodata) - *(.init.rodata.rel) - *(.init.rodata.str*) + *(.init.rodata.*) . = ALIGN(POINTER_ALIGN); __setup_start = .; @@ -221,8 +220,7 @@ SECTIONS __initcall_end = .; *(.init.data) - *(.init.data.rel) - *(.init.data.rel.*) + *(.init.data.*) . = ALIGN(4); __trampoline_rel_start = .; *(.trampoline_rel)
>>> On 19.06.19 at 13:09, <andrew.cooper3@citrix.com> wrote: > On 19/06/2019 12:02, Roger Pau Monne wrote: >> Or else clang adds a .init.rodata.cst8 section to the resulting object >> file, which is not handled by the Xen linker script and can end up >> before the text section which contains the headers, thus resulting in >> a not usable binary. >> >> Signed-off-by: Roger Pau Monné <roger.pau@citrix.com> > > I think Clang is actually adding a .rodata.cst8 section, and the bulk > objcopy turns it into .init.rodata.cst8. > > This is a good change so Reviewed-by: Andrew Cooper > <andrew.cooper3@citrix.com> (subject to some clarity over the exact > section), but I also think this is needed as well: > > diff --git a/xen/arch/x86/xen.lds.S b/xen/arch/x86/xen.lds.S > index cb42dc8..4f23059 100644 > --- a/xen/arch/x86/xen.lds.S > +++ b/xen/arch/x86/xen.lds.S > @@ -206,8 +206,7 @@ SECTIONS > #endif > > *(.init.rodata) > - *(.init.rodata.rel) > - *(.init.rodata.str*) > + *(.init.rodata.*) > > . = ALIGN(POINTER_ALIGN); > __setup_start = .; > @@ -221,8 +220,7 @@ SECTIONS > __initcall_end = .; > > *(.init.data) > - *(.init.data.rel) > - *(.init.data.rel.*) > + *(.init.data.*) > . = ALIGN(4); > __trampoline_rel_start = .; > *(.trampoline_rel) And the same thing then also for Arm. Jan
>>> On 19.06.19 at 13:02, <roger.pau@citrix.com> wrote: > Or else clang adds a .init.rodata.cst8 section to the resulting object > file, which is not handled by the Xen linker script and can end up > before the text section which contains the headers, thus resulting in > a not usable binary. To be honest I'd prefer if we went with just the change suggested by Andrew, getting the linker script back in line with SPECIAL_DATA_SECTIONS. The static const items in the decompressors were left un-annotated intentionally, since the .rodata.* thingies want/need taking care of anyway. After all you won't (I hope) suggest also annotating the various string literals. Jan
On Wed, Jun 19, 2019 at 05:50:52AM -0600, Jan Beulich wrote: > >>> On 19.06.19 at 13:02, <roger.pau@citrix.com> wrote: > > Or else clang adds a .init.rodata.cst8 section to the resulting object > > file, which is not handled by the Xen linker script and can end up > > before the text section which contains the headers, thus resulting in > > a not usable binary. > > To be honest I'd prefer if we went with just the change suggested > by Andrew, getting the linker script back in line with > SPECIAL_DATA_SECTIONS. The static const items in the > decompressors were left un-annotated intentionally, since the > .rodata.* thingies want/need taking care of anyway. After all you > won't (I hope) suggest also annotating the various string literals. OK, I think regardless of the rest of the lld 8 series it is worth sending the linker script change in order to prevent having this orphaned section. Andrew, since you where the one to propose it, could you please send a formal patch? Thanks, Roger.
diff --git a/xen/common/decompress.h b/xen/common/decompress.h index 647b7b1e83..4a429bca12 100644 --- a/xen/common/decompress.h +++ b/xen/common/decompress.h @@ -10,6 +10,7 @@ #define STATIC #define INIT __init #define INITDATA __initdata +#define INITCONST __initconst #define malloc xmalloc_bytes #define free xfree @@ -22,6 +23,7 @@ #define STATIC static #define INIT #define INITDATA +#define INITCONST #define large_malloc malloc #define large_free free diff --git a/xen/common/xz/dec_bcj.c b/xen/common/xz/dec_bcj.c index 86c1192199..0a9a45de2b 100644 --- a/xen/common/xz/dec_bcj.c +++ b/xen/common/xz/dec_bcj.c @@ -87,10 +87,10 @@ static inline int INIT bcj_x86_test_msbyte(uint8_t b) static size_t INIT bcj_x86(struct xz_dec_bcj *s, uint8_t *buf, size_t size) { - static const bool_t mask_to_allowed_status[8] + static const bool_t INITCONST mask_to_allowed_status[8] = { true, true, true, false, true, false, false, false }; - static const uint8_t mask_to_bit_num[8] = { 0, 1, 2, 2, 3, 3, 3, 3 }; + static const uint8_t INITCONST mask_to_bit_num[8] = { 0, 1, 2, 2, 3, 3, 3, 3 }; size_t i; size_t prev_pos = (size_t)-1; @@ -180,7 +180,7 @@ static size_t INIT bcj_powerpc(struct xz_dec_bcj *s, uint8_t *buf, size_t size) #ifdef XZ_DEC_IA64 static size_t INIT bcj_ia64(struct xz_dec_bcj *s, uint8_t *buf, size_t size) { - static const uint8_t branch_table[32] = { + static const uint8_t INITCONST branch_table[32] = { 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 4, 4, 6, 6, 0, 0, 7, 7, diff --git a/xen/common/xz/dec_stream.c b/xen/common/xz/dec_stream.c index b8b566307c..61eb2ffb55 100644 --- a/xen/common/xz/dec_stream.c +++ b/xen/common/xz/dec_stream.c @@ -138,7 +138,7 @@ struct xz_dec { #ifdef XZ_DEC_ANY_CHECK /* Sizes of the Check field with different Check IDs */ -static const uint8_t check_sizes[16] = { +static const uint8_t INITCONST check_sizes[16] = { 0, 4, 4, 4, 8, 8, 8,
Or else clang adds a .init.rodata.cst8 section to the resulting object file, which is not handled by the Xen linker script and can end up before the text section which contains the headers, thus resulting in a not usable binary. Signed-off-by: Roger Pau Monné <roger.pau@citrix.com> --- Cc: Andrew Cooper <andrew.cooper3@citrix.com> Cc: George Dunlap <George.Dunlap@eu.citrix.com> Cc: Ian Jackson <ian.jackson@eu.citrix.com> Cc: Jan Beulich <jbeulich@suse.com> Cc: Julien Grall <julien.grall@arm.com> Cc: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com> Cc: Stefano Stabellini <sstabellini@kernel.org> Cc: Tim Deegan <tim@xen.org> Cc: Wei Liu <wl@xen.org> --- xen/common/decompress.h | 2 ++ xen/common/xz/dec_bcj.c | 6 +++--- xen/common/xz/dec_stream.c | 2 +- 3 files changed, 6 insertions(+), 4 deletions(-)