Skip to content

Fix SIZEOF_LONG and SIZEOF_LONG_LONG logic#3283

Closed
RipleyTom wants to merge 1 commit intowolfSSL:masterfrom
RipleyTom:bet_preproc_logic
Closed

Fix SIZEOF_LONG and SIZEOF_LONG_LONG logic#3283
RipleyTom wants to merge 1 commit intowolfSSL:masterfrom
RipleyTom:bet_preproc_logic

Conversation

@RipleyTom
Copy link

@RipleyTom RipleyTom commented Sep 9, 2020

Somehow we're hitting a case where one is defined but not the other, improved logic so that both are guaranteed to be defined.

To be specific we're hitting an #error below in the same file: #error "bad math long / long long settings" because SIZEOF_LONG is defined as 4 and SIZEOF_LONG_LONG is undefined so it's hitting none of the cases listed(and on msvc it should hit the SIZEOF_LONG_LONG==8 cases).

@wolfSSL-Bot
Copy link

Can one of the admins verify this patch?

@dgarske dgarske self-assigned this Sep 9, 2020
@RipleyTom
Copy link
Author

RipleyTom commented Sep 9, 2020

I have some more informations, finally setup VM to actually test myself without relying on our CI and our issue specifically happens because curl/libcurl also defines a SIZEOF_LONG !

@RipleyTom RipleyTom closed this Sep 9, 2020
@RipleyTom RipleyTom reopened this Sep 9, 2020
@RipleyTom
Copy link
Author

RipleyTom commented Sep 9, 2020

Sorry i was getting confused because somehow libcurl decided to name some of its files the same as some wolfssl files.
The issue is actually correct, the error happens because wolfssl types.h ends up included in curl build, which already defines SIZEOF_LONG but not SIZEOF_LONG_LONG.

Not sure if the solution I propose is ok though, maybe naming it something that wouldn't lead to name collisions would be better, esp in a header that gets included in other projects for imports but that'd be quite a big change.

@RipleyTom
Copy link
Author

RipleyTom commented Sep 10, 2020

From what I can tell the define comes from here in libcurl:
https://github.com/curl/curl/blob/master/lib/config-win32.h#L406

It gets defined before the include of wolfssl headers, reaches types.h header, doesn't define SIZEOF_LONG_LONG because SIZEOF_LONG is already defined and can't find a proper setting and crashes on #error "bad math long / long long settings".

@dgarske
Copy link
Contributor

dgarske commented Sep 10, 2020

Hi @RipleyTom ,

My understanding is that we only require one or the other. So either SIZEOF_LONG_LONG or SIZEOF_LONG needs to be defined. The section of code in types.h makes sure one or the other is set for Windows.

It looks like Curl is always using 4 for SIZEOF_LONG, so it must be SIZEOF_LONG_LONG that is conflicting. Is the error happening when building your source code? Perhaps you should #undef SIZEOF_LONG_LONG before including the wolfSSL or Curl headers?

Your patch is good, but I would still want to decouple the SIZEOF_LONG_LONG and SIZEOF_LONG so the user could still override it.

We integrate very nice with Curl and the maintainer @bagder is part of wolfSSL, so I am surprised you are experiencing this.

Thanks,
David Garske, wolfSSL

@RipleyTom
Copy link
Author

RipleyTom commented Sep 11, 2020

Below is the breakdown of the logic that leads to the error:

    enum {
// SIZEOF_LONG is defined so it's not hit
    #if !defined(USE_FAST_MATH) && !defined(SIZEOF_LONG) && !defined(SIZEOF_LONG_LONG)
        CTC_SETTINGS = 0x0
// SIZEOF_LONG is 4 so it's not hit
    #elif !defined(USE_FAST_MATH) && defined(SIZEOF_LONG) && (SIZEOF_LONG == 8)
        CTC_SETTINGS = 0x1
// SIZEOF_LONG_LONG is not defined so it's not hit
    #elif !defined(USE_FAST_MATH) && defined(SIZEOF_LONG_LONG) && (SIZEOF_LONG_LONG == 8)
        CTC_SETTINGS = 0x2
// SIZEOF_LONG_LONG is not defined so it's not hit
    #elif !defined(USE_FAST_MATH) && defined(SIZEOF_LONG_LONG) && (SIZEOF_LONG_LONG == 4)
        CTC_SETTINGS = 0x4
// SIZEOF_LONG is defined so it's not hit
    #elif defined(USE_FAST_MATH) && !defined(SIZEOF_LONG) && !defined(SIZEOF_LONG_LONG)
        CTC_SETTINGS = 0x8
// SIZEOF_LONG is 4 so it's not hit
    #elif defined(USE_FAST_MATH) && defined(SIZEOF_LONG) && (SIZEOF_LONG == 8)
        CTC_SETTINGS = 0x10
// SIZEOF_LONG_LONG is not defined so it's not hit
    #elif defined(USE_FAST_MATH) && defined(SIZEOF_LONG_LONG) && (SIZEOF_LONG_LONG == 8)
        CTC_SETTINGS = 0x20
// SIZEOF_LONG_LONG is not defined so it's not hit
    #elif defined(USE_FAST_MATH) && defined(SIZEOF_LONG_LONG) && (SIZEOF_LONG_LONG == 4)
        CTC_SETTINGS = 0x40
    #else
// Error here
        #error "bad math long / long long settings"
    #endif
    };

Curl has some special code in the configure part of it that undef both before including WolfSSL:

/* These aren't needed for detection and confuse WolfSSL.
   They are set up properly later if it is detected.  */
#undef SIZEOF_LONG
#undef SIZEOF_LONG_LONG
#include <wolfssl/ssl.h>

But that code is not part of the vcxproj we use to build curl on windows.
I personally fixed the issue by forcing SIZEOF_LONG=4 and SIZEOF_LONG_LONG=8 in the curl vcxproj(as we are building only for windows x64 with that build system)j but I still think the preprocessor logic in types.h is no ideal that's why I left the PR open.

As for my patches it defines them only if there are undefined before the inclusion of the header, which wouldn't happen if user declared them already.

edit:
I think #2600 was the same issue and the patch proposed indeed fixed it by forcing both to get defined, related PR only defines both if none are defined which doesn't fix the issue.

@dgarske
Copy link
Contributor

dgarske commented Sep 22, 2020

@RipleyTom , I like this change. In order to accept a PR we need a signed contributors agreement. Would you mind emailing support@wolfssl.com to request this? Thanks, David Garske

@dgarske
Copy link
Contributor

dgarske commented Nov 10, 2020

@RipleyTom , checking in to see about the contributor agreement status? Would you mind emailing support@wolfssl.com to request this? Thanks, David Garske

@hunter-kk
Copy link

hunter-kk commented May 6, 2022

Below is the breakdown of the logic that leads to the error:

    enum {
// SIZEOF_LONG is defined so it's not hit
    #if !defined(USE_FAST_MATH) && !defined(SIZEOF_LONG) && !defined(SIZEOF_LONG_LONG)
        CTC_SETTINGS = 0x0
// SIZEOF_LONG is 4 so it's not hit
    #elif !defined(USE_FAST_MATH) && defined(SIZEOF_LONG) && (SIZEOF_LONG == 8)
        CTC_SETTINGS = 0x1
// SIZEOF_LONG_LONG is not defined so it's not hit
    #elif !defined(USE_FAST_MATH) && defined(SIZEOF_LONG_LONG) && (SIZEOF_LONG_LONG == 8)
        CTC_SETTINGS = 0x2
// SIZEOF_LONG_LONG is not defined so it's not hit
    #elif !defined(USE_FAST_MATH) && defined(SIZEOF_LONG_LONG) && (SIZEOF_LONG_LONG == 4)
        CTC_SETTINGS = 0x4
// SIZEOF_LONG is defined so it's not hit
    #elif defined(USE_FAST_MATH) && !defined(SIZEOF_LONG) && !defined(SIZEOF_LONG_LONG)
        CTC_SETTINGS = 0x8
// SIZEOF_LONG is 4 so it's not hit
    #elif defined(USE_FAST_MATH) && defined(SIZEOF_LONG) && (SIZEOF_LONG == 8)
        CTC_SETTINGS = 0x10
// SIZEOF_LONG_LONG is not defined so it's not hit
    #elif defined(USE_FAST_MATH) && defined(SIZEOF_LONG_LONG) && (SIZEOF_LONG_LONG == 8)
        CTC_SETTINGS = 0x20
// SIZEOF_LONG_LONG is not defined so it's not hit
    #elif defined(USE_FAST_MATH) && defined(SIZEOF_LONG_LONG) && (SIZEOF_LONG_LONG == 4)
        CTC_SETTINGS = 0x40
    #else
// Error here
        #error "bad math long / long long settings"
    #endif
    };

Curl has some special code in the configure part of it that undef both before including WolfSSL:

/* These aren't needed for detection and confuse WolfSSL.
   They are set up properly later if it is detected.  */
#undef SIZEOF_LONG
#undef SIZEOF_LONG_LONG
#include <wolfssl/ssl.h>

But that code is not part of the vcxproj we use to build curl on windows. I personally fixed the issue by forcing SIZEOF_LONG=4 and SIZEOF_LONG_LONG=8 in the curl vcxproj(as we are building only for windows x64 with that build system)j but I still think the preprocessor logic in types.h is no ideal that's why I left the PR open.

As for my patches it defines them only if there are undefined before the inclusion of the header, which wouldn't happen if user declared them already.

edit: I think #2600 was the same issue and the patch proposed indeed fixed it by forcing both to get defined, related PR only defines both if none are defined which doesn't fix the issue.

Hi, @dgarske @RipleyTom @wolfSSL-Bot @bagder
I had the same problem when compiling with NDK. In the 64-bit compilation environment, curl SIZEOF_LONG=8, SIZEOF_LONG_LONG is not defined, I can compile it. When compiling in 32-bit environment, curl SIZEOF_LONG=4, SIZEOF_LONG_LONG is not defined, so it goes to "# error "bad math long / long long settings".

In file included from /Users/kk/work/3rdSrc/curl/lib/http_aws_sigv4.c:33:
In file included from /Users/kk/work/3rdSrc/curl/lib/curl_sha256.h:35:
In file included from /Users/kk/work/3rdSrc/curl/3rdparty/libwolfssl/android/armeabi-v7a/usr/include/wolfssl/openssl/sha.h:29:
/Users/kk/work/3rdSrc/curl/3rdparty/libwolfssl/android/armeabi-v7a/usr/include/wolfssl/wolfcrypt/types.h:1069:10: error: "bad math long / long long settings"
        #error "bad math long / long long settings"
         ^
/Users/kk/work/3rdSrc/curl/3rdparty/libwolfssl/android/armeabi-v7a/usr/include/wolfssl/wolfcrypt/types.h:1071:5: error: use of empty enum
    };
    ^

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