Unit test framework pkg enable sanitize #6408

mdkinney · 2024-11-06T01:46:19Z

Description

UnitTestFrameworkPkg: Use /MTd and enable Address Sanitizers

Update host based unit test VS20xx builds to use /MTd instead of
/MT to enable use of debug libraries for host based unit tests.
Enable /fsanitize=address for host based unit test VS2019 builds
Enable /fsanitize=address for host based unit test VS2022 builds
Enable -fsanitize=address for host based unit test GCC builds
Add UNIT_TESTING_ADDRESS_SANITIZER_ENABLE define that is set to
TRUE by default so it is always enabled, but can be set to FALSE
to temporarily disable during development/debug of unit tests.
Add HostMemoryAllocationBelowAddressLib to support allocating
buffers below a specified address in unit test code and mocks to
support code under test that requires a buffer below a specific
address (e.g. 4GB). The services included are:

VOID * EFIAPI HostAllocatePoolBelowAddress (IN VOID   *MaximumAddress, IN UINTN  Length);
VOID EFIAPI HostFreePoolBelowAddress (IN VOID  *Address);
VOID * EFIAPI HostAllocateAlignedPagesBelowAddress (IN VOID   *MaximumAddress, IN UINTN  Pages, IN UINTN  Alignment );
VOID EFIAPI HostFreeAlignedPagesBelowAddress (IN VOID   *Buffer, IN UINTN  Pages);

Enabling the Address Sanitizer can detect double frees, buffer
overflow, buffer underflow, access to invalid addresses, and
various exceptions. These can be detected in both the unit test
case sources as well as the code under test.

Breaking change?
- Breaking change - Does this PR cause a break in build or boot behavior?
- Examples: Does it add a new library class or move a module to a different repo.
Impacts security?
- Security - Does this PR have a direct security impact?
- Examples: Crypto algorithm change or buffer overflow fix.
Includes tests?
- Tests - Does this PR include any explicit test code?
- Examples: Unit tests or integration tests.

How This Was Tested

Integration Instructions

The environment variable GTEST_CATCH_EXCEPTION must be set to 0 for so all exceptions are handled by the address sanitizer and not GoogleTest. This allows stack back trace and other details to be logged by the address sanitizer so the source of the issue identified address sanitizer can be determined.

The environment variable ASAN_OPTIONS must be set to detect_leaks=0 to disable memory leak detection. The unit test frameworks may have memory leaks and some firmware code under test use cases may perform a memory allocation without a matching memory free as their expected behavior.

Windows

set GTEST_CATCH_EXCEPTIONS=0
set ASAN_OPTIONS=detect_leaks=0

Linux

export GTEST_CATCH_EXCEPTIONS=0
export ASAN_OPTIONS=detect_leaks=0

mdkinney · 2024-11-06T02:04:15Z

@heatd @mhaeuser @shijunjing please provide feedback on unconditionally adding address sanitizer to all host based unit tests.

There are email discussion about ASAN, UBSAN, and fuzzing and this would be first step along that path.

mhaeuser · 2024-11-06T13:53:50Z

...meworkPkg/Test/UnitTest/Sample/SampleUnitTestBufferUnderflow/SampleUnitTestBufferUnderflow.c

+
+  Pointer = (UINT8 *)AllocatePool (100);
+  UT_ASSERT_NOT_NULL (Pointer);
+  Pointer[110] = 0;


This is the same test as for Overflow.

good catch. I will fix on next update

mhaeuser · 2024-11-06T13:55:26Z

...rameworkPkg/Test/UnitTest/Sample/SampleUnitTestInvalidAddress/SampleUnitTestInvalidAddress.c

+  IN UNIT_TEST_CONTEXT  Context
+  )
+{
+  *(UINT8 *)(NULL) = 0;


Due to various quirks around the 0-address, I think "null dereference" and "invalid dereference" should be two entirely distinct concepts.

I agree, The will change this one to match google test death test on access to address (-1) and will add a new unit tests that does NULL pointer dereference.

I agree, The will change this one to match google test death test on access to address (-1) and will add a new unit tests that does NULL pointer dereference.

you can't actually reliably do NULL pointer dereference in C without workarounds. Here's clang generating a completely empty function (not even a RET!) because of UB: https://godbolt.org/z/dGzj893e6

Yes. Adding these types of unit tests to verify the ability to catch developer errors are challenging. I would like to see CLANG used for host based unit testing in the future, but right now, the only tool change enabled are VS20xx and GCC.

I'm not sure this is challenging, you "simply" need to employ volatile for such cases. Somehow I missed this, thanks @heatd!

Since we compile unit tests with all optimizations disabled, I am not sure of even volatile is required. But can always add volatile as needed for compiler compatibity.

Here is what is see from VS2019 for the statement *(UINT8 *)(NULL) = 0;

Sample Unit Test Sanitize NULL Pointer v0.1 --------------------------------------------------------- ------------ RUNNING ALL TEST SUITES -------------- --------------------------------------------------------- --------------------------------------------------------- RUNNING TEST SUITE: Sanitize tests --------------------------------------------------------- [==========] Running 1 test(s). [ RUN ] Sanitize NULL Address ================================================================= ==15628==ERROR: AddressSanitizer: access-violation on unknown address 0x000000000000 (pc 0x7ff79c2215d1 bp 0x000000000000 sp 0x0027c54feb60 T0) ==15628==The signal is caused by a WRITE memory access. ==15628==Hint: address points to the zero page. #0 0x7ff79c2215d0 in SanitizeNullAddress c:\work\github\tianocore\edk2\UnitTestFrameworkPkg\Test\UnitTest\Sample\SampleUnitTestNullAddress\SampleUnitTestNullAddress.c:40 #1 0x7ff79c25641d in CmockaUnitTestFunctionRunner c:\work\github\tianocore\edk2\UnitTestFrameworkPkg\Library\UnitTestLib\RunTestsCmocka.c:56 #2 0x7ff79c251aa1 in cmocka_run_one_test_or_fixture c:\work\github\tianocore\edk2\UnitTestFrameworkPkg\Library\CmockaLib\cmocka\src\cmocka.c:2890 #3 0x7ff79c251f8d in cmocka_run_one_tests c:\work\github\tianocore\edk2\UnitTestFrameworkPkg\Library\CmockaLib\cmocka\src\cmocka.c:2998 #4 0x7ff79c24a49b in _cmocka_run_group_tests c:\work\github\tianocore\edk2\UnitTestFrameworkPkg\Library\CmockaLib\cmocka\src\cmocka.c:3129 #5 0x7ff79c257594 in RunTestSuite c:\work\github\tianocore\edk2\UnitTestFrameworkPkg\Library\UnitTestLib\RunTestsCmocka.c:220 #6 0x7ff79c256f10 in RunAllTestSuites c:\work\github\tianocore\edk2\UnitTestFrameworkPkg\Library\UnitTestLib\RunTestsCmocka.c:276 #7 0x7ff79c221899 in UefiTestMain c:\work\github\tianocore\edk2\UnitTestFrameworkPkg\Test\UnitTest\Sample\SampleUnitTestNullAddress\SampleUnitTestNullAddress.c:90 #8 0x7ff79c221981 in main c:\work\github\tianocore\edk2\UnitTestFrameworkPkg\Test\UnitTest\Sample\SampleUnitTestNullAddress\SampleUnitTestNullAddress.c:109 #9 0x7ff79c2abce8 in invoke_main D:\a\_work\1\s\src\vctools\crt\vcstartup\src\startup\exe_common.inl:78 #10 0x7ff79c2abc3d in __scrt_common_main_seh D:\a\_work\1\s\src\vctools\crt\vcstartup\src\startup\exe_common.inl:288 #11 0x7ff79c2abafd in __scrt_common_main D:\a\_work\1\s\src\vctools\crt\vcstartup\src\startup\exe_common.inl:330 #12 0x7ff79c2abd5d in mainCRTStartup D:\a\_work\1\s\src\vctools\crt\vcstartup\src\startup\exe_main.cpp:16 #13 0x7ffe7cd4257c (C:\windows\System32\KERNEL32.DLL+0x18001257c) #14 0x7ffe7de8af07 (C:\windows\SYSTEM32\ntdll.dll+0x18005af07) AddressSanitizer can not provide additional info. SUMMARY: AddressSanitizer: access-violation c:\work\github\tianocore\edk2\UnitTestFrameworkPkg\Test\UnitTest\Sample\SampleUnitTestNullAddress\SampleUnitTestNullAddress.c:40 in SanitizeNullAddress ==15628==ABORTING

mhaeuser · 2024-11-06T13:56:56Z

...rameworkPkg/Test/UnitTest/Sample/SampleUnitTestInvalidAddress/SampleUnitTestInvalidAddress.c

+  Status = RunAllTestSuites (Framework);
+
+EXIT:
+  if (Framework) {


Don't the same rules apply as for EDK II code, where "implicit" comparisons are forbidden?

Yes. Pointer should check against NULL. Only BOOLEAN variable can be used this way. Unfortunately this pattern is widely used in unit tests. I will fix in a different PR

mhaeuser · 2024-11-06T14:12:20Z

UnitTestFrameworkPkg/Library/UnitTestLib/UnitTestLib.c

+  // TODO: Finish this function.
+  if (SuiteEntry) {
+    TestCase = (UNIT_TEST_LIST_ENTRY *)GetFirstNode (&(SuiteEntry->UTS.TestCaseList));
+    while ((LIST_ENTRY *)TestCase != &(SuiteEntry->UTS.TestCaseList)) {


The double-casts and the manual comparison look very weird, should this not use (BASE_)CR and functions like IsNull()?

heatd · 2024-11-07T18:49:31Z

@heatd @mhaeuser @shijunjing please provide feedback on unconditionally adding address sanitizer to all host based unit tests.

There are email discussion about ASAN, UBSAN, and fuzzing and this would be first step along that path.

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

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

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

You shouldn't enable ASAN unconditionally, because various sanitizers are incompatible with each other. For instance, clang supports MSAN which can catch uninitialized memory reads (also TSAN, but concurrency is nonexistent in UEFI).

I didn't look too much at the code but otherwise SGTM.
Also consider that the OpenCore folks have their own test harness and fuzzing harness they used on Ext4Dxe (and other components, I believe) to great effect.

I also think that doing ASAN in an actual firmware environment could be very beneficial for catching bugs, which otherwise you're not catching (particularly considering that very little EFI code has unit tests).

mdkinney · 2024-11-07T19:02:53Z

@heatd I am aware that some sanitizers can not be combined. I think I will be valuable to enable more sanitizers over time, but that will have to be balanced against the CI time if unit tests have to be build and run with multiple configurations. More investigation is needed. I am suggesting in this draft PR that address sanitization always be enabled when running edk2 host-based unit tests and we can handle sanitizer compatibilities as they are enabled later.

I agree that this feature only provide real value if the unit tests start to cover large amounts of FW sources. We should focus efforts on this gap.

Enabling sanitizers in target FW environments is very challenging, would require significant development, support, and maintenance. It would also always be behind the latest features on sanitizers targeted for OS applications.

mdkinney · 2024-11-07T19:04:04Z

I have pushed an update that should address some of the CI failures and some of the feedback from @mhaeuser

heatd · 2024-11-07T19:22:19Z

@heatd I am aware that some sanitizers can not be combined. I think I will be valuable to enable more sanitizers over time, but that will have to be balanced against the CI time if unit tests have to be build and run with multiple configurations. More investigation is needed. I am suggesting in this draft PR that address sanitization always be enabled when running edk2 host-based unit tests and we can handle sanitizer compatibilities as they are enabled later.

OK.

I agree that this feature only provide real value if the unit tests start to cover large amounts of FW sources. We should focus efforts on this gap.

Right. Feasibility of that testing goal aside, I seriously suspect unit tests can't carry nearly as far as a real workload.

Enabling sanitizers in target FW environments is very challenging, would require significant development, support, and maintenance. It would also always be behind the latest features on sanitizers targeted for OS applications.

FWIW, I suspect it wouldn't be that hard to do this. Basically you'd need to reserve 1/8th of memory in PEI to be set up as shadow memory in DxeCore (I think?). Steven Shi IIRC had a nice sample implementation of this that I remember fairly impressed me at the time. You'd also probably want to add a heap quarantine a-la ASAN malloc, but that's also not super hard.

Also, these interfaces are quite stable. ASAN doesn't really have new features.

In any case, these are just things folks should consider; I'm certainly not enlisting to do any of this work anymore :). Having any kind of ASAN is already a good improvement.

mdkinney · 2024-11-07T19:27:16Z

Right. Feasibility of that testing goal aside, I seriously suspect unit tests can't carry nearly as far as a real workload.

Actually unit tests can cover corner cases that are very hard or impossible to reproduce on real targets. Mostly around error paths for out of memory and error paths for hardware errors/failures. Host based unit testing can use mocks to and code coverage to help ensure all code paths are tested.

mhaeuser · 2024-11-07T21:14:48Z

Actually unit tests can cover corner cases that are very hard or impossible to reproduce on real targets. Mostly around error paths for out of memory and error paths for hardware errors/failures. Host based unit testing can use mocks to and code coverage to help ensure all code paths are tested.

I have to say, despite being a fan of automated tests, I have never been a fan of classical unit tests, let alone test-driven development (though I acknowledge its advantages). This is less about "this is useless", it is not, but more about "this is very time-consuming and prone to errors that effectively risk making it useless". People tend to think of the typical corner-cases that are already prominent during development, but miss out on the finer details (this obviously includes myself).

My personal approach has been to utilize fuzz-testing using fault injection (e.g. our AUDK userland fuzzing allocator is shimmed to randomly inject allocation faults to test exactly these OOM paths) as much as possible, and when there is a bug, I save the bugged input for regression-testing. To make this somewhat scale, I wrote multiple scripts to better handle corpus-based testing (heavily PoC): https://github.com/mhaeuser/MastersThesis/tree/ue_test/Scripts/Corpus

Examples of this include:

ExtendConfig: The fuzzing environment configuration is handled by a trailer of a fixed structure. This includes things like the allocation failure modulus (when it holds that NumAllocs % AllocMod == AllocMod - 1, the allocation is faulted). The script helps updating the corpus (and any stored inputs for regression- or unit-testing) in case the config struct layout is updated.
FabricateAllocModulus: The idea here was to pre-generate allocation failures (by taking an otherwise valid input that would trigger all success code paths and continuously increasing the allocation failure threshold), so coverage-based fuzzing would easily pick up on generating allocation failures across various kinds of inputs.

If you give it enough valid inputs and enough ways to break the environment, the fuzzer will do its work, given enough time and RAM. I combined this with manually checking the coverage reports and fabricating faulty inputs for unreached error paths alike FabricateAllocModulus, to speed up fuzz-testing coverage and ensure lots of permutations. Throwing a fuzzing worker at the problem feels both safer and easier in most cases, rather than sitting down and thinking of all critical error conditions, risking to miss some important combination.

mdkinney · 2024-11-07T23:24:35Z

I agree with the potential value of fuzz testing and how it can compliment other testing methods. There are just better development and debug tools for host based unit tests that are OS applications than target FW envs, so I am interested in seeing how we can apply fuzz testing in host envs.

mdkinney · 2024-11-08T03:54:03Z

@VivianNK please review

mhaeuser · 2024-11-08T10:25:40Z

There are just better development and debug tools for host based unit tests that are OS applications than target FW envs, so I am interested in seeing how we can apply fuzz testing in host envs.

All our fuzz-testing is in host envs. I don’t think fuzz-testing in FW envs makes any sense (this is separate from sanitizers, which absolutely do make sense there!).

VivianNK · 2024-11-08T19:43:09Z

UnitTestFrameworkPkg/Library/UnitTestLib/UnitTestLib.c

+  IN UNIT_TEST_LIST_ENTRY  *TestEntry
+  )
+{
+  // TODO: Finish this function.


What's the goal in "finishing" the function?

Old comment that needs to be removed now that the function is implemented

Oh, that function is finished? Then the superfluous return type should be dropped, the result is discarded anyway.

VivianNK · 2024-11-08T19:45:12Z

UnitTestFrameworkPkg/Test/GoogleTest/Sample/SampleGoogleTest/SampleGoogleTest.cpp

+  ASSERT_NE (Pointer, (UINT8 *)NULL);
+  FreePool (Pointer);
+  //
+  // Second free that should be caught by address sanitizer, log details, and exit


It exits the entire GoogleTest or is it able to continue execution outside of this TEST() function? What does "exit" mean in this case?

These test cases verify that the address sanitizer does catch the issue and is verified with the use of EXPECT_DEATH() that allows the rest of the tests to be run.

If a developer actually has a double free in their unit test code or code under test, then the address sanitizer would catch the condition, log a stack back trace and other details, and terminate/exit the unit test application. This is the expected developer experience when a condition is caught by the address sanitizer. It would also generate an error for CI.

VivianNK · 2024-11-08T19:47:51Z

UnitTestFrameworkPkg/Test/GoogleTest/Sample/SampleGoogleTest/SampleGoogleTest.cpp

 int
 main (
  int   argc,
  char  *argv[]
  )
 {
  testing::InitGoogleTest (&argc, argv);
+  testing::FLAGS_gtest_catch_exceptions = 0;


Moving forward we should be adding this flag to any GoogleTest that uses EXPECT_DEATH?

This is a setting for the address sanitizer, not EXPECT_DEATH(). When GoogleTest is combined with the address sanitizer, all exceptions need to be handled by the address sanitizer so it can show details of the failure. This can be done globally with setting the env var GTEST_CATCH_EXCEPTIONS=0. I have done this env var setting in this PR for Azure Pipelines CI agents. And it is recommended that developers set this as well to always see full details.

I have updated the Integration Instruction in the description of this PR with those details. They also need to be added to the UnitTestFrameworkPkg ReadMe.

tianocore-assign-reviewers · 2024-12-19T19:49:14Z

⚠ WARNING: Cannot add some reviewers: A user specified as a reviewer for this PR is not a collaborator of the repository. Please add them as a collaborator to the repository so they can be requested in the future.

Non-collaborators requested:

@Javagedes

Attn Admins:

Admin Instructions:

Add the non-collaborators as collaborators to the appropriate team(s) listed in teams
If they are no longer needed as reviewers, remove them from Maintainers.txt

mdkinney · 2024-12-19T21:23:43Z

@os-d I updated this PR to latest which included the use of VS2022. It is getting linker errors for ASAN symbols missing.

I can build locally with VS2022 tools installed without these errors. Is there a version of the CI agents for VS2022 that include ASAN libs pre-installed? How do we update VS2022 to install ASAN libs if that is not available?

This page claims that Microsoft.VisualStudio.Component.VC.ASAN is installed.

https://github.com/actions/runner-images/blob/main/images/windows/Windows2022-Readme.md

os-d · 2024-12-19T22:44:33Z

@os-d I updated this PR to latest which included the use of VS2022. It is getting linker errors for ASAN symbols missing.

I can build locally with VS2022 tools installed without these errors. Is there a version of the CI agents for VS2022 that include ASAN libs pre-installed? How do we update VS2022 to install ASAN libs if that is not available?

This page claims that Microsoft.VisualStudio.Component.VC.ASAN is installed.

https://github.com/actions/runner-images/blob/main/images/windows/Windows2022-Readme.md

@mdkinney , I'm not familiar with the runner image composition. It does look like the version of the ASAN libs was updated by a major version between 2019 and 2022, did you confirm your local ASAN libs is the same major version?

mdkinney · 2024-12-19T22:54:08Z

This conversation implies that LTCG and ASAN are not compatible with each other.

https://developercommunity.visualstudio.com/t/ebpf-for-windows:-ASAN-linker-error-seen/10508799#T-N10509530

mdkinney · 2024-12-20T00:17:03Z

@os-d Adding /LTCG:OFF for unit test builds did not help. Same missing symbols.

os-d · 2024-12-20T00:52:43Z

This conversation implies that LTCG and ASAN are not compatible with each other.

https://developercommunity.visualstudio.com/t/ebpf-for-windows:-ASAN-linker-error-seen/10508799#T-N10509530

@mdkinney , I can see the private chat there and I’m not sure how relevant it is here, it looks like they had a mismatch of some component using ASAN and some not. Here is the OSS PR where they fixed it: microsoft/ebpf-for-windows#3027, which, they did come to the conclusion that LTCG should be disabled and Whole Program Optimization should be disabled. WPO (/GL) is set, out of curiousity, if you also disable that, does it build with ASAN? I.e. do /GL- in your CC flags.

That being said, this looks suspiciously like what changed, perhaps more info here: https://learn.microsoft.com/en-us/cpp/sanitizers/asan-building?view=msvc-170

which links to: https://devblogs.microsoft.com/cppblog/msvc-address-sanitizer-one-dll-for-all-runtime-configurations/

mdkinney · 2024-12-20T03:24:42Z

@os-d Thanks. I think the key is this note:

The names of the ASan .lib files needed by the linker have changed (the linker normally takes care of this if not manually specifying lib names via /INFERASANLIBS)

I could not get /INFERASANLIBS to work, so the specific libs are specified, and looks like there are different filename. Not sure why my local version of VS2022 works and the one in the CI agent does not. I only installed the build essentials and not enterprise. I will install enterprise edition locally and see if I can reproduce and generate the right solution.

mdkinney · 2024-12-20T07:09:44Z

@os-d I have installed VS2022 Enterprise locally and still can not reproduce this issue.

Add use of DEBUG_CLEAR_MEMORY() macros on all allocation and free operations in MemoryAllocationLibPosix to match behavior of all other MemoryAllocationLib instances. Signed-off-by: Michael D Kinney <[email protected]>

Implement FreeUnitTestEntry(), FreeUnitTestSuiteEntry(), and FreeUnitTestFramework() so the UnitTestLib does not introduce any memory leaks. Signed-off-by: Michael D Kinney <[email protected]>

Update DSC files to always enable DEBUG_CLEAR_MEMORY() macros so memory is cleared on every memory allocation/free operation. Signed-off-by: Michael D Kinney <[email protected]>

Add Empty_C_File_Host_Application_Build.c to BaseTools/Conf that is a C source file with only comments that is used to compile into an OBJ file using CC_FLAGS for a HOST_APPLICATION module and the OBJ is passed into the VS20xx DLINK action to provide the context required to select the correct default windows application libraries. Update build_rule.template to compile the empty C file and generate OBJ in the OUTPUT_DIR of the HOST_APPLICATION component and use the OBJ in the DLINK action. This simplifies CC_FLAGS and DLINK_FLAGS for all modules of type HOST_APPLICATION. Signed-off-by: Michael D Kinney <[email protected]>

* Update host based unit test VS20xx builds to use /MTd instead of /MT to enable use of debug libraries for host based unit tests. * Enable /fsanitize=address for host based unit test VS2019 builds * Enable /fsanitize=address for host based unit test VS2022 builds * Enable -fsanitize=address for host based unit test GCC builds * Add UNIT_TESTING_ADDRESS_SANITIZER_ENABLE define that is set to TRUE by default so it is always enabled, but can be set to FALSE to temporarily disable during development/debug of unit tests. * Add address sanitizer information to ReadMe.md Enabling the Address Sanitizer can detect double frees, buffer overflow, buffer underflow, access to invalid addresses, and various exceptions. These can be detected in both the unit test case sources as well as the code under test. Signed-off-by: Michael D Kinney <[email protected]>

Add GoogleTest and Framework based unit tests that are expected to fail and be caught by Address Sanitizer. These unit tests verify that an address sanitizer is enabled and detecting the following conditions. It also provide examples of the expected output when an Address Sanitizer detected these types of issues. * double free * buffer overflow * buffer underflow * null ptr * invalid address * divide by zero Signed-off-by: Michael D Kinney <[email protected]>

Use snprintf() in host based unit tests to format log messages and add host specific wrapper for LongJump() that is decorated with NORETURN to provide hint to address sanitizer that LongJump() never returns. Signed-off-by: Michael D Kinney <[email protected]>

mdkinney · 2024-12-28T21:58:25Z

@os-d I was able to reproduce the VS2022 issue locally and have a new approach that cleans up HOST_APPLICATION component builds for VS2019 and VS2022 by maximizing the compiler selection of default libraries.

…ress Add HostMemoryAllocationBelowAddressLib class and implementation that uses OS specific services to perform pool and page allocations below a specified address in a host based unit test application execution environment. This library class is only required for mocking buffers that are assumed to be below a specific address by code under test. Signed-off-by: Michael D Kinney <[email protected]>

The environment variable `GTEST_CATCH_EXCEPTION` must be set to `0` for so all exceptions are handled by the address sanitizer and not GoogleTest. This allows stack back trace and other details to be logged by the address sanitizer so the source of the issue identified address sanitizer can be determined. The environment variable `ASAN_OPTIONS` must be set to `detect_leaks=0` to disable memory leak detection. The unit test frameworks may have memory leaks and some firmware code under test use cases may perform a memory allocation without a matching memory free as their expected behavior. Signed-off-by: Michael D Kinney <[email protected]>

mdkinney requested review from ajfish and leiflindholm November 6, 2024 01:46

mhaeuser reviewed Nov 6, 2024

View reviewed changes

mdkinney force-pushed the UnitTestFrameworkPkg_EnableSanitize branch from 1e87ddd to 8e59990 Compare November 7, 2024 18:56

mdkinney force-pushed the UnitTestFrameworkPkg_EnableSanitize branch 3 times, most recently from 0519d7b to 1202caa Compare November 8, 2024 03:21

mdkinney requested review from spbrogan and makubacki November 8, 2024 03:32

This was referenced Nov 8, 2024

CryptoPkg/BaseCryptLib: Fix serial number read overrun #6418

Merged

UefiCpuPkg/MtrrLib: Fix unit test read overflow #6419

Merged

NetworkPkg/Dhcp6Dxe: Fix sanitizer issues #6420

Merged

MdePkg/Test/DevicePathLib: Remove FreePool(NULL) #6421

Merged

VivianNK reviewed Nov 8, 2024

View reviewed changes

VivianNK approved these changes Nov 8, 2024

View reviewed changes

mdkinney force-pushed the UnitTestFrameworkPkg_EnableSanitize branch 2 times, most recently from bf4d2d4 to 056be99 Compare December 6, 2024 01:44

mdkinney marked this pull request as ready for review December 19, 2024 19:48

mdkinney force-pushed the UnitTestFrameworkPkg_EnableSanitize branch from 056be99 to d217f6b Compare December 19, 2024 19:49

tianocore-assign-reviewers bot requested review from bexcran, BobCF, lgao4 and YuweiChen1110 December 19, 2024 19:49

mdkinney force-pushed the UnitTestFrameworkPkg_EnableSanitize branch from d217f6b to fe2aa39 Compare December 19, 2024 22:51

mdkinney added 3 commits December 27, 2024 15:38

UnitTestFrameworkPkg/UnitTestLib: Implement Free*() services

d12a381

Implement FreeUnitTestEntry(), FreeUnitTestSuiteEntry(), and FreeUnitTestFramework() so the UnitTestLib does not introduce any memory leaks. Signed-off-by: Michael D Kinney <[email protected]>

UnitTestFraworkPkg: Enable DEBUG_CLEAR_MEMORY in host tests

3ec7520

Update DSC files to always enable DEBUG_CLEAR_MEMORY() macros so memory is cleared on every memory allocation/free operation. Signed-off-by: Michael D Kinney <[email protected]>

mdkinney force-pushed the UnitTestFrameworkPkg_EnableSanitize branch from 988bae0 to f69d5b0 Compare December 27, 2024 23:39

mdkinney added 4 commits December 27, 2024 20:25

mdkinney force-pushed the UnitTestFrameworkPkg_EnableSanitize branch from f69d5b0 to c6930e8 Compare December 28, 2024 21:55

mdkinney added 2 commits December 28, 2024 15:25

mdkinney force-pushed the UnitTestFrameworkPkg_EnableSanitize branch from c6930e8 to 6fc0a9e Compare December 28, 2024 23:26

Unit test framework pkg enable sanitize #6408

Are you sure you want to change the base?

Unit test framework pkg enable sanitize #6408

Conversation

mdkinney commented Nov 6, 2024 • edited Loading

Description

How This Was Tested

Integration Instructions

Windows

Linux

mdkinney commented Nov 6, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

mdkinney Nov 7, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

heatd commented Nov 7, 2024

mdkinney commented Nov 7, 2024

mdkinney commented Nov 7, 2024

heatd commented Nov 7, 2024

mdkinney commented Nov 7, 2024

mhaeuser commented Nov 7, 2024

mdkinney commented Nov 7, 2024

mdkinney commented Nov 8, 2024

mhaeuser commented Nov 8, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

VivianNK Nov 8, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

tianocore-assign-reviewers bot commented Dec 19, 2024

mdkinney commented Dec 19, 2024 • edited Loading

os-d commented Dec 19, 2024

mdkinney commented Dec 19, 2024

mdkinney commented Dec 20, 2024

os-d commented Dec 20, 2024

mdkinney commented Dec 20, 2024

mdkinney commented Dec 20, 2024

mdkinney commented Dec 28, 2024

mdkinney commented Nov 6, 2024 •

edited

Loading

mdkinney Nov 7, 2024 •

edited

Loading

VivianNK Nov 8, 2024 •

edited

Loading

mdkinney commented Dec 19, 2024 •

edited

Loading