From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from ma1-aaemail-dr-lapp03.apple.com (ma1-aaemail-dr-lapp03.apple.com [17.171.2.72]) by mx.groups.io with SMTP id smtpd.web12.3864.1601088769747138645 for ; Fri, 25 Sep 2020 19:52:50 -0700 Authentication-Results: mx.groups.io; dkim=pass header.i=@apple.com header.s=20180706 header.b=fs2+NpWK; spf=pass (domain: apple.com, ip: 17.171.2.72, mailfrom: afish@apple.com) Received: from pps.filterd (ma1-aaemail-dr-lapp03.apple.com [127.0.0.1]) by ma1-aaemail-dr-lapp03.apple.com (8.16.0.42/8.16.0.42) with SMTP id 08Q2lawZ060479; Fri, 25 Sep 2020 19:52:48 -0700 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=apple.com; h=from : message-id : content-type : mime-version : subject : date : in-reply-to : cc : to : references; s=20180706; bh=JNVBs1pUBgczeMSmoadHrIq6neWJjZ0qknS6zw3ZQdI=; b=fs2+NpWK7bCcUdQq+o8DbRkqchgsX7t4YHQL5ow1Azl6GU9yg3WCwG1M5pGBsg2AcX5P ESnu74fTmgQNdsddLYflpRBzBVGUvzNHMW/laFvRDSlzJSU/8wJP9NEpUus67EToYxbx VbfHGhLpSs+iVoY5gh/6U3fXU5QhaMj2rEBvtCZuMPaBvo/AhjKNqSwt9yJeI6NidiJF 5aQIe2cbBG2kBTlF23k3lpkpM+MRFO3eMiEX+pVCsAXeKOA/IW/sYM2XlswrSYrTI2wO dR2bpQm/hyqKlbmeQQxJ4Mc5t6f9nv6vxRe5/4a4y0n3zUOoNpLLQS8xeJj4jvZ//TZX 6A== Received: from rn-mailsvcp-mta-lapp04.rno.apple.com (rn-mailsvcp-mta-lapp04.rno.apple.com [10.225.203.152]) by ma1-aaemail-dr-lapp03.apple.com with ESMTP id 33ngywftt2-1 (version=TLSv1.2 cipher=ECDHE-RSA-AES128-GCM-SHA256 bits=128 verify=NO); Fri, 25 Sep 2020 19:52:48 -0700 Received: from rn-mailsvcp-mmp-lapp02.rno.apple.com (rn-mailsvcp-mmp-lapp02.rno.apple.com [17.179.253.15]) by rn-mailsvcp-mta-lapp04.rno.apple.com (Oracle Communications Messaging Server 8.1.0.6.20200729 64bit (built Jul 29 2020)) with ESMTPS id <0QH800K1UVZZP270@rn-mailsvcp-mta-lapp04.rno.apple.com>; Fri, 25 Sep 2020 19:52:47 -0700 (PDT) Received: from process_milters-daemon.rn-mailsvcp-mmp-lapp02.rno.apple.com by rn-mailsvcp-mmp-lapp02.rno.apple.com (Oracle Communications Messaging Server 8.1.0.6.20200729 64bit (built Jul 29 2020)) id <0QH800700VBKRM00@rn-mailsvcp-mmp-lapp02.rno.apple.com>; Fri, 25 Sep 2020 19:52:47 -0700 (PDT) X-Va-A: X-Va-T-CD: 7f9a830a999000dcc54e624653256590 X-Va-E-CD: aaa40577493d9888a8da782c61ae9a59 X-Va-R-CD: 8aecc1adf28852e6ada0ffc403f1c723 X-Va-CD: 0 X-Va-ID: 06af5854-b239-4d14-8717-a092488a6eae X-V-A: X-V-T-CD: 7f9a830a999000dcc54e624653256590 X-V-E-CD: aaa40577493d9888a8da782c61ae9a59 X-V-R-CD: 8aecc1adf28852e6ada0ffc403f1c723 X-V-CD: 0 X-V-ID: c163b58b-bd59-4c6a-93d7-2cb78c7a36b2 X-Proofpoint-Virus-Version: vendor=fsecure engine=2.50.10434:6.0.235,18.0.687 definitions=2020-09-26_03:2020-09-24,2020-09-26 signatures=0 Received: from [17.235.44.121] (unknown [17.235.44.121]) by rn-mailsvcp-mmp-lapp02.rno.apple.com (Oracle Communications Messaging Server 8.1.0.6.20200729 64bit (built Jul 29 2020)) with ESMTPSA id <0QH800MFKVZX1K00@rn-mailsvcp-mmp-lapp02.rno.apple.com>; Fri, 25 Sep 2020 19:52:46 -0700 (PDT) From: "Andrew Fish" Message-id: <4929ABCC-75D2-42B3-938D-EE9C78B1FFDD@apple.com> MIME-version: 1.0 (Mac OS X Mail 13.4 \(3608.80.23.2.2\)) Subject: Re: [EXTERNAL] [edk2-devel] ECC: Won't somebody PLEASE think of the... test structures. Date: Fri, 25 Sep 2020 19:52:44 -0700 In-reply-to: Cc: edk2-devel-groups-io , Ken Taylor To: Bret Barkelew References: <29096719f8d541c3aa25f9738a61b1d3@SCL-EXCHMB-13.phoenix.com> X-Mailer: Apple Mail (2.3608.80.23.2.2) X-Proofpoint-Virus-Version: vendor=fsecure engine=2.50.10434:6.0.235,18.0.687 definitions=2020-09-26_03:2020-09-24,2020-09-26 signatures=0 Content-type: multipart/alternative; boundary="Apple-Mail=_90FAE24C-6D7A-4B4A-AD87-2EC0B7DD6E88" --Apple-Mail=_90FAE24C-6D7A-4B4A-AD87-2EC0B7DD6E88 Content-Transfer-Encoding: quoted-printable Content-Type: text/plain; charset=utf-8 Bret, Details aside this ECC issue got me thinking it might be useful to some ho= w tag code that follows different, subsetted, or alternate rules, or uses d= ifferent build environments. I was kind of thinking of doing something like= how we tag the licenses in the header comments [1]. I would say nothing me= ans edk2 rules. I can see vendors maybe having different internal rules, or= us wanting to distinguish test code from production code. The general idea= if we start this tools can be smarter and that seems like a good thing.=20 This is just a wild idea, so I=E2=80=99d like to see what other people th= ink? [1] SPDX-License-Identifier: BSD-2-Clause-Patent Thanks, Andrew Fish > On Sep 24, 2020, at 7:57 PM, Bret Barkelew = wrote: >=20 > Andrew, > > That=E2=80=99s actually what got me here. EccCheck runs as part of our C= I now (but it didn=E2=80=99t when I wrote these tests a year ago). I need t= o either figure out how to get this code to pass EccCheck in a reasonable w= ay, or just not contribute the tests and say =E2=80=9Cgo to Mu for the test= s, if you want them=E2=80=9D. > > Skipping the contribution isn=E2=80=99t a desirable outcome at all for a= number of reasons, not the least of which is that we (MS and others) are t= rying encourage all contributions to come with tests, so the process of wri= ting them needs to be simple and painless. > > - Bret=20 > > From: Andrew Fish > Sent: Thursday, September 24, 2020 7:48 PM > To: edk2-devel-groups-io ; Bret Barkelew > Cc: Ken Taylor > Subject: [EXTERNAL] Re: [edk2-devel] ECC: Won't somebody PLEASE think of= the... test structures. > > Bret, > > I=E2=80=99ve had this issue with EFI code too. It will compile with for = DEBUG and RELEASE as the optimizer removes the memcpy/memset. So you only s= ee a build failure when you compiler NOOPT (and there are no intrinsic libs= ). I mostly see this in platform code when I try to compile a single driver= /lib NOOPT and it fails to link due to the missing intrinsic.=20 > > The easy to enforce this is to compile with optimizations enabled and do= n=E2=80=99t enable intrinsic libs. Not sure if that is really practical fro= m the test point of view.=20 > > Seems the tool caught the coding style violation so I guess we could try= to =E2=80=9Cmake running that tool easier=E2=80=9D. Maybe hooking into pat= chcheck.py, making some kind of githook, or adding a git command? > > Thanks, > > Andrew Fish > >=20 >=20 > On Sep 24, 2020, at 7:25 PM, Bret Barkelew via groups.io > wrote: > > So for context, this is a new host-based test that should only run withi= n a platform OS, so intrinsics aren=E2=80=99t the big deal that they would = be in FW code. > > But we do need to figure out how to simultaneously adhere to the coding = convention while enabling test authoring. > Or we chose to not enforce quite as many things for tests. > > I=E2=80=99d prefer the first.=20 > > - Bret=20 > > From: Ken Taylor > Sent: Thursday, September 24, 2020 6:57 PM > To: devel@edk2.groups.io ; Bret Barkelew > Subject: [EXTERNAL] RE: ECC: Won't somebody PLEASE think of the... test = structures. > > If the structure is a non-static local variable, most compilers will sil= ently inject an intrinsic call to memcpy in function initialization. This = leads to an intermittent linker error. > > If the compiler you use automatically supports an intrinsic memcpy in th= e given architecture or optimizes out the memcpy, it will build for you and= you won=E2=80=99t know you need to link to an intrinsic support library in= order to build cross platform. This leads to code that builds for you, bu= t not for me. > > Regards, > -Ken. > > From: devel@edk2.groups.io [mailto:devel@e= dk2.groups.io ] On Behalf Of Bret Barkelew via= groups.io > Sent: Thursday, September 24, 2020 6:23 PM > To: devel@edk2.groups.io > Subject: [edk2-devel] ECC: Won't somebody PLEASE think of the... test st= ructures. > > ERROR - EFI coding style error > ERROR - *Error code: 5007 > ERROR - *There should be no initialization of a variable as part of its = declaration > ERROR - *file: //home/corthon/_uefi/edk2_qemu_ci/edk2/MdeModulePkg/Libra= ry/VariablePolicyLib/VariablePolicyUnitTest/VariablePolicyUnitTest.c > ERROR - *Line number: 333 > ERROR - *Variable Name: MatchCheckPolicy > > EccCheck no likey: > SIMPLE_VARIABLE_POLICY_ENTRY ValidationPolicy =3D { > { > VARIABLE_POLICY_ENTRY_REVISION, > sizeof(VARIABLE_POLICY_ENTRY) + sizeof(TEST_VAR_1_NAME), > sizeof(VARIABLE_POLICY_ENTRY), > TEST_GUID_1, > TEST_POLICY_MIN_SIZE_NULL, > TEST_POLICY_MAX_SIZE_NULL, > TEST_POLICY_ATTRIBUTES_NULL, > TEST_POLICY_ATTRIBUTES_NULL, > VARIABLE_POLICY_TYPE_NO_LOCK > }, > TEST_VAR_1_NAME > }; > > But you can=E2=80=99t init this structure separately without addressing = each field. > Can a brother get an override? > > - Bret=20 > > >=20 > <95B370A7BEED49AAB797022E8F260C8F.png> > > --Apple-Mail=_90FAE24C-6D7A-4B4A-AD87-2EC0B7DD6E88 Content-Transfer-Encoding: quoted-printable Content-Type: text/html; charset=utf-8 Bret,

