Bug 55368 – Fix alignment warnings in ARMv7

• Component: JavaScriptCore
• Status: RESOLVED FIXED

[Xan Lopez]

ToT in ARMv7 has a lot of alignment related warnings, which makes really difficult any other kind of warning in the build. Attached patch uses reinterpret_cast_ptr macro in wtf/StdLibExtras to get rid of most of them, since in ARM this adjusts the alignment properly.

[Xan Lopez]

Created attachment 84048 [details]
alignment.diff

[Xan Lopez]

Comment on attachment 84048 [details]
alignment.diff

OK, this fails in a few places in debug builds because there's an ASSERT checking the correctness of the cast that uses __alignof__, which needs the size of the class, which is not available in many places (since we forward declare instead of including the header). I'll see if I can either fix it for good or at least attach a patch with the easy fixes.

[Rob Buis]

Xan, any update? This seems useful since it may obscure more important warnings, like not returning a value in a method that should return a value.

[Rob Buis]

Created attachment 122580 [details]
Patch

[Rob Buis]

Adapted patch to compile against HEAD. The use of reinterpret_cast_ptr for debug builds everywhere turned out to be hard (at the very least intrusive) to fix (the __alignof__ problem), so I chose static_cast in a few places.

[Ryuan Choi]

*** Bug 60690 has been marked as a duplicate of this bug. ***

[Filip Pizlo]

Comment on attachment 122580 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=122580&action=review

> Source/JavaScriptCore/heap/HandleTypes.h:38
> +    static ExternalType getFromSlot(HandleSlot slot) { return (slot && *slot) ? reinterpret_cast<ExternalType>(static_cast<void*>(slot->asCell())) : 0; }

What, exactly, are you getting by putting in this static_cast?

[Rob Buis]

(In reply to comment #7)
> (From update of attachment 122580 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=122580&action=review
> 
> > Source/JavaScriptCore/heap/HandleTypes.h:38
> > +    static ExternalType getFromSlot(HandleSlot slot) { return (slot && *slot) ? reinterpret_cast<ExternalType>(static_cast<void*>(slot->asCell())) : 0; }
> 
> What, exactly, are you getting by putting in this static_cast?

I am trying to emulate reinterpret_cast_ptr, which implicitly casts to void*, because actually using it and including StdLibExtras.h for these two files would mean including a lot more include files to fix build problems. In fact I gave up on that approach simply because there were so many files to fix.
Cheers,

Rob.

[WebKit Review Bot]

Comment on attachment 122580 [details]
Patch

Clearing flags on attachment: 122580

Committed r106686: <http://trac.webkit.org/changeset/106686>

[WebKit Review Bot]

All reviewed patches have been landed.  Closing bug.