[EFL] Fix several warnings for ARM cross compilation

• Component: Tools / Tests
• Status: RESOLVED WONTFIX

[Gabor Rapcsanyi]

[EFL] Disable -Werror when cross compiling

[Gabor Rapcsanyi]

Created attachment 216581 [details]
proposed patch

[Gyuyoung Kim]

Comment on attachment 216581 [details]
proposed patch

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

> ChangeLog:3
> +        [EFL] Disable -Werror when cross compiling

I wonder why you need to disable -Werror when compiling on cross platform. Isn't it nice to remove all build warnings ?

[Gabor Rapcsanyi]

Created attachment 216650 [details]
warnings

We would like to make an ARM EFL bot but there are these warnings which block us. There are many 'increases required alignment of target type' warning because of -Wcast-align.
I made a small example and tried it on our ARM compiler (gcc version 4.6.3 (Ubuntu/Linaro 4.6.3-1ubuntu5)):

int main() {
  unsigned char* d1;
  short int d2;

  d2 = *reinterpret_cast<short int*>(d1);
}

g++ -Wcast-align main.cpp
main.cpp: In function ‘int main()’:
main.cpp:5:40: warning: cast from ‘unsigned char*’ to ‘short int*’ increases required alignment of target type [-Wcast-align]


With the x86 gcc there is no problem.
So while we fixing these warnings we would like to disable -Werror for cross compilation.

[Ryuan Choi]

