0

base: process: move content::SetProcessTitleFromCommandLine to //base

crrev.com/c/758962 introduced BrokerProcessPreSandboxHook() in
content/gpu/gpu_sandbox_hook_linux.cc because
SetProcessTitleFromCommandLine() was in service_manager package as the
comment in BrokerProcessPreSandboxHook() make an excuse.

However the PreSandboxHook is so complex that other broker processes
forget to call SetProcessTitleFromCommandLine().

By moving SetProcessTitleFromCommandLine() to //base package, the
complex PreSandboxHook is no longer needed (in the following CL).

Change-Id: Ie418c4583893d0755434667b301184934aa1199d
Bug: b:328170541
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/5348437
Reviewed-by: Matthew Denton <mpdenton@chromium.org>
Commit-Queue: Shin Kawamura <kawasin@google.com>
Reviewed-by: Avi Drissman <avi@chromium.org>
Reviewed-by: Gabriel Charette <gab@chromium.org>
Reviewed-by: Tom Sepez <tsepez@chromium.org>
Reviewed-by: Alex Moshchuk <alexmos@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1270022}
This commit is contained in:
Shintaro Kawamura
2024-03-08 05:16:24 +00:00
committed by Chromium LUCI CQ
parent 29890a7226
commit 20d4a7d30f
14 changed files with 60 additions and 62 deletions

@ -558,6 +558,8 @@ component("base") {
"process/process_handle.cc",
"process/process_handle.h",
"process/process_info.h",
"process/set_process_title.cc",
"process/set_process_title.h",
"profiler/frame.cc",
"profiler/frame.h",
"profiler/metadata_recorder.cc",
@ -2212,7 +2214,11 @@ component("base") {
configs += linux_configs
all_dependent_configs += linux_configs
sources += [ "system/sys_info_linux.cc" ]
sources += [
"process/set_process_title_linux.cc",
"process/set_process_title_linux.h",
"system/sys_info_linux.cc",
]
if (!is_cronet_build) {
# These dependencies are not required on Android.
sources += [
@ -3712,6 +3718,7 @@ test("base_unittests") {
"debug/proc_maps_linux_unittest.cc",
"files/scoped_file_linux_unittest.cc",
"nix/mime_util_xdg_unittest.cc",
"process/set_process_title_linux_unittest.cc",
]
if (!is_nacl) {

@ -2,20 +2,20 @@
// Use of this source code is governed by a BSD-style license that can be
// found in the LICENSE file.
#include "build/build_config.h"
// Define _GNU_SOURCE to ensure that <errno.h> defines
// program_invocation_short_name. Keep this at the top of the file since some
// system headers might include <errno.h> and the header could be skipped on
// subsequent includes.
#if (defined(OS_LINUX) || defined(OS_CHROMEOS)) && !defined(_GNU_SOURCE)
#if (BUILDFLAG(IS_LINUX) || BUILDFLAG(IS_CHROMEOS)) && !defined(_GNU_SOURCE)
#define _GNU_SOURCE
#endif
#include "content/common/set_process_title.h"
#include "base/process/set_process_title.h"
#include <stddef.h>
#include "build/build_config.h"
#if BUILDFLAG(IS_POSIX) && !BUILDFLAG(IS_MAC) && !BUILDFLAG(IS_SOLARIS)
#include <limits.h>
#include <stdlib.h>
@ -37,14 +37,14 @@
#include "base/strings/string_util.h"
#include "base/threading/platform_thread.h"
// Linux/glibc doesn't natively have setproctitle().
#include "content/common/set_process_title_linux.h"
#include "base/process/set_process_title_linux.h"
#endif // BUILDFLAG(IS_LINUX) || BUILDFLAG(IS_CHROMEOS)
namespace content {
namespace base {
// TODO(jrg): Find out if setproctitle or equivalent is available on Android.
#if BUILDFLAG(IS_POSIX) && !BUILDFLAG(IS_APPLE) && !BUILDFLAG(IS_SOLARIS) && \
!BUILDFLAG(IS_ANDROID) && !BUILDFLAG(IS_FUCHSIA)
!BUILDFLAG(IS_ANDROID) && !BUILDFLAG(IS_FUCHSIA) && !BUILDFLAG(IS_NACL)
void SetProcessTitleFromCommandLine(const char** main_argv) {
// Build a single string which consists of all the arguments separated
@ -109,4 +109,4 @@ void SetProcessTitleFromCommandLine(const char** /* main_argv */) {}
#endif
} // namespace content
} // namespace base

@ -2,10 +2,12 @@
// Use of this source code is governed by a BSD-style license that can be
// found in the LICENSE file.
#ifndef CONTENT_COMMON_SET_PROCESS_TITLE_H_
#define CONTENT_COMMON_SET_PROCESS_TITLE_H_
#ifndef BASE_PROCESS_SET_PROCESS_TITLE_H_
#define BASE_PROCESS_SET_PROCESS_TITLE_H_
namespace content {
#include "base/base_export.h"
namespace base {
// Sets OS-specific process title information based on the command line. This
// does nothing if the OS doesn't support or need this capability.
@ -21,8 +23,8 @@ namespace content {
// makes the process name that shows up in "ps" etc. for the child processes
// show as "exe" instead of "chrome" or something reasonable. This function
// will try to fix it so the "effective" command line shows up instead.
void SetProcessTitleFromCommandLine(const char** main_argv);
BASE_EXPORT void SetProcessTitleFromCommandLine(const char** main_argv);
} // namespace content
} // namespace base
#endif // CONTENT_COMMON_SET_PROCESS_TITLE_H_
#endif // BASE_PROCESS_SET_PROCESS_TITLE_H_

@ -37,7 +37,7 @@
// this position within the glibc project, leaving applications caught in the
// middle. (Also, only a very few applications need or want this anyway.)
#include "content/common/set_process_title_linux.h"
#include "base/process/set_process_title_linux.h"
#include <stdarg.h>
#include <stddef.h>
@ -51,6 +51,7 @@
#include "base/files/file_util.h"
#include "base/no_destructor.h"
#include "base/numerics/safe_conversions.h"
extern char** environ;
@ -77,7 +78,8 @@ void setproctitle(const char* fmt, ...) {
return;
// The title can be up to the end of envp.
const size_t avail_size = g_envp_end - g_argv_start - 1;
const size_t avail_size =
base::checked_cast<size_t>(g_envp_end - g_argv_start - 1);
// Linux 4.18--5.2 have a bug where we can never set a process title
// shorter than the initial argv. Check if the bug exists in the current
@ -103,11 +105,14 @@ void setproctitle(const char* fmt, ...) {
size_t size;
va_start(ap, fmt);
if (fmt[0] == '-') {
size = vsnprintf(g_argv_start, avail_size, &fmt[1], ap);
size = base::checked_cast<size_t>(
vsnprintf(g_argv_start, avail_size, &fmt[1], ap));
} else {
size = snprintf(g_argv_start, avail_size, "%s ", g_orig_argv0);
size = base::checked_cast<size_t>(
snprintf(g_argv_start, avail_size, "%s ", g_orig_argv0));
if (size < avail_size)
size += vsnprintf(&g_argv_start[size], avail_size - size, fmt, ap);
size += base::checked_cast<size_t>(
vsnprintf(&g_argv_start[size], avail_size - size, fmt, ap));
}
va_end(ap);
@ -122,7 +127,8 @@ void setproctitle(const char* fmt, ...) {
// On buggy kernels we can never make the process title shorter than the
// initial argv. In that case, just leave the remaining bytes filled with
// null characters.
const size_t argv_size = g_argv_end - g_argv_start - 1;
const size_t argv_size =
base::checked_cast<size_t>(g_argv_end - g_argv_start - 1);
if (!buggy_kernel && size < argv_size)
g_argv_end[-1] = '.';
}

@ -2,8 +2,10 @@
// Use of this source code is governed by a BSD-style license that can be
// found in the LICENSE file.
#ifndef CONTENT_COMMON_SET_PROCESS_TITLE_LINUX_H_
#define CONTENT_COMMON_SET_PROCESS_TITLE_LINUX_H_
#ifndef BASE_PROCESS_SET_PROCESS_TITLE_LINUX_H_
#define BASE_PROCESS_SET_PROCESS_TITLE_LINUX_H_
#include "base/base_export.h"
// Set the process title that will show in "ps" and similar tools. Takes
// printf-style format string and arguments. After calling setproctitle()
@ -13,10 +15,10 @@
//
// This signature and naming is to be compatible with most other Unix
// implementations of setproctitle().
void setproctitle(const char* fmt, ...);
BASE_EXPORT void setproctitle(const char* fmt, ...);
// Initialize state needed for setproctitle() on Linux. Pass the argv pointer
// from main() to setproctitle_init() before calling setproctitle().
void setproctitle_init(const char** main_argv);
BASE_EXPORT void setproctitle_init(const char** main_argv);
#endif // CONTENT_COMMON_SET_PROCESS_TITLE_LINUX_H_
#endif // BASE_PROCESS_SET_PROCESS_TITLE_LINUX_H_

@ -7,10 +7,10 @@
#include "base/files/file_path.h"
#include "base/files/file_util.h"
#include "base/process/set_process_title_linux.h"
#include "base/strings/string_piece.h"
#include "base/strings/string_util.h"
#include "build/build_config.h"
#include "content/common/set_process_title_linux.h"
#include "testing/gtest/include/gtest/gtest.h"
#if BUILDFLAG(IS_CHROMEOS)

@ -25,7 +25,7 @@
// Note: The special-case IS_CHROMEOS code inside GetDebugBasenameForModule to
// handle the interaction between that function and
// SetProcessTitleFromCommandLine() is tested in
// content/common/set_process_title_linux_unittest.cc due to dependency issues.
// base/process/set_process_title_linux_unittest.cc due to dependency issues.
namespace base {
namespace {

@ -13,6 +13,10 @@
#include "base/win/com_init_util.h"
#endif // BUILDFLAG(IS_WIN)
#if BUILDFLAG(IS_LINUX) || BUILDFLAG(IS_CHROMEOS)
#include "base/process/set_process_title_linux.h"
#endif
namespace base {
namespace {
@ -66,6 +70,11 @@ class BaseUnittestSuite : public TestSuite {
} // namespace base
int main(int argc, char** argv) {
#if BUILDFLAG(IS_LINUX) || BUILDFLAG(IS_CHROMEOS)
// For setproctitle unit tests.
setproctitle_init(const_cast<const char**>(argv));
#endif
base::BaseUnittestSuite test_suite(argc, argv);
return base::LaunchUnitTests(
argc, argv,

@ -21,6 +21,7 @@
#include "base/process/launch.h"
#include "base/process/memory.h"
#include "base/process/process.h"
#include "base/process/set_process_title.h"
#include "base/run_loop.h"
#include "base/strings/string_number_conversions.h"
#include "base/task/single_thread_task_executor.h"
@ -35,7 +36,6 @@
#include "components/tracing/common/tracing_switches.h"
#include "content/app/content_main_runner_impl.h"
#include "content/common/mojo_core_library_support.h"
#include "content/common/set_process_title.h"
#include "content/public/app/content_main_delegate.h"
#include "content/public/common/content_switches.h"
#include "mojo/core/embedder/configuration.h"
@ -245,7 +245,7 @@ RunContentProcess(ContentMainParams params,
base::EnableTerminationOnHeapCorruption();
SetProcessTitleFromCommandLine(argv);
base::SetProcessTitleFromCommandLine(argv);
#endif // !BUILDFLAG(IS_ANDROID)
InitTimeTicksAtUnixEpoch();

@ -222,8 +222,6 @@ source_set("common") {
"service_worker/service_worker_resource_loader.h",
"service_worker/service_worker_router_evaluator.cc",
"service_worker/service_worker_router_evaluator.h",
"set_process_title.cc",
"set_process_title.h",
"skia_utils.cc",
"skia_utils.h",
"thread_pool_util.cc",
@ -453,7 +451,6 @@ source_set("common") {
public_deps += [ "//sandbox/policy" ]
deps += [
":sandbox_support_linux",
":set_process_title_linux",
"//media/gpu:buildflags",
"//sandbox/linux:sandbox_services",
"//sandbox/linux:seccomp_bpf",
@ -524,14 +521,6 @@ component("main_frame_counter") {
deps = [ "//base:base" ]
}
if (is_linux || is_chromeos) {
source_set("set_process_title_linux") {
public = [ "set_process_title_linux.h" ]
sources = [ "set_process_title_linux.cc" ]
deps = [ "//base" ]
}
}
# See comment at the top of //content/BUILD.gn for how this works.
group("for_content_tests") {
visibility = [

@ -20,11 +20,11 @@
#include "base/functional/bind.h"
#include "base/logging.h"
#include "base/path_service.h"
#include "base/process/set_process_title.h"
#include "base/strings/stringprintf.h"
#include "build/build_config.h"
#include "build/buildflag.h"
#include "build/chromeos_buildflags.h"
#include "content/common/set_process_title.h"
#include "content/public/common/content_switches.h"
#include "media/gpu/buildflags.h"
#include "sandbox/linux/bpf_dsl/policy.h"
@ -670,7 +670,7 @@ bool BrokerProcessPreSandboxHook(
// Oddly enough, we call back into gpu to invoke this service manager
// method, since it is part of the embedder component, and the service
// mananger's sandbox component is a lower layer that can't depend on it.
SetProcessTitleFromCommandLine(nullptr);
base::SetProcessTitleFromCommandLine(nullptr);
return true;
}

@ -2293,9 +2293,6 @@ static_library("run_all_unittests") {
":test_support",
"//base/test:test_support",
]
if (is_linux || is_chromeos) {
deps += [ "//content/common:set_process_title_linux" ]
}
}
test("content_unittests") {
@ -3387,11 +3384,6 @@ test("content_unittests") {
deps += [ "//third_party/boringssl" ]
}
if (is_linux || is_chromeos) {
sources += [ "../common/set_process_title_linux_unittest.cc" ]
deps += [ "//content/common:set_process_title_linux" ]
}
if (enable_vr) {
sources += [ "../browser/xr/service/xr_runtime_manager_unittest.cc" ]
deps += [ "//device/vr:vr_fakes" ]

@ -9,16 +9,7 @@
#include "content/public/test/unittest_test_suite.h"
#include "content/test/content_test_suite.h"
#if BUILDFLAG(IS_LINUX) || BUILDFLAG(IS_CHROMEOS)
#include "content/common/set_process_title_linux.h"
#endif
int main(int argc, char** argv) {
#if BUILDFLAG(IS_LINUX) || BUILDFLAG(IS_CHROMEOS)
// For setproctitle unit tests.
setproctitle_init(const_cast<const char**>(argv));
#endif
content::UnitTestTestSuite test_suite(
new content::ContentTestSuite(argc, argv),
base::BindRepeating(content::UnitTestTestSuite::CreateTestContentClients),

@ -34,11 +34,11 @@
#include "base/process/launch.h"
#include "base/process/process.h"
#include "base/process/process_handle.h"
#include "base/process/set_process_title.h"
#include "base/time/time.h"
#include "base/trace_event/trace_event.h"
#include "build/build_config.h"
#include "build/chromeos_buildflags.h"
#include "content/common/set_process_title.h"
#include "content/common/zygote/zygote_commands_linux.h"
#include "content/public/common/content_descriptors.h"
#include "content/public/common/content_switches.h"
@ -628,7 +628,7 @@ base::ProcessId Zygote::ReadArgsAndFork(base::PickleIterator iter,
// Update the process title. The argv was already cached by the call to
// SetProcessTitleFromCommandLine in ChromeMain, so we can pass NULL here
// (we don't have the original argv at this point).
SetProcessTitleFromCommandLine(nullptr);
base::SetProcessTitleFromCommandLine(nullptr);
} else if (child_pid < 0) {
LOG(ERROR) << "Zygote could not fork: process_type " << process_type
<< " numfds " << numfds << " child_pid " << child_pid;