Allow building if libsanitizer on RHEL5 (i.e. with 2.6.18-ish kernel headers)

Hi!

Here is an alternative version of the patch I've posted earlier to allow
building libsanitizer on 2.6.18-ish kernel headers, this time by adding
5 tiny kernel header wrappers.
The only drawback of this are warnings like:
../../../../libsanitizer/include/linux/aio_abi.h:2:2: warning: #include_next is a GCC extension [enabled by default]
(and generally just on the two source files that include these problematic
kernel headers, so about 10 warnings total).
We could avoid that by not building with -pedantic, or by using
-isystem instead of -I for the libsanitizer/include headers (which has
drawback that we wouldn't get warnings for stuff in
libsanitizer/include/sanitizer/ headers), or these could live in
some other directory, say libsanitizer/include/wrappers/linux/*.h
and we would add -isystem $(top_srcdir)/include/wrappers/.

Note that testing still shows some issues, e.g. because the
kThreadDescriptorSize change has not been applied.  But it at least builds
and for many tests works.

2013-12-06  Jakub Jelinek  <jakub@redhat.com>

	* include/linux/aio_abi.h: New header.
	* include/linux/mroute.h: New header.
	* include/linux/mroute6.h: New header.
	* include/linux/perf_event.h: New header.
	* include/linux/types.h: New header.


	Jakub

On Fri, Dec 6, 2013 at 1:43 PM, Jakub Jelinek <jakub@redhat.com> wrote:
> Hi!
>
> Here is an alternative version of the patch I've posted earlier to allow
> building libsanitizer on 2.6.18-ish kernel headers, this time by adding
> 5 tiny kernel header wrappers.
> The only drawback of this are warnings like:
> ../../../../libsanitizer/include/linux/aio_abi.h:2:2: warning: #include_next is a GCC extension [enabled by default]
> (and generally just on the two source files that include these problematic
> kernel headers, so about 10 warnings total).
> We could avoid that by not building with -pedantic, or by using
> -isystem instead of -I for the libsanitizer/include headers (which has
> drawback that we wouldn't get warnings for stuff in
> libsanitizer/include/sanitizer/ headers), or these could live in
> some other directory, say libsanitizer/include/wrappers/linux/*.h
> and we would add -isystem $(top_srcdir)/include/wrappers/.
>
> Note that testing still shows some issues, e.g. because the
> kThreadDescriptorSize change has not been applied.  But it at least builds
> and for many tests works.

What's wrong with just copying the kernel headers 1:1?  ISTR that is what
kernel folks recommend if you use kernel APIs that don't have a glibc
correspondent.

Of course you then need runtime checks on whether the running kernel
supports the API.

Richard.

> 2013-12-06  Jakub Jelinek  <jakub@redhat.com>
>
>         * include/linux/aio_abi.h: New header.
>         * include/linux/mroute.h: New header.
>         * include/linux/mroute6.h: New header.
>         * include/linux/perf_event.h: New header.
>         * include/linux/types.h: New header.
>
> --- libsanitizer/include/linux/aio_abi.h.jj     2013-12-06 06:02:29.000000000 -0500
> +++ libsanitizer/include/linux/aio_abi.h        2013-12-06 06:03:11.000000000 -0500
> @@ -0,0 +1,7 @@
> +#include <linux/version.h>
> +#include_next <linux/aio_abi.h>
> +/* IOCB_CMD_PREADV/PWRITEV has been added in 2.6.19 */
> +#if LINUX_VERSION_CODE < 132627
> +#define IOCB_CMD_PREADV 7
> +#define IOCB_CMD_PWRITEV 8
> +#endif
> --- libsanitizer/include/linux/mroute.h.jj      2013-12-06 06:14:30.000000000 -0500
> +++ libsanitizer/include/linux/mroute.h 2013-12-06 06:10:09.000000000 -0500
> @@ -0,0 +1,8 @@
> +#include <linux/version.h>
> +/* <linux/mroute.h> before 2.6.26 included <linux/in.h>
> +   which clashes with userspace headers.  */
> +#if LINUX_VERSION_CODE < 132634
> +#define _LINUX_IN_H
> +#include <linux/types.h>
> +#endif
> +#include_next <linux/mroute.h>
> --- libsanitizer/include/linux/mroute6.h.jj     2013-12-06 03:58:19.000000000 -0500
> +++ libsanitizer/include/linux/mroute6.h        2013-12-06 06:13:59.000000000 -0500
> @@ -0,0 +1,5 @@
> +#include <linux/version.h>
> +/* <linux/mroute6.h> has been added in 2.6.26 */
> +#if LINUX_VERSION_CODE >= 132634
> +#include_next <linux/mroute6.h>
> +#endif
> --- libsanitizer/include/linux/perf_event.h.jj  2013-12-06 03:58:01.000000000 -0500
> +++ libsanitizer/include/linux/perf_event.h     2013-12-06 05:59:16.000000000 -0500
> @@ -0,0 +1,7 @@
> +#include <linux/version.h>
> +/* <linux/perf_event.h> has been added in 2.6.32 */
> +#if LINUX_VERSION_CODE >= 132640
> +#include_next <linux/perf_event.h>
> +#else
> +#define perf_event_attr __sanitizer_perf_event_attr
> +#endif
> --- libsanitizer/include/linux/types.h.jj       2013-12-06 03:57:37.000000000 -0500
> +++ libsanitizer/include/linux/types.h  2013-12-06 03:57:33.000000000 -0500
> @@ -0,0 +1,12 @@
> +#ifndef LINUX_TYPES_WRAPPER_H
> +#define LINUX_TYPES_WRAPPER_H
> +
> +/* Before
> +   https://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/commit/include/linux/types.h?id=6c7c6afbb8c0e60d32a563cae7c6889211e9d9d8
> +   linux/types.h conflicted with sys/ustat.h.  Work around it.  */
> +
> +#define ustat __asan_bad_ustat
> +#include_next <linux/types.h>
> +#undef ustat
> +
> +#endif
>
>         Jakub

