[PATCH] D54837: [Sanitizer] getentropy interception

[Vitaly Buka via Phabricator via llvm-commits]

vitalybuka added inline comments.


================
Comment at: lib/sanitizer_common/sanitizer_common_interceptors.inc:7351
 
+#if SANITIZER_INTERCEPT_GETENTROPY
+INTERCEPTOR(int, getentropy, void *buffer, SIZE_T length) {
----------------
I guess you can have interceptor regardless of "check_library_exists(c getentropy"



================
Comment at: lib/sanitizer_common/sanitizer_linux.cc:166
+// unlike OpenBSD's implementation
+int internal_getentropy(void *buffer, unsigned long length) {
+  // Consumers checks already getentropy boundaries
----------------
I'd inline this into GetRandom to keep have "#if"s



================
Comment at: lib/sanitizer_common/sanitizer_linux.cc:170
+  // possible issue.
+  return internal_syscall(SYSCALL(getrandom), buffer,
+           length, 0);
----------------
so here syscall is used when we have "check_library_exists(c getentropy"
which does not looks right.
I'd recommend just to update existing hardcoded  "# define SANITIZER_USE_GETENTROPY 1"


================
Comment at: lib/sanitizer_common/sanitizer_platform_interceptors.h:522
 #define SANITIZER_INTERCEPT_GETMNTINFO SI_NETBSD
 #define SANITIZER_INTERCEPT_MI_VECTOR_HASH SI_NETBSD
+#define SANITIZER_INTERCEPT_GETENTROPY SANITIZER_USE_GETENTROPY
----------------
I guess you can have interceptor regardless of "check_library_exists(c getentropy"
So I'd define SANITIZER_INTERCEPT_GETENTROPY as usual, just a platform.


================
Comment at: test/sanitizer_common/TestCases/Posix/getentropy.cc:1
+// RUN: %clangxx -O0 -g %s -o %t && %run %t | FileCheck %s
+
----------------
do you need "-g" here?


Repository:
  rCRT Compiler Runtime

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D54837/new/

https://reviews.llvm.org/D54837