Bug 60690 – Avoid increasing required alignment of target type warning on ARM

• Component: JavaScriptCore
• Status: RESOLVED DUPLICATE

[Ryuan Choi]

-Wcast-align make too many warnings on arm build.

[Ryuan Choi]

Created attachment 93246 [details]
Patch

[Antonio Gomes]

Comment on attachment 93246 [details]
Patch

Are you removing the option for arm builds or generally?

[Raphael Kubo da Costa (:rakuco)]

It looks like the patch unconditionally removes the flag when CMake is used. IMO, removing this warning can potentially end up hiding places which should ideally be fixed.

Ryuan, are these flags causing any trouble besides polluting your compilation output?

[Ryuan Choi]

(In reply to comment #2)
> (From update of attachment 93246 [details])
> Are you removing the option for arm builds or generally?

OK, My hope is removing this option for arm builds.
I'll prepare new patch to check it.

(In reply to comment #3)
> It looks like the patch unconditionally removes the flag when CMake is used. IMO, removing this warning can potentially end up hiding places which should ideally be fixed.
> 
> Ryuan, are these flags causing any trouble besides polluting your compilation output?

Below is warning message of a file.
Those warnings, which generated by almost files, make it difficult to detect important warnings and errors(if exist).

In file included from /workspace/git/webkit-efl/Source/JavaScriptCore/runtime/JSObject.h:31:0,
                 from /workspace/git/webkit-efl/Source/JavaScriptCore/runtime/JSObjectWithGlobalObject.h:29,
                 from /workspace/git/webkit-efl/Source/JavaScriptCore/runtime/JSFunction.h:27,
                 from /workspace/git/webkit-efl/Source/JavaScriptCore/runtime/Executable.h:30,
                 from /workspace/git/webkit-efl/Source/JavaScriptCore/bytecode/EvalCodeCache.h:32,
                 from /workspace/git/webkit-efl/Source/JavaScriptCore/bytecode/CodeBlock.h:33,
                 from /workspace/git/webkit-efl/Source/JavaScriptCore/runtime/JSActivation.h:32,
                 from /workspace/git/webkit-efl/Source/JavaScriptCore/runtime/Arguments.h:27,
                 from /workspace/git/webkit-efl/Source/JavaScriptCore/runtime/Arguments.cpp:26:
/workspace/git/webkit-efl/Source/JavaScriptCore/runtime/JSCell.h: In member function 'void* JSC::MarkedBlock::allocate()':
/workspace/git/webkit-efl/Source/JavaScriptCore/runtime/JSCell.h:410:78: warning: cast from 'char (*)[8]' to 'JSC::JSCell*' increases required alignment of target type
In file included from /workspace/git/webkit-efl/Source/JavaScriptCore/collector/handles/Handle.h:29:0,
                 from /workspace/git/webkit-efl/Source/JavaScriptCore/collector/handles/HandleHeap.h:30,
                 from /workspace/git/webkit-efl/Source/JavaScriptCore/runtime/Heap.h:25,
                 from /workspace/git/webkit-efl/Source/JavaScriptCore/runtime/JSGlobalData.h:33,
                 from /workspace/git/webkit-efl/Source/JavaScriptCore/interpreter/CallFrame.h:26,
                 from /workspace/git/webkit-efl/Source/JavaScriptCore/runtime/ArgList.h:25,
                 from /workspace/git/webkit-efl/Source/JavaScriptCore/runtime/JSObject.h:26,
                 from /workspace/git/webkit-efl/Source/JavaScriptCore/runtime/JSObjectWithGlobalObject.h:29,
                 from /workspace/git/webkit-efl/Source/JavaScriptCore/runtime/JSFunction.h:27,
                 from /workspace/git/webkit-efl/Source/JavaScriptCore/runtime/Executable.h:30,
                 from /workspace/git/webkit-efl/Source/JavaScriptCore/bytecode/EvalCodeCache.h:32,
                 from /workspace/git/webkit-efl/Source/JavaScriptCore/bytecode/CodeBlock.h:33,
                 from /workspace/git/webkit-efl/Source/JavaScriptCore/runtime/JSActivation.h:32,
                 from /workspace/git/webkit-efl/Source/JavaScriptCore/runtime/Arguments.h:27,
                 from /workspace/git/webkit-efl/Source/JavaScriptCore/runtime/Arguments.cpp:26:
/workspace/git/webkit-efl/Source/JavaScriptCore/runtime/WriteBarrier.h: In member function 'T* JSC::WriteBarrierBase<T>::get() const [with T = JSC::JSGlobalObject]':
/workspace/git/webkit-efl/Source/JavaScriptCore/runtime/ScopeChain.h:76:86:   instantiated from here
/workspace/git/webkit-efl/Source/JavaScriptCore/runtime/WriteBarrier.h:89:56: warning: cast from 'JSC::JSCell*' to 'JSC::JSGlobalObject*' increases required alignment of target type
/workspace/git/webkit-efl/Source/JavaScriptCore/runtime/WriteBarrier.h: In member function 'T* JSC::WriteBarrierBase<T>::get() const [with T = JSC::JSFunction]':
/workspace/git/webkit-efl/Source/JavaScriptCore/runtime/JSGlobalObject.h:185:70:   instantiated from here
/workspace/git/webkit-efl/Source/JavaScriptCore/runtime/WriteBarrier.h:89:56: warning: cast from 'JSC::JSCell*' to 'JSC::JSFunction*' increases required alignment of target type
/workspace/git/webkit-efl/Source/JavaScriptCore/runtime/WriteBarrier.h: In member function 'T* JSC::WriteBarrierBase<T>::get() const [with T = JSC::StringPrototype]':
/workspace/git/webkit-efl/Source/JavaScriptCore/runtime/JSGlobalObject.h:193:81:   instantiated from here
/workspace/git/webkit-efl/Source/JavaScriptCore/runtime/WriteBarrier.h:89:56: warning: cast from 'JSC::JSCell*' to 'JSC::StringPrototype*' increases required alignment of target type
/workspace/git/webkit-efl/Source/JavaScriptCore/runtime/WriteBarrier.h: In member function 'T* JSC::WriteBarrIn file included from /workspace/git/webkit-efl/Source/JavaScriptCore/runtime/JSObject.h:31:0,
                 from /workspace/git/webkit-efl/Source/JavaScriptCore/runtime/JSObjectWithGlobalObject.h:29,
                 from /workspace/git/webkit-efl/Source/JavaScriptCore/runtime/JSFunction.h:27,
                 from /workspace/git/webkit-efl/Source/JavaScriptCore/runtime/Executable.h:30,
                 from /workspace/git/webkit-efl/Source/JavaScriptCore/bytecode/EvalCodeCache.h:32,
                 from /workspace/git/webkit-efl/Source/JavaScriptCore/bytecode/CodeBlock.h:33,
                 from /workspace/git/webkit-efl/Source/JavaScriptCore/runtime/JSActivation.h:32,
                 from /workspace/git/webkit-efl/Source/JavaScriptCore/runtime/Arguments.h:27,
                 from /workspace/git/webkit-efl/Source/JavaScriptCore/runtime/Arguments.cpp:26:
/workspace/git/webkit-efl/Source/JavaScriptCore/runtime/JSCell.h: In member function 'void* JSC::MarkedBlock::allocate()':
/workspace/git/webkit-efl/Source/JavaScriptCore/runtime/JSCell.h:410:78: warning: cast from 'char (*)[8]' to 'JSC::JSCell*' increases required alignment of target type
In file included from /workspace/git/webkit-efl/Source/JavaScriptCore/collector/handles/Handle.h:29:0,
                 from /workspace/git/webkit-efl/Source/JavaScriptCore/collector/handles/HandleHeap.h:30,
                 from /workspace/git/webkit-efl/Source/JavaScriptCore/runtime/Heap.h:25,
                 from /workspace/git/webkit-efl/Source/JavaScriptCore/runtime/JSGlobalData.h:33,
                 from /workspace/git/webkit-efl/Source/JavaScriptCore/interpreter/CallFrame.h:26,
                 from /workspace/git/webkit-efl/Source/JavaScriptCore/runtime/ArgList.h:25,
                 from /workspace/git/webkit-efl/Source/JavaScriptCore/runtime/JSObject.h:26,
                 from /workspace/git/webkit-efl/Source/JavaScriptCore/runtime/JSObjectWithGlobalObject.h:29,
                 from /workspace/git/webkit-efl/Source/JavaScriptCore/runtime/JSFunction.h:27,
                 from /workspace/git/webkit-efl/Source/JavaScriptCore/runtime/Executable.h:30,
                 from /workspace/git/webkit-efl/Source/JavaScriptCore/bytecode/EvalCodeCache.h:32,
                 from /workspace/git/webkit-efl/Source/JavaScriptCore/bytecode/CodeBlock.h:33,
                 from /workspace/git/webkit-efl/Source/JavaScriptCore/runtime/JSActivation.h:32,
                 from /workspace/git/webkit-efl/Source/JavaScriptCore/runtime/Arguments.h:27,
                 from /workspace/git/webkit-efl/Source/JavaScriptCore/runtime/Arguments.cpp:26:
/workspace/git/webkit-efl/Source/JavaScriptCore/runtime/WriteBarrier.h: In member function 'T* JSC::WriteBarrierBase<T>::get() const [with T = JSC::JSGlobalObject]':
/workspace/git/webkit-efl/Source/JavaScriptCore/runtime/ScopeChain.h:76:86:   instantiated from here
/workspace/git/webkit-efl/Source/JavaScriptCore/runtime/WriteBarrier.h:89:56: warning: cast from 'JSC::JSCell*' to 'JSC::JSGlobalObject*' increases required alignment of target type
/workspace/git/webkit-efl/Source/JavaScriptCore/runtime/WriteBarrier.h: In member function 'T* JSC::WriteBarrierBase<T>::get() const [with T = JSC::JSFunction]':
/workspace/git/webkit-efl/Source/JavaScriptCore/runtime/JSGlobalObject.h:185:70:   instantiated from here
/workspace/git/webkit-efl/Source/JavaScriptCore/runtime/WriteBarrier.h:89:56: warning: cast from 'JSC::JSCell*' to 'JSC::JSFunction*' increases required alignment of target type
/workspace/git/webkit-efl/Source/JavaScriptCore/runtime/WriteBarrier.h: In member function 'T* JSC::WriteBarrierBase<T>::get() const [with T = JSC::StringPrototype]':
/workspace/git/webkit-efl/Source/JavaScriptCore/runtime/JSGlobalObject.h:193:81:   instantiated from here
/workspace/git/webkit-efl/Source/JavaScriptCore/runtime/WriteBarrier.h:89:56: warning: cast from 'JSC::JSCell*' to 'JSC::StringPrototype*' increases required alignment of target type
/workspace/git/webkit-efl/Source/JavaScriptCore/runtime/WriteBarrier.h: In member function 'T* JSC::WriteBarrierBase<T>::get() const [with T = JSC::NumberPrototype]':
/workspace/git/webkit-efl/Source/JavaScriptCore/runtime/JSGlobalObject.h:194:81:   instantiated from here
/workspace/git/webkit-efl/Source/JavaScriptCore/runtime/WriteBarrier.h:89:56: warning: cast from 'JSC::JSCell*' to 'JSC::NumberPrototype*' increases required alignment of target type
/workspace/git/webkit-efl/Source/JavaScriptCore/runtime/WriteBarrier.h: In member function 'T* JSC::WriteBarrierBase<T>::get() const [with T = JSC::JSActivation]':
/workspace/git/webkit-efl/Source/JavaScriptCore/runtime/Arguments.cpp:260:133:   instantiated from here
/workspace/git/webkit-efl/Source/JavaScriptCore/runtime/WriteBarrier.h:89:56: warning: cast from 'JSC::JSCell*' to 'JSC::JSActivation*' increases required alignment of target type
ierBase<T>::get() const [with T = JSC::NumberPrototype]':
/workspace/git/webkit-efl/Source/JavaScriptCore/runtime/JSGlobalObject.h:194:81:   instantiated from here
/workspace/git/webkit-efl/Source/JavaScriptCore/runtime/WriteBarrier.h:89:56: warning: cast from 'JSC::JSCell*' to 'JSC::NumberPrototype*' increases required alignment of target type
/workspace/git/webkit-efl/Source/JavaScriptCore/runtime/WriteBarrier.h: In member function 'T* JSC::WriteBarrierBase<T>::get() const [with T = JSC::JSActivation]':
/workspace/git/webkit-efl/Source/JavaScriptCore/runtime/Arguments.cpp:260:133:   instantiated from here
/workspace/git/webkit-efl/Source/JavaScriptCore/runtime/WriteBarrier.h:89:56: warning: cast from 'JSC::JSCell*' to 'JSC::JSActivation*' increases required alignment of target type

[Ryuan Choi]

Created attachment 93400 [details]
Patch

[Antonio Gomes]

Comment on attachment 93400 [details]
Patch

Lucas?

[Lucas De Marchi]

Comment on attachment 93400 [details]
Patch

It warns only on ARM because on x86 integers can be accessed other than on 4-byte boundaries. This means this warning is irrelevant on x86.

I'm wondering if we shouldn't fix these casts instead of removing the warnings. A quick grep reveals that both GTK and QT use -Wcast-align:

$ find . -name '*.pri' -exec grep Wall {} \;
*-g++*:QMAKE_CXXFLAGS += -Wall -Wextra -Wreturn-type -fno-strict-aliasing -Wcast-align -Wchar-subscripts -Wformat-security -Wreturn-type -Wno-unused-parameter -Wno-sign-compare -Wno-switch -Wno-switch-enum -Wundef -Wmissing-noreturn -Winit-self

$ find . -name GNUmakefile.am -exec grep -B1 -A5 Wall {} \;
global_cppflags += \
	-Wall -W -Wcast-align -Wchar-subscripts -Wreturn-type \
	-Wformat -Wformat-security -Wno-format-y2k -Wundef \
	-Wmissing-format-attribute -Wpointer-arith -Wwrite-strings \
	-Wno-unused-parameter -Wno-parentheses \
	-fno-exceptions -DENABLE_GLIB_SUPPORT=1

[Antonio Gomes]

That is an intesresting point. Ossy, have you seen and fixed it on Qt (arm builds)? We see this a lot on our BlackBerry port as well, so a fix would not be so bad :)

(In reply to comment #7)
> (From update of attachment 93400 [details])
> It warns only on ARM because on x86 integers can be accessed other than on 4-byte boundaries. This means this warning is irrelevant on x86.
> 
> I'm wondering if we shouldn't fix these casts instead of removing the warnings. A quick grep reveals that both GTK and QT use -Wcast-align:
> 
> $ find . -name '*.pri' -exec grep Wall {} \;
> *-g++*:QMAKE_CXXFLAGS += -Wall -Wextra -Wreturn-type -fno-strict-aliasing -Wcast-align -Wchar-subscripts -Wformat-security -Wreturn-type -Wno-unused-parameter -Wno-sign-compare -Wno-switch -Wno-switch-enum -Wundef -Wmissing-noreturn -Winit-self
> 
> $ find . -name GNUmakefile.am -exec grep -B1 -A5 Wall {} \;
> global_cppflags += \
>     -Wall -W -Wcast-align -Wchar-subscripts -Wreturn-type \
>     -Wformat -Wformat-security -Wno-format-y2k -Wundef \
>     -Wmissing-format-attribute -Wpointer-arith -Wwrite-strings \
>     -Wno-unused-parameter -Wno-parentheses \
>     -fno-exceptions -DENABLE_GLIB_SUPPORT=1

[Gabor Loki]

Well, we should try to fix the alignment warnings before landing such a patch.

If you make sure the cast is OK, you can use the reinterpret_cast_ptr feature which was landed in bug 38045 . Otherwise that could be a real problem.

[Ryuan Choi]

(In reply to comment #9)
> Well, we should try to fix the alignment warnings before landing such a patch.
> 
> If you make sure the cast is OK, you can use the reinterpret_cast_ptr feature which was landed in bug 38045 . Otherwise that could be a real problem.

Thank you for your comment.

I don't know it too much. but I prepared a patch like you mentioned.

[Ryuan Choi]

Created attachment 94390 [details]
Patch

[Gabor Loki]

> Created an attachment (id=94390)

Although it looks good to me, it would be nice to do the following as well:
- do a Debug build and check the usual tests
- check weather unvalidatedGet causes any alignment warning in WriteBarrier.h

[Eric Seidel (no email)]

The JIT ninjas would know.

[Oliver Hunt]

(In reply to comment #12)
> > Created an attachment (id=94390) [details]
> 
> Although it looks good to me, it would be nice to do the following as well:
> - do a Debug build and check the usual tests
> - check weather unvalidatedGet causes any alignment warning in WriteBarrier.h

Default debug build should hit unvalidatedGet()

That said this warning has always irked me as it seems hellbent on occuring for completely valid casts, like this:
warning: cast from 'JSC::JSCell*' to 'JSC::JSGlobalObject*' increases required alignment of target type

JSGlobalObject is a subclass of JSCell.  It would be a compiler bug if the compiler allowed JSGlobalObject to have alignment  requirements that differed from jscell.

[Loïc Yhuel]

I applied the patch for my SH4 build, and there are errors on debug build due to the WriteBarrier.h change.

In Source/JavaScriptCore/runtime/ScopeChain.h :
Source/JavaScriptCore/wtf/StdLibExtras.h:68: error: invalid application of ‘__alignof__’ to incomplete type ‘JSC::JSGlobalObject’

Including JSGlobalObject.h produces more errors of the same type, on ObjectPrototype, FunctionPrototype, ...
But adding many includes just to remove warnings is perhaps too much.

[Oliver Hunt]

(In reply to comment #15)
> I applied the patch for my SH4 build, and there are errors on debug build due to the WriteBarrier.h change.
> 
> In Source/JavaScriptCore/runtime/ScopeChain.h :
> Source/JavaScriptCore/wtf/StdLibExtras.h:68: error: invalid application of ‘__alignof__’ to incomplete type ‘JSC::JSGlobalObject’
> 
> Including JSGlobalObject.h produces more errors of the same type, on ObjectPrototype, FunctionPrototype, ...
> But adding many includes just to remove warnings is perhaps too much.

I would just disable that warning.  At least for JavaScriptCore, I have never seen it produce a warning that was actually valid.

[Eric Seidel (no email)]

Oliver: So should this be fixed or closed as Invalid?

[Oliver Hunt]

Comment on attachment 94390 [details]
Patch

r- as this patch will probably not apply cleanly on tot anymore, and there was no response to my comments suggesting simply disabling the warning.

[Lucas De Marchi]

(In reply to comment #16)
> (In reply to comment #15)
> > I applied the patch for my SH4 build, and there are errors on debug build due to the WriteBarrier.h change.
> > 
> > In Source/JavaScriptCore/runtime/ScopeChain.h :
> > Source/JavaScriptCore/wtf/StdLibExtras.h:68: error: invalid application of ‘__alignof__’ to incomplete type ‘JSC::JSGlobalObject’
> > 
> > Including JSGlobalObject.h produces more errors of the same type, on ObjectPrototype, FunctionPrototype, ...
> > But adding many includes just to remove warnings is perhaps too much.
> 
> I would just disable that warning.  At least for JavaScriptCore, I have never seen it produce a warning that was actually valid.

Ok. If Ryuan Choi is not interested in this patch anymore I'll prepare a new one.

[Ryuan Choi]

Created attachment 111196 [details]
Patch

[Ryuan Choi]

(In reply to comment #18)
> (From update of attachment 94390 [details])
> r- as this patch will probably not apply cleanly on tot anymore, and there was no response to my comments suggesting simply disabling the warning.

Sorry about too late answer.

I upload patch which is just disabling warning.

[Darin Adler]

Comment on attachment 111196 [details]
Patch

It does not seem right to remove this warning for all platforms just because it's not working well for ARM.

[Ryuan Choi]

Created attachment 111208 [details]
Patch

[Gyuyoung Kim]

Comment on attachment 111208 [details]
Patch

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

> Source/cmake/WebKitHelpers.cmake:24
> +    SET(OLD_COMPILE_FLAGS "-W -DANOTHER_BRICK_IN_THE -Wall -Wextra -Wchar-subscripts -Wformat -Wformat-security -Wmissing-format-attribute -Wno-format-y2k -Wno-parentheses -Wno-unused-parameter -Wpointer-arith  -Wreturn-type -Wundef -Wwrite-strings ${OLD_COMPILE_FLAGS}")

It seems this removal is still adjusted into all platforms, isn't it ?

[Ryuan Choi]

(In reply to comment #24)
> (From update of attachment 111208 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=111208&action=review
> 
> > Source/cmake/WebKitHelpers.cmake:24
> > +    SET(OLD_COMPILE_FLAGS "-W -DANOTHER_BRICK_IN_THE -Wall -Wextra -Wchar-subscripts -Wformat -Wformat-security -Wmissing-format-attribute -Wno-format-y2k -Wno-parentheses -Wno-unused-parameter -Wpointer-arith  -Wreturn-type -Wundef -Wwrite-strings ${OLD_COMPILE_FLAGS}")
> 
> It seems this removal is still adjusted into all platforms, isn't it ?

No, -Wcast-align will be added if WTF_CPU_ARM is not enabled, so other platforms still check it.

[Gyuyoung Kim]

Comment on attachment 111208 [details]
Patch

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

>>> Source/cmake/WebKitHelpers.cmake:24
>>> +    SET(OLD_COMPILE_FLAGS "-W -DANOTHER_BRICK_IN_THE -Wall -Wextra -Wchar-subscripts -Wformat -Wformat-security -Wmissing-format-attribute -Wno-format-y2k -Wno-parentheses -Wno-unused-parameter -Wpointer-arith  -Wreturn-type -Wundef -Wwrite-strings ${OLD_COMPILE_FLAGS}")
>> 
>> It seems this removal is still adjusted into all platforms, isn't it ?
> 
> No, -Wcast-align will be added if WTF_CPU_ARM is not enabled, so other platforms still check it.

Opps. I missed below code. I'm Sorry. LGTM.

[Ryuan Choi]

Oliver, Darin,

Should I fix anything for this bug?

I simply disabled "-Wcast-align" for ARM build.

[Ryuan Choi]

*** This bug has been marked as a duplicate of bug 55368 ***