[lldb-dev] lldb typedefs

[Greg Clayton]

> On Jun 26, 2014, at 7:50 PM, Zachary Turner <zturner at google.com> wrote:
> 
> Is there a clear explanation of the intention of each of lldb's typedefs?
> 
> I've found a number of issues on Windows related to incorrect type usage, but the purpose of the types is ambiguous enough that I'm not yet sure how to fix these issues correctly.  Some of the issues I've run into are:
> 
> 1) We use off_t in some places.  I think this is almost always an error, as off_t's purpose is to deal with files and only in a limited number of cases do we actually deal with files.

::off_t should only be used for cases where we are dealing with native files. Anything else should use a value that is large enough to represent an offset for _any_ target we debug. 

> 2) I'm not sure what lldb::offset_t is supposed to represent.  Is it an offset in a process's address space?  Then why not just use size_t in that case?

I believe size_t can be 32 bit on 32 bit systems. If we are talking about an offset, it needs to be large enough to represent an offset for a 64 bit process being debugged in a 32 bit built lldb.

> 3) Why is lldb::addr_t always 64-bit?  I assume it's because a target process can be either 32 or 64-bit, is this just for convenience so that we only need a single type to represent a pointer in the target process?

Yes. LLDB can debug any process even when we build lldb in 32 bit mode. The addresses must be able to represent _any_ address from any target so lldb::addr_t must be big enough to represent the largest address for any target we support which is currently 64 bit.

> 4) Windows has separate notions of pids and process handles, and similarly for threads.  I'd like to abstract this into separate typedefs, if there's no objections (on non-Windows platforms, pid_t would just be the same type as process_handle_t), and update various methods to take handles instead of pids.

There are some defines that aren't clear. lldb::pid_t is one of them. Right now lldb::pid_t and lldb::tid_t are supposed to be able to represent any pid or tid for any target we support. This is why they are 64 bit.

As far as "pid_t" goes, do not change this. We used the longer "lldb::thread_t" to represent a native host thread. These specific defines are all defined in lldb-types.h:

#ifdef _WIN32

#include <process.h>

namespace lldb
{
    typedef void*               mutex_t;
    typedef void*               condition_t;
    typedef void*               rwlock_t;
    typedef uintptr_t           thread_t;                   // Host thread type
    typedef uint32_t            thread_key_t;
    typedef void *              thread_arg_t;               // Host thread argument type
    typedef unsigned            thread_result_t;            // Host thread result type
    typedef thread_result_t     (*thread_func_t)(void *);   // Host thread function type
}

#else

#include <pthread.h>

namespace lldb
{
    //----------------------------------------------------------------------
    // MacOSX Types
    //----------------------------------------------------------------------
    typedef ::pthread_mutex_t   mutex_t;
    typedef pthread_cond_t      condition_t;
    typedef pthread_rwlock_t    rwlock_t;
    typedef pthread_t           thread_t;                   // Host thread type
    typedef pthread_key_t       thread_key_t;
    typedef void *              thread_arg_t;               // Host thread argument type
    typedef void *              thread_result_t;            // Host thread result type
    typedef void *              (*thread_func_t)(void *);   // Host thread function type
} // namespace lldb

#endif


These are the ones that change from system to system. 

The ones below are designed to be used for all platforms no matter what they are and these defines must work for all targets:


namespace lldb 
{
    typedef uint64_t    addr_t;
    typedef uint64_t    user_id_t;
    typedef uint64_t    pid_t;
    typedef uint64_t    tid_t;
    typedef uint64_t    offset_t;
    typedef int32_t     break_id_t;
    typedef int32_t     watch_id_t;
    typedef void *      clang_type_t;
    typedef uint64_t    queue_id_t;
}


So feel free to add a "typedef process_handle_t process_t;" for windows and add a "typedef ::pid_t process_t;" for the #else clause. We could make this clearer by adding an extra "host" namespace for all of these:


#ifdef _WIN32

#include <process.h>

namespace lldb
{
    namespace host 
    {
        typedef void*               mutex_t;
        typedef void*               condition_t;
        typedef void*               rwlock_t;
        typedef uintptr_t           thread_t;                   // Host thread type
        typedef uint32_t            thread_key_t;
        typedef void *              thread_arg_t;               // Host thread argument type
        typedef unsigned            thread_result_t;            // Host thread result type
        typedef thread_result_t     (*thread_func_t)(void *);   // Host thread function type
    }
}

#else

#include <pthread.h>

namespace lldb
{
    namespace host
    {
        //----------------------------------------------------------------------
        // MacOSX Types
        //----------------------------------------------------------------------
        typedef ::pthread_mutex_t   mutex_t;
        typedef pthread_cond_t      condition_t;
        typedef pthread_rwlock_t    rwlock_t;
        typedef pthread_t           thread_t;                   // Host thread type
        typedef pthread_key_t       thread_key_t;
        typedef void *              thread_arg_t;               // Host thread argument type
        typedef void *              thread_result_t;            // Host thread result type
        typedef void *              (*thread_func_t)(void *);   // Host thread function type
    }
} // namespace lldb

#endif


Then we could overload the lldb::host::pid_t (::pid_t or ::process_handle_t for windows) from lldb::pid_t (always uint64_t or largest unit required to hold a process ID for any target).


So to sum up:

for case #1 above, if there are places people are using ::off_t for something that might be too small for any target, switch it over to use lldb::offset_t.
for case #2 leave lldb::offset_t alone, size_t can be 32 bits in 32 bit builds, and lldb::offset_t represents an offset that must be valid for all targets so 64 bit is required
for case #3 it should be obvious: lldb::addr_t must be able to handle any address for any target and must be big enough for the largest address we support for any target even in a 32 bit build
for case #4 feel free to add a "lldb::process_t" typedef and set it accordingly. Feel free to add the "host" namespace if this makes things more clear. If we add the "host" namespace, we can then add definitions for lldb::host::pid_t and lldb::host::tid_t which can match the current systems native notion of what those are.


Greg