[U-Boot] [PATCH] drivers/bios_emulator: Fix gcc 4.4 compiler warning
biosemu.c: In function 'BE_setVGA': biosemu.c:147: warning: dereferencing type-punned pointer will break strict-aliasing rules
Signed-off-by: Kumar Gala galak@kernel.crashing.org --- drivers/bios_emulator/biosemu.c | 3 ++- 1 files changed, 2 insertions(+), 1 deletions(-)
diff --git a/drivers/bios_emulator/biosemu.c b/drivers/bios_emulator/biosemu.c index d0c6521..cfa836c 100644 --- a/drivers/bios_emulator/biosemu.c +++ b/drivers/bios_emulator/biosemu.c @@ -144,7 +144,8 @@ void X86API BE_setVGA(BE_VGAInfo * info) _BE_env.biosmem_base = _BE_env.busmem_base + 0x20000; _BE_env.biosmem_limit = 0xC7FFF; } - if (*((u32 *) info->LowMem) == 0) + if ((info->LowMem[0] == 0) && (info->LowMem[1] == 0) && + (info->LowMem[2] == 0) && (info->LowMem[3] == 0)) _BE_bios_init((u32 *) info->LowMem); memcpy((u8 *) M.mem_base, info->LowMem, sizeof(info->LowMem)); }
ahci.c: In function 'ata_scsiop_read_capacity10': ahci.c:616: warning: dereferencing type-punned pointer will break strict-aliasing rules
Signed-off-by: Kumar Gala galak@kernel.crashing.org --- drivers/block/ahci.c | 14 ++++++-------- 1 files changed, 6 insertions(+), 8 deletions(-)
diff --git a/drivers/block/ahci.c b/drivers/block/ahci.c index e1b66fd..b0d1792 100644 --- a/drivers/block/ahci.c +++ b/drivers/block/ahci.c @@ -602,7 +602,7 @@ static int ata_scsiop_read10(ccb * pccb) */ static int ata_scsiop_read_capacity10(ccb *pccb) { - u8 buf[8]; + u32 cap;
if (!ataid[pccb->target]) { printf("scsi_ahci: SCSI READ CAPACITY10 command failure. " @@ -611,14 +611,12 @@ static int ata_scsiop_read_capacity10(ccb *pccb) return -EPERM; }
- memset(buf, 0, 8); + cap = le32_to_cpu(ataid[pccb->target]->lba_capacity); + memcpy(pccb->pdata, &cap, sizeof(cap));
- *(u32 *) buf = le32_to_cpu(ataid[pccb->target]->lba_capacity); - - buf[6] = 512 >> 8; - buf[7] = 512 & 0xff; - - memcpy(pccb->pdata, buf, 8); + pccb->pdata[4] = pccb->pdata[5] = 0; + pccb->pdata[6] = 512 >> 8; + pccb->pdata[7] = 512 & 0xff;
return 0; }
The version of dlmalloc we are using generates a fair number of warnings of the following form:
warning: dereferencing pointer '({anonymous})' does break strict-aliasing rules
Trying to fix these is just paper taping over the issue and we really just need to look into a new malloc implementation. So we disable these particular warning checks for dlmalloc.c only.
Signed-off-by: Kumar Gala galak@kernel.crashing.org --- common/Makefile | 1 + 1 files changed, 1 insertions(+), 0 deletions(-)
diff --git a/common/Makefile b/common/Makefile index c8e5d26..5b3490a 100644 --- a/common/Makefile +++ b/common/Makefile @@ -34,6 +34,7 @@ COBJS-y += console.o COBJS-y += command.o COBJS-y += devices.o COBJS-y += dlmalloc.o +CFLAGS_dlmalloc.o += -fno-strict-aliasing COBJS-y += exports.o COBJS-$(CONFIG_SYS_HUSH_PARSER) += hush.o COBJS-y += image.o
Dear Kumar Gala,
In message 1247495041-8605-3-git-send-email-galak@kernel.crashing.org you wrote:
The version of dlmalloc we are using generates a fair number of warnings of the following form:
warning: dereferencing pointer '({anonymous})' does break strict-aliasing rules
Trying to fix these is just paper taping over the issue and we really just need to look into a new malloc implementation. So we disable these particular warning checks for dlmalloc.c only.
Signed-off-by: Kumar Gala galak@kernel.crashing.org
Well, if you think fixing the problems is "paper taping over the issue", then how do you call NOT fixing the problems and shutting off the warnings?
NAK.
Best regards,
Wolfgang Denk
On Thu, 2009-07-23 at 21:06 +0200, Wolfgang Denk wrote:
Dear Kumar Gala,
In message 1247495041-8605-3-git-send-email-galak@kernel.crashing.org you wrote:
The version of dlmalloc we are using generates a fair number of warnings of the following form:
warning: dereferencing pointer '({anonymous})' does break strict-aliasing rules
Trying to fix these is just paper taping over the issue and we really just need to look into a new malloc implementation. So we disable these particular warning checks for dlmalloc.c only.
Signed-off-by: Kumar Gala galak@kernel.crashing.org
Well, if you think fixing the problems is "paper taping over the issue", then how do you call NOT fixing the problems and shutting off the warnings?
Well he removed the optimization so the problem is no longer a problem. It's the description of what was done that is wrong.
Dear Kenneth Johansson,
In message 1248453626.8967.27.camel@localhost.localdomain you wrote:
Well, if you think fixing the problems is "paper taping over the issue", then how do you call NOT fixing the problems and shutting off the warnings?
Well he removed the optimization so the problem is no longer a problem.
Of course it still is a problem. It has NOT been fixed - just hushed up.
Best regards,
Wolfgang Denk
On Sun, Jul 26, 2009 at 11:26:18PM +0200, Wolfgang Denk wrote:
Dear Kenneth Johansson,
In message 1248453626.8967.27.camel@localhost.localdomain you wrote:
Well, if you think fixing the problems is "paper taping over the issue", then how do you call NOT fixing the problems and shutting off the warnings?
Well he removed the optimization so the problem is no longer a problem.
Of course it still is a problem. It has NOT been fixed - just hushed up.
The patch title is bad -- it's not disabling warnings, it's disabling an aspect of C99 that the code is incompatible with (and which is pretty questionable in the first place with such low level code). Note that in Linux, this is disabled for the entire kernel.
As things stand, GCC may do bad things with that code. With this patch, it may not (at least not this particular sort of bad thing). Those bad things are not limited to the places where it warns -- those are just the violations it detected.
Do you have an alternative malloc implementation in mind that is designed to work with strict aliasing, or a suggested fix to the current one?
-Scott
Dear Scott,
In message 20090728225244.GA8187@b07421-ec1.am.freescale.net you wrote:
The patch title is bad -- it's not disabling warnings, it's disabling an aspect of C99 that the code is incompatible with (and which is pretty questionable in the first place with such low level code). Note that in Linux, this is disabled for the entire kernel.
I know. But Linux is bigger than U-Boot, and I think we should be able to fix the few isolated places that throw such warnings.
As things stand, GCC may do bad things with that code. With this patch, it may not (at least not this particular sort of bad thing). Those bad things are not limited to the places where it warns -- those are just the violations it detected.
Agreed, and that's why I want to see this fixed.
Do you have an alternative malloc implementation in mind that is designed to work with strict aliasing, or a suggested fix to the current one?
I did not look into this yet - there was some discussion about a malloc replacement, but it faded away without visible result.
I cannot do everything myself, but I can oppose changes that are IMO to the worse.
Best regards,
Wolfgang Denk
On Jul 29, 2009, at 12:47 AM, Wolfgang Denk wrote:
Dear Scott,
In message 20090728225244.GA8187@b07421-ec1.am.freescale.net you wrote:
The patch title is bad -- it's not disabling warnings, it's disabling an aspect of C99 that the code is incompatible with (and which is pretty questionable in the first place with such low level code). Note that in Linux, this is disabled for the entire kernel.
I know. But Linux is bigger than U-Boot, and I think we should be able to fix the few isolated places that throw such warnings.
As things stand, GCC may do bad things with that code. With this patch, it may not (at least not this particular sort of bad thing). Those bad things are not limited to the places where it warns -- those are just the violations it detected.
Agreed, and that's why I want to see this fixed.
Do you have an alternative malloc implementation in mind that is designed to work with strict aliasing, or a suggested fix to the current one?
I did not look into this yet - there was some discussion about a malloc replacement, but it faded away without visible result.
I cannot do everything myself, but I can oppose changes that are IMO to the worse.
Do we have any ideas what type of performance we're looking for from malloc?
- k
On Wed, 2009-07-29 at 07:47 +0200, Wolfgang Denk wrote:
Dear Scott,
In message 20090728225244.GA8187@b07421-ec1.am.freescale.net you wrote:
The patch title is bad -- it's not disabling warnings, it's disabling an aspect of C99 that the code is incompatible with (and which is pretty questionable in the first place with such low level code). Note that in Linux, this is disabled for the entire kernel.
I know. But Linux is bigger than U-Boot, and I think we should be able to fix the few isolated places that throw such warnings.
As things stand, GCC may do bad things with that code. With this patch, it may not (at least not this particular sort of bad thing). Those bad things are not limited to the places where it warns -- those are just the violations it detected.
Agreed, and that's why I want to see this fixed.
Personally I feel this optimization to be in the "luxury" category. I'm not sure it's worth the trouble in a project like u-boot. Maybe it should be default off and only turned on for specific modules. like checksum and compression stuff.
On Jul 29, 2009, at 12:47 AM, Wolfgang Denk wrote:
Dear Scott,
In message 20090728225244.GA8187@b07421-ec1.am.freescale.net you wrote:
The patch title is bad -- it's not disabling warnings, it's disabling an aspect of C99 that the code is incompatible with (and which is pretty questionable in the first place with such low level code). Note that in Linux, this is disabled for the entire kernel.
I know. But Linux is bigger than U-Boot, and I think we should be able to fix the few isolated places that throw such warnings.
As things stand, GCC may do bad things with that code. With this patch, it may not (at least not this particular sort of bad thing). Those bad things are not limited to the places where it warns -- those are just the violations it detected.
Agreed, and that's why I want to see this fixed.
I can understand that desire, however it seems a lot of our platforms are already globally turning on -fno-strict-aliasing:
cpu/74xx_7xx/config.mk:PLATFORM_RELFLAGS += -fPIC -ffixed-r14 -meabi - fno-strict-aliasing cpu/arm1136/config.mk:PLATFORM_RELFLAGS += -fno-strict-aliasing -fno- common -ffixed-r8 \ cpu/arm1176/config.mk:PLATFORM_RELFLAGS += -fno-strict-aliasing -fno- common -ffixed-r8 \ cpu/arm1176/s3c64xx/config.mk:PLATFORM_RELFLAGS += -fno-strict- aliasing -fno-common -ffixed-r8 \ cpu/arm720t/config.mk:PLATFORM_RELFLAGS += -fno-strict-aliasing -fno- common -ffixed-r8 \ cpu/arm920t/config.mk:PLATFORM_RELFLAGS += -fno-strict-aliasing -fno- common -ffixed-r8 \ cpu/arm925t/config.mk:PLATFORM_RELFLAGS += -fno-strict-aliasing -fno- common -ffixed-r8 \ cpu/arm926ejs/config.mk:PLATFORM_RELFLAGS += -fno-strict-aliasing - fno-common -ffixed-r8 \ cpu/arm926ejs/davinci/config.mk:PLATFORM_RELFLAGS += -fno-strict- aliasing -fno-common -ffixed-r8 \ cpu/arm946es/config.mk:PLATFORM_RELFLAGS += -fno-strict-aliasing -fno- common -ffixed-r8 \ cpu/arm_cortexa8/config.mk:PLATFORM_RELFLAGS += -fno-strict-aliasing - fno-common -ffixed-r8 \ cpu/arm_cortexa8/omap3/config.mk:PLATFORM_RELFLAGS += -fno-strict- aliasing -fno-common -ffixed-r8 \ cpu/arm_intcm/config.mk:PLATFORM_RELFLAGS += -fno-strict-aliasing - fno-common -ffixed-r8 \ cpu/ixp/config.mk:PLATFORM_RELFLAGS += -fno-strict-aliasing -fno- common -ffixed-r8 \ cpu/lh7a40x/config.mk:PLATFORM_RELFLAGS += -fno-strict-aliasing -fno- common -ffixed-r8 \ cpu/mpc824x/config.mk:PLATFORM_RELFLAGS += -fPIC -ffixed-r14 -meabi - fno-strict-aliasing cpu/mpc8xx/config.mk:PLATFORM_RELFLAGS += -fPIC -ffixed-r14 -meabi - fno-strict-aliasing cpu/ppc4xx/config.mk:PLATFORM_RELFLAGS += -fPIC -ffixed-r14 -meabi - fno-strict-aliasing cpu/pxa/config.mk:PLATFORM_RELFLAGS += -fno-strict-aliasing -fno- common -ffixed-r8 \ cpu/s3c44b0/config.mk:PLATFORM_RELFLAGS += -fno-strict-aliasing -fno- common -ffixed-r8 \ cpu/sa1100/config.mk:PLATFORM_RELFLAGS += -fno-strict-aliasing -fno- common -ffixed-r8 \
- k
Dear Kumar Gala,
In message 1247495041-8605-2-git-send-email-galak@kernel.crashing.org you wrote:
ahci.c: In function 'ata_scsiop_read_capacity10': ahci.c:616: warning: dereferencing type-punned pointer will break strict-aliasing rules
Signed-off-by: Kumar Gala galak@kernel.crashing.org
drivers/block/ahci.c | 14 ++++++-------- 1 files changed, 6 insertions(+), 8 deletions(-)
Applied, thanks.
Best regards,
Wolfgang Denk
Dear Kumar Gala,
In message 1247495041-8605-1-git-send-email-galak@kernel.crashing.org you wrote:
biosemu.c: In function 'BE_setVGA': biosemu.c:147: warning: dereferencing type-punned pointer will break strict-aliasing rules
Signed-off-by: Kumar Gala galak@kernel.crashing.org
drivers/bios_emulator/biosemu.c | 3 ++- 1 files changed, 2 insertions(+), 1 deletions(-)
Applied, thanks.
Best regards,
Wolfgang Denk
participants (4)
-
Kenneth Johansson -
Kumar Gala -
Scott Wood -
Wolfgang Denk