1587116 - Build fails in skia on OpenBSD/aarch64 due to sys/auxv.h inclusion problem

Status: RESOLVED FIXED

(Core :: Graphics, defect, P5)

Description

[stu-mozbug]

User Agent: Mozilla/5.0 (X11; OpenBSD amd64; rv:69.0) Gecko/20100101 Firefox/69.0

Steps to reproduce:

Attempt to build Firefox on OpenBSD/aarch64.

Actual results:

In file included from /usr/obj/ports/firefox-69.0.2/build-aarch64/gfx/skia/Unified_cpp_gfx_skia1.cpp:137:
In file included from /usr/obj/ports/firefox-69.0.2/firefox-69.0.2/gfx/skia/skia/src/core/SkCpu.cpp:74:
/usr/obj/ports/firefox-69.0.2/build-aarch64/dist/system_wrappers/sys/auxv.h:3:15: fatal error: 'sys/auxv.h' file not found
#include_next <sys/auxv.h>
^~~~~~~~~~~~

Expected results:

skia uses "__has_include(<sys/auxv.h>)" to decide whether the OS has auxv.h. For some reason the generated dist/system_wrappers/sys/auxv.h appears to satisfy this requirement so the "#if __has_include" codepath is taken despite the OS not having this header.

This naive change to gfx/skia/skia/src/core/SkCpu.cpp allows it to build:

+-#elif defined(SK_CPU_ARM64) && __has_include(<sys/auxv.h>)
++#elif !defined(OpenBSD) && defined(SK_CPU_ARM64) && __has_include(<sys/auxv.h>)

Comment 1

[BugBot [:suhaib / :marco/ :calixte]]

Bugbug thinks this bug should belong to this component, but please revert this change in case of error.

Comment 2

[Kris Taeleman (:ktaeleman)]

Hi Landry, it looks like you've been maintaining the OpenBSD port so I'm adding you to see if this is something you're familiar with.

Comment 3

[Landry Breuil (:gaston)]

Jan, have you seen this on FreeBSD/arm64 ? I thought it was something already reported by you but i couldnt find anything...

(In reply to Kris Taeleman (:kris/kris) from comment #2)

Hi Landry, it looks like you've been maintaining the OpenBSD port so I'm adding you to see if this is something you're familiar with.

thanks, i have zero experience with arm64 though :)

Comment 4

[Landry Breuil (:gaston)]

Vlad (or anyone else comfortable with skia), since system_wrappers ships a fake auxv.h __has_include will always be defined.. what should be done when it doesnt exist in the end on the underlying OS ?

Comment 5

[Jan Beich]

Attachment: v1

Sorry, I can't submit on Phabricator due to bug 1536716.
https://treeherder.mozilla.org/#/jobs?repo=try&revision=37cdab961b68fefab4ee26ad1a89658b43073f58

(In reply to Landry Breuil (:gaston) from comment #3)

Jan, have you seen this on FreeBSD/arm64 ? I thought it was something already reported by you but i couldnt find anything...

FreeBSD has <sys/auxv.h> since 12.0-RELEASE but it contains elf_aux_info rather than getauxval. FreeBSD also has aarch64-specific way to detect CPU features which can be used on older releases. See also bug 1575843.
https://github.com/freebsd/freebsd-ports/blob/master/www/firefox/files/patch-gfx_skia_skia_src_core_SkCpu.cpp

(In reply to Landry Breuil (:gaston) from comment #4)

what should be done when it doesnt exist in the end on the underlying OS ?

__has_include_next may help but I couldn't figure out how to make the wrappers respect __has_include.
https://clang.llvm.org/docs/LanguageExtensions.html#langext-has-include-next
https://gcc.gnu.org/onlinedocs/cpp/Wrapper-Headers.html

Comment 6

[Landry Breuil (:gaston)]

Sorry for the long delay - this looks like a correct fix, i'm trying to find someone with an OpenBSD/arm64 to testbuild that but i think it should be commited. :kris, is this your area or config/system-headers.mozbuild changes should get reviewed by build peers only ?

Comment 7

[Landry Breuil (:gaston)]

Comment on attachment 9102723 [details] [diff] [review]
v1

Tested working fine on OpenBSD/arm64, mike can you push this ?

Comment 8

[Kris Taeleman (:ktaeleman)]

Hey Landry, build peers are probably the best for reviewing this as I have no experience with this.
Thanks for helping with this!

Kris

Comment 9

[Landry Breuil (:gaston)]

Attachment: Bug 1587116 - only include sys/auxv.h on platforms where it might exist r?glandium

Comment 10

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/d3885b9d42d3
only include sys/auxv.h on platforms where it might exist r=glandium

Comment 11

[Alexandru Michis [:malexandru]]

https://hg.mozilla.org/mozilla-central/rev/d3885b9d42d3