232452 2021-10-28 13:23:01 -0700 Build failure with glibc 2.26 2021-11-22 06:07:09 -0800 1 1 1 Unclassified WebKit WebKitGTK WebKit Nightly Build All Linux NEW P2 Normal --- 1 mgorse mcatanzaro aperez bugs-noreply mcatanzaro mgorse oldest_to_newest 1809801 0 mgorse 2021-10-28 13:23:01 -0700 The build fails with glibc 2.26 after the fix for bug 231479. We have this conditional #if !defined(MFD_ALLOW_SEALING) && HAVE(LINUX_MEMFD_H) guarding the definition for our memfd_create implementation, but now MEMFD_ALLOW_SEALING can be defined, even if glibc hasn't defined it, because it might be defined in linux/memfd.h, which now we've already included. 1809807 1 442735 mgorse 2021-10-28 13:27:47 -0700 Created attachment 442735 Patch. 1809828 2 442735 mcatanzaro 2021-10-28 14:13:29 -0700 Comment on attachment 442735 Patch. View in context: https://bugs.webkit.org/attachment.cgi?id=442735&action=review > Source/WebKit/UIProcess/Launcher/glib/BubblewrapLauncher.cpp:51 > -#if !defined(MFD_ALLOW_SEALING) && HAVE(LINUX_MEMFD_H) > +#if HAVE(LINUX_MEMFD_H) && !__GLIBC_PREREQ(2, 27) Mmmm... we have to be friendly to other libcs, though. I think this will go badly if __GLIBC_PREREQ is not defined. I'm also suspicious that this guard no longer matches the guard around #include <linux/memfd.h>. I think the intent is: if the libc supports memfd_create(), then MFD_ALLOW_SEALING is defined in sys/mman.h and we can use the libc memfd_create(). If not, then #include linux/memfd and define everything ourselves. But after my commit, that's broken, because #including linux/memfd.h can define MFD_ALLOW_SEALING. Maybe a simpler solution would be to just revert this back to how it was before? The style checker will not be happy -- it whines when the #includes are not alphabetical -- but in this case we seem to have a good reason to ignore it. I would also add a warning comment to indicate why the #includes have to be in a special order. Sound good? Then we don't need __GLIBC_PREREQ, which seems fragile. (Especially bad for things like uclibc-ng, which pretends to be glibc.) 1809855 3 mgorse 2021-10-28 15:01:05 -0700 Thanks for the review. That's fine with me. I was assuming that glibc would be in use because linux/memfd.h would only exist on Linux, but I wasn't thinking about uclibc-ng... 1810127 4 442833 mcatanzaro 2021-10-29 10:16:06 -0700 Created attachment 442833 Patch 1810129 5 mcatanzaro 2021-10-29 10:17:06 -0700 Please make sure my patch works for you! Thanks. 1810212 6 mgorse 2021-10-29 13:30:13 -0700 +#include <linux/memfd.h> Does this need to be guarded by #if HAVE_LINUX_FD_H? The old code had the entire block guarded by both. Anyway, I'm testing a build on SLE-15-SP2 (which is where I was seeing the failure). 1810269 7 442833 aperez 2021-10-29 15:02:28 -0700 Comment on attachment 442833 Patch View in context: https://bugs.webkit.org/attachment.cgi?id=442833&action=review > Source/WebKit/UIProcess/Launcher/glib/BubblewrapLauncher.cpp:-38 > - May I suggest this solution, which works everywhere, for all the libc implementations we support: #if __has_include(<linux/memfd.h>) #include <linux/meemfd.h> #endif #ifndef MFD_ALLOW_SEALING #define MFD_ALLOW_SEALING 0x0002U #endif =) 1810708 8 mgorse 2021-11-01 11:38:46 -0700 Builds for me with the patch, fwiw. 1810874 9 mcatanzaro 2021-11-01 14:27:34 -0700 (In reply to Adrian Perez from comment #7) > May I suggest this solution, which works everywhere, for all the libc > implementations we support: > > #if __has_include(<linux/memfd.h>) > #include <linux/meemfd.h> > #endif Again, there is no point to this guard because the build cannot succeed without it. There is no fallback for lack of linux/memfd.h if memfd stuff isn't defined in sys/mman.h, and we cannot add one. I could add an #error to fail the build more cleanly in this case, though. > #ifndef MFD_ALLOW_SEALING > #define MFD_ALLOW_SEALING 0x0002U > #endif > > =) That's not right, because if we #include linux/memfd.h, then we have MFD_ALLOW_SEALING for sure. 1810876 10 mcatanzaro 2021-11-01 14:28:59 -0700 > we cannot add one Note I'm assuming lack of linux/memfd.h indicates old kernel. If we're just missing kernel headers, hmm, that we *could* try to handle. But then we have to fail at runtime rather than build time as syscall() could fail on ancient kernels. 1811878 11 mcatanzaro 2021-11-04 09:27:12 -0700 Adrian, any further thoughts? 1817276 12 mcatanzaro 2021-11-22 06:07:09 -0800 Ping Adrian 442735 2021-10-28 13:27:47 -0700 2021-10-29 10:15:58 -0700 Patch. build.patch text/plain 1243 mgorse ZGlmZiAtLWdpdCBhL1NvdXJjZS9XZWJLaXQvQ2hhbmdlTG9nIGIvU291cmNlL1dlYktpdC9DaGFu Z2VMb2cKaW5kZXggNDVkNGE4NDNiYTc5Li5hZDA2YjEyZTNkOWUgMTAwNjQ0Ci0tLSBhL1NvdXJj ZS9XZWJLaXQvQ2hhbmdlTG9nCisrKyBiL1NvdXJjZS9XZWJLaXQvQ2hhbmdlTG9nCkBAIC0xLDMg KzEsMTMgQEAKKzIwMjEtMTAtMjggIE1pa2UgR29yc2UgIDxtZ29yc2VAc3VzZS5jb20+CisKKyAg ICAgICAgQnVpbGQgZmFpbHVyZSB3aXRoIGdsaWJjIDIuMjYKKyAgICAgICAgaHR0cHM6Ly9idWdz LndlYmtpdC5vcmcvc2hvd19idWcuY2dpP2lkPTIzMjQ1MgorCisgICAgICAgIFJldmlld2VkIGJ5 IE5PQk9EWSAoT09QUyEpLgorCisgICAgICAgICogVUlQcm9jZXNzL0xhdW5jaGVyL2dsaWIvQnVi Ymxld3JhcExhdW5jaGVyLmNwcDogVXNlIGdsaWJjCisgICAgICAgIHZlcnNpb24gY2hlY2sgdG8g Z3VhcmQgZGVmaW5pdGlvbiBvZiBtZW1mZF9jcmVhdGUgYW5kIHJlbGF0ZWQgbWFjcm9zLgorCiAy MDIxLTEwLTI4ICBUaW0gSG9ydG9uICA8dGltb3RoeV9ob3J0b25AYXBwbGUuY29tPgogCiAgICAg ICAgIFVucmV2aWV3ZWQgYnVpbGQgZml4LgpkaWZmIC0tZ2l0IGEvU291cmNlL1dlYktpdC9VSVBy b2Nlc3MvTGF1bmNoZXIvZ2xpYi9CdWJibGV3cmFwTGF1bmNoZXIuY3BwIGIvU291cmNlL1dlYktp dC9VSVByb2Nlc3MvTGF1bmNoZXIvZ2xpYi9CdWJibGV3cmFwTGF1bmNoZXIuY3BwCmluZGV4IGU5 ZmU1MzMzNGViNi4uNmNiZDAxNTNkODhlIDEwMDY0NAotLS0gYS9Tb3VyY2UvV2ViS2l0L1VJUHJv Y2Vzcy9MYXVuY2hlci9nbGliL0J1YmJsZXdyYXBMYXVuY2hlci5jcHAKKysrIGIvU291cmNlL1dl YktpdC9VSVByb2Nlc3MvTGF1bmNoZXIvZ2xpYi9CdWJibGV3cmFwTGF1bmNoZXIuY3BwCkBAIC00 OCw3ICs0OCw3IEBACiAjZGVmaW5lIEJBU0VfRElSRUNUT1JZICJ3cGUiCiAjZW5kaWYKIAotI2lm ICFkZWZpbmVkKE1GRF9BTExPV19TRUFMSU5HKSAmJiBIQVZFKExJTlVYX01FTUZEX0gpCisjaWYg SEFWRShMSU5VWF9NRU1GRF9IKSAmJiAhX19HTElCQ19QUkVSRVEoMiwgMjcpCiAKIC8vIFRoZXNl IGRlZmluZXMgd2VyZSBhZGRlZCBpbiBnbGliYyAyLjI3LCB0aGUgc2FtZSByZWxlYXNlIHRoYXQg YWRkZWQgbWVtZmRfY3JlYXRlLgogLy8gQnV0IHRoZSBrZXJuZWwgYWRkZWQgYWxsIG9mIHRoaXMg aW4gTGludXggMy4xNy4gU28gaXQncyB0b3RhbGx5IHNhZmUgZm9yIHVzIHRvCg== 442833 2021-10-29 10:16:06 -0700 2021-10-29 15:02:28 -0700 Patch bug-232452-20211029121605.patch text/plain 2369 mcatanzaro U3VidmVyc2lvbiBSZXZpc2lvbjogMjg1MDMyCmRpZmYgLS1naXQgYS9Tb3VyY2UvV2ViS2l0L0No YW5nZUxvZyBiL1NvdXJjZS9XZWJLaXQvQ2hhbmdlTG9nCmluZGV4IDUzNTAwYmM3ZjU3ZDRkNTVh NmI0NmU0NjgyNTgyZDJlMzI2NDgyYjUuLjUxOTQ2MGM5YjQ4MmRhMTAzMGMzMTZmODg0YTlkMjY1 NThjNWY1OTkgMTAwNjQ0Ci0tLSBhL1NvdXJjZS9XZWJLaXQvQ2hhbmdlTG9nCisrKyBiL1NvdXJj ZS9XZWJLaXQvQ2hhbmdlTG9nCkBAIC0xLDMgKzEsMTcgQEAKKzIwMjEtMTAtMjkgIE1pY2hhZWwg Q2F0YW56YXJvICA8bWNhdGFuemFyb0Bnbm9tZS5vcmc+CisKKyAgICAgICAgQnVpbGQgZmFpbHVy ZSB3aXRoIGdsaWJjIDIuMjYKKyAgICAgICAgaHR0cHM6Ly9idWdzLndlYmtpdC5vcmcvc2hvd19i dWcuY2dpP2lkPTIzMjQ1MgorCisgICAgICAgIFJldmlld2VkIGJ5IE5PQk9EWSAoT09QUyEpLgor CisgICAgICAgIERvbid0IGluY2x1ZGUgbGludXgvbWVtZmQuaCB1bnRpbCBhZnRlciB3ZSd2ZSB0 ZXN0ZWQgTUZEX0FMTE9XX1NFQUxJTkcuCisKKyAgICAgICAgQWxzbywgdGhlIGJ1aWxkIGlzIGdv aW5nIHRvIGZhaWwgbGF0ZXIgb24gaWYgbWlzc2luZyBsaW51eC9tZW1mZC5oLCBzbyBtaWdodCBh cyB3ZWxsCisgICAgICAgIGFzc3VtZSBpdCBpcyBhdmFpbGFibGUuIEtlcm5lbHMgdGhpcyBvbGQg cHJvYmFibHkgZG9uJ3Qgc3VwcG9ydCBidWJibGV3cmFwIGFueXdheS4KKworICAgICAgICAqIFVJ UHJvY2Vzcy9MYXVuY2hlci9nbGliL0J1YmJsZXdyYXBMYXVuY2hlci5jcHA6CisKIDIwMjEtMTAt MjkgIEF5dW1pIEtvamltYSAgPGF5dW1pX2tvamltYUBhcHBsZS5jb20+CiAKICAgICAgICAgVW5y ZXZpZXdlZCwgcmV2ZXJ0aW5nIHIyODUwMDcuCmRpZmYgLS1naXQgYS9Tb3VyY2UvV2ViS2l0L1VJ UHJvY2Vzcy9MYXVuY2hlci9nbGliL0J1YmJsZXdyYXBMYXVuY2hlci5jcHAgYi9Tb3VyY2UvV2Vi S2l0L1VJUHJvY2Vzcy9MYXVuY2hlci9nbGliL0J1YmJsZXdyYXBMYXVuY2hlci5jcHAKaW5kZXgg MGQ1ZGQ0ZjY5ODZkNjczM2Y2ZmZjYjY4YjlmYWI5MTEzYmI1MzI3Yi4uZGZhOWFjMzlhMDQ1ZTdj Y2U5ZGMyMzFjNTI5MTE1OTYzZWMwMjFmNiAxMDA2NDQKLS0tIGEvU291cmNlL1dlYktpdC9VSVBy b2Nlc3MvTGF1bmNoZXIvZ2xpYi9CdWJibGV3cmFwTGF1bmNoZXIuY3BwCisrKyBiL1NvdXJjZS9X ZWJLaXQvVUlQcm9jZXNzL0xhdW5jaGVyL2dsaWIvQnViYmxld3JhcExhdW5jaGVyLmNwcApAQCAt MzIsMTAgKzMyLDYgQEAKICNpbmNsdWRlIDx3dGYvZ2xpYi9HUmVmUHRyLmg+CiAjaW5jbHVkZSA8 d3RmL2dsaWIvR1VuaXF1ZVB0ci5oPgogCi0jaWYgIWRlZmluZWQoTUZEX0FMTE9XX1NFQUxJTkcp ICYmIEhBVkUoTElOVVhfTUVNRkRfSCkKLSNpbmNsdWRlIDxsaW51eC9tZW1mZC5oPgotI2VuZGlm Ci0KICNpbmNsdWRlICJTeXNjYWxscy5oIgogCiAjaWYgUExBVEZPUk0oR1RLKQpAQCAtNDgsMTIg KzQ0LDE2IEBACiAjZGVmaW5lIEJBU0VfRElSRUNUT1JZICJ3cGUiCiAjZW5kaWYKIAotI2lmICFk ZWZpbmVkKE1GRF9BTExPV19TRUFMSU5HKSAmJiBIQVZFKExJTlVYX01FTUZEX0gpCi0KKyNpZiAh ZGVmaW5lZChNRkRfQUxMT1dfU0VBTElORykKIC8vIFRoZXNlIGRlZmluZXMgd2VyZSBhZGRlZCBp biBnbGliYyAyLjI3LCB0aGUgc2FtZSByZWxlYXNlIHRoYXQgYWRkZWQgbWVtZmRfY3JlYXRlLgog Ly8gQnV0IHRoZSBrZXJuZWwgYWRkZWQgYWxsIG9mIHRoaXMgaW4gTGludXggMy4xNy4gU28gaXQn cyB0b3RhbGx5IHNhZmUgZm9yIHVzIHRvCiAvLyBkZXBlbmQgb24sIGFzIGxvbmcgYXMgd2UgZGVm aW5lIGl0IGFsbCBvdXJzZWx2ZXMuIFJlbW92ZSB0aGlzIG9uY2Ugd2UgZGVwZW5kIG9uCiAvLyBn bGliYyAyLjI3LgorLy8KKy8vIFAuUy4gbGludXgvbWVtZmQuaCBkZWZpbmVzIE1GRF9BTExPV19T RUFMSU5HLCBzbyBtdXN0IG5vdCBiZSBpbmNsdWRlZCBwcmlvcgorLy8gdG8gdGhpcyBwb2ludC4K KworI2luY2x1ZGUgPGxpbnV4L21lbWZkLmg+CiAKICNkZWZpbmUgRl9BRERfU0VBTFMgMTAzMwog I2RlZmluZSBGX0dFVF9TRUFMUyAxMDM0CkBAIC02Nyw3ICs2Nyw3IEBAIHN0YXRpYyBpbnQgbWVt ZmRfY3JlYXRlKGNvbnN0IGNoYXIqIG5hbWUsIHVuc2lnbmVkIGZsYWdzKQogewogICAgIHJldHVy biBzeXNjYWxsKF9fTlJfbWVtZmRfY3JlYXRlLCBuYW1lLCBmbGFncyk7CiB9Ci0jZW5kaWYgLy8g I2lmICFkZWZpbmVkKE1GRF9BTExPV19TRUFMSU5HKSAmJiBIQVZFKExJTlVYX01FTUZEX0gpCisj ZW5kaWYgLy8gI2lmICFkZWZpbmVkKE1GRF9BTExPV19TRUFMSU5HKQogCiBuYW1lc3BhY2UgV2Vi S2l0IHsKIHVzaW5nIG5hbWVzcGFjZSBXZWJDb3JlOwo=