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(&asanThreadRegistry());
> +
> +    valid1 = GetStackVariableBeginning(a1, &shadow_offset1);
> +    valid2 = GetStackVariableBeginning(a2, &shadow_offset2);
> +  }
> +
> +  if (valid1 && valid2) {
> +    if (shadow_offset1 == shadow_offset2)
> +      return;
>    }
> +  else if (!valid1 && !valid2) {
> +    AsanChunkView chunk1 = FindHeapChunkByAddress(a1);
> +    AsanChunkView chunk2 = FindHeapChunkByAddress(a2);
> +    valid1 = chunk1.IsAllocated();
> +    valid2 = chunk2.IsAllocated();
> +
> +    if (valid1 && valid2) {
> +      if (chunk1.Eq(chunk2))
> +     return;
> +    }
> +    else if (!valid1 && !valid2) {
> +      uptr offset = a1 < a2 ? a2 - a1 : a1 - a2;
> +      uptr left = a1 < a2 ? a1 : a2;
> +      if (offset <= 2048) {
> +     if (__asan_region_is_poisoned (left, offset) == 0)
> +       return;
> +     else {
> +       GET_CALLER_PC_BP_SP;
> +       ReportInvalidPointerPair(pc, bp, sp, a1, a2);
> +       return;
> +     }

My assumption was that this offset/left stuff would be done
right after the if (a1 == a2) above, i.e. after the most common
case - equality comparison, handle the case of pointers close to each other
without any expensive locking and data structure lookup (also, you only need
to compute left if offset is <= 2048).  Only for larger distances, you'd
go through the other cases.  I.e. see if one or both pointers point
into a stack variable, or heap, or global registered variable.

For those 3 cases, I wonder if pairs of valid1 = ...; valid2 = ...;
is what is most efficient.  What we really want is to error out if
one pointer is valid in any of the categories, but the other is
not.  So, isn't the best general algorithm something like:
  if (a1 == a2)
    return;
  if (distance <= 2048)
    {
      if (not poisoned area)
        return;
    }
  else if (heap (a1))
    {
      if (heap (a2) && same_heap_object)
        return;
    }
  else if (stackvar (a1))
    {
      if (stackvar (a2) && same_stackvar)
        return;
    }
  else if (globalvar (a1))
    {
      if (globalvar (a2) && same_globalvar)
        return;
    }
  else
    return; /* ??? We don't know what the pointers point to, punt.
               Or perhaps try the not poisoned area check even for
               slightly larger distances (like 16K) and only punt
               for larger?  */
  error;

?  What order of the a1 tests should be done depends on how expensive
those tests are and perhaps some data gathering on real-world apps
on what pointers in the comparisons/subtracts are most common.

        Jakub

Reply via email to