Skip to content

Enable Windows 7 compat path on MinGW builds#3239

Open
justsmth wants to merge 1 commit into
aws:mainfrom
justsmth:fix-win7-mingw
Open

Enable Windows 7 compat path on MinGW builds#3239
justsmth wants to merge 1 commit into
aws:mainfrom
justsmth:fix-win7-mingw

Conversation

@justsmth
Copy link
Copy Markdown
Contributor

Issues:

Addresses aws/aws-lc-rs#1111

Description of changes:

crypto/rand_extra/windows.c gates the Windows 7 compatibility branch (AWSLC_WINDOWS_7_COMPAT, which uses BCryptGenRandom instead of ProcessPrng) on:

#if !defined(__MINGW32__) && defined(_WIN32_WINNT) && _WIN32_WINNT <= _WIN32_WINNT_WIN7

The !defined(__MINGW32__) guard was suppressing that branch on MinGW even though the top-level CMakeLists.txt already forces -D_WIN32_WINNT=_WIN32_WINNT_WIN7 for MINGW AND NOT CLANG. So MinGW builds were effectively configured to target Windows 7 but then took the ProcessPrng/bcryptprimitives.dll path, which aborts at runtime on Windows 7 because GetProcAddress for ProcessPrng returns NULL there.

This change drops the MinGW guard so the Win7 path is taken when _WIN32_WINNT targets Win7, regardless of toolchain, and adds an explicit target_link_libraries(... bcrypt) for MinGW in crypto/CMakeLists.txt. The link is required because MinGW ignores the #pragma comment(lib, "bcrypt.lib") that MSVC uses inside the same file.

Call-outs:

  • The bcrypt link is added unconditionally on MinGW rather than mirroring the preprocessor condition, since detecting the effective _WIN32_WINNT at configure time is fragile. bcrypt.dll exists on all supported Windows versions, and the extra PE import is negligible when the Win7 branch isn't active.
  • The existing cross-mingw CI job (which sets _WIN32_WINNT=_WIN32_WINNT_WIN7 via the top-level CMakeLists.txt) will now naturally exercise AWSLC_WINDOWS_7_COMPAT; no new CI job is needed.
  • A follow-up is needed in aws-lc-rs to handle the x86_64-win7-windows-gnu Rust target in its builder once this change is pulled in via submodule bump.

Testing:

Local cross-compile from macOS with x86_64-w64-mingw32-gcc 14.0.0:

  • Static libcrypto.a builds; nm on rand_extra/windows.c.obj shows only U BCryptGenRandom.
  • Shared libcrypto.dll links cleanly; objdump -x shows bcrypt.dll / BCryptGenRandom in the import table and no bcryptprimitives.dll / ProcessPrng references.

The existing cross-mingw CI job will now exercise this code path end-to-end under Wine (Wine implements BCryptGenRandom).

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.

The `AWSLC_WINDOWS_7_COMPAT` branch in crypto/rand_extra/windows.c was
guarded by `!defined(__MINGW32__)`, which prevented it from being taken
on MinGW even though the top-level CMakeLists.txt already sets
`-D_WIN32_WINNT=_WIN32_WINNT_WIN7` for MINGW-and-not-Clang builds. As
a result, MinGW builds took the ProcessPrng path and aborted at runtime
on Windows 7 (GetProcAddress for ProcessPrng returns NULL on Win7).

Drop the MinGW guard so the Win7 path is selected when _WIN32_WINNT
targets Win7, and add an explicit `bcrypt` link for MinGW since it
ignores the `#pragma comment(lib, "bcrypt.lib")` used by MSVC.

Addresses aws/aws-lc-rs#1111
@justsmth justsmth requested a review from a team as a code owner May 11, 2026 18:04
@codecov-commenter
Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 78.14%. Comparing base (9651480) to head (c3aaed1).

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #3239      +/-   ##
==========================================
+ Coverage   78.12%   78.14%   +0.02%     
==========================================
  Files         689      689              
  Lines      123214   123214              
  Branches    17137    17136       -1     
==========================================
+ Hits        96257    96286      +29     
+ Misses      26047    26018      -29     
  Partials      910      910              

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

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.

2 participants