public inbox for devel@edk2.groups.io
 help / color / mirror / Atom feed
From: "Leif Lindholm" <leif@nuviainc.com>
To: devel@edk2.groups.io, afish@apple.com
Cc: bob.c.feng@intel.com, PierreGondois <pierre.gondois@arm.com>,
	"Gao, Liming" <liming.gao@intel.com>,
	"tomas@nuviainc.com" <tomas@nuviainc.com>
Subject: Re: [edk2-devel] [PATCH V2 1/2] BaseTools: Add gcc flag to warn on void* pointer arithmetic
Date: Thu, 23 Jul 2020 10:33:05 +0100	[thread overview]
Message-ID: <20200723093305.GJ1337@vanye> (raw)
In-Reply-To: <EF42DD5B-80D0-4230-81A1-EB29F970558A@apple.com>

Hi Andrew,

Agreed.

I also think this should be applied across all architectures, not just
ARM/AARCH64. Since Visual Studio has never been been able to compile
the affected code, I expect impact to Ia32/X64 to be minimal.

Regards,

Leif

On Wed, Jul 22, 2020 at 19:49:01 -0700, Andrew Fish via groups.io wrote:
> Bob,
> 
> It also looks like clang could use this flag as the default seems to
> be to follow the GCC behavior.
> 
> Thanks,
> 
> Andrew Fish
> 
> > On Jul 22, 2020, at 6:56 PM, Bob Feng <bob.c.feng@intel.com> wrote:
> > 
> > Hi Leif
> > 
> > I agree to revert that patch for now and I sent a revert patch for review. After resolving the build break issue for ARM/AARCH64 platforms in edk2-platforms, and make sure there is no platform build break with this patch, we will push it again.
> > 
> > Thanks,
> > Bob 
> > 
> > -----Original Message-----
> > From: devel@edk2.groups.io <mailto:devel@edk2.groups.io> <devel@edk2.groups.io <mailto:devel@edk2.groups.io>> On Behalf Of Leif Lindholm
> > Sent: Thursday, July 23, 2020 2:06 AM
> > To: devel@edk2.groups.io <mailto:devel@edk2.groups.io>; Feng, Bob C <bob.c.feng@intel.com <mailto:bob.c.feng@intel.com>>
> > Cc: PierreGondois <pierre.gondois@arm.com <mailto:pierre.gondois@arm.com>>; Gao, Liming <liming.gao@intel.com <mailto:liming.gao@intel.com>>; tomas@nuviainc.com <mailto:tomas@nuviainc.com>
> > Subject: Re: [edk2-devel] [PATCH V2 1/2] BaseTools: Add gcc flag to warn on void* pointer arithmetic
> > 
> > Hi Bob,
> > 
> > This patch also breaks about half of the ARM/AARCH64 platforms in edk2-platforms. I agree it should go in at a later stage, but for now, can we please revert it?
> > 
> > Regards,
> > 
> > Leif
> > 
> > On Mon, Jul 20, 2020 at 04:10:27 +0000, Bob Feng wrote:
> >> Reviewed-by: Bob Feng<bob.c.feng@intel.com>
> >> 
> >> 
> >> -----Original Message-----
> >> From: PierreGondois <pierre.gondois@arm.com>
> >> Sent: Tuesday, July 7, 2020 4:35 PM
> >> To: devel@edk2.groups.io
> >> Cc: Pierre Gondois <Pierre.Gondois@arm.com>; Feng, Bob C 
> >> <bob.c.feng@intel.com>; Gao, Liming <liming.gao@intel.com>; 
> >> tomas.pilar@arm.com; nd@arm.com
> >> Subject: [PATCH V2 1/2] BaseTools: Add gcc flag to warn on void* 
> >> pointer arithmetic
> >> 
> >> From: Pierre Gondois <pierre.gondois@arm.com>
> >> 
> >> By default, gcc allows void* pointer arithmetic.
> >> This is a GCC extension.
> >> However:
> >> - the C reference manual states that void*
> >>   pointer "cannot be operands of addition
> >>   or subtraction operators". Cf s5.3.1
> >>   "Generic Pointers";
> >> - Visual studio compiler treat such operation as
> >>   an error.
> >> 
> >> To prevent such pointer arithmetic, the "-Wpointer-arith"
> >> flag should be set for all GCC versions.
> >> 
> >> The "-Wpointer-arith"  allows to:
> >>  "Warn about anything that depends on the "size of"
> >>  a function type or of void. GNU C assigns these
> >>  types a size of 1, for convenience in calculations
> >>  with void * pointers and pointers to functions."
> >> 
> >> This flag is available since GCC2.95.3 which came out in 2001.
> >> 
> >> Signed-off-by: Pierre Gondois <pierre.gondois@arm.com>
> >> ---
> >> 
> >> The changes can be seen at: 
> >> https://github.com/PierreARM/edk2/commits/831_Add_gcc_flag_warning_v2
> >> 
> >> Notes:
> >>    v1:
> >>     - Add "-Wpointer-arith" gcc flag. [Pierre]
> >>    v2:
> >>     - Only add the flag for ARM and AARCH64. [Tomas]
> >> 
> >> BaseTools/Conf/tools_def.template | 6 +++---
> >> 1 file changed, 3 insertions(+), 3 deletions(-)
> >> 
> >> diff --git a/BaseTools/Conf/tools_def.template 
> >> b/BaseTools/Conf/tools_def.template
> >> index 
> >> 8aeb8a2a6417e41c5660cda5066f52adc8cc3089..397b011ba38f97f81f314f8641ac
> >> 8bb95d5a2197 100755
> >> --- a/BaseTools/Conf/tools_def.template
> >> +++ b/BaseTools/Conf/tools_def.template
> >> @@ -1,7 +1,7 @@
> >> #
> >> #  Copyright (c) 2006 - 2018, Intel Corporation. All rights 
> >> reserved.<BR>  #  Portions copyright (c) 2008 - 2009, Apple Inc. All 
> >> rights reserved.<BR> -#  Portions copyright (c) 2011 - 2019, ARM Ltd. 
> >> All rights reserved.<BR>
> >> +#  Portions copyright (c) 2011 - 2020, ARM Ltd. All rights 
> >> +reserved.<BR>
> >> #  Copyright (c) 2015, Hewlett-Packard Development Company, L.P.<BR>  #  (C) Copyright 2020, Hewlett Packard Enterprise Development LP<BR>  #  Copyright (c) Microsoft Corporation
> >> @@ -1921,9 +1921,9 @@ NOOPT_*_*_OBJCOPY_ADDDEBUGFLAG     = --add-gnu-debuglink=$(DEBUG_DIR)/$(MODULE_N
> >> DEFINE GCC_ALL_CC_FLAGS            = -g -Os -fshort-wchar -fno-builtin -fno-strict-aliasing -Wall -Werror -Wno-array-bounds -include AutoGen.h -fno-common
> >> DEFINE GCC_IA32_CC_FLAGS           = DEF(GCC_ALL_CC_FLAGS) -m32 -malign-double -freorder-blocks -freorder-blocks-and-partition -O2 -mno-stack-arg-probe
> >> DEFINE GCC_X64_CC_FLAGS            = DEF(GCC_ALL_CC_FLAGS) -mno-red-zone -Wno-address -mno-stack-arg-probe
> >> -DEFINE GCC_ARM_CC_FLAGS            = DEF(GCC_ALL_CC_FLAGS) -mlittle-endian -mabi=aapcs -fno-short-enums -funsigned-char -ffunction-sections -fdata-sections -fomit-frame-pointer -Wno-address -mthumb -mfloat-abi=soft -fno-pic -fno-pie
> >> +DEFINE GCC_ARM_CC_FLAGS            = DEF(GCC_ALL_CC_FLAGS) -Wpointer-arith -mlittle-endian -mabi=aapcs -fno-short-enums -funsigned-char -ffunction-sections -fdata-sections -fomit-frame-pointer -Wno-address -mthumb -mfloat-abi=soft -fno-pic -fno-pie
> >> DEFINE GCC_ARM_CC_XIPFLAGS         = -mno-unaligned-access
> >> -DEFINE GCC_AARCH64_CC_FLAGS        = DEF(GCC_ALL_CC_FLAGS) -mlittle-endian -fno-short-enums -fverbose-asm -funsigned-char  -ffunction-sections -fdata-sections -Wno-address -fno-asynchronous-unwind-tables -fno-unwind-tables -fno-pic -fno-pie -ffixed-x18
> >> +DEFINE GCC_AARCH64_CC_FLAGS        = DEF(GCC_ALL_CC_FLAGS) -Wpointer-arith -mlittle-endian -fno-short-enums -fverbose-asm -funsigned-char -ffunction-sections -fdata-sections -Wno-address -fno-asynchronous-unwind-tables -fno-unwind-tables -fno-pic -fno-pie -ffixed-x18
> >> DEFINE GCC_AARCH64_CC_XIPFLAGS     = -mstrict-align -mgeneral-regs-only
> >> DEFINE GCC_DLINK_FLAGS_COMMON      = -nostdlib --pie
> >> DEFINE GCC_DLINK2_FLAGS_COMMON     = -Wl,--script=$(EDK_TOOLS_PATH)/Scripts/GccBase.lds
> >> --
> >> 'Guid(CE165669-3EF3-493F-B85D-6190EE5B9759)'
> >> 
> >> 
> >> 
> >> 
> > 
> > 
> > 
> > 
> > 
> 
> 
> 
> 

  reply	other threads:[~2020-07-23  9:33 UTC|newest]

Thread overview: 31+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-07-07  8:35 [PATCH V2 0/2] Add gcc flag for void* pointer arithmetics PierreGondois
2020-07-07  8:35 ` [PATCH V2 1/2] BaseTools: Add gcc flag to warn on void* pointer arithmetic PierreGondois
2020-07-16  9:07   ` [edk2-devel] " Yuwei Chen
2020-07-20  4:10   ` Bob Feng
2020-07-22 18:05     ` [edk2-devel] " Leif Lindholm
2020-07-22 21:13       ` Andrew Fish
2020-07-23  1:56       ` Bob Feng
2020-07-23  2:49         ` Andrew Fish
2020-07-23  9:33           ` Leif Lindholm [this message]
2020-07-24  3:56             ` Bob Feng
2020-07-24  9:01               ` PierreGondois
2020-07-24 11:05                 ` Leif Lindholm
2020-07-24 11:03               ` Leif Lindholm
2020-07-07  8:35 ` [PATCH V2 2/2] BaseTools: Factorize GCC flags PierreGondois
2020-07-20  4:11   ` Bob Feng
2020-07-30 12:08     ` [edk2-devel] " Leif Lindholm
2020-07-22 11:03   ` Laszlo Ersek
2020-07-22 11:24     ` Laszlo Ersek
2020-08-26 16:42     ` Laszlo Ersek
2020-08-27  8:32       ` PierreGondois
2020-08-27 14:55         ` Laszlo Ersek
2020-08-27 15:25           ` Leif Lindholm
2020-08-28 16:56             ` Laszlo Ersek
2020-08-28 19:15               ` Leif Lindholm
2020-08-31 13:22                 ` Ard Biesheuvel
2020-08-31 13:43                   ` 回复: " gaoliming
2020-08-31 14:03                   ` Laszlo Ersek
2020-08-31 14:37                     ` Ard Biesheuvel
2020-08-31 16:18                       ` Laszlo Ersek
2020-08-31 16:27                         ` Laszlo Ersek
2020-08-31 17:14                           ` Ard Biesheuvel

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-list from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20200723093305.GJ1337@vanye \
    --to=devel@edk2.groups.io \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox