[EXTERNAL] [edk2-devel] ECC: Won't somebody PLEASE think of the... test structures.

Sun Sep 27 03:40:27 UTC 2020

I’m not opposed to it, but I think our team would suggest that it’s a good use of a multi-repo solution where each repo can have its own enforced style.

What would your thoughts be on that?

- Bret

From: Andrew Fish<mailto:afish at apple.com>
Sent: Friday, September 25, 2020 7:52 PM
To: Bret Barkelew<mailto:Bret.Barkelew at microsoft.com>
Cc: edk2-devel-groups-io<mailto:devel at edk2.groups.io>; Ken Taylor<mailto:Ken_Taylor at phoenix.com>
Subject: Re: [EXTERNAL] [edk2-devel] ECC: Won't somebody PLEASE think of the... test structures.

Bret,

Details aside this ECC issue got me thinking it might be useful to some how tag code that follows different, subsetted, 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 comments [1]. I would say nothing means 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.

 This is just a wild idea, so I’d like to see 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.Barkelew at microsoft.com<mailto:Bret.Barkelew at microsoft.com>> wrote:

Andrew,

That’s actually what got me here. EccCheck runs as part of our CI now (but it didn’t when I wrote these tests a year ago). I need to either figure out how to get this code to pass EccCheck in a reasonable way, or just not contribute the tests and say “go to Mu for the tests, if you want them”.

Skipping the contribution isn’t a desirable outcome at all for a number of reasons, not the least of which is that we (MS and others) are trying encourage all contributions to come with tests, so the process of writing them needs to be simple and painless.

- Bret

From: Andrew Fish<mailto:afish at apple.com>
Sent: Thursday, September 24, 2020 7:48 PM
To: edk2-devel-groups-io<mailto:devel at edk2.groups.io>; Bret Barkelew<mailto:Bret.Barkelew at microsoft.com>
Cc: Ken Taylor<mailto:Ken_Taylor at phoenix.com>
Subject: [EXTERNAL] Re: [edk2-devel] ECC: Won't somebody PLEASE think of the... test structures.

Bret,

I’ve 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 see 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.

The easy to enforce this is to compile with optimizations enabled and don’t enable intrinsic libs. Not sure if that is really practical from the test point of view.

Seems the tool caught the coding style violation so I guess we could try to “make running that tool easier”. Maybe hooking into patchcheck.py, making some kind of githook, or adding a git command?

Thanks,

Andrew Fish

On Sep 24, 2020, at 7:25 PM, Bret Barkelew via groups.io<https://nam06.safelinks.protection.outlook.com/?url=http%3A%2F%2Fgroups.io%2F&data=02%7C01%7CBret.Barkelew%40microsoft.com%7Cf0f32ca53bd744c42f2308d861c7422f%7C72f988bf86f141af91ab2d7cd011db47%7C1%7C0%7C637366855730968867&sdata=xEsoTWc5tpwrKQP7s24kKRKKmR768vzm3vphI7GGJQA%3D&reserved=0> <bret.barkelew=microsoft.com at groups.io<mailto:bret.barkelew=microsoft.com at groups.io>> wrote:

So for context, this is a new host-based test that should only run within a platform OS, so intrinsics aren’t 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’d prefer the first.

- Bret

From: Ken Taylor<mailto:Ken_Taylor at phoenix.com>
Sent: Thursday, September 24, 2020 6:57 PM
To: devel at edk2.groups.io<mailto:devel at edk2.groups.io>; Bret Barkelew<mailto:Bret.Barkelew at microsoft.com>
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 silently 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 the given architecture or optimizes out the memcpy, it will build for you and you won’t know you need to link to an intrinsic support library in order to build cross platform.  This leads to code that builds for you, but not for me.

Regards,
-Ken.

From: devel at edk2.groups.io<mailto:devel at edk2.groups.io> [mailto:devel at edk2.groups.io] On Behalf Of Bret Barkelew via groups.io<https://nam06.safelinks.protection.outlook.com/?url=http%3A%2F%2Fgroups.io%2F&data=02%7C01%7CBret.Barkelew%40microsoft.com%7Cf0f32ca53bd744c42f2308d861c7422f%7C72f988bf86f141af91ab2d7cd011db47%7C1%7C0%7C637366855730978863&sdata=mGjZ1q6XdEvg9JFgjHw2v%2BgFx%2BywT%2FXuQjfy2IQDBLE%3D&reserved=0>
Sent: Thursday, September 24, 2020 6:23 PM
To: devel at edk2.groups.io<mailto:devel at edk2.groups.io>
Subject: [edk2-devel] ECC: Won't somebody PLEASE think of the... test structures.

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/Library/VariablePolicyLib/VariablePolicyUnitTest/VariablePolicyUnitTest.c
ERROR - *Line number: 333
ERROR - *Variable Name: MatchCheckPolicy

EccCheck no likey:
SIMPLE_VARIABLE_POLICY_ENTRY   ValidationPolicy = {
    {
      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’t init this structure separately without addressing each field.
Can a brother get an override?

- Bret

<95B370A7BEED49AAB797022E8F260C8F.png>

-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#65663): https://edk2.groups.io/g/devel/message/65663
Mute This Topic: https://groups.io/mt/77131032/1813853
Group Owner: devel+owner at edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [edk2-devel-archive at redhat.com]
-=-=-=-=-=-=-=-=-=-=-=-

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://listman.redhat.com/archives/edk2-devel-archive/attachments/20200927/970ddb9a/attachment.htm>