Skip to content

Fix Windows OPENSSL_SMALL Debug builds with NASM 2.16.01 - #3420

Merged
justsmth merged 1 commit into
aws:mainfrom
justsmth:fix-nasm-2.16.01-codeview
Aug 19, 2026
Merged

justsmth merged 1 commit into
aws:mainfrom
justsmth:fix-nasm-2.16.01-codeview

Conversation

@justsmth

Copy link
Copy Markdown
Contributor

Description of changes:

NASM 2.16.01 can crash while generating CodeView debug information for empty text sections produced when AVX-512 assembly is disabled in OPENSSL_SMALL builds. Emit an unreferenced byte in the affected disabled assembly sources so the section is non-empty, and update CI to reproduce the failure using NASM 2.16.01 with a Windows Debug build.

Call-outs:

The byte is emitted only when MY_ASSEMBLER_IS_TOO_OLD_FOR_512AVX disables the corresponding AVX-512 implementation.

Testing:

  • Regenerated the pre-generated assembly sources and verified the affected files assemble directly with NASM 2.16.01 and debug information enabled.
  • Built and ran the C/C++ tests on Windows Server 2022 using the affected NASM version and the pre-generated OPENSSL_SMALL Debug configuration.
  • Built and tested the change on Linux and macOS, including FIPS and blackbox SSL runner coverage.

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license and the ISC license.

Emit a byte when AVX-512 assembly is disabled so NASM 2.16.01 does not crash while generating CodeView information for an empty section.

Pin NASM 2.16.01 and run the pre-generated OPENSSL_SMALL Windows CI leg in Debug mode to cover the failure.
@github-actions

Copy link
Copy Markdown
Contributor

🔒 Security ReviewView Report

Please review before merging.

@codecov-commenter

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 78.21%. Comparing base (1ab8a2c) to head (8a5fa07).
⚠️ Report is 2 commits behind head on main.

Additional details and impacted files
@@           Coverage Diff           @@
##             main    #3420   +/-   ##
=======================================
  Coverage   78.20%   78.21%           
=======================================
  Files         695      695           
  Lines      124271   124282   +11     
  Branches    17265    17267    +2     
=======================================
+ Hits        97186    97205   +19     
+ Misses      26161    26155    -6     
+ Partials      924      922    -2     

☔ View full report in Codecov by Harness.
📢 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.

@justsmth
justsmth requested a review from geedo0 August 14, 2026 13:46
@justsmth
justsmth requested a review from sgmenda August 14, 2026 19:57

@sgmenda sgmenda left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

lgtm with nits

- name: Install NASM
uses: ilammy/setup-nasm@v1.5.1
with:
version: "2.16.01"

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

do we wanna globally fix the nasm version? versus test this and other versions customers use?

- { name: "pre-generated", disable_perl: "ON", disable_avx: "OFF" }
- { name: "perlasm, AVX disabled", disable_perl: "OFF", disable_avx: "ON" }
- { name: "perlasm", build_type: "Release", disable_perl: "OFF", disable_avx: "OFF" }
- { name: "pre-generated, Debug", build_type: "Debug", disable_perl: "ON", disable_avx: "OFF" }

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

do we wanna also test pre-generated, Release?

@justsmth
justsmth merged commit 6e077c6 into aws:main Aug 19, 2026
535 of 538 checks passed
@justsmth
justsmth deleted the fix-nasm-2.16.01-codeview branch August 19, 2026 13:21
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants