Skip to content

BaseTools: Disable VS2019/2022 ARM/AARCH64 Stack Cookies #10803

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

Merged
merged 2 commits into from
Mar 12, 2025

Conversation

os-d
Copy link
Contributor

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

Description

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 full support for the assembly files that this toolchain.

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 revisited.

Note that the main use case of GCC on ARM/AARCH64 remains supporting stack cookies.

Closes #10802.

  • 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

Building AARCH64 VS.

Integration Instructions

N/A.

@github-actions github-actions bot added the impact:security This change has a direct security impact such as changing a crypto algorithm. label Mar 1, 2025
@leiflindholm
Copy link
Member

Looks sensible to me.

@os-d
Copy link
Contributor Author

os-d commented Mar 11, 2025

@bexcran @BobCF @lgao4 @YuweiChen1110 friendly ping to please review

@lgao4 lgao4 added the push Auto push patch series in PR if all checks pass label Mar 12, 2025
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]>
@lgao4 lgao4 force-pushed the vsarm_stack_cookies branch from c127124 to cae6141 Compare March 12, 2025 03:09
@mergify mergify bot merged commit 81031a5 into tianocore:master Mar 12, 2025
126 checks passed
@os-d os-d deleted the vsarm_stack_cookies branch March 13, 2025 20:33
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
impact:security This change has a direct security impact such as changing a crypto algorithm. push Auto push patch series in PR if all checks pass
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[Bug]: <title>error LNK2001: unresolved external symbol __security_pop_cookie Compile for AARCH64 in SafeString.obj
3 participants