[PATCH] D38971: Enhance libsanitizer support for invalid-pointer-pair.

[Aleksey Shlyapnikov via Phabricator via llvm-commits]

alekseyshl added a comment.

Very sorry for the delay!



================
Comment at: lib/asan/asan_descriptions.cc:344
+    for (uptr j = 0; j < other.size; j++) {
+      __asan_global a = globals[i];
+      __asan_global b = other.globals[j];
----------------
Move it to the outer loop


================
Comment at: lib/asan/asan_descriptions.cc:345
+      __asan_global a = globals[i];
+      __asan_global b = other.globals[j];
+
----------------
const __asan_global &b and the same for a


================
Comment at: lib/asan/asan_descriptions.h:149
+
+  // Returns true when this descriptions points inside a same global variable
+  // as other. Descriptions can have different address within the variable
----------------
this description points inside the same


================
Comment at: lib/asan/asan_report.cc:307
+
+  uptr offset = a1 < a2 ? a2 - a1 : a1 - a2;
+  uptr left = a1 < a2 ? a1 : a2;
----------------
uptr offset = right - left;


================
Comment at: lib/asan/asan_report.cc:319
+    return (shadow_offset2 = t->GetStackFrameVariableBeginning(right)) == 0 ||
+      shadow_offset1 != shadow_offset2;
+
----------------
What are we trying to save here? Why not just being explicit:
  if (uptr shadow_offset_left = t->GetStackFrameVariableBeginning(left)) {
    uptr shadow_offset_right = t->GetStackFrameVariableBeginning(right);
    return shadow_offset_right == 0 || shadow_offset_left != shadow_offset_right;
  }


================
Comment at: lib/asan/asan_report.cc:332
+  if (GetGlobalAddressInformation(left, 0, &gdesc1))
+    return GetGlobalAddressInformation(right - 1, 0, &gdesc2) == 0 ||
+        !gdesc1.PointsInsideTheSameVariable(gdesc2);
----------------
I wonder why "right - 1"?


================
Comment at: lib/asan/asan_thread.cc:369
+
+  return (uptr)shadow_ptr;
+}
----------------
It returns a pointer to one of the redzones, not to the variable beginning (as the function name suggests), right?


================
Comment at: lib/asan/asan_thread.h:93
 
+  // Return beginning of a stack variable in shadow memory
+  uptr GetStackFrameVariableBeginning(uptr addr);
----------------
Returns a pointer to the start of the stack variable's shadow memory.


================
Comment at: lib/asan/asan_thread.h:94
+  // Return beginning of a stack variable in shadow memory
+  uptr GetStackFrameVariableBeginning(uptr addr);
+
----------------
GetStackVariableShadowStart


https://reviews.llvm.org/D38971