diff mbox series

[1/1] qt5webkit: fix build issue with 32-bits armv8-a

Message ID 20180302105833.6828-1-gael.portay@savoirfairelinux.com
State Superseded
Headers show
Series [1/1] qt5webkit: fix build issue with 32-bits armv8-a | expand

Commit Message

Gaël PORTAY March 2, 2018, 10:58 a.m. UTC 
Adds WTF platform support for the 32-bits armv8-a architectures.

Fixes:

	In file included from ./config.h:30:0,
	                 from ...
	./wtf/Platform.h:323:6: error: #error "Not supported ARM
architecture"
	 #    error "Not supported ARM architecture"
	      ^~~~~

The patch is an adaptation of an upstream fix in version 5.212 of
qtwebkit[1].

Unfortunately, the commit cannot be backported and has to be fixed since
the toolchain does not define __ARM_ARCH_8__ but __ARM_ARCH_8A__.

	$ host/bin/arm-buildroot-linux-gnueabihf-g++ -dM -E - < /dev/null | grep -i ARM
	#define __ARM_SIZEOF_WCHAR_T 4
	#define __ARM_FEATURE_SAT 1
	#define __ARM_ARCH_ISA_ARM 1
	#define __ARMEL__ 1
	#define __ARM_FEATURE_UNALIGNED 1
	#define __ARM_FEATURE_IDIV 1
	#define __ARM_FP 14
	#define __ARM_ARCH_8A__ 1
	#define __ARM_NEON_FP 6
	#define __ARM_SIZEOF_MINIMAL_ENUM 4
	#define __ARM_PCS_VFP 1
	#define __ARM_FEATURE_LDREX 15
	#define __ARM_FEATURE_QBIT 1
	#define __ARM_FEATURE_FMA 1
	#define __ARM_NEON__ 1
	#define __ARM_ARCH_PROFILE 65
	#define __ARM_32BIT_STATE 1
	#define __ARM_FEATURE_CLZ 1
	#define __ARM_ARCH_ISA_THUMB 2
	#define __ARM_ARCH 8
	#define __arm__ 1
	#define __ARM_FEATURE_SIMD32 1
	#define __ARM_FEATURE_CRC32 1
	#define __ARM_NEON 1
	#define __ARM_ARCH_EXT_IDIV__ 1
	#define __ARM_EABI__ 1
	#define __ARM_FEATURE_DSP 1

[1]: https://github.com/qt/qtwebkit/blob/35655d5f4bad248ead1700b59c381cc568b4e98b/Source/WTF/wtf/Platform.h#L241-L242

Cc: Arnout Vandecappelle <arnout@mind.be>
Reported-by: Brock Williams <brock@cottonwoodcomputer.com>
Signed-off-by: Gaël PORTAY <gael.portay@savoirfairelinux.com>
---

Hi,

This issue was reported a while ago[2].

[2]: http://lists.busybox.net/pipermail/buildroot/2017-August/200540.html

Regards,
Gael
 .../0005-Detect-32-bits-armv8-a-architecture.patch | 47 ++++++++++++++++++++++
 .../0003-Detect-32-bits-armv8-a-architecture.patch | 47 ++++++++++++++++++++++
 2 files changed, 94 insertions(+)
 create mode 100644 package/qt5/qt5webkit/5.6.3/0005-Detect-32-bits-armv8-a-architecture.patch
 create mode 100644 package/qt5/qt5webkit/5.9.1/0003-Detect-32-bits-armv8-a-architecture.patch

Comments

Adrian Perez de Castro March 2, 2018, 11:26 a.m. UTC | #1 
Hi everybody,

WebKit{GTK+,WPE} maintainer here chipping in with a heads up...

On Fri,  2 Mar 2018 05:58:33 -0500, Gaël PORTAY <gael.portay@savoirfairelinux.com> wrote:
> Adds WTF platform support for the 32-bits armv8-a architectures.
> 
> Fixes:
> 
> 	In file included from ./config.h:30:0,
> 	                 from ...
> 	./wtf/Platform.h:323:6: error: #error "Not supported ARM
> architecture"
> 	 #    error "Not supported ARM architecture"
> 	      ^~~~~
> 
> The patch is an adaptation of an upstream fix in version 5.212 of
> qtwebkit[1].

