77422 2012-01-31 03:48:31 -0800 Fix type punning warning in HashTable.h debug builds 2012-02-02 11:13:08 -0800 1 1 1 Unclassified WebKit Web Template Framework 528+ (Nightly build) Unspecified Unspecified RESOLVED FIXED P2 Normal --- 1 wingo wingo andersca aroben barraclough darin webkit.review.bot oldest_to_newest 545925 0 wingo 2012-01-31 03:48:31 -0800 The patch to be attached fixes warnings like this one: ../../Source/JavaScriptCore/API/JSObjectRef.cpp:548:1: instantiated from here ../../Source/JavaScriptCore/wtf/HashTable.h:481:90: warning: dereferencing type-punned pointer will break strict-aliasing rules [-Wstrict-aliasing] Why does it work, you ask? That, dear sir or madam, I do not know. 545927 1 124704 wingo 2012-01-31 03:50:10 -0800 Created attachment 124704 Patch 545929 2 webkit.review.bot 2012-01-31 03:51:53 -0800 Attachment 124704 did not pass style-queue: Failed to run "['Tools/Scripts/update-webkit']" exit_code: 9 Updating OpenSource First, rewinding head to replay your work on top of it... Applying: Fix compilation errors on build-webkit --debug --no-workers on mac. Using index info to reconstruct a base tree... Falling back to patching base and 3-way merge... Auto-merging LayoutTests/ChangeLog CONFLICT (content): Merge conflict in LayoutTests/ChangeLog Auto-merging LayoutTests/platform/qt/Skipped CONFLICT (content): Merge conflict in LayoutTests/platform/qt/Skipped Auto-merging Source/WebCore/ChangeLog CONFLICT (content): Merge conflict in Source/WebCore/ChangeLog Failed to merge in the changes. Patch failed at 0001 Fix compilation errors on build-webkit --debug --no-workers on mac. When you have resolved this problem run "git rebase --continue". If you would prefer to skip this patch, instead run "git rebase --skip". To restore the original branch and stop rebasing run "git rebase --abort". rebase refs/remotes/origin/master: command returned error: 1 Died at Tools/Scripts/update-webkit line 164. If any of these errors are false positives, please file a bug against check-webkit-style. 545932 3 wingo 2012-01-31 04:01:35 -0800 (In reply to comment #2) > Falling back to patching base and 3-way merge... > Auto-merging LayoutTests/ChangeLog My patch does not touch this file. What's up with that? Andy 545934 4 wingo 2012-01-31 04:04:22 -0800 Adding Darin and Adam for review. 546098 5 darin 2012-01-31 08:45:00 -0800 (In reply to comment #0) > Why does it work, you ask? That, dear sir or madam, I do not know. Oof. That’s a problem. We should figure out why. If possible, this fix should be done in the reinterpret_cast_ptr template rather than at the call site. 546368 6 wingo 2012-01-31 12:30:21 -0800 (In reply to comment #5) > (In reply to comment #0) > > Why does it work, you ask? That, dear sir or madam, I do not know. > > Oof. That’s a problem. We should figure out why. If possible, this fix should be done in the reinterpret_cast_ptr template rather than at the call site. I did some investigations before making this patch. The reinterpret_cast_ptr looks correct to my knowledge. The case that is taken for the aligned buffer is the __may_alias__ case. This is x86-64, so reinterpret_cast_ptr is reinterpret_cast. Every time I think I find a gcc bug I have been wrong. Still, it sounds possible in this case. This is gcc 4.6.2 from Debian unstable, with the default compilation options for the GTK+ port. FWIW, I have seen this warning for months, but only got bothered enough to look at it today. 547106 7 wingo 2012-02-01 08:52:00 -0800 WDYT, Darin? It is a bit inscrutable, but it is a correct change, and it makes --debug more useful for the gtk port. As it is, this error typically occurs for every file that is compiled, and it is preceded by hundreds of lines of template gobbledygook each time. 547186 8 darin 2012-02-01 10:33:01 -0800 (In reply to comment #7) > but it is a correct change I’m not sure what you mean by “correct change”. It’s probably OK to change this so the compiler stops complaining, but there is no reason this change should have any effect. 547332 9 wingo 2012-02-01 13:33:22 -0800 (In reply to comment #8) > (In reply to comment #7) > > but it is a correct change > > I’m not sure what you mean by “correct change”. It’s probably OK to change this so the compiler stops complaining, but there is no reason this change should have any effect. I didn't express myself well. I meant exactly what you said here. 547355 10 darin 2012-02-01 14:04:41 -0800 Anders, Gavin, do you have any alternative to landing this, and/or do you think it’s OK to just do this to sidestep the warning Andy is running into? 547496 11 barraclough 2012-02-01 17:00:27 -0800 I've got no better suggestions, this is fine with me unless Anders has any better ideas. G. 547532 12 andersca 2012-02-01 17:51:50 -0800 I don't have any better ideas, this looks good. 548000 13 124704 webkit.review.bot 2012-02-02 11:13:03 -0800 Comment on attachment 124704 Patch Clearing flags on attachment: 124704 Committed r106574: <http://trac.webkit.org/changeset/106574> 548001 14 webkit.review.bot 2012-02-02 11:13:08 -0800 All reviewed patches have been landed. Closing bug. 124704 2012-01-31 03:50:10 -0800 2012-02-02 11:13:03 -0800 Patch bug-77422-20120131125008.patch text/plain 1613 wingo U3VidmVyc2lvbiBSZXZpc2lvbjogMTA2MjI2CmRpZmYgLS1naXQgYS9Tb3VyY2UvSmF2YVNjcmlw dENvcmUvQ2hhbmdlTG9nIGIvU291cmNlL0phdmFTY3JpcHRDb3JlL0NoYW5nZUxvZwppbmRleCA5 NzNhYWU0OTMzNTMwNjYyYzU4MzNiOWRmZWU4MWQyOWMzOGJhMjVkLi44NmRmN2VjOWViMzE4YjBk Y2U4ZmViNzZjMzUwYzdmYjg3ZWYzZGMxIDEwMDY0NAotLS0gYS9Tb3VyY2UvSmF2YVNjcmlwdENv cmUvQ2hhbmdlTG9nCisrKyBiL1NvdXJjZS9KYXZhU2NyaXB0Q29yZS9DaGFuZ2VMb2cKQEAgLTEs MyArMSwxMyBAQAorMjAxMi0wMS0zMSAgQW5keSBXaW5nbyAgPHdpbmdvQGlnYWxpYS5jb20+CisK KyAgICAgICAgRml4IHR5cGUgcHVubmluZyB3YXJuaW5nIGluIEhhc2hUYWJsZS5oIGRlYnVnIGJ1 aWxkcworICAgICAgICBodHRwczovL2J1Z3Mud2Via2l0Lm9yZy9zaG93X2J1Zy5jZ2k/aWQ9Nzc0 MjIKKworICAgICAgICBSZXZpZXdlZCBieSBOT0JPRFkgKE9PUFMhKS4KKworICAgICAgICAqIHd0 Zi9IYXNoVGFibGUuaCAoV1RGOjpIYXNoVGFibGU6OmNoZWNrS2V5KTogRml4IHR5cGUgcHVubmlu ZworICAgICAgICB3YXJuaW5nIGFwcGVhcmluZyBpbiBkZWJ1ZyBidWlsZHMgd2l0aCBnY2MtNC42 LjIgb24gR05VL0xpbnV4LgorCiAyMDEyLTAxLTI5ICBab2x0YW4gSGVyY3plZyAgPHpoZXJjemVn QHdlYmtpdC5vcmc+CiAKICAgICAgICAgQ3VzdG9tIHdyaXR0ZW4gQ1NTIGxleGVyCmRpZmYgLS1n aXQgYS9Tb3VyY2UvSmF2YVNjcmlwdENvcmUvd3RmL0hhc2hUYWJsZS5oIGIvU291cmNlL0phdmFT Y3JpcHRDb3JlL3d0Zi9IYXNoVGFibGUuaAppbmRleCA0NGY5MTQzMzAzZjA4NzFlZjQyMzUxMTk3 MmI3NmMyYzQ3MTA5ZjVlLi5jYmNjMDk4MTc0NTg2N2I0ZWQwOWFiOGU4OGU1YWYxZWQ0ZjU1ZThj IDEwMDY0NAotLS0gYS9Tb3VyY2UvSmF2YVNjcmlwdENvcmUvd3RmL0hhc2hUYWJsZS5oCisrKyBi L1NvdXJjZS9KYXZhU2NyaXB0Q29yZS93dGYvSGFzaFRhYmxlLmgKQEAgLTQ3OCw3ICs0NzgsOCBA QCBuYW1lc3BhY2UgV1RGIHsKICAgICAgICAgICAgIHJldHVybjsKICAgICAgICAgQVNTRVJUKCFI YXNoVHJhbnNsYXRvcjo6ZXF1YWwoS2V5VHJhaXRzOjplbXB0eVZhbHVlKCksIGtleSkpOwogICAg ICAgICBBbGlnbmVkQnVmZmVyPHNpemVvZihWYWx1ZVR5cGUpLCBXVEZfQUxJR05fT0YoVmFsdWVU eXBlKT4gZGVsZXRlZFZhbHVlQnVmZmVyOwotICAgICAgICBWYWx1ZVR5cGUmIGRlbGV0ZWRWYWx1 ZSA9ICpyZWludGVycHJldF9jYXN0X3B0cjxWYWx1ZVR5cGUqPihkZWxldGVkVmFsdWVCdWZmZXIu YnVmZmVyKTsKKyAgICAgICAgVmFsdWVUeXBlKiBkZWxldGVkVmFsdWVQdHIgPSByZWludGVycHJl dF9jYXN0X3B0cjxWYWx1ZVR5cGUqPihkZWxldGVkVmFsdWVCdWZmZXIuYnVmZmVyKTsKKyAgICAg ICAgVmFsdWVUeXBlJiBkZWxldGVkVmFsdWUgPSAqZGVsZXRlZFZhbHVlUHRyOwogICAgICAgICBU cmFpdHM6OmNvbnN0cnVjdERlbGV0ZWRWYWx1ZShkZWxldGVkVmFsdWUpOwogICAgICAgICBBU1NF UlQoIUhhc2hUcmFuc2xhdG9yOjplcXVhbChFeHRyYWN0b3I6OmV4dHJhY3QoZGVsZXRlZFZhbHVl KSwga2V5KSk7CiAgICAgfQo=