[PATCH] D58379: [compiler-rt] Intercept the bcmp() function.

[Clement Courbet via Phabricator via llvm-commits]

courbet added a comment.

Thanks



================
Comment at: lib/asan/tests/asan_mem_test.cc:212
 
-
-TEST(AddressSanitizer, MemCmpOOBTest) {
+template <int(*cmpfn)(const void*, const void*, size_t)>
+void CmpOOBTestCommon() {
----------------
vitalybuka wrote:
> Maybe https://github.com/google/googletest/blob/master/googletest/docs/advanced.md#how-to-write-value-parameterized-tests instead of function?
The template is more consistent with the two other tests above. If we want to be even more consistent I can define functors:

```
class MemCmpWrapper {
 public:
  static int transfer(const void *l, const void *r, size_t size) {
    return Ident(memcmp)(l, r, size);
  }
};
```

but that seems overkill to me.


================
Comment at: lib/sanitizer_common/sanitizer_common_interceptors.inc:874
+  void *ctx;
+  COMMON_INTERCEPTOR_ENTER(ctx, bcmp, a1, a2, size);
+  if (common_flags()->intercept_memcmp) {
----------------
vitalybuka wrote:
> courbet wrote:
> > krytarowski wrote:
> > > Can we reuse code with memcmp(3)? bcmp(3) is a subset of memcmp(3).
> > Looking at `COMMON_INTERCEPTOR_ENTER` implementation for, e.g.  tsan:
> > 
> > ```
> > 
> > #define SCOPED_TSAN_INTERCEPTOR(func, ...) \
> >     SCOPED_INTERCEPTOR_RAW(func, __VA_ARGS__); \
> >     if (REAL(func) == 0) { \
> >       Printf("FATAL: ThreadSanitizer: failed to intercept %s\n", #func); \
> >       Die(); \
> >     } \
> >     if (thr->in_rtl > 1) \
> >       return REAL(func)(__VA_ARGS__); 
> > 
> > 
> > #define COMMON_INTERCEPTOR_ENTER(ctx, func, ...)      \
> >   SCOPED_TSAN_INTERCEPTOR(func, __VA_ARGS__);         \
> >   TsanInterceptorContext _ctx = {thr, caller_pc, pc}; \
> >   ctx = (void *)&_ctx;                                \
> >   (void) ctx;
> > 
> > ```
> > 
> > Given that the implementation calls `REAL(func)`, it feels preferable to call the real `bcmp` instead of real `memcmp`, even though calling memcmp does work. But again, I'm not familiar with how you guys typically approach this, so you tell me :)
> > 
> Sometimes we do macro
> But I'd prefer inline function, e.g.
> 
> int memcpy_interceptor_impl(ctx, a1, a2, size, real_fn_ptr) {
> ....
> }
I'm not sure I understand your suggestion correctly, but if you're suggesting we refactor using a function, then the issue is that REAL(x) only works with x a macro parameter, not if `x` is a function pointer:

```
# define PTR_TO_REAL(x) real_##x
...
# define REAL(x) __interception::PTR_TO_REAL(x)
```

Refactoring using a macro would be possible, but it would look quite ugly.


Repository:
  rCRT Compiler Runtime

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

https://reviews.llvm.org/D58379