From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from mail-wr1-f67.google.com (mail-wr1-f67.google.com [209.85.221.67]) by mx.groups.io with SMTP id smtpd.web10.3885.1595588618220189793 for ; Fri, 24 Jul 2020 04:03:38 -0700 Authentication-Results: mx.groups.io; dkim=pass header.i=@nuviainc-com.20150623.gappssmtp.com header.s=20150623 header.b=ea+A5ReQ; spf=pass (domain: nuviainc.com, ip: 209.85.221.67, mailfrom: leif@nuviainc.com) Received: by mail-wr1-f67.google.com with SMTP id f2so7911958wrp.7 for ; Fri, 24 Jul 2020 04:03:38 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=nuviainc-com.20150623.gappssmtp.com; s=20150623; h=date:from:to:cc:subject:message-id:references:mime-version :content-disposition:in-reply-to:user-agent; bh=bzZHMO5i3Z6cmPnTSYUFMgUeQrKdOynBEz/AgMCDqt0=; b=ea+A5ReQG7shw24mBaq4bpry5QTCViAY7nNzmxYkzM4g7ruMJtZAZt3QsagKNO8LVc r7iBroDUnBTnTfA+ildt6y3sMNptJxERFheROd67q1fhW0y+GTppnMmgTpuKRB78rkgj uI03I0ZxyHP7frTkyRwAj/ha7Yl9KugYPnEZ3UAte42yRHLnbkt8JyhYraJLPO4rT9JG WwRtT0UhhBpWP5lqBZXVNYwhtg+Y9+kkRmlPai90ME9xjq+Y6PDGb2MUe5f5qLrlE61q Q+GWctcxFr1xgICg8WZbJG97H+FmbKPEICRpslYUaaOmi89i6BcEEm1/ALF+uYZ7y0yL on7Q== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:date:from:to:cc:subject:message-id:references :mime-version:content-disposition:in-reply-to:user-agent; bh=bzZHMO5i3Z6cmPnTSYUFMgUeQrKdOynBEz/AgMCDqt0=; b=qZ/SmOZoUWatdLm8SY4+zzmyIfY62LfNfL6eLju/HkFg7sagJ+bJqecxhLnx40L7Vg UHo0+eUkrWZbZCDDa30M0Y8ocn+iM3ApMpVEYmczFTQBzD/N0q+Z8Tyj7GEKrxMEsfmG tjpC3bsAWQe74KCao3Owtqwe3sF1kV96ESWmau5wfLyhYp5HP0N8ppEm+riLppt6dIdh KtwLDxenXz6TWc4nkcMI+0O7qgJ3Aanx9Pth5gN225rAvfKOwnbA+RGH10hvOnPhF0e0 cF+ghyJnu5eVpdY5XtylpPhc9Ydwx+PhmEUgIGnbIMZXYJfOhg7Nv3NdJt/mCNmKsnTq KL8A== X-Gm-Message-State: AOAM533iLI3Q3UoGRhPtTUJb6rYWmLZOlE4DozNx60M6sjX9BwddWKf9 VoHk+8gKQNYgc3QHFITds6JHwQ== X-Google-Smtp-Source: ABdhPJzz3qRNImBFZQ53tnyfwU60Hl0HcO8vUA9WLh2CdboeYjlhV+WhHKl+b7MDvXESl40s9/PD0Q== X-Received: by 2002:a5d:4d4b:: with SMTP id a11mr8030352wru.230.1595588616726; Fri, 24 Jul 2020 04:03:36 -0700 (PDT) Return-Path: Received: from vanye ([2001:470:1f09:12f0:b26e:bfff:fea9:f1b8]) by smtp.gmail.com with ESMTPSA id c128sm7197316wma.5.2020.07.24.04.03.35 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Fri, 24 Jul 2020 04:03:35 -0700 (PDT) Date: Fri, 24 Jul 2020 12:03:33 +0100 From: "Leif Lindholm" To: "Feng, Bob C" Cc: "devel@edk2.groups.io" , "afish@apple.com" , PierreGondois , "Gao, Liming" , "tomas@nuviainc.com" Subject: Re: [edk2-devel] [PATCH V2 1/2] BaseTools: Add gcc flag to warn on void* pointer arithmetic Message-ID: <20200724110333.GR1337@vanye> References: <20200707083522.138944-1-pierre.gondois@arm.com> <20200707083522.138944-2-pierre.gondois@arm.com> <20200722180534.GG1337@vanye> <20200723093305.GJ1337@vanye> MIME-Version: 1.0 In-Reply-To: User-Agent: Mutt/1.10.1 (2018-07-13) Content-Type: text/plain; charset=us-ascii Content-Disposition: inline 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 > Sent: Thursday, July 23, 2020 5:33 PM > To: devel@edk2.groups.io; afish@apple.com > Cc: Feng, Bob C ; PierreGondois ; Gao, Liming ; tomas@nuviainc.com > 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 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 > > > > On Behalf Of > > > Leif Lindholm > > > Sent: Thursday, July 23, 2020 2:06 AM > > > To: devel@edk2.groups.io ; Feng, Bob C > > > > > > > Cc: PierreGondois > > >; Gao, Liming > > >; 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 > > >> > > >> > > >> -----Original Message----- > > >> From: PierreGondois > > >> Sent: Tuesday, July 7, 2020 4:35 PM > > >> To: devel@edk2.groups.io > > >> Cc: Pierre Gondois ; Feng, Bob C > > >> ; Gao, Liming ; > > >> 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 > > >> > > >> 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 > > >> --- > > >> > > >> 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.
# Portions copyright (c) 2008 - 2009, Apple Inc. > > >> All rights reserved.
-# Portions copyright (c) 2011 - 2019, ARM Ltd. > > >> All rights reserved.
> > >> +# Portions copyright (c) 2011 - 2020, ARM Ltd. All rights > > >> +reserved.
> > >> # Copyright (c) 2015, Hewlett-Packard Development Company, L.P.
# (C) Copyright 2020, Hewlett Packard Enterprise Development LP
# 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)' > > >> > > >> > > >> > > >> > > > > > > > > > > > > > > > > > > > > > > >