[PATCH] D56157: [sanitizer_common] Implement popen, popenve, pclose interceptors

[Dmitry Vyukov via Phabricator via cfe-commits]

dvyukov added inline comments.


================
Comment at: lib/tsan/rtl/tsan_interceptors.cc:2259
 
+#define COMMON_INTERCEPTOR_PIPE_OPEN(ctx, file) \
+  if (file) {                                   \
----------------
krytarowski wrote:
> dvyukov wrote:
> > An alternative would be to pass NULL to COMMON_INTERCEPTOR_FILE_OPEN and then do:
> > 
> >   if (path)
> >     Acquire(thr, pc, File2addr(path))
> > 
> > Just to not multiply entities. But neither approach looks strictly better to me, so I am not too strong about it.
> Is `File2addr()` producing any useful result?
> 
> Reusing `COMMON_INTERCEPTOR_FILE_OPEN` looks fine.
> Is File2addr() producing any useful result?

What exactly do you mean?
It produces a degenerate, but useful result. The original idea was that it can return different results for different files, depending on io_sync flag value. E.g. not synchronizing deletion of one file with open of another file. But this gets tricky with links, bind mounts, etc and was never implemented.
But at least if a file is not involved at all, then we need to skip the Acquire.


Repository:
  rCRT Compiler Runtime

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

https://reviews.llvm.org/D56157