Re: [PATCH 0/7] Libsanitizer merge from upstream r285547.

[Maxim Ostapenko]

On 07/11/16 13:04, Jakub Jelinek wrote:

On Mon, Nov 07, 2016 at 11:22:28AM +0300, Maxim Ostapenko wrote:

Hi,

this patch set performs libsanitizer merge from upstream.

Patch 1 is the library merge itself.

Patch 2 is the reapplied change for SPARC by David S. Miller.

Patch 3 changes heuristic for extracting last PC from stack frame for ARM in
fast unwind routine. More details can be found here
(https://gcc.gnu.org/bugzilla/show_bug.cgi?id=61771).

Patch 4 replaces Jakub's fix for
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=63888 and removes
CheckODRViolationViaPoisoning call from RegisterGlobal to avoid false
positive odr violation reports.

Patch 5 combines necessary compiler changes.

Patch 6 adds several new tests, backported from upstream.

The patches 1-6 are ok for trunk now, if you fix the missing space
before ( in patch 5.


Ok, I'm going to land these shortly, thank you for review.




Patch 7 adds support for ASan odr indicators at compiler side.

This one can be applied incrementally once the issues reported in there
are resolved.


Yes, I'll fix the patch.



And the libtsan ABI stuff (__intercept*stat*) can be resolved incrementally
too.

Thanks.

Jakub

Re: [PATCH 0/7] Libsanitizer merge from upstream r285547.

[Jakub Jelinek]

On Mon, Nov 07, 2016 at 11:22:28AM +0300, Maxim Ostapenko wrote:
> Hi,
> 
> this patch set performs libsanitizer merge from upstream.
> 
> Patch 1 is the library merge itself.
> 
> Patch 2 is the reapplied change for SPARC by David S. Miller.
> 
> Patch 3 changes heuristic for extracting last PC from stack frame for ARM in
> fast unwind routine. More details can be found here
> (https://gcc.gnu.org/bugzilla/show_bug.cgi?id=61771).
> 
> Patch 4 replaces Jakub's fix for
> https://gcc.gnu.org/bugzilla/show_bug.cgi?id=63888 and removes
> CheckODRViolationViaPoisoning call from RegisterGlobal to avoid false
> positive odr violation reports.
> 
> Patch 5 combines necessary compiler changes.
> 
> Patch 6 adds several new tests, backported from upstream.

The patches 1-6 are ok for trunk now, if you fix the missing space
before ( in patch 5.

> Patch 7 adds support for ASan odr indicators at compiler side.

This one can be applied incrementally once the issues reported in there
are resolved.

And the libtsan ABI stuff (__intercept*stat*) can be resolved incrementally
too.

Thanks.

Jakub

Re: [PATCH 0/7] Libsanitizer merge from upstream r285547.

[Maxim Ostapenko]

On 07/11/16 12:20, Jakub Jelinek wrote:

On Mon, Nov 07, 2016 at 12:14:39PM +0300, Maxim Ostapenko wrote:

libubsan is definitely compatible.

Nice.


For libtsan we have several changes:

1) Several interceptors (34 of them) were added and __interceptor_lstat{64}
were removed.

That is bad, I think we need to readd those and perhaps just do what
lstat*/stat* do.  Weren't we solving the same thing a year ago on some other
symbol?


Yeah, that was __tls_get_addr. Actually,  *stat interceptors were moved 
from tsan to common, but it seems that lstat/lstat64 were missed. This 
should be fixed upstream, I suppose.



2) __interceptor_strchr has change in its parameters type:
__interceptor_strchr(char*, int) -> __interceptor_strchr(const char*, int)

That is not a big deal, the function is extern "C".


3) tsan's internal type __tsan::ReportDesc has several changes, but it seems
that this doesn't introduce ABI incompatibility with compiler side.

If __tsan::ReportDesc is not defined in publicly installed headers, I think
we are fine.


I don't see __tsan::ReportDesc in any tsan interface header:

$ grep -nr ReportDesc libsanitizer/tsan/tsan_interface*
$

But since tsan has weak

SANITIZER_WEAK_CXX_DEFAULT_IMPL
bool OnReport(const ReportDesc *rep, bool suppressed {
...
}

that can be overwritten by C++ application (in debugging purposes 
though), is it OK to not change libtsan version?




Jakub

Re: [PATCH 0/7] Libsanitizer merge from upstream r285547.

[Maxim Ostapenko]

On 07/11/16 12:28, Yuri Gribov wrote:

On Mon, Nov 7, 2016 at 9:20 AM, Jakub Jelinek  wrote:

On Mon, Nov 07, 2016 at 12:14:39PM +0300, Maxim Ostapenko wrote:

libubsan is definitely compatible.

Nice.


For libtsan we have several changes:

1) Several interceptors (34 of them) were added and __interceptor_lstat{64}
were removed.

That is bad, I think we need to readd those and perhaps just do what
lstat*/stat* do.  Weren't we solving the same thing a year ago on some other
symbol?


2) __interceptor_strchr has change in its parameters type:
__interceptor_strchr(char*, int) -> __interceptor_strchr(const char*, int)

