This is an archive of the discontinued LLVM Phabricator instance.

Handle NetBSD symbol renaming in msan_interceptors.cc
ClosedPublic

Authored by krytarowski on Dec 2 2017, 5:33 AM.

Details

Summary

NetBSD renames symbols for historical and compat reasons.

Add required symbol renames in sanitizer_common_interceptors.inc:

  • gettimeofday -> __gettimeofday50
  • getrusage -> __getrusage50
  • shmctl -> __shmctl50

Additionally handle sigaction symbol mangling.
Rename the function symbol in the file to SIGACTION_SYMNAME and define
it as __sigaction14 for NetBSD and sigaction for !NetBSD. We cannot use
simple renaming with the proprocessor, as there are valid fields named
sigaction and they must be left intact.

Sponsored by <The NetBSD Foundation>

Diff Detail

Repository
rL LLVM

Event Timeline

krytarowski created this revision.
vitalybuka added inline comments.Dec 5 2017, 4:38 PM
lib/msan/msan_interceptors.cc
47

Why not the same for __libc_thr_keycreate?

krytarowski added inline comments.Dec 5 2017, 4:47 PM
lib/msan/msan_interceptors.cc
47

This one we are not intercepting here, but calling with REAL(). We need to use a 3rd party symbol name in order to not mangle too much, as there are struct fields named 'sigaction'.

vitalybuka added inline comments.Dec 5 2017, 5:11 PM
lib/msan/msan_interceptors.cc
47

could you please use all capitals in macro names for consistency?

maybe better place for this macros is
sanitizer_platform_limits_netbsd.h and sanitizer_platform_limits_posix.h

krytarowski added inline comments.Dec 5 2017, 6:09 PM
lib/msan/msan_interceptors.cc
47

could you please use all capitals in macro names for consistency?

Do you mean SIGACTION_SYMNAME?

vitalybuka added inline comments.Dec 5 2017, 6:10 PM
lib/msan/msan_interceptors.cc
47

yes

krytarowski edited the summary of this revision. (Show Details)
  • rename sigaction_symname to SIGACTION_SYMNAME
  • put SIGACTION_SYMNAME in sanitizer_common/sanitizer_platform_limits_posix.h and ..netbsd.h
vitalybuka accepted this revision.Dec 5 2017, 6:48 PM
vitalybuka added inline comments.
lib/msan/msan_interceptors.cc
37

I expected you remove these too. However looks like we do the same in sanitizer_common_interceptors.inc

shmctl50 already defined there. Could you just delete "#define shmctl shmctl50" and "DECLARE_REAL(int, shmctl, int shmid, int cmd, void *buf)"
and move "INTERCEPTOR(void *, shmat" to just before "INTERCEPTOR(int, dl_iterate_phdr", after sanitizer_common_interceptors.inc ?

lib/sanitizer_common/sanitizer_platform_limits_netbsd.h
579 ↗(On Diff #125664)

So we don't remove gettimeofday ang getrusage, maybe we could return this back. It's up to you.

This revision is now accepted and ready to land.Dec 5 2017, 6:48 PM
krytarowski added inline comments.Dec 6 2017, 11:34 AM
lib/msan/msan_interceptors.cc
37

It's not clear to me. shmctl is used in the shmat interceptor.

vitalybuka added inline comments.Dec 6 2017, 11:47 AM
lib/msan/msan_interceptors.cc
37

Correct. You just need to move shmat interceptor after "#include sanitizer_common_interceptors.inc"
sanitizer_common_interceptors.inc already declares READ(shmctl)

Apply changes from review.

krytarowski marked 8 inline comments as done.Dec 6 2017, 12:45 PM

I think it's done now.

vitalybuka accepted this revision.Dec 6 2017, 1:17 PM
This revision was automatically updated to reflect the committed changes.