WebKit Bugzilla
New
Browse
Log In
×
Sign in with GitHub
or
Remember my login
Create Account
·
Forgot Password
Forgotten password account recovery
RESOLVED FIXED
34663
Avoid crash due to Xcode 3.1 linker (radar 7070016)
https://bugs.webkit.org/show_bug.cgi?id=34663
Summary
Avoid crash due to Xcode 3.1 linker (radar 7070016)
Stephen White
Reported
2010-02-05 14:18:50 PST
[Chromium] Crash in Chrome/Mac Release test_shell
Attachments
Stack trace from test_shell crash
(3.10 KB, text/plain)
2010-02-05 14:22 PST
,
Stephen White
no flags
Details
Possible fix (NULL ptr check)
(1.31 KB, patch)
2010-02-05 14:30 PST
,
Stephen White
no flags
Details
Formatted Diff
Diff
Patch
(2.09 KB, patch)
2010-02-06 10:34 PST
,
Dimitri Glazkov (Google)
no flags
Details
Formatted Diff
Diff
Patch
(2.36 KB, patch)
2010-02-06 10:40 PST
,
Dimitri Glazkov (Google)
abarth
: review+
Details
Formatted Diff
Diff
Patch
(2.64 KB, patch)
2010-02-08 08:19 PST
,
Stephen White
dglazkov
: review+
Details
Formatted Diff
Diff
Show Obsolete
(3)
View All
Add attachment
proposed patch, testcase, etc.
Stephen White
Comment 1
2010-02-05 14:20:39 PST
This can be reproduced by building the Release test_shell, and invoking it without any arguments. Stack trace is attached.
Stephen White
Comment 2
2010-02-05 14:22:54 PST
Created
attachment 48261
[details]
Stack trace from test_shell crash
Stephen White
Comment 3
2010-02-05 14:30:35 PST
Created
attachment 48262
[details]
Possible fix (NULL ptr check)
Eric Seidel (no email)
Comment 4
2010-02-05 14:58:17 PST
Comment on
attachment 48262
[details]
Possible fix (NULL ptr check) I don't understand why deviceRGBColorSpaceRef() would ever fail.
Eric Seidel (no email)
Comment 5
2010-02-05 14:58:48 PST
I don't think deviceRGBColorSpaceRef is meant to be null checked. When is the system not going to be able to give you a color space for the device?
Stephen White
Comment 6
2010-02-05 15:02:54 PST
(In reply to
comment #5
)
> I don't think deviceRGBColorSpaceRef is meant to be null checked. When is the > system not going to be able to give you a color space for the device?
Well, the chromium canaries have been doing it for a day now, which is blocking me from rolling DEPS in Chrome:
http://build.chromium.org/buildbot/waterfall.fyi/builders/Webkit%20Mac%20(webkit.org)/builds/14302
(At least, I *think* this is the same crash. Since they don't have symbols, it's tough to tell). I do think there is a deeper problem here, but I'm not enough of a Macspert to figure it out.
Dimitri Glazkov (Google)
Comment 7
2010-02-06 10:34:00 PST
Created
attachment 48288
[details]
Patch
Dimitri Glazkov (Google)
Comment 8
2010-02-06 10:40:14 PST
Created
attachment 48289
[details]
Patch
Adam Barth
Comment 9
2010-02-06 10:43:08 PST
Comment on
attachment 48289
[details]
Patch ok. was there a perf reason for having this inline?
Adam Barth
Comment 10
2010-02-06 10:44:26 PST
Also, should we be using DEFINE_STATIC_LOCAL, or whatever that nutty macro is called?
Dimitri Glazkov (Google)
Comment 11
2010-02-06 11:04:04 PST
Landed as
http://trac.webkit.org/changeset/54465
. We should also ensure that this pattern doesn't repeat elsewhere.
Peter Kasting
Comment 12
2010-02-06 11:27:25 PST
(In reply to
comment #11
)
> We should also ensure that this pattern doesn't repeat elsewhere.
What about the function directly below the one you patched? It uses the same pattern. (The FIXME comments no longer seem applicable either maybe?)
Dimitri Glazkov (Google)
Comment 13
2010-02-06 11:37:51 PST
(In reply to
comment #10
)
> Also, should we be using DEFINE_STATIC_LOCAL, or whatever that nutty macro is > called?
No, because CGColorSpaceRef is a pointer.
Peter Kasting
Comment 14
2010-02-06 12:21:50 PST
Reopening due to at least one other function (just below the first one in the same file) using this pattern. Turns out this is tripping an Xcode linker bug in the 3.1 toolchain, radar 7070016, which is fixed in 3.2. The patch method here avoids triggering the bug. Thanks to Mark Mentovai for tracking this down.
Darin Adler
Comment 15
2010-02-07 13:08:24 PST
(In reply to
comment #14
)
> Turns out this is tripping an Xcode linker bug in the 3.1 toolchain, radar > 7070016, which is fixed in 3.2. The patch method here avoids triggering the > bug.
And we need to support Xcode 3.1?
Stephen White
Comment 16
2010-02-08 07:27:35 PST
(In reply to
comment #15
)
> (In reply to
comment #14
) > > Turns out this is tripping an Xcode linker bug in the 3.1 toolchain, radar > > 7070016, which is fixed in 3.2. The patch method here avoids triggering the > > bug. > > And we need to support Xcode 3.1?
I'm afraid so, for the time being. The chromium buildbots and canaries all use Xcode 3.1, and there are some blocking issues that prevent us switching to Xcode 3.2 just yet.
Stephen White
Comment 17
2010-02-08 08:19:40 PST
Created
attachment 48334
[details]
Patch
Dimitri Glazkov (Google)
Comment 18
2010-02-08 09:13:13 PST
Comment on
attachment 48334
[details]
Patch thanks!
Stephen White
Comment 19
2010-02-08 12:35:22 PST
Committed
r54502
: <
http://trac.webkit.org/changeset/54502
>
Note
You need to
log in
before you can comment on or make changes to this bug.
Top of Page
Format For Printing
XML
Clone This Bug