Re: [edk2-devel] [PATCH V2 1/2] BaseTools: Add gcc flag to warn on void* pointer arithmetic

["Andrew Fish" <afish@apple.com>]

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

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)'
>> 
>> 
>> 
>> 
> 
> 
> 
> 
> 


[-- Attachment #2: Type: text/html, Size: 29524 bytes --]