Bug 217514 - [GLIB] Fix linking error for systems not providing <sys/memfd.h>
Summary: [GLIB] Fix linking error for systems not providing <sys/memfd.h>
Alias: None
Product: WebKit
Classification: Unclassified
Component: New Bugs (show other bugs)
Version: WebKit Nightly Build
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Sergio Villar Senin
Keywords: InRadar
Depends on:
Reported: 2020-10-09 04:35 PDT by Sergio Villar Senin
Modified: 2020-10-14 10:45 PDT (History)
12 users (show)

See Also:

Patch (1.27 KB, patch)
2020-10-09 04:38 PDT, Sergio Villar Senin
no flags Details | Formatted Diff | Diff
Patch (3.26 KB, patch)
2020-10-09 05:13 PDT, Sergio Villar Senin
no flags Details | Formatted Diff | Diff
Patch (1.95 KB, patch)
2020-10-13 02:10 PDT, Sergio Villar Senin
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Sergio Villar Senin 2020-10-09 04:35:49 PDT
[GLIB] Fix linking error for systems not providing <sys/memfd.h>
Comment 1 Sergio Villar Senin 2020-10-09 04:38:40 PDT
Created attachment 410931 [details]
Comment 2 Sergio Villar Senin 2020-10-09 05:13:35 PDT
Created attachment 410934 [details]
Comment 3 Sergio Villar Senin 2020-10-09 05:14:33 PDT
Thanks for the review Carlos!.

Adrian suggested to do a compile check, so I'm attaching an alternative patch with some extra cmake changes.
Comment 4 Adrian Perez 2020-10-09 05:19:38 PDT
Comment on attachment 410931 [details]

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

I think there is a better approach, as outlined below.

> Source/WebKit/UIProcess/Launcher/glib/BubblewrapLauncher.cpp:45
>  #include <sys/memfd.h>

It seems that there was a mistake in the manual pages for memfd_create,
and it was never intended for it to suggest using <sys/memfd.h>. There is
some discussion about that here.


> Source/WebKit/UIProcess/Launcher/glib/BubblewrapLauncher.cpp:49
> +#include <sys/mman.h>

The <sys/mman.h> header always exists, we do not need to check whether it's available.
What we can do is:

  1. Always include <sys/mman.h>. This also works for other systems, for example
     FreeBSD also has a memfd_create() function defined in this header:

  2. If the MFD_* macros are not defined, include <linux/memfd.h> and use syscall2()
     Note: Use this fall only for linux, e.g. with a __has_include(<linux/memfd.h>) guard.
Comment 5 Adrian Perez 2020-10-09 05:22:05 PDT
On a second thought, I think we do not need the CMake compile check
when using the approach of always using <sys/mman.h> and the fallback
with <linux/memfd.h>+syscall2() only on Linux. That will also cover
the BSDs which implement a compatible memfd_create() function, without
adding complexity in CMake.

BTW, thanks Sergio for the patch, and sorry for the back and forth
about the CMake check O:-)
Comment 6 Konstantin Tokarev 2020-10-09 05:28:06 PDT
Does bubblewrap work on BSD at all? BubblewrapLauncher makes unconditional use of seccomp which is Linux-only.
Comment 7 Konstantin Tokarev 2020-10-09 06:58:14 PDT
OTOH, we have other usage of memfd in Platform/unix/SharedMemoryUnix.cpp, where it indeed could be checked in more cross-platform way.
Comment 8 Patrick Griffis 2020-10-09 08:07:50 PDT
(In reply to Konstantin Tokarev from comment #6)
> Does bubblewrap work on BSD at all?

No it is Linux specific.
Comment 9 Carlos Garcia Campos 2020-10-11 00:24:04 PDT
Comment on attachment 410934 [details]

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

> Source/cmake/OptionsCommon.cmake:-126

We use this in SharedMemoryUnix.cpp
Comment 10 Sergio Villar Senin 2020-10-13 02:10:18 PDT
Created attachment 411203 [details]
Comment 11 Konstantin Tokarev 2020-10-13 02:21:29 PDT
I think it would be more correct to add compilation tests for memfd_create and use its results in all places where memfd is used
Comment 12 EWS 2020-10-14 10:44:58 PDT
Committed r268472: <https://trac.webkit.org/changeset/268472>

All reviewed patches have been landed. Closing bug and clearing flags on attachment 411203 [details].
Comment 13 Radar WebKit Bug Importer 2020-10-14 10:45:22 PDT