Bug 77422 – Fix type punning warning in HashTable.h debug builds

• Component: Web Template Framework
• Status: RESOLVED FIXED

[Andy Wingo]

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.

[Andy Wingo]

Created attachment 124704 [details]
Patch

[WebKit Review Bot]

Attachment 124704 [details] 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.

[Andy Wingo]

(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

[Andy Wingo]

Adding Darin and Adam for review.

[Darin Adler]

(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.

[Andy Wingo]

(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.

[Andy Wingo]

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.

[Darin Adler]

(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.

[Andy Wingo]

(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.

[Darin Adler]

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?

[Gavin Barraclough]

I've got no better suggestions, this is fine with me unless Anders has any better ideas.
G.

[Anders Carlsson]

I don't have any better ideas, this looks good.

[WebKit Review Bot]

Comment on attachment 124704 [details]
Patch

Clearing flags on attachment: 124704

Committed r106574: <http://trac.webkit.org/changeset/106574>

[WebKit Review Bot]

All reviewed patches have been landed.  Closing bug.