I fixed this upstream a while ago:

  https://bugs.webkit.org/show_bug.cgi?id=174425

Note that you might need also the following patch for bmalloc:

  https://bugs.webkit.org/show_bug.cgi?id=174424

For WebKitGTK+ those patches were merged upstream since version 2.16.6; so
I expect that newer versions of qt5webkit based on some upstream WebKit
release from around July 2017 should be including the two patches linked
above.

That being said, I understand that your patch here targets versions of
qt5webkit based on *older* WebKit revisions, right? That's fine of course
and I think it would be desirable to check both for __ARM_ARCH_8__ and
__ARM_ARCH_8A__ like upstream WebKit does:

   #if defined(__ARM_ARCH_8__) || defined(__ARM_ARCH_8A__)
   // ...

Of course if qt5webkit does NOT use bmalloc, then you do not need the
corresponding fix for it — sorry, I don't know what's the qt5webkit situation
in this regard, I have not been following it much since the Qt port was
removed from the upstream WebKit source tree.

> Unfortunately, the commit cannot be backported and has to be fixed since
> the toolchain does not define __ARM_ARCH_8__ but __ARM_ARCH_8A__.
>
> 	$ host/bin/arm-buildroot-linux-gnueabihf-g++ -dM -E - < /dev/null | grep -i ARM
> 	#define __ARM_SIZEOF_WCHAR_T 4
> 	#define __ARM_FEATURE_SAT 1
> 	#define __ARM_ARCH_ISA_ARM 1
> 	#define __ARMEL__ 1
> 	#define __ARM_FEATURE_UNALIGNED 1
> 	#define __ARM_FEATURE_IDIV 1
> 	#define __ARM_FP 14
> 	#define __ARM_ARCH_8A__ 1
> 	#define __ARM_NEON_FP 6
> 	#define __ARM_SIZEOF_MINIMAL_ENUM 4
> 	#define __ARM_PCS_VFP 1
> 	#define __ARM_FEATURE_LDREX 15
> 	#define __ARM_FEATURE_QBIT 1
> 	#define __ARM_FEATURE_FMA 1
> 	#define __ARM_NEON__ 1
> 	#define __ARM_ARCH_PROFILE 65
> 	#define __ARM_32BIT_STATE 1
> 	#define __ARM_FEATURE_CLZ 1
> 	#define __ARM_ARCH_ISA_THUMB 2
> 	#define __ARM_ARCH 8
> 	#define __arm__ 1
> 	#define __ARM_FEATURE_SIMD32 1
> 	#define __ARM_FEATURE_CRC32 1
> 	#define __ARM_NEON 1
> 	#define __ARM_ARCH_EXT_IDIV__ 1
> 	#define __ARM_EABI__ 1
> 	#define __ARM_FEATURE_DSP 1
> 
> [1]: https://github.com/qt/qtwebkit/blob/35655d5f4bad248ead1700b59c381cc568b4e98b/Source/WTF/wtf/Platform.h#L241-L242
> 
> Cc: Arnout Vandecappelle <arnout@mind.be>
> Reported-by: Brock Williams <brock@cottonwoodcomputer.com>
> Signed-off-by: Gaël PORTAY <gael.portay@savoirfairelinux.com>

Reviewed-by: Adrian Perez de Castro <aperez@igalia.com>

Patch looks good to me, but please double-check the comments above, in
particular the situation with bmalloc which might need a similar fix.

> ---
> 
> Hi,
> 
> This issue was reported a while ago[2].
> 
> [2]: http://lists.busybox.net/pipermail/buildroot/2017-August/200540.html
> 
> Regards,
> Gael
>  .../0005-Detect-32-bits-armv8-a-architecture.patch | 47 ++++++++++++++++++++++
>  .../0003-Detect-32-bits-armv8-a-architecture.patch | 47 ++++++++++++++++++++++
>  2 files changed, 94 insertions(+)
>  create mode 100644 package/qt5/qt5webkit/5.6.3/0005-Detect-32-bits-armv8-a-architecture.patch
>  create mode 100644 package/qt5/qt5webkit/5.9.1/0003-Detect-32-bits-armv8-a-architecture.patch
> 
> diff --git a/package/qt5/qt5webkit/5.6.3/0005-Detect-32-bits-armv8-a-architecture.patch b/package/qt5/qt5webkit/5.6.3/0005-Detect-32-bits-armv8-a-architecture.patch
> new file mode 100644
> index 0000000000..c9ca7e6f96
> --- /dev/null
> +++ b/package/qt5/qt5webkit/5.6.3/0005-Detect-32-bits-armv8-a-architecture.patch
> @@ -0,0 +1,47 @@
> +From 69182ba2b2f41cde695c7ec5f8eb285797d23569 Mon Sep 17 00:00:00 2001
> +From: =?utf-8?q?Ga=C3=ABl=20PORTAY?= <gael.portay@savoirfairelinux.com>
> +Date: Tue, 15 Aug 2017 18:28:49 -0400
> +Subject: [PATCH 1/1] Detect 32-bits armv8-a architecture
> +MIME-Version: 1.0
> +Content-Type: text/plain; charset=utf-8
> +Content-Transfer-Encoding: 8bit
> +
> +Adds WTF platform support for the 32-bits armv8-a architectures.
> +
> +Theses toolchains define __ARM_ARCH_8A__ (for ARM architecture version)
> +and __arm__ (for 32-bits word-size; __aarch64__ for 64-bits).
> +
> +This commit catches this new architecture (armv8a) within a #ifdef/#endif
> +inside the if statement dedicated for 32-bits ARM detection.
> +
> +Fixes:
> +
> +	In file included from ./config.h:30:0,
> +	                 from ...
> +	./wtf/Platform.h:323:6: error: #error "Not supported ARM architecture"
> +	 #    error "Not supported ARM architecture"
> +	      ^~~~~
> +
> +Upstream-Status: Inappropriate
> +Signed-off-by: Gaël PORTAY <gael.portay@savoirfairelinux.com>
> +---
> + Source/WTF/wtf/Platform.h | 3 +++
> + 1 file changed, 3 insertions(+)
> +
> +diff --git a/Source/WTF/wtf/Platform.h b/Source/WTF/wtf/Platform.h
> +index 562840cf7..27a9035fd 100644
> +--- a/Source/WTF/wtf/Platform.h
> ++++ b/Source/WTF/wtf/Platform.h
> +@@ -243,6 +243,9 @@
> +     || defined(__ARM_ARCH_7S__)
> + #define WTF_ARM_ARCH_VERSION 7
> + 
> ++#elif defined(__ARM_ARCH_8A__)
> ++#define WTF_ARM_ARCH_VERSION 8
> ++
> + /* MSVC sets _M_ARM */
> + #elif defined(_M_ARM)
> + #define WTF_ARM_ARCH_VERSION _M_ARM
> +-- 
> +2.16.1
> +
> diff --git a/package/qt5/qt5webkit/5.9.1/0003-Detect-32-bits-armv8-a-architecture.patch b/package/qt5/qt5webkit/5.9.1/0003-Detect-32-bits-armv8-a-architecture.patch
> new file mode 100644
> index 0000000000..c9ca7e6f96
> --- /dev/null
> +++ b/package/qt5/qt5webkit/5.9.1/0003-Detect-32-bits-armv8-a-architecture.patch
> @@ -0,0 +1,47 @@
> +From 69182ba2b2f41cde695c7ec5f8eb285797d23569 Mon Sep 17 00:00:00 2001
> +From: =?utf-8?q?Ga=C3=ABl=20PORTAY?= <gael.portay@savoirfairelinux.com>
> +Date: Tue, 15 Aug 2017 18:28:49 -0400
> +Subject: [PATCH 1/1] Detect 32-bits armv8-a architecture
> +MIME-Version: 1.0
> +Content-Type: text/plain; charset=utf-8
> +Content-Transfer-Encoding: 8bit
> +
> +Adds WTF platform support for the 32-bits armv8-a architectures.
> +
> +Theses toolchains define __ARM_ARCH_8A__ (for ARM architecture version)
> +and __arm__ (for 32-bits word-size; __aarch64__ for 64-bits).
> +
> +This commit catches this new architecture (armv8a) within a #ifdef/#endif
> +inside the if statement dedicated for 32-bits ARM detection.
> +
> +Fixes:
> +
> +	In file included from ./config.h:30:0,
> +	                 from ...
> +	./wtf/Platform.h:323:6: error: #error "Not supported ARM architecture"
> +	 #    error "Not supported ARM architecture"
> +	      ^~~~~
> +
> +Upstream-Status: Inappropriate
> +Signed-off-by: Gaël PORTAY <gael.portay@savoirfairelinux.com>
> +---
> + Source/WTF/wtf/Platform.h | 3 +++
> + 1 file changed, 3 insertions(+)
> +
> +diff --git a/Source/WTF/wtf/Platform.h b/Source/WTF/wtf/Platform.h
> +index 562840cf7..27a9035fd 100644
> +--- a/Source/WTF/wtf/Platform.h
> ++++ b/Source/WTF/wtf/Platform.h
> +@@ -243,6 +243,9 @@
> +     || defined(__ARM_ARCH_7S__)
> + #define WTF_ARM_ARCH_VERSION 7
> + 
> ++#elif defined(__ARM_ARCH_8A__)
> ++#define WTF_ARM_ARCH_VERSION 8
> ++
> + /* MSVC sets _M_ARM */
> + #elif defined(_M_ARM)
> + #define WTF_ARM_ARCH_VERSION _M_ARM
> +-- 
> +2.16.1
> +
> -- 
> 2.11.0
> 
> _______________________________________________
> buildroot mailing list
> buildroot@busybox.net
> http://lists.busybox.net/mailman/listinfo/buildroot
Gaël PORTAY March 2, 2018, 11:49 a.m. UTC | #2 
Hi Adrian,

On Fri, Mar 02, 2018 at 11:26:52AM +0000, Adrian Perez de Castro wrote:
> > The patch is an adaptation of an upstream fix in version 5.212 of
> > qtwebkit[1].
> 
> I fixed this upstream a while ago:
> 
>   https://bugs.webkit.org/show_bug.cgi?id=174425
> 
> Note that you might need also the following patch for bmalloc:
> 
>   https://bugs.webkit.org/show_bug.cgi?id=174424
> 
> For WebKitGTK+ those patches were merged upstream since version 2.16.6; so
> I expect that newer versions of qt5webkit based on some upstream WebKit
> release from around July 2017 should be including the two patches linked
> above.
> 

It seems your modifications has not yet landed into qt5webkit (even in
5.212).

> That being said, I understand that your patch here targets versions of
> qt5webkit based on *older* WebKit revisions, right? That's fine of course
> and I think it would be desirable to check both for __ARM_ARCH_8__ and
> __ARM_ARCH_8A__ like upstream WebKit does:
> 
>    #if defined(__ARM_ARCH_8__) || defined(__ARM_ARCH_8A__)
>    // ...
> 

I will add this double check so the fix is closer that the upstream
version of WebKit.

> Of course if qt5webkit does NOT use bmalloc, then you do not need the
> corresponding fix for it — sorry, I don't know what's the qt5webkit situation
> in this regard, I have not been following it much since the Qt port was
> removed from the upstream WebKit source tree.
> 

It looks like qt5webkit does not use bmalloc.

> 
> Reviewed-by: Adrian Perez de Castro <aperez@igalia.com>
> 
> Patch looks good to me, but please double-check the comments above, in
> particular the situation with bmalloc which might need a similar fix.
> 

Thanks for your review.
Gaël PORTAY March 2, 2018, 12:08 p.m. UTC | #3 
Adrian,

On Fri, Mar 02, 2018 at 11:26:52AM +0000, Adrian Perez de Castro wrote:
> Of course if qt5webkit does NOT use bmalloc, then you do not need the
> corresponding fix for it — sorry, I don't know what's the qt5webkit situation
> in this regard, I have not been following it much since the Qt port was
> removed from the upstream WebKit source tree.
> 

qt5webkit uses bmalloc in 5.212.
Peter Korsgaard April 8, 2018, 3:10 p.m. UTC | #4 
>>>>> "Gaël" == Gaël PORTAY <gael.portay@savoirfairelinux.com> writes:

 > Adds WTF platform support for the 32-bits armv8-a architectures.
 > Fixes:

 > 	In file included from ./config.h:30:0,
 > 	                 from ...
 > 	./wtf/Platform.h:323:6: error: #error "Not supported ARM
 > architecture"
 > 	 #    error "Not supported ARM architecture"
 > 	      ^~~~~

 > The patch is an adaptation of an upstream fix in version 5.212 of
 > qtwebkit[1].

 > Unfortunately, the commit cannot be backported and has to be fixed since
 > the toolchain does not define __ARM_ARCH_8__ but __ARM_ARCH_8A__.

 > 	$ host/bin/arm-buildroot-linux-gnueabihf-g++ -dM -E - < /dev/null | grep -i ARM
 > 	#define __ARM_SIZEOF_WCHAR_T 4
 > 	#define __ARM_FEATURE_SAT 1
 > 	#define __ARM_ARCH_ISA_ARM 1
 > 	#define __ARMEL__ 1
 > 	#define __ARM_FEATURE_UNALIGNED 1
 > 	#define __ARM_FEATURE_IDIV 1
 > 	#define __ARM_FP 14
 > 	#define __ARM_ARCH_8A__ 1
 > 	#define __ARM_NEON_FP 6
 > 	#define __ARM_SIZEOF_MINIMAL_ENUM 4
 > 	#define __ARM_PCS_VFP 1
 > 	#define __ARM_FEATURE_LDREX 15
 > 	#define __ARM_FEATURE_QBIT 1
 > 	#define __ARM_FEATURE_FMA 1
 > 	#define __ARM_NEON__ 1
 > 	#define __ARM_ARCH_PROFILE 65
 > 	#define __ARM_32BIT_STATE 1
 > 	#define __ARM_FEATURE_CLZ 1
 > 	#define __ARM_ARCH_ISA_THUMB 2
 > 	#define __ARM_ARCH 8
 > 	#define __arm__ 1
 > 	#define __ARM_FEATURE_SIMD32 1
 > 	#define __ARM_FEATURE_CRC32 1
 > 	#define __ARM_NEON 1
 > 	#define __ARM_ARCH_EXT_IDIV__ 1
 > 	#define __ARM_EABI__ 1
 > 	#define __ARM_FEATURE_DSP 1

 > [1]: https://github.com/qt/qtwebkit/blob/35655d5f4bad248ead1700b59c381cc568b4e98b/Source/WTF/wtf/Platform.h#L241-L242

 > Cc: Arnout Vandecappelle <arnout@mind.be>
 > Reported-by: Brock Williams <brock@cottonwoodcomputer.com>
 > Signed-off-by: Gaël PORTAY <gael.portay@savoirfairelinux.com>

Committed to 2018.02.x, thanks.
diff mbox series

Patch

diff --git a/package/qt5/qt5webkit/5.6.3/0005-Detect-32-bits-armv8-a-architecture.patch b/package/qt5/qt5webkit/5.6.3/0005-Detect-32-bits-armv8-a-architecture.patch
new file mode 100644
index 0000000000..c9ca7e6f96
--- /dev/null
+++ b/package/qt5/qt5webkit/5.6.3/0005-Detect-32-bits-armv8-a-architecture.patch
@@ -0,0 +1,47 @@ 
+From 69182ba2b2f41cde695c7ec5f8eb285797d23569 Mon Sep 17 00:00:00 2001
+From: =?utf-8?q?Ga=C3=ABl=20PORTAY?= <gael.portay@savoirfairelinux.com>
+Date: Tue, 15 Aug 2017 18:28:49 -0400
+Subject: [PATCH 1/1] Detect 32-bits armv8-a architecture
+MIME-Version: 1.0
+Content-Type: text/plain; charset=utf-8
+Content-Transfer-Encoding: 8bit
+
+Adds WTF platform support for the 32-bits armv8-a architectures.
+
+Theses toolchains define __ARM_ARCH_8A__ (for ARM architecture version)
+and __arm__ (for 32-bits word-size; __aarch64__ for 64-bits).
+
+This commit catches this new architecture (armv8a) within a #ifdef/#endif
+inside the if statement dedicated for 32-bits ARM detection.
+
+Fixes:
+
+	In file included from ./config.h:30:0,
+	                 from ...
+	./wtf/Platform.h:323:6: error: #error "Not supported ARM architecture"
+	 #    error "Not supported ARM architecture"
+	      ^~~~~
+
+Upstream-Status: Inappropriate
+Signed-off-by: Gaël PORTAY <gael.portay@savoirfairelinux.com>
+---
+ Source/WTF/wtf/Platform.h | 3 +++
+ 1 file changed, 3 insertions(+)
+
+diff --git a/Source/WTF/wtf/Platform.h b/Source/WTF/wtf/Platform.h
+index 562840cf7..27a9035fd 100644
+--- a/Source/WTF/wtf/Platform.h
++++ b/Source/WTF/wtf/Platform.h
+@@ -243,6 +243,9 @@
+     || defined(__ARM_ARCH_7S__)
+ #define WTF_ARM_ARCH_VERSION 7
+ 
++#elif defined(__ARM_ARCH_8A__)
++#define WTF_ARM_ARCH_VERSION 8
++
+ /* MSVC sets _M_ARM */
+ #elif defined(_M_ARM)
+ #define WTF_ARM_ARCH_VERSION _M_ARM
+-- 
+2.16.1
+
diff --git a/package/qt5/qt5webkit/5.9.1/0003-Detect-32-bits-armv8-a-architecture.patch b/package/qt5/qt5webkit/5.9.1/0003-Detect-32-bits-armv8-a-architecture.patch
new file mode 100644
index 0000000000..c9ca7e6f96
--- /dev/null
+++ b/package/qt5/qt5webkit/5.9.1/0003-Detect-32-bits-armv8-a-architecture.patch
@@ -0,0 +1,47 @@ 
+From 69182ba2b2f41cde695c7ec5f8eb285797d23569 Mon Sep 17 00:00:00 2001
+From: =?utf-8?q?Ga=C3=ABl=20PORTAY?= <gael.portay@savoirfairelinux.com>
+Date: Tue, 15 Aug 2017 18:28:49 -0400
+Subject: [PATCH 1/1] Detect 32-bits armv8-a architecture
+MIME-Version: 1.0
+Content-Type: text/plain; charset=utf-8
+Content-Transfer-Encoding: 8bit
+
+Adds WTF platform support for the 32-bits armv8-a architectures.
+
+Theses toolchains define __ARM_ARCH_8A__ (for ARM architecture version)
+and __arm__ (for 32-bits word-size; __aarch64__ for 64-bits).
+
+This commit catches this new architecture (armv8a) within a #ifdef/#endif
+inside the if statement dedicated for 32-bits ARM detection.
+
+Fixes:
+
+	In file included from ./config.h:30:0,
+	                 from ...
+	./wtf/Platform.h:323:6: error: #error "Not supported ARM architecture"
+	 #    error "Not supported ARM architecture"
+	      ^~~~~
+
+Upstream-Status: Inappropriate
+Signed-off-by: Gaël PORTAY <gael.portay@savoirfairelinux.com>
+---
+ Source/WTF/wtf/Platform.h | 3 +++
+ 1 file changed, 3 insertions(+)
+
+diff --git a/Source/WTF/wtf/Platform.h b/Source/WTF/wtf/Platform.h
+index 562840cf7..27a9035fd 100644
+--- a/Source/WTF/wtf/Platform.h
++++ b/Source/WTF/wtf/Platform.h
+@@ -243,6 +243,9 @@
+     || defined(__ARM_ARCH_7S__)
+ #define WTF_ARM_ARCH_VERSION 7
+ 
++#elif defined(__ARM_ARCH_8A__)
++#define WTF_ARM_ARCH_VERSION 8
++
+ /* MSVC sets _M_ARM */
+ #elif defined(_M_ARM)
+ #define WTF_ARM_ARCH_VERSION _M_ARM
+-- 
+2.16.1
+