Add winsock2 header into build_info.h

[irwir]

Description

Ensures proper socket definitions on Windows (as a part of resolving mbedtls issue #10097).

PR checklist

• changelog not required
• framework PR provided #193
• mbedtls development PR provided #10140
• mbedtls 3.6 PR not required because: not planned
• tests not required

[gilles-peskine-arm]

TF-PSA-Crypto needs to build outside of Mbed TLS. So it doesn't seem right to me that the Windows build (with an unchanged mbedtls) is failing here.

[irwir]

This is odd if an additional standard header breaks things so badly.

[2025-04-21T19:59:35.604Z] 291>C:/Users/Administrator/AppData/Local/Temp/tmp5cfern6y/tf-psa-crypto/tests/suites/test_suite_ecp.function(19): error C2143: syntax error : missing ')' before 'constant' [C:\Users\Administrator\AppData\Local\Temp\tmp5cfern6y\cmake_solution\tf-psa-crypto\tests\test_suite_ecp.vcxproj]

How to parse this?
The C2143 error code from a compiler and the last word vcxproj indicate a compilation error.
But the source file is test_suite_ecp.function while make build uses tf-psa-crypto/tests/test_suite_ecp.c?

[irwir]

[2025-04-21T19:59:36.764Z] cl : Command line warning D9002: ignoring unknown option '/utf-8' [C:\Users\Administrator\AppData\Local\Temp\tmp5cfern6y\cmake_solution\tf-psa-crypto\tests\test_suite_gcm.aes128_en.vcxproj]

Numerous warnings in the log. Maybe VS 2013 did not have this option?
Unlike for mbdetls, tests were never tried with later versions of Visual Studio.

[gilles-peskine-arm]

test_suite_xxx.c is generated from test_suite_xxx.function using framework/scripts/generate_test_code.py. It does a couple of substitutions and adds some boilerplate as well as code to prepare arguments to test functions.

The offending line (more visible in the message from MinGW) is

inline static int mbedtls_ecp_group_cmp(mbedtls_ecp_group *grp1,

It looks like the parser gets confused because it doesn't recognize inline as intended.

We use the inline keyword in our code base. For compilers such as MSVC that don't define it, tf-psa-crypto/build_info.h defines it. This happens shortly before the inclusion of winsock2.h. I have no idea why including winsock2.h breaks this.

As for /utf-8, the warning is annoying but harmless. It was added to fix a warning that does break the build in some environments (those using a non-UTF-8 multibyte encoding by default, if I understand correctly).

[irwir]

test_suite_xxx.c is generated from test_suite_xxx.function

The question was why .function instead of the expected .c in the compiler diagnostics. This was explained by #line directive.

I have no idea why including winsock2.h breaks this.

It is difficult to guess without installed VS 2013 and the generated projects and sources.
A trivial inlined function after including build_info.h could be compiled even in VS 2012. The issue should be somewhere deeper.
With VS 2022, I copied and renamed cert_app.vcxproj, changed the source file to test_suite_ecp.c. Some paths had to be added, but finally it compiled.

[irwir]

As for /utf-8

Available since VS 2015.

[irwir]

As far as I remember, a bug in VS 2015 increased the minimum required version for mbedtls to VS 2017, even though the official documentation still says VS 2013 (also in solution path visualc/VS2013/mbedTLS.sln).

Quote from the current README.md in TF-PSA-Crypto:

The following instructions have been tested on Microsoft Visual Studio Community 2019 Version 16.11.26.

Yet there is nothing but VS 2013 in TF OpenCI: PR tests.

Should this have been changed in development branches?

[irwir]

Two questions.
Could TF OpenCI: PR tests be retried with VS 2017?
Would it be possible to get the failing generated projects and source files, as artifacts maybe?
At least one, like test_suite_ecp.c and test_suite_ecp.vcxproj for investigation in VS 2017.

It looks like the parser gets confused because it doesn't recognize inline as intended.

Text search in TF-PSA-Crypto sources had found 51 files containing the inline word. A few tests had been built successfully before the failure.

[gilles-peskine-arm]

Could TF OpenCI: PR tests be retried with VS 2017?

Sure. Since the last CI run, we've switched our CI from VS2013 to VS2017. I've kicked a new run.

Would it be possible to get the failing generated projects and source files, as artifacts maybe?

Setting this up would require some Groovy code. But I don't really see the point: these files don't depend on the platform, on the library configuration or (in principle) on tool versions, you can generate them locally on any platform.

[irwir]

Setting this up would require some Groovy code. But I don't really see the point: these files don't depend on the platform, on the library configuration or (in principle) on tool versions, you can generate them locally on any platform.

It could be generated provided everything is installed, configured and it is perfectly well known what commands to run.
But there is only VS 2017 for C/C++ builds. Nothing else, and nothing is planned to be added.
There is also Ubuntu 25.04 which is used for generating files with make.
So it was cheaper to manually create a VS project for the test code and then figure out what was wrong.

Build process now is working, but tests are failing.
With make all two versions had been built - with included winsock2.h and without it.
The only difference in the sources and generated files was build_info.h.
Knowing this, I locally created two builds of test_suite_ssl - with winsock2.h and with the #include commented out.
All compiled files were deleted before the rebuild.
The results are exactly the same, some of Session serialization tests had failed.

[irwir]

Every test in test_suite_ssl.datax that contains path name of a certificate file - failed.
Corrected path names had this outcome:
PASSED (854 / 854 tests (146 skipped))

[gilles-peskine-arm]

You need to run the tests suites from the tests directory.

There's code in tests/suites/host_test.function that tries to automatically change into the directory containing the executable. This is relatively new but our scripts might have gotten used to that. But that code isn't implemented for Windows (patches welcome).

[irwir]

TF-PSA-Crypto/tests/suites/host_test.function

Lines 446 to 449 in 864dee0

	/* We might want to allow backslash as well, for Windows. But then we also
	* need to consider chdir() vs _chdir(), and different conventions
	* regarding paths in argv[0] (naively enabling this code with
	* backslash support on Windows leads to chdir into the wrong directory

This code correctly changed directory for me; verified with _getcwd().
Certainly backslash for argv[0] because the path was created by CRT.
chdir is the same as _chdir; deprecation warnings have low level (ignored now, and could be disabled).
strdup (or _strdup) and truncation with null character could give shorter code.

But there is a catch.
Full path for a data file may be specified in command line; and the data may contain paths relative to the file's location.
Which means, chdir should be to the data file's path, not executable's path. Also (forward) slashes might be here.
Unless this line

You need to run the tests suites from the tests directory.

means "executable must be copied to tests directory" - which is not always a great idea.
Another trouble:
const char *default_filename = "./test_suite_ssl.datax";
With the current code this path in command line means not what might be expected:
tests/test_suite_ssl ./test_suite_ssl.datax

[irwir]

To verify that only tests were failing, one final check was attempted with the line #include <winsock2.h> commented out. Build process would not start automatically, hence the commit was reverted.

But that code isn't implemented for Windows (patches welcome).

It is hard to provide useful patches without decent understanding of the testing structure and the future plans.

[irwir]

Currently this PR seems to be blocked by a flaw in test code.
If so, are there any plans to move on?

[gilles-peskine-arm]

This pull request is blocked because it fails the CI. It's not a flaw in test code: the patch makes the existing code fail to compile. Since it only happens on Windows, which most of us are unfamiliar with, this is unlikely to progress unless a Windows expert can fix the problem.

[irwir]

This pull request is blocked because it fails the CI. It's not a flaw in test code: the patch makes the existing code fail to compile.

Compilation had been fixed months ago (the second commit). Since then the test_suite_ssl failed in every test that required to read a certificate file. With a bit of code tweaking test_suite_ssl passed all 854 tests (VS 2017).
The history is in the above conversation.
Outdated build logs had been deleted, but for verification you could run the process again.

[gilles-peskine-arm]

Ok, I've kicked a new CI run.

[irwir]

New kind of errors in build (exception in compiler?).
Synced PR with Mbed-TLS/TF-PSA-Crypto:development branch.

[irwir]

Rebased the PR and somehow all tests had passed.

[irwir]

It would be nice to add under the Windows condition this definition:

#ifndef WIN32_LEAN_AND_MEAN
#define WIN32_LEAN_AND_MEAN
#endif

But the macro must be added into name checker's exclusion list.

[gilles-peskine-arm]

But the macro must be added into name checker's exclusion list.

That needs a patch to https://github.com/Mbed-TLS/mbedtls-framework/blob/main/scripts/check_names.py

[irwir]

At line 343 like this?

exclusions = (
"asm", "inline", "EMIT", "CRT_SECURE_NO_DEPRECATE", "MULADDC",
"WIN32_LEAN_AND_MEAN"
)

[gilles-peskine-arm]

Yes, that looks right, WIN32_LEAN_AND_MEAN and _CRT_SECURE_NO_DEPRECATE have very similar roles.

[irwir]

If this was merged, <windows.h> and <winsock2.h> could be removed from .c files in TF-PSA-Crypto and mbedtls.