diff mbox

Xen: remove -fshort-wchar gcc flag

Message ID 16541778.WMH6K24aDt@wuerfel (mailing list archive)
State New, archived
Headers show

Commit Message

Arnd Bergmann Aug. 11, 2016, 12:39 p.m. UTC
A previous patch added the --no-wchar-size-warning to the Makefile to
avoid this harmless warning:

arm-linux-gnueabi-ld: warning: drivers/xen/efi.o uses 2-byte wchar_t yet the output is to use 4-byte wchar_t; use of wchar_t values across objects may fail

Changing kbuild to use thin archives instead of recursive linking
unfortunately brings the same warning back during the final link.

This time, we remove the -fshort-wchar flag that originally caused
the warning, hopefully fixing the problem for good. I don't see
any reason for having the flag in the first place, as the Xen code
does not use wchar_t at all.

Signed-off-by: Arnd Bergmann <arnd@arndb.de>
Fixes: 971a69db7dc0 ("Xen: don't warn about 2-byte wchar_t in efi")
---
On Thursday, August 11, 2016 8:16:14 PM CEST Nicholas Piggin wrote:
> Hi,
> 
> I would like to submit the kbuild changes in patches 1-3 for
> consideration.
> 
> I've taken on the feedback, so thanks everybody for that. The
> biggest change since last time is a more general way for
> architectures to do a post-link pass in patch 3.
> 
> On the question of whether to enable thin archives unconditionally,
> I prefer to have architectures enable them as they are tested. But
> I would like to see everybody moved as soon as possible and the
> incremental linking removed.

It would be nice to get this patch merged along with the thin
archive conversion, either by merging it through the xen
Tree, or by making it part of Nick's series with an Ack
from the xen maintainers.

Comments

Jan Beulich Aug. 11, 2016, 12:51 p.m. UTC | #1
>>> On 11.08.16 at 14:39, <arnd@arndb.de> wrote:
> A previous patch added the --no-wchar-size-warning to the Makefile to
> avoid this harmless warning:
> 
> arm-linux-gnueabi-ld: warning: drivers/xen/efi.o uses 2-byte wchar_t yet the 
> output is to use 4-byte wchar_t; use of wchar_t values across objects may 
> fail
> 
> Changing kbuild to use thin archives instead of recursive linking
> unfortunately brings the same warning back during the final link.
> 
> This time, we remove the -fshort-wchar flag that originally caused
> the warning, hopefully fixing the problem for good. I don't see
> any reason for having the flag in the first place, as the Xen code
> does not use wchar_t at all.

It uses efi_char16_t, and by dropping -fshort-wchar you'd open
up a trap for anyone to fall into who were to add wide string
literals to that same file. EFI using 16-bit characters requires
code interfacing with EFI to do so too.

Jan
Arnd Bergmann Aug. 11, 2016, 2:01 p.m. UTC | #2
On Thursday, August 11, 2016 6:51:33 AM CEST Jan Beulich wrote:
> >>> On 11.08.16 at 14:39, <arnd@arndb.de> wrote:
> > A previous patch added the --no-wchar-size-warning to the Makefile to
> > avoid this harmless warning:
> > 
> > arm-linux-gnueabi-ld: warning: drivers/xen/efi.o uses 2-byte wchar_t yet the 
> > output is to use 4-byte wchar_t; use of wchar_t values across objects may 
> > fail
> > 
> > Changing kbuild to use thin archives instead of recursive linking
> > unfortunately brings the same warning back during the final link.
> > 
> > This time, we remove the -fshort-wchar flag that originally caused
> > the warning, hopefully fixing the problem for good. I don't see
> > any reason for having the flag in the first place, as the Xen code
> > does not use wchar_t at all.
> 
> It uses efi_char16_t, and by dropping -fshort-wchar you'd open
> up a trap for anyone to fall into who were to add wide string
> literals to that same file. EFI using 16-bit characters requires
> code interfacing with EFI to do so too.

I don't understand. How is this different from other source files
that use efi_char16_t or the wchar_t definition from include/linux/nls.h?

As far as I can tell, they all use 16-bit characters, but none of the
others sets the flag. Maybe we should just always build with -fshort-wchar
from the top-level Makefile?

	Arnd
Jan Beulich Aug. 11, 2016, 2:06 p.m. UTC | #3
>>> On 11.08.16 at 16:01, <arnd@arndb.de> wrote:
> On Thursday, August 11, 2016 6:51:33 AM CEST Jan Beulich wrote:
>> >>> On 11.08.16 at 14:39, <arnd@arndb.de> wrote:
>> > A previous patch added the --no-wchar-size-warning to the Makefile to
>> > avoid this harmless warning:
>> > 
>> > arm-linux-gnueabi-ld: warning: drivers/xen/efi.o uses 2-byte wchar_t yet the 
>> > output is to use 4-byte wchar_t; use of wchar_t values across objects may 
>> > fail
>> > 
>> > Changing kbuild to use thin archives instead of recursive linking
>> > unfortunately brings the same warning back during the final link.
>> > 
>> > This time, we remove the -fshort-wchar flag that originally caused
>> > the warning, hopefully fixing the problem for good. I don't see
>> > any reason for having the flag in the first place, as the Xen code
>> > does not use wchar_t at all.
>> 
>> It uses efi_char16_t, and by dropping -fshort-wchar you'd open
>> up a trap for anyone to fall into who were to add wide string
>> literals to that same file. EFI using 16-bit characters requires
>> code interfacing with EFI to do so too.
> 
> I don't understand. How is this different from other source files
> that use efi_char16_t or the wchar_t definition from include/linux/nls.h?

Perhaps they didn't run into the issue yet?

> As far as I can tell, they all use 16-bit characters, but none of the
> others sets the flag. Maybe we should just always build with -fshort-wchar
> from the top-level Makefile?

Quite possible, unless elsewhere in the tree 4-byte characters are
needed.

Jan
David Vrabel Aug. 24, 2016, 5:18 p.m. UTC | #4
On 11/08/16 13:39, Arnd Bergmann wrote:
> A previous patch added the --no-wchar-size-warning to the Makefile to
> avoid this harmless warning:
> 
> arm-linux-gnueabi-ld: warning: drivers/xen/efi.o uses 2-byte wchar_t yet the output is to use 4-byte wchar_t; use of wchar_t values across objects may fail
> 
> Changing kbuild to use thin archives instead of recursive linking
> unfortunately brings the same warning back during the final link.
> 
> This time, we remove the -fshort-wchar flag that originally caused
> the warning, hopefully fixing the problem for good. I don't see
> any reason for having the flag in the first place, as the Xen code
> does not use wchar_t at all.
> 
> Signed-off-by: Arnd Bergmann <arnd@arndb.de>
> Fixes: 971a69db7dc0 ("Xen: don't warn about 2-byte wchar_t in efi")

Acked-by: David Vrabel <david.vrabel@citrix.com>

David
diff mbox

Patch

diff --git a/drivers/xen/Makefile b/drivers/xen/Makefile
index 8feab810aed9..7f188b8d0c67 100644
--- a/drivers/xen/Makefile
+++ b/drivers/xen/Makefile
@@ -7,9 +7,6 @@  obj-y	+= xenbus/
 nostackp := $(call cc-option, -fno-stack-protector)
 CFLAGS_features.o			:= $(nostackp)
 
-CFLAGS_efi.o				+= -fshort-wchar
-LDFLAGS					+= $(call ld-option, --no-wchar-size-warning)
-
 dom0-$(CONFIG_ARM64) += arm-device.o
 dom0-$(CONFIG_PCI) += pci.o
 dom0-$(CONFIG_USB_SUPPORT) += dbgp.o