Page MenuHomePhabricator

[sanitizer_common] Add internal_clone().
ClosedPublic

Authored by earthdok on Aug 30 2013, 7:18 AM.

Details

Summary

Add a wrapper for the clone syscall for use in StopTheWorld. We
implement it only for x86_64, so stop building StopTheWorld for other platforms
(no one uses it outside x86_64 anyway).

See https://code.google.com/p/address-sanitizer/issues/detail?id=214 for why we
can't use the glibc clone() wrapper.

Diff Detail

Event Timeline

eugenis accepted this revision.Aug 30 2013, 7:37 AM

Looks great.
Please explain the stale cache issue in the comments.

lib/sanitizer_common/sanitizer_linux_libcdep.cc
292 ↗(On Diff #3922)

add some comments explaining how this is different from the libc clone() wrapper.

(other comments discussed offline)

lib/sanitizer_common/sanitizer_linux_libcdep.cc
299 ↗(On Diff #3922)

I suggest to assert that child_stack is aligned on 16.

lib/sanitizer_common/sanitizer_stoptheworld_linux_libcdep.cc
375

/*parent_tidptr*/0, ...

glider added inline comments.Aug 30 2013, 9:15 AM
lib/sanitizer_common/sanitizer_linux_libcdep.cc
331 ↗(On Diff #3922)

_exit(%rax)

pcc requested changes to this revision.Aug 30 2013, 10:58 AM
pcc added inline comments.
lib/sanitizer_common/sanitizer_linux.h
32

Make this declaration conditional on defined(x86_64).

lib/sanitizer_common/sanitizer_linux_libcdep.cc
347 ↗(On Diff #3922)

The other syscall wrappers return a uptr with a potentially negative error value instead of setting errno. You can easily do that here as well, this would also allow you to move the implementation to sanitizer_linux.cc.

earthdok updated this revision to Unknown Object (????).Aug 30 2013, 11:38 AM
  • addressed all comments
earthdok updated this revision to Unknown Object (????).Aug 30 2013, 11:40 AM
  • eliminated a file with whitespace-only changes
glider accepted this revision.Sep 2 2013, 4:30 AM

LGTM with a nit.

lib/sanitizer_common/sanitizer_linux.cc
682

s/taks/task

Committed r189753

earthdok closed this revision.Dec 5 2014, 9:40 AM