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
bsanitizer changes. This is subject for libsanitizer review process. Martin >From 100b723b9b7fb10dedb2154f30e1ebd6ef885ab4 Mon Sep 17 00:00:00 2001 From: marxin <mli...@suse.cz> Date: Wed, 8 Nov 2017 13:16:17 +0100 Subject: [PATCH] Add -fsanitize=pointer-{compare,subtract}. gcc/ChangeLog: 201

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
(if the lock above is released, you'd of course need to retake it for > if (GetStackVariableBeginning(right - 1, _offset2)). Yes, should be fixed now. Martin > > Jakub > >From 55e226413b9b3533b4167a4895b40f66cd665f11 Mon Sep 17 00:00:00 2001 From: marxin <mli...@suse.cz>

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
fixed. > >> + } else { >> +// check whether left is an address of a global variable >> +GlobalAddressDescription gdesc1, gdesc2; >> +if (GetGlobalAddressInformation(left, 0, )) { >> + if (GetGlobalAddressInformation(right - 1, 0, ) >> +

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
_type == kAccessTypeInside) { >> + HeapAddressDescription hdesc2; >> + valid2 = GetHeapAddressInformation(a2, 0, ); > > Again, no need for valid1/valid2. Why do you use above left and right - 1 > and here a1 and a2? Shouldn't it be alwa

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
> > Jakub > Thanks Jakub for valuable feedback. I'm sending new version where I implemented what you pointed. I also moved builtin created to FE and fixed quite some bugs I seen on postgres database. I guess I should slowly start with review process of libsanitizer change

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
> Jakub > >From c83ea69668cc6c153024d648fb8ca565f8f16025 Mon Sep 17 00:00:00 2001 From: marxin <mli...@suse.cz> Date: Thu, 5 Oct 2017 12:14:25 +0200 Subject: [PATCH] Add -fsanitize=pointer-{compare,subtract}. gcc/ChangeLog: 2017-10-06 Martin Liska &l

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
ke_stack()) { >> +bottom = fake_stack()->AddrIsInFakeStack(addr); >> +CHECK(bottom); >> + } >> + uptr aligned_addr = addr & ~(SANITIZER_WORDSIZE/8 - 1); // align addr. >> + u8 *shadow_ptr = (u8*)MemToShadow(aligned_addr); >> +

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
ather than polluting > specs for it everywhere. Agree. > > >> - if (flag_sanitize & SANITIZE_ADDRESS) >> + if (flag_sanitize & SANITIZE_ADDRESS >> + || flag_sanitize & SANITIZE_POINTER_COMPARE >> + || flag_sanitize & SANITIZE_POINT

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))) > +

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

2017-10-06 Thread Martin Liška
Hi. Adding a missing functionality mentioned and explained here: https://github.com/google/sanitizers/wiki/AddressSanitizerClangVsGCC-(5.0-vs-7.1)#feature-8 Currently it only works for heap allocated variables. I'm working on support for stack and global variables. The functionality is not