Re: [PATCH] Add -fsanitize=pointer-{compare,subtract}.

2017-12-21 Thread Martin Liška
On 12/05/2017 10:27 AM, Jakub Jelinek wrote: > The most important change I've done in the testsuite was pointer-subtract-2.c > used -fsanitize=address,pointer-subtract, but the function was actually > doing pointer comparison. Guess that needs to be propagated upstream at > some point. Another

Re: [PATCH] Add -fsanitize=pointer-{compare,subtract}.

2017-12-18 Thread Martin Liška
On 12/05/2017 10:27 AM, Jakub Jelinek wrote: > On Tue, Nov 21, 2017 at 12:59:46PM +0100, Martin Liška wrote: >> On 10/16/2017 10:39 PM, Martin Liška wrote: >>> Hi. >>> >>> All nits included in mainline review request I've just done: >>> https://reviews.llvm.org/D38971 >>> >>> Martin >> >> Hi. >>

Re: [PATCH] Add -fsanitize=pointer-{compare,subtract}.

2017-12-05 Thread Jakub Jelinek
On Tue, Nov 21, 2017 at 12:59:46PM +0100, Martin Liška wrote: > On 10/16/2017 10:39 PM, Martin Liška wrote: > > Hi. > > > > All nits included in mainline review request I've just done: > > https://reviews.llvm.org/D38971 > > > > Martin > > Hi. > > There's updated version of patch where I added

Re: [PATCH] Add -fsanitize=pointer-{compare,subtract}.

2017-11-21 Thread Martin Liška
On 10/16/2017 10:39 PM, Martin Liška wrote: > Hi. > > All nits included in mainline review request I've just done: > https://reviews.llvm.org/D38971 > > Martin Hi. There's updated version of patch where I added new test-cases and it's rebased with latest version of libsanitizer changes. This

Re: [PATCH] Add -fsanitize=pointer-{compare,subtract}.

2017-10-16 Thread Martin Liška
Hi. All nits included in mainline review request I've just done: https://reviews.llvm.org/D38971 Martin

Re: [PATCH] Add -fsanitize=pointer-{compare,subtract}.

2017-10-16 Thread Jakub Jelinek
On Mon, Oct 16, 2017 at 03:38:28PM +0200, Martin Liška wrote: > --- a/libsanitizer/asan/asan_report.cc > +++ b/libsanitizer/asan/asan_report.cc > @@ -344,14 +344,70 @@ static INLINE void CheckForInvalidPointerPair(void *p1, > void *p2) { >if (!flags()->detect_invalid_pointer_pairs) return; >

Re: [PATCH] Add -fsanitize=pointer-{compare,subtract}.

2017-10-16 Thread Martin Liška
On 10/16/2017 02:21 PM, Jakub Jelinek wrote: > On Mon, Oct 16, 2017 at 01:57:59PM +0200, Martin Liška wrote: >> Agree. Do you feel that it's right moment to trigger review process of >> libsanitizer >> changes? > > Yes. We don't have that much time left to get it through... Good, I've

Re: [PATCH] Add -fsanitize=pointer-{compare,subtract}.

2017-10-16 Thread Jakub Jelinek
On Mon, Oct 16, 2017 at 01:57:59PM +0200, Martin Liška wrote: > Agree. Do you feel that it's right moment to trigger review process of > libsanitizer > changes? Yes. We don't have that much time left to get it through... > --- a/libsanitizer/asan/asan_report.cc > +++

Re: [PATCH] Add -fsanitize=pointer-{compare,subtract}.

2017-10-16 Thread Martin Liška
On 10/13/2017 03:13 PM, Jakub Jelinek wrote: > On Fri, Oct 13, 2017 at 02:53:50PM +0200, Martin Liška wrote: >> @@ -3826,6 +3827,19 @@ pointer_diff (location_t loc, tree op0, tree op1) >> pedwarn (loc, OPT_Wpointer_arith, >> "pointer to a function used in subtraction"); >> >> +

Re: [PATCH] Add -fsanitize=pointer-{compare,subtract}.

2017-10-13 Thread Jakub Jelinek
On Fri, Oct 13, 2017 at 02:53:50PM +0200, Martin Liška wrote: > @@ -3826,6 +3827,19 @@ pointer_diff (location_t loc, tree op0, tree op1) > pedwarn (loc, OPT_Wpointer_arith, >"pointer to a function used in subtraction"); > > + if (sanitize_flags_p (SANITIZE_POINTER_SUBTRACT)) >

Re: [PATCH] Add -fsanitize=pointer-{compare,subtract}.

2017-10-13 Thread Martin Liška
On 10/13/2017 01:17 PM, Jakub Jelinek wrote: > On Fri, Oct 13, 2017 at 01:01:37PM +0200, Martin Liška wrote: >> @@ -3826,6 +3827,18 @@ pointer_diff (location_t loc, tree op0, tree op1) >> pedwarn (loc, OPT_Wpointer_arith, >> "pointer to a function used in subtraction"); >> >> +

Re: [PATCH] Add -fsanitize=pointer-{compare,subtract}.

2017-10-13 Thread Jakub Jelinek
On Fri, Oct 13, 2017 at 01:01:37PM +0200, Martin Liška wrote: > @@ -3826,6 +3827,18 @@ pointer_diff (location_t loc, tree op0, tree op1) > pedwarn (loc, OPT_Wpointer_arith, >"pointer to a function used in subtraction"); > > + if (sanitize_flags_p (SANITIZE_POINTER_SUBTRACT)) >

Re: [PATCH] Add -fsanitize=pointer-{compare,subtract}.

2017-10-13 Thread Martin Liška
On 10/12/2017 01:34 PM, Jakub Jelinek wrote: > On Thu, Oct 12, 2017 at 01:13:56PM +0200, Martin Liška wrote: >> + if (a1 == a2) >> +return; >> + >> + uptr shadow_offset1, shadow_offset2; >> + bool valid1, valid2; >> + { >> +ThreadRegistryLock l(()); >> + >> +valid1 =

Re: [PATCH] Add -fsanitize=pointer-{compare,subtract}.

2017-10-12 Thread Jakub Jelinek
On Thu, Oct 12, 2017 at 01:30:34PM +0200, Martin Liška wrote: > There's one false positive I've noticed: > > $ cat /tmp/ptr-cmp.c > int > __attribute__((noinline)) > foo(char *p1, char *p2) > { > if (p2 != 0 && p1 > p2) > return 0; > > return 1; > } Guess that is an argument for

Re: [PATCH] Add -fsanitize=pointer-{compare,subtract}.

2017-10-12 Thread Jakub Jelinek
On Thu, Oct 12, 2017 at 01:13:56PM +0200, Martin Liška wrote: > + if (a1 == a2) > +return; > + > + uptr shadow_offset1, shadow_offset2; > + bool valid1, valid2; > + { > +ThreadRegistryLock l(()); > + > +valid1 = GetStackVariableBeginning(a1, _offset1); > +valid2 =

Re: [PATCH] Add -fsanitize=pointer-{compare,subtract}.

2017-10-12 Thread Martin Liška
Hi. There's one false positive I've noticed: $ cat /tmp/ptr-cmp.c int __attribute__((noinline)) foo(char *p1, char *p2) { if (p2 != 0 && p1 > p2) return 0; return 1; } int main(int argc, char **argv) { return foo(argv[0], 0); } $ gcc /tmp/ptr-cmp.c

Re: [PATCH] Add -fsanitize=pointer-{compare,subtract}.

2017-10-12 Thread Martin Liška
On 10/11/2017 04:22 PM, Jakub Jelinek wrote: > On Wed, Oct 11, 2017 at 03:36:40PM +0200, Martin Liška wrote: >>> std::swap(addr1, addr2); ? I don't see it used in any of libsanitizer >>> though, so not sure if the corresponding STL header is included. >> >> They don't use it anywhere and I had

Re: [PATCH] Add -fsanitize=pointer-{compare,subtract}.

2017-10-11 Thread Jakub Jelinek
On Wed, Oct 11, 2017 at 03:36:40PM +0200, Martin Liška wrote: > > std::swap(addr1, addr2); ? I don't see it used in any of libsanitizer > > though, so not sure if the corresponding STL header is included. > > They don't use it anywhere and I had some #include issues. That's why I did > it

Re: [PATCH] Add -fsanitize=pointer-{compare,subtract}.

2017-10-11 Thread Martin Liška
On 10/11/2017 09:37 AM, Jakub Jelinek wrote: > On Wed, Oct 11, 2017 at 07:55:44AM +0200, Martin Liška wrote: >>> Conceptually, these two instrumentations rely on address sanitization, >>> not really sure if we should supporting them for kernel sanitization (but I >>> bet it is just going to be too

Re: [PATCH] Add -fsanitize=pointer-{compare,subtract}.

2017-10-11 Thread Jakub Jelinek
On Wed, Oct 11, 2017 at 07:55:44AM +0200, Martin Liška wrote: > > Conceptually, these two instrumentations rely on address sanitization, > > not really sure if we should supporting them for kernel sanitization (but I > > bet it is just going to be too costly for kernel). > > So, we also need to

Re: [PATCH] Add -fsanitize=pointer-{compare,subtract}.

2017-10-10 Thread Martin Liška
On 10/06/2017 03:33 PM, Jakub Jelinek wrote: > On Fri, Oct 06, 2017 at 02:46:05PM +0200, Martin Liška wrote: >> + if (sanitize_comparison_p) >> +{ >> + if (is_gimple_assign (s) >> + && gimple_assign_rhs_class (s) == GIMPLE_BINARY_RHS >> + &&

Re: [PATCH] Add -fsanitize=pointer-{compare,subtract}.

2017-10-06 Thread Jakub Jelinek
On Fri, Oct 06, 2017 at 02:46:05PM +0200, Martin Liška wrote: > + if (sanitize_comparison_p) > + { > + if (is_gimple_assign (s) > + && gimple_assign_rhs_class (s) == GIMPLE_BINARY_RHS > + && POINTER_TYPE_P (TREE_TYPE (gimple_assign_rhs1 (s))) > +