(In reply to comment #3)
> Created an attachment (id=216650) [details]
> warnings
> 
> We would like to make an ARM EFL bot but there are these warnings which block us. There are many 'increases required alignment of target type' warning because of -Wcast-align.
> I made a small example and tried it on our ARM compiler (gcc version 4.6.3 (Ubuntu/Linaro 4.6.3-1ubuntu5)):
> 
> int main() {
>   unsigned char* d1;
>   short int d2;
> 
>   d2 = *reinterpret_cast<short int*>(d1);
> }
> 
> g++ -Wcast-align main.cpp
> main.cpp: In function ‘int main()’:
> main.cpp:5:40: warning: cast from ‘unsigned char*’ to ‘short int*’ increases required alignment of target type [-Wcast-align]
> 
> 
> With the x86 gcc there is no problem.
> So while we fixing these warnings we would like to disable -Werror for cross compilation.

Can't we use reinterpret_cast_ptr for it if casting is fine?

[Csaba Osztrogonác]

(In reply to comment #4)
> Can't we use reinterpret_cast_ptr for it if casting is fine?

Yes, it is the proper fix, but before using it, each cast should be checked 
one by one. When I fixed similar warnings in the past, I have to say it isn't
trivial to decide if a cast is fine or not in many cases.

[Gabor Rapcsanyi]

(In reply to comment #4)
> (In reply to comment #3)
> > Created an attachment (id=216650) [details] [details]
> > warnings
> > 
> > We would like to make an ARM EFL bot but there are these warnings which block us. There are many 'increases required alignment of target type' warning because of -Wcast-align.
> > I made a small example and tried it on our ARM compiler (gcc version 4.6.3 (Ubuntu/Linaro 4.6.3-1ubuntu5)):
> > 
> > int main() {
> >   unsigned char* d1;
> >   short int d2;
> > 
> >   d2 = *reinterpret_cast<short int*>(d1);
> > }
> > 
> > g++ -Wcast-align main.cpp
> > main.cpp: In function ‘int main()’:
> > main.cpp:5:40: warning: cast from ‘unsigned char*’ to ‘short int*’ increases required alignment of target type [-Wcast-align]
> > 
> > 
> > With the x86 gcc there is no problem.
> > So while we fixing these warnings we would like to disable -Werror for cross compilation.
> 
> Can't we use reinterpret_cast_ptr for it if casting is fine?

Unfortunately this problem exists with the dependent libs as well:
/home/rgabor/WebKit/WebKitBuild/Dependencies/Root/include/eina-1/eina/eina_inline_hash.x:112:73: warning: cast from 'const unsigned char*' to 'const unsigned int*' increases required alignment of target type [-Wcast-align]
/home/rgabor/WebKit/WebKitBuild/Dependencies/Root/include/gstreamer-1.0/gst/gstbuffer.h:333:71: warning: cast from 'GstMiniObject* {aka _GstMiniObject*}' to 'GstBuffer* {aka _GstBuffer*}' increases required alignment of target type [-Wcast-align]

[Ryuan Choi]

(In reply to comment #6)
> (In reply to comment #4)
> > (In reply to comment #3)
> > > Created an attachment (id=216650) [details] [details] [details]
> > > warnings
> > > 
> > > We would like to make an ARM EFL bot but there are these warnings which block us. There are many 'increases required alignment of target type' warning because of -Wcast-align.
> > > I made a small example and tried it on our ARM compiler (gcc version 4.6.3 (Ubuntu/Linaro 4.6.3-1ubuntu5)):
> > > 
> > > int main() {
> > >   unsigned char* d1;
> > >   short int d2;
> > > 
> > >   d2 = *reinterpret_cast<short int*>(d1);
> > > }
> > > 
> > > g++ -Wcast-align main.cpp
> > > main.cpp: In function ‘int main()’:
> > > main.cpp:5:40: warning: cast from ‘unsigned char*’ to ‘short int*’ increases required alignment of target type [-Wcast-align]
> > > 
> > > 
> > > With the x86 gcc there is no problem.
> > > So while we fixing these warnings we would like to disable -Werror for cross compilation.
> > 
> > Can't we use reinterpret_cast_ptr for it if casting is fine?
> 
> Unfortunately this problem exists with the dependent libs as well:
> /home/rgabor/WebKit/WebKitBuild/Dependencies/Root/include/eina-1/eina/eina_inline_hash.x:112:73: warning: cast from 'const unsigned char*' to 'const unsigned int*' increases required alignment of target type [-Wcast-align]
> /home/rgabor/WebKit/WebKitBuild/Dependencies/Root/include/gstreamer-1.0/gst/gstbuffer.h:333:71: warning: cast from 'GstMiniObject* {aka _GstMiniObject*}' to 'GstBuffer* {aka _GstBuffer*}' increases required alignment of target type [-Wcast-align]

For EFL side, I landed https://git.enlightenment.org/core/efl.git/commit/?id=ec1ba326909fae86cf7738b8475bfd05bc2323cb

For jhbuild, I think that we can add the patch.
Can we make similiar patch for gstreamer?

[Gabor Rapcsanyi]

(In reply to comment #7)
> (In reply to comment #6)
> > (In reply to comment #4)
> > > (In reply to comment #3)
> > > > Created an attachment (id=216650) [details] [details] [details] [details]
> > > > warnings
> > > > 
> > > > We would like to make an ARM EFL bot but there are these warnings which block us. There are many 'increases required alignment of target type' warning because of -Wcast-align.
> > > > I made a small example and tried it on our ARM compiler (gcc version 4.6.3 (Ubuntu/Linaro 4.6.3-1ubuntu5)):
> > > > 
> > > > int main() {
> > > >   unsigned char* d1;
> > > >   short int d2;
> > > > 
> > > >   d2 = *reinterpret_cast<short int*>(d1);
> > > > }
> > > > 
> > > > g++ -Wcast-align main.cpp
> > > > main.cpp: In function ‘int main()’:
> > > > main.cpp:5:40: warning: cast from ‘unsigned char*’ to ‘short int*’ increases required alignment of target type [-Wcast-align]
> > > > 
> > > > 
> > > > With the x86 gcc there is no problem.
> > > > So while we fixing these warnings we would like to disable -Werror for cross compilation.
> > > 
> > > Can't we use reinterpret_cast_ptr for it if casting is fine?
> > 
> > Unfortunately this problem exists with the dependent libs as well:
> > /home/rgabor/WebKit/WebKitBuild/Dependencies/Root/include/eina-1/eina/eina_inline_hash.x:112:73: warning: cast from 'const unsigned char*' to 'const unsigned int*' increases required alignment of target type [-Wcast-align]
> > /home/rgabor/WebKit/WebKitBuild/Dependencies/Root/include/gstreamer-1.0/gst/gstbuffer.h:333:71: warning: cast from 'GstMiniObject* {aka _GstMiniObject*}' to 'GstBuffer* {aka _GstBuffer*}' increases required alignment of target type [-Wcast-align]
> 
> For EFL side, I landed https://git.enlightenment.org/core/efl.git/commit/?id=ec1ba326909fae86cf7738b8475bfd05bc2323cb
> 
> For jhbuild, I think that we can add the patch.
> Can we make similiar patch for gstreamer?

I will investigate this.

[Gabor Rapcsanyi]

Created attachment 217042 [details]
Fix for ARM warnings

This patch fixes all of the warnings. Mostly I changed some reinterpret_cast to reinterpret_cast_ptr and add some UNUSED_PARAM macro.
There was a strange warning here:
WebMemorySamplerLinux.cpp:70:19: warning: comparison is always false due to limited range of data type
As I found out on ARM architecture the char type is unsigned by default and this caused the problem. I changed that to signed char.

[Philippe Normand]

Comment on attachment 217042 [details]
Fix for ARM warnings

Have you submitted the patches upstream?

[Gabor Rapcsanyi]

(In reply to comment #10)
> (From update of attachment 217042 [details])
> Have you submitted the patches upstream?

I have made the bugreports:
Gstreamer: https://bugzilla.gnome.org/show_bug.cgi?id=712368
Glib: https://bugzilla.gnome.org/show_bug.cgi?id=712370

[Gyuyoung Kim]

Comment on attachment 217042 [details]
Fix for ARM warnings

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

> Source/JavaScriptCore/ChangeLog:3
> +        [EFL] Fix several warnings for ARM cross compilation

I think your patch doesn't only deal with EFL port area. So, it looks it would be better to split this patch into two patches, one is for WebCore and JS and WK2, other one is for only EFL. Anyway, current patch is not only for EFL port. So, [EFL] prefix is not proper for this patch.

> Source/JavaScriptCore/llint/LLIntSlowPaths.cpp:965
> +    UNUSED_PARAM(pc);

BTW, isn't the *pc* argument being used below line ?

http://trac.webkit.org/browser/trunk/Source/JavaScriptCore/llint/LLIntSlowPaths.cpp#L987

[Gyuyoung Kim]

Comment on attachment 217042 [details]
Fix for ARM warnings

Cleared review? from attachment 217042 [details] so that this bug does not appear in http://webkit.org/pending-review. If you would like this patch reviewed, please attach it to a new bug or this bug again.

[Michael Catanzaro]

Closing this bug because the EFL port has been removed from trunk.

If you feel this bug applies to a different upstream WebKit port and was closed in error, please either update the title and reopen the bug, or leave a comment to request this.