Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Some programs hang because of the replaced CFAllocator #10

Closed
ramosian-glider opened this issue Aug 31, 2015 · 14 comments
Closed

Some programs hang because of the replaced CFAllocator #10

ramosian-glider opened this issue Aug 31, 2015 · 14 comments

Comments

@ramosian-glider
Copy link
Member

Originally reported on Google Code with ID 10

=====================================================================
#import <Foundation/Foundation.h>
#import <WebKit/WebKit.h>

@interface WebKitDelegate : NSObject
@end

@implementation WebKitDelegate

- (void)webView:(WebView *)sender didFinishLoadForFrame:(WebFrame *)frame
{
    NSLog(@"didFinishLoadForFrame");
    [sender displayIfNeeded];
    if (frame == [sender mainFrame])
        exit(0);
}

- (NSURLRequest *)webView:(WebView *)sender resource:(id)identifier willSendRequest:(NSURLRequest
*)request redirectResponse:(NSURLResponse *)redirectResponse fromDataSource:(WebDataSource
*)dataSource;
{
    NSLog(@"Will send request %@ (redirect %@)",request,redirectResponse);
    return request;
}

- (void)webView:(WebView *)sender resource:(id)identifier didReceiveResponse:(NSURLResponse
*)response fromDataSource:(WebDataSource *)dataSource
{
    NSLog(@"Got response %@",response);
}
@end

int main(int argc, char * const argv[])
{
    //// CFAllocatorSetDefault(kCFAllocatorMallocZone);
    NSAutoreleasePool* pool = [[NSAutoreleasePool alloc] init];
    WebView *webView = [[WebView alloc] initWithFrame:NSMakeRect(0, 0, 800, 600) frameName:nil
groupName:@"org.webkit.parseWebKit"];
    WebKitDelegate* delegate = [[WebKitDelegate alloc] init];
    [webView setFrameLoadDelegate:delegate];
    [webView setResourceLoadDelegate:delegate];
    [[webView mainFrame] loadHTMLString:@"<html><body>test</body></html>" baseURL:nil];
    [[NSRunLoop currentRunLoop] run];
    [pool drain];
    return 0;
}
=====================================================================

The program above hangs under ASan. If the "CFAllocatorSetDefault" line is uncommented,
the program hangs without ASan.

Reported by [email protected] on 2011-11-21 09:12:15

@ramosian-glider
Copy link
Member Author

While the problem is caused by a subtle bug in CoreFoundation, this may make ASan unusable
for some applications.
A temporary solution is add a flag to disable replacing the default CFAllocator:

  $ ASAN_OPTIONS="replace_cfallocator=0" asan/Release/parseWebKit

I haven't observed such problems with Chromium, thus replace_cfallocator is 1 by default.

Reported by [email protected] on 2011-11-21 09:17:37

@ramosian-glider
Copy link
Member Author

I've added the flag in r1084.

Reported by ramosian.glider on 2011-11-21 09:33:23

  • Status changed: Accepted

@ramosian-glider
Copy link
Member Author

Reported by [email protected] on 2012-01-19 19:25:05

  • Blocking: #110589

@ramosian-glider
Copy link
Member Author

Braden Thomas supposes that the problem may be caused by CFStringCreateCopy which normally
does not copy constant strings, but does so if the allocator is replaced:
=================================================
$ cat t.mm
#import <Foundation/Foundation.h>
#include <stdio.h>

int main() {
#ifdef REPLACE
  CFAllocatorSetDefault(kCFAllocatorMallocZone);
#endif
  CFStringRef str = CFSTR("Hello world!\n");
  CFStringRef str2 = CFStringCreateCopy(0, str);
  fprintf(stderr, "str: %p\n", str);
  fprintf(stderr, "str2: %p\n", str2);
  return 0;
}


$ ../../../../build/Release+Asserts/bin/clang++   t.mm -framework Foundation -o t 
&& ./t
str: 0x100001060
str2: 0x100001060


$ ../../../../build/Release+Asserts/bin/clang++   t.mm -framework Foundation -DREPLACE
-o t  && ./t
str: 0x100001070
str2: 0x1001099d8
=================================================

If so, we can try to intercept CFStringCreateCopy and make it leave constant strings
as is

Reported by ramosian.glider on 2012-01-20 15:35:41

@ramosian-glider
Copy link
Member Author

