diff mbox

[oe,meta-browser] chromium: fix null pointer dereference in V8 with gcc-6

Message ID 1491590447-19476-1-git-send-email-andrey.konovalov@linaro.org
State Accepted
Headers show

Commit Message

Andrey Konovalov April 7, 2017, 6:40 p.m. UTC
This patch prevents "Aw Snap" error when loading a page with JavaScript.

Tested by running:
- chromium-wayland on Beagle X15
- chromium-wayland on HiKey (with a separate patch to enable aarch64 build)
- cromium on dragonboard-410c (with a separate patch to enable aarch64 build)

Based on the fix for Gentoo's bug 588596:
https://bugs.gentoo.org/show_bug.cgi?id=588596#c10

Signed-off-by: Andrey Konovalov <andrey.konovalov@linaro.org>

---
 recipes-browser/chromium/chromium.inc | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

-- 
2.1.4

-- 
_______________________________________________
Openembedded-devel mailing list
Openembedded-devel@lists.openembedded.org
http://lists.openembedded.org/mailman/listinfo/openembedded-devel

Comments

Andrey Konovalov April 7, 2017, 7:16 p.m. UTC | #1
On 04/07/2017 09:40 PM, Andrey Konovalov wrote:
> This patch prevents "Aw Snap" error when loading a page with JavaScript.

>

> Tested by running:

> - chromium-wayland on Beagle X15

> - chromium-wayland on HiKey (with a separate patch to enable aarch64 build)

> - cromium on dragonboard-410c (with a separate patch to enable aarch64 build)

>

> Based on the fix for Gentoo's bug 588596:

> https://bugs.gentoo.org/show_bug.cgi?id=588596#c10

>

> Signed-off-by: Andrey Konovalov <andrey.konovalov@linaro.org>

> ---

>   recipes-browser/chromium/chromium.inc | 3 ++-

>   1 file changed, 2 insertions(+), 1 deletion(-)

>

> diff --git a/recipes-browser/chromium/chromium.inc b/recipes-browser/chromium/chromium.inc

> index 60edccb..7e057ec 100644

> --- a/recipes-browser/chromium/chromium.inc

> +++ b/recipes-browser/chromium/chromium.inc

> @@ -15,7 +15,8 @@ CHROMIUM_BUILD_TYPE ??= "Release"

>   inherit gettext pythonnative

>

>   ARMFPABI_armv7a = "${@bb.utils.contains('TUNE_FEATURES', 'callconvention-hard', 'arm_float_abi=hard', 'arm_float_abi=softfp', d)}"

> -GYP_DEFINES += "${ARMFPABI} release_extra_cflags='-Wno-error=unused-local-typedefs' sysroot=''"

> +GYP_DEFINES += " ${ARMFPABI} release_extra_cflags='-Wno-error=unused-local-typedefs' sysroot='' \


Oops, by occasion I've added an extra space before ${ARMFPABI}. I updated the pull request in github, and
can send an updated patch to the list if needed.

Thanks,
Andrey

> +	${@bb.utils.contains("AVAILTUNES", "mips", "", "release_extra_cflags='-fno-delete-null-pointer-checks'", d)}"

>   GYP_DEFINES_append_x86 = " generate_character_data=0"

>

>   do_configure() {

>


-- 
_______________________________________________
Openembedded-devel mailing list
Openembedded-devel@lists.openembedded.org
http://lists.openembedded.org/mailman/listinfo/openembedded-devel
Andre McCurdy April 11, 2017, 12:40 a.m. UTC | #2
On Fri, Apr 7, 2017 at 11:40 AM, Andrey Konovalov
<andrey.konovalov@linaro.org> wrote:
> This patch prevents "Aw Snap" error when loading a page with JavaScript.

>

> Tested by running:

> - chromium-wayland on Beagle X15

> - chromium-wayland on HiKey (with a separate patch to enable aarch64 build)

> - cromium on dragonboard-410c (with a separate patch to enable aarch64 build)

>

> Based on the fix for Gentoo's bug 588596:

> https://bugs.gentoo.org/show_bug.cgi?id=588596#c10

>

> Signed-off-by: Andrey Konovalov <andrey.konovalov@linaro.org>

> ---

>  recipes-browser/chromium/chromium.inc | 3 ++-

>  1 file changed, 2 insertions(+), 1 deletion(-)

>

> diff --git a/recipes-browser/chromium/chromium.inc b/recipes-browser/chromium/chromium.inc

> index 60edccb..7e057ec 100644

> --- a/recipes-browser/chromium/chromium.inc

> +++ b/recipes-browser/chromium/chromium.inc

> @@ -15,7 +15,8 @@ CHROMIUM_BUILD_TYPE ??= "Release"

>  inherit gettext pythonnative

>

>  ARMFPABI_armv7a = "${@bb.utils.contains('TUNE_FEATURES', 'callconvention-hard', 'arm_float_abi=hard', 'arm_float_abi=softfp', d)}"


Not related to the patch, but this over-ride should be duplicated for _armv7ve

> -GYP_DEFINES += "${ARMFPABI} release_extra_cflags='-Wno-error=unused-local-typedefs' sysroot=''"

> +GYP_DEFINES += " ${ARMFPABI} release_extra_cflags='-Wno-error=unused-local-typedefs' sysroot='' \

> +       ${@bb.utils.contains("AVAILTUNES", "mips", "", "release_extra_cflags='-fno-delete-null-pointer-checks'", d)}"


What's the significance of skipping the workaround if AVAILTUNES
contains "mips" ?

>  GYP_DEFINES_append_x86 = " generate_character_data=0"

>

>  do_configure() {

> --

> 2.1.4

>

> --

> _______________________________________________

> Openembedded-devel mailing list

> Openembedded-devel@lists.openembedded.org

> http://lists.openembedded.org/mailman/listinfo/openembedded-devel

-- 
_______________________________________________
Openembedded-devel mailing list
Openembedded-devel@lists.openembedded.org
http://lists.openembedded.org/mailman/listinfo/openembedded-devel
Andrey Konovalov April 11, 2017, 1:02 p.m. UTC | #3
On 04/11/2017 03:40 AM, Andre McCurdy wrote:
> On Fri, Apr 7, 2017 at 11:40 AM, Andrey Konovalov

> <andrey.konovalov@linaro.org> wrote:

>> This patch prevents "Aw Snap" error when loading a page with JavaScript.

>>

>> Tested by running:

>> - chromium-wayland on Beagle X15

>> - chromium-wayland on HiKey (with a separate patch to enable aarch64 build)

>> - cromium on dragonboard-410c (with a separate patch to enable aarch64 build)

>>

>> Based on the fix for Gentoo's bug 588596:

>> https://bugs.gentoo.org/show_bug.cgi?id=588596#c10

>>

>> Signed-off-by: Andrey Konovalov <andrey.konovalov@linaro.org>

>> ---

>>   recipes-browser/chromium/chromium.inc | 3 ++-

>>   1 file changed, 2 insertions(+), 1 deletion(-)

>>

>> diff --git a/recipes-browser/chromium/chromium.inc b/recipes-browser/chromium/chromium.inc

>> index 60edccb..7e057ec 100644

>> --- a/recipes-browser/chromium/chromium.inc

>> +++ b/recipes-browser/chromium/chromium.inc

>> @@ -15,7 +15,8 @@ CHROMIUM_BUILD_TYPE ??= "Release"

>>   inherit gettext pythonnative

>>

>>   ARMFPABI_armv7a = "${@bb.utils.contains('TUNE_FEATURES', 'callconvention-hard', 'arm_float_abi=hard', 'arm_float_abi=softfp', d)}"

>

> Not related to the patch, but this over-ride should be duplicated for _armv7ve

>

>> -GYP_DEFINES += "${ARMFPABI} release_extra_cflags='-Wno-error=unused-local-typedefs' sysroot=''"

>> +GYP_DEFINES += " ${ARMFPABI} release_extra_cflags='-Wno-error=unused-local-typedefs' sysroot='' \

>> +       ${@bb.utils.contains("AVAILTUNES", "mips", "", "release_extra_cflags='-fno-delete-null-pointer-checks'", d)}"

>

> What's the significance of skipping the workaround if AVAILTUNES

> contains "mips" ?


This change (to add '-fno-delete-null-pointer-checks' to gcc flags) was initially merged into chromium directly,
but soon reverted as it broke mips somehow:
https://bugs.chromium.org/p/v8/issues/detail?id=3782#c10

>>   GYP_DEFINES_append_x86 = " generate_character_data=0"

>>

>>   do_configure() {

>> --

>> 2.1.4

>>

>> --

>> _______________________________________________

>> Openembedded-devel mailing list

>> Openembedded-devel@lists.openembedded.org

>> http://lists.openembedded.org/mailman/listinfo/openembedded-devel


-- 
_______________________________________________
Openembedded-devel mailing list
Openembedded-devel@lists.openembedded.org
http://lists.openembedded.org/mailman/listinfo/openembedded-devel
Andre McCurdy April 11, 2017, 6:51 p.m. UTC | #4
On Tue, Apr 11, 2017 at 6:02 AM, Andrey Konovalov
<andrey.konovalov@linaro.org> wrote:
> On 04/11/2017 03:40 AM, Andre McCurdy wrote:

>>

>> On Fri, Apr 7, 2017 at 11:40 AM, Andrey Konovalov

>> <andrey.konovalov@linaro.org> wrote:

>>>

>>> This patch prevents "Aw Snap" error when loading a page with JavaScript.

>>>

>>> Tested by running:

>>> - chromium-wayland on Beagle X15

>>> - chromium-wayland on HiKey (with a separate patch to enable aarch64

>>> build)

>>> - cromium on dragonboard-410c (with a separate patch to enable aarch64

>>> build)

>>>

>>> Based on the fix for Gentoo's bug 588596:

>>> https://bugs.gentoo.org/show_bug.cgi?id=588596#c10

>>>

>>> Signed-off-by: Andrey Konovalov <andrey.konovalov@linaro.org>

>>> ---

>>>   recipes-browser/chromium/chromium.inc | 3 ++-

>>>   1 file changed, 2 insertions(+), 1 deletion(-)

>>>

>>> diff --git a/recipes-browser/chromium/chromium.inc

>>> b/recipes-browser/chromium/chromium.inc

>>> index 60edccb..7e057ec 100644

>>> --- a/recipes-browser/chromium/chromium.inc

>>> +++ b/recipes-browser/chromium/chromium.inc

>>> @@ -15,7 +15,8 @@ CHROMIUM_BUILD_TYPE ??= "Release"

>>>   inherit gettext pythonnative

>>>

>>>   ARMFPABI_armv7a = "${@bb.utils.contains('TUNE_FEATURES',

>>> 'callconvention-hard', 'arm_float_abi=hard', 'arm_float_abi=softfp', d)}"

>>

>>

>> Not related to the patch, but this over-ride should be duplicated for

>> _armv7ve

>>

>>> -GYP_DEFINES += "${ARMFPABI}

>>> release_extra_cflags='-Wno-error=unused-local-typedefs' sysroot=''"

>>> +GYP_DEFINES += " ${ARMFPABI}

>>> release_extra_cflags='-Wno-error=unused-local-typedefs' sysroot='' \

>>> +       ${@bb.utils.contains("AVAILTUNES", "mips", "",

>>> "release_extra_cflags='-fno-delete-null-pointer-checks'", d)}"

>>

>>

>> What's the significance of skipping the workaround if AVAILTUNES

>> contains "mips" ?

>

>

> This change (to add '-fno-delete-null-pointer-checks' to gcc flags) was

> initially merged into chromium directly,

> but soon reverted as it broke mips somehow:

> https://bugs.chromium.org/p/v8/issues/detail?id=3782#c10


OK, thanks. Note that building for MIPS isn't currently supported at
all according to the COMPATIBLE_MACHINE definitions at the top of the
recipe.

>>>   GYP_DEFINES_append_x86 = " generate_character_data=0"

>>>

>>>   do_configure() {

>>> --

>>> 2.1.4

>>>

>>> --

>>> _______________________________________________

>>> Openembedded-devel mailing list

>>> Openembedded-devel@lists.openembedded.org

>>> http://lists.openembedded.org/mailman/listinfo/openembedded-devel

>

>

-- 
_______________________________________________
Openembedded-devel mailing list
Openembedded-devel@lists.openembedded.org
http://lists.openembedded.org/mailman/listinfo/openembedded-devel
Andrey Konovalov April 11, 2017, 7:42 p.m. UTC | #5
On 04/11/2017 09:51 PM, Andre McCurdy wrote:
> On Tue, Apr 11, 2017 at 6:02 AM, Andrey Konovalov

> <andrey.konovalov@linaro.org> wrote:

>> On 04/11/2017 03:40 AM, Andre McCurdy wrote:

>>>

>>> On Fri, Apr 7, 2017 at 11:40 AM, Andrey Konovalov

>>> <andrey.konovalov@linaro.org> wrote:

<snip>
>>>> -GYP_DEFINES += "${ARMFPABI}

>>>> release_extra_cflags='-Wno-error=unused-local-typedefs' sysroot=''"

>>>> +GYP_DEFINES += " ${ARMFPABI}

>>>> release_extra_cflags='-Wno-error=unused-local-typedefs' sysroot='' \

>>>> +       ${@bb.utils.contains("AVAILTUNES", "mips", "",

>>>> "release_extra_cflags='-fno-delete-null-pointer-checks'", d)}"

>>>

>>>

>>> What's the significance of skipping the workaround if AVAILTUNES

>>> contains "mips" ?

>>

>>

>> This change (to add '-fno-delete-null-pointer-checks' to gcc flags) was

>> initially merged into chromium directly,

>> but soon reverted as it broke mips somehow:

>> https://bugs.chromium.org/p/v8/issues/detail?id=3782#c10

>

> OK, thanks. Note that building for MIPS isn't currently supported at

> all according to the COMPATIBLE_MACHINE definitions at the top of the

> recipe.


Indeed.
So my check for "mips" in AVAILTUNES is a NOP until someone enables building for MIPS.
Thanks for pointing that out!
-- 
_______________________________________________
Openembedded-devel mailing list
Openembedded-devel@lists.openembedded.org
http://lists.openembedded.org/mailman/listinfo/openembedded-devel
diff mbox

Patch

diff --git a/recipes-browser/chromium/chromium.inc b/recipes-browser/chromium/chromium.inc
index 60edccb..7e057ec 100644
--- a/recipes-browser/chromium/chromium.inc
+++ b/recipes-browser/chromium/chromium.inc
@@ -15,7 +15,8 @@  CHROMIUM_BUILD_TYPE ??= "Release"
 inherit gettext pythonnative
 
 ARMFPABI_armv7a = "${@bb.utils.contains('TUNE_FEATURES', 'callconvention-hard', 'arm_float_abi=hard', 'arm_float_abi=softfp', d)}"
-GYP_DEFINES += "${ARMFPABI} release_extra_cflags='-Wno-error=unused-local-typedefs' sysroot=''"
+GYP_DEFINES += " ${ARMFPABI} release_extra_cflags='-Wno-error=unused-local-typedefs' sysroot='' \
+	${@bb.utils.contains("AVAILTUNES", "mips", "", "release_extra_cflags='-fno-delete-null-pointer-checks'", d)}"
 GYP_DEFINES_append_x86 = " generate_character_data=0"
 
 do_configure() {