From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mx1.redhat.com (mx1.redhat.com [209.132.183.28]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by ml01.01.org (Postfix) with ESMTPS id 8DACF2193931C for ; Mon, 3 Apr 2017 09:34:31 -0700 (PDT) Received: from smtp.corp.redhat.com (int-mx05.intmail.prod.int.phx2.redhat.com [10.5.11.15]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mx1.redhat.com (Postfix) with ESMTPS id E717CC04B924; Mon, 3 Apr 2017 16:34:30 +0000 (UTC) DMARC-Filter: OpenDMARC Filter v1.3.2 mx1.redhat.com E717CC04B924 Authentication-Results: ext-mx07.extmail.prod.ext.phx2.redhat.com; dmarc=none (p=none dis=none) header.from=redhat.com Authentication-Results: ext-mx07.extmail.prod.ext.phx2.redhat.com; spf=pass smtp.mailfrom=lersek@redhat.com DKIM-Filter: OpenDKIM Filter v2.11.0 mx1.redhat.com E717CC04B924 Received: from lacos-laptop-7.usersys.redhat.com (ovpn-117-102.phx2.redhat.com [10.3.117.102]) by smtp.corp.redhat.com (Postfix) with ESMTP id CD41017575; Mon, 3 Apr 2017 16:34:29 +0000 (UTC) To: Ard Biesheuvel References: <559D2DF22BC9A3468B4FA1AA547F0EF102551554@shsmsx102.ccr.corp.intel.com> <1eca3726-e92f-7fbb-d61f-f8026c0b4762@redhat.com> Cc: "Song, BinX" , "edk2-devel@lists.01.org" , "Gao, Liming" From: Laszlo Ersek Message-ID: <7fe99ab9-78c8-94d5-fa00-94cd342768be@redhat.com> Date: Mon, 3 Apr 2017 18:34:28 +0200 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:45.0) Gecko/20100101 Thunderbird/45.8.0 MIME-Version: 1.0 In-Reply-To: X-Scanned-By: MIMEDefang 2.79 on 10.5.11.15 X-Greylist: Sender IP whitelisted, not delayed by milter-greylist-4.5.16 (mx1.redhat.com [10.5.110.31]); Mon, 03 Apr 2017 16:34:31 +0000 (UTC) Subject: Re: [PATCH] MdeModulePkg: Fix GCC48/GCC49 build error X-BeenThere: edk2-devel@lists.01.org X-Mailman-Version: 2.1.22 Precedence: list List-Id: EDK II Development List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Mon, 03 Apr 2017 16:34:31 -0000 Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: 7bit On 04/03/17 18:21, Ard Biesheuvel wrote: > On 3 April 2017 at 17:16, Laszlo Ersek wrote: >> adding Ard >> >> On 04/01/17 10:38, Song, BinX wrote: >>> - Fix GCC48/GCC49 build error >>> >>> Cc: Liming Gao >>> Contributed-under: TianoCore Contribution Agreement 1.0 >>> Signed-off-by: Bell Song >>> --- >>> .../Library/BrotliCustomDecompressLib/BrotliCustomDecompressLib.inf | 3 +++ >>> 1 file changed, 3 insertions(+) >>> >>> diff --git a/MdeModulePkg/Library/BrotliCustomDecompressLib/BrotliCustomDecompressLib.inf b/MdeModulePkg/Library/BrotliCustomDecompressLib/BrotliCustomDecompressLib.inf >>> index 578f97f..4c9aff5 100644 >>> --- a/MdeModulePkg/Library/BrotliCustomDecompressLib/BrotliCustomDecompressLib.inf >>> +++ b/MdeModulePkg/Library/BrotliCustomDecompressLib/BrotliCustomDecompressLib.inf >>> @@ -54,3 +54,6 @@ >>> DebugLib >>> BaseMemoryLib >>> ExtractGuidedSectionLib >>> + >>> +[BuildOptions] >>> + GCC:*_*_*_CC_FLAGS = -fno-builtin >>> >> >> In "BaseTools/Conf/tools_def.template", we currently have: >> >> DEFINE GCC_ARM_CC_FLAGS = DEF(GCC_ALL_CC_FLAGS) [...] -fno-builtin [...] >> DEFINE GCC_AARCH64_CC_FLAGS = DEF(GCC_ALL_CC_FLAGS) [...] -fno-builtin [...] >> >> DEFINE GCC5_IA32_CC_FLAGS = DEF(GCC49_IA32_CC_FLAGS) -fno-builtin >> DEFINE GCC5_X64_CC_FLAGS = DEF(GCC49_X64_CC_FLAGS) -fno-builtin >> >> Now, GCC5_IA32_CC_FLAGS goes back to GCC44_ALL_CC_FLAGS, via: >> - GCC49_IA32_CC_FLAGS >> - GCC48_IA32_CC_FLAGS >> - GCC47_IA32_CC_FLAGS >> - GCC46_IA32_CC_FLAGS >> - GCC45_IA32_CC_FLAGS >> - GCC44_IA32_CC_FLAGS >> - GCC44_ALL_CC_FLAGS >> >> (similarly for GCC5_X64_CC_FLAGS.) >> >> So, instead of this patch for BrotliCustomDecompressLib, how about: >> >> - moving "-fno-builtin" from >> GCC_ARM_CC_FLAGS and >> GCC_AARCH64_CC_FLAGS >> to >> GCC_ALL_CC_FLAGS, and >> >> - moving "-fno-builtin" from >> GCC5_IA32_CC_FLAGS and >> GCC5_X64_CC_FLAGS >> to >> GCC44_ALL_CC_FLAGS? >> >> Do we have any reason for permitting builtins at all? >> > > Well, one thing I noticed the other day is that GCC does not > 'recognize' memcpy() and memset() when -fno-builtin is defined, which > means trivial memcpys and memsets will not be inlined. But memcpy() and memset(), as written, cannot be called in edk2 anyway - explicitly, because we don't allow that, - implicitly (via struct assignment or initialization, for example), because we forbid that as well -- source code is supposed to use CopyMem(), CopyGuid(), and the like. So, yes, memcpy() and memset() would not be inlined with my suggestion, but edk2 code shouldn't exist in the first place that leads to the generation of memcpy() and memset() calls. > I guess that > argues for not permitting it at all, but it also means adding it > unconditionally may affect how code is currently generated for some > platforms. If said code doesn't conform to the above requirement, then yes, it could happen. Thanks! Laszlo