Bug 172445 – [WTF] Compilation fails with system malloc

• Component: Web Template Framework
• Status: RESOLVED FIXED

[Tomas Popela]

../../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

[Tomas Popela]

Created attachment 310860 [details]
Patch

[Tomas Popela]

Created attachment 310862 [details]
Patch

[Michael Catanzaro]

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.

[Tomas Popela]

(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" ?

[Michael Catanzaro]

That sounds good.

[Tomas Popela]

Created attachment 310884 [details]
Patch

Fail with an error message if not on UNIX and also return memory in bytes

[Michael Catanzaro]

Comment on attachment 310884 [details]
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.

[Tomas Popela]

Committed r217270: <http://trac.webkit.org/changeset/217270>

[Simon Fraser (smfr)]

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>
         ^~~~~~~~~~~~~~~

[Simon Fraser (smfr)]

I would think that it's OK to use bmalloc::api::availableMemory() even if using system malloc, at least on macOS/iOS.

[Michael Catanzaro]

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.

[Alberto Garcia]

(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

[Michael Catanzaro]

For the record: r=me

[Alberto Garcia]

(In reply to Michael Catanzaro from comment #13)
> For the record: r=me

Committed r240038: <https://trac.webkit.org/changeset/240038/webkit>