search

GerHobbelt / pthread-win32

clone of pthread-win32 (a.k.a. pthreads4w) + local tweaks (including MSVC2008 - MSVC2022 project files)
Other
343 stars 165 forks source link

pthread_t need to be a scalar to satisfy gcc #2

Open martell opened 9 years ago

martell commented 9 years ago

Hi @GerHobbelt,

Would you be able to help me with something.

I want to change pthreads.h pthread_t from a struct to a uintptr_t As gcc assumes a scalar value for c++11 threading

I don't know how to propagate this change throughout pthread without breaking it :( and I really need todo this

this is the error gcc will produce

libstdc++-v3/include/thread:88:30: error: no match for ‘operator<’ (operand types are ‘std::thread::native_handle_type {aka pte_handle_t}’ and ‘std::thread::native_handle_type {aka pte_handle_t}’) { return __x._M_thread < __y._M_thread; }
martell commented 9 years ago 
diff --git a/pthread.h b/pthread.h
index 1d86403..eec5148 100644
--- a/pthread.h
+++ b/pthread.h
@@ -391,12 +391,15 @@ enum
      * that available with a simple pointer. It should scale for either
      * IA-32 or IA-64.
      */
+/*
     typedef struct
       {
-        void * p;                   /* Pointer to actual object */
-        unsigned int x;             /* Extra information - reuse count etc */
+        void * p;                   
+        unsigned int x;             
       } pte_handle_t;
+*/

+    typedef uintptr_t pte_handle_t;
     typedef pte_handle_t pthread_t;
     typedef struct pthread_attr_t_ * pthread_attr_t;
     typedef struct pthread_once_t_ pthread_once_t;

This is what I need to change it to

GerHobbelt commented 9 years ago

Hi Martell,

I'm sorry but I'm right now completely swamped in a release run where I'm sitting in the critical paths (plural :-( ), so all moments of, ah, sanity are spent on that.

I haven't touched C and pthreads in a while (~ 2-3 years by now), so I'm running this off my geriatric memory.

Anything I write after this line is: 1) flow of consciousness and recall kicking in while I write, so the statements here are soft, rather than hard guarantees. 2) check everything I say (as I always advise people to do anyway); I may have mixed up one or two issues in my brains; generally I have the habit of writing long(ish) comments where I touch complex stuff and I've run into a spot of trouble myself.

With those caveats said, off we go:

WinPthreads is 'special' in that it has to be cross-platform compatible source code (32 and 64-bit support for both int=32-bit and int=64-bit compiler platforms); I know that one (hairy) way to make sure the type is big enough for any platform to work on is to use a union and then let the platform specific code, which is the only part which really cares about the exact 'meaning' of each bit in there, use the element in the union that matches the native OS handles (HANDLE=void* while Unix boxes generally produce int sized handles)

Detail: Windows native APIs, IIRC, produce 64-bit HANDLEs, at least on Win64, which have to fit in the pthread_t type to be carried around undamaged, you cannot strip them to 'int' (32-bit) and expect to live happily ever after, unless you introduce a whole new layer of indirection so that pthreads indexes into a list (array/hashtable) which then stores those big HANDLEs; currently WinPthread doesn't do that but uses a union instead, which is the max size of 'void *' and 'unsigned int'. I mention the extra layer of indirection only as a thought, but realize that you are then talking about a major overhaul and added complexity, so the better question to ask is:

when you are talking about gcc requiring an uintptr_t, that sounds to me like the pthread_t union then just needs an extra field of uintptr_t type so that GCC-specific code (and I bet you'll get some nasty surprises too when it comes to gcc on Win64/Win32 vs. linux/osx vs embedded platforms) can use that union-member.

What has me confused during writing and thinking about it right now is that I see a struct in your diff, rather than a union...

I do seem to recall some old trouble with gcc yakking about pointer aliasing trouble thanks to the use of that union typedef and I remember that episode as I recall some gcc4 early versions screwing up the generated code for the union-based code; there's also that comment which says 'extra information' which looks to me, today, like we might be indeed talking about a struct because we might be keeping counters in there as a kind of 'smart pointer' emulation (C++ smart pointers are pointer+counter).

So getting out of the deep end and wondering about the higher level problem:

ifdef GCC4_CX11_SOMETHING

typedef uintptr_t pthread_t;

else

// legacy code: typedef struct bla bla pthread_t;

endif

style hacks to make it happen on Cx11?

On that same subject: what does it mean exactly: 'gcc requires a scalar type: uintptr_t'? When we check the sizeof uintptr_t, is it large enough to handle a pthread_t when we hard cast the one into the other? Do we need to discard the .x 'counter' element in that struct and does the cast work then?

That's my starting point: my thinking process when I start to attack the problem you describe. I hope it helps you a little. Note that, for me, there are a few unclarities at the very start:

(I bet there is as C has always strives to be backwards compatible, so it may not be nice, it may even be very compiler specific and/or platform specific, but my first bet goes to the side which says: you can tell gcc it's otherwise... use a #pragma xyz-something or whatever and gcc will work, while nobody else will.)

And then of course the question nobody would like to ask: why do we use the WinPthreads library if this is so gcc/Cx11 specific anyway? Why don't we use the native threads that may be in there (remember, I haven't read the Cx11 spec yet, so I have no clue. Blog articles on the net are mere hints to me which can fall either way; what's the reference manual/document itself say? --> Where can I go and twist its arm?)

My 1 cent. I really hope it helps you move forward.

Met vriendelijke groeten / Best regards,

Ger Hobbelt

web: http://www.hobbelt.com/ http://www.hebbut.net/ mail: ger@hobbelt.com

mobile: +31-6-11 120 978

On Mon, Oct 6, 2014 at 6:49 PM, Martell Malone notifications@github.com wrote:

Hi @GerHobbelt https://github.com/GerHobbelt,

Would you be able to help me with something.

I want to change pthreads.h pthread_t from a struct to a uintptr_t As gcc assumes a scalar value for c++11 threading

I don't know how to propagate this change throughout pthread without breaking it :( and I really need todo this

— Reply to this email directly or view it on GitHub https://github.com/GerHobbelt/pthread-win32/issues/2.

martell commented 9 years ago

Wow that is a really long response. Thanks for taking the time.

The struct is actually already in pthread.h my diff just removes that and replaces it with uintptr_t After some further investigation I can see that the x element is just used for reusing memory of old threads when creating new one. pthreads-win32 seems to hang onto finished threads in a stack and then uses that memoey again. This seems to me optimisation for the sake of optimisation.

gcc or rather libstdc++ requires a variable type so that it can do comparators. Technically this is a bug in libstdc++ as the spec does not dis allow struct. gcc devs have acknowledged this but its a minor bug as pthreads-win32 is the only implementation not to use a scalar value

The reason I went with uintptr_t is because that is what the mingw-w64 libpthreads does.

I am not actually working from pthreads-win32 I'm working of a fork called pthreads-embedded This is based on pthreads-win32 but restructured to support multiple embedded platforms like the psp or the ps3. It does however have this common issue when trying to deal with gcc/libstdc++ for c++ 11.