Fix ptrace_hook dependency graph

PiperOrigin-RevId: 321748143
Change-Id: Idb453054b78e932ce13c5f44f7d408cc0f9c31f2

cmake/libunwind/Download.cmake

@@ -136,6 +136,7 @@ foreach(wrapped _wrapped "")
   )
   target_link_libraries(unwind_ptrace${wrapped} PRIVATE
     sapi::base
+    sandbox2::ptrace_hook
   )
 endforeach()
 target_compile_definitions(unwind_ptrace_wrapped PUBLIC

sandboxed_api/bazel/external/libunwind.BUILD

@@ -155,7 +155,9 @@ filegroup(
         ]] if do_wrap else []
     ),
     visibility = ["//visibility:public"],
-    deps = [":included_sources"],
+    deps = [":included_sources"] + (
+      ["@com_google_sandboxed_api//sandboxed_api/sandbox2/unwind:ptrace_hook"] if do_wrap else []
+    ),
     # This forces a link failure in any target that depends on both
     # unwind-ptrace and unwind-ptrace-wrapped.
     alwayslink = 1,

sandboxed_api/sandbox2/BUILD.bazel

@@ -271,13 +271,11 @@ cc_library(
     visibility = ["//visibility:public"],
     deps = [
         ":client",
-        ":executor",
         ":comms",
+        ":executor",
+        ":forkserver",
         ":forkserver_cc_proto",
         ":global_forkserver",
-        ":sanitizer",
-        ":violation_cc_proto",
-        ":forkserver",
         ":ipc",
         ":limits",
         ":logsink",
@@ -287,20 +285,10 @@ cc_library(
         ":policy",
         ":regs",
         ":result",
+        ":sanitizer",
         ":syscall",
         ":util",
-        "@com_google_absl//absl/base:core_headers",
+        ":violation_cc_proto",
-        "@com_google_absl//absl/container:flat_hash_map",
-        "@com_google_absl//absl/container:flat_hash_set",
-        "//sandboxed_api/util:flags",
-        "@com_google_absl//absl/memory",
-        "@com_google_absl//absl/status",
-        "@com_google_absl//absl/strings",
-        "@com_google_absl//absl/strings:str_format",
-        "@com_google_absl//absl/synchronization",
-        "@com_google_absl//absl/time",
-        "@com_google_absl//absl/types:optional",
-        "@org_kernel_libcap//:libcap",
         "//sandboxed_api/sandbox2/network_proxy:client",
         "//sandboxed_api/sandbox2/network_proxy:filtering",
         "//sandboxed_api/sandbox2/network_proxy:server",
@@ -309,12 +297,21 @@ cc_library(
         "//sandboxed_api/sandbox2/util:bpf_helper",
         "//sandboxed_api/sandbox2/util:file_base",
         "//sandboxed_api/sandbox2/util:fileops",
+        "//sandboxed_api/util:flags",
         "//sandboxed_api/util:raw_logging",
         "//sandboxed_api/util:status",
         "//sandboxed_api/util:statusor",
-
+        "@com_google_absl//absl/base:core_headers",
-        # Do not remove this dependency as it defines ptrace_wrapped.
+        "@com_google_absl//absl/container:flat_hash_map",
-        "//sandboxed_api/sandbox2/unwind:ptrace_hook",  # buildcleaner: keep
+        "@com_google_absl//absl/container:flat_hash_set",
+        "@com_google_absl//absl/memory",
+        "@com_google_absl//absl/status",
+        "@com_google_absl//absl/strings",
+        "@com_google_absl//absl/strings:str_format",
+        "@com_google_absl//absl/synchronization",
+        "@com_google_absl//absl/time",
+        "@com_google_absl//absl/types:optional",
+        "@org_kernel_libcap//:libcap",
     ],
 )
 
@@ -372,7 +369,6 @@ cc_library(
         ":syscall",
         ":util",
         "//sandboxed_api/sandbox2/unwind",
-        "//sandboxed_api/sandbox2/unwind:ptrace_hook",
         "//sandboxed_api/sandbox2/unwind:unwind_cc_proto",
         "//sandboxed_api/sandbox2/util:bpf_helper",
         "//sandboxed_api/sandbox2/util:fileops",

sandboxed_api/sandbox2/CMakeLists.txt

@@ -291,7 +291,6 @@ target_link_libraries(sandbox2_sandbox2
           sandbox2::network_proxy_client
           sandbox2::notify
           sandbox2::policy
-          sandbox2::ptrace_hook
           sandbox2::regs
           sandbox2::result
           sandbox2::sanitizer
@@ -363,12 +362,10 @@ target_link_libraries(sandbox2_forkserver PRIVATE
   sandbox2::forkserver_proto
   sandbox2::namespace
   sandbox2::policy
-  sandbox2::ptrace_hook
   sandbox2::strerror
   sandbox2::sanitizer
   sandbox2::syscall
   sandbox2::unwind
-  sandbox2::unwind_proto
   sandbox2::util
   sapi::base
   sapi::raw_logging

sandboxed_api/sandbox2/forkserver.cc

@@ -50,7 +50,6 @@
 #include "sandboxed_api/sandbox2/policy.h"
 #include "sandboxed_api/sandbox2/sanitizer.h"
 #include "sandboxed_api/sandbox2/syscall.h"
-#include "sandboxed_api/sandbox2/unwind/ptrace_hook.h"
 #include "sandboxed_api/sandbox2/unwind/unwind.h"
 #include "sandboxed_api/sandbox2/unwind/unwind.pb.h"
 #include "sandboxed_api/sandbox2/util.h"
@@ -346,18 +345,8 @@ void ForkServer::LaunchChild(const ForkRequest& request, int execve_fd,
 
     c.EnableSandbox();
     if (request.mode() == FORKSERVER_FORK_JOIN_SANDBOX_UNWIND) {
-      UnwindSetup pb_setup;
+      exit(RunLibUnwindAndSymbolizer(&client_comms) ? EXIT_SUCCESS
-      if (!client_comms.RecvProtoBuf(&pb_setup)) {
+                                                    : EXIT_FAILURE);
-        exit(1);
-      }
-
-      std::string data = pb_setup.regs();
-      InstallUserRegs(data.c_str(), data.length());
-      ArmPtraceEmulation();
-      RunLibUnwindAndSymbolizer(pb_setup.pid(), &client_comms,
-                                pb_setup.default_max_frames(),
-                                pb_setup.delim());
-      exit(0);
     } else {
       ExecuteProcess(execve_fd, argv, envp);
     }

sandboxed_api/sandbox2/stack_trace.h

@@ -27,7 +27,6 @@
 #include "sandboxed_api/sandbox2/mounts.h"
 #include "sandboxed_api/sandbox2/policy.h"
 #include "sandboxed_api/sandbox2/regs.h"
-#include "sandboxed_api/sandbox2/unwind/unwind.pb.h"
 
 namespace sandbox2 {
 

sandboxed_api/sandbox2/unwind/BUILD.bazel

@@ -12,20 +12,21 @@
 # See the License for the specific language governing permissions and
 # limitations under the License.
 
+load("//sandboxed_api/bazel:build_defs.bzl", "sapi_platform_copts")
+load("//sandboxed_api/bazel:proto.bzl", "sapi_proto_library")
+
 package(default_visibility = [
     "//sandboxed_api/sandbox2:__subpackages__",
 ])
 
 licenses(["notice"])  # Apache 2.0
 
-load("//sandboxed_api/bazel:build_defs.bzl", "sapi_platform_copts")
-load("//sandboxed_api/bazel:proto.bzl", "sapi_proto_library")
-
 cc_library(
     name = "ptrace_hook",
     srcs = ["ptrace_hook.cc"],
     hdrs = ["ptrace_hook.h"],
     copts = sapi_platform_copts(),
+    visibility = ["@org_gnu_libunwind//:__subpackages__"],
 )
 
 cc_library(
@@ -50,6 +51,7 @@ cc_library(
         ]
     ]),
     deps = [
+        ":ptrace_hook",
         ":unwind_cc_proto",
         "//sandboxed_api/sandbox2:comms",
         "//sandboxed_api/sandbox2/util:maps_parser",

sandboxed_api/sandbox2/unwind/CMakeLists.txt

@@ -32,6 +32,7 @@ target_link_libraries(sandbox2_unwind PRIVATE
   sandbox2::comms
   sandbox2::maps_parser
   sandbox2::minielf
+  sandbox2::ptrace_hook
   sandbox2::strerror
   sandbox2::unwind_proto
   sapi::base

sandboxed_api/sandbox2/unwind/unwind.cc

@@ -27,6 +27,7 @@
 #include "absl/strings/str_cat.h"
 #include "libunwind-ptrace.h"
 #include "sandboxed_api/sandbox2/comms.h"
+#include "sandboxed_api/sandbox2/unwind/ptrace_hook.h"
 #include "sandboxed_api/sandbox2/unwind/unwind.pb.h"
 #include "sandboxed_api/sandbox2/util/maps_parser.h"
 #include "sandboxed_api/sandbox2/util/minielf.h"
@@ -116,18 +117,27 @@ void GetIPList(pid_t pid, std::vector<uintptr_t>* ips, int max_frames) {
   _UPT_destroy(ui);
 }
 
-void RunLibUnwindAndSymbolizer(pid_t pid, Comms* comms, int max_frames,
+bool RunLibUnwindAndSymbolizer(Comms* comms) {
-                               const std::string& delim) {
+  UnwindSetup pb_setup;
+  if (!comms->RecvProtoBuf(&pb_setup)) {
+    return false;
+  }
+
+  std::string data = pb_setup.regs();
+  InstallUserRegs(data.c_str(), data.length());
+  ArmPtraceEmulation();
+
   UnwindResult msg;
   std::string stack_trace;
   std::vector<uintptr_t> ips;
 
-  RunLibUnwindAndSymbolizer(pid, &stack_trace, &ips, max_frames, delim);
+  RunLibUnwindAndSymbolizer(pb_setup.pid(), &stack_trace, &ips,
+                            pb_setup.default_max_frames(), pb_setup.delim());
   for (const auto& i : ips) {
     msg.add_ip(i);
   }
   msg.set_stacktrace(stack_trace.c_str(), stack_trace.size());
-  comms->SendProtoBuf(msg);
+  return comms->SendProtoBuf(msg);
 }
 
 void RunLibUnwindAndSymbolizer(pid_t pid, std::string* stack_trace_out,

sandboxed_api/sandbox2/unwind/unwind.h

@@ -26,8 +26,7 @@
 namespace sandbox2 {
 
 void GetIPList(pid_t pid, std::vector<uintptr_t>* ips, int max_frames);
-void RunLibUnwindAndSymbolizer(pid_t pid, Comms* comms, int max_frames,
+bool RunLibUnwindAndSymbolizer(Comms* comms);
-                               const std::string& delim);
 
 void RunLibUnwindAndSymbolizer(pid_t pid, std::string* stack_trace_out,
                                std::vector<uintptr_t>* ips, int max_frames,