Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

mbedtls_platform_gmtime_r windows bug #4015

Closed
miroslav-mastny-regosys opened this issue Jan 12, 2021 · 7 comments
Closed

mbedtls_platform_gmtime_r windows bug #4015

miroslav-mastny-regosys opened this issue Jan 12, 2021 · 7 comments
Labels
bug component-platform Portability layer and build scripts good-first-issue Good for newcomers needs-backports Backports are missing or are pending review and approval.

Comments

@miroslav-mastny-regosys

Description

  • Type: Bug
  • Priority: Major

OS
windows (C++ Builder)

mbed TLS build:
Version: 2.16.9

WIN32 function gmtime_s has parameters and return value wrong order.

Should be:
#if defined(_WIN32) && !defined(EFIX64) && !defined(EFI32)
return( ( gmtime_s( tt, tm_buf ) == 0 ) ? NULL: tm_buf );
#elif !defined(PLATFORM_UTIL_USE_GMTIME)

Currently implemented:
#if defined(_WIN32) && !defined(EFIX64) && !defined(EFI32)
return( ( gmtime_s( tm_buf, tt ) == 0 ) ? tm_buf : NULL );
#elif !defined(PLATFORM_UTIL_USE_GMTIME)

@gabor-mezei-arm
Copy link
Contributor

gabor-mezei-arm commented Jan 13, 2021

It looks like the C++ Builder using the standard C function instead of the windows' one.

Can you help me about which macro is defined by C++ Builder and what are their value?
These are the interested macros:
_POSIX_VERSION
_POSIX_THREAD_SAFE_FUNCTIONS
__STDC__

@gabor-mezei-arm gabor-mezei-arm added bug Community component-platform Portability layer and build scripts needs-backports Backports are missing or are pending review and approval. labels Jan 13, 2021
@gabor-mezei-arm gabor-mezei-arm self-assigned this Jan 13, 2021
@miroslav-mastny-regosys
Copy link
Author

miroslav-mastny-regosys commented Jan 13, 2021

Any POSIX macro is not set.
Macro STDC is set to 1 when building as 32bit C project and ANSI conformance compiler flag is set (it seems is not set by default and there is not direct switch in project setting - only user compiler switch field is useable for that)

C++Builder definition of gmtime_s in file <time.h>:
struct tm * _RTLENTRY _EXPFUNC gmtime_s(const time_t * _RESTRICT clock,
struct tm * _RESTRICT result);

@gabor-mezei-arm gabor-mezei-arm removed their assignment Jan 13, 2021
@gabor-mezei-arm gabor-mezei-arm added the good-first-issue Good for newcomers label Jan 13, 2021
@gabor-mezei-arm
Copy link
Contributor

The gmtime handling is not prepared for standard-C-like gmtime_s function in windows.

@gstrauss
Copy link
Contributor

gstrauss commented Feb 3, 2022

Is this still an issue? The usage of gmtime_s() is centralized to a single use in library/platform_util.c:mbedtls_platform_gmtime_r() and the argument order looks correct to me.

@gabor-mezei-arm
Copy link
Contributor

Yes, the gmtime_s function should be called the same way as gmtime_r if the __STDC__ and _WIN32 macros are defined.

The standard (C11):
struct tm *gmtime_s( const time_t *restrict timer, struct tm *restrict buf );

The MSVC provides:
errno_t gmtime_s( struct tm* tmDest, const __time_t* sourceTime );

@gstrauss
Copy link
Contributor

gstrauss commented Feb 3, 2022

ick. Now I see that the signature is incompatible depending on the "standards" view in the headers.
https://en.cppreference.com/w/c/chrono/gmtime
Since gmtime_s() was added in C11 Annex K, is the definition properly hidden by checking for the following?

#define __STDC_WANT_LIB_EXT1__ 1
#include <time.h>
...
#ifdef __STDC_LIB_EXT1__
/* gmtime_s() C11 prototype view */
#else
/* gmtime_s() MSVC prototype view */
#endif

@daverodgman
Copy link
Contributor

Fixed by #4211

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug component-platform Portability layer and build scripts good-first-issue Good for newcomers needs-backports Backports are missing or are pending review and approval.
Projects
None yet
Development

No branches or pull requests

4 participants