[PATCH] Add a libc-independent sanitizer_common for Linux/x86_64.

[Peter Collingbourne]

  > 1. Please try to split this CL into several smaller ones.

  Done; please see D755, D756, D757.

  > 2. This huge amount of ifdefs looks horrible. It will cause us pain during maintenance.

  I agree, but I can't really see a better way to exclude the libc-dependent stuff.  In any case, I think the amount of code we have to ifdef will decrease over time as we move things like the allocator into sanitizer_common.  I see the end goal for this work as returning to the state where we have one sanitizer_common, which would then subsume the use cases for SANITIZER_NOLIBC (and SANITIZER_GO).  But I don't think this can (or should) be done as a single monolithic patch so I propose that we temporarily have two versions of sanitizer_common during the transition.

  > 3. +1 to what reid says: I'd like to understand the motivation better.

  I've added more information on motivation to the first commit message.


================
Comment at: lib/sanitizer_common/sanitizer_common.h:116
@@ -115,3 +115,3 @@
 
-fd_t OpenFile(const char *filename, bool write);
+uptr OpenFile(const char *filename, bool write);
 // Opens the file 'file_name" and reads up to 'max_len' bytes.
----------------
Kostya Serebryany wrote:
> Peter Collingbourne wrote:
> > Reid Kleckner wrote:
> > > The vast majority of this change is the fd_t rename, and it probably needs more motivation in the change description.
> > > 
> > > First, is fd_t pointer sized?  Generally Linux fds are 32-bit ints.  I don't know about other platforms.
> > > 
> > > Second, is it unreasonable to provide our own typedef for fd_t in sanitizer_common.h if we can't get fd_t from the system somehow?
> > > The vast majority of this change is the fd_t rename, and it probably needs more motivation in the change description.
> > 
> > That's true, I'll write something more extensive up.  It's not really a rename though -- I'm only touching the return type of syscalls and syscall-like functions (which is why fd_t is still used for storage etc).  In moving away from libc I wanted to move to a model where the result of these functions is potentially contained in the return value itself rather than being implicit in errno, so they would have to be of some type that can hold any potential result (or an error value) and can be converted to the "correct" type.  On Linux/x86 I know this would be uptr (aka uintptr_t), as this is the same width as the register used for syscall return values (eax/rax).  I considered a more flexible approach where the return value would be returned as a pair or through a reference parameter (i.e. the error_code mechanism we currently have in LLVM), but ultimately I felt that would be overkill for the platforms we currently support.  We could consider having a type syscall_t (type!
 defed to 
 uptr initially) which we could redefine if uptr turns out not to be enough.
> > 
> > > First, is fd_t pointer sized? Generally Linux fds are 32-bit ints. I don't know about other platforms.
> > 
> > For all the platforms we currently support, I believe fd_t is of narrower or equal width to uptr.  We could probably add a static assertion somewhere to check this, though.
> > 
> > > Second, is it unreasonable to provide our own typedef for fd_t in sanitizer_common.h if we can't get fd_t from the system somehow?
> > 
> > I believe we already typedef fd_t to int in sanitizer_internal_defs.h.  POSIX says this is correct, but it might have to change for non-POSIX platforms.
> We should define FD_T (capital) like we do with other types in lib/interception/interception.h:
> // These typedefs should be used only in the interceptor definitions to replace
> // the standard system types (e.g. SSIZE_T instead of ssize_t)
> typedef __sanitizer::uptr    SIZE_T;
> typedef __sanitizer::sptr    SSIZE_T;
> typedef __sanitizer::sptr    PTRDIFF_T;
> typedef __sanitizer::s64     INTMAX_T;
> typedef __sanitizer::OFF_T   OFF_T;
> typedef __sanitizer::OFF64_T OFF64_T;
> 
I think that if we want to make the interceptors use FD_T that should be part of a separate change.

================
Comment at: lib/sanitizer_common/sanitizer_allocator.cc:33
@@ -32,1 +32,3 @@
 
+#if !SANITIZER_NOLIBC
+
----------------
Kostya Serebryany wrote:
> samsonov@ is going to get rid of this libc dependency *really* soon.
> https://code.google.com/p/address-sanitizer/issues/detail?id=176
Cool, that should allow us to remove a number of ifdefs from this patch.


http://llvm-reviews.chandlerc.com/D654