Bug 56214 - Ensure all values are correctly tagged in the registerfile
Summary: Ensure all values are correctly tagged in the registerfile
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: New Bugs (show other bugs)
Version: 528+ (Nightly build)
Hardware: Other OS X 10.5
: P2 Normal
Assignee: Oliver Hunt
URL:
Keywords:
Depends on: 56251
Blocks:
  Show dependency treegraph
 
Reported: 2011-03-11 12:52 PST by Oliver Hunt
Modified: 2011-03-14 11:29 PDT (History)
9 users (show)

See Also:


Attachments
Patch (29.83 KB, patch)
2011-03-11 13:07 PST, Oliver Hunt
no flags Details | Formatted Diff | Diff
Patch (29.75 KB, patch)
2011-03-11 13:34 PST, Oliver Hunt
no flags Details | Formatted Diff | Diff
Patch (39.44 KB, patch)
2011-03-11 15:09 PST, Oliver Hunt
no flags Details | Formatted Diff | Diff
Patch (42.06 KB, patch)
2011-03-11 16:09 PST, Oliver Hunt
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Oliver Hunt 2011-03-11 12:52:30 PST
Ensure all values are correctly tagged in the registerfile
Comment 1 Oliver Hunt 2011-03-11 13:07:17 PST
Created attachment 85516 [details]
Patch
Comment 2 Early Warning System Bot 2011-03-11 13:27:34 PST
Attachment 85516 [details] did not build on qt:
Build output: http://queues.webkit.org/results/8132369
Comment 3 Oliver Hunt 2011-03-11 13:34:36 PST
Created attachment 85520 [details]
Patch
Comment 4 Build Bot 2011-03-11 13:50:22 PST
Attachment 85516 [details] did not build on win:
Build output: http://queues.webkit.org/results/8132382
Comment 5 Gavin Barraclough 2011-03-11 14:09:06 PST
Comment on attachment 85520 [details]
Patch

Source/JavaScriptCore/JavaScriptCore.xcodeproj/project.pbxproj change should probably be reverted?

> *this = JSValue(reinterpret_cast<JSCell*>(activation));

These casts are all incorrect, they should be static_casts (reinterpret_cast is almost always the wrong cast when casting within a class hierarchy).
But can't you just delete all these overloaded= operators? - why doesn't the compiler just call the operator=(JSValue) automatically?

If we're going to access the two halves of the value like we do in JSVALUE32_64 it seems like we should have the IntValueDescriptor defined in JSValue, where it is for JSVALUE32_64 (better still, unify the two!)

r- for the reinterpreting.
Comment 6 Build Bot 2011-03-11 15:00:29 PST
Attachment 85520 [details] did not build on win:
Build output: http://queues.webkit.org/results/8132411
Comment 7 Oliver Hunt 2011-03-11 15:09:47 PST
Created attachment 85543 [details]
Patch
Comment 8 Early Warning System Bot 2011-03-11 15:24:20 PST
Attachment 85543 [details] did not build on qt:
Build output: http://queues.webkit.org/results/8135475
Comment 9 Build Bot 2011-03-11 16:03:50 PST
Attachment 85543 [details] did not build on win:
Build output: http://queues.webkit.org/results/8133485
Comment 10 Oliver Hunt 2011-03-11 16:09:20 PST
Created attachment 85551 [details]
Patch
Comment 11 Gavin Barraclough 2011-03-11 16:30:28 PST
Comment on attachment 85551 [details]
Patch

r is me
Comment 12 Build Bot 2011-03-11 19:01:02 PST
Attachment 85551 [details] did not build on win:
Build output: http://queues.webkit.org/results/8149539
Comment 13 WebKit Commit Bot 2011-03-11 19:12:25 PST
Comment on attachment 85551 [details]
Patch

Clearing flags on attachment: 85551

Committed r80919: <http://trac.webkit.org/changeset/80919>
Comment 14 WebKit Commit Bot 2011-03-11 19:12:29 PST
All reviewed patches have been landed.  Closing bug.
Comment 15 WebKit Review Bot 2011-03-11 19:38:32 PST
http://trac.webkit.org/changeset/80919 might have broken Windows Release (Build) and Windows Debug (Build)
Comment 16 Ryosuke Niwa 2011-03-11 23:08:51 PST
It seems like this change caused a Windows build failure:
http://build.webkit.org/builders/Windows%20Release%20%28Build%29/builds/13160
Comment 17 Oliver Hunt 2011-03-12 11:42:41 PST
The commit queue is broken and needs to be disabled.  I'll re-land on monday with the windows fix.
Comment 18 Adam Barth 2011-03-12 11:54:16 PST
> The commit queue is broken and needs to be disabled.

https://bugs.webkit.org/show_bug.cgi?id=56251#c5
Comment 19 Oliver Hunt 2011-03-14 11:21:58 PDT
Committed r81040: <http://trac.webkit.org/changeset/81040>
Comment 20 WebKit Review Bot 2011-03-14 11:29:19 PDT
http://trac.webkit.org/changeset/81040 might have broken Windows Release (Build) and Windows Debug (Build)