Details aside this ECC issue got me thinki= ng it might be useful to some how tag code that follows different, subsette= d, or alternate rules, or uses different build environments. I was kind of = thinking of doing something like how we tag the licenses in the header comm= ents [1]. I would say nothing means edk2 rules. I can see vendors maybe hav= ing different internal rules, or us wanting to distinguish test code from p= roduction code. The general idea if we start this tools can be smarter and = that seems like a good thing. 

 This is just a wild idea, so I=E2=80=99d like to s= ee what other people think?

[1] SPDX-License-Identifier: BSD-2-Clause-Patent=

Thanks,

Andrew Fish

On Sep 24, 2020, at 7:57 PM, Bret Barkelew= <Bret.Barkele= w@microsoft.com> wrote:

Andrew,
&n= bsp;
That=E2=80=99s actually what got me here. Ec= cCheck runs as part of our CI now (but it didn=E2=80=99t when I wrote these= tests a year ago). I need to either figure out how to get this code to pas= s EccCheck in a reasonable way, or just not contribute the tests and say = =E2=80=9Cgo to Mu for the tests, if you want them=E2=80=9D.
 
Skipping the contri= bution isn=E2=80=99t a desirable outcome at all for a number of reasons, no= t the least of which is that we (MS and others) are trying encourage all co= ntributions to come with tests, so the process of writing them needs to be = simple and painless.
 
