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

Kostya Serebryany kcc at google.com
Wed Apr 10 23:54:19 PDT 2013


  General notes:
  1. Please try to split this CL into several smaller ones.
  2. This huge amount of ifdefs looks horrible. It will cause us pain during maintenance.
  3. +1 to what reid says: I'd like to understand the motivation better.


================
Comment at: lib/sanitizer_common/sanitizer_allocator.cc:33
@@ -32,1 +32,3 @@
 
+#if !SANITIZER_NOLIBC
+
----------------
samsonov@ is going to get rid of this libc dependency *really* soon.
https://code.google.com/p/address-sanitizer/issues/detail?id=176

================
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.
----------------
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 (typede!
 fed to up
 tr 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;



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



More information about the llvm-commits mailing list