Patchwork [3/3] libsanitizer: add LFS guards

login
register
mail settings
Submitter aldot
Date April 4, 2013, 7:53 p.m.
Message ID <1365105210-16552-4-git-send-email-rep.dot.nop@gmail.com>
Download mbox | patch
Permalink /patch/233933/
State New
Headers show

Comments

aldot - April 4, 2013, 7:53 p.m.
uClibc can be built without Largefile support, add the corresponding
guards. uClibc does not have __libc_malloc()/__libc_free(), add guard.

libsanitizer/ChangeLog

2013-03-24  Bernhard Reutner-Fischer  <aldot@gcc.gnu.org>

	* sanitizer_common/sanitizer_allocator.cc (libc_malloc,
	libc_free): Guard with !uClibc.
	* interception/interception_type_test.cc <OFF64_T>: add LFS guard.
	* sanitizer_common/sanitizer_platform_limits_posix.cc
	<struct_stat64_sz, struct_rlimit64_sz, struct_statfs64_sz>: Likewise.

Signed-off-by: Bernhard Reutner-Fischer <rep.dot.nop@gmail.com>
---
 libsanitizer/interception/interception_type_test.cc              |    2 +-
 libsanitizer/sanitizer_common/sanitizer_allocator.cc             |    4 +++-
 libsanitizer/sanitizer_common/sanitizer_platform_limits_posix.cc |    4 +++-
 3 files changed, 7 insertions(+), 3 deletions(-)
Konstantin Serebryany - April 5, 2013, 6:26 a.m.
[resending in plain text]

