[PATCH] D35690: [Sanitizers] TSan allocator set errno on failure.

[Aleksey Shlyapnikov via Phabricator via llvm-commits]

alekseyshl added inline comments.


================
Comment at: lib/tsan/tests/unit/tsan_mman_test.cc:72
     void *p2 = user_realloc(thr, pc, p, 0);
-    EXPECT_NE(p2, (void*)0);
+    EXPECT_EQ(p2, (void*)0);
   }
----------------
alekseyshl wrote:
> dvyukov wrote:
> > dvyukov wrote:
> > > Is there a particular reason for this change? If not, then I would prefer to leave the current behavior. It worked for 5 years. We don't know what will happen with this one.
> > Note: the current behavior is correct according to both POSIX, linux man pages and C: 
> > 
> > If size is 0, either a null pointer or a unique pointer that can be successfully passed to free() shall be returned.
> > 
> > If size was equal to 0, either NULL or a pointer suitable to be passed to free() is returned.
> Wait, that's what the man I quoted in comments: "if size is equal to zero, and ptr is not NULL, then the call is equivalent to free(ptr)", so I'm freeing the pointer and it seems only natural to return NULL in this case, cause what else would I return?
> 
> What you're saying is correct for user_realloc(thr, pc, NULL, 0) case.
What I mean to say, it seems strange to return a dangling pointer and stdlib's realloc returns NULL in this case anyway.

user_realloc(thr, pc, NULL, 0) is tested first hand in this test.


================
Comment at: test/tsan/allocator_returns_null.cc:40
+// Until we figure out why errno is not properly set on windows
+// UNSUPPORTED: win32
+
----------------
dvyukov wrote:
> Are C++ tsan tests run on windows? That's surprising to me.
Yep, got carried away unifying tests with other sanitizers.


https://reviews.llvm.org/D35690