Chromium: figure out what to do with the sandboxing for code coverage reports |
|||||||||||
Issue descriptionSandbox'ed processed fail to write coverage dumps. Disabling sandbox will likely affect coverage of the sandboxing itself.
,
May 12 2018
It turns out that we lose a lot with `--no-sandbox`. The report is not ready yet, but based on the dump size, I expect it to be less than 50%. Even browser_tests itself report significantly lower coverage with `--no-sandbox`. I'll post an update with breakdown by all test targets, just in case any of them show a better coverage without sandbox. Either way, we'll have to find a way to allow reading/writing coverage profile files from inside the sandbox for coverage builds.
,
May 12 2018
Here is the breakdown: $ ./sandbox.py Target Name | with sandbox | w/o sandbox | diff | -------------------------------------------------------------------------------------- chrome_app_unittests | 1653576 | 1653336 | 240 | gcm_unit_tests | 3145456 | 3145456 | 0 | views_unittests | 19537080 | 19547944 | -10864 | ui_base_unittests | 4680008 | 4679904 | 104 | boringssl_ssl_tests | 2351864 | 2351864 | 0 | gin_unittests | 5172648 | 5172248 | 400 | extensions_browsertests | 45073632 | 74931200 | -29857568 | breakpad_unittests | 804984 | 806864 | -1880 | content_unittests | 99814024 | 99834000 | -19976 | url_unittests | 2863384 | 2863568 | -184 | gfx_unittests | 7515184 | 7515184 | 0 | jingle_unittests | 4149936 | 4150120 | -184 | snapshot_unittests | 11423536 | 11423880 | -344 | sql_unittests | 2487744 | 2487744 | 0 | courgette_unittests | 2389608 | 2389800 | -192 | display_unittests | 2895704 | 2895600 | 104 | ipc_tests | 4445848 | 4446008 | -160 | service_manager_unittests | 6394024 | 6395480 | -1456 | nacl_loader_unittests | 1683160 | 1683160 | 0 | unit_tests | 142264680 | 142294480 | -29800 | printing_unittests | 1623048 | 1623720 | -672 | device_unittests | 13653888 | 13653128 | 760 | ppapi_unittests | 5482424 | 5482424 | 0 | storage_unittests | 12113232 | 12115552 | -2320 | angle_unittests | 6391256 | 6391256 | 0 | cast_unittests | 5482576 | 5482472 | 104 | boringssl_crypto_tests | 2858896 | 2859000 | -104 | headless_unittests | 3315880 | 3316008 | -128 | webkit_unit_tests | 67195784 | 67178712 | 17072 | libjingle_xmpp_unittests | 1893264 | 1893264 | 0 | remoting_unittests | 23797232 | 23797064 | 168 | headless_browsertests | 54568816 | 73869416 | -19300600 | compositor_unittests | 13429464 | 13429576 | -112 | swiftshader_unittests | 5094160 | 5093976 | 184 | components_unittests | 123415440 | 123403768 | 11672 | accessibility_unittests | 3774912 | 3775136 | -224 | aura_unittests | 13947944 | 13920400 | 27544 | services_unittests | 52451144 | 52451840 | -696 | ui_touch_selection_unittests | 1701816 | 1701720 | 96 | cacheinvalidation_unittests | 2861104 | 2860864 | 240 | midi_unittests | 2196224 | 2196032 | 192 | vr_common_unittests | 7131160 | 7131160 | 0 | interactive_ui_tests | 102370384 | 146712288 | -44341904 | pdfium_unittests | 5457168 | 5459696 | -2528 | google_apis_unittests | 7155096 | 7154744 | 352 | base_unittests | 18179600 | 18182824 | -3224 | sync_integration_tests | 82889912 | 108327472 | -25437560 | components_browsertests | 51204544 | 66626368 | -15421824 | wtf_unittests | 3419856 | 3419856 | 0 | content_browsertests | 113498576 | 141620448 | -28121872 | leveldb_service_unittests | 5760600 | 5760600 | 0 | gn_unittests | 4343920 | 4343056 | 864 | app_shell_unittests | 21225472 | 21220208 | 5264 | gl_unittests | 3052872 | 3052872 | 0 | battor_agent_unittests | 1982952 | 1983200 | -248 | wm_unittests | 11722288 | 11722624 | -336 | gpu_unittests | 20863608 | 20863608 | 0 | traffic_annotation_auditor_unittests | 1991800 | 1990888 | 912 | media_blink_unittests | 11618848 | 11618848 | 0 | audio_unittests | 5105768 | 5105768 | 0 | viz_unittests | 19902232 | 19902232 | 0 | blink_platform_unittests | 23446296 | 23447320 | -1024 | native_theme_unittests | 1524024 | 1524952 | -928 | keyboard_unittests | 10504216 | 10500736 | 3480 | media_unittests | 28757992 | 28752840 | 5152 | skia_unittests | 2956712 | 2956712 | 0 | cc_unittests | 26079248 | 26079256 | -8 | blink_common_unittests | 3379720 | 3379840 | -120 | media_mojo_unittests | 9432896 | 9433000 | -104 | filesystem_service_unittests | 4632832 | 4632832 | 0 | capture_unittests | 3498088 | 3494400 | 3688 | sandbox_linux_unittests | 2144384 | 2144384 | 0 | events_unittests | 6144408 | 6144736 | -328 | browser_tests | 229541512 | 146084784 | 83456728 | net_unittests | 61031408 | 61056032 | -24624 | pdf_unittests | 3098768 | 3099152 | -384 | blink_heap_unittests | 8564608 | 8565200 | -592 | dbus_unittests | 3182744 | 3182928 | -184 | crypto_unittests | 1830576 | 1830776 | -200 | media_service_unittests | 6125024 | 6125024 | 0 | I'll update the CL[1] to add "--no-sandbox" flag only for the following tests: extensions_browsertests headless_browsertests interactive_ui_tests sync_integration_tests components_browsertests content_browsertests Also will manually add that on one or two bots. [1]: https://chrome-internal-review.googlesource.com/c/chrome/tools/code-coverage/+/625112
,
May 12 2018
Compared some logs: components_browsertests: https://chromium-coverage.appspot.com/reports/558114/linux/metadata/components_browsertests.log vs https://chromium-coverage.appspot.com/reports/558122/linux/metadata/components_browsertests.log don't see any failure when running without the sandbox, so don't think we need to add "--no-sandbox" content_browsertests: https://chromium-coverage.appspot.com/reports/558114/linux/metadata/content_browsertests.log vs https://chromium-coverage.appspot.com/reports/558122/linux/metadata/content_browsertests.log it has Counter overflow warnings when run with "--no-sandbox": _ZN3IPCL10WriteParamImEEvPN4base6PickleERKT_: Counter overflow which is bad. Similar to issue 801362 . sync_integration_tests: https://chromium-coverage.appspot.com/reports/558114/linux/metadata/sync_integration_tests.log vs https://chromium-coverage.appspot.com/reports/558122/linux/metadata/sync_integration_tests.log also has warnings without the sandbox. interactive_ui_tests: https://chromium-coverage.appspot.com/reports/558114/linux/metadata/interactive_ui_tests.log vs https://chromium-coverage.appspot.com/reports/558122/linux/metadata/interactive_ui_tests.log looks good with "--no-sandbox", there are failures without it headless_browsertests: https://chromium-coverage.appspot.com/reports/558114/linux/metadata/headless_browsertests.log vs https://chromium-coverage.appspot.com/reports/558122/linux/metadata/headless_browsertests.log don't see any reason to use "--no-sandbox" extensions_browsertests: https://chromium-coverage.appspot.com/reports/558114/linux/metadata/extensions_browsertests.log vs https://chromium-coverage.appspot.com/reports/558122/linux/metadata/extensions_browsertests.log don't see any reason to use "--no-sandbox" Also checked browser_tests: https://chromium-coverage.appspot.com/reports/558114/linux/metadata/browser_tests.log vs https://chromium-coverage.appspot.com/reports/558122/linux/metadata/browser_tests.log There is a bunch of warnings again, i.e. we can't use "--no-sandbox" at this point.
,
May 12 2018
,
May 12 2018
I've reverted my change from bot code-coverage-linux-0001. And also am going to delete that report from the dashboard. If you want to check it yourself, here are the links: Report: https://chromium-coverage.appspot.com/reports/558114/linux/chromium/src/report.html Logs: https://chromium-coverage.appspot.com/reports/558114/linux/metadata/index.html
,
May 12 2018
Also, after giving it another thought, I'm not too concerned about the lack of coverage dumps from sandbox'ed processes, as most of their functionality is actually tested with other stuff (e.g. content_unittests tests the renderer, and works fine in its default configuration). That said, we still have to find a solution for getting coverage dumps from sandbox'ed processes, but it's not a blocker.
,
May 16 2018
Probably also the reason for lower sandbox code coverage on https://chromium-coverage.appspot.com/reports/558824/linux/chromium/src/sandbox/report.html
,
May 16 2018
Nice to see win/ coverage there :) Kudos to wfh@ for his fuzz target for win_sandbox that works on Linux :)
,
May 16 2018
,
May 17 2018
,
May 17 2018
I have an idea about how this would work with the sandbox (get clang/llvm to write the profile in a shared memory in the renderer, and browser puts that into a file) ... this may not work very well for cases where profile-files are merged though (i.e. %m in LLVM_PROFILE_FILE).
,
May 17 2018
Yeah, but I wonder if we could do anything simpler, i.e. simply allow sandboxed processes to read and write certain files when "#if defined(CLANG_COVERAGE)"
,
May 17 2018
Sadrul@, adding this to your queue. Any help is highly appreciated here.
,
May 17 2018
,
Aug 3
This bug has an owner, thus, it's been triaged. Changing status to "assigned".
,
Dec 1
Friendly PING, is there any update on this?
,
Dec 11
,
Dec 11
,
Dec 11
|
|||||||||||
►
Sign in to add a comment |
|||||||||||
Comment 1 by mmoroz@chromium.org
, May 12 2018