Compilation for platforms without pthreads

[projectgoav]

I am currently upgrading our LibreSSL version from 2.6. It now seems that v3.0+ requires some pthread related functions - pthread_once & pthread_mutex_lock|unlock.

We run FreeRTOS which does not support pthreads. Is there any way for us to define / configure SSL to not require these functions or should be we be providing a FreeRTOS implementation going forward?

[botovq]

The libraries require the API, but there already is compat glue for pthreads on Windows. If you can fill this in appropriately for FreeRTOS, it should work fine.

[projectgoav]

Thanks!

Annoyingly the compiler we use defines a number of the required pthread structures in the sys/types header typedef'd as integers. This isn't really compatible with providing a FreeRTOS implementation under the pthread APIs.

Would the project be willing to accept a patch that abstracts pthread_once and pthread_mutex to something like ssl_once and ssl_mutex with a pthreads & Windows implementation? This may make it a little easier for other platforms to provide compatibility glue.

[botovq]

Ideally, this would be transparent from our side, so that our code can keep using the standard names. In #961 I have started moving our compat glue into its own libressl_ namespace, but I haven't done this for pthreads.h yet.

I think the same technique should work here: above each compat type or symbol declaration foo there should be a

#define foo libressl_foo

in the compat pthreads.h. When needed, our code pulls in the compat header and will then use things from that namespace, ignoring whatever is in sys/types.h.

[projectgoav]

That all makes sense to me. It sounds similar to what I had thought of but in a much cleaner implementation.

Just to confirm I've understood you correctly, in the case of pthread_once you'd have something like:

// pthread.h
#ifdef WIN32
#define INIT_ONCE libressl_once
// ... 
#else
#define pthread_once libressl_once
// ... 
#endif

Then in calling code we'd use libressl_once?

[botovq]

The point of doing this is that the code in our libcrypto/libssl/libtls can remain unmodified and keep using pthread_once. Since it has pthread.h in scope, the #defines in the compat header will result in the preprocessor replacing any occurrence of pthread_once with libressl_pthread_once, so this won't clash with what was previously done in sys/types.h.

So in the compat header you'd do

// pthread.h
#ifdef WIN32
...
/*
 * Once definitions.
 */
#define pthread_once libressl_pthread_once
struct pthread_once {
	INIT_ONCE once;
};
#define pthread_once_t libressl_pthread_once_t
typedef struct pthread_once pthread_once_t;

If I understand correctly, INIT_ONCE comes from windows, so it should not be renamed.

The replacement code should keep using the standard names. The preprocessor already takes care of the renaming.

[projectgoav]

Ok, I'm with you now!

However, in our case I don't think this will work. We're using the RTEMs compiler which provides a sys/types.h as follows:

...
typedef struct {
  int   is_initialized;  /* is this structure initialized? */
  int   init_executed;   /* has the initialization routine been run? */
} pthread_once_t;       /* dynamic package initialization */
...

Taking your suggestion above, I've added the code above in include/compat/pthread.h but upon compiling I get

In file included from cryptlib.c:117:0:

../include/compat/pthread.h:17:24: error: conflicting types for 'libressl_pthread_once_t'
 #define pthread_once_t libressl_pthread_once_t
                        ^
../include/compat/pthread.h:17:24: note: previous declaration of 'libressl_pthread_once_t' was here
 #define pthread_once_t libressl_pthread_once_t
                        ^
../include/compat/pthread.h:18:29: note: in expansion of macro 'pthread_once_t'
 typedef struct pthread_once pthread_once_t;

The same also happens for pthread_t and pthead_mutex_t which are defined as follows in sys/types.h

typedef unsigned int pthread_t;          /* identify a thread */
typedef unsigned int pthread_mutex_t;    /* identify a mutex */

[botovq]

I will need to play with it a bit. Some toy examples seem to work here with clang.

[projectgoav]

Not a problem, let me know if there is anything I can do to help or test.

As an aside, I totally understand if this is out with the realm and goals of the LibreSSL project. I know it's great for things to be portable across platforms but there comes a point where certian (niche) setups can't be maintaned!

[botovq]

I think the problem you run into simply is that cryptlib.c has #include <pthread.h> at the very top. Then it stomps on the defines in sys/types.h once that's included from some other header below.

Could you try adding #include <sys/types.h> at the top of your compat pthread.h?

Regarding support, if it is only about having some compat shims, there should be no issue to help you with that and carrying it in our portable layer. That's what it is for. The line will be crossed when we need to do modify our code in the openbsd repo in an intrusive way. If/when we get it working, we should think about how we ensure we don't break you. If we can set up some CI on github, that would be ideal. Or maybe you have some test environment where you can regularly pull portable HEAD and notify us when we break something.

[projectgoav]

Adding the sys/types.h at the top has solved the compilation issue I have mentioned above. I've managed to build and run a v3 libreSSL. Thanks very much for your help on this.

Quick question about pthread_mutex_lock. What would you expect to happen if you call this with an uninitialised pthread_mutex_t?

It seems the Windows compat shim checks for an uninitilaised case, initialises then locks. I had to do the same for my FreeRTOS shim otherwise we'd crash. This feels wrong that there are some places the code where this occurs but it might be my misunderstanding of the POSIX/pthread spec.

[botovq]

It seems the Windows compat shim checks for an uninitilaised case, initialises then locks.

This is the way static mutexes are ininitialized/implemented (which are used in the code):

/*
 * Static mutex initialization values.
 */
#define PTHREAD_MUTEX_INITIALIZER       { .lock = NULL }

Now the spec (reasonably) says that calling pthread_mutex_lock() on an uninitialized mutex is undefined behavior. But initialization to NULL isn't really uninitialized, so I think this is fine.

[botovq]

I should have said: that you got it working is great news. Do the regress tests pass? I would be happy to merge support into this repository, so feel free to open a PR with what you have. It would be great to figure out a way to test it.

Also try to avoid to get this far behind again. There are numerous really bad issues that have been fixed since 2.6.

This may or may not be relevant to you: there is some undefined behavior in the windows pthreads.h shim. That should be fixed with #967.

[projectgoav]

Do the regress tests pass?

I am unable to confirm that at the moment. Honestly, I'm not really sure how to deploy them to our hardware... I only really know how to deploy our entire app! At least doing that allows us to boot, run and pass our own set of nightly tests without issue. I'll investigate how we could deploy and run the provided tests.

feel free to open a PR with what you have

Certainly can do. I'll tidy it up a bit as it's got some other specifics in there for our hardware. I don't currently know how to implement pthread_self() on FreeRTOS so I've stubbed it to 0. It 'works' (under the same conditions as the above point) but I'd probably want a better solution before providing a PR.