chromiumembedded / cef

Chromium Embedded Framework (CEF). A simple framework for embedding Chromium-based browsers in other applications.
https://bitbucket.org/chromiumembedded/cef/
Other
3.13k stars 450 forks source link

Linux: Build with use_allocator_shim=false #3095

Open magreenblatt opened 3 years ago

magreenblatt commented 3 years ago

Original report by Henri Beauchamp (Bitbucket: Henri Beauchamp).


Greetings,

I recent CEF/Chromium versions (v77+), the implementation of a SHIM causes CEF (compiled with use_allocator=none) to crash whenever it is used together with jemalloc under Linux.

The reason for that crash is that the hooking mechanism of the SHIM into malloc functions is totally flawed; the hook/override for malloc_usable_size() is not implemented in the same way as for all other malloc functions, and this results in CEF memory being properly allocated and deallocated by jemalloc, but in CEF wrongly calling libc’s malloc_usable_size() with an address of a memory block allocated by jemalloc.

Here is one of such crash I captured (CEF being used as a plugin from a Second Life viewer using jemalloc):

Thread 13 "ThreadPoolForeg" received signal SIGSEGV, Segmentation fault.
[Switching to LWP 8148]
0x00007ffff732fcf8 in malloc_usable_size () from /lib64/libc.so.6
(gdb) bt
#0  0x00007ffff732fcf8 in malloc_usable_size () at /lib64/libc.so.6
#1  0x00007ffff27ded76 in  () at ~/CoolVLViewer-x86_64-1.28.1.0/lib/libcef.so
#2  0x0000000000000000 in  ()

There is an option to disable that SHIM, but alas you cannot compile CEF/Chromium with both ‘use_allocator=none' and 'use_allocator_shim=false' options (the compilation stops with a missing __libc_malloc symbol error).

The culprit code for the compilation failure resides (currently, i.e. for CEF v88) in chromium_git/chromium/src/base/process/memory_linux.cc and is in UncheckedMalloc().

Instead of:

#if BUILDFLAG(USE_ALLOCATOR_SHIM)
.../...
#elif defined(MEMORY_TOOL_REPLACES_ALLOCATOR) || \
    (!defined(LIBC_GLIBC) && !BUILDFLAG(USE_TCMALLOC))
  *result = malloc(size);
#elif defined(LIBC_GLIBC) && !BUILDFLAG(USE_TCMALLOC)
  *result = __libc_malloc(size);
#elif BUILDFLAG(USE_TCMALLOC)
  *result = tc_malloc_skip_new_handler(size);
#endif

It should read:

#if BUILDFLAG(USE_ALLOCATOR_SHIM)
.../...
#elif defined(MEMORY_TOOL_REPLACES_ALLOCATOR) || \
    (!defined(LIBC_GLIBC) && !BUILDFLAG(USE_TCMALLOC))
  *result = malloc(size);
#elif defined(LIBC_GLIBC) && !BUILDFLAG(USE_TCMALLOC)
  *result = malloc(size);
#elif BUILDFLAG(USE_TCMALLOC)
  *result = tc_malloc_skip_new_handler(size);
#endif

With this change implemented, CEF runs fine together with jemalloc…

magreenblatt commented 3 years ago

Please file a bug with Chromium at https://crbug.com/new.

magreenblatt commented 3 years ago

Original comment by Henri Beauchamp (Bitbucket: Henri Beauchamp).


I don't have a Google acount (and won’t create one)… So I cannot report bugs over there… :confused:

magreenblatt commented 3 years ago

This issue requires review by the Chromium developers. It’s unlikely that this issue will be resolved any time soon if you’re unwilling to begin that process. Additionally, usage of jemalloc is specific to your application.

magreenblatt commented 3 years ago

Original comment by Czarek Tomczak (Bitbucket: Czarek, GitHub: Czarek).


Apply this little patch below to fix build issues when disabling allocator shim for Linux. Tested on branch 3945.
https://gist.github.com/cztomczak/ec6067cf8c04a2dc4335661eb9245ae4

magreenblatt commented 3 years ago

@{557058:93489ef7-acae-448f-9840-15ddb0beb530} Can you submit your fix as a PR? Thanks.

magreenblatt commented 3 years ago

See also issue #3061 for Mac.

magreenblatt commented 3 years ago

Original comment by Czarek Tomczak (Bitbucket: Czarek, GitHub: Czarek).


Working on the PR fix.

Currently when you try to build master branch with the use_allocator_shim=false GN flag you get this error:

[8609/47156] CXX obj/base/base/memory_linux.o
FAILED: obj/base/base/memory_linux.o
../../third_party/llvm-build/Release+Asserts/bin/clang++ …………….. ../../base/process/memory_linux.cc -o obj/base/base/memory_linux.o
../../base/process/memory_linux.cc:122:13: error: use of undeclared identifier '__libc_malloc'
*result = __libc_malloc(size);
^
1 error generated.
[8622/47156] CXX obj/base/base/file_path_watcher_linux.o

magreenblatt commented 3 years ago

Original comment by Czarek Tomczak (Bitbucket: Czarek, GitHub: Czarek).


Sent PR: https://bitbucket.org/chromiumembedded/cef/pull-requests/380/linux-fix-compile-errors-when-allocator

magreenblatt commented 2 years ago

Original comment by Henri Beauchamp (Bitbucket: Henri Beauchamp).


@Czarek Tomczak

While your pull request solves the building error, it does not solve the crash seen in CEF when used in conjunction with jemalloc. I am still using (with 100% success) my proposed solution for my builds (in use by the Cool VL Viewer).

magreenblatt commented 1 year ago

linux: Set use_allocator=none by default (see issue #3095)

→ <<cset b950336a0012 (bb)>>

magreenblatt commented 1 year ago

linux: Set use_allocator=none by default (see issue #3095)

→ <<cset 26c0b5e46202 (bb)>>

magreenblatt commented 1 year ago

Original comment by Henri Beauchamp (Bitbucket: Henri Beauchamp).


The commit disables the Chromium custom allocator by default, which is good to enable the use of jemalloc of other allocators, but alas and as I explained in my initial post, you then get crashes because of the flawed hooking code used to redirect calls from malloc & Co to the Chromium SHIM: you must then also disable the shim, but then you get compilation failures because this case (use_allocator=none & use_allocator_shim=false) is not coped with in the memory_linux.cc code; you need to patch the latter to replace the calls to _libc_malloc() and __libc_free with calls to malloc() and free().

FYI, here is the bash code I am using in my CEF build script for Linux, to patch the culprit lines:

    # Patch Chromium to build properly against glibc without SHIM
    file="$CURDIR/CEF/chromium_git/chromium/src/base/process/memory_linux.cc"
    if [ -f "$file" ] ; then
        echo "**** Patching the __libc_malloc and __libc_free calls in $file"
        sed -i -e 's/__libc_malloc/malloc/' "$file"
        sed -i -e 's/__libc_free/free/' "$file"
    fi

Alternatively, you could patch the #directives blocks in that file to use malloc() and free() when defined(LIBC_GLIBC) && !BUILDFLAG(USE_TCMALLOC) && !BUILDFLAG(USE_ALLOCATOR_SHIM).

magreenblatt commented 3 years ago
magreenblatt commented 3 years ago
magreenblatt commented 3 years ago
magreenblatt commented 3 years ago
magreenblatt commented 2 months ago

The allocator shim will remain enabled by default in non-Official builds so that we can benefit from BackupRefPtr checks. See #3239.