Skip to content

Conversation

os-d
Copy link
Contributor

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

Description

Commit 1203e4a was upstreamed to edk2 as 81031a5.

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

How This Was Tested

N/A.

Integration Instructions

N/A.

@os-d os-d requested review from apop5 and makubacki 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 81031a5.
@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 <osde@microsoft.com>
(cherry picked from commit 81031a5)
@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.

4 participants