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

[Aleksey Shlyapnikov via Phabricator via llvm-commits]

alekseyshl added inline comments.


================
Comment at: lib/asan/asan_report.cc:340
+
+  /* At this point we know nothing about both a1 and a2 addresses.  */
+  return false;
----------------
Change it to // style comment.


================
Comment at: lib/asan/asan_thread.cc:369
+
+  return (uptr)shadow_ptr;
+}
----------------
alekseyshl wrote:
> It returns a pointer to one of the redzones, not to the variable beginning (as the function name suggests), right?
That was a question, actually. This function does not what its name suggests. At least, acknowledge it in the comment and explain why it is ok.


================
Comment at: lib/asan/asan_thread.h:93
 
+  // Return beginning of a stack variable in shadow memory
+  uptr GetStackFrameVariableBeginning(uptr addr);
----------------
alekseyshl wrote:
> Returns a pointer to the start of the stack variable's shadow memory.
It's marked as done, but I do not see the change.


================
Comment at: test/asan/TestCases/invalid-pointer-pairs-compare-errors.cc:17
+int
+main() {
+  /* Heap allocated memory.  */
----------------
int main() {


================
Comment at: test/asan/TestCases/invalid-pointer-pairs-compare-errors.cc:100
+  foo(&stack1, 0);
+
+  return 0;
----------------
free(heap1);


================
Comment at: test/asan/TestCases/invalid-pointer-pairs-subtract-errors.cc:44
+  // CHECK: #{{[0-9]+ .*}} in main {{.*}}invalid-pointer-pairs-subtract-errors.cc:[[@LINE+1]]
+  foo(&stack1, &global1[0]);
+  return 0;
----------------
free(heap1);
free(heap2);


================
Comment at: test/asan/TestCases/invalid-pointer-pairs-subtract-success.cc:27
+
+  /* Stack variable.  */
+  char stack[10000];
----------------
In all the tests, change /* */ comments to //


================
Comment at: test/asan/TestCases/invalid-pointer-pairs-threads.cc:7
+#include <assert.h>
+#include <pthread.h>
+#include <stdlib.h>
----------------
This test should be in test/asan/TestCases/Posix/


================
Comment at: test/asan/TestCases/invalid-pointer-pairs-threads.cc:33
+  do {
+  } while (pointers[0] == NULL || pointers[1] == NULL);
+
----------------
This and sleep in threads, use synchronization instead (for example, two barriers of count 3, barrier_assign and barrier_check, wait for _assign here and wait for _check after the pointer check).

That will be less flaky, not racy and much faster than 1 second.


https://reviews.llvm.org/D38971