=
- Bret 
 
<= div style=3D"border-style: solid none none; border-top-width: 1pt; border-t= op-color: rgb(225, 225, 225); padding: 3pt 0in 0in;" class=3D"">
From: Andrew FishSent: = ;Thursday, September 24, 2020 7:48 PM
To: edk2-devel-groups-io;=  Bret Barkelew
Cc: = Ken Taylor
Subject: [EXTERNAL] = Re: [edk2-devel] ECC: Won't somebody PLEASE think of the... test structures= .
 
Bret,
 
I=E2=80=99ve = had this issue with EFI code too. It will compile with for DEBUG and RELEAS= E as the optimizer removes the memcpy/memset. So you only see a build failu= re when you compiler NOOPT (and there are no intrinsic libs). I mostly see = this in platform code when I try to compile a single driver/lib NOOPT and i= t fails to link due to the missing intrinsic. 
 
=
The easy to enforce this is to compile= with optimizations enabled and don=E2=80=99t enable intrinsic libs. Not su= re if that is really practical from the test point of view. 
&nb= sp;
Seems the tool caught t= he coding style violation so I guess we could try to =E2=80=9Cmake running = that tool easier=E2=80=9D. Maybe hooking into patchcheck.py, making some ki= nd of githook, or adding a git command?  
<= /div>
 
