WebKit Bugzilla
New
Browse
Log In
×
Sign in with GitHub
or
Remember my login
Create Account
·
Forgot Password
Forgotten password account recovery
RESOLVED FIXED
65944
Crash in QuotesData::operator== when compiled with RVCT
https://bugs.webkit.org/show_bug.cgi?id=65944
Summary
Crash in QuotesData::operator== when compiled with RVCT
Yong Li
Reported
2011-08-09 14:35:53 PDT
"if (!this || !&other)" in QuotesData::operator==() doesn't generate expected code when being compiled with RVCT. This can cause crashes
Attachments
the patch
(4.77 KB, patch)
2011-08-09 14:56 PDT
,
Yong Li
yong.li.webkit
: review-
gyuyoung.kim
: commit-queue-
Details
Formatted Diff
Diff
updated
(4.77 KB, patch)
2011-08-10 07:10 PDT
,
Yong Li
no flags
Details
Formatted Diff
Diff
Fix style issue
(4.87 KB, patch)
2011-08-10 07:36 PDT
,
Yong Li
no flags
Details
Formatted Diff
Diff
Show Obsolete
(2)
View All
Add attachment
proposed patch, testcase, etc.
Yong Li
Comment 1
2011-08-09 14:56:39 PDT
Created
attachment 103403
[details]
the patch
WebKit Review Bot
Comment 2
2011-08-09 14:58:36 PDT
Attachment 103403
[details]
did not pass style-queue: Failed to run "['Tools/Scripts/check-webkit-style', '--diff-files', u'Source/WebCore/ChangeLog', u'Source/WebCor..." exit_code: 1 Source/WebCore/rendering/style/QuotesData.h:37: The parameter name "a" adds no information, so it should be removed. [readability/parameter_name] [5] Total errors found: 1 in 6 files If any of these errors are false positives, please file a bug against check-webkit-style.
Gyuyoung Kim
Comment 3
2011-08-09 15:02:16 PDT
Comment on
attachment 103403
[details]
the patch
Attachment 103403
[details]
did not pass efl-ews (efl): Output:
http://queues.webkit.org/results/9340314
WebKit Review Bot
Comment 4
2011-08-09 15:02:18 PDT
Comment on
attachment 103403
[details]
the patch
Attachment 103403
[details]
did not pass chromium-ews (chromium-xvfb): Output:
http://queues.webkit.org/results/9338347
WebKit Review Bot
Comment 5
2011-08-09 15:12:08 PDT
Comment on
attachment 103403
[details]
the patch
Attachment 103403
[details]
did not pass mac-ews (mac): Output:
http://queues.webkit.org/results/9332544
Gustavo Noronha (kov)
Comment 6
2011-08-09 15:13:02 PDT
Comment on
attachment 103403
[details]
the patch
Attachment 103403
[details]
did not pass gtk-ews (gtk): Output:
http://queues.webkit.org/results/9340320
WebKit Review Bot
Comment 7
2011-08-09 15:24:46 PDT
Comment on
attachment 103403
[details]
the patch
Attachment 103403
[details]
did not pass cr-mac-ews (chromium): Output:
http://queues.webkit.org/results/9338352
Early Warning System Bot
Comment 8
2011-08-09 16:19:09 PDT
Comment on
attachment 103403
[details]
the patch
Attachment 103403
[details]
did not pass qt-ews (qt): Output:
http://queues.webkit.org/results/9337415
Yong Li
Comment 9
2011-08-10 07:10:20 PDT
Created
attachment 103484
[details]
updated
WebKit Review Bot
Comment 10
2011-08-10 07:12:02 PDT
Attachment 103484
[details]
did not pass style-queue: Failed to run "['Tools/Scripts/check-webkit-style', '--diff-files', u'Source/WebCore/ChangeLog', u'Source/WebCor..." exit_code: 1 Source/WebCore/rendering/style/QuotesData.h:37: The parameter name "a" adds no information, so it should be removed. [readability/parameter_name] [5] Total errors found: 1 in 6 files If any of these errors are false positives, please file a bug against check-webkit-style.
Yong Li
Comment 11
2011-08-10 07:36:10 PDT
Created
attachment 103489
[details]
Fix style issue No more "a"
Antonio Gomes
Comment 12
2011-08-10 07:43:44 PDT
Comment on
attachment 103489
[details]
Fix style issue View in context:
https://bugs.webkit.org/attachment.cgi?id=103489&action=review
> Source/WebCore/rendering/style/QuotesData.h:37 > - bool operator==(const QuotesData&) const; > void operator delete(void* p) { delete[] static_cast<char*>(p); } > + static bool equal(const QuotesData*, const QuotesData*);
Should we add a comment about avoiding "==" so no one tries to get it back in?
Yong Li
Comment 13
2011-08-10 08:02:24 PDT
(In reply to
comment #12
)
> (From update of
attachment 103489
[details]
) > View in context:
https://bugs.webkit.org/attachment.cgi?id=103489&action=review
> > > Source/WebCore/rendering/style/QuotesData.h:37 > > - bool operator==(const QuotesData&) const; > > void operator delete(void* p) { delete[] static_cast<char*>(p); } > > + static bool equal(const QuotesData*, const QuotesData*); > > Should we add a comment about avoiding "==" so no one tries to get it back in?
operator== is not a problem. The problem is the old operator== is bad written. It checks "this" pointer and the address of a reference (QuotesData&), and expects to work with null "this" and a reference deferencing a null pointer. If someone in the future wants to write a normal operator==, and doesn't use operator== on null pointers, it should be fine.
Alexey Proskuryakov
Comment 14
2011-08-10 15:46:05 PDT
> The problem is the old operator== is bad written
Are you sure that the blame is pointed correctly here? Seems like a bug in this particular compiler to me.
Yong Li
Comment 15
2011-08-11 07:14:47 PDT
(In reply to
comment #14
)
> > The problem is the old operator== is bad written > > Are you sure that the blame is pointed correctly here? Seems like a bug in this particular compiler to me.
This was discussed @
https://bugs.webkit.org/show_bug.cgi?id=36901
from [Daniel Bates]: By 8.3.2 (5) of the C++ standard <
http://www.open-std.org/jtc1/sc22/wg21/docs/papers/2010/n3092.pdf
>: A reference shall be initialized to refer to a valid object or function. [ Note: in particular, a null reference cannot exist in a well-defined program, because the only way to create such a reference would be to bind it to the “object” obtained by dereferencing a null pointer, which causes undefined behavior....] RVCT uses a strict interpretation of the standard (compared to GCC) and assumes a reference must always be initialized with a valid object. Hence it optimizes away !&other. Another one is "!this". Although RVCT has a compiler option "--allow_null_this", we should still avoid writing such code.
Yong Li
Comment 16
2011-08-11 11:22:44 PDT
Comment on
attachment 103489
[details]
Fix style issue Thanks Alexey!
WebKit Review Bot
Comment 17
2011-08-11 12:23:10 PDT
Comment on
attachment 103489
[details]
Fix style issue Clearing flags on attachment: 103489 Committed
r92871
: <
http://trac.webkit.org/changeset/92871
>
WebKit Review Bot
Comment 18
2011-08-11 12:23:15 PDT
All reviewed patches have been landed. Closing bug.
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