That is not a big deal, the function is extern "C".


3) tsan's internal type __tsan::ReportDesc has several changes, but it seems
that this doesn't introduce ABI incompatibility with compiler side.

If __tsan::ReportDesc is not defined in publicly installed headers, I think
we are fine.

As a side note, why is it in the list of exported symbols?


Because it appears as a type of parameter of exported __tsan::OnReport 
function:


// Can be overriden by an application/test to intercept reports.
#ifdef TSAN_EXTERNAL_HOOKS
bool OnReport(const ReportDesc *rep, bool suppressed);
#else
SANITIZER_WEAK_CXX_DEFAULT_IMPL
bool OnReport(const ReportDesc *rep, bool suppressed) {
  (void)rep;
  return suppressed;
}
#endif

This function can be overridden by application for debugging purpose though.



-I

Re: [PATCH 0/7] Libsanitizer merge from upstream r285547.

[Yuri Gribov]

On Mon, Nov 7, 2016 at 9:20 AM, Jakub Jelinek  wrote:
> On Mon, Nov 07, 2016 at 12:14:39PM +0300, Maxim Ostapenko wrote:
>> libubsan is definitely compatible.
>
> Nice.
>
>> For libtsan we have several changes:
>>
>> 1) Several interceptors (34 of them) were added and __interceptor_lstat{64}
>> were removed.
>
> That is bad, I think we need to readd those and perhaps just do what
> lstat*/stat* do.  Weren't we solving the same thing a year ago on some other
> symbol?
>
>> 2) __interceptor_strchr has change in its parameters type:
>> __interceptor_strchr(char*, int) -> __interceptor_strchr(const char*, int)
>
> That is not a big deal, the function is extern "C".
>
>> 3) tsan's internal type __tsan::ReportDesc has several changes, but it seems
>> that this doesn't introduce ABI incompatibility with compiler side.
>
> If __tsan::ReportDesc is not defined in publicly installed headers, I think
> we are fine.

As a side note, why is it in the list of exported symbols?

-I

Re: [PATCH 0/7] Libsanitizer merge from upstream r285547.

[Jakub Jelinek]

On Mon, Nov 07, 2016 at 12:14:39PM +0300, Maxim Ostapenko wrote:
> libubsan is definitely compatible.

Nice.

> For libtsan we have several changes:
> 
> 1) Several interceptors (34 of them) were added and __interceptor_lstat{64}
> were removed.

That is bad, I think we need to readd those and perhaps just do what
lstat*/stat* do.  Weren't we solving the same thing a year ago on some other
symbol?

> 2) __interceptor_strchr has change in its parameters type:
> __interceptor_strchr(char*, int) -> __interceptor_strchr(const char*, int)

That is not a big deal, the function is extern "C".

> 3) tsan's internal type __tsan::ReportDesc has several changes, but it seems
> that this doesn't introduce ABI incompatibility with compiler side.

If __tsan::ReportDesc is not defined in publicly installed headers, I think
we are fine.

Jakub

Re: [PATCH 0/7] Libsanitizer merge from upstream r285547.

[Maxim Ostapenko]

On 07/11/16 11:39, Jakub Jelinek wrote:

On Mon, Nov 07, 2016 at 11:22:28AM +0300, Maxim Ostapenko wrote:

this patch set performs libsanitizer merge from upstream.

Patch 1 is the library merge itself.

Patch 2 is the reapplied change for SPARC by David S. Miller.

Patch 3 changes heuristic for extracting last PC from stack frame for ARM in
fast unwind routine. More details can be found here
(https://gcc.gnu.org/bugzilla/show_bug.cgi?id=61771).

Patch 4 replaces Jakub's fix for
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=63888 and removes
CheckODRViolationViaPoisoning call from RegisterGlobal to avoid false
positive odr violation reports.

Patch 5 combines necessary compiler changes.

Patch 6 adds several new tests, backported from upstream.

Patch 7 adds support for ASan odr indicators at compiler side.

The whole patch set was regtested/bootstrapped/ASan bootstrapped on
x86_64-unknown-linux-gnu and i386-unknown-linux-gnu.
Also, passed regression tests on arm-linux-gnueabi and aarch64-linux under
QEMU.

So, libasan.so.* is again ABI incompatible, but libtsan and libubsan stay
(hopefully) backwards ABI compatible?



libubsan is definitely compatible.
For libtsan we have several changes:

1) Several interceptors (34 of them) were added and 
__interceptor_lstat{64} were removed.

2) __interceptor_strchr has change in its parameters type:
__interceptor_strchr(char*, int) -> __interceptor_strchr(const char*, int)
3) tsan's internal type __tsan::ReportDesc has several changes, but it 
seems that this doesn't introduce ABI incompatibility with compiler side.


Full abidiff listing is attached.

So, I suppose libtsan is also compatible.

-Maxim



Jakub





Functions changes summary: 4 Removed, 3 Changed (70 filtered out), 34 Added functions
Variables changes summary: 0 Removed, 0 Changed, 0 Added variable
Function symbols changes summary: 0 Removed, 10 Added function symbols not referenced by debug info
Variable symbols changes summary: 0 Removed, 0 Added variable symbol not referenced by debug info

