public inbox for devel@edk2.groups.io
 help / color / mirror / Atom feed
From: "Tomas Pilar (tpilar)" <Tomas.Pilar@arm.com>
To: "devel@edk2.groups.io" <devel@edk2.groups.io>,
	Pierre Gondois <Pierre.Gondois@arm.com>
Cc: "bob.c.feng@intel.com" <bob.c.feng@intel.com>,
	"liming.gao@intel.com" <liming.gao@intel.com>, nd <nd@arm.com>
Subject: Re: [edk2-devel] [PATCH v1] BaseTools: Add gcc flag to warn on void* pointer arithmetic
Date: Tue, 30 Jun 2020 16:29:58 +0000	[thread overview]
Message-ID: <AM5PR0801MB1844E219C0913F43A969FCACE76F0@AM5PR0801MB1844.eurprd08.prod.outlook.com> (raw)
In-Reply-To: <20200630152416.28240-1-pierre.gondois@arm.com>

It looks like GCC5_X64_CC_FLAGS pick up GCC48_ALL_CC_FLAGS as the lowest level flag list, so this change will not apply to compilations on the X64 architecture.

That said, I suspect that X64 architecture will likely have the biggest issues with disallowing void* arithmetic, so we either:

1. Mend the tools_def to make GCC48_ALL_CC_FLAGS pick up GCC_ALL_CC_FLAGS, rerun tests and CI to make sure this change does not break large parts of existing codebase.
2. Move this change to ARM/AARCH64 low level CC_FLAGS lists to explicitly exclude this change from X64 architecture compilation.

Cheers,
Tom

NB. I personally like the void* arithmetic GNU C extension - I like being able to combine the implicit pointer casts with raw pointer arithmetic. But I can appreciate if we want to keep the codebase strict.

-----Original Message-----
From: devel@edk2.groups.io <devel@edk2.groups.io> On Behalf Of PierreGondois via groups.io
Sent: 30 June 2020 16:24
To: devel@edk2.groups.io
Cc: Pierre Gondois <Pierre.Gondois@arm.com>; bob.c.feng@intel.com; liming.gao@intel.com; nd <nd@arm.com>
Subject: [edk2-devel] [PATCH v1] 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_v1
Notes:
    v1:
     - Add "-Wpointer-arith" gcc flag. [Pierre]

 BaseTools/Conf/tools_def.template | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/BaseTools/Conf/tools_def.template b/BaseTools/Conf/tools_def.template
index 8aeb8a2a6417e41c5660cda5066f52adc8cc3089..fba71665d6ade4dad08a5a0c343f357fd413a8a4 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
@@ -1918,7 +1918,7 @@ NOOPT_*_*_OBJCOPY_ADDDEBUGFLAG     = --add-gnu-debuglink=$(DEBUG_DIR)/$(MODULE_N
 *_*_*_DTCPP_PATH                   = DEF(DTCPP_BIN)
 *_*_*_DTC_PATH                     = DEF(DTC_BIN)
 
-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_ALL_CC_FLAGS            = -g -Os -fshort-wchar -fno-builtin -fno-strict-aliasing -Wall -Werror -Wpointer-arith -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
-- 
'Guid(CE165669-3EF3-493F-B85D-6190EE5B9759)'





      parent reply	other threads:[~2020-06-30 16:30 UTC|newest]

Thread overview: 3+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-06-30 15:24 [PATCH v1] BaseTools: Add gcc flag to warn on void* pointer arithmetic PierreGondois
2020-06-30 15:36 ` [EXTERNAL] [edk2-devel] " Bret Barkelew
2020-06-30 16:29 ` Tomas Pilar (tpilar) [this message]

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=AM5PR0801MB1844E219C0913F43A969FCACE76F0@AM5PR0801MB1844.eurprd08.prod.outlook.com \
    --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