[PATCH] D43539: Add new interceptors: getttyent(3) family

Vitaly Buka via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Mon Feb 26 10:02:36 PST 2018


vitalybuka requested changes to this revision.
vitalybuka added inline comments.
This revision now requires changes to proceed.


================
Comment at: lib/sanitizer_common/sanitizer_common_interceptors.inc:6844
+  COMMON_INTERCEPTOR_ENTER(ctx, getttyent);
+  ttyent = REAL(getttyent)();
+  if (ttyent) {
----------------
vitalybuka wrote:
> struct __sanitizer_ttyent *ttyent = REAL...
> here and below
unchanged
> struct __sanitizer_ttyent *ttyent = REAL...  here and below




================
Comment at: lib/sanitizer_common/sanitizer_common_interceptors.inc:6845
+  ttyent = REAL(getttyent)();
+  if (ttyent) {
+    COMMON_INTERCEPTOR_WRITE_RANGE(ctx, ttyent, struct_ttyent_sz);
----------------
vitalybuka wrote:
> please no brackets here  and below
unchanged
> please no brackets here and below




================
Comment at: lib/sanitizer_common/sanitizer_common_interceptors.inc:6862
+}
+INTERCEPTOR(int, setttyent) {
+  void *ctx;
----------------
krytarowski wrote:
> vitalybuka wrote:
> > it does not check anything, why do you want to intercept it?
> > same for endttyent
> Disable recursive interceptors.
I don't see how this helps.
Also you already removed such empty interceptors from other patches.


================
Comment at: lib/sanitizer_common/sanitizer_common_interceptors.inc:7121
 
 #if SANITIZER_NETBSD
   COMMON_INTERCEPT_FUNCTION(__libc_mutex_lock);
----------------
krytarowski wrote:
> vitalybuka wrote:
> > Could you please convert this peace into INIT_ style?
> > As a separate patch.
> > Trivial but I can't be sure that it compiles if I do it myself.
> Do you mean, `__libc_mutex_lock` and the others bellow?
Yes.
> Do you mean, __libc_mutex_lock and the others bellow?




================
Comment at: lib/sanitizer_common/sanitizer_platform_interceptors.h:464
 #define SANITIZER_INTERCEPT_STRMODE SI_NETBSD
+#define SANITIZER_INTERCEPT_GETTTYENT SI_NETBSD
 
----------------
krytarowski wrote:
> vitalybuka wrote:
> > Why only NETBSD?
> I can assure only NetBSD. Feel free to switch it to POSIX or other combination of OSes in a new commit.
> I can assure only NetBSD. Feel free to switch it to POSIX or other combination of OSes in a new commit.

SGTM




================
Comment at: test/sanitizer_common/TestCases/NetBSD/getttyent.cc:13
+  typ = getttyent();
+  if (!typ)
+    exit(1);
----------------
vitalybuka wrote:
> test would be cleaner without 
> if (!typ) exit(1);
> 
> if it nullptr it will fail anyway at printf 
also unchanged


Repository:
  rL LLVM

https://reviews.llvm.org/D43539





More information about the llvm-commits mailing list