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

[build] Issue with 32-bit build (i686) #185

Open
kingtaurus opened this issue Jan 9, 2015 · 4 comments
Open

[build] Issue with 32-bit build (i686) #185

kingtaurus opened this issue Jan 9, 2015 · 4 comments

Comments

@kingtaurus
Copy link
Contributor

I'm trying to build vogl on an i686 Ubuntu 14.04 machine

gking@ubuntu:~/Programming/vogl/clang$ uname -a
Linux ubuntu 3.13.0-41-generic #70-Ubuntu SMP Tue Nov 25 14:41:08 UTC 2014 i686 i686 i686 GNU/Linux
gking@ubuntu:/etc$ cat os-release 
NAME="Ubuntu"
VERSION="14.04.1 LTS, Trusty Tahr"
ID=ubuntu
ID_LIKE=debian
PRETTY_NAME="Ubuntu 14.04.1 LTS"
VERSION_ID="14.04"
HOME_URL="http://www.ubuntu.com/"
SUPPORT_URL="http://help.ubuntu.com/"
BUG_REPORT_URL="http://bugs.launchpad.net/ubuntu/"

So, pulling from vogl/master (making sure I'm up to date):

gking@ubuntu:~/Programming/vogl/clang$ date && git pull upstream master
Wed Jan  7 21:05:25 PST 2015
From https://github.com/ValveSoftware/vogl
 * branch            master     -> FETCH_HEAD
Already up-to-date.

Build Error:

/home/gking/Programming/vogl/src/voglcore/vogl_port_posix.cpp:111:12: error: reinterpret_cast from 'pthread_t' (aka 'unsigned long') to
      'uintptr_t' (aka 'unsigned int') is not allowed [Semantic Issue]
    return reinterpret_cast<uintptr_t>(pthread_self());
           ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~

This issue arises because (/usr/include/stdint.h):

/* Types for `void *' pointers.  */
#if __WORDSIZE == 64
# ifndef __intptr_t_defined
typedef long int                intptr_t;
#  define __intptr_t_defined
# endif
typedef unsigned long int       uintptr_t;
#else
# ifndef __intptr_t_defined
typedef int                     intptr_t;
#  define __intptr_t_defined
# endif
typedef unsigned int            uintptr_t;
#endif

Now pthread_t (/usr/include/i386-linux-gnu/bits/pthreadtypes.h):

/* Thread identifiers.  The structure of the attribute type is not
   exposed on purpose.  */
typedef unsigned long int pthread_t;

I believe that unsigned long int in this case is the same type as unsigned int (i386) - thus the reinterpret_cast<> is invalid. I can write up a simple fix (just surprised that no one else has stumbled on this) - however I feel that I should report this first.

Edit: improved formatting and fixed a few small mistakes

@computerquip
Copy link

Issue #197 also includes solutions. Apologies for duplicating.

@kingtaurus
Copy link
Contributor Author

So, pthread_self() returns pthread_t (according to pthread_self specification). So I'm confused as to why the reinterpret_cast<> is required.

@computerquip
Copy link

pthread_t is an abstract type. It can be, quite literally, anything.
EDIT: Well, for what it's worth, I'd say it's healthier for the long term to avoid the reinterpret cast in this case. Some minor design changes can fix the issue but I'm not really sure what would be a desirable way to go about it without some insight.

@kingtaurus
Copy link
Contributor Author

Well the options are to avoid the cast altogether:
(1) both 32-bit and 64-bit linux define a pthread_t through the following typedef:

typedef unsigned long int pthread_t;

See /usr/include/x86_64-linux-gnu/bits/pthreadtypes.h and /usr/include/i386-linux-gnu/bits/pthreadtypes.h; which should convert directly to uint64_t without issue.

This would require OS X specific code to handle the pthread_t (and its associated pthread_t definition). _pthread_types.h , pthread.h

typedef __darwin_pthread_t      pthread_t;

and

typedef struct _opaque_pthread_t
            *__darwin_pthread_t;    /* [???] Used for pthreads */

finally,

struct _opaque_pthread_t {
    long __sig;
    struct __darwin_pthread_handler_rec  *__cleanup_stack;
    char __opaque[__PTHREAD_SIZE__];
};

where

#if defined(__LP64__)
#define __PTHREAD_SIZE__        8176
//...
#else // !__LP64__
#define __PTHREAD_SIZE__        4088
//...
#endif

and

struct __darwin_pthread_handler_rec {
    void (*__routine)(void *);  // Routine to call
    void *__arg;            // Argument to pass
    struct __darwin_pthread_handler_rec *__next;
};

Although this information is really recent, it appears that the storage class is a signed long (so, unpacking this from the pthread_t would convert directly to uint64_t easily). Further the reinterpret_cast<> just converts the address of _opaque_pthread_t that is allocated on the heap and not the actual pthread_t id (which most likely is a bug).

(2) Move to C++11 (or boost::thread)and use std::thread, std::mutex, and what not. This would probably require a re-write within the core libraries (and a lot more testing).

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

No branches or pull requests

2 participants