From mboxrd@z Thu Jan 1 00:00:00 1970 Authentication-Results: mx.groups.io; dkim=missing; spf=pass (domain: redhat.com, ip: 209.132.183.28, mailfrom: lersek@redhat.com) Received: from mx1.redhat.com (mx1.redhat.com [209.132.183.28]) by groups.io with SMTP; Mon, 29 Apr 2019 11:44:05 -0700 Received: from smtp.corp.redhat.com (int-mx03.intmail.prod.int.phx2.redhat.com [10.5.11.13]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mx1.redhat.com (Postfix) with ESMTPS id BFCF6A85F; Mon, 29 Apr 2019 18:44:04 +0000 (UTC) Received: from lacos-laptop-7.usersys.redhat.com (ovpn-121-31.rdu2.redhat.com [10.10.121.31]) by smtp.corp.redhat.com (Postfix) with ESMTP id C31D71915B; Mon, 29 Apr 2019 18:44:03 +0000 (UTC) Subject: Re: [edk2-devel] [Patch] BaseTools: Add GCC flags to Basetool build. To: devel@edk2.groups.io, bob.c.feng@intel.com Cc: Liming Gao References: <20190429080144.11700-1-bob.c.feng@intel.com> From: "Laszlo Ersek" Message-ID: <9c02bd29-d640-ed1b-d9f4-e5a58a429aef@redhat.com> Date: Mon, 29 Apr 2019 20:44:02 +0200 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:52.0) Gecko/20100101 Thunderbird/52.9.1 MIME-Version: 1.0 In-Reply-To: <20190429080144.11700-1-bob.c.feng@intel.com> X-Scanned-By: MIMEDefang 2.79 on 10.5.11.13 X-Greylist: Sender IP whitelisted, not delayed by milter-greylist-4.5.16 (mx1.redhat.com [10.5.110.29]); Mon, 29 Apr 2019 18:44:04 +0000 (UTC) Content-Type: text/plain; charset=utf-8 Content-Language: en-US Content-Transfer-Encoding: 7bit Hi Bob, On 04/29/19 10:01, Bob Feng wrote: > BZ:https://bugzilla.tianocore.org/show_bug.cgi?id=1764 > > Some compiler flags restrict the compiler from making > arbitrary decisions while handling undefined C/C++ behaviors. > Therefore they can be used to fix some issues caused by undefined behavior. > > For example, for GCC, the following flags are available: > -fno-strict-overflow tells the compiler NOT to assume > that signed overflow does not occur. > -fno-delete-null-pointer-checks tells > the compiler NOT to assume that null pointer deference does not exist. > -fwrapv tells the compiler that signed overflow always wraps. > > This patch is going to add these 3 build options to > BaseTool GCC build option. > > Signed-off-by: Bob Feng > Cc: Liming Gao > --- > BaseTools/Source/C/Makefiles/header.makefile | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/BaseTools/Source/C/Makefiles/header.makefile b/BaseTools/Source/C/Makefiles/header.makefile > index 90fb3453ad..f1aed62769 100644 > --- a/BaseTools/Source/C/Makefiles/header.makefile > +++ b/BaseTools/Source/C/Makefiles/header.makefile > @@ -68,11 +68,11 @@ BUILD_OPTFLAGS = -O2 $(EXTRA_OPTFLAGS) > > ifeq ($(DARWIN),Darwin) > # assume clang or clang compatible flags on OS X > BUILD_CFLAGS = -MD -fshort-wchar -fno-strict-aliasing -Wall -Werror -Wno-deprecated-declarations -Wno-self-assign -Wno-unused-result -nostdlib -g > else > -BUILD_CFLAGS = -MD -fshort-wchar -fno-strict-aliasing -Wall -Werror -Wno-deprecated-declarations -Wno-stringop-truncation -Wno-restrict -Wno-unused-result -nostdlib -g > +BUILD_CFLAGS = -MD -fshort-wchar -fno-strict-aliasing -Wall -Werror -Wno-deprecated-declarations -Wno-stringop-truncation -Wno-restrict -Wno-unused-result -nostdlib -g -fwrapv -fno-delete-null-pointer-checks -fno-strict-overflow (1) This line is hard to review. Please add a patch first that implements no functional changes, just splits this line into multiple <=80char lines, using the backslash. (2) When you add "-W" and "-f" options, please try to preserve the grouping. We already have two "-f" options, so the new ones should be adjacent to those. (3) My gcc-4.8 manual states, under "-fstrict-overflow": See also the -fwrapv option. Using -fwrapv means that integer signed overflow is fully defined: it wraps. When -fwrapv is used, there is no difference between -fstrict-overflow and -fno-strict-overflow for integers. With -fwrapv certain types of overflow are permitted. For example, if the compiler gets an overflow when doing arithmetic on constants, the overflowed value can still be used with -fwrapv, but not otherwise. The point is that, with gcc-4.8, "-fno-strict-overflow" is redundant, once we specify "-fwrapv" -- assuming we don't use floating point in the C-language utilities in BaseTools. Additionally, checking the latest published GCC docs on the web: https://gcc.gnu.org/onlinedocs/gcc-8.3.0/gcc/Option-Summary.html it looks like the "-f[no-]strict-overflow" option has been removed altogether. Therefore I believe we should drop "-fno-strict-overflow". Thanks, Laszlo > endif > BUILD_LFLAGS = > BUILD_CXXFLAGS = -Wno-unused-result > > ifeq ($(HOST_ARCH), IA32) >