-Wcast-align make too many warnings on arm build.
Created attachment 93246 [details] Patch
Comment on attachment 93246 [details] Patch Are you removing the option for arm builds or generally?
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?
(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
Created attachment 93400 [details] Patch
Comment on attachment 93400 [details] Patch Lucas?
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
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
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.
(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.
Created attachment 94390 [details] Patch
> 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
The JIT ninjas would know.
(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.
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.
(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.
Oliver: So should this be fixed or closed as Invalid?
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.
(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.
Created attachment 111196 [details] Patch
(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.
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.
Created attachment 111208 [details] Patch
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 ?
(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.
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.
Oliver, Darin, Should I fix anything for this bug? I simply disabled "-Wcast-align" for ARM build.
*** This bug has been marked as a duplicate of bug 55368 ***