Bug 178090 – [WPE] Build failure due to invalid cast of EGLNativeWindowType when targetting 64-bit ARM

• Status: RESOLVED
• Resolution: FIXED
• Component: WPE WebKit

[Adrian Perez]

This is trying to build the upstream code with Buildroot targeting
ARM Cortex A53 (AArch64):

  WebProcess/WebPage/wpe/AcceleratedSurfaceWPE.cpp: In member function ‘virtual uint64_t WebKit::AcceleratedSurfaceWPE::window() const’:
  WebProcess/WebPage/wpe/AcceleratedSurfaceWPE.cpp:78:99: error: invalid cast from type ‘EGLNativeWindowType {aka long unsigned int}’ to type ‘uint64_t {aka long long unsigned int}’
       return reinterpret_cast<uint64_t>(wpe_renderer_backend_egl_target_get_native_window(m_backend));
                                                                                                     ^

Changing the cast to use “static_cast” works fine, I'll send a patch.

[Adrian Perez]

Created attachment 323197 [details]
Patch

[Adrian Perez]

Comment on attachment 323197 [details]
Patch

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

> Source/WebKit/ChangeLog:3
> +        [WPE] Build failure in Buildroot when targetting 32-bit ARM

I have to edit the ChangeLog to follow the change in the bug title 8-)

[Adrian Perez]

Created attachment 323199 [details]
Patch

[Build Bot]

Attachment 323199 [details] did not pass style-queue:


ERROR: Source/WebKit/ChangeLog:3:  Please consider whether the use of security-sensitive phrasing could help someone exploit WebKit: invalid cast  [changelog/unwantedsecurityterms] [3]
Total errors found: 1 in 2 files


If any of these errors are false positives, please file a bug against check-webkit-style.

[Zan Dobersek]

Comment on attachment 323199 [details]
Patch

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

> Source/WebKit/WebProcess/WebPage/wpe/AcceleratedSurfaceWPE.cpp:75
> +

No need for this newline.

[Adrian Perez]

(In reply to Zan Dobersek from comment #5)
> Comment on attachment 323199 [details]
> Patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=323199&action=review
> 
> > Source/WebKit/WebProcess/WebPage/wpe/AcceleratedSurfaceWPE.cpp:75
> > +
> 
> No need for this newline.

Indeed, I'll remove it before landing. Thanks!

[Adrian Perez]

Created attachment 323298 [details]
Patch for landing

[WebKit Commit Bot]

Comment on attachment 323298 [details]
Patch for landing

Clearing flags on attachment: 323298

Committed r223130: <http://trac.webkit.org/changeset/223130>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.

[Carlos Alberto Lopez Perez]

(In reply to WebKit Commit Bot from comment #8)
> Comment on attachment 323298 [details]
> Patch for landing
> 
> Clearing flags on attachment: 323298
> 
> Committed r223130: <http://trac.webkit.org/changeset/223130>

This has broken the build for me (GCC 5.3; ARMv7 Thumb2):


[.... gcc .... ] -c /home/igalia/clopez/wpe-test/tmp/work/cortexa9t2hf-neon-mx6qdl-poky-linux-gnueabi/wpewebkit/0.r223132-r0/trunk/Source/WebKit/WebProcess/WebPage/wpe/AcceleratedSurfaceWPE.cpp
/home/igalia/clopez/wpe-test/tmp/work/cortexa9t2hf-neon-mx6qdl-poky-linux-gnueabi/wpewebkit/0.r223132-r0/trunk/Source/WebKit/WebProcess/WebPage/wpe/AcceleratedSurfaceWPE.cpp: In member function 'virtual uint64_t WebKit::AcceleratedSurfaceWPE::window() const':
/home/igalia/clopez/wpe-test/tmp/work/cortexa9t2hf-neon-mx6qdl-poky-linux-gnueabi/wpewebkit/0.r223132-r0/trunk/Source/WebKit/WebProcess/WebPage/wpe/AcceleratedSurfaceWPE.cpp:79:94: error: invalid static_cast from type 'EGLNativeWindowType {aka wl_egl_window*}' to type 'uint64_t {aka long long unsigned int}'
     return static_cast<uint64_t>(wpe_renderer_backend_egl_target_get_native_window(m_backend));

[Adrian Perez]

(In reply to Carlos Alberto Lopez Perez from comment #10)
> (In reply to WebKit Commit Bot from comment #8)
> > Comment on attachment 323298 [details]
> > Patch for landing
> > 
> > Clearing flags on attachment: 323298
> > 
> > Committed r223130: <http://trac.webkit.org/changeset/223130>
> 
> This has broken the build for me (GCC 5.3; ARMv7 Thumb2):
> 
> 
> [.... gcc .... ] -c
> /home/igalia/clopez/wpe-test/tmp/work/cortexa9t2hf-neon-mx6qdl-poky-linux-
> gnueabi/wpewebkit/0.r223132-r0/trunk/Source/WebKit/WebProcess/WebPage/wpe/
> AcceleratedSurfaceWPE.cpp
> /home/igalia/clopez/wpe-test/tmp/work/cortexa9t2hf-neon-mx6qdl-poky-linux-
> gnueabi/wpewebkit/0.r223132-r0/trunk/Source/WebKit/WebProcess/WebPage/wpe/
> AcceleratedSurfaceWPE.cpp: In member function 'virtual uint64_t
> WebKit::AcceleratedSurfaceWPE::window() const':
> /home/igalia/clopez/wpe-test/tmp/work/cortexa9t2hf-neon-mx6qdl-poky-linux-
> gnueabi/wpewebkit/0.r223132-r0/trunk/Source/WebKit/WebProcess/WebPage/wpe/
> AcceleratedSurfaceWPE.cpp:79:94: error: invalid static_cast from type
> 'EGLNativeWindowType {aka wl_egl_window*}' to type 'uint64_t {aka long long
> unsigned int}'
>      return
> static_cast<uint64_t>(wpe_renderer_backend_egl_target_get_native_window(m_bac
> kend));

Right, we'll have to handle the configurations where “EGLNativeWindowType”
is a pointer.

[Michael Catanzaro]

Would it be correct to use a C-style cast here, with a comment explaining why C++ casts won't work?

[Adrian Perez]

(In reply to Michael Catanzaro from comment #12)
> Would it be correct to use a C-style cast here, with a comment explaining
> why C++ casts won't work?

Yes, I arrived to the same conclusion after checking the C++ standard
and confirming that e.g. reinterpret_cast will only convert types of
the same bit width (makes sense) and that static_cast will never convert
a pointer to an integer :-(

I've made a rollout of the previous patch and I will be posting a new
one so it's easier to track and backport the change to stable branches.

[Adrian Perez]

Created attachment 323356 [details]
Patch

[Build Bot]

Attachment 323356 [details] did not pass style-queue:


ERROR: Source/WebKit/ChangeLog:3:  Please consider whether the use of security-sensitive phrasing could help someone exploit WebKit: invalid cast  [changelog/unwantedsecurityterms] [3]
Total errors found: 1 in 2 files


If any of these errors are false positives, please file a bug against check-webkit-style.

[WebKit Commit Bot]

Comment on attachment 323356 [details]
Patch

Clearing flags on attachment: 323356

Committed r223416: <https://trac.webkit.org/changeset/223416>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.