More fixes for building x86 in Visual Studio for non-windows OS #8098

dgarske · 2024-10-22T18:57:38Z

Description

More fixes for building x86 in Visual Studio for non-windows OS (Watcom C compiler). Followup to PR #7884. Fixes ZD 18465

Consolidate the USE_WINDOWS_API to a single place.
Expand the WOLFSSL_NOT_WINDOWS_API improvement for intrinsics and word sizes.
Fix for macro variadic ... when no variables are used (some compilers like Watcom C have issue with this).
Fix for Watcom C compiler "long long" -> "__int64".
Fix a couple of minor cast warnings reported from VS.
Fixes for consistency in SAVE_VECTOR_REGISTERS , ASSERT_SAVED_VECTOR_REGISTERS and RESTORE_VECTOR_REGISTERS.

Testing

Worked with customer

Checklist

added tests
updated/added doxygen
updated appropriate READMEs
Updated manual and documentation

dgarske · 2024-10-23T17:26:13Z

Stage 'PRB-medium-runtime.txt_4'
--enable-all --enable-asn=template --enable-dtls-mtu 
FAIL: scripts/openssl.test

dgarske · 2024-10-23T18:01:38Z

socat Tests:

FAILED: 146 216 309 310 326 386 399 402 459 460 467 468 478 492 528 530
FAILED unexpected: 326

…om C compiler). Followup to PR wolfSSL#7884. Fixes ZD 18465 * Consolidate the USE_WINDOWS_API to a single place. * Expand the `WOLFSSL_NOT_WINDOWS_API` improvement for intrinsics and word sizes. * Fix for macro variadic `...` when no variables are used (some compilers like Watcom C have issue with this). * Fix for Watcom C compiler "long long" -> "__int64". * Fix a couple of minor cast warnings reported from VS.

douzzer · 2024-11-01T20:44:18Z

wolfssl/wolfcrypt/types.h

+        #ifdef __WATCOMC__
+        #define SAVE_VECTOR_REGISTERS() WC_DO_NOTHING


turns out there's a fallthrough definition that works for both:

@@ -1758,7 +1758,7 @@ typedef struct w64wrapper { #endif #ifndef SAVE_VECTOR_REGISTERS - #define SAVE_VECTOR_REGISTERS(...) WC_DO_NOTHING + #define SAVE_VECTOR_REGISTERS(fail_clause) WC_DO_NOTHING #endif #ifndef SAVE_VECTOR_REGISTERS2 #define SAVE_VECTOR_REGISTERS2() 0 @@ -1772,10 +1772,10 @@ typedef struct w64wrapper { #define WC_DEBUG_SET_VECTOR_REGISTERS_RETVAL(x) WC_DO_NOTHING #endif #ifndef ASSERT_SAVED_VECTOR_REGISTERS - #define ASSERT_SAVED_VECTOR_REGISTERS(...) WC_DO_NOTHING + #define ASSERT_SAVED_VECTOR_REGISTERS(fail_clause) WC_DO_NOTHING #endif #ifndef ASSERT_RESTORED_VECTOR_REGISTERS - #define ASSERT_RESTORED_VECTOR_REGISTERS(...) WC_DO_NOTHING + #define ASSERT_RESTORED_VECTOR_REGISTERS(fail_clause) WC_DO_NOTHING #endif #ifndef RESTORE_VECTOR_REGISTERS #define RESTORE_VECTOR_REGISTERS() WC_DO_NOTHING

on the other hand, defining it to take no args doesn't work on C99, for example:

wolfcrypt/src/sp_int.c:19625:5: error: 'SAVE_VECTOR_REGISTERS' undeclared (first use in this function) 19625 | SAVE_VECTOR_REGISTERS(err = _svr_ret;); | ^~~~~~~~~~~~~~~~~~~~~ wolfcrypt/src/rsa.c:4754:46: error: macro "SAVE_VECTOR_REGISTERS" passed 1 arguments, but takes just 0 4754 | SAVE_VECTOR_REGISTERS(ret = _svr_ret;); | ^

The reason the always-expect-one-arg works even when some invocations pass no args is that (I've just learned from research) invoking a cpp macro with () for args actually passes a single no-tokens arg implicitly. wild stuff.

douzzer · 2024-11-01T20:45:49Z

src/tls.c

+#define CAN_GET_SIZE()  0
+#define CAN_WRITE()     0
+#define CAN_PARSE()     0


These should be defined to take one dummy arg, as explained in my comment below.

…_VECTOR_REGISTERS`.

douzzer · 2024-11-01T22:07:26Z

retest this please ("FAIL: scripts/external.test")

douzzer · 2024-11-01T22:10:15Z

src/ocsp.c

@@ -1634,7 +1634,8 @@ int wolfSSL_OCSP_REQ_CTX_nbio(WOLFSSL_OCSP_REQ_CTX *ctx)
        case ORIOS_WRITE:
        {
            const unsigned char *req;
-            int reqLen = wolfSSL_BIO_get_mem_data(ctx->reqResp, &req);
+            int reqLen = wolfSSL_BIO_get_mem_data(ctx->reqResp,
+                (unsigned char*)&req);


req should be cast to (void *) here.

douzzer · 2024-11-01T22:10:34Z

src/ocsp.c

@@ -1710,7 +1711,8 @@ int wolfSSL_OCSP_sendreq_nbio(OcspResponse **presp, WOLFSSL_OCSP_REQ_CTX *ctx)
    if (ret != WOLFSSL_SUCCESS)
        return ret;

-    len = wolfSSL_BIO_get_mem_data(ctx->reqResp, &resp);
+    len = wolfSSL_BIO_get_mem_data(ctx->reqResp,
+        (unsigned char*)&resp);


same for resp here.

douzzer · 2024-11-01T22:12:56Z

src/tls.c

@@ -14764,7 +14764,7 @@ static word16 TLSX_GetMinSize_Client(word16* type)
 }
    #define TLSX_GET_MIN_SIZE_CLIENT TLSX_GetMinSize_Client
 #else
-    #define TLSX_GET_MIN_SIZE_CLIENT(...) 0
+    #define TLSX_GET_MIN_SIZE_CLIENT() 0


this should also take one dummy arg.

douzzer · 2024-11-01T22:13:08Z

src/tls.c

@@ -14833,7 +14833,7 @@ static word16 TLSX_GetMinSize_Server(const word16 *type)
 }
    #define TLSX_GET_MIN_SIZE_SERVER TLSX_GetMinSize_Server
 #else
-    #define TLSX_GET_MIN_SIZE_SERVER(...) 0
+    #define TLSX_GET_MIN_SIZE_SERVER() 0


douzzer · 2024-11-01T22:16:05Z

wolfssl/wolfcrypt/logging.h

@@ -178,7 +178,7 @@ WOLFSSL_API void wolfSSL_SetLoggingPrefix(const char* prefix);
    WOLFSSL_API void WOLFSSL_MSG_EX(const char* fmt, ...);
    #define HAVE_WOLFSSL_MSG_EX
 #else
-    #define WOLFSSL_MSG_EX(...) WC_DO_NOTHING
+    #define WOLFSSL_MSG_EX() WC_DO_NOTHING


I suspect this is going to cause problems, given that WOLFSSL_MSG_EX() always has at least 1 arg, and sometimes has more than 1.

can we gate the WOLFSSL_MSG_EX() definition in some way, so that WOLFSSL_MSG_EX(...) is kept on targets that support variadic macros?

… warnings.

dgarske self-assigned this Oct 22, 2024

dgarske changed the title ~~More fixes for building x86 in Visual Studio for non-windows OS. Foll…~~ More fixes for building x86 in Visual Studio for non-windows OS Oct 22, 2024

dgarske force-pushed the x86_notwindows_2 branch from 194e26b to 08502ce Compare October 24, 2024 18:10

dgarske force-pushed the x86_notwindows_2 branch from 7f0d820 to 84b5d66 Compare October 29, 2024 18:50

dgarske assigned wolfSSL-Bot and unassigned dgarske Oct 29, 2024

dgarske requested a review from douzzer October 29, 2024 23:12

douzzer requested changes Nov 1, 2024

View reviewed changes

Improvement for SAVE_VECTOR_REGISTERS

99daac3

dgarske requested a review from douzzer November 1, 2024 21:02

Fixes for consistency in ASSERT_SAVED_VECTOR_REGISTERS and `RESTORE…

671f931

…_VECTOR_REGISTERS`.

dgarske assigned douzzer Nov 1, 2024

douzzer requested changes Nov 1, 2024

View reviewed changes

douzzer assigned dgarske Nov 1, 2024

Peer review feedback: Improve workaround for variadic macros and cast…

aad0f6e

… warnings.

dgarske requested a review from douzzer November 4, 2024 19:15

dgarske removed their assignment Nov 4, 2024

douzzer approved these changes Nov 5, 2024

View reviewed changes

douzzer merged commit 8ecf064 into wolfSSL:master Nov 5, 2024
143 checks passed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

More fixes for building x86 in Visual Studio for non-windows OS #8098

More fixes for building x86 in Visual Studio for non-windows OS #8098

dgarske commented Oct 22, 2024 •

edited

Loading

dgarske commented Oct 23, 2024

dgarske commented Oct 23, 2024 •

edited

Loading

douzzer Nov 1, 2024

douzzer Nov 1, 2024

douzzer commented Nov 1, 2024

douzzer Nov 1, 2024

douzzer Nov 1, 2024

douzzer Nov 1, 2024

douzzer Nov 1, 2024

douzzer Nov 1, 2024

		#ifdef __WATCOMC__
		#define SAVE_VECTOR_REGISTERS() WC_DO_NOTHING

More fixes for building x86 in Visual Studio for non-windows OS #8098

More fixes for building x86 in Visual Studio for non-windows OS #8098

Conversation

dgarske commented Oct 22, 2024 • edited Loading

Description

Testing

Checklist

dgarske commented Oct 23, 2024

dgarske commented Oct 23, 2024 • edited Loading

douzzer Nov 1, 2024

Choose a reason for hiding this comment

douzzer Nov 1, 2024

Choose a reason for hiding this comment

douzzer commented Nov 1, 2024

douzzer Nov 1, 2024

Choose a reason for hiding this comment

douzzer Nov 1, 2024

Choose a reason for hiding this comment

douzzer Nov 1, 2024

Choose a reason for hiding this comment

douzzer Nov 1, 2024

Choose a reason for hiding this comment

douzzer Nov 1, 2024

Choose a reason for hiding this comment

dgarske commented Oct 22, 2024 •

edited

Loading

dgarske commented Oct 23, 2024 •

edited

Loading