At least the initial WebKit example and the program from comment 4 behave correctly
if I wrap CFStringCreateCopy() into a function that checks for the string constness
before comparing the allocators.

Original CFStringCreateCopy() implementation from http://opensource.apple.com/source/CF/CF-476.19/CFString.c
:
===========================================
CFStringRef CFStringCreateWithSubstring(CFAllocatorRef alloc, CFStringRef str, CFRange
range) {
//      CF_OBJC_FUNCDISPATCH1(__kCFStringTypeID, CFStringRef , str, "_createSubstringWithRange:",
CFRangeMake(range.location, range.length));

    __CFAssertIsString(str);
    __CFAssertRangeIsInStringBounds(str, range.location, range.length);

    if ((range.location == 0) && (range.length == __CFStrLength(str))) {    /* The substring
is the whole string... */
    return (CFStringRef)CFStringCreateCopy(alloc, str);
    } else if (__CFStrIsEightBit(str)) {
    const uint8_t *contents = (const uint8_t *)__CFStrContents(str);
        return __CFStringCreateImmutableFunnel3(alloc, contents + range.location +
__CFStrSkipAnyLengthByte(str), range.length, __CFStringGetEightBitStringEncoding(),
false, false, false, false, false, ALLOCATORSFREEFUNC, 0);
    } else {
    const UniChar *contents = (UniChar *)__CFStrContents(str);
        return __CFStringCreateImmutableFunnel3(alloc, contents + range.location, range.length
* sizeof(UniChar), kCFStringEncodingUnicode, false, true, false, false, false, ALLOCATORSFREEFUNC,
0);
    }
}
===========================================

My wrapper:

545 extern "C"
546 CFStringRef WRAP(CFStringCreateCopy)(CFAllocatorRef alloc, CFStringRef str) {
547   if (__CFStrIsConstant(str)) {
548     return str;
549   } else {
550     return real_CFStringCreateCopy(alloc, str);
551   }
552 }

Reported by ramosian.glider on 2012-01-20 16:12:42

@ramosian-glider
Copy link
Member Author

BookmarkAllTabsControllerTest.BookmarkAllTabs (http://code.google.com/p/chromium/issues/detail?id=110589)
does not fail anymore with this fix, neither does any of the unit_tests.
Moreover, the problem with blank omnibox in Chromium built with ASan disappears too.

I'm going to land the wrapper and propose an upstream fix for CFStringCreateCopy.

Reported by ramosian.glider on 2012-01-23 08:58:58

@ramosian-glider
Copy link
Member Author

FTR, this is how omnibox used to behave for me:

> One of the problems I'm facing is that the Omnibox in my build is
> broken: it remains empty while I type in the address (my query appears
> in the drop-down list of suggestions). When I hit enter, the text
> appears, but it is gray instead of black.

Reported by ramosian.glider on 2012-01-23 09:09:02

@ramosian-glider
Copy link
Member Author

As of r148696 the problem does not occur anymore.
I'm going to close the bug after I remove the replace_cfallocator flag, which will
happen after the Apple folks confirm everything is correct.

Reported by ramosian.glider on 2012-01-23 10:51:05

@ramosian-glider
Copy link
Member Author

can this be closed? 

Reported by konstantin.s.serebryany on 2012-02-24 22:57:00

@ramosian-glider
Copy link
Member Author

I haven't received any feedback from Apple about this, so let's keep it for some time,
ok? We need to remove the flag once we're sure everything is all right.

Reported by ramosian.glider on 2012-02-28 11:01:39

@ramosian-glider
Copy link
Member Author

Reported by konstantin.s.serebryany on 2012-05-22 08:39:25

  • Labels added: OpSys-OSX

@ramosian-glider
Copy link
Member Author

We've decided to keep this flag for now, since there are problems with CFAllocator on
other OS X versions.

Reported by ramosian.glider on 2012-05-22 08:45:12

@ramosian-glider
Copy link
Member Author

This has been fixed by the recent switch to the dynamic runtime, which does not replace
CFAllocator.
I've removed the replace_cfallocator flag.

Reported by ramosian.glider on 2013-02-07 16:00:04

  • Status changed: Fixed

@ramosian-glider
Copy link
Member Author

Adding Project:AddressSanitizer as part of GitHub migration.

Reported by ramosian.glider on 2015-07-30 09:12:57

  • Labels added: ProjectAddressSanitizer

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

1 participant