On Fri, Dec 06, 2013 at 01:52:29PM +0100, Richard Biener wrote:
> > Note that testing still shows some issues, e.g. because the
> > kThreadDescriptorSize change has not been applied.  But it at least builds
> > and for many tests works.
> 
> What's wrong with just copying the kernel headers 1:1?  ISTR that is what
> kernel folks recommend if you use kernel APIs that don't have a glibc
> correspondent.

Well, for many things that is already the case, sanitizer_common has
apparently it's own __sanitizer_* macros/constants/structures that
are supposed to match kernel ones.  The problem is that
the code is comparing them as compile time assertions against the kernel
ones inside of the library code.  IMHO that is not a very good idea, much
better would be just say a testcase that would include the sanitizer +
kernel headers, guarded by recent enough LINUX_VERSION_CODE or configure or
similar, so it wouldn't prevent library build on older kernel headers,
the kernel ABI better be stable (only new things added, not size of
structures/magic constants etc. changed from time to time).

But Kostya is apparently not willing to do that, so this patch provides
a workaround in non-compiler-rt maintained files.

> Of course you then need runtime checks on whether the running kernel
> supports the API.

Usually not, say if the structures/magic constants etc. are to wrap syscalls
or ioctls etc., if kernel doesn't support those syscalls/ioctls, either
nothing will use those syscalls/ioctls, or if it does, it will fail from the
kernel, worst case you get some diagnostics from pre-syscall/ioctl wrapper,
otherwise the kernel syscall/ioctl will just fail and post-syscall/ioctl
wrapper will do likely nothing at all.

	Jakub

On Fri, Dec 6, 2013 at 2:04 PM, Jakub Jelinek <jakub@redhat.com> wrote:
> On Fri, Dec 06, 2013 at 01:52:29PM +0100, Richard Biener wrote:
>> > Note that testing still shows some issues, e.g. because the
>> > kThreadDescriptorSize change has not been applied.  But it at least builds
>> > and for many tests works.
>>
>> What's wrong with just copying the kernel headers 1:1?  ISTR that is what
>> kernel folks recommend if you use kernel APIs that don't have a glibc
>> correspondent.
>
> Well, for many things that is already the case, sanitizer_common has
> apparently it's own __sanitizer_* macros/constants/structures that
> are supposed to match kernel ones.  The problem is that
> the code is comparing them as compile time assertions against the kernel
> ones inside of the library code.  IMHO that is not a very good idea, much
> better would be just say a testcase that would include the sanitizer +
> kernel headers, guarded by recent enough LINUX_VERSION_CODE or configure or
> similar, so it wouldn't prevent library build on older kernel headers,
> the kernel ABI better be stable (only new things added, not size of
> structures/magic constants etc. changed from time to time).
>
> But Kostya is apparently not willing to do that, so this patch provides
> a workaround in non-compiler-rt maintained files.

:(

>> Of course you then need runtime checks on whether the running kernel
>> supports the API.
>
> Usually not, say if the structures/magic constants etc. are to wrap syscalls
> or ioctls etc., if kernel doesn't support those syscalls/ioctls, either
> nothing will use those syscalls/ioctls, or if it does, it will fail from the
> kernel, worst case you get some diagnostics from pre-syscall/ioctl wrapper,
> otherwise the kernel syscall/ioctl will just fail and post-syscall/ioctl
> wrapper will do likely nothing at all.

Yes, what I meant is that the code needs to expect things to fail dependent
on the kernel currently running.  Not sure if sanitizer does that.

Richard.

>         Jakub