From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from mail-wm1-f68.google.com (mail-wm1-f68.google.com [209.85.128.68]) by mx.groups.io with SMTP id smtpd.web12.3863.1595588760196754040 for ; Fri, 24 Jul 2020 04:06:00 -0700 Authentication-Results: mx.groups.io; dkim=pass header.i=@nuviainc-com.20150623.gappssmtp.com header.s=20150623 header.b=OeUvllMg; spf=pass (domain: nuviainc.com, ip: 209.85.128.68, mailfrom: leif@nuviainc.com) Received: by mail-wm1-f68.google.com with SMTP id o8so7593731wmh.4 for ; Fri, 24 Jul 2020 04:05:59 -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=ZgIWGRTK6IRUG0ZzN3H4yvQ9h/BTBhCq/KGNvvx1/gw=; b=OeUvllMgbDwbOoQIvPjAtrfuyQrgWtDasEmXYS0Wjq1zY3rLRyJva7k1JodVsL6L9o KMpTlh5Hlprb74/XtIqvp4r4/dr79dGMBbgeNw3WaGdEQ91geVdJBwGWnQXfVzN6sADr SttoRXW4iRcK4KaLwW6OQqVflZiXPxjDbZHpLAGQDuX9S4rLaWe9+OGCxRWvwDuSG+11 ZjD06vNDvT285MUMdGcCUjloEbdN6EIUFGH3+65dyToG7TkE1jbVFUvME7wzdGEDDxuU 3VJfq7kSaoTGMVpL2dNdngiH912jhI5jpMCq2cga9B0qkibrywIGTiCXl8nowWlW887f iwHw== 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=ZgIWGRTK6IRUG0ZzN3H4yvQ9h/BTBhCq/KGNvvx1/gw=; b=fEic8Uh3qb6ue8ceeYAblodtSx7IT2Jzt/up2q00SB/pITSV+WgCDd3ddaRAizZ0Gr m99CN95mtUMby1n7ncBeX9pm8dZtAUNhm5PhUgzrPsuENsB5h9k5XREOwyxxYWPIxCAT OSiYUfsvZk6Un+SXw6MISnnTHCA3hNxGGGkA5ONnqGQwkHb7B81WX7wrn1KtZP7apVyy 69VTLJ45eaI5VKxFZQqPOAYofGPhrwyLDfnuUMkmHGWSysWKzJt8MWh04E5QeEK3+58D Q2rRTVFvluAM9o/TAcLV0peUmCkYAO08JrMtsPY/hYGrqpEagPK1yhUa1V6HUOwWmlam ti9g== X-Gm-Message-State: AOAM533+9Xr98x7111bUWDXtQ8qTqcJef9+tk60srmtNLRcR+68EMpp2 pwVGEgA/I1A9EpNmmOqodYkscA== X-Google-Smtp-Source: ABdhPJwTYv52Zvjj9Q4Cl+tUj0TDT0aoPtR6NeJtqB9jAoiZb7ag5fqWzf4pgKrbGSq51okRFw2xTA== X-Received: by 2002:a1c:f31a:: with SMTP id q26mr8047629wmq.59.1595588758609; Fri, 24 Jul 2020 04:05:58 -0700 (PDT) Return-Path: Received: from vanye ([2001:470:1f09:12f0:b26e:bfff:fea9:f1b8]) by smtp.gmail.com with ESMTPSA id x11sm915430wrl.28.2020.07.24.04.05.57 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Fri, 24 Jul 2020 04:05:58 -0700 (PDT) Date: Fri, 24 Jul 2020 12:05:55 +0100 From: "Leif Lindholm" To: Pierre Gondois Cc: "devel@edk2.groups.io" , "bob.c.feng@intel.com" , "afish@apple.com" , "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: <20200724110555.GS1337@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 On Fri, Jul 24, 2020 at 09:01:16 +0000, Pierre Gondois wrote: > Hello everyone, > > Thank you for reverting the patch. I should have tested building on more platforms. > I can fix the pieces of code highlighted by the -Wpointer-arith flag > on ARM platforms. For now I saw 10-15 places that would need to be > modified. > If this is not a huge task I will try to do it for other platforms. That would certainly be most appreciated. If not, I will get to it eventually, but it might drag out a few weeks. Regards, Leif > Regards, > Pierre > > -----Original Message----- > From: devel@edk2.groups.io On Behalf Of Bob Feng via groups.io > Sent: 24 July 2020 04:56 > To: Leif Lindholm ; devel@edk2.groups.io; afish@apple.com > Cc: Pierre Gondois ; Gao, Liming ; tomas@nuviainc.com > Subject: Re: [edk2-devel] [PATCH V2 1/2] BaseTools: Add gcc flag to warn on void* pointer arithmetic > > 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)' > > >> > > >> > > >> > > >> > > > > > > > > > > > > > > > > > > > > > > > > > > > IMPORTANT NOTICE: The contents of this email and any attachments are confidential and may also be privileged. If you are not the intended recipient, please notify the sender immediately and do not disclose the contents to any other person, use it for any purpose, or store or copy the information in any medium. Thank you.