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] Revert Mu Change in Favor of edk2 Change #1305

Merged
merged 2 commits into from
Mar 22, 2025

Conversation

os-d
Copy link
Contributor

@os-d os-d commented Mar 13, 2025

Description

Commit 1203e4a was upstreamed to edk2 as 81031a51a0148a5ff7904f02cc405c8f3b910c55.

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

How This Was Tested

N/A.

Integration Instructions

N/A.

Sorry, something went wrong.

@os-d os-d requested review from makubacki and apop5 March 13, 2025 20:35
@github-actions github-actions bot added the impact:non-functional Does not have a functional impact label Mar 13, 2025
@codecov-commenter
Copy link

codecov-commenter commented Mar 13, 2025

Codecov Report

All modified and coverable lines are covered by tests ✅

Please upload report for BASE (dev/202502@4efe7fb). Learn more about missing BASE report.

Additional details and impacted files
@@              Coverage Diff              @@
##             dev/202502    #1305   +/-   ##
=============================================
  Coverage              ?    1.59%           
=============================================
  Files                 ?     1408           
  Lines                 ?   364660           
  Branches              ?     4570           
=============================================
  Hits                  ?     5799           
  Misses                ?   358788           
  Partials              ?       73           
Flag Coverage Δ
MdeModulePkg 0.64% <ø> (?)
MdePkg 5.54% <ø> (?)
NetworkPkg 0.55% <ø> (?)
PolicyServicePkg 30.42% <ø> (?)
UefiCpuPkg 4.86% <ø> (?)

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.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

Copy link
Member

@makubacki makubacki left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you please include the "(cherry picked from commit ...)" message at the end of the commit? The one that comes from the -x argument.

This reverts commit 1203e4a as
it was upstreamed to edk2 as 81031a51a0148a5ff7904f02cc405c8f3b910c55.
@apop5 apop5 force-pushed the 2502_basetools_cp branch from a767875 to f3b7235 Compare March 22, 2025 02:48
VS2019/VS2022 ARM/AARCH64 is not a widely used toolchain, for one
thing edk2 can't be built with it, it will break. Downstream
platforms rarely use it and if they do, they must have heavy edits
in order to support building edk2. In particular, edk2 does not
have support for the assembly files that this toolchain uses fully.

As a result, the corresponding StackCheckLib does not have the assembly
file needed to satisfy the definitions the compiler expects.

Unfortunately, the VS ARM/AARCH64 compiler has a different ABI than
the IA32/X64 VS toolchain for stack cookies, so this also needs more
investigation.

For now, disable stack cookie checking in VS ARM/AARCH64 as this does
not affect many platforms. However, it does allow for the use case
reported in the bug mentioning this, which is building a shell and
attempting to boot to it.

When VS ARM/AARCH64 support is revisited in edk2 (or if there is a
clean way to add stack cookie support without the full support), this
will be revisted.

Signed-off-by: Oliver Smith-Denny <[email protected]>
(cherry picked from commit 81031a51a0148a5ff7904f02cc405c8f3b910c55)
@apop5 apop5 force-pushed the 2502_basetools_cp branch from f3b7235 to 187ad62 Compare March 22, 2025 02:50
@apop5 apop5 enabled auto-merge (rebase) March 22, 2025 02:51
@apop5 apop5 merged commit cf9c7ce into microsoft:dev/202502 Mar 22, 2025
32 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
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants