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

[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
> +++ b/libsanitizer/asan/asan_report.cc
> +  {
> +    uptr shadow_offset1, shadow_offset2;
> +    ThreadRegistryLock l(&asanThreadRegistry());
> +
> +    // check whether left is a stack memory pointer
> +    if (GetStackVariableBeginning(left, &shadow_offset1)) {
> +      if (GetStackVariableBeginning(right - 1, &shadow_offset2) &&
> +	  shadow_offset1 == shadow_offset2)
> +	return;
> +      else
> +	goto do_error;
> +    }

Do you need the ThreadRegistryLock for the following calls?
If not, the } should be closed and it should be reindented.

> +    // check whether left is a heap memory address
> +    HeapAddressDescription hdesc1, hdesc2;
> +    if (GetHeapAddressInformation(left, 0, &hdesc1) &&
> +	hdesc1.chunk_access.access_type == kAccessTypeInside) {
> +      if (GetHeapAddressInformation(right, 0, &hdesc2) &&
> +	  hdesc2.chunk_access.access_type == kAccessTypeInside &&
> +	  (hdesc1.chunk_access.chunk_begin
> +	   == hdesc2.chunk_access.chunk_begin))
> +	return;
> +      else
> +	goto do_error;
> +    }
> +    // check whether left is an address of a global variable
> +    GlobalAddressDescription gdesc1, gdesc2;
> +    if (GetGlobalAddressInformation(left, 0, &gdesc1)) {
> +      if (GetGlobalAddressInformation(right - 1, 0, &gdesc2) &&
> +	  gdesc1.PointsInsideASameVariable (gdesc2))
> +	return;
> +    } else
> +      goto do_error;

??  If we don't know anything about the left object, doing a goto do_error;
sounds dangerous, it could be say mmapped region or whatever else.

Though, we could at that spot at least check if right isn't one
of the 3 kinds of regions we track and if yes, error out.
So perhaps move the else goto do_error; inside of the {} and
do
  if (GetStackVariableBeginning(right - 1, &shadow_offset2) ||
      GetHeapAddressInformation(right, 0, &hdesc2) ||
      GetGlobalAddressInformation(right - 1, 0, &gdesc2))
    goto do_error;
  return;
(if the lock above is released, you'd of course need to retake it for
if (GetStackVariableBeginning(right - 1, &shadow_offset2)).

	Jakub