4 Removed functions:

  'function int __interceptor_lstat(const char*, void*)'{lstat, aliases __interceptor_lstat}
  'function int __interceptor_lstat64(const char*, void*)'{lstat64, aliases __interceptor_lstat64}
  'function int __interceptor_stat(const char*, void*)'{__interceptor_stat, aliases stat}
  'function int __interceptor_stat64(const char*, void*)'{stat64, aliases __interceptor_stat64}

34 Added functions:

  'function char* __interceptor_ctermid(char*)'{__interceptor_ctermid, aliases ctermid}
  'function int __interceptor_epoll_pwait(int, void*, int, int, void*)'{epoll_pwait, aliases __interceptor_epoll_pwait}
  'function int __interceptor_eventfd_read(int, __sanitizer::u64*)'{eventfd_read, aliases __interceptor_eventfd_read}
  'function int __interceptor_eventfd_write(int, __sanitizer::u64)'{__interceptor_eventfd_write, aliases eventfd_write}
  'function void* __interceptor_memmem(SIZE_T, SIZE_T)'{__interceptor_memmem, aliases memmem}
  'function int __interceptor_pthread_sigmask(int, const __sanitizer::__sanitizer_sigset_t*, __sanitizer::__sanitizer_sigset_t*)'{__interceptor_pthread_sigmask, aliases pthread_sigmask}
  'function SSIZE_T __interceptor_recvfrom(int, void*, SIZE_T, int, void*, int*)'{__interceptor_recvfrom, aliases recvfrom}
  'function SSIZE_T __interceptor_sendto(int, void*, SIZE_T, int, void*, int)'{__interceptor_sendto, aliases sendto}
  'function int __interceptor_sigblock(int)'{sigblock, aliases __interceptor_sigblock}
  'function int __interceptor_sigsetmask(int)'{sigsetmask, aliases __interceptor_sigsetmask}
  'function SIZE_T __interceptor_strnlen(const char*, SIZE_T)'{__interceptor_strnlen, aliases strnlen}
  'function int __interceptor_ttyname_r(int, char*, SIZE_T)'{__interceptor_ttyname_r, aliases ttyname_r}
  'function void __sanitizer_cov_trace_pc_guard_init()'{__sanitizer_cov_trace_pc_guard_init}
  'function int __sanitizer_install_malloc_and_free_hooks(void (typedef __sanitizer::uptr)*, void ()*)'{__sanitizer_install_malloc_and_free_hooks}
  'function void __sanitizer_set_report_fd(void*)'{__sanitizer_set_report_fd}
  'function void __sanitizer_symbolize_global(__sanitizer::uptr, const char*, char*, __sanitizer::uptr)'{__sanitizer_symbolize_global}
  'function void __sanitizer_symbolize_pc(__sanitizer::uptr, const char*, char*, __sanitizer::uptr)'{__sanitizer_symbolize_pc}
  'function void __sanitizer_syscall_post_impl_rt_sigaction(long int, long int, const __sanitizer::__sanitizer_kernel_sigaction_t*, __sanitizer::__sanitizer_kernel_sigaction_t*, SIZE_T)'{__sanitizer_syscall_post_impl_rt_sigaction}
  'function void __sanitizer_syscall_post_impl_sigaction(long int, long int, const __sanitizer::__sanitizer_kernel_sigaction_t*, __sanitizer::__sanitizer_kernel_sigaction_t*)'{__sanitizer_syscall_post_impl_sigaction}
  'function void 

Re: [PATCH 0/7] Libsanitizer merge from upstream r285547.

[Jakub Jelinek]

On Mon, Nov 07, 2016 at 11:22:28AM +0300, Maxim Ostapenko wrote:
> this patch set performs libsanitizer merge from upstream.
> 
> Patch 1 is the library merge itself.
> 
> Patch 2 is the reapplied change for SPARC by David S. Miller.
> 
> Patch 3 changes heuristic for extracting last PC from stack frame for ARM in
> fast unwind routine. More details can be found here
> (https://gcc.gnu.org/bugzilla/show_bug.cgi?id=61771).
> 
> Patch 4 replaces Jakub's fix for
> https://gcc.gnu.org/bugzilla/show_bug.cgi?id=63888 and removes
> CheckODRViolationViaPoisoning call from RegisterGlobal to avoid false
> positive odr violation reports.
> 
> Patch 5 combines necessary compiler changes.
> 
> Patch 6 adds several new tests, backported from upstream.
> 
> Patch 7 adds support for ASan odr indicators at compiler side.
> 
> The whole patch set was regtested/bootstrapped/ASan bootstrapped on
> x86_64-unknown-linux-gnu and i386-unknown-linux-gnu.
> Also, passed regression tests on arm-linux-gnueabi and aarch64-linux under
> QEMU.

So, libasan.so.* is again ABI incompatible, but libtsan and libubsan stay
(hopefully) backwards ABI compatible?

Jakub