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; Thu, 11 Jul 2019 03:02:29 -0700 Received: from smtp.corp.redhat.com (int-mx01.intmail.prod.int.phx2.redhat.com [10.5.11.11]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mx1.redhat.com (Postfix) with ESMTPS id E6428308AA11; Thu, 11 Jul 2019 10:02:28 +0000 (UTC) Received: from lacos-laptop-7.usersys.redhat.com (ovpn-117-231.ams2.redhat.com [10.36.117.231]) by smtp.corp.redhat.com (Postfix) with ESMTP id C6215600CD; Thu, 11 Jul 2019 10:02:27 +0000 (UTC) Subject: Re: [PATCH] UefiCpuPkg/PiSmmCpu: Fix GCC7/GCC8 build failure To: Ray Ni , devel@edk2.groups.io Cc: Michael D Kinney , Eric Dong References: <20190711011638.95268-1-ray.ni@intel.com> From: "Laszlo Ersek" Message-ID: Date: Thu, 11 Jul 2019 12:02:21 +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: <20190711011638.95268-1-ray.ni@intel.com> X-Scanned-By: MIMEDefang 2.79 on 10.5.11.11 X-Greylist: Sender IP whitelisted, not delayed by milter-greylist-4.5.16 (mx1.redhat.com [10.5.110.41]); Thu, 11 Jul 2019 10:02:29 +0000 (UTC) Content-Type: text/plain; charset=utf-8 Content-Language: en-US Content-Transfer-Encoding: 7bit Hello Ray, On 07/11/19 03:16, Ray Ni wrote: > Signed-off-by: Ray Ni > Cc: Michael D Kinney > Cc: Eric Dong > Cc: Laszlo Ersek > --- > UefiCpuPkg/PiSmmCpuDxeSmm/X64/PageTbl.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/UefiCpuPkg/PiSmmCpuDxeSmm/X64/PageTbl.c b/UefiCpuPkg/PiSmmCpuDxeSmm/X64/PageTbl.c > index c31160735a..a3b62f7787 100644 > --- a/UefiCpuPkg/PiSmmCpuDxeSmm/X64/PageTbl.c > +++ b/UefiCpuPkg/PiSmmCpuDxeSmm/X64/PageTbl.c > @@ -571,7 +571,7 @@ ReclaimPages ( > // > // First, find the leaf entry has the smallest access record value > // > - for (Pml5Index = 0; Pml5Index < Enable5LevelPaging ? (EFI_PAGE_SIZE / sizeof (*Pml4)) : 1; Pml5Index++) { > + for (Pml5Index = 0; Pml5Index < (Enable5LevelPaging ? (EFI_PAGE_SIZE / sizeof (*Pml4)) : 1); Pml5Index++) { > if ((Pml5[Pml5Index] & IA32_PG_P) == 0 || (Pml5[Pml5Index] & IA32_PG_PMNT) != 0) { > // > // If the PML5 entry is not present or is masked, skip it > This is not a GCC7/GCC8 build failure, but a genuine bug in the code that GCC7/GCC8 helpfully reported. The conditional operator ?: has weaker precedence than the relational operator <, and so the patch incurs a behavioral change -- thus, it is a bugfix. If we were only adding the parentheses to reinforce the operator bingings that are already in place, i.e., preserving the behavior, *then* we could call this a "build failure". (1) Please update the subject accordingly, for example: UefiCpuPkg/PiSmmCpuDxeSmm: ReclaimPages: fix incorrect operator binding (71 characters) (2) I'd suggest also adding, to the commit message body: Fixes: 7365eb2c8cf1d7112330d09918c0c67e8d0b827a With those: Reviewed-by: Laszlo Ersek Thanks Laszlo