[PATCH] D31457: [asan] Add strndup/__strndup interceptors if targeting linux.

[Evgeniy Stepanov via Phabricator via llvm-commits]

eugenis added inline comments.


================
Comment at: lib/msan/msan_interceptors.cc:1339
+    GET_STORE_STACK_TRACE;                              \
+    char *res = REAL(strndup)(src, size);               \
+    CopyShadowAndOrigin(res, src, size, &stack);        \
----------------
I don't like that the code tries to combine two very different implementations in one "common" interceptor - one that calls REAL(strlen) and one that uses malloc directly.

Common interceptor should implement the logic of strndup, and call individual sanitizers only to update their specific state - READ_RANGE at the start, and something like COPY_RANGE after.

I think the common implementation should use malloc directly. 



================
Comment at: lib/sanitizer_common/sanitizer_common_interceptors.inc:315
+  COMMON_INTERCEPTOR_ENTER(ctx, strndup, s, size);
+  uptr from_length = REAL(strlen)(s);
+  uptr copy_length = Min(size, from_length + 1);
----------------
this should use strnlen, like the msan interceptor it is replacing


================
Comment at: lib/sanitizer_common/tests/sanitizer_test_utils.h:127
 
+#if defined(__linux__) && !defined(__ANDROID__)
+# define SANITIZER_TEST_HAS_STRNDUP 1
----------------
filcab wrote:
> We should also enter here if `defined(__APPLE__)` and if `defined(__FreeBSD__)`, I think.
> Can't think of more OS we support that should be here.
> 
> I'm not sure we should be taking Android out of the picture, as it seems `strndup` is available, AFAICT. @kcc/@vitalybuka (or others who might be working on Android) can you confirm if we should expect `strndup` to be available?
Yes, strndup is available on Android.


https://reviews.llvm.org/D31457