Hi Bob,
I would say we should start by sending out an RFC set updating
tools_def.template for the various toolchains, and then make sure we
fix things in other trees (I would also like to, for example, verify
this doesn't break edk2-libc). Once other trees are cleaned up, we
can send out a PATCH and merge to master.
This is not critical for edk2-stable202008, but it is *useful*, so it
would be good if we could hit that.
/
Leif
On Fri, Jul 24, 2020 at 03:56:25 +0000, Feng, Bob C wrote:
> What would be your suggestion on the approach to apply this build
> option to all architectures? Change the tool_def.txt firstly and
> push platform owner to fix the corresponding firmware code, or
> changing the tool_def.txt later, until this change will not break
> any platform build?
>
> -----Original Message-----
> From: Leif Lindholm <[email protected]>
> Sent: Thursday, July 23, 2020 5:33 PM
> To: [email protected]; [email protected]
> Cc: Feng, Bob C <[email protected]>; PierreGondois
> <[email protected]>; Gao, Liming <[email protected]>;
> [email protected]
> Subject: Re: [edk2-devel] [PATCH V2 1/2] BaseTools: Add gcc flag to warn on
> void* pointer arithmetic
>
> 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 <[email protected]> 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: [email protected] <mailto:[email protected]>
> > > <[email protected] <mailto:[email protected]>> On Behalf Of
> > > Leif Lindholm
> > > Sent: Thursday, July 23, 2020 2:06 AM
> > > To: [email protected] <mailto:[email protected]>; Feng, Bob C
> > > <[email protected] <mailto:[email protected]>>
> > > Cc: PierreGondois <[email protected]
> > > <mailto:[email protected]>>; Gao, Liming <[email protected]
> > > <mailto:[email protected]>>; [email protected]
> > > <mailto:[email protected]>
> > > 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<[email protected]>
> > >>
> > >>
> > >> -----Original Message-----
> > >> From: PierreGondois <[email protected]>
> > >> Sent: Tuesday, July 7, 2020 4:35 PM
> > >> To: [email protected]
> > >> Cc: Pierre Gondois <[email protected]>; Feng, Bob C
> > >> <[email protected]>; Gao, Liming <[email protected]>;
> > >> [email protected]; [email protected]
> > >> Subject: [PATCH V2 1/2] BaseTools: Add gcc flag to warn on void*
> > >> pointer arithmetic
> > >>
> > >> From: Pierre Gondois <[email protected]>
> > >>
> > >> 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 <[email protected]>
> > >> ---
> > >>
> > >> 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..397b011ba38f97f81f314f864
> > >> 1ac
> > >> 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)'
> > >>
> > >>
> > >>
> > >>
> > >
> > >
> > >
> > >
> > >
> >
> >
> >
> >
-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#63242): https://edk2.groups.io/g/devel/message/63242
Mute This Topic: https://groups.io/mt/75351535/21656
Group Owner: [email protected]
Unsubscribe: https://edk2.groups.io/g/devel/unsub [[email protected]]
-=-=-=-=-=-=-=-=-=-=-=-
