On Mon, Dec 02, 2013 at 05:43:17PM +0100, Konstantin Serebryany wrote:
> > No, so your patch doesn't regress anything. I can configure with
> > --disable-libsanitizer to skip build of libsanitizer, although it
> > would be nice to support RHEL5 derived long-term distributions.
> Ok, so this does not gate the merge.
Well, it regresses against 4.8, so it still is a P1 regression.
BTW, even the merge itself apparently regresses, powerpc* doesn't build any
longer and it did before this merge.
The first attached patch fixes the build on powerpc* (tested on RHEL6/ppc*)
and the second patch fixes the build on RHEL5/x86_64, beyond what I've
posted earlier the patch attempts to handle the .cfi* stuff (tried looking
if clang defines some usable macro, but couldn't find any, so no idea how
you can find out if you are compiled with clang -fno-dwarf2-cfi-asm
(when tsan will probably not build at all), or with the -fdwarf2-cfi-asm
default. Probably either you need to convince llvm folks to add something,
or add configure test, moved the linux/mroute* headers to the Linux specific
file so that linux/in.h stuff doesn't clash with netinet/in.h etc. and
finally hacks to avoid including linux/types.h at all costs, that header
historically has always been terrible portability minefield, as it defines
types that glibc headers define too.
With these two patches on top of your patch, I get libsanitizer to build
in multilib configurations on Fedora19/x86_64, RHEL6/ppc* and RHEL5/x86_64
(in all cases both 32-bit and 64-bit builds, but not the third x32 multilib).
Testsuite passes on Fedora19, on RHEL5/x86_64 tests that fail typically fail
on
==2738==AddressSanitizer CHECK failed:
../../../../libsanitizer/sanitizer_common/sanitizer_linux_libcdep.cc:260
"((*tls_addr + *tls_size)) <= ((*stk_addr + *stk_size))" (0x2af8df1bc240,
0x2af8df1bc000)
which clearly is a bug in sanitizer_common,
#if defined(__x86_64__) || defined(__i386__)
// sizeof(struct thread) from glibc.
// There has been a report of this being different on glibc 2.11 and 2.13. We
// don't know when this change happened, so 2.14 is a conservative estimate.
#if __GLIBC_PREREQ(2, 14)
const uptr kThreadDescriptorSize = FIRST_32_SECOND_64(1216, 2304);
#else
const uptr kThreadDescriptorSize = FIRST_32_SECOND_64(1168, 2304);
#endif
uptr ThreadDescriptorSize() {
return kThreadDescriptorSize;
}
but also the InitTlsSize hack can't ever work reliably.
If you need this info, ask glibc folks for a proper supported API.
Hardcoding size of glibc private structure that glibc has changed multiple
times (note, RHEL5 has glibc 2.5 and clearly the size doesn't match there
even with the one you've computed on glibc 2.11) will most likely break any
time glibc adds further fields in there, not to mention that the above
means that if you e.g. build libsanitizer say on glibc 2.11 and run against
glibc 2.14, it will also not work. Plus calling _dl_get_tls_static_info,
which is a GLIBC_PRIVATE symbol, is also going to break whenever glibc
decides to change the ABI of that function.
I believe libthread_db.so.1 has some APIs to query sizeof (struct pthread),
but have never used those APIs so don't know how to query that.
My recommendation would be to just ifdef out this for now until you use
some sane reliable and supportable API. Otherwise, it may work if you are
lucky and always build libsanitizer against the exact same version of glibc
as you run it against, perhaps that is the only use model that llvm cares
about, but for GCC that is definitely not acceptable.
Jakub
--- libsanitizer/sanitizer_common/sanitizer_platform_limits_posix.h.jj3
2013-12-03 03:33:20.000000000 -0500
+++ libsanitizer/sanitizer_common/sanitizer_platform_limits_posix.h
2013-12-03 05:26:41.864437661 -0500
@@ -140,23 +140,32 @@ namespace __sanitizer {
int gid;
int cuid;
int cgid;
-#ifdef __powerpc64__
+#ifdef __powerpc__
unsigned mode;
unsigned __seq;
+ u64 __unused1;
+ u64 __unused2;
#else
unsigned short mode;
unsigned short __pad1;
unsigned short __seq;
unsigned short __pad2;
+#if defined(__x86_64__) && !defined(_LP64)
+ u64 __unused1;
+ u64 __unused2;
+#else
+ unsigned long __unused1;
+ unsigned long __unused2;
+#endif
#endif
- uptr __unused1;
- uptr __unused2;
};
struct __sanitizer_shmid_ds {
__sanitizer_ipc_perm shm_perm;
#ifndef __powerpc__
uptr shm_segsz;
+ #elif !defined(__powerpc64__)
+ uptr __unused0;
#endif
uptr shm_atime;
#ifndef _LP64
@@ -288,17 +297,20 @@ namespace __sanitizer {
typedef long __sanitizer_clock_t;
#if SANITIZER_LINUX
-#if defined(_LP64) || defined(__x86_64__)
+#if defined(_LP64) || defined(__x86_64__) || defined(__powerpc__)
typedef unsigned __sanitizer___kernel_uid_t;
typedef unsigned __sanitizer___kernel_gid_t;
- typedef long long __sanitizer___kernel_off_t;
#else
typedef unsigned short __sanitizer___kernel_uid_t;
typedef unsigned short __sanitizer___kernel_gid_t;
+#endif
+#if defined(__x86_64__) && !defined(_LP64)
+ typedef long long __sanitizer___kernel_off_t;
+#else
typedef long __sanitizer___kernel_off_t;
#endif
-#if defined(__powerpc64__)
+#if defined(__powerpc__)
typedef unsigned int __sanitizer___kernel_old_uid_t;
typedef unsigned int __sanitizer___kernel_old_gid_t;
#else
--- libsanitizer/sanitizer_common/sanitizer_linux.cc.jj 2013-12-03
11:47:47.399488381 +0100
+++ libsanitizer/sanitizer_common/sanitizer_linux.cc 2013-12-03
11:48:01.905411215 +0100
@@ -814,6 +814,9 @@ uptr internal_clone(int (*fn)(void *), v
* %r8 = new_tls,
* %r10 = child_tidptr)
*/
+#ifdef __GCC_HAVE_DWARF2_CFI_ASM
+ ".cfi_endproc\n"
+#endif
"syscall\n"
/* if (%rax != 0)
@@ -826,6 +829,10 @@ uptr internal_clone(int (*fn)(void *), v
// XXX: We should also terminate the CFI unwind chain
// here. Unfortunately clang 3.2 doesn't support the
// necessary CFI directives, so we skip that part.
+#ifdef __GCC_HAVE_DWARF2_CFI_ASM
+ ".cfi_startproc\n"
+ ".cfi_undefined %%rip;\n"
+#endif
"xorq %%rbp,%%rbp\n"
/* Call "fn(arg)". */
--- libsanitizer/sanitizer_common/sanitizer_platform_limits_posix.cc.jj
2013-12-03 11:47:47.246489195 +0100
+++ libsanitizer/sanitizer_common/sanitizer_platform_limits_posix.cc
2013-12-03 11:51:31.884291618 +0100
@@ -17,6 +17,18 @@
#include "sanitizer_internal_defs.h"
#include "sanitizer_platform_limits_posix.h"
+#if SANITIZER_LINUX && !SANITIZER_ANDROID
+#include <asm/types.h>
+#include <asm/posix_types.h>
+#define _LINUX_TYPES_H
+#define __le16 __u16
+#define __le32 __u32
+#define __le64 __u64
+#define __be16 __u16
+#define __be32 __u32
+#define __be64 __u64
+#endif
+
#include <arpa/inet.h>
#include <dirent.h>
#include <errno.h>
@@ -86,8 +98,6 @@
#include <linux/if_eql.h>
#include <linux/if_plip.h>
#include <linux/lp.h>
-#include <linux/mroute.h>
-#include <linux/mroute6.h>
#include <linux/scc.h>
#include <linux/serial.h>
#include <sys/msg.h>
@@ -321,11 +331,6 @@ namespace __sanitizer {
unsigned struct_unimapinit_sz = sizeof(struct unimapinit);
#endif
-#if !SANITIZER_ANDROID && !SANITIZER_MAC
- unsigned struct_sioc_sg_req_sz = sizeof(struct sioc_sg_req);
- unsigned struct_sioc_vif_req_sz = sizeof(struct sioc_vif_req);
-#endif
-
unsigned IOCTL_NOT_PRESENT = 0;
unsigned IOCTL_FIOASYNC = FIOASYNC;
@@ -372,10 +377,6 @@ namespace __sanitizer {
unsigned IOCTL_TIOCSPGRP = TIOCSPGRP;
unsigned IOCTL_TIOCSTI = TIOCSTI;
unsigned IOCTL_TIOCSWINSZ = TIOCSWINSZ;
-#if (SANITIZER_LINUX && !SANITIZER_ANDROID)
- unsigned IOCTL_SIOCGETSGCNT = SIOCGETSGCNT;
- unsigned IOCTL_SIOCGETVIFCNT = SIOCGETVIFCNT;
-#endif
#if SANITIZER_LINUX
unsigned IOCTL_EVIOCGABS = EVIOCGABS(0);
unsigned IOCTL_EVIOCGBIT = EVIOCGBIT(0, 0);
--- libsanitizer/sanitizer_common/sanitizer_platform_limits_linux.cc.jj
2013-12-03 11:47:47.449488116 +0100
+++ libsanitizer/sanitizer_common/sanitizer_platform_limits_linux.cc
2013-12-03 11:54:34.800468807 +0100
@@ -26,6 +26,7 @@
// With old kernels (and even new kernels on powerpc) asm/stat.h uses types
that
// are not defined anywhere in userspace headers. Fake them. This seems to work
// fine with newer headers, too.
+#include <asm/types.h>
#include <asm/posix_types.h>
#define ino_t __kernel_ino_t
#define mode_t __kernel_mode_t
@@ -42,6 +43,14 @@
#undef gid_t
#undef off_t
+#define _LINUX_TYPES_H
+#define __le16 __u16
+#define __le32 __u32
+#define __le64 __u64
+#define __be16 __u16
+#define __be32 __u32
+#define __be64 __u64
+
#include <linux/aio_abi.h>
#if SANITIZER_ANDROID
@@ -51,11 +60,32 @@
#endif
#if !SANITIZER_ANDROID
+#include <linux/version.h>
+// <linux/perf_event.h> has been added in 2.6.32
+#if LINUX_VERSION_CODE >= 132640
#include <linux/perf_event.h>
#endif
+#include <bits/sockaddr.h>
+#include <linux/in.h>
+#include <linux/mroute.h>
+// <linux/mroute6.h> has been added in 2.6.26
+#if LINUX_VERSION_CODE >= 132634
+#include <linux/in6.h>
+#include <linux/mroute6.h>
+#endif
+#endif
namespace __sanitizer {
unsigned struct_statfs64_sz = sizeof(struct statfs64);
+
+#if !SANITIZER_ANDROID
+ unsigned struct_sioc_sg_req_sz = sizeof(struct sioc_sg_req);
+ unsigned struct_sioc_vif_req_sz = sizeof(struct sioc_vif_req);
+
+ unsigned IOCTL_SIOCGETSGCNT = SIOCGETSGCNT;
+ unsigned IOCTL_SIOCGETVIFCNT = SIOCGETVIFCNT;
+#endif
+
} // namespace __sanitizer
#if !defined(__powerpc64__)
@@ -75,15 +105,18 @@ CHECK_SIZE_AND_OFFSET(io_event, res);
CHECK_SIZE_AND_OFFSET(io_event, res2);
#if !SANITIZER_ANDROID
+#if LINUX_VERSION_CODE >= 132640
COMPILER_CHECK(sizeof(struct __sanitizer_perf_event_attr) <=
sizeof(struct perf_event_attr));
CHECK_SIZE_AND_OFFSET(perf_event_attr, type);
CHECK_SIZE_AND_OFFSET(perf_event_attr, size);
#endif
+#endif
COMPILER_CHECK(iocb_cmd_pread == IOCB_CMD_PREAD);
COMPILER_CHECK(iocb_cmd_pwrite == IOCB_CMD_PWRITE);
-#if !SANITIZER_ANDROID
+#if !SANITIZER_ANDROID && LINUX_VERSION_CODE >= 132627
+// IOCB_CMD_PREADV/PWRITEV has been added in 2.6.19
COMPILER_CHECK(iocb_cmd_preadv == IOCB_CMD_PREADV);
COMPILER_CHECK(iocb_cmd_pwritev == IOCB_CMD_PWRITEV);
#endif
--- libsanitizer/tsan/tsan_rtl_amd64.S.jj 2012-11-23 00:14:41.336231221
+0100
+++ libsanitizer/tsan/tsan_rtl_amd64.S 2013-12-03 11:48:01.905411215 +0100
@@ -1,42 +1,58 @@
+#ifdef __GCC_HAVE_DWARF2_CFI_ASM
+#define CFI_STARTPROC .cfi_startproc
+#define CFI_ENDPROC .cfi_endproc
+#define CFI_ADJUST_CFA_OFFSET(n) .cfi_adjust_cfa_offset n
+#define CFI_REL_OFFSET(reg,n) .cfi_rel_offset reg, n
+#define CFI_DEF_CFA_REGISTER(reg) .cfi_def_cfa_register reg
+#define CFI_RESTORE(reg) .cfi_restore reg
+#else
+#define CFI_STARTPROC
+#define CFI_ENDPROC
+#define CFI_ADJUST_CFA_OFFSET(n)
+#define CFI_REL_OFFSET(reg,n)
+#define CFI_DEF_CFA_REGISTER(reg)
+#define CFI_RESTORE(reg)
+#endif
+
.section .text
.globl __tsan_trace_switch_thunk
__tsan_trace_switch_thunk:
- .cfi_startproc
+ CFI_STARTPROC
# Save scratch registers.
push %rax
- .cfi_adjust_cfa_offset 8
- .cfi_rel_offset %rax, 0
+ CFI_ADJUST_CFA_OFFSET(8)
+ CFI_REL_OFFSET(%rax, 0)
push %rcx
- .cfi_adjust_cfa_offset 8
- .cfi_rel_offset %rcx, 0
+ CFI_ADJUST_CFA_OFFSET(8)
+ CFI_REL_OFFSET(%rcx, 0)
push %rdx
- .cfi_adjust_cfa_offset 8
- .cfi_rel_offset %rdx, 0
+ CFI_ADJUST_CFA_OFFSET(8)
+ CFI_REL_OFFSET(%rdx, 0)
push %rsi
- .cfi_adjust_cfa_offset 8
- .cfi_rel_offset %rsi, 0
+ CFI_ADJUST_CFA_OFFSET(8)
+ CFI_REL_OFFSET(%rsi, 0)
push %rdi
- .cfi_adjust_cfa_offset 8
- .cfi_rel_offset %rdi, 0
+ CFI_ADJUST_CFA_OFFSET(8)
+ CFI_REL_OFFSET(%rdi, 0)
push %r8
- .cfi_adjust_cfa_offset 8
- .cfi_rel_offset %r8, 0
+ CFI_ADJUST_CFA_OFFSET(8)
+ CFI_REL_OFFSET(%r8, 0)
push %r9
- .cfi_adjust_cfa_offset 8
- .cfi_rel_offset %r9, 0
+ CFI_ADJUST_CFA_OFFSET(8)
+ CFI_REL_OFFSET(%r9, 0)
push %r10
- .cfi_adjust_cfa_offset 8
- .cfi_rel_offset %r10, 0
+ CFI_ADJUST_CFA_OFFSET(8)
+ CFI_REL_OFFSET(%r10, 0)
push %r11
- .cfi_adjust_cfa_offset 8
- .cfi_rel_offset %r11, 0
+ CFI_ADJUST_CFA_OFFSET(8)
+ CFI_REL_OFFSET(%r11, 0)
# Align stack frame.
push %rbx # non-scratch
- .cfi_adjust_cfa_offset 8
- .cfi_rel_offset %rbx, 0
+ CFI_ADJUST_CFA_OFFSET(8)
+ CFI_REL_OFFSET(%rbx, 0)
mov %rsp, %rbx # save current rsp
- .cfi_def_cfa_register %rbx
+ CFI_DEF_CFA_REGISTER(%rbx)
shr $4, %rsp # clear 4 lsb, align to 16
shl $4, %rsp
@@ -44,78 +60,78 @@ __tsan_trace_switch_thunk:
# Unalign stack frame back.
mov %rbx, %rsp # restore the original rsp
- .cfi_def_cfa_register %rsp
+ CFI_DEF_CFA_REGISTER(%rsp)
pop %rbx
- .cfi_adjust_cfa_offset -8
+ CFI_ADJUST_CFA_OFFSET(-8)
# Restore scratch registers.
pop %r11
- .cfi_adjust_cfa_offset -8
+ CFI_ADJUST_CFA_OFFSET(-8)
pop %r10
- .cfi_adjust_cfa_offset -8
+ CFI_ADJUST_CFA_OFFSET(-8)
pop %r9
- .cfi_adjust_cfa_offset -8
+ CFI_ADJUST_CFA_OFFSET(-8)
pop %r8
- .cfi_adjust_cfa_offset -8
+ CFI_ADJUST_CFA_OFFSET(-8)
pop %rdi
- .cfi_adjust_cfa_offset -8
+ CFI_ADJUST_CFA_OFFSET(-8)
pop %rsi
- .cfi_adjust_cfa_offset -8
+ CFI_ADJUST_CFA_OFFSET(-8)
pop %rdx
- .cfi_adjust_cfa_offset -8
+ CFI_ADJUST_CFA_OFFSET(-8)
pop %rcx
- .cfi_adjust_cfa_offset -8
+ CFI_ADJUST_CFA_OFFSET(-8)
pop %rax
- .cfi_adjust_cfa_offset -8
- .cfi_restore %rax
- .cfi_restore %rbx
- .cfi_restore %rcx
- .cfi_restore %rdx
- .cfi_restore %rsi
- .cfi_restore %rdi
- .cfi_restore %r8
- .cfi_restore %r9
- .cfi_restore %r10
- .cfi_restore %r11
+ CFI_ADJUST_CFA_OFFSET(-8)
+ CFI_RESTORE(%rax)
+ CFI_RESTORE(%rbx)
+ CFI_RESTORE(%rcx)
+ CFI_RESTORE(%rdx)
+ CFI_RESTORE(%rsi)
+ CFI_RESTORE(%rdi)
+ CFI_RESTORE(%r8)
+ CFI_RESTORE(%r9)
+ CFI_RESTORE(%r10)
+ CFI_RESTORE(%r11)
ret
- .cfi_endproc
+ CFI_ENDPROC
.globl __tsan_report_race_thunk
__tsan_report_race_thunk:
- .cfi_startproc
+ CFI_STARTPROC
# Save scratch registers.
push %rax
- .cfi_adjust_cfa_offset 8
- .cfi_rel_offset %rax, 0
+ CFI_ADJUST_CFA_OFFSET(8)
+ CFI_REL_OFFSET(%rax, 0)
push %rcx
- .cfi_adjust_cfa_offset 8
- .cfi_rel_offset %rcx, 0
+ CFI_ADJUST_CFA_OFFSET(8)
+ CFI_REL_OFFSET(%rcx, 0)
push %rdx
- .cfi_adjust_cfa_offset 8
- .cfi_rel_offset %rdx, 0
+ CFI_ADJUST_CFA_OFFSET(8)
+ CFI_REL_OFFSET(%rdx, 0)
push %rsi
- .cfi_adjust_cfa_offset 8
- .cfi_rel_offset %rsi, 0
+ CFI_ADJUST_CFA_OFFSET(8)
+ CFI_REL_OFFSET(%rsi, 0)
push %rdi
- .cfi_adjust_cfa_offset 8
- .cfi_rel_offset %rdi, 0
+ CFI_ADJUST_CFA_OFFSET(8)
+ CFI_REL_OFFSET(%rdi, 0)
push %r8
- .cfi_adjust_cfa_offset 8
- .cfi_rel_offset %r8, 0
+ CFI_ADJUST_CFA_OFFSET(8)
+ CFI_REL_OFFSET(%r8, 0)
push %r9
- .cfi_adjust_cfa_offset 8
- .cfi_rel_offset %r9, 0
+ CFI_ADJUST_CFA_OFFSET(8)
+ CFI_REL_OFFSET(%r9, 0)
push %r10
- .cfi_adjust_cfa_offset 8
- .cfi_rel_offset %r10, 0
+ CFI_ADJUST_CFA_OFFSET(8)
+ CFI_REL_OFFSET(%r10, 0)
push %r11
- .cfi_adjust_cfa_offset 8
- .cfi_rel_offset %r11, 0
+ CFI_ADJUST_CFA_OFFSET(8)
+ CFI_REL_OFFSET(%r11, 0)
# Align stack frame.
push %rbx # non-scratch
- .cfi_adjust_cfa_offset 8
- .cfi_rel_offset %rbx, 0
+ CFI_ADJUST_CFA_OFFSET(8)
+ CFI_REL_OFFSET(%rbx, 0)
mov %rsp, %rbx # save current rsp
- .cfi_def_cfa_register %rbx
+ CFI_DEF_CFA_REGISTER(%rbx)
shr $4, %rsp # clear 4 lsb, align to 16
shl $4, %rsp
@@ -123,40 +139,40 @@ __tsan_report_race_thunk:
# Unalign stack frame back.
mov %rbx, %rsp # restore the original rsp
- .cfi_def_cfa_register %rsp
+ CFI_DEF_CFA_REGISTER(%rsp)
pop %rbx
- .cfi_adjust_cfa_offset -8
+ CFI_ADJUST_CFA_OFFSET(-8)
# Restore scratch registers.
pop %r11
- .cfi_adjust_cfa_offset -8
+ CFI_ADJUST_CFA_OFFSET(-8)
pop %r10
- .cfi_adjust_cfa_offset -8
+ CFI_ADJUST_CFA_OFFSET(-8)
pop %r9
- .cfi_adjust_cfa_offset -8
+ CFI_ADJUST_CFA_OFFSET(-8)
pop %r8
- .cfi_adjust_cfa_offset -8
+ CFI_ADJUST_CFA_OFFSET(-8)
pop %rdi
- .cfi_adjust_cfa_offset -8
+ CFI_ADJUST_CFA_OFFSET(-8)
pop %rsi
- .cfi_adjust_cfa_offset -8
+ CFI_ADJUST_CFA_OFFSET(-8)
pop %rdx
- .cfi_adjust_cfa_offset -8
+ CFI_ADJUST_CFA_OFFSET(-8)
pop %rcx
- .cfi_adjust_cfa_offset -8
+ CFI_ADJUST_CFA_OFFSET(-8)
pop %rax
- .cfi_adjust_cfa_offset -8
- .cfi_restore %rax
- .cfi_restore %rbx
- .cfi_restore %rcx
- .cfi_restore %rdx
- .cfi_restore %rsi
- .cfi_restore %rdi
- .cfi_restore %r8
- .cfi_restore %r9
- .cfi_restore %r10
- .cfi_restore %r11
+ CFI_ADJUST_CFA_OFFSET(-8)
+ CFI_RESTORE(%rax)
+ CFI_RESTORE(%rbx)
+ CFI_RESTORE(%rcx)
+ CFI_RESTORE(%rdx)
+ CFI_RESTORE(%rsi)
+ CFI_RESTORE(%rdi)
+ CFI_RESTORE(%r8)
+ CFI_RESTORE(%r9)
+ CFI_RESTORE(%r10)
+ CFI_RESTORE(%r11)
ret
- .cfi_endproc
+ CFI_ENDPROC
#ifdef __linux__
/* We do not need executable stack. */
--- libsanitizer/tsan/tsan_rtl.h.jj 2013-12-03 11:47:47.152489695 +0100
+++ libsanitizer/tsan/tsan_rtl.h 2013-12-03 11:48:01.888411307 +0100
@@ -732,13 +732,18 @@ void AcquireReleaseImpl(ThreadState *thr
#if TSAN_DEBUG == 0
// The caller may not create the stack frame for itself at all,
// so we create a reserve stack frame for it (1024b must be enough).
+#ifdef __GCC_HAVE_DWARF2_CFI_ASM
+#define CFI_ADJUST_CFA_OFFSET(n) ".cfi_adjust_cfa_offset " #n ";"
+#else
+#define CFI_ADJUST_CFA_OFFSET(n)
+#endif
#define HACKY_CALL(f) \
__asm__ __volatile__("sub $1024, %%rsp;" \
- ".cfi_adjust_cfa_offset 1024;" \
+ CFI_ADJUST_CFA_OFFSET(1024) \
".hidden " #f "_thunk;" \
"call " #f "_thunk;" \
"add $1024, %%rsp;" \
- ".cfi_adjust_cfa_offset -1024;" \
+ CFI_ADJUST_CFA_OFFSET(-1024) \
::: "memory", "cc");
#else
#define HACKY_CALL(f) f()
