Bug 230460

Summary: Make valgrind work properly without extra environment variables
Product: WebKit Reporter: Michael Catanzaro <mcatanzaro>
Component: bmallocAssignee: Michael Catanzaro <mcatanzaro>
Status: RESOLVED FIXED    
Severity: Normal CC: annulen, aperez, bugs-noreply, cgarcia, ews-watchlist, ggaren, glenn, gyuyoung.kim, jbedard, mcatanzaro, ryuan.choi, sergio, webkit-bug-importer
Priority: P2 Keywords: InRadar
Version: WebKit Nightly Build   
Hardware: PC   
OS: Linux   
Attachments:
Description Flags
Patch
none
Patch none

Description Michael Catanzaro 2021-09-19 09:37:59 PDT 
Currently to run WebKit under valgrind, we have to manually disable bmalloc with Malloc=1, or else:

 * The Gigacage breaks everything, because valgrind shadows the entire address space in resident memory, and it just can't handle that much address space
 * Even with GIGACAGE_ENABLED=0, diagnostics are worse because bmalloc does not use valgrind's annotations

Using valgrind.h, we can automatically disable bmalloc when running under valgrind to avoid needing to use the environment variable.

Additionally, WebKitGTK and WPE WebKit users also have to set the WEBKIT_FORCE_SANDBOX=0 environment variable, because the web process sandbox breaks valgrind for some reason I never attempted to understand. If we have bmalloc expose valgrind.h as a public header, then we can check for valgrind in higher layers and make this work automatically too.
Comment 1 Michael Catanzaro 2021-09-19 09:48:20 PDT 
Created attachment 438597 [details]
Patch
Comment 2 Michael Catanzaro 2021-09-19 09:51:22 PDT 
Q: valgrind.h is complicated we only need RUNNING_ON_VALGRIND. Do we really need the whole thing?
A: RUNNING_ON_VALGRIND is also really complicated. :P The header is designed to be a copylib and it's normal for projects that implement their own memory allocators to just include the whole thing (e.g. GLib has its own copy).

Q: What is up with Source/bmalloc/bmalloc.xcodeproj/project.pbxproj?
A: I don't know. Perhaps webkit-patch sorts it automatically?
Comment 3 Michael Catanzaro 2021-09-23 08:11:26 PDT 
Ping bmalloc reviewers!
Comment 4 Radar WebKit Bug Importer 2021-09-26 09:38:16 PDT 
<rdar://problem/83549100>
Comment 5 Michael Catanzaro 2021-11-02 13:44:20 PDT 
(In reply to Michael Catanzaro from comment #3)
> Ping bmalloc reviewers!

Final ping before I ask for GTK/WPE reviewers
Comment 6 Michael Catanzaro 2021-11-11 10:01:55 PST 
Ping GTK/WPE reviewers
Comment 7 EWS 2021-11-12 12:46:10 PST 
Tools/Scripts/svn-apply failed to apply attachment 438597 [details] to trunk.
Please resolve the conflicts and upload a new patch.
Comment 8 Adrian Perez 2021-11-14 14:39:14 PST 
(In reply to EWS from comment #7)
> Tools/Scripts/svn-apply failed to apply attachment 438597 [details] to trunk.
> Please resolve the conflicts and upload a new patch.

The patch will need a rebase before it can land. Michael, could you take
a stab at that when you have some spare cycles? Thanks!
Comment 9 Michael Catanzaro 2021-11-15 11:17:14 PST 
Created attachment 444277 [details]
Patch
Comment 10 EWS 2021-11-15 11:46:20 PST 
Committed r285819 (244261@main): <https://commits.webkit.org/244261@main>

All reviewed patches have been landed. Closing bug and clearing flags on attachment 444277 [details].