On Fri, Apr 5, 2013 at 10:24 AM, Konstantin Serebryany
<konstantin.s.serebryany@gmail.com> wrote:
> Hi Bernhard,
>
> The libsanitizer code is the exact copy of some revision of the upstream
> code in LLVM repo.
> libsanitizer/README.gcc:
>   Trivial and urgent fixes (portability, build fixes, etc.) may go directly
> to the
>   GCC tree.  All non-trivial changes, functionality improvements, etc.
> should go
>   through the upstream tree first and then be merged back to the GCC tree.
>
> These patches look trivial, but they will not apply to upstream trunk
> because we've changed how we use the guards.
> So, I'd ask you to apply the changes to upstream too (or, better, do it
> first), otherwise they will get lost during the next merge.
>
> I don't mind if you apply the patches to 4.8 branch, but I don't think I may
> approve it.
>
>
>
>
> On Thu, Apr 4, 2013 at 11:53 PM, Bernhard Reutner-Fischer
> <rep.dot.nop@gmail.com> wrote:
>>
>> uClibc can be built without Largefile support, add the corresponding
>> guards. uClibc does not have __libc_malloc()/__libc_free(), add guard.
>>
>> libsanitizer/ChangeLog
>>
>> 2013-03-24  Bernhard Reutner-Fischer  <aldot@gcc.gnu.org>
>>
>>         * sanitizer_common/sanitizer_allocator.cc (libc_malloc,
>>         libc_free): Guard with !uClibc.
>>         * interception/interception_type_test.cc <OFF64_T>: add LFS guard.
>>         * sanitizer_common/sanitizer_platform_limits_posix.cc
>>         <struct_stat64_sz, struct_rlimit64_sz, struct_statfs64_sz>:
>> Likewise.
>>
>> Signed-off-by: Bernhard Reutner-Fischer <rep.dot.nop@gmail.com>
>> ---
>>  libsanitizer/interception/interception_type_test.cc              |    2
>> +-
>>  libsanitizer/sanitizer_common/sanitizer_allocator.cc             |    4
>> +++-
>>  libsanitizer/sanitizer_common/sanitizer_platform_limits_posix.cc |    4
>> +++-
>>  3 files changed, 7 insertions(+), 3 deletions(-)
>>
>> diff --git a/libsanitizer/interception/interception_type_test.cc
>> b/libsanitizer/interception/interception_type_test.cc
>> index f664eee..91fab63 100644
>> --- a/libsanitizer/interception/interception_type_test.cc
>> +++ b/libsanitizer/interception/interception_type_test.cc
>> @@ -22,7 +22,7 @@ COMPILER_CHECK(sizeof(SSIZE_T) == sizeof(ssize_t));
>>  COMPILER_CHECK(sizeof(PTRDIFF_T) == sizeof(ptrdiff_t));
>>  COMPILER_CHECK(sizeof(INTMAX_T) == sizeof(intmax_t));
>>
>> -#ifndef __APPLE__
>> +#if !defined __APPLE__ && (defined __USE_LARGEFILE64 && defined
>> __off64_t_defined)
>>  COMPILER_CHECK(sizeof(OFF64_T) == sizeof(off64_t));
>>  #endif
>>
>> diff --git a/libsanitizer/sanitizer_common/sanitizer_allocator.cc
>> b/libsanitizer/sanitizer_common/sanitizer_allocator.cc
>> index a54de9d..e17cf22 100644
>> --- a/libsanitizer/sanitizer_common/sanitizer_allocator.cc
>> +++ b/libsanitizer/sanitizer_common/sanitizer_allocator.cc
>> @@ -9,11 +9,13 @@
>>  // run-time libraries.
>>  // This allocator that is used inside run-times.
>>
>> //===----------------------------------------------------------------------===//
>> +
>> +#include <features.h>
>>  #include "sanitizer_common.h"
>>
>>  // FIXME: We should probably use more low-level allocator that would
>>  // mmap some pages and split them into chunks to fulfill requests.
>> -#if defined(__linux__) && !defined(__ANDROID__)
>> +#if defined(__linux__) && !defined(__ANDROID__) && !defined __UCLIBC__
>>  extern "C" void *__libc_malloc(__sanitizer::uptr size);
>>  extern "C" void __libc_free(void *ptr);
>>  # define LIBC_MALLOC __libc_malloc
>> diff --git
>> a/libsanitizer/sanitizer_common/sanitizer_platform_limits_posix.cc
>> b/libsanitizer/sanitizer_common/sanitizer_platform_limits_posix.cc
>> index c4be1aa..c5e8f19 100644
>> --- a/libsanitizer/sanitizer_common/sanitizer_platform_limits_posix.cc
>> +++ b/libsanitizer/sanitizer_common/sanitizer_platform_limits_posix.cc
>> @@ -32,7 +32,9 @@
>>  namespace __sanitizer {
>>    unsigned struct_utsname_sz = sizeof(struct utsname);
>>    unsigned struct_stat_sz = sizeof(struct stat);
>> +#ifdef __USE_LARGEFILE64
>>    unsigned struct_stat64_sz = sizeof(struct stat64);
>> +#endif
>>    unsigned struct_rusage_sz = sizeof(struct rusage);
>>    unsigned struct_tm_sz = sizeof(struct tm);
>>
>> @@ -43,7 +45,7 @@ namespace __sanitizer {
>>    unsigned struct_epoll_event_sz = sizeof(struct epoll_event);
>>  #endif // __linux__
>>
>> -#if defined(__linux__) && !defined(__ANDROID__)
>> +#if defined(__linux__) && !defined(__ANDROID__) && defined
>> __USE_LARGEFILE64
>>    unsigned struct_rlimit64_sz = sizeof(struct rlimit64);
>>    unsigned struct_statfs64_sz = sizeof(struct statfs64);
>>  #endif // __linux__ && !__ANDROID__
>> --
>> 1.7.10.4
>>
>
Jakub Jelinek - April 5, 2013, 6:37 a.m.
On Thu, Apr 04, 2013 at 09:53:30PM +0200, Bernhard Reutner-Fischer wrote:
> uClibc can be built without Largefile support, add the corresponding
> guards. uClibc does not have __libc_malloc()/__libc_free(), add guard.

Ugh, this is very ugly.  In addition to the stuff mentioned by Konstantin
that this really should go into upstream first:

> --- a/libsanitizer/interception/interception_type_test.cc
> +++ b/libsanitizer/interception/interception_type_test.cc
> @@ -22,7 +22,7 @@ COMPILER_CHECK(sizeof(SSIZE_T) == sizeof(ssize_t));
>  COMPILER_CHECK(sizeof(PTRDIFF_T) == sizeof(ptrdiff_t));
>  COMPILER_CHECK(sizeof(INTMAX_T) == sizeof(intmax_t));
>  
> -#ifndef __APPLE__
> +#if !defined __APPLE__ && (defined __USE_LARGEFILE64 && defined __off64_t_defined)

Using the internal implementation detail of __USE_LARGEFILE64 is very ugly,
but why __off64_t_defined?  That macro is there just to avoid typedefing it
multiple times, if you include more than one of the sys/types.h, stdio.h and
unistd.h headers.  If you include any of those headers, it will be defined
when __USE_LARGEFILE64 is defined.  Or is uClibc not guaranteeing that?

> --- a/libsanitizer/sanitizer_common/sanitizer_allocator.cc
> +++ b/libsanitizer/sanitizer_common/sanitizer_allocator.cc
> @@ -9,11 +9,13 @@
>  // run-time libraries.
>  // This allocator that is used inside run-times.
>  //===----------------------------------------------------------------------===//
> +
> +#include <features.h>

I'm afraid features.h won't exist on many targets, it isn't a standard
header.  I'd say you want to include some standard header instead (stdio.h?)
or guard this.

	Jakub
Konstantin Serebryany - April 5, 2013, 6:42 a.m.
On Fri, Apr 5, 2013 at 10:37 AM, Jakub Jelinek <jakub@redhat.com> wrote:
> On Thu, Apr 04, 2013 at 09:53:30PM +0200, Bernhard Reutner-Fischer wrote:
>> uClibc can be built without Largefile support, add the corresponding
>> guards. uClibc does not have __libc_malloc()/__libc_free(), add guard.
>
> Ugh, this is very ugly.  In addition to the stuff mentioned by Konstantin
> that this really should go into upstream first:
>
>> --- a/libsanitizer/interception/interception_type_test.cc
>> +++ b/libsanitizer/interception/interception_type_test.cc
>> @@ -22,7 +22,7 @@ COMPILER_CHECK(sizeof(SSIZE_T) == sizeof(ssize_t));
>>  COMPILER_CHECK(sizeof(PTRDIFF_T) == sizeof(ptrdiff_t));
>>  COMPILER_CHECK(sizeof(INTMAX_T) == sizeof(intmax_t));
>>
>> -#ifndef __APPLE__
>> +#if !defined __APPLE__ && (defined __USE_LARGEFILE64 && defined __off64_t_defined)
>
> Using the internal implementation detail of __USE_LARGEFILE64 is very ugly,
> but why __off64_t_defined?  That macro is there just to avoid typedefing it
> multiple times, if you include more than one of the sys/types.h, stdio.h and
> unistd.h headers.  If you include any of those headers, it will be defined
> when __USE_LARGEFILE64 is defined.  Or is uClibc not guaranteeing that?
>
>> --- a/libsanitizer/sanitizer_common/sanitizer_allocator.cc
>> +++ b/libsanitizer/sanitizer_common/sanitizer_allocator.cc
>> @@ -9,11 +9,13 @@
>>  // run-time libraries.
>>  // This allocator that is used inside run-times.
>>  //===----------------------------------------------------------------------===//
>> +
>> +#include <features.h>

I overlooked this.
The sanitizer files (other than *_linux.cc and such) may not include
*any* system header files.
We've been there, it cost us lots of pain and lots of work to get rid of.

--kcc


>
> I'm afraid features.h won't exist on many targets, it isn't a standard
> header.  I'd say you want to include some standard header instead (stdio.h?)
> or guard this.
>
>         Jakub
aldot - April 5, 2013, 9:46 a.m.
On 5 April 2013 08:42, Konstantin Serebryany
<konstantin.s.serebryany@gmail.com> wrote:
>
> On Fri, Apr 5, 2013 at 10:37 AM, Jakub Jelinek <jakub@redhat.com> wrote:
> > On Thu, Apr 04, 2013 at 09:53:30PM +0200, Bernhard Reutner-Fischer wrote:
> >> uClibc can be built without Largefile support, add the corresponding
> >> guards. uClibc does not have __libc_malloc()/__libc_free(), add guard.
> >
> > Ugh, this is very ugly.  In addition to the stuff mentioned by Konstantin
> > that this really should go into upstream first:
> >
> >> --- a/libsanitizer/interception/interception_type_test.cc
> >> +++ b/libsanitizer/interception/interception_type_test.cc
> >> @@ -22,7 +22,7 @@ COMPILER_CHECK(sizeof(SSIZE_T) == sizeof(ssize_t));
> >>  COMPILER_CHECK(sizeof(PTRDIFF_T) == sizeof(ptrdiff_t));
> >>  COMPILER_CHECK(sizeof(INTMAX_T) == sizeof(intmax_t));
> >>
> >> -#ifndef __APPLE__
> >> +#if !defined __APPLE__ && (defined __USE_LARGEFILE64 && defined __off64_t_defined)
> >
> > Using the internal implementation detail of __USE_LARGEFILE64 is very ugly,
> > but why __off64_t_defined?  That macro is there just to avoid typedefing it
> > multiple times, if you include more than one of the sys/types.h, stdio.h and
> > unistd.h headers.  If you include any of those headers, it will be defined
> > when __USE_LARGEFILE64 is defined.  Or is uClibc not guaranteeing that?


It does guarantee that, let me see if i can drop that  && defined
__off64_t_defined.
>
> >
> >> --- a/libsanitizer/sanitizer_common/sanitizer_allocator.cc
> >> +++ b/libsanitizer/sanitizer_common/sanitizer_allocator.cc
> >> @@ -9,11 +9,13 @@
> >>  // run-time libraries.
> >>  // This allocator that is used inside run-times.
> >>  //===----------------------------------------------------------------------===//
> >> +
> >> +#include <features.h>
>
> I overlooked this.
> The sanitizer files (other than *_linux.cc and such) may not include
> *any* system header files.
> We've been there, it cost us lots of pain and lots of work to get rid of.


So how do you suggest i should deal with it then?
I do not have a CPP token inside of the compiler to denote the libc
type, AFAICS.

thanks,
>
>
> --kcc
>
>
> >
> > I'm afraid features.h won't exist on many targets, it isn't a standard
> > header.  I'd say you want to include some standard header instead (stdio.h?)
> > or guard this.
> >
> >         Jakub
Jakub Jelinek - April 5, 2013, 9:49 a.m.
On Fri, Apr 05, 2013 at 11:46:44AM +0200, Bernhard Reutner-Fischer wrote:
> > >> --- a/libsanitizer/sanitizer_common/sanitizer_allocator.cc
> > >> +++ b/libsanitizer/sanitizer_common/sanitizer_allocator.cc
> > >> @@ -9,11 +9,13 @@
> > >>  // run-time libraries.
> > >>  // This allocator that is used inside run-times.
> > >>  //===----------------------------------------------------------------------===//
> > >> +
> > >> +#include <features.h>
> >
> > I overlooked this.
> > The sanitizer files (other than *_linux.cc and such) may not include
> > *any* system header files.
> > We've been there, it cost us lots of pain and lots of work to get rid of.
> 
> 
> So how do you suggest i should deal with it then?
> I do not have a CPP token inside of the compiler to denote the libc
> type, AFAICS.

autoconf, and pass some -DHAS___LIBC_MALLOC or similar down to the compiler
(or just -DUCLIBC or whatever, -DUCLIBC would have the advantage that
(provided llvm doesn't support uClibc) that you'd only need to change gcc's
configury)?

	Jakub
aldot - April 17, 2014, 1:49 p.m.
Respun. First two patches are for gcc, the last one is for upstream
LLVM.

The gcc part was bootstrapped and regtested on x86_64-unknown-linux-gnu
without regressions and bootstrapped on x86_64-unknown-linux-uclibc to
verify that the configury works as expected and that the library links
without errors. These two patches are essentially "backports" of the
LLVM bits in patch #3.

The LLVM part was compiled on x86_64 (X86_64 ?) against glibc and
verified that the configury picks up the previously hard-coded values
both with "configure && make" as well as with "cmake && make".

LLVM'er, please install the LLVM bits.

Ok for trunk?


Bernhard Reutner-Fischer (3):
  libsanitizer: Fix !statfs64 builds
  libsanitizer: add conditionals for libc
  [LLVM] [sanitizer] add conditionals for libc

 libsanitizer/asan/Makefile.am                      |   6 +
 libsanitizer/asan/Makefile.in                      |  17 +-
 libsanitizer/config.h.in                           |  60 +++++
 libsanitizer/configure                             | 281 ++++++++++++++++++++-
 libsanitizer/configure.ac                          |  38 +++
 libsanitizer/interception/interception_linux.cc    |   2 +
 libsanitizer/interception/interception_linux.h     |   8 +
 libsanitizer/lsan/Makefile.am                      |   6 +
 libsanitizer/lsan/Makefile.in                      |  11 +-
 libsanitizer/sanitizer_common/Makefile.am          |   5 +
 libsanitizer/sanitizer_common/Makefile.in          |  18 +-
 .../sanitizer_common_interceptors.inc              | 100 +++++++-
 .../sanitizer_platform_interceptors.h              |   4 +-
 .../sanitizer_platform_limits_linux.cc             |   2 +
 .../sanitizer_platform_limits_posix.cc             |  44 +++-
 .../sanitizer_platform_limits_posix.h              |  27 +-
 .../sanitizer_common/sanitizer_posix_libcdep.cc    |   7 +
 libsanitizer/tsan/Makefile.am                      |   6 +
 libsanitizer/tsan/Makefile.in                      |  11 +-
 19 files changed, 619 insertions(+), 34 deletions(-)

Patch

diff --git a/libsanitizer/interception/interception_type_test.cc b/libsanitizer/interception/interception_type_test.cc
index f664eee..91fab63 100644
--- a/libsanitizer/interception/interception_type_test.cc
+++ b/libsanitizer/interception/interception_type_test.cc
@@ -22,7 +22,7 @@  COMPILER_CHECK(sizeof(SSIZE_T) == sizeof(ssize_t));
 COMPILER_CHECK(sizeof(PTRDIFF_T) == sizeof(ptrdiff_t));
 COMPILER_CHECK(sizeof(INTMAX_T) == sizeof(intmax_t));
 
-#ifndef __APPLE__
+#if !defined __APPLE__ && (defined __USE_LARGEFILE64 && defined __off64_t_defined)
 COMPILER_CHECK(sizeof(OFF64_T) == sizeof(off64_t));
 #endif
 
diff --git a/libsanitizer/sanitizer_common/sanitizer_allocator.cc b/libsanitizer/sanitizer_common/sanitizer_allocator.cc
index a54de9d..e17cf22 100644
--- a/libsanitizer/sanitizer_common/sanitizer_allocator.cc
+++ b/libsanitizer/sanitizer_common/sanitizer_allocator.cc
@@ -9,11 +9,13 @@ 
 // run-time libraries.
 // This allocator that is used inside run-times.
 //===----------------------------------------------------------------------===//
+
+#include <features.h>
 #include "sanitizer_common.h"
 
 // FIXME: We should probably use more low-level allocator that would
 // mmap some pages and split them into chunks to fulfill requests.
-#if defined(__linux__) && !defined(__ANDROID__)
+#if defined(__linux__) && !defined(__ANDROID__) && !defined __UCLIBC__
 extern "C" void *__libc_malloc(__sanitizer::uptr size);
 extern "C" void __libc_free(void *ptr);
 # define LIBC_MALLOC __libc_malloc
diff --git a/libsanitizer/sanitizer_common/sanitizer_platform_limits_posix.cc b/libsanitizer/sanitizer_common/sanitizer_platform_limits_posix.cc
index c4be1aa..c5e8f19 100644
--- a/libsanitizer/sanitizer_common/sanitizer_platform_limits_posix.cc
+++ b/libsanitizer/sanitizer_common/sanitizer_platform_limits_posix.cc
@@ -32,7 +32,9 @@ 
 namespace __sanitizer {
   unsigned struct_utsname_sz = sizeof(struct utsname);
   unsigned struct_stat_sz = sizeof(struct stat);
+#ifdef __USE_LARGEFILE64
   unsigned struct_stat64_sz = sizeof(struct stat64);
+#endif
   unsigned struct_rusage_sz = sizeof(struct rusage);
   unsigned struct_tm_sz = sizeof(struct tm);
 
@@ -43,7 +45,7 @@  namespace __sanitizer {
   unsigned struct_epoll_event_sz = sizeof(struct epoll_event);
 #endif // __linux__
 
-#if defined(__linux__) && !defined(__ANDROID__)
+#if defined(__linux__) && !defined(__ANDROID__) && defined __USE_LARGEFILE64
   unsigned struct_rlimit64_sz = sizeof(struct rlimit64);
   unsigned struct_statfs64_sz = sizeof(struct statfs64);
 #endif // __linux__ && !__ANDROID__