<div dir="ltr">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:<div>
<br></div><div><span style="font-family:arial,sans-serif;font-size:13px">namespace lldb</span><br style="font-family:arial,sans-serif;font-size:13px"><span style="font-family:arial,sans-serif;font-size:13px">{</span><br style="font-family:arial,sans-serif;font-size:13px">
<span style="font-family:arial,sans-serif;font-size:13px"> typedef uint64_t addr_t;</span><br style="font-family:arial,sans-serif;font-size:13px"><span style="font-family:arial,sans-serif;font-size:13px"> typedef uint64_t user_id_t;</span><br style="font-family:arial,sans-serif;font-size:13px">
<span style="font-family:arial,sans-serif;font-size:13px"> typedef uint64_t pid_t;</span><br style="font-family:arial,sans-serif;font-size:13px"><span style="font-family:arial,sans-serif;font-size:13px"> typedef uint64_t tid_t;</span><br style="font-family:arial,sans-serif;font-size:13px">
<span style="font-family:arial,sans-serif;font-size:13px"> typedef uint64_t offset_t;</span><br style="font-family:arial,sans-serif;font-size:13px"><span style="font-family:arial,sans-serif;font-size:13px"> typedef int32_t break_id_t;</span><br style="font-family:arial,sans-serif;font-size:13px">
<span style="font-family:arial,sans-serif;font-size:13px"> typedef int32_t watch_id_t;</span><br style="font-family:arial,sans-serif;font-size:13px"><span style="font-family:arial,sans-serif;font-size:13px"> typedef void * clang_type_t;</span><br style="font-family:arial,sans-serif;font-size:13px">
<span style="font-family:arial,sans-serif;font-size:13px"> typedef uint64_t queue_id_t;</span><br style="font-family:arial,sans-serif;font-size:13px"><span style="font-family:arial,sans-serif;font-size:13px">}</span><br>
</div><div><span style="font-family:arial,sans-serif;font-size:13px"><br></span></div><div><span style="font-family:arial,sans-serif;font-size:13px">would become this:</span></div><div><span style="font-family:arial,sans-serif;font-size:13px"><br>
</span></div><div><span style="font-family:arial,sans-serif;font-size:13px">namespace lldb</span><br style="font-family:arial,sans-serif;font-size:13px"><span style="font-family:arial,sans-serif;font-size:13px">{</span></div>
<div> namespace target</div><div> {</div><span style="font-size:13px;font-family:arial,sans-serif"> typedef uint64_t addr_t;</span><br style="font-size:13px;font-family:arial,sans-serif"><span style="font-size:13px;font-family:arial,sans-serif"> typedef uint64_t user_id_t;</span><br style="font-size:13px;font-family:arial,sans-serif">
<span style="font-size:13px;font-family:arial,sans-serif"> typedef uint64_t pid_t;</span><br style="font-size:13px;font-family:arial,sans-serif"><span style="font-size:13px;font-family:arial,sans-serif"> typedef uint64_t tid_t;</span><br style="font-size:13px;font-family:arial,sans-serif">
<span style="font-size:13px;font-family:arial,sans-serif"> typedef uint64_t offset_t;</span><br style="font-size:13px;font-family:arial,sans-serif"><div> }</div><div><br><span style="font-family:arial,sans-serif;font-size:13px"> typedef int32_t break_id_t;</span><br style="font-family:arial,sans-serif;font-size:13px">
<span style="font-family:arial,sans-serif;font-size:13px"> typedef int32_t watch_id_t;</span><br style="font-family:arial,sans-serif;font-size:13px"><span style="font-family:arial,sans-serif;font-size:13px"> typedef void * clang_type_t;</span><br style="font-family:arial,sans-serif;font-size:13px">
<span style="font-family:arial,sans-serif;font-size:13px"> typedef uint64_t queue_id_t;</span><br style="font-family:arial,sans-serif;font-size:13px"><span style="font-family:arial,sans-serif;font-size:13px">}</span><span style="font-family:arial,sans-serif;font-size:13px"><br>
</span></div><div><span style="font-family:arial,sans-serif;font-size:13px"><br></span></div><div><font face="arial, sans-serif">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.</font></div>
<div><font face="arial, sans-serif"><br></font></div><div><font face="arial, sans-serif">thoughts?</font></div></div><div class="gmail_extra"><br><br><div class="gmail_quote">On Fri, Jun 27, 2014 at 9:47 AM, Greg Clayton <span dir="ltr"><<a href="mailto:gclayton@apple.com" target="_blank">gclayton@apple.com</a>></span> wrote:<br>
<blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div class=""><br>
> On Jun 26, 2014, at 7:50 PM, Zachary Turner <<a href="mailto:zturner@google.com">zturner@google.com</a>> wrote:<br>
><br>
> Is there a clear explanation of the intention of each of lldb's typedefs?<br>
><br>
> 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:<br>
><br>
> 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.<br>
<br>
</div>::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.<br>
<div class=""><br>
> 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?<br>
<br>
</div>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.<br>
<div class=""><br>
> 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?<br>
<br>
</div>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.<br>
<div class=""><br>
> 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.<br>
<br>
</div>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.<br>
<br>
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:<br>
<br>
#ifdef _WIN32<br>
<br>
#include <process.h><br>
<br>
namespace lldb<br>
{<br>
typedef void* mutex_t;<br>
typedef void* condition_t;<br>
typedef void* rwlock_t;<br>
typedef uintptr_t thread_t; // Host thread type<br>
typedef uint32_t thread_key_t;<br>
typedef void * thread_arg_t; // Host thread argument type<br>
typedef unsigned thread_result_t; // Host thread result type<br>
typedef thread_result_t (*thread_func_t)(void *); // Host thread function type<br>
}<br>
<br>
#else<br>
<br>
#include <pthread.h><br>
<br>
namespace lldb<br>
{<br>
//----------------------------------------------------------------------<br>
// MacOSX Types<br>
//----------------------------------------------------------------------<br>
typedef ::pthread_mutex_t mutex_t;<br>
typedef pthread_cond_t condition_t;<br>
typedef pthread_rwlock_t rwlock_t;<br>
typedef pthread_t thread_t; // Host thread type<br>
typedef pthread_key_t thread_key_t;<br>
typedef void * thread_arg_t; // Host thread argument type<br>
typedef void * thread_result_t; // Host thread result type<br>
typedef void * (*thread_func_t)(void *); // Host thread function type<br>
} // namespace lldb<br>
<br>
#endif<br>
<br>
<br>
These are the ones that change from system to system.<br>
<br>
The ones below are designed to be used for all platforms no matter what they are and these defines must work for all targets:<br>
<br>
<br>
namespace lldb<br>
{<br>
typedef uint64_t addr_t;<br>
typedef uint64_t user_id_t;<br>
typedef uint64_t pid_t;<br>
typedef uint64_t tid_t;<br>
typedef uint64_t offset_t;<br>
typedef int32_t break_id_t;<br>
typedef int32_t watch_id_t;<br>
typedef void * clang_type_t;<br>
typedef uint64_t queue_id_t;<br>
}<br>
<br>
<br>
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:<br>
<br>
<br>
#ifdef _WIN32<br>
<br>
#include <process.h><br>
<br>
namespace lldb<br>
{<br>
namespace host<br>
{<br>
typedef void* mutex_t;<br>
typedef void* condition_t;<br>
typedef void* rwlock_t;<br>
typedef uintptr_t thread_t; // Host thread type<br>
typedef uint32_t thread_key_t;<br>
typedef void * thread_arg_t; // Host thread argument type<br>
typedef unsigned thread_result_t; // Host thread result type<br>
typedef thread_result_t (*thread_func_t)(void *); // Host thread function type<br>
}<br>
}<br>
<br>
#else<br>
<br>
#include <pthread.h><br>
<br>
namespace lldb<br>
{<br>
namespace host<br>
{<br>
//----------------------------------------------------------------------<br>
// MacOSX Types<br>
//----------------------------------------------------------------------<br>
typedef ::pthread_mutex_t mutex_t;<br>
typedef pthread_cond_t condition_t;<br>
typedef pthread_rwlock_t rwlock_t;<br>
typedef pthread_t thread_t; // Host thread type<br>
typedef pthread_key_t thread_key_t;<br>
typedef void * thread_arg_t; // Host thread argument type<br>
typedef void * thread_result_t; // Host thread result type<br>
typedef void * (*thread_func_t)(void *); // Host thread function type<br>
}<br>
} // namespace lldb<br>
<br>
#endif<br>
<br>
<br>
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).<br>
<br>
<br>
So to sum up:<br>
<br>
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.<br>
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<br>
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<br>
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.<br>
<span class="HOEnZb"><font color="#888888"><br>
<br>
Greg<br>
</font></span></blockquote></div><br></div>