Thanks,
 
Andrew Fish
 


<= blockquote style=3D"margin-top: 5pt; margin-bottom: 5pt;" class=3D"">
On Sep 24, 2020, at 7:25 PM, Bret Barkelew via groups.io <bret.barkelew=3Dmicrosoft.com@groups.io> wrote= :
 
So for context, thi= s is a new host-based test that should only run within a platform OS, so in= trinsics aren=E2=80=99t the big deal that they would be in FW code.
 
But we do need to f= igure out how to simultaneously adhere to the coding convention while enabl= ing test authoring.
Or we chose to not enforce quite as many things for tests.
 
I=E2=80=99d prefer = the first. 
 
- Bret 
 
From: Ken Taylor
Sent: Thursday= , September 24, 2020 6:57 PM
To: devel@ed= k2.groups.io; Bret Barkelew
S= ubject: [EXTERNAL] RE= : ECC: Won't somebody PLEASE think of the... test structures.
 
If the structure is a non-static local var= iable, most compilers will silently inject an intrinsic call to memcpy in f= unction initialization.  This leads to an intermittent linker error.
 
If the compiler you use automatically supports a= n intrinsic memcpy in the given architecture or optimizes out the memcpy, i= t will build for you and you won=E2=80=99t know you need to link to an intr= insic support library in order to build cross platform.  This leads to= code that builds for you, but not for me.
 
R= egards,
-Ken.
 
From:=  devel@edk2.groups.io [mailto:devel@edk2.groups.i= o] On B= ehalf Of Bret Barkele= w via groups.io<= /a>
Sent: Thursday, September 24, 2020 6:23 PM
To: devel@edk2.groups.io
Subje= ct: [edk2-devel] ECC:= Won't somebody PLEASE think of the... test structures.
 
ERROR - EFI cod= ing style error
ERROR - *Error code: 5007
ERROR - *There should be no initialization of a varia= ble as part of its declaration
ERROR - *file: //home/corthon/_uefi/edk2_qemu_ci/edk2= /MdeModulePkg/Library/VariablePolicyLib/VariablePolicyUnitTest/VariablePoli= cyUnitTest.c
ERROR - *Line number: 333
ERROR - *Variable Name: MatchCheckPolicy
 =
EccCheck no likey:
SIMPLE_VARIABLE_POLICY_E= NTRY   ValidationPolicy =3D {
    {
      VARIA= BLE_POLICY_ENTRY_REVISION,
      sizeof(VARIABLE_POLICY_ENTRY= ) + sizeof(TEST_VAR_1_NAME),
      sizeof(VARIABLE_POLICY_ENT= RY),
 =      TEST_GUID_1,
      TEST_POLICY_MIN_S= IZE_NULL,
&= nbsp;     TEST_POLICY_MAX_SIZE_NULL,
     = ; TEST_POLICY_ATTRIBUTES_NULL,
      TEST_POLICY_ATTRIBUTES= _NULL,
&nbs= p;     VARIABLE_POLICY_TYPE_NO_LOCK
    },
    = TEST_VAR_1_NAME
  };
 
But you can=E2=80=99t init this structure separately without address= ing each field.
Can a brother get an override?
 
- Bret 
 = ;
 
<95B370A7BEED49AAB7= 97022E8F260C8F.png>
 
 

--Apple-Mail=_90FAE24C-6D7A-4B4A-AD87-2EC0B7DD6E88--