172445 2017-05-22 06:40:28 -0700 [WTF] Compilation fails with system malloc 2019-01-16 14:32:26 -0800 1 1 1 Unclassified WebKit Web Template Framework WebKit Nightly Build Unspecified Unspecified RESOLVED FIXED P2 Normal --- 1 tpopela tpopela benjamin berto buildbot cdumez cgarcia cmarcelo dbates mcatanzaro simon.fraser oldest_to_newest 1311317 0 tpopela 2017-05-22 06:40:28 -0700 ../../lib/../Source/WTF/wtf/CMakeFiles/WTF.dir/RAMSize.cpp.o: In function `bmalloc::api::availableMemory()': /builddir/build/BUILD/webkitgtk-2.17.3/Source/bmalloc/bmalloc/bmalloc.h:91: undefined reference to `bmalloc::availableMemory()' collect2: error: ld returned 1 exit status 1311321 1 310860 tpopela 2017-05-22 06:47:43 -0700 Created attachment 310860 Patch 1311324 2 310862 tpopela 2017-05-22 07:02:11 -0700 Created attachment 310862 Patch 1311332 3 310862 mcatanzaro 2017-05-22 07:20:43 -0700 Comment on attachment 310862 Patch Hm, good catch, thanks, but I think this should ensure the build fails if USE_SYSTEM_MALLOC is OFF and OS is not UNIX. We should not allow other platforms to remain broken at runtime without any compiler error. 1311354 4 tpopela 2017-05-22 07:59:35 -0700 (In reply to Michael Catanzaro from comment #3) > Comment on attachment 310862 [details] > Patch > > Hm, good catch, thanks, but I think this should ensure the build fails if > USE_SYSTEM_MALLOC is OFF and OS is not UNIX. We should not allow other > platforms to remain broken at runtime without any compiler error. What do you suggest? "Missing a platform specific way of determining the available RAM" ? 1311364 5 mcatanzaro 2017-05-22 08:21:00 -0700 That sounds good. 1311380 6 310884 tpopela 2017-05-22 08:57:30 -0700 Created attachment 310884 Patch Fail with an error message if not on UNIX and also return memory in bytes 1311383 7 310884 mcatanzaro 2017-05-22 09:02:53 -0700 Comment on attachment 310884 Patch View in context: https://bugs.webkit.org/attachment.cgi?id=310884&action=review > Source/WTF/wtf/RAMSize.cpp:38 > +#error "Missing a platform specific way of determining the available RAM" r=me if you move the #error down to computeRAMSize() instead of doing it up here. 1311784 8 tpopela 2017-05-23 00:17:38 -0700 Committed r217270: <http://trac.webkit.org/changeset/217270> 1442226 9 simon.fraser 2018-07-14 11:16:46 -0700 Compilation is still broken on macOS with USE_SYSTEM_MALLOC defined: CompileC RAMSize.o /Volumes/Data/Development/apple/webkit/OpenSource/Source/WTF/wtf/RAMSize.cpp:36:10: fatal error: 'sys/sysinfo.h' file not found #include <sys/sysinfo.h> ^~~~~~~~~~~~~~~ 1442227 10 simon.fraser 2018-07-14 11:18:35 -0700 I would think that it's OK to use bmalloc::api::availableMemory() even if using system malloc, at least on macOS/iOS. 1442230 11 mcatanzaro 2018-07-14 11:23:48 -0700 It's definitely not OK for any CMake ports (including Mac's CMake build) because no code under the bmalloc dir is compiled when USE_SYSTEM_MALLOC is enabled. 1492908 12 berto 2019-01-09 03:13:41 -0800 (In reply to Michael Catanzaro from comment #3) > I think this should ensure the build fails if USE_SYSTEM_MALLOC is > OFF and OS is not UNIX. We should not allow other platforms to > remain broken at runtime without any compiler error. For the record that "OS is not UNIX" part is incorrect: the sysinfo() call we're using to determine the amount of memory is specific to Linux. In particular it's not available in FreeBSD, OpenBSD, NetBSD or the Hurd. So the code should probably use OS(LINUX) instead. diff --git a/Source/WTF/wtf/RAMSize.cpp b/Source/WTF/wtf/RAMSize.cpp index b64dfea3402..2e69ba93149 100644 --- a/Source/WTF/wtf/RAMSize.cpp +++ b/Source/WTF/wtf/RAMSize.cpp @@ -32,9 +32,9 @@ #if OS(WINDOWS) #include <windows.h> #elif defined(USE_SYSTEM_MALLOC) && USE_SYSTEM_MALLOC -#if OS(UNIX) +#if OS(LINUX) #include <sys/sysinfo.h> -#endif // OS(UNIX) +#endif // OS(LINUX) #else #include <bmalloc/bmalloc.h> #endif @@ -55,13 +55,13 @@ static size_t computeRAMSize() return ramSizeGuess; return status.ullTotalPhys; #elif defined(USE_SYSTEM_MALLOC) && USE_SYSTEM_MALLOC -#if OS(UNIX) +#if OS(LINUX) struct sysinfo si; sysinfo(&si); return si.totalram * si.mem_unit; #else #error "Missing a platform specific way of determining the available RAM" -#endif // OS(UNIX) +#endif // OS(LINUX) #else return bmalloc::api::availableMemory(); #endif 1495129 13 mcatanzaro 2019-01-16 11:22:41 -0800 For the record: r=me 1495237 14 berto 2019-01-16 14:32:26 -0800 (In reply to Michael Catanzaro from comment #13) > For the record: r=me Committed r240038: <https://trac.webkit.org/changeset/240038/webkit> 310860 2017-05-22 06:47:43 -0700 2017-05-22 07:02:07 -0700 Patch bug-172445-20170522154742.patch text/plain 1775 tpopela U3VidmVyc2lvbiBSZXZpc2lvbjogMjE3MjEwCmRpZmYgLS1naXQgYS9Tb3VyY2UvV1RGL0NoYW5n ZUxvZyBiL1NvdXJjZS9XVEYvQ2hhbmdlTG9nCmluZGV4IGNiOWFlYTcxZjA0NGZkMWJhOTRmNTUw NzFkODMzYzkyMjA4NmQ5MDEuLmQ1MGFmYzEzZTk3MzA2MTYxZmYwYTYzMGFmNGIxOWU0OGM0YTZj MzYgMTAwNjQ0Ci0tLSBhL1NvdXJjZS9XVEYvQ2hhbmdlTG9nCisrKyBiL1NvdXJjZS9XVEYvQ2hh bmdlTG9nCkBAIC0xLDMgKzEsMTggQEAKKzIwMTctMDUtMjIgIFRvbWFzIFBvcGVsYSAgPHRwb3Bl bGFAcmVkaGF0LmNvbT4KKworICAgICAgICBbV1RGXSBDb21waWxhdGlvbiBmYWlscyB3aXRoIHN5 c3RlbSBtYWxsb2MKKyAgICAgICAgaHR0cHM6Ly9idWdzLndlYmtpdC5vcmcvc2hvd19idWcuY2dp P2lkPTE3MjQ0NQorCisgICAgICAgIFJldmlld2VkIGJ5IE5PQk9EWSAoT09QUyEpLgorCisgICAg ICAgIFdlIGFyZSB1c2luZyB0aGUgYm1hbGxvYyBldmVuIGlmIHRoZSBzeXN0ZW0gbWFsbG9jIHNo b3VsZCBiZSB1c2VkLgorICAgICAgICBEb24ndCB1c2UgYm1hbGxvYyBpZiB0aGUgc3lzdGVtIG1h bGxvYyBpcyByZXF1ZXN0ZWQgYW5kIGFkZCB0aGUKKyAgICAgICAgbWlzc2luZyBpbXBsZW1lbnRh dGlvbiBmb3IgY29tcHV0ZVJBTVNpemUoKSBvbiBVTklYIHRoYXQncyB1dGlsaXppbmcKKyAgICAg ICAgdGhlIHN5c2luZm8oKSBjYWxsLgorCisgICAgICAgICogd3RmL1JBTVNpemUuY3BwOgorICAg ICAgICAoV1RGOjpjb21wdXRlUkFNU2l6ZSk6CisKIDIwMTctMDUtMTkgIERvbiBPbG1zdGVhZCAg PGRvbi5vbG1zdGVhZEBhbS5zb255LmNvbT4KIAogICAgICAgICBbV1RGXSBSZW1vdmUgUExBVEZP Uk0oV0lOKSByZWZlcmVuY2VzCmRpZmYgLS1naXQgYS9Tb3VyY2UvV1RGL3d0Zi9SQU1TaXplLmNw cCBiL1NvdXJjZS9XVEYvd3RmL1JBTVNpemUuY3BwCmluZGV4IGIwMTdhMjZiZGUzZDJiNGEyNmVj Mjc5OTFkOGZlYzRjMzQxODI5MzAuLmM2NDczMDdhYjkxNjgxZmNjZDc0NzFmNjc0MGU2OGEyOTFl YzdlZDcgMTAwNjQ0Ci0tLSBhL1NvdXJjZS9XVEYvd3RmL1JBTVNpemUuY3BwCisrKyBiL1NvdXJj ZS9XVEYvd3RmL1JBTVNpemUuY3BwCkBAIC0zMSwxMiArMzEsMTYgQEAKIAogI2lmIE9TKFdJTkRP V1MpCiAjaW5jbHVkZSA8d2luZG93cy5oPgorI2lmIGRlZmluZWQoVVNFX1NZU1RFTV9NQUxMT0Mp ICYmIFVTRV9TWVNURU1fTUFMTE9DCisjaWYgT1MoVU5JWCkKKyNpbmNsdWRlIDxzeXMvc3lzaW5m by5oPgorI2VuZGlmIC8vIE9TKFVOSVgpCiAjZWxzZQogI2luY2x1ZGUgPGJtYWxsb2MvYm1hbGxv Yy5oPgogI2VuZGlmCiAKIG5hbWVzcGFjZSBXVEYgewotICAgIAorCiAjaWYgT1MoV0lORE9XUykK IHN0YXRpYyBjb25zdCBzaXplX3QgcmFtU2l6ZUd1ZXNzID0gNTEyICogTUI7CiAjZW5kaWYKQEAg LTUwLDYgKzU0LDEyIEBAIHN0YXRpYyBzaXplX3QgY29tcHV0ZVJBTVNpemUoKQogICAgIGlmICgh cmVzdWx0KQogICAgICAgICByZXR1cm4gcmFtU2l6ZUd1ZXNzOwogICAgIHJldHVybiBzdGF0dXMu dWxsVG90YWxQaHlzOworI2VsaWYgZGVmaW5lZChVU0VfU1lTVEVNX01BTExPQykgJiYgVVNFX1NZ U1RFTV9NQUxMT0MKKyNpZiBPUyhVTklYKQorICAgIHN0cnVjdCBzeXNpbmZvIHNpOworICAgIHN5 c2luZm8oJnNpKTsKKyAgICByZXR1cm4gc2kudG90YWxyYW0gLyBNQjsKKyNlbmRpZiAvLyBPUyhV TklYKQogI2Vsc2UKICAgICByZXR1cm4gYm1hbGxvYzo6YXBpOjphdmFpbGFibGVNZW1vcnkoKTsK ICNlbmRpZgo= 310862 2017-05-22 07:02:11 -0700 2017-05-22 08:57:25 -0700 Patch bug-172445-20170522160209.patch text/plain 1777 tpopela U3VidmVyc2lvbiBSZXZpc2lvbjogMjE3MjEwCmRpZmYgLS1naXQgYS9Tb3VyY2UvV1RGL0NoYW5n ZUxvZyBiL1NvdXJjZS9XVEYvQ2hhbmdlTG9nCmluZGV4IGNiOWFlYTcxZjA0NGZkMWJhOTRmNTUw NzFkODMzYzkyMjA4NmQ5MDEuLmQ1MGFmYzEzZTk3MzA2MTYxZmYwYTYzMGFmNGIxOWU0OGM0YTZj MzYgMTAwNjQ0Ci0tLSBhL1NvdXJjZS9XVEYvQ2hhbmdlTG9nCisrKyBiL1NvdXJjZS9XVEYvQ2hh bmdlTG9nCkBAIC0xLDMgKzEsMTggQEAKKzIwMTctMDUtMjIgIFRvbWFzIFBvcGVsYSAgPHRwb3Bl bGFAcmVkaGF0LmNvbT4KKworICAgICAgICBbV1RGXSBDb21waWxhdGlvbiBmYWlscyB3aXRoIHN5 c3RlbSBtYWxsb2MKKyAgICAgICAgaHR0cHM6Ly9idWdzLndlYmtpdC5vcmcvc2hvd19idWcuY2dp P2lkPTE3MjQ0NQorCisgICAgICAgIFJldmlld2VkIGJ5IE5PQk9EWSAoT09QUyEpLgorCisgICAg ICAgIFdlIGFyZSB1c2luZyB0aGUgYm1hbGxvYyBldmVuIGlmIHRoZSBzeXN0ZW0gbWFsbG9jIHNo b3VsZCBiZSB1c2VkLgorICAgICAgICBEb24ndCB1c2UgYm1hbGxvYyBpZiB0aGUgc3lzdGVtIG1h bGxvYyBpcyByZXF1ZXN0ZWQgYW5kIGFkZCB0aGUKKyAgICAgICAgbWlzc2luZyBpbXBsZW1lbnRh dGlvbiBmb3IgY29tcHV0ZVJBTVNpemUoKSBvbiBVTklYIHRoYXQncyB1dGlsaXppbmcKKyAgICAg ICAgdGhlIHN5c2luZm8oKSBjYWxsLgorCisgICAgICAgICogd3RmL1JBTVNpemUuY3BwOgorICAg ICAgICAoV1RGOjpjb21wdXRlUkFNU2l6ZSk6CisKIDIwMTctMDUtMTkgIERvbiBPbG1zdGVhZCAg PGRvbi5vbG1zdGVhZEBhbS5zb255LmNvbT4KIAogICAgICAgICBbV1RGXSBSZW1vdmUgUExBVEZP Uk0oV0lOKSByZWZlcmVuY2VzCmRpZmYgLS1naXQgYS9Tb3VyY2UvV1RGL3d0Zi9SQU1TaXplLmNw cCBiL1NvdXJjZS9XVEYvd3RmL1JBTVNpemUuY3BwCmluZGV4IGIwMTdhMjZiZGUzZDJiNGEyNmVj Mjc5OTFkOGZlYzRjMzQxODI5MzAuLmY5ZGY2YzIwOTI5NGMxNDcwMzY0N2VmM2YxNjYxM2VhYjdk ZTUwMGYgMTAwNjQ0Ci0tLSBhL1NvdXJjZS9XVEYvd3RmL1JBTVNpemUuY3BwCisrKyBiL1NvdXJj ZS9XVEYvd3RmL1JBTVNpemUuY3BwCkBAIC0zMSwxMiArMzEsMTYgQEAKIAogI2lmIE9TKFdJTkRP V1MpCiAjaW5jbHVkZSA8d2luZG93cy5oPgorI2VsaWYgZGVmaW5lZChVU0VfU1lTVEVNX01BTExP QykgJiYgVVNFX1NZU1RFTV9NQUxMT0MKKyNpZiBPUyhVTklYKQorI2luY2x1ZGUgPHN5cy9zeXNp bmZvLmg+CisjZW5kaWYgLy8gT1MoVU5JWCkKICNlbHNlCiAjaW5jbHVkZSA8Ym1hbGxvYy9ibWFs bG9jLmg+CiAjZW5kaWYKIAogbmFtZXNwYWNlIFdURiB7Ci0gICAgCisKICNpZiBPUyhXSU5ET1dT KQogc3RhdGljIGNvbnN0IHNpemVfdCByYW1TaXplR3Vlc3MgPSA1MTIgKiBNQjsKICNlbmRpZgpA QCAtNTAsNiArNTQsMTIgQEAgc3RhdGljIHNpemVfdCBjb21wdXRlUkFNU2l6ZSgpCiAgICAgaWYg KCFyZXN1bHQpCiAgICAgICAgIHJldHVybiByYW1TaXplR3Vlc3M7CiAgICAgcmV0dXJuIHN0YXR1 cy51bGxUb3RhbFBoeXM7CisjZWxpZiBkZWZpbmVkKFVTRV9TWVNURU1fTUFMTE9DKSAmJiBVU0Vf U1lTVEVNX01BTExPQworI2lmIE9TKFVOSVgpCisgICAgc3RydWN0IHN5c2luZm8gc2k7CisgICAg c3lzaW5mbygmc2kpOworICAgIHJldHVybiBzaS50b3RhbHJhbSAvIE1COworI2VuZGlmIC8vIE9T KFVOSVgpCiAjZWxzZQogICAgIHJldHVybiBibWFsbG9jOjphcGk6OmF2YWlsYWJsZU1lbW9yeSgp OwogI2VuZGlmCg== 310884 2017-05-22 08:57:30 -0700 2017-05-22 09:02:53 -0700 Patch bug-172445-20170522175728.patch text/plain 1868 tpopela U3VidmVyc2lvbiBSZXZpc2lvbjogMjE3MjEwCmRpZmYgLS1naXQgYS9Tb3VyY2UvV1RGL0NoYW5n ZUxvZyBiL1NvdXJjZS9XVEYvQ2hhbmdlTG9nCmluZGV4IGNiOWFlYTcxZjA0NGZkMWJhOTRmNTUw NzFkODMzYzkyMjA4NmQ5MDEuLmQ1MGFmYzEzZTk3MzA2MTYxZmYwYTYzMGFmNGIxOWU0OGM0YTZj MzYgMTAwNjQ0Ci0tLSBhL1NvdXJjZS9XVEYvQ2hhbmdlTG9nCisrKyBiL1NvdXJjZS9XVEYvQ2hh bmdlTG9nCkBAIC0xLDMgKzEsMTggQEAKKzIwMTctMDUtMjIgIFRvbWFzIFBvcGVsYSAgPHRwb3Bl bGFAcmVkaGF0LmNvbT4KKworICAgICAgICBbV1RGXSBDb21waWxhdGlvbiBmYWlscyB3aXRoIHN5 c3RlbSBtYWxsb2MKKyAgICAgICAgaHR0cHM6Ly9idWdzLndlYmtpdC5vcmcvc2hvd19idWcuY2dp P2lkPTE3MjQ0NQorCisgICAgICAgIFJldmlld2VkIGJ5IE5PQk9EWSAoT09QUyEpLgorCisgICAg ICAgIFdlIGFyZSB1c2luZyB0aGUgYm1hbGxvYyBldmVuIGlmIHRoZSBzeXN0ZW0gbWFsbG9jIHNo b3VsZCBiZSB1c2VkLgorICAgICAgICBEb24ndCB1c2UgYm1hbGxvYyBpZiB0aGUgc3lzdGVtIG1h bGxvYyBpcyByZXF1ZXN0ZWQgYW5kIGFkZCB0aGUKKyAgICAgICAgbWlzc2luZyBpbXBsZW1lbnRh dGlvbiBmb3IgY29tcHV0ZVJBTVNpemUoKSBvbiBVTklYIHRoYXQncyB1dGlsaXppbmcKKyAgICAg ICAgdGhlIHN5c2luZm8oKSBjYWxsLgorCisgICAgICAgICogd3RmL1JBTVNpemUuY3BwOgorICAg ICAgICAoV1RGOjpjb21wdXRlUkFNU2l6ZSk6CisKIDIwMTctMDUtMTkgIERvbiBPbG1zdGVhZCAg PGRvbi5vbG1zdGVhZEBhbS5zb255LmNvbT4KIAogICAgICAgICBbV1RGXSBSZW1vdmUgUExBVEZP Uk0oV0lOKSByZWZlcmVuY2VzCmRpZmYgLS1naXQgYS9Tb3VyY2UvV1RGL3d0Zi9SQU1TaXplLmNw cCBiL1NvdXJjZS9XVEYvd3RmL1JBTVNpemUuY3BwCmluZGV4IGIwMTdhMjZiZGUzZDJiNGEyNmVj Mjc5OTFkOGZlYzRjMzQxODI5MzAuLjk1ZGEzNmFlM2FmZTE3MTNkNWQ5Y2Y1MWNiNjEwNDc0YjNh ZDE2NzcgMTAwNjQ0Ci0tLSBhL1NvdXJjZS9XVEYvd3RmL1JBTVNpemUuY3BwCisrKyBiL1NvdXJj ZS9XVEYvd3RmL1JBTVNpemUuY3BwCkBAIC0zMSwxMiArMzEsMTggQEAKIAogI2lmIE9TKFdJTkRP V1MpCiAjaW5jbHVkZSA8d2luZG93cy5oPgorI2VsaWYgZGVmaW5lZChVU0VfU1lTVEVNX01BTExP QykgJiYgVVNFX1NZU1RFTV9NQUxMT0MKKyNpZiBPUyhVTklYKQorI2luY2x1ZGUgPHN5cy9zeXNp bmZvLmg+CisjZWxzZQorI2Vycm9yICJNaXNzaW5nIGEgcGxhdGZvcm0gc3BlY2lmaWMgd2F5IG9m IGRldGVybWluaW5nIHRoZSBhdmFpbGFibGUgUkFNIgorI2VuZGlmIC8vIE9TKFVOSVgpCiAjZWxz ZQogI2luY2x1ZGUgPGJtYWxsb2MvYm1hbGxvYy5oPgogI2VuZGlmCiAKIG5hbWVzcGFjZSBXVEYg ewotICAgIAorCiAjaWYgT1MoV0lORE9XUykKIHN0YXRpYyBjb25zdCBzaXplX3QgcmFtU2l6ZUd1 ZXNzID0gNTEyICogTUI7CiAjZW5kaWYKQEAgLTUwLDYgKzU2LDEyIEBAIHN0YXRpYyBzaXplX3Qg Y29tcHV0ZVJBTVNpemUoKQogICAgIGlmICghcmVzdWx0KQogICAgICAgICByZXR1cm4gcmFtU2l6 ZUd1ZXNzOwogICAgIHJldHVybiBzdGF0dXMudWxsVG90YWxQaHlzOworI2VsaWYgZGVmaW5lZChV U0VfU1lTVEVNX01BTExPQykgJiYgVVNFX1NZU1RFTV9NQUxMT0MKKyNpZiBPUyhVTklYKQorICAg IHN0cnVjdCBzeXNpbmZvIHNpOworICAgIHN5c2luZm8oJnNpKTsKKyAgICByZXR1cm4gc2kudG90 YWxyYW0gKiBzaS5tZW1fdW5pdDsKKyNlbmRpZiAvLyBPUyhVTklYKQogI2Vsc2UKICAgICByZXR1 cm4gYm1hbGxvYzo6YXBpOjphdmFpbGFibGVNZW1vcnkoKTsKICNlbmRpZgo=