public inbox for devel@edk2.groups.io
 help / color / mirror / Atom feed
* [edk2-devel] [PATCH v1 0/7] Add DebugMacroCheck
@ 2023-08-14 20:48 Michael Kubacki
  2023-08-14 22:51 ` Pedro Falcato
  0 siblings, 1 reply; 8+ messages in thread
From: Michael Kubacki @ 2023-08-14 20:48 UTC (permalink / raw)
  To: devel
  Cc: Abner Chang, Alexei Fedorov, Ard Biesheuvel, Gerd Hoffmann,
	Igor Kulchytskyy, Jian J Wang, Jiewen Yao, Jordan Justen,
	Leif Lindholm, Liming Gao, Michael D Kinney, Nickle Wang,
	Pierre Gondois, Sami Mujawar, Sean Brogan

From: Michael Kubacki <michael.kubacki@microsoft.com>

Adds a new script and build plugin called DebugMacroCheck.

The script verifies that the number of print specifiers match the
number of arguments in DEBUG() calls.

Overview:

- Build plugin: BuildPlugin/DebugMacroCheckBuildPlugin.py
  - Runs on any build target that is not NO-TARGET
- Standalone script: DebugMacroCheck.py
  - Run `DebugMacroCheck.py --help` to see command line options
- Unit tests:
  - Tests/test_DebugMacroCheck.py
  - Can be run with:
    `python -m unittest discover -s ./.pytool/Plugin/DebugMacroCheck/tests -v`
  - Also visible in VS Code Test Explorer

Background:

The tool has been constantly run against edk2 derived code for about
a year now. During that time, its found over 20 issues in edk2, over
50 issues in various vendor code, and numerous other issues specific
to Project Mu.

See the following series for a batch of issues previously fixed in
edk2 discovered by the tool:

  https://edk2.groups.io/g/devel/message/93104

I've received interest from vendors to place it in edk2 to
immediately find issues in the upstream and make it easier for edk2
consumers to directly acquire it. That led to this patch series.

This would run in edk2 as a build plugin. All issues in the edk2
codebase have been resolved so this would find new issues before
they are merged into the codebase.

The script is meant to be portable so it can be run as a build plugin
or dropped as a standalone script into other environments alongside
the unit tests.

Series Overview:

- Fixes outstanding issues in RedfishPkg
- Adds the `regex` PIP module to pip-requirements.txt
- Adds exceptions for debug macro usage in ArmVirtPkg,
  DynamicTablesPkg, and SecurityPkg
- Disables the plugin in OvmfPkg per maintainer's previous
  preferences
- Adds the plugin

The plugin (this series) is running with passing CI results as shown
in this PR:
  https://github.com/tianocore/edk2/pull/4736

Cc: Abner Chang <abner.chang@amd.com>
Cc: Alexei Fedorov <Alexei.Fedorov@arm.com>
Cc: Ard Biesheuvel <ardb+tianocore@kernel.org>
Cc: Gerd Hoffmann <kraxel@redhat.com>
Cc: Igor Kulchytskyy <igork@ami.com>
Cc: Jian J Wang <jian.j.wang@intel.com>
Cc: Jiewen Yao <jiewen.yao@intel.com>
Cc: Jordan Justen <jordan.l.justen@intel.com>
Cc: Leif Lindholm <quic_llindhol@quicinc.com>
Cc: Liming Gao <gaoliming@byosoft.com.cn>
Cc: Michael D Kinney <michael.d.kinney@intel.com>
Cc: Nickle Wang <nicklew@nvidia.com>
Cc: Pierre Gondois <pierre.gondois@arm.com>
Cc: Sami Mujawar <Sami.Mujawar@arm.com>
Cc: Sean Brogan <sean.brogan@microsoft.com>

Michael Kubacki (7):
  RedfishPkg/PlatformHostInterfaceBmcUsbNicLib: Fix DEBUG macro args
  pip-requirements.txt: Add regex
  SecurityPkg.ci.yaml: Add debug macro exception
  ArmVirtPkg.ci.yaml: Add debug macro exception
  DynamicTablesPkg.ci.yaml: Add debug macro exception
  OvmfPkg/PlatformCI: Disable DebugMacroCheck
  .pytool/Plugin: Add DebugMacroCheck

 RedfishPkg/Library/PlatformHostInterfaceBmcUsbNicLib/PlatformHostInterfaceBmcUsbNicLib.c |   8 +-
 .pytool/Plugin/DebugMacroCheck/BuildPlugin/DebugMacroCheckBuildPlugin.py                 | 127 +++
 .pytool/Plugin/DebugMacroCheck/BuildPlugin/DebugMacroCheck_plug_in.yaml                  |  11 +
 .pytool/Plugin/DebugMacroCheck/DebugMacroCheck.py                                        | 859 ++++++++++++++++++++
 .pytool/Plugin/DebugMacroCheck/Readme.md                                                 | 253 ++++++
 .pytool/Plugin/DebugMacroCheck/tests/DebugMacroDataSet.py                                | 674 +++++++++++++++
 .pytool/Plugin/DebugMacroCheck/tests/MacroTest.py                                        | 131 +++
 .pytool/Plugin/DebugMacroCheck/tests/__init__.py                                         |   0
 .pytool/Plugin/DebugMacroCheck/tests/test_DebugMacroCheck.py                             | 201 +++++
 ArmVirtPkg/ArmVirtPkg.ci.yaml                                                            |   8 +
 DynamicTablesPkg/DynamicTablesPkg.ci.yaml                                                |   8 +
 OvmfPkg/PlatformCI/PlatformBuildLib.py                                                   |   1 +
 SecurityPkg/SecurityPkg.ci.yaml                                                          |   9 +
 pip-requirements.txt                                                                     |   2 +-
 14 files changed, 2287 insertions(+), 5 deletions(-)
 create mode 100644 .pytool/Plugin/DebugMacroCheck/BuildPlugin/DebugMacroCheckBuildPlugin.py
 create mode 100644 .pytool/Plugin/DebugMacroCheck/BuildPlugin/DebugMacroCheck_plug_in.yaml
 create mode 100644 .pytool/Plugin/DebugMacroCheck/DebugMacroCheck.py
 create mode 100644 .pytool/Plugin/DebugMacroCheck/Readme.md
 create mode 100644 .pytool/Plugin/DebugMacroCheck/tests/DebugMacroDataSet.py
 create mode 100644 .pytool/Plugin/DebugMacroCheck/tests/MacroTest.py
 create mode 100644 .pytool/Plugin/DebugMacroCheck/tests/__init__.py
 create mode 100644 .pytool/Plugin/DebugMacroCheck/tests/test_DebugMacroCheck.py

-- 
2.41.0.windows.3



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#107735): https://edk2.groups.io/g/devel/message/107735
Mute This Topic: https://groups.io/mt/100745693/7686176
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [rebecca@openfw.io]
-=-=-=-=-=-=-=-=-=-=-=-



^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [edk2-devel] [PATCH v1 0/7] Add DebugMacroCheck
  2023-08-14 20:48 Michael Kubacki
@ 2023-08-14 22:51 ` Pedro Falcato
  2023-08-15  0:23   ` Andrew Fish via groups.io
  0 siblings, 1 reply; 8+ messages in thread
From: Pedro Falcato @ 2023-08-14 22:51 UTC (permalink / raw)
  To: devel, mikuback
  Cc: Abner Chang, Alexei Fedorov, Ard Biesheuvel, Gerd Hoffmann,
	Igor Kulchytskyy, Jian J Wang, Jiewen Yao, Jordan Justen,
	Leif Lindholm, Liming Gao, Michael D Kinney, Nickle Wang,
	Pierre Gondois, Sami Mujawar, Sean Brogan

On Mon, Aug 14, 2023 at 9:49 PM Michael Kubacki
<mikuback@linux.microsoft.com> wrote:
>
> From: Michael Kubacki <michael.kubacki@microsoft.com>
>
> Adds a new script and build plugin called DebugMacroCheck.
>
> The script verifies that the number of print specifiers match the
> number of arguments in DEBUG() calls.
>
> Overview:
>
> - Build plugin: BuildPlugin/DebugMacroCheckBuildPlugin.py
>   - Runs on any build target that is not NO-TARGET
> - Standalone script: DebugMacroCheck.py
>   - Run `DebugMacroCheck.py --help` to see command line options
> - Unit tests:
>   - Tests/test_DebugMacroCheck.py
>   - Can be run with:
>     `python -m unittest discover -s ./.pytool/Plugin/DebugMacroCheck/tests -v`
>   - Also visible in VS Code Test Explorer
>
> Background:
>
> The tool has been constantly run against edk2 derived code for about
> a year now. During that time, its found over 20 issues in edk2, over
> 50 issues in various vendor code, and numerous other issues specific
> to Project Mu.
>
> See the following series for a batch of issues previously fixed in
> edk2 discovered by the tool:
>
>   https://edk2.groups.io/g/devel/message/93104
>
> I've received interest from vendors to place it in edk2 to
> immediately find issues in the upstream and make it easier for edk2
> consumers to directly acquire it. That led to this patch series.
>
> This would run in edk2 as a build plugin. All issues in the edk2
> codebase have been resolved so this would find new issues before
> they are merged into the codebase.

Hi,

I really like this change but I cannot stop and think that if DEBUG
and PrintLib were ISO C compliant, we could be using normal interfaces
with normal argument types and the compiler's intrinsic knowledge of
printf-like functions.
Have you explored that option for future code? See e.g
https://godbolt.org/z/4e8d3WToT (I don't know what MSVC uses, if
anything).

-- 
Pedro


-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#107744): https://edk2.groups.io/g/devel/message/107744
Mute This Topic: https://groups.io/mt/100745693/7686176
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [rebecca@openfw.io]
-=-=-=-=-=-=-=-=-=-=-=-



^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [edk2-devel] [PATCH v1 0/7] Add DebugMacroCheck
  2023-08-14 22:51 ` Pedro Falcato
@ 2023-08-15  0:23   ` Andrew Fish via groups.io
  2023-08-15  0:26     ` Michael Kubacki
  2023-08-15  0:57     ` Pedro Falcato
  0 siblings, 2 replies; 8+ messages in thread
From: Andrew Fish via groups.io @ 2023-08-15  0:23 UTC (permalink / raw)
  To: edk2-devel-groups-io, Pedro Falcato
  Cc: Michael Kubacki, Abner Chang, Alexei Fedorov, Ard Biesheuvel,
	Gerd Hoffmann, Igor Kulchytskyy, Jian J Wang, Jiewen Yao,
	Jordan Justen, Leif Lindholm, Liming Gao, Mike Kinney,
	Nickle Wang, Pierre Gondois, Sami Mujawar, Sean Brogan

[-- Attachment #1: Type: text/plain, Size: 2753 bytes --]



> On Aug 14, 2023, at 3:51 PM, Pedro Falcato <pedro.falcato@gmail.com> wrote:
> 
> On Mon, Aug 14, 2023 at 9:49 PM Michael Kubacki
> <mikuback@linux.microsoft.com <mailto:mikuback@linux.microsoft.com>> wrote:
>> 
>> From: Michael Kubacki <michael.kubacki@microsoft.com>
>> 
>> Adds a new script and build plugin called DebugMacroCheck.
>> 
>> The script verifies that the number of print specifiers match the
>> number of arguments in DEBUG() calls.
>> 
>> Overview:
>> 
>> - Build plugin: BuildPlugin/DebugMacroCheckBuildPlugin.py
>>  - Runs on any build target that is not NO-TARGET
>> - Standalone script: DebugMacroCheck.py
>>  - Run `DebugMacroCheck.py --help` to see command line options
>> - Unit tests:
>>  - Tests/test_DebugMacroCheck.py
>>  - Can be run with:
>>    `python -m unittest discover -s ./.pytool/Plugin/DebugMacroCheck/tests -v`
>>  - Also visible in VS Code Test Explorer
>> 
>> Background:
>> 
>> The tool has been constantly run against edk2 derived code for about
>> a year now. During that time, its found over 20 issues in edk2, over
>> 50 issues in various vendor code, and numerous other issues specific
>> to Project Mu.
>> 
>> See the following series for a batch of issues previously fixed in
>> edk2 discovered by the tool:
>> 
>>  https://edk2.groups.io/g/devel/message/93104
>> 
>> I've received interest from vendors to place it in edk2 to
>> immediately find issues in the upstream and make it easier for edk2
>> consumers to directly acquire it. That led to this patch series.
>> 
>> This would run in edk2 as a build plugin. All issues in the edk2
>> codebase have been resolved so this would find new issues before
>> they are merged into the codebase.
> 
> Hi,
> 
> I really like this change but I cannot stop and think that if DEBUG
> and PrintLib were ISO C compliant, we could be using normal interfaces
> with normal argument types and the compiler's intrinsic knowledge of
> printf-like functions.
> Have you explored that option for future code? See e.g
> https://godbolt.org/z/4e8d3WToT (I don't know what MSVC uses, if
> anything).
> 

I have a dream that we add an eft_print as an attribute format archetype and then do what you recommend. After all clang and gcc are open source. 

Thanks,

Andrew Fish

> -- 
> Pedro
> 
> 
> 



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#107746): https://edk2.groups.io/g/devel/message/107746
Mute This Topic: https://groups.io/mt/100745693/7686176
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/leave/12367111/7686176/1913456212/xyzzy [rebecca@openfw.io]
-=-=-=-=-=-=-=-=-=-=-=-



[-- Attachment #2: Type: text/html, Size: 15387 bytes --]

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [edk2-devel] [PATCH v1 0/7] Add DebugMacroCheck
  2023-08-15  0:23   ` Andrew Fish via groups.io
@ 2023-08-15  0:26     ` Michael Kubacki
  2023-08-15  1:12       ` Andrew Fish via groups.io
  2023-08-15  0:57     ` Pedro Falcato
  1 sibling, 1 reply; 8+ messages in thread
From: Michael Kubacki @ 2023-08-15  0:26 UTC (permalink / raw)
  To: devel, afish, Pedro Falcato
  Cc: Abner Chang, Alexei Fedorov, Ard Biesheuvel, Gerd Hoffmann,
	Igor Kulchytskyy, Jian J Wang, Jiewen Yao, Jordan Justen,
	Leif Lindholm, Liming Gao, Mike Kinney, Nickle Wang,
	Pierre Gondois, Sami Mujawar, Sean Brogan

On 8/14/2023 8:23 PM, Andrew Fish via groups.io wrote:
> 
> 
>> On Aug 14, 2023, at 3:51 PM, Pedro Falcato <pedro.falcato@gmail.com> 
>> wrote:
>>
>> On Mon, Aug 14, 2023 at 9:49 PM Michael Kubacki
>> <mikuback@linux.microsoft.com <mailto:mikuback@linux.microsoft.com>> 
>> wrote:
>>>
>>> From: Michael Kubacki <michael.kubacki@microsoft.com>
>>>
>>> Adds a new script and build plugin called DebugMacroCheck.
>>>
>>> The script verifies that the number of print specifiers match the
>>> number of arguments in DEBUG() calls.
>>>
>>> Overview:
>>>
>>> - Build plugin: BuildPlugin/DebugMacroCheckBuildPlugin.py
>>>  - Runs on any build target that is not NO-TARGET
>>> - Standalone script: DebugMacroCheck.py
>>>  - Run `DebugMacroCheck.py --help` to see command line options
>>> - Unit tests:
>>>  - Tests/test_DebugMacroCheck.py
>>>  - Can be run with:
>>>    `python -m unittest discover -s 
>>> ./.pytool/Plugin/DebugMacroCheck/tests -v`
>>>  - Also visible in VS Code Test Explorer
>>>
>>> Background:
>>>
>>> The tool has been constantly run against edk2 derived code for about
>>> a year now. During that time, its found over 20 issues in edk2, over
>>> 50 issues in various vendor code, and numerous other issues specific
>>> to Project Mu.
>>>
>>> See the following series for a batch of issues previously fixed in
>>> edk2 discovered by the tool:
>>>
>>>  https://edk2.groups.io/g/devel/message/93104
>>>
>>> I've received interest from vendors to place it in edk2 to
>>> immediately find issues in the upstream and make it easier for edk2
>>> consumers to directly acquire it. That led to this patch series.
>>>
>>> This would run in edk2 as a build plugin. All issues in the edk2
>>> codebase have been resolved so this would find new issues before
>>> they are merged into the codebase.
>>
>> Hi,
>>
>> I really like this change but I cannot stop and think that if DEBUG
>> and PrintLib were ISO C compliant, we could be using normal interfaces
>> with normal argument types and the compiler's intrinsic knowledge of
>> printf-like functions.
>> Have you explored that option for future code? See e.g
>> https://godbolt.org/z/4e8d3WToT <https://godbolt.org/z/4e8d3WToT>(I 
>> don't know what MSVC uses, if
>> anything).
>>
> 
> I have a dream that we add an eft_print as an attribute format archetype 
> and then do what you recommend. After all clang and gcc are open source.
> 
I agree that would be preferred. I did something in similar in GCC at 
the time, but I couldn't find an equivalent in VS. The issues kept 
appearing so this was a cross-platform way to address it.

I uploaded some usage examples to the PR for reference:
https://github.com/tianocore/edk2/pull/4736

Thanks,
Michael
> Thanks,
> 
> Andrew Fish
> 
>> --
>> Pedro
>>
>>
> 


-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#107747): https://edk2.groups.io/g/devel/message/107747
Mute This Topic: https://groups.io/mt/100745693/7686176
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [rebecca@openfw.io]
-=-=-=-=-=-=-=-=-=-=-=-



^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [edk2-devel] [PATCH v1 0/7] Add DebugMacroCheck
  2023-08-15  0:23   ` Andrew Fish via groups.io
  2023-08-15  0:26     ` Michael Kubacki
@ 2023-08-15  0:57     ` Pedro Falcato
  1 sibling, 0 replies; 8+ messages in thread
From: Pedro Falcato @ 2023-08-15  0:57 UTC (permalink / raw)
  To: Andrew (EFI) Fish
  Cc: edk2-devel-groups-io, Michael Kubacki, Abner Chang,
	Alexei Fedorov, Ard Biesheuvel, Gerd Hoffmann, Igor Kulchytskyy,
	Jian J Wang, Jiewen Yao, Jordan Justen, Leif Lindholm, Liming Gao,
	Mike Kinney, Nickle Wang, Pierre Gondois, Sami Mujawar,
	Sean Brogan

On Tue, Aug 15, 2023 at 1:23 AM Andrew (EFI) Fish <afish@apple.com> wrote:
>
>
>
> On Aug 14, 2023, at 3:51 PM, Pedro Falcato <pedro.falcato@gmail.com> wrote:
>
> On Mon, Aug 14, 2023 at 9:49 PM Michael Kubacki
> <mikuback@linux.microsoft.com> wrote:
>
>
> From: Michael Kubacki <michael.kubacki@microsoft.com>
>
> Adds a new script and build plugin called DebugMacroCheck.
>
> The script verifies that the number of print specifiers match the
> number of arguments in DEBUG() calls.
>
> Overview:
>
> - Build plugin: BuildPlugin/DebugMacroCheckBuildPlugin.py
>  - Runs on any build target that is not NO-TARGET
> - Standalone script: DebugMacroCheck.py
>  - Run `DebugMacroCheck.py --help` to see command line options
> - Unit tests:
>  - Tests/test_DebugMacroCheck.py
>  - Can be run with:
>    `python -m unittest discover -s ./.pytool/Plugin/DebugMacroCheck/tests -v`
>  - Also visible in VS Code Test Explorer
>
> Background:
>
> The tool has been constantly run against edk2 derived code for about
> a year now. During that time, its found over 20 issues in edk2, over
> 50 issues in various vendor code, and numerous other issues specific
> to Project Mu.
>
> See the following series for a batch of issues previously fixed in
> edk2 discovered by the tool:
>
>  https://edk2.groups.io/g/devel/message/93104
>
> I've received interest from vendors to place it in edk2 to
> immediately find issues in the upstream and make it easier for edk2
> consumers to directly acquire it. That led to this patch series.
>
> This would run in edk2 as a build plugin. All issues in the edk2
> codebase have been resolved so this would find new issues before
> they are merged into the codebase.
>
>
> Hi,
>
> I really like this change but I cannot stop and think that if DEBUG
> and PrintLib were ISO C compliant, we could be using normal interfaces
> with normal argument types and the compiler's intrinsic knowledge of
> printf-like functions.
> Have you explored that option for future code? See e.g
> https://godbolt.org/z/4e8d3WToT (I don't know what MSVC uses, if
> anything).
>
>
> I have a dream that we add an eft_print as an attribute format archetype and then do what you recommend. After all clang and gcc are open source.

I don't think the upstream compiler folks are willing to support our
broken printf variant. Nor should we encourage things like

VOID
Foo (
  UINTN Val
)
{
  DEBUG ((DEBUG_INFO, "%Lx", (UINT64) Val);
}

while not providing anything that looks but doesn't look like normal C
printf semantics.

-- 
Pedro


-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#107748): https://edk2.groups.io/g/devel/message/107748
Mute This Topic: https://groups.io/mt/100745693/7686176
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [rebecca@openfw.io]
-=-=-=-=-=-=-=-=-=-=-=-



^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [edk2-devel] [PATCH v1 0/7] Add DebugMacroCheck
  2023-08-15  0:26     ` Michael Kubacki
@ 2023-08-15  1:12       ` Andrew Fish via groups.io
  0 siblings, 0 replies; 8+ messages in thread
From: Andrew Fish via groups.io @ 2023-08-15  1:12 UTC (permalink / raw)
  To: Michael Kubacki
  Cc: edk2-devel-groups-io, Pedro Falcato, Abner Chang, Alexei Fedorov,
	Ard Biesheuvel, Gerd Hoffmann, Igor Kulchytskyy, Jian J Wang,
	Jiewen Yao, Jordan Justen, Leif Lindholm, Liming Gao, Mike Kinney,
	Nickle Wang, Pierre Gondois, Sami Mujawar, Sean Brogan



> On Aug 14, 2023, at 5:26 PM, Michael Kubacki <mikuback@linux.microsoft.com> wrote:
> 
> On 8/14/2023 8:23 PM, Andrew Fish via groups.io wrote:
>>> On Aug 14, 2023, at 3:51 PM, Pedro Falcato <pedro.falcato@gmail.com> wrote:
>>> 
>>> On Mon, Aug 14, 2023 at 9:49 PM Michael Kubacki
>>> <mikuback@linux.microsoft.com <mailto:mikuback@linux.microsoft.com>> wrote:
>>>> 
>>>> From: Michael Kubacki <michael.kubacki@microsoft.com>
>>>> 
>>>> Adds a new script and build plugin called DebugMacroCheck.
>>>> 
>>>> The script verifies that the number of print specifiers match the
>>>> number of arguments in DEBUG() calls.
>>>> 
>>>> Overview:
>>>> 
>>>> - Build plugin: BuildPlugin/DebugMacroCheckBuildPlugin.py
>>>>  - Runs on any build target that is not NO-TARGET
>>>> - Standalone script: DebugMacroCheck.py
>>>>  - Run `DebugMacroCheck.py --help` to see command line options
>>>> - Unit tests:
>>>>  - Tests/test_DebugMacroCheck.py
>>>>  - Can be run with:
>>>>    `python -m unittest discover -s ./.pytool/Plugin/DebugMacroCheck/tests -v`
>>>>  - Also visible in VS Code Test Explorer
>>>> 
>>>> Background:
>>>> 
>>>> The tool has been constantly run against edk2 derived code for about
>>>> a year now. During that time, its found over 20 issues in edk2, over
>>>> 50 issues in various vendor code, and numerous other issues specific
>>>> to Project Mu.
>>>> 
>>>> See the following series for a batch of issues previously fixed in
>>>> edk2 discovered by the tool:
>>>> 
>>>>  https://edk2.groups.io/g/devel/message/93104
>>>> 
>>>> I've received interest from vendors to place it in edk2 to
>>>> immediately find issues in the upstream and make it easier for edk2
>>>> consumers to directly acquire it. That led to this patch series.
>>>> 
>>>> This would run in edk2 as a build plugin. All issues in the edk2
>>>> codebase have been resolved so this would find new issues before
>>>> they are merged into the codebase.
>>> 
>>> Hi,
>>> 
>>> I really like this change but I cannot stop and think that if DEBUG
>>> and PrintLib were ISO C compliant, we could be using normal interfaces
>>> with normal argument types and the compiler's intrinsic knowledge of
>>> printf-like functions.
>>> Have you explored that option for future code? See e.g
>>> https://godbolt.org/z/4e8d3WToT <https://godbolt.org/z/4e8d3WToT>(I don't know what MSVC uses, if
>>> anything).
>>> 
>> I have a dream that we add an eft_print as an attribute format archetype and then do what you recommend. After all clang and gcc are open source.
> I agree that would be preferred. I did something in similar in GCC at the time, but I couldn't find an equivalent in VS. The issues kept appearing so this was a cross-platform way to address it.
> 

This is a case that CI can help :)

Thanks,

Andrew Fish 

> I uploaded some usage examples to the PR for reference:
> https://github.com/tianocore/edk2/pull/4736
> 
> Thanks,
> Michael
>> Thanks,
>> Andrew Fish
>>> --
>>> Pedro
>>> 
>>> 
>> 



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#107752): https://edk2.groups.io/g/devel/message/107752
Mute This Topic: https://groups.io/mt/100745693/7686176
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/leave/12367111/7686176/1913456212/xyzzy [rebecca@openfw.io]
-=-=-=-=-=-=-=-=-=-=-=-



^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [edk2-devel] [PATCH v1 0/7] Add DebugMacroCheck
       [not found] <177B5B086FA2671E.12117@groups.io>
@ 2023-08-29 23:27 ` Michael Kubacki
  2023-09-04  3:29   ` 回复: " gaoliming via groups.io
  0 siblings, 1 reply; 8+ messages in thread
From: Michael Kubacki @ 2023-08-29 23:27 UTC (permalink / raw)
  To: devel
  Cc: Abner Chang, Alexei Fedorov, Ard Biesheuvel, Gerd Hoffmann,
	Igor Kulchytskyy, Jian J Wang, Jiewen Yao, Jordan Justen,
	Leif Lindholm, Liming Gao, Michael D Kinney, Nickle Wang,
	Pierre Gondois, Sami Mujawar, Sean Brogan

Hi all,

Now that the stable tag is out, I would appreciate reviews for this series.

Thanks,
Michael

On 8/14/2023 4:48 PM, Michael Kubacki wrote:
> From: Michael Kubacki <michael.kubacki@microsoft.com>
> 
> Adds a new script and build plugin called DebugMacroCheck.
> 
> The script verifies that the number of print specifiers match the
> number of arguments in DEBUG() calls.
> 
> Overview:
> 
> - Build plugin: BuildPlugin/DebugMacroCheckBuildPlugin.py
>    - Runs on any build target that is not NO-TARGET
> - Standalone script: DebugMacroCheck.py
>    - Run `DebugMacroCheck.py --help` to see command line options
> - Unit tests:
>    - Tests/test_DebugMacroCheck.py
>    - Can be run with:
>      `python -m unittest discover -s ./.pytool/Plugin/DebugMacroCheck/tests -v`
>    - Also visible in VS Code Test Explorer
> 
> Background:
> 
> The tool has been constantly run against edk2 derived code for about
> a year now. During that time, its found over 20 issues in edk2, over
> 50 issues in various vendor code, and numerous other issues specific
> to Project Mu.
> 
> See the following series for a batch of issues previously fixed in
> edk2 discovered by the tool:
> 
>    https://edk2.groups.io/g/devel/message/93104
> 
> I've received interest from vendors to place it in edk2 to
> immediately find issues in the upstream and make it easier for edk2
> consumers to directly acquire it. That led to this patch series.
> 
> This would run in edk2 as a build plugin. All issues in the edk2
> codebase have been resolved so this would find new issues before
> they are merged into the codebase.
> 
> The script is meant to be portable so it can be run as a build plugin
> or dropped as a standalone script into other environments alongside
> the unit tests.
> 
> Series Overview:
> 
> - Fixes outstanding issues in RedfishPkg
> - Adds the `regex` PIP module to pip-requirements.txt
> - Adds exceptions for debug macro usage in ArmVirtPkg,
>    DynamicTablesPkg, and SecurityPkg
> - Disables the plugin in OvmfPkg per maintainer's previous
>    preferences
> - Adds the plugin
> 
> The plugin (this series) is running with passing CI results as shown
> in this PR:
>    https://github.com/tianocore/edk2/pull/4736
> 
> Cc: Abner Chang <abner.chang@amd.com>
> Cc: Alexei Fedorov <Alexei.Fedorov@arm.com>
> Cc: Ard Biesheuvel <ardb+tianocore@kernel.org>
> Cc: Gerd Hoffmann <kraxel@redhat.com>
> Cc: Igor Kulchytskyy <igork@ami.com>
> Cc: Jian J Wang <jian.j.wang@intel.com>
> Cc: Jiewen Yao <jiewen.yao@intel.com>
> Cc: Jordan Justen <jordan.l.justen@intel.com>
> Cc: Leif Lindholm <quic_llindhol@quicinc.com>
> Cc: Liming Gao <gaoliming@byosoft.com.cn>
> Cc: Michael D Kinney <michael.d.kinney@intel.com>
> Cc: Nickle Wang <nicklew@nvidia.com>
> Cc: Pierre Gondois <pierre.gondois@arm.com>
> Cc: Sami Mujawar <Sami.Mujawar@arm.com>
> Cc: Sean Brogan <sean.brogan@microsoft.com>
> 
> Michael Kubacki (7):
>    RedfishPkg/PlatformHostInterfaceBmcUsbNicLib: Fix DEBUG macro args
>    pip-requirements.txt: Add regex
>    SecurityPkg.ci.yaml: Add debug macro exception
>    ArmVirtPkg.ci.yaml: Add debug macro exception
>    DynamicTablesPkg.ci.yaml: Add debug macro exception
>    OvmfPkg/PlatformCI: Disable DebugMacroCheck
>    .pytool/Plugin: Add DebugMacroCheck
> 
>   RedfishPkg/Library/PlatformHostInterfaceBmcUsbNicLib/PlatformHostInterfaceBmcUsbNicLib.c |   8 +-
>   .pytool/Plugin/DebugMacroCheck/BuildPlugin/DebugMacroCheckBuildPlugin.py                 | 127 +++
>   .pytool/Plugin/DebugMacroCheck/BuildPlugin/DebugMacroCheck_plug_in.yaml                  |  11 +
>   .pytool/Plugin/DebugMacroCheck/DebugMacroCheck.py                                        | 859 ++++++++++++++++++++
>   .pytool/Plugin/DebugMacroCheck/Readme.md                                                 | 253 ++++++
>   .pytool/Plugin/DebugMacroCheck/tests/DebugMacroDataSet.py                                | 674 +++++++++++++++
>   .pytool/Plugin/DebugMacroCheck/tests/MacroTest.py                                        | 131 +++
>   .pytool/Plugin/DebugMacroCheck/tests/__init__.py                                         |   0
>   .pytool/Plugin/DebugMacroCheck/tests/test_DebugMacroCheck.py                             | 201 +++++
>   ArmVirtPkg/ArmVirtPkg.ci.yaml                                                            |   8 +
>   DynamicTablesPkg/DynamicTablesPkg.ci.yaml                                                |   8 +
>   OvmfPkg/PlatformCI/PlatformBuildLib.py                                                   |   1 +
>   SecurityPkg/SecurityPkg.ci.yaml                                                          |   9 +
>   pip-requirements.txt                                                                     |   2 +-
>   14 files changed, 2287 insertions(+), 5 deletions(-)
>   create mode 100644 .pytool/Plugin/DebugMacroCheck/BuildPlugin/DebugMacroCheckBuildPlugin.py
>   create mode 100644 .pytool/Plugin/DebugMacroCheck/BuildPlugin/DebugMacroCheck_plug_in.yaml
>   create mode 100644 .pytool/Plugin/DebugMacroCheck/DebugMacroCheck.py
>   create mode 100644 .pytool/Plugin/DebugMacroCheck/Readme.md
>   create mode 100644 .pytool/Plugin/DebugMacroCheck/tests/DebugMacroDataSet.py
>   create mode 100644 .pytool/Plugin/DebugMacroCheck/tests/MacroTest.py
>   create mode 100644 .pytool/Plugin/DebugMacroCheck/tests/__init__.py
>   create mode 100644 .pytool/Plugin/DebugMacroCheck/tests/test_DebugMacroCheck.py
> 


-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#108102): https://edk2.groups.io/g/devel/message/108102
Mute This Topic: https://groups.io/mt/100745693/7686176
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [rebecca@openfw.io]
-=-=-=-=-=-=-=-=-=-=-=-



^ permalink raw reply	[flat|nested] 8+ messages in thread

* 回复: [edk2-devel] [PATCH v1 0/7] Add DebugMacroCheck
  2023-08-29 23:27 ` [edk2-devel] [PATCH v1 0/7] Add DebugMacroCheck Michael Kubacki
@ 2023-09-04  3:29   ` gaoliming via groups.io
  0 siblings, 0 replies; 8+ messages in thread
From: gaoliming via groups.io @ 2023-09-04  3:29 UTC (permalink / raw)
  To: devel, mikuback
  Cc: 'Abner Chang', 'Alexei Fedorov',
	'Ard Biesheuvel', 'Gerd Hoffmann',
	'Igor Kulchytskyy', 'Jian J Wang',
	'Jiewen Yao', 'Jordan Justen',
	'Leif Lindholm', 'Michael D Kinney',
	'Nickle Wang', 'Pierre Gondois',
	'Sami Mujawar', 'Sean Brogan'

Michael:
  I think this change is helpful. For this patch set, Acked-by: Liming Gao <gaoliming@byosoft.com.cn>

Thanks
Liming
> -----邮件原件-----
> 发件人: devel@edk2.groups.io <devel@edk2.groups.io> 代表 Michael
> Kubacki
> 发送时间: 2023年8月30日 7:27
> 收件人: devel@edk2.groups.io
> 抄送: Abner Chang <abner.chang@amd.com>; Alexei Fedorov
> <Alexei.Fedorov@arm.com>; Ard Biesheuvel <ardb+tianocore@kernel.org>;
> Gerd Hoffmann <kraxel@redhat.com>; Igor Kulchytskyy <igork@ami.com>;
> Jian J Wang <jian.j.wang@intel.com>; Jiewen Yao <jiewen.yao@intel.com>;
> Jordan Justen <jordan.l.justen@intel.com>; Leif Lindholm
> <quic_llindhol@quicinc.com>; Liming Gao <gaoliming@byosoft.com.cn>;
> Michael D Kinney <michael.d.kinney@intel.com>; Nickle Wang
> <nicklew@nvidia.com>; Pierre Gondois <pierre.gondois@arm.com>; Sami
> Mujawar <Sami.Mujawar@arm.com>; Sean Brogan
> <sean.brogan@microsoft.com>
> 主题: Re: [edk2-devel] [PATCH v1 0/7] Add DebugMacroCheck
> 
> Hi all,
> 
> Now that the stable tag is out, I would appreciate reviews for this series.
> 
> Thanks,
> Michael
> 
> On 8/14/2023 4:48 PM, Michael Kubacki wrote:
> > From: Michael Kubacki <michael.kubacki@microsoft.com>
> >
> > Adds a new script and build plugin called DebugMacroCheck.
> >
> > The script verifies that the number of print specifiers match the
> > number of arguments in DEBUG() calls.
> >
> > Overview:
> >
> > - Build plugin: BuildPlugin/DebugMacroCheckBuildPlugin.py
> >    - Runs on any build target that is not NO-TARGET
> > - Standalone script: DebugMacroCheck.py
> >    - Run `DebugMacroCheck.py --help` to see command line options
> > - Unit tests:
> >    - Tests/test_DebugMacroCheck.py
> >    - Can be run with:
> >      `python -m unittest discover
> -s ./.pytool/Plugin/DebugMacroCheck/tests -v`
> >    - Also visible in VS Code Test Explorer
> >
> > Background:
> >
> > The tool has been constantly run against edk2 derived code for about
> > a year now. During that time, its found over 20 issues in edk2, over
> > 50 issues in various vendor code, and numerous other issues specific
> > to Project Mu.
> >
> > See the following series for a batch of issues previously fixed in
> > edk2 discovered by the tool:
> >
> >    https://edk2.groups.io/g/devel/message/93104
> >
> > I've received interest from vendors to place it in edk2 to
> > immediately find issues in the upstream and make it easier for edk2
> > consumers to directly acquire it. That led to this patch series.
> >
> > This would run in edk2 as a build plugin. All issues in the edk2
> > codebase have been resolved so this would find new issues before
> > they are merged into the codebase.
> >
> > The script is meant to be portable so it can be run as a build plugin
> > or dropped as a standalone script into other environments alongside
> > the unit tests.
> >
> > Series Overview:
> >
> > - Fixes outstanding issues in RedfishPkg
> > - Adds the `regex` PIP module to pip-requirements.txt
> > - Adds exceptions for debug macro usage in ArmVirtPkg,
> >    DynamicTablesPkg, and SecurityPkg
> > - Disables the plugin in OvmfPkg per maintainer's previous
> >    preferences
> > - Adds the plugin
> >
> > The plugin (this series) is running with passing CI results as shown
> > in this PR:
> >    https://github.com/tianocore/edk2/pull/4736
> >
> > Cc: Abner Chang <abner.chang@amd.com>
> > Cc: Alexei Fedorov <Alexei.Fedorov@arm.com>
> > Cc: Ard Biesheuvel <ardb+tianocore@kernel.org>
> > Cc: Gerd Hoffmann <kraxel@redhat.com>
> > Cc: Igor Kulchytskyy <igork@ami.com>
> > Cc: Jian J Wang <jian.j.wang@intel.com>
> > Cc: Jiewen Yao <jiewen.yao@intel.com>
> > Cc: Jordan Justen <jordan.l.justen@intel.com>
> > Cc: Leif Lindholm <quic_llindhol@quicinc.com>
> > Cc: Liming Gao <gaoliming@byosoft.com.cn>
> > Cc: Michael D Kinney <michael.d.kinney@intel.com>
> > Cc: Nickle Wang <nicklew@nvidia.com>
> > Cc: Pierre Gondois <pierre.gondois@arm.com>
> > Cc: Sami Mujawar <Sami.Mujawar@arm.com>
> > Cc: Sean Brogan <sean.brogan@microsoft.com>
> >
> > Michael Kubacki (7):
> >    RedfishPkg/PlatformHostInterfaceBmcUsbNicLib: Fix DEBUG macro
> args
> >    pip-requirements.txt: Add regex
> >    SecurityPkg.ci.yaml: Add debug macro exception
> >    ArmVirtPkg.ci.yaml: Add debug macro exception
> >    DynamicTablesPkg.ci.yaml: Add debug macro exception
> >    OvmfPkg/PlatformCI: Disable DebugMacroCheck
> >    .pytool/Plugin: Add DebugMacroCheck
> >
> >
> RedfishPkg/Library/PlatformHostInterfaceBmcUsbNicLib/PlatformHostInterfa
> ceBmcUsbNicLib.c |   8 +-
> >   .pytool/Plugin/DebugMacroCheck/BuildPlugin/DebugMacroCheckBuildPl
> ugin.py                 | 127 +++
> >   .pytool/Plugin/DebugMacroCheck/BuildPlugin/DebugMacroCheck_plug_
> in.yaml                  |  11 +
> >   .pytool/Plugin/DebugMacroCheck/DebugMacroCheck.py
> | 859 ++++++++++++++++++++
> >   .pytool/Plugin/DebugMacroCheck/Readme.md
> | 253 ++++++
> >   .pytool/Plugin/DebugMacroCheck/tests/DebugMacroDataSet.py
> | 674 +++++++++++++++
> >   .pytool/Plugin/DebugMacroCheck/tests/MacroTest.py
> | 131 +++
> >   .pytool/Plugin/DebugMacroCheck/tests/__init__.py
> |   0
> >   .pytool/Plugin/DebugMacroCheck/tests/test_DebugMacroCheck.py
> | 201 +++++
> >   ArmVirtPkg/ArmVirtPkg.ci.yaml
> |   8 +
> >   DynamicTablesPkg/DynamicTablesPkg.ci.yaml
> |   8 +
> >   OvmfPkg/PlatformCI/PlatformBuildLib.py
> |   1 +
> >   SecurityPkg/SecurityPkg.ci.yaml
> |   9 +
> >   pip-requirements.txt
> |   2 +-
> >   14 files changed, 2287 insertions(+), 5 deletions(-)
> >   create mode
> 100644 .pytool/Plugin/DebugMacroCheck/BuildPlugin/DebugMacroCheckBuil
> dPlugin.py
> >   create mode
> 100644 .pytool/Plugin/DebugMacroCheck/BuildPlugin/DebugMacroCheck_pl
> ug_in.yaml
> >   create mode
> 100644 .pytool/Plugin/DebugMacroCheck/DebugMacroCheck.py
> >   create mode 100644 .pytool/Plugin/DebugMacroCheck/Readme.md
> >   create mode
> 100644 .pytool/Plugin/DebugMacroCheck/tests/DebugMacroDataSet.py
> >   create mode
> 100644 .pytool/Plugin/DebugMacroCheck/tests/MacroTest.py
> >   create mode
> 100644 .pytool/Plugin/DebugMacroCheck/tests/__init__.py
> >   create mode
> 100644 .pytool/Plugin/DebugMacroCheck/tests/test_DebugMacroCheck.py
> >
> 
> 
> 
> 





-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#108247): https://edk2.groups.io/g/devel/message/108247
Mute This Topic: https://groups.io/mt/101142278/7686176
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [rebecca@openfw.io]
-=-=-=-=-=-=-=-=-=-=-=-



^ permalink raw reply	[flat|nested] 8+ messages in thread

end of thread, other threads:[~2023-09-04  3:29 UTC | newest]

Thread overview: 8+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
     [not found] <177B5B086FA2671E.12117@groups.io>
2023-08-29 23:27 ` [edk2-devel] [PATCH v1 0/7] Add DebugMacroCheck Michael Kubacki
2023-09-04  3:29   ` 回复: " gaoliming via groups.io
2023-08-14 20:48 Michael Kubacki
2023-08-14 22:51 ` Pedro Falcato
2023-08-15  0:23   ` Andrew Fish via groups.io
2023-08-15  0:26     ` Michael Kubacki
2023-08-15  1:12       ` Andrew Fish via groups.io
2023-08-15  0:57     ` Pedro Falcato

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox