[llvm-commits] [ASan]: get rid of CoreFoundation header in interceptors on Mac (issue 5848068)
samsonov at google.com
samsonov at google.com
Tue Mar 20 23:00:13 PDT 2012
On 2012/03/20 16:35:55, glider wrote:
> Then we might be doing the wrong thing.
> asan_mac.h should contain interfaces provided by asan_mac.cc that are
> to be used by other modules. Common interfaces should be in other,
> common .h files.
> Maybe we just do not want asan_interfaces to include asan_mac.h?
Currently it is the only inclusion of this header (except asan_mac.cc),
and this
header barely contains any interfaces.
> On Tue, Mar 20, 2012 at 8:27 PM, Kostya Serebryany
<mailto:kcc at google.com> wrote:
> >
> >
> > On Tue, Mar 20, 2012 at 5:54 AM, Alexey Samsonov
<mailto:samsonov at google.com>
> > wrote:
> >>
> >> On Tue, Mar 20, 2012 at 4:52 PM, Alexander Potapenko
<mailto:glider at google.com>
> >> wrote:
> >>>
> >>> I see no sense in removing Mac-specific headers from asan_mac.h
> >>
> >>
> >> asan_mac.h is included in asan_interceptors.h, I think we may want
to get
> >> rid of it completely.
> >
> >
> > agree. We may keep mac-specific headers in *_mac.cc, but should not
use them
> > in any of the header files included into non-mac .cc files.
> >
> >>
> >>
> >>>
> >>>
> >>> On Tue, Mar 20, 2012 at 3:47 PM, mailto: <samsonov at google.com>
wrote:
> >>> > Reviewers: ramosian.glider, kcc,
> >>> >
> >>> > Description:
> >>> > [ASan]: get rid of CoreFoundation header in interceptors on Mac
> >>> >
> >>> > Please review this at http://codereview.appspot.com/5848068/
> >>> >
> >>> > Affected files:
> >>> > M asan_interceptors.cc
> >>> > M asan_mac.cc
> >>> > M asan_mac.h
> >>> >
> >>> >
> >>> > Index: asan_mac.h
> >>> >
===================================================================
> >>> > --- asan_mac.h (revision 153085)
> >>> > +++ asan_mac.h (working copy)
> >>> > @@ -18,8 +18,6 @@
> >>> >
> >>> > #include "asan_interceptors.h"
> >>> >
> >>> > -#include <CoreFoundation/CFString.h>
> >>> > -
> >>> > typedef void* pthread_workqueue_t;
> >>> > typedef void* pthread_workitem_handle_t;
> >>> >
> >>> > @@ -29,6 +27,10 @@
> >>> > typedef void (*dispatch_function_t)(void *block);
> >>> > typedef void* (*worker_t)(void *block);
> >>> >
> >>> > +namespace __asan {
> >>> > +void PatchCFStringCreateCopy();
> >>> > +} // namespace __asan
> >>> > +
> >>> > DECLARE_REAL_AND_INTERCEPTOR(void, dispatch_async_f,
dispatch_queue_t
> >>> > dq,
> >>> >
void *ctxt,
> >>> >
> >>> > dispatch_function_t
> >>> > func);
> >>> > @@ -54,9 +56,6 @@
> >>> >
void *
workitem_arg,
> >>> >
pthread_workitem_handle_t *
> >>> > itemhandlep,
> >>> >
unsigned
int *gencountp);
> >>> > -DECLARE_REAL_AND_INTERCEPTOR(CFStringRef, CFStringCreateCopy,
> >>> > -
CFAllocatorRef alloc,
> >>> > -
CFStringRef str);
> >>> >
> >>> > // A wrapper for the ObjC blocks used to support
libdispatch.
> >>> > typedef struct {
> >>> > Index: asan_interceptors.cc
> >>> >
===================================================================
> >>> > --- asan_interceptors.cc (revision
153085)
> >>> > +++ asan_interceptors.cc (working
copy)
> >>> > @@ -720,14 +720,8 @@
> >>> >
CHECK(INTERCEPT_FUNCTION(dispatch_barrier_async_f));
> >>> > CHECK(INTERCEPT_FUNCTION(dispatch_group_async_f));
> >>> >
> >>> > - // Normally CFStringCreateCopy should not copy constant
CF strings.
> >>> > - // Replacing the default CFAllocator causes constant
strings to be
> >>> > copied
> >>> > - // rather than just returned, which leads to bugs in big
> >>> > applications
> >>> > like
> >>> > - // Chromium and WebKit, see
> >>> > //
http://code.google.com/p/address-sanitizer/issues/detail?id=10
> >>> > - // Until this problem is fixed we need to check that the
string is
> >>> > - // non-constant before calling CFStringCreateCopy.
> >>> > - CHECK(INTERCEPT_FUNCTION(CFStringCreateCopy));
> >>> > + PatchCFStringCreateCopy();
> >>> > #endif
> >>> >
> >>> > if (FLAG_v > 0) {
> >>> > Index: asan_mac.cc
> >>> >
===================================================================
> >>> > --- asan_mac.cc (revision 153084)
> >>> > +++ asan_mac.cc (working copy)
> >>> > @@ -16,6 +16,7 @@
> >>> >
> >>> > #include "asan_mac.h"
> >>> >
> >>> > +#include "asan_interceptors.h"
> >>> > #include "asan_internal.h"
> >>> > #include "asan_mapping.h"
> >>> > #include "asan_procmaps.h"
> >>> > @@ -34,6 +35,7 @@
> >>> > #include <fcntl.h>
> >>> > #include <unistd.h>
> >>> > #include <libkern/OSAtomic.h>
> >>> > +#include <CoreFoundation/CFString.h>
> >>> >
> >>> > namespace __asan {
> >>> >
> >>> > @@ -610,4 +612,17 @@
> >>> > }
> >>> > }
> >>> >
> >>> > +namespace __asan {
> >>> > +void PatchCFStringCreateCopy() {
> >>> > + // Normally CFStringCreateCopy should not copy constant
CF strings.
> >>> > + // Replacing the default CFAllocator causes constant
strings to be
> >>> > copied
> >>> > + // rather than just returned, which leads to bugs in big
> >>> > applications
> >>> > like
> >>> > + // Chromium and WebKit, see
> >>> > + //
http://code.google.com/p/address-sanitizer/issues/detail?id=10
> >>> > + // Until this problem is fixed we need to check that the
string is
> >>> > + // non-constant before calling CFStringCreateCopy.
> >>> > + CHECK(INTERCEPT_FUNCTION(CFStringCreateCopy));
> >>> > +}
> >>> > +} // namespace __asan
> >>> > +
> >>> > #endif // __APPLE__
> >>> >
> >>> >
> >>> > _______________________________________________
> >>> > llvm-commits mailing list
> >>> > mailto:llvm-commits at cs.uiuc.edu
> >>> > http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits
> >>>
> >>>
> >>>
> >>> --
> >>> Alexander Potapenko
> >>> Software Engineer
> >>> Google Moscow
> >>
> >>
> >>
> >>
> >> --
> >> Alexey Samsonov
> >> Software Engineer, Moscow
> >> mailto:samsonov at google.com
> >>
> >>
> >> _______________________________________________
> >> llvm-commits mailing list
> >> mailto:llvm-commits at cs.uiuc.edu
> >> http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits
> >>
> >
> --
> Alexander Potapenko
> Software Engineer
> Google Moscow
http://codereview.appspot.com/5848068/
More information about the llvm-commits
mailing list