Issue 5848068: [ASan]: get rid of CoreFoundation header in interceptors on Mac (Closed)
|
Created:
12 years, 1 month ago by samsonov Modified:
12 years, 1 month ago CC:
llvm-commits_cs.uiuc.edu Base URL:
https://llvm.org/svn/llvm-project/compiler-rt/trunk/lib/asan/ Visibility:
Public. |
Description[ASan]: get rid of CoreFoundation header in interceptors on Mac
Patch Set 1 #
MessagesTotal messages: 7
I see no sense in removing Mac-specific headers from asan_mac.h On Tue, Mar 20, 2012 at 3:47 PM, <samsonov@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 > llvm-commits@cs.uiuc.edu > http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits -- Alexander Potapenko Software Engineer Google Moscow
Sign in to reply to this message.
On Tue, Mar 20, 2012 at 4:52 PM, Alexander Potapenko <glider@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. > > On Tue, Mar 20, 2012 at 3:47 PM, <samsonov@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 > > llvm-commits@cs.uiuc.edu > > http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits > > > > -- > Alexander Potapenko > Software Engineer > Google Moscow > -- Alexey Samsonov Software Engineer, Moscow samsonov@google.com
Sign in to reply to this message.
On Tue, Mar 20, 2012 at 5:54 AM, Alexey Samsonov <samsonov@google.com>wrote: > On Tue, Mar 20, 2012 at 4:52 PM, Alexander Potapenko <glider@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, <samsonov@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 >> > llvm-commits@cs.uiuc.edu >> > http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits >> >> >> >> -- >> Alexander Potapenko >> Software Engineer >> Google Moscow >> > > > > -- > Alexey Samsonov > Software Engineer, Moscow > samsonov@google.com > > > _______________________________________________ > llvm-commits mailing list > llvm-commits@cs.uiuc.edu > http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits > >
Sign in to reply to this message.
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? On Tue, Mar 20, 2012 at 8:27 PM, Kostya Serebryany <kcc@google.com> wrote: > > > On Tue, Mar 20, 2012 at 5:54 AM, Alexey Samsonov <samsonov@google.com> > wrote: >> >> On Tue, Mar 20, 2012 at 4:52 PM, Alexander Potapenko <glider@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, <samsonov@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 >>> > llvm-commits@cs.uiuc.edu >>> > http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits >>> >>> >>> >>> -- >>> Alexander Potapenko >>> Software Engineer >>> Google Moscow >> >> >> >> >> -- >> Alexey Samsonov >> Software Engineer, Moscow >> samsonov@google.com >> >> >> _______________________________________________ >> llvm-commits mailing list >> llvm-commits@cs.uiuc.edu >> http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits >> > -- Alexander Potapenko Software Engineer Google Moscow
Sign in to reply to this message.
On Tue, Mar 20, 2012 at 9:35 AM, Alexander Potapenko <glider@google.com>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? > That's another option, but then we should put a guard into asan_mac.h. #ifndef APPLE #error > On Tue, Mar 20, 2012 at 8:27 PM, Kostya Serebryany <kcc@google.com> wrote: > > > > > > On Tue, Mar 20, 2012 at 5:54 AM, Alexey Samsonov <samsonov@google.com> > > wrote: > >> > >> On Tue, Mar 20, 2012 at 4:52 PM, Alexander Potapenko <glider@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, <samsonov@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 > >>> > llvm-commits@cs.uiuc.edu > >>> > http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits > >>> > >>> > >>> > >>> -- > >>> Alexander Potapenko > >>> Software Engineer > >>> Google Moscow > >> > >> > >> > >> > >> -- > >> Alexey Samsonov > >> Software Engineer, Moscow > >> samsonov@google.com > >> > >> > >> _______________________________________________ > >> llvm-commits mailing list > >> llvm-commits@cs.uiuc.edu > >> http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits > >> > > > > > > -- > Alexander Potapenko > Software Engineer > Google Moscow >
Sign in to reply to this message.
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@google.com> wrote: > > > > > > On Tue, Mar 20, 2012 at 5:54 AM, Alexey Samsonov <mailto:samsonov@google.com> > > wrote: > >> > >> On Tue, Mar 20, 2012 at 4:52 PM, Alexander Potapenko <mailto:glider@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@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@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@google.com > >> > >> > >> _______________________________________________ > >> llvm-commits mailing list > >> mailto:llvm-commits@cs.uiuc.edu > >> http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits > >> > > > > > > -- > Alexander Potapenko > Software Engineer > Google Moscow
Sign in to reply to this message.
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
