Skip to content

Commit

Permalink
MdePkg/BaseMemoryLib: Prevent VS2022 (17.5) linker failure (#290)
Browse files Browse the repository at this point in the history
Fixes #291 

## Description

After updating from Visual Studio 17.4 to 17.5, a large number of
linker failures began to appear in several modules because `memset`
is an unresolved symbol.

It was found that the following functions in BaseMemoryLib/MemLibGeneric.c
were having their loop pattern replaced with the `memset` intrinsic
function:

- `InternalMemSetMem16()`
- `InternalMemSetMem32()`
- `InternalMemSetMem64()`

An example of an error related to `InternalMemSetMem64()` in
VariableSmmRuntimeDxe is shown below:

```
INFO - BaseMemoryLib.lib(MemLibGeneric.obj) : error LNK2001:
         unresolved external symbol memset
INFO - <...>\VariableSmmRuntimeDxe.dll : fatal error LNK1120:
         1 unresolved externals
```

This was reproduced in several environments including:

- Public VM image:
  https://github.com/actions/runner-images/blob/win22/20230226.1/images/win/Windows2022-Readme.md

- Locally when updating from 17.4.4 to 17.5.1

> Note: The previous public VM image (with 17.4) did not have this issue
  https://github.com/actions/runner-images/blob/win22/20230219.1/images/win/Windows2022-Readme.md

This change updates the type cast for the destination buffer to be
a pointer to `volatile` data to prevent this optimization and introduce
a minimum delta to prior implementation.

---

- [ ] Impacts functionality?
  - **Functionality** - Does the change ultimately impact how firmware functions?
  - Examples: Add a new library, publish a new PPI, update an algorithm, ...
- [ ] Impacts security?
  - **Security** - Does the change have a direct security impact on an application,
    flow, or firmware?
  - Examples: Crypto algorithm change, buffer overflow fix, parameter
    validation improvement, ...
- [ ] Breaking change?
  - **Breaking change** - Will anyone consuming this change experience a break
    in build or boot behavior?
  - Examples: Add a new library class, move a module to a different repo, call
    a function in a new library class in a pre-existing module, ...
- [ ] Includes tests?
  - **Tests** - Does the change include any explicit test code?
  - Examples: Unit tests, integration tests, robot tests, ...
- [ ] Includes documentation?
  - **Documentation** - Does the change contain explicit documentation additions
    outside direct code modifications (and comments)?
  - Examples: Update readme file, add feature readme file, link to documentation
    on an a separate Web page, ...

## How This Was Tested

Built several times with and without the change on Visual Studio 17.5.1
locally and on CI agents.

## Integration Instructions

Include this change if building with the 17.5.1. It is currently the latest
release in most recent Windows VM images.

Signed-off-by: Michael Kubacki <michael.kubacki@microsoft.com>
  • Loading branch information
makubacki authored and kenlautner committed May 5, 2023
1 parent d64b277 commit cabef2a
Showing 1 changed file with 9 additions and 3 deletions.
12 changes: 9 additions & 3 deletions MdePkg/Library/BaseMemoryLib/MemLibGeneric.c
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,9 @@ InternalMemSetMem16 (
)
{
for ( ; Length != 0; Length--) {
((UINT16 *)Buffer)[Length - 1] = Value;
// MU_CHANGE use a pointer to volatile data to prevent Visual Studio 17.5 (VS2022)
// from replacing the assignment with a `memset()` intrinsic
((volatile UINT16 *)Buffer)[Length - 1] = Value;
}

return Buffer;
Expand All @@ -57,7 +59,9 @@ InternalMemSetMem32 (
)
{
for ( ; Length != 0; Length--) {
((UINT32 *)Buffer)[Length - 1] = Value;
// MU_CHANGE use a pointer to volatile data to prevent Visual Studio 17.5 (VS2022)
// from replacing the assignment with a `memset()` intrinsic
((volatile UINT32 *)Buffer)[Length - 1] = Value;
}

return Buffer;
Expand All @@ -82,7 +86,9 @@ InternalMemSetMem64 (
)
{
for ( ; Length != 0; Length--) {
((UINT64 *)Buffer)[Length - 1] = Value;
// MU_CHANGE use a pointer to volatile data to prevent Visual Studio 17.5 (VS2022)
// from replacing the assignment with a `memset()` intrinsic
((volatile UINT64 *)Buffer)[Length - 1] = Value;
}

return Buffer;
Expand Down

0 comments on commit cabef2a

Please sign in to comment.