Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[CHERRY-PICK] VS22 linker changes from edk2 [Rebase & FF] #1284

Merged
merged 3 commits into from
Mar 3, 2025

Conversation

makubacki
Copy link
Member

@makubacki makubacki commented Feb 26, 2025

Description

Cherry picks a commit upstreamed (reworked from Mu original) and a similar commit made upstream to eliminate Mu delta.


[NEW] MdeModulePkg/HiiDatabaseDxe: Avoid struct assignment

Struct assignments are not permitted in EDK2, as they may be converted
by the compiler into calls to the 'memcpy' intrinsic, which is not
guaranteed to be available in EDK2.

So replace the assignment with a call to CopyMem (), and -while at it-
replace the loop with a single CopyMem () call, as the loop operates on
items that are contiguous in memory.


[NON-FUNCTIONAL REFACTOR] MdeModulePkg/HiiDatabaseDxe: Prevent linker error

Prevent an issue where memcpy() instrinsic is being used after
recent MSVC linker update in windows-2022 VM image from 14.31.31103
to 14.32.31326.


[DROP MU_CHANGE TAGS] MdePkg/BaseMemoryLib: Prevent potential VS2022 linker failure

After updating between various VS2022 versions such as 17.4 to
17.5, , linker failures began to appear in several modules because
memset is an unresolved symbol.

The following functions in BaseMemoryLib/MemLibGeneric.c have 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:

Note: This image (with 17.4) does 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 with a
relatively minimum delta to prior implementation.


  • Impacts functionality?
  • Impacts security?
  • Breaking change?
  • Includes tests?
  • Includes documentation?
  • Backport to release branch?

How This Was Tested

  • VS22 build and CI.
  • QEMU boot.

Integration Instructions

  • N/A

@makubacki makubacki requested review from os-d and apop5 February 26, 2025 14:22
@makubacki makubacki self-assigned this Feb 26, 2025
@github-actions github-actions bot added impact:non-functional Does not have a functional impact type:backport Backport changes in a dev branch PR to its release branch. labels Feb 26, 2025
@makubacki makubacki enabled auto-merge (rebase) February 26, 2025 14:23
@codecov-commenter
Copy link

codecov-commenter commented Feb 26, 2025

Codecov Report

Attention: Patch coverage is 0% with 61 lines in your changes missing coverage. Please review.

Project coverage is 1.60%. Comparing base (c99305d) to head (4150657).

Files with missing lines Patch % Lines
MdeModulePkg/Universal/HiiDatabaseDxe/Image.c 0.00% 61 Missing ⚠️
Additional details and impacted files
@@              Coverage Diff               @@
##           dev/202405    #1284      +/-   ##
==============================================
- Coverage        1.60%    1.60%   -0.01%     
==============================================
  Files            1383     1383              
  Lines          359859   359864       +5     
  Branches         5524     5524              
==============================================
  Hits             5760     5760              
- Misses         353992   353997       +5     
  Partials          107      107              
Flag Coverage Δ
MdeModulePkg 0.67% <0.00%> (-0.01%) ⬇️
MdePkg 5.59% <ø> (ø)

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@makubacki makubacki force-pushed the cp_hiidb_lnk_changes_2405 branch from 1bbd3c2 to 796aece Compare February 28, 2025 16:33
@makubacki makubacki changed the title [CHERRY-PICK] MdeModulePkg/HiiDatabaseDxe: VS22 linker changes from edk2 [Rebase & FF] [CHERRY-PICK] VS22 linker changes from edk2 [Rebase & FF] Feb 28, 2025
@makubacki makubacki requested a review from kuqin12 February 28, 2025 16:36
@makubacki makubacki disabled auto-merge February 28, 2025 16:37
@makubacki makubacki enabled auto-merge (rebase) February 28, 2025 16:37
ardbiesheuvel and others added 3 commits March 3, 2025 16:14
Struct assignments are not permitted in EDK2, as they may be converted
by the compiler into calls to the 'memcpy' intrinsic, which is not
guaranteed to be available in EDK2.

So replace the assignment with a call to CopyMem (), and -while at it-
replace the loop with a single CopyMem () call, as the loop operates on
items that are contiguous in memory.

Signed-off-by: Ard Biesheuvel <[email protected]>
(cherry picked from commit de4cc40)
Prevent an issue where `memcpy()` instrinsic is being used after
recent MSVC linker update in windows-2022 VM image from 14.31.31103
to 14.32.31326.

Signed-off-by: Michael Kubacki <[email protected]>
(cherry picked from commit 29f02d0161bc5d10a6e3b2cfc4d10799644964a5)
…ailure

After updating between various VS2022 versions such as 17.4 to
17.5, , linker failures began to appear in several modules because
`memset` is an unresolved symbol.

The following functions in BaseMemoryLib/MemLibGeneric.c have 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: This image (with 17.4) does 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 with a
relatively minimum delta to prior implementation.

Signed-off-by: Michael Kubacki <[email protected]>
(cherry picked from commit c46bc0ea98b4a8483c970e2282ea46891ea94f57)
@makubacki makubacki force-pushed the cp_hiidb_lnk_changes_2405 branch from 796aece to 4150657 Compare March 3, 2025 21:14
@makubacki makubacki merged commit b077d2f into microsoft:dev/202405 Mar 3, 2025
31 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
impact:non-functional Does not have a functional impact type:backport Backport changes in a dev branch PR to its release branch.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants