[llvm-commits] [compiler-rt] r159132 - /compiler-rt/trunk/lib/asan/asan_malloc_linux.cc

[Chandler Carruth]

On Mon, Jun 25, 2012 at 3:18 AM, Kostya Serebryany <kcc at google.com> wrote:

>
>
> On Mon, Jun 25, 2012 at 2:15 PM, Chandler Carruth <chandlerc at google.com>wrote:
>
>> On Mon, Jun 25, 2012 at 2:58 AM, Kostya Serebryany <kcc at google.com>wrote:
>>
>>> Author: kcc
>>> Date: Mon Jun 25 04:58:29 2012
>>> New Revision: 159132
>>>
>>> URL: http://llvm.org/viewvc/llvm-project?rev=159132&view=rev
>>> Log:
>>> [asan] get rid of '#include <malloc.h>' in the implementation of malloc
>>> interceptors
>>>
>>> Modified:
>>>    compiler-rt/trunk/lib/asan/asan_malloc_linux.cc
>>>
>>> Modified: compiler-rt/trunk/lib/asan/asan_malloc_linux.cc
>>> URL:
>>> http://llvm.org/viewvc/llvm-project/compiler-rt/trunk/lib/asan/asan_malloc_linux.cc?rev=159132&r1=159131&r2=159132&view=diff
>>>
>>> ==============================================================================
>>> --- compiler-rt/trunk/lib/asan/asan_malloc_linux.cc (original)
>>> +++ compiler-rt/trunk/lib/asan/asan_malloc_linux.cc Mon Jun 25 04:58:29
>>> 2012
>>> @@ -20,15 +20,13 @@
>>>  #include "asan_internal.h"
>>>  #include "asan_stack.h"
>>>
>>> -#include <malloc.h>
>>> -
>>>  #ifdef ANDROID
>>>  struct MallocDebug {
>>> -  void* (*malloc)(size_t bytes);
>>> +  void* (*malloc)(uptr bytes);
>>>
>>
>> This seems really wrong to me... There are definitely platforms where
>> sizeof(size_t) != sizeof(void*)...
>>
>
> There are such platforms indeed, but asan will be broken on such systems
> in all other possible ways, not just this.
>

Why not define these according to the standard?

In particular, I don't understand an aversion to 'stddef.h' and 'size_t'.
These don't come from the OS, they come from the standard and from the
compiler. The 'stddef.h' that ships with Clang doesn't include any other
headers. It cannot be simulated by your code because it leverages
implementation-details to expose standard interfaces such as 'size_t'.

For example, 'stddef.h' can be included into code that is compiled with
-ffreestanding, and thus without any system libraries or headers. It seems
like that should be a sufficiently strict bar for portability for ASan?


> Otoh, this change protects us from problems like this one, where malloc.h
> has some unexpected features.
>

My comment here isn't about including 'malloc.h', but about using 'uptr'
instead of 'size_t'.


>
> --kcc
>
>
>>
>>
>>>   void  (*free)(void* mem);
>>> -  void* (*calloc)(size_t n_elements, size_t elem_size);
>>> -  void* (*realloc)(void* oldMem, size_t bytes);
>>> -  void* (*memalign)(size_t alignment, size_t bytes);
>>> +  void* (*calloc)(uptr n_elements, uptr elem_size);
>>> +  void* (*realloc)(void* oldMem, uptr bytes);
>>> +  void* (*memalign)(uptr alignment, uptr bytes);
>>>  };
>>>
>>>  const MallocDebug asan_malloc_dispatch ALIGNED(32) = {
>>> @@ -64,18 +62,18 @@
>>>   asan_free(ptr, &stack);
>>>  }
>>>
>>> -INTERCEPTOR(void*, malloc, size_t size) {
>>> +INTERCEPTOR(void*, malloc, uptr size) {
>>>   GET_STACK_TRACE_HERE_FOR_MALLOC;
>>>   return asan_malloc(size, &stack);
>>>  }
>>>
>>> -INTERCEPTOR(void*, calloc, size_t nmemb, size_t size) {
>>> +INTERCEPTOR(void*, calloc, uptr nmemb, uptr size) {
>>>   if (!asan_inited) {
>>>     // Hack: dlsym calls calloc before REAL(calloc) is retrieved from
>>> dlsym.
>>> -    const size_t kCallocPoolSize = 1024;
>>> +    const uptr kCallocPoolSize = 1024;
>>>     static uptr calloc_memory_for_dlsym[kCallocPoolSize];
>>> -    static size_t allocated;
>>> -    size_t size_in_words = ((nmemb * size) + kWordSize - 1) / kWordSize;
>>> +    static uptr allocated;
>>> +    uptr size_in_words = ((nmemb * size) + kWordSize - 1) / kWordSize;
>>>     void *mem = (void*)&calloc_memory_for_dlsym[allocated];
>>>     allocated += size_in_words;
>>>     CHECK(allocated < kCallocPoolSize);
>>> @@ -85,26 +83,34 @@
>>>   return asan_calloc(nmemb, size, &stack);
>>>  }
>>>
>>> -INTERCEPTOR(void*, realloc, void *ptr, size_t size) {
>>> +INTERCEPTOR(void*, realloc, void *ptr, uptr size) {
>>>   GET_STACK_TRACE_HERE_FOR_MALLOC;
>>>   return asan_realloc(ptr, size, &stack);
>>>  }
>>>
>>> -INTERCEPTOR(void*, memalign, size_t boundary, size_t size) {
>>> +INTERCEPTOR(void*, memalign, uptr boundary, uptr size) {
>>>   GET_STACK_TRACE_HERE_FOR_MALLOC;
>>>   return asan_memalign(boundary, size, &stack);
>>>  }
>>>
>>> -INTERCEPTOR(void*, __libc_memalign, size_t align, size_t s)
>>> +INTERCEPTOR(void*, __libc_memalign, uptr align, uptr s)
>>>   ALIAS("memalign");
>>>
>>> -INTERCEPTOR(size_t, malloc_usable_size, void *ptr) {
>>> +INTERCEPTOR(uptr, malloc_usable_size, void *ptr) {
>>>   GET_STACK_TRACE_HERE_FOR_MALLOC;
>>>   return asan_malloc_usable_size(ptr, &stack);
>>>  }
>>>
>>> -INTERCEPTOR(struct mallinfo, mallinfo, void) {
>>> -  struct mallinfo res;
>>> +// We avoid including malloc.h for portability reasons.
>>> +// man mallinfo says the fields are "long", but the implementation uses
>>> int.
>>> +// It doesn't matter much -- we just need to make sure that the libc's
>>> mallinfo
>>> +// is not called.
>>> +struct fake_mallinfo {
>>> +  int x[10];
>>> +};
>>> +
>>> +INTERCEPTOR(struct fake_mallinfo, mallinfo, void) {
>>> +  struct fake_mallinfo res;
>>>   REAL(memset)(&res, 0, sizeof(res));
>>>   return res;
>>>  }
>>> @@ -113,18 +119,18 @@
>>>   return -1;
>>>  }
>>>
>>> -INTERCEPTOR(int, posix_memalign, void **memptr, size_t alignment,
>>> size_t size) {
>>> +INTERCEPTOR(int, posix_memalign, void **memptr, uptr alignment, uptr
>>> size) {
>>>   GET_STACK_TRACE_HERE_FOR_MALLOC;
>>>   // Printf("posix_memalign: %zx %zu\n", alignment, size);
>>>   return asan_posix_memalign(memptr, alignment, size, &stack);
>>>  }
>>>
>>> -INTERCEPTOR(void*, valloc, size_t size) {
>>> +INTERCEPTOR(void*, valloc, uptr size) {
>>>   GET_STACK_TRACE_HERE_FOR_MALLOC;
>>>   return asan_valloc(size, &stack);
>>>  }
>>>
>>> -INTERCEPTOR(void*, pvalloc, size_t size) {
>>> +INTERCEPTOR(void*, pvalloc, uptr size) {
>>>   GET_STACK_TRACE_HERE_FOR_MALLOC;
>>>   return asan_pvalloc(size, &stack);
>>>  }
>>>
>>>
>>> _______________________________________________
>>> llvm-commits mailing list
>>> llvm-commits at cs.uiuc.edu
>>> http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits
>>>
>>
>>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20120625/0e29a75f/attachment.html>