[lldb-dev] lldb typedefs

Zachary Turner zturner at google.com
Fri Jun 27 10:02:11 PDT 2014


Thanks for the clarification.  I had a similar idea related to your host
namespace.  What about taking it one step further and making all the host /
target specific types live in either a lldb::host or lldb::target?  So for
example this:

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;
}

would become this:

namespace lldb
{
    namespace target
    {
        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;
}

I'm guessing there's already a number of inconsistencies related to using
these target types to reference things on the host, and vice versa.  Making
the distinction more clear in both directions would probably be helpful.

thoughts?


On Fri, Jun 27, 2014 at 9:47 AM, Greg Clayton <gclayton at apple.com> wrote:

>
> > 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
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/lldb-dev/attachments/20140627/b4b46ba0/attachment.html>


More information about the lldb-dev mailing list