[PATCH] x86/boot: fix build with certain older gcc versions

[PATCH] x86/boot: fix build with certain older gcc versions

[Jan Beulich]

[-- Attachment #1: Type: text/plain, Size: 1421 bytes --]

Despite all attempts so far (ending in commit fecf584294 ["Config.mk:
fix comment for debug option"] adjusting the respective comment),
Config.mk's debug= setting still affects the hypervisor build: CFLAGS
gets -g added there.

xen/arch/x86/boot/build32.mk includes that file, and hence inherits the
setting too. Some gcc versions take -g to create an .eh_frame section
despite -fno-asynchronous-unwind-tables (which instead one would expect
to produce .debug_frame).

In turn, commit 93c0c0287a ("x86/boot: create *.lnk files with linker
script") was - in my understanding - supposed to make sure .text is
first, but apparently it did also not really achieve that effect: Both
reloc.lnk and reloc.bin in the case here ended up with .eh_frame first,
which obviously rendered the whole final binary unusable.

Explicitly suppress generation of any kind of debug info when building
reloc.o.

Signed-off-by: Jan Beulich <jbeulich@suse.com>

--- a/xen/arch/x86/boot/build32.mk
+++ b/xen/arch/x86/boot/build32.mk
@@ -4,7 +4,7 @@ include $(XEN_ROOT)/Config.mk
 
 $(call cc-options-add,CFLAGS,CC,$(EMBEDDED_EXTRA_CFLAGS))
 
-CFLAGS += -Werror -fno-asynchronous-unwind-tables -fno-builtin -msoft-float
+CFLAGS += -Werror -fno-asynchronous-unwind-tables -fno-builtin -g0 -msoft-float
 CFLAGS := $(filter-out -flto,$(CFLAGS)) 
 
 # NB. awk invocation is a portable alternative to 'head -n -1'




[-- Attachment #2: x86-boot-reloc-no-debug-info.patch --]
[-- Type: text/plain, Size: 1470 bytes --]

x86/boot: fix build with certain older gcc versions

Despite all attempts so far (ending in commit fecf584294 ["Config.mk:
fix comment for debug option"] adjusting the respective comment),
Config.mk's debug= setting still affects the hypervisor build: CFLAGS
gets -g added there.

xen/arch/x86/boot/build32.mk includes that file, and hence inherits the
setting too. Some gcc versions take -g to create an .eh_frame section
despite -fno-asynchronous-unwind-tables (which instead one would expect
to produce .debug_frame).

In turn, commit 93c0c0287a ("x86/boot: create *.lnk files with linker
script") was - in my understanding - supposed to make sure .text is
first, but apparently it did also not really achieve that effect: Both
reloc.lnk and reloc.bin in the case here ended up with .eh_frame first,
which obviously rendered the whole final binary unusable.

Explicitly suppress generation of any kind of debug info when building
reloc.o.

Signed-off-by: Jan Beulich <jbeulich@suse.com>

--- a/xen/arch/x86/boot/build32.mk
+++ b/xen/arch/x86/boot/build32.mk
@@ -4,7 +4,7 @@ include $(XEN_ROOT)/Config.mk
 
 $(call cc-options-add,CFLAGS,CC,$(EMBEDDED_EXTRA_CFLAGS))
 
-CFLAGS += -Werror -fno-asynchronous-unwind-tables -fno-builtin -msoft-float
+CFLAGS += -Werror -fno-asynchronous-unwind-tables -fno-builtin -g0 -msoft-float
 CFLAGS := $(filter-out -flto,$(CFLAGS)) 
 
 # NB. awk invocation is a portable alternative to 'head -n -1'

[-- Attachment #3: Type: text/plain, Size: 127 bytes --]

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

Re: [PATCH] x86/boot: fix build with certain older gcc versions

[Andrew Cooper]

On 15/12/16 14:04, Jan Beulich wrote:
> Despite all attempts so far (ending in commit fecf584294 ["Config.mk:
> fix comment for debug option"] adjusting the respective comment),
> Config.mk's debug= setting still affects the hypervisor build: CFLAGS
> gets -g added there.
>
> xen/arch/x86/boot/build32.mk includes that file, and hence inherits the
> setting too. Some gcc versions take -g to create an .eh_frame section
> despite -fno-asynchronous-unwind-tables (which instead one would expect
> to produce .debug_frame).
>
> In turn, commit 93c0c0287a ("x86/boot: create *.lnk files with linker
> script") was - in my understanding - supposed to make sure .text is
> first, but apparently it did also not really achieve that effect: Both
> reloc.lnk and reloc.bin in the case here ended up with .eh_frame first,
> which obviously rendered the whole final binary unusable.
>
> Explicitly suppress generation of any kind of debug info when building
> reloc.o.
>
> Signed-off-by: Jan Beulich <jbeulich@suse.com>

Reviewed-by: Andrew Cooper <andrew.cooper3@citrix.com>

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

Re: [PATCH] x86/boot: fix build with certain older gcc versions

[Daniel Kiper]

On Thu, Dec 15, 2016 at 07:04:51AM -0700, Jan Beulich wrote:
> Despite all attempts so far (ending in commit fecf584294 ["Config.mk:
> fix comment for debug option"] adjusting the respective comment),
> Config.mk's debug= setting still affects the hypervisor build: CFLAGS
> gets -g added there.
>
> xen/arch/x86/boot/build32.mk includes that file, and hence inherits the
> setting too. Some gcc versions take -g to create an .eh_frame section
> despite -fno-asynchronous-unwind-tables (which instead one would expect
> to produce .debug_frame).

Hmmm... Smells like a bug...

> In turn, commit 93c0c0287a ("x86/boot: create *.lnk files with linker
> script") was - in my understanding - supposed to make sure .text is
> first, but apparently it did also not really achieve that effect: Both

The main goal was to have text and data sections merged into one text section.

> reloc.lnk and reloc.bin in the case here ended up with .eh_frame first,
> which obviously rendered the whole final binary unusable.

Should not we discard unneeded sections in build32.lds? It looks a bit
more complicated but maybe we should consider such option too.

> Explicitly suppress generation of any kind of debug info when building
> reloc.o.
>
> Signed-off-by: Jan Beulich <jbeulich@suse.com>
>
> --- a/xen/arch/x86/boot/build32.mk
> +++ b/xen/arch/x86/boot/build32.mk
> @@ -4,7 +4,7 @@ include $(XEN_ROOT)/Config.mk
>
>  $(call cc-options-add,CFLAGS,CC,$(EMBEDDED_EXTRA_CFLAGS))
>
> -CFLAGS += -Werror -fno-asynchronous-unwind-tables -fno-builtin -msoft-float
> +CFLAGS += -Werror -fno-asynchronous-unwind-tables -fno-builtin -g0 -msoft-float

CFLAGS = $(subst -g,-g0,$(CFLAGS))?

Daniel

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

Re: [PATCH] x86/boot: fix build with certain older gcc versions

[Daniel Kiper]

On Fri, Dec 16, 2016 at 11:33:18AM +0100, Daniel Kiper wrote:
> On Thu, Dec 15, 2016 at 07:04:51AM -0700, Jan Beulich wrote:

[...]

> > --- a/xen/arch/x86/boot/build32.mk
> > +++ b/xen/arch/x86/boot/build32.mk
> > @@ -4,7 +4,7 @@ include $(XEN_ROOT)/Config.mk
> >
> >  $(call cc-options-add,CFLAGS,CC,$(EMBEDDED_EXTRA_CFLAGS))
> >
> > -CFLAGS += -Werror -fno-asynchronous-unwind-tables -fno-builtin -msoft-float
> > +CFLAGS += -Werror -fno-asynchronous-unwind-tables -fno-builtin -g0 -msoft-float
>
> CFLAGS = $(subst -g,-g0,$(CFLAGS))?

...or better:

CFLAGS := $(patsubst -g%,-g0,$(CFLAGS))

Daniel

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

Re: [PATCH] x86/boot: fix build with certain older gcc versions

[Jan Beulich]

>>> On 16.12.16 at 11:33, <daniel.kiper@oracle.com> wrote:
> On Thu, Dec 15, 2016 at 07:04:51AM -0700, Jan Beulich wrote:
>> Despite all attempts so far (ending in commit fecf584294 ["Config.mk:
>> fix comment for debug option"] adjusting the respective comment),
>> Config.mk's debug= setting still affects the hypervisor build: CFLAGS
>> gets -g added there.
>>
>> xen/arch/x86/boot/build32.mk includes that file, and hence inherits the
>> setting too. Some gcc versions take -g to create an .eh_frame section
>> despite -fno-asynchronous-unwind-tables (which instead one would expect
>> to produce .debug_frame).
> 
> Hmmm... Smells like a bug...

Possibly - I have no idea what the formal spec of linker behavior
here is (assuming there is one anywhere, which I doubt).

>> In turn, commit 93c0c0287a ("x86/boot: create *.lnk files with linker
>> script") was - in my understanding - supposed to make sure .text is
>> first, but apparently it did also not really achieve that effect: Both
> 
> The main goal was to have text and data sections merged into one text 
> section.
> 
>> reloc.lnk and reloc.bin in the case here ended up with .eh_frame first,
>> which obviously rendered the whole final binary unusable.
> 
> Should not we discard unneeded sections in build32.lds? It looks a bit
> more complicated but maybe we should consider such option too.

Hmm - would you mean a wholesale *(*) pattern to be used here?
I think this whole keep-some-discard-some approach is rather
fragile, not matter how much we customize it.

Jan


_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

Re: [PATCH] x86/boot: fix build with certain older gcc versions

[Daniel Kiper]

On Fri, Dec 16, 2016 at 05:38:35AM -0700, Jan Beulich wrote:
> >>> On 16.12.16 at 11:33, <daniel.kiper@oracle.com> wrote:
> > On Thu, Dec 15, 2016 at 07:04:51AM -0700, Jan Beulich wrote:
> >> Despite all attempts so far (ending in commit fecf584294 ["Config.mk:
> >> fix comment for debug option"] adjusting the respective comment),
> >> Config.mk's debug= setting still affects the hypervisor build: CFLAGS
> >> gets -g added there.
> >>
> >> xen/arch/x86/boot/build32.mk includes that file, and hence inherits the
> >> setting too. Some gcc versions take -g to create an .eh_frame section
> >> despite -fno-asynchronous-unwind-tables (which instead one would expect
> >> to produce .debug_frame).
> >
> > Hmmm... Smells like a bug...
>
> Possibly - I have no idea what the formal spec of linker behavior
> here is (assuming there is one anywhere, which I doubt).

:-)))

> >> In turn, commit 93c0c0287a ("x86/boot: create *.lnk files with linker
> >> script") was - in my understanding - supposed to make sure .text is
> >> first, but apparently it did also not really achieve that effect: Both
> >
> > The main goal was to have text and data sections merged into one text
> > section.
> >
> >> reloc.lnk and reloc.bin in the case here ended up with .eh_frame first,
> >> which obviously rendered the whole final binary unusable.
> >
> > Should not we discard unneeded sections in build32.lds? It looks a bit
> > more complicated but maybe we should consider such option too.
>
> Hmm - would you mean a wholesale *(*) pattern to be used here?

I was thinking about discarding specific sections which are known to do harm.

> I think this whole keep-some-discard-some approach is rather
> fragile, not matter how much we customize it.

That is why I am not convinced that we should do that. I just wanted
to be sure that this option was considered too. So, I agree that -g0
approach is better. However, I think that we should replace -g% with
-g0 as I said in my earlier email.

Daniel

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

Re: [PATCH] x86/boot: fix build with certain older gcc versions

[Wei Liu]

On Thu, Dec 15, 2016 at 07:04:51AM -0700, Jan Beulich wrote:
> Despite all attempts so far (ending in commit fecf584294 ["Config.mk:
> fix comment for debug option"] adjusting the respective comment),
> Config.mk's debug= setting still affects the hypervisor build: CFLAGS
> gets -g added there.
> 

Should we try to:

1. remove debug_symbols in Config.mk
2. push adding -g to tools/Rules.mk

?

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

Re: [PATCH] x86/boot: fix build with certain older gcc versions

[Jan Beulich]

>>> On 16.12.16 at 14:57, <wei.liu2@citrix.com> wrote:
> On Thu, Dec 15, 2016 at 07:04:51AM -0700, Jan Beulich wrote:
>> Despite all attempts so far (ending in commit fecf584294 ["Config.mk:
>> fix comment for debug option"] adjusting the respective comment),
>> Config.mk's debug= setting still affects the hypervisor build: CFLAGS
>> gets -g added there.
>> 
> 
> Should we try to:
> 
> 1. remove debug_symbols in Config.mk
> 2. push adding -g to tools/Rules.mk

I think that's very desirable (for 1. provided nothing else references
that variable anymore).

Jan


_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel