Snap for 10276566 from 53a8558d57cef212315484797092a82245f0586d to tm-platform-releaseandroid-platform-13.0.0_r9android-platform-13.0.0_r8android-platform-13.0.0_r19android-platform-13.0.0_r18android-platform-13.0.0_r17android-platform-13.0.0_r16android-platform-13.0.0_r15android-platform-13.0.0_r14android-platform-13.0.0_r13android-platform-13.0.0_r12android-platform-13.0.0_r11android-platform-13.0.0_r10android13-platform-release

Change-Id: I19cfac1ddd78f4936dcc7e02a8b44528af7de118

14 files changed, 129 insertions, 9 deletions

diff --git a/include/perfetto/ext/ipc/host.h b/include/perfetto/ext/ipc/host.h
index a9b5824b9..0f8533ee3 100644
--- a/include/perfetto/ext/ipc/host.h
+++ b/include/perfetto/ext/ipc/host.h
@@ -59,6 +59,9 @@ class Host {
   // case of errors (e.g., another service with the same name is already
   // registered).
   virtual bool ExposeService(std::unique_ptr<Service>) = 0;
+
+  // Overrides the default send timeout for the per-connection sockets.
+  virtual void SetSocketSendTimeoutMs(uint32_t timeout_ms) = 0;
 };
 
 }  // namespace ipc
diff --git a/protos/perfetto/config/perfetto_config.proto b/protos/perfetto/config/perfetto_config.proto
index 00fee9061..3bc791895 100644
--- a/protos/perfetto/config/perfetto_config.proto
+++ b/protos/perfetto/config/perfetto_config.proto
@@ -1918,6 +1918,10 @@ message TraceConfig {
     // On R-, this override only affected userdebug builds. Since S, it also
     // affects user builds.
     optional uint64 max_upload_per_day_bytes = 1;
+
+    // Overrides the guardrail for maximum trace buffer size.
+    // Available on U+
+    optional uint32 max_tracing_buffer_size_kb = 2;
   }
   optional GuardrailOverrides guardrail_overrides = 11;
 
diff --git a/protos/perfetto/config/trace_config.proto b/protos/perfetto/config/trace_config.proto
index be1ba8237..4fc271724 100644
--- a/protos/perfetto/config/trace_config.proto
+++ b/protos/perfetto/config/trace_config.proto
@@ -208,6 +208,10 @@ message TraceConfig {
     // On R-, this override only affected userdebug builds. Since S, it also
     // affects user builds.
     optional uint64 max_upload_per_day_bytes = 1;
+
+    // Overrides the guardrail for maximum trace buffer size.
+    // Available on U+
+    optional uint32 max_tracing_buffer_size_kb = 2;
   }
   optional GuardrailOverrides guardrail_overrides = 11;
 
diff --git a/protos/perfetto/trace/perfetto_trace.proto b/protos/perfetto/trace/perfetto_trace.proto
index 58a0f68e6..7a80660bb 100644
--- a/protos/perfetto/trace/perfetto_trace.proto
+++ b/protos/perfetto/trace/perfetto_trace.proto
@@ -1918,6 +1918,10 @@ message TraceConfig {
     // On R-, this override only affected userdebug builds. Since S, it also
     // affects user builds.
     optional uint64 max_upload_per_day_bytes = 1;
+
+    // Overrides the guardrail for maximum trace buffer size.
+    // Available on U+
+    optional uint32 max_tracing_buffer_size_kb = 2;
   }
   optional GuardrailOverrides guardrail_overrides = 11;
 
diff --git a/src/android_stats/perfetto_atoms.h b/src/android_stats/perfetto_atoms.h
index 75cbbd837..14740ae80 100644
--- a/src/android_stats/perfetto_atoms.h
+++ b/src/android_stats/perfetto_atoms.h
@@ -70,6 +70,7 @@ enum class PerfettoStatsdAtom {
   kTracedStartTracingInvalidSessionState = 36,
   kTracedEnableTracingInvalidFilter = 47,
   kTracedEnableTracingOobTargetBuffer = 48,
+  kTracedEnableTracingInvalidTriggerMode = 52,
 
   // Checkpoints inside perfetto_cmd after tracing has finished.
   kOnTracingDisabled = 4,
diff --git a/src/ipc/host_impl.cc b/src/ipc/host_impl.cc
index c182cd17e..1bf234835 100644
--- a/src/ipc/host_impl.cc
+++ b/src/ipc/host_impl.cc
@@ -116,6 +116,12 @@ bool HostImpl::ExposeService(std::unique_ptr<Service> service) {
   return true;
 }
 
+void HostImpl::SetSocketSendTimeoutMs(uint32_t timeout_ms) {
+  PERFETTO_DCHECK_THREAD(thread_checker_);
+  // Should be less than the watchdog period (30s).
+  socket_tx_timeout_ms_ = timeout_ms;
+}
+
 void HostImpl::OnNewIncomingConnection(
     base::UnixSocket*,
     std::unique_ptr<base::UnixSocket> new_conn) {
@@ -125,8 +131,7 @@ void HostImpl::OnNewIncomingConnection(
   clients_by_socket_[new_conn.get()] = client.get();
   client->id = client_id;
   client->sock = std::move(new_conn);
-  // Watchdog is 30 seconds, so set the socket timeout to 10 seconds.
-  client->sock->SetTxTimeout(10000);
+  client->sock->SetTxTimeout(socket_tx_timeout_ms_);
   clients_[client_id] = std::move(client);
 }
 
diff --git a/src/ipc/host_impl.h b/src/ipc/host_impl.h
index ac0a59319..de7d07e29 100644
--- a/src/ipc/host_impl.h
+++ b/src/ipc/host_impl.h
@@ -32,6 +32,8 @@
 namespace perfetto {
 namespace ipc {
 
+constexpr uint32_t kDefaultIpcTxTimeoutMs = 10000;
+
 class HostImpl : public Host, public base::UnixSocket::EventListener {
  public:
   HostImpl(const char* socket_name, base::TaskRunner*);
@@ -40,6 +42,7 @@ class HostImpl : public Host, public base::UnixSocket::EventListener {
 
   // Host implementation.
   bool ExposeService(std::unique_ptr<Service>) override;
+  void SetSocketSendTimeoutMs(uint32_t timeout_ms) override;
 
   // base::UnixSocket::EventListener implementation.
   void OnNewIncomingConnection(base::UnixSocket*,
@@ -88,6 +91,7 @@ class HostImpl : public Host, public base::UnixSocket::EventListener {
   std::map<base::UnixSocket*, ClientConnection*> clients_by_socket_;
   ServiceID last_service_id_ = 0;
   ClientID last_client_id_ = 0;
+  uint32_t socket_tx_timeout_ms_ = kDefaultIpcTxTimeoutMs;
   PERFETTO_THREAD_CHECKER(thread_checker_)
   base::WeakPtrFactory<HostImpl> weak_ptr_factory_;  // Keep last.
 };
diff --git a/src/perfetto_cmd/perfetto_cmd.cc b/src/perfetto_cmd/perfetto_cmd.cc
index 3ed31406f..aafc8445d 100644
--- a/src/perfetto_cmd/perfetto_cmd.cc
+++ b/src/perfetto_cmd/perfetto_cmd.cc
@@ -1002,7 +1002,9 @@ void PerfettoCmd::OnConnect() {
   }
 
   PERFETTO_DCHECK(trace_config_);
-  trace_config_->set_enable_extra_guardrails(save_to_incidentd_);
+  trace_config_->set_enable_extra_guardrails(
+      (save_to_incidentd_ || report_to_android_framework_) &&
+      !ignore_guardrails_);
 
   // Set the statsd logging flag if we're uploading
 
diff --git a/src/profiling/common/producer_support.cc b/src/profiling/common/producer_support.cc
index 8ab0b42a6..a16014997 100644
--- a/src/profiling/common/producer_support.cc
+++ b/src/profiling/common/producer_support.cc
@@ -110,10 +110,25 @@ bool CanProfileAndroid(const DataSourceConfig& ds_config,
     return true;
   }
 
+  bool trusted_initiator = ds_config.session_initiator() ==
+                           DataSourceConfig::SESSION_INITIATOR_TRUSTED_SYSTEM;
+
   uint64_t uid_without_profile = uid % kAidUserOffset;
   uint64_t uid_for_lookup = 0;
-  if (uid_without_profile >= kAidAppStart &&
-      uid_without_profile <= kAidAppEnd) {
+  if (uid_without_profile < kAidAppStart) {
+    // Platform processes are considered profileable by the platform itself.
+    // This includes platform UIDs from other profiles, e.g. "u10_system".
+    // It's possible that this is an app (e.g. com.android.settings runs as
+    // AID_SYSTEM), but we will skip checking packages.list for the profileable
+    // manifest flags, as running under a platform UID is considered sufficient.
+    // Minor consequence: shell cannot profile platform apps, even if their
+    // manifest flags opt into profiling from shell. Resolving this would
+    // require definitively disambiguating native processes from apps if both
+    // can run as the same platform UID.
+    return trusted_initiator;
+
+  } else if (uid_without_profile >= kAidAppStart &&
+             uid_without_profile <= kAidAppEnd) {
     // normal app
     uid_for_lookup = uid_without_profile;
 
@@ -133,8 +148,6 @@ bool CanProfileAndroid(const DataSourceConfig& ds_config,
     // even be the package in which the service was defined).
     // TODO(rsavitski): find a way for the platform to tell native services
     // about isolated<->app relations.
-    bool trusted_initiator = ds_config.session_initiator() ==
-                             DataSourceConfig::SESSION_INITIATOR_TRUSTED_SYSTEM;
     return trusted_initiator &&
            AllPackagesProfileableByTrustedInitiator(packages_list_path);
 
diff --git a/src/traced/service/builtin_producer.cc b/src/traced/service/builtin_producer.cc
index a2007beb5..0664b5d0d 100644
--- a/src/traced/service/builtin_producer.cc
+++ b/src/traced/service/builtin_producer.cc
@@ -46,9 +46,11 @@ namespace {
 
 constexpr char kHeapprofdDataSourceName[] = "android.heapprofd";
 constexpr char kJavaHprofDataSourceName[] = "android.java_hprof";
+constexpr char kJavaHprofOomDataSourceName[] = "android.java_hprof.oom";
 constexpr char kTracedPerfDataSourceName[] = "linux.perf";
 constexpr char kLazyHeapprofdPropertyName[] = "traced.lazy.heapprofd";
 constexpr char kLazyTracedPerfPropertyName[] = "traced.lazy.traced_perf";
+constexpr char kJavaHprofOomActivePropertyName[] = "traced.oome_heap_session.count";
 
 }  // namespace
 
@@ -64,6 +66,8 @@ BuiltinProducer::~BuiltinProducer() {
     SetAndroidProperty(kLazyHeapprofdPropertyName, "");
   if (!lazy_traced_perf_.instance_ids.empty())
     SetAndroidProperty(kLazyTracedPerfPropertyName, "");
+  if (!java_hprof_oome_instances_.empty())
+    SetAndroidProperty(kJavaHprofOomActivePropertyName, "");
 }
 
 void BuiltinProducer::ConnectInProcess(TracingService* svc) {
@@ -102,6 +106,11 @@ void BuiltinProducer::OnConnect() {
     lazy_traced_perf_dsd.set_name(kTracedPerfDataSourceName);
     endpoint_->RegisterDataSource(lazy_traced_perf_dsd);
   }
+  {
+    DataSourceDescriptor java_hprof_oome_dsd;
+    java_hprof_oome_dsd.set_name(kJavaHprofOomDataSourceName);
+    endpoint_->RegisterDataSource(java_hprof_oome_dsd);
+  }
 }
 
 void BuiltinProducer::SetupDataSource(DataSourceInstanceID ds_id,
@@ -120,6 +129,13 @@ void BuiltinProducer::SetupDataSource(DataSourceInstanceID ds_id,
     lazy_traced_perf_.instance_ids.emplace(ds_id);
     return;
   }
+
+  if (ds_config.name() == kJavaHprofOomDataSourceName) {
+    java_hprof_oome_instances_.emplace(ds_id);
+    SetAndroidProperty(kJavaHprofOomActivePropertyName,
+                       std::to_string(java_hprof_oome_instances_.size()));
+    return;
+  }
 }
 
 void BuiltinProducer::StartDataSource(DataSourceInstanceID ds_id,
@@ -152,6 +168,13 @@ void BuiltinProducer::StopDataSource(DataSourceInstanceID ds_id) {
 
   MaybeInitiateLazyStop(ds_id, &lazy_heapprofd_, kLazyHeapprofdPropertyName);
   MaybeInitiateLazyStop(ds_id, &lazy_traced_perf_, kLazyTracedPerfPropertyName);
+
+  auto oome_it = java_hprof_oome_instances_.find(ds_id);
+  if (oome_it != java_hprof_oome_instances_.end()) {
+    java_hprof_oome_instances_.erase(oome_it);
+    SetAndroidProperty(kJavaHprofOomActivePropertyName,
+                       std::to_string(java_hprof_oome_instances_.size()));
+  }
 }
 
 void BuiltinProducer::MaybeInitiateLazyStop(DataSourceInstanceID ds_id,
diff --git a/src/traced/service/builtin_producer.h b/src/traced/service/builtin_producer.h
index 9873458ab..7014d4419 100644
--- a/src/traced/service/builtin_producer.h
+++ b/src/traced/service/builtin_producer.h
@@ -34,6 +34,7 @@ namespace perfetto {
 // * perfetto metatrace
 // * lazy heapprofd daemon starter (android only)
 // * lazy traced_perf daemon starter (android only)
+// * java_hprof oom data source counter (android only)
 class BuiltinProducer : public Producer {
  public:
   BuiltinProducer(base::TaskRunner* task_runner, uint32_t lazy_stop_delay_ms);
@@ -83,6 +84,7 @@ class BuiltinProducer : public Producer {
   MetatraceState metatrace_;
   LazyAndroidDaemonState lazy_heapprofd_;
   LazyAndroidDaemonState lazy_traced_perf_;
+  std::set<DataSourceInstanceID> java_hprof_oome_instances_;
 
   base::WeakPtrFactory<BuiltinProducer> weak_factory_;  // Keep last.
 };
diff --git a/src/tracing/core/tracing_service_impl.cc b/src/tracing/core/tracing_service_impl.cc
index 807b955d8..93419734e 100644
--- a/src/tracing/core/tracing_service_impl.cc
+++ b/src/tracing/core/tracing_service_impl.cc
@@ -613,6 +613,15 @@ base::Status TracingServiceImpl::EnableTracing(ConsumerEndpointImpl* consumer,
         cfg.trigger_config().trigger_timeout_ms());
   }
 
+  // This check has been introduced in May 2023 after finding b/274931668.
+  if (static_cast<int>(cfg.trigger_config().trigger_mode()) >
+      TraceConfig::TriggerConfig::TriggerMode_MAX) {
+    MaybeLogUploadEvent(
+        cfg, PerfettoStatsdAtom::kTracedEnableTracingInvalidTriggerMode);
+    return PERFETTO_SVC_ERR(
+        "The trace config specified an invalid trigger_mode");
+  }
+
   if (has_trigger_config && cfg.duration_ms() != 0) {
     MaybeLogUploadEvent(
         cfg, PerfettoStatsdAtom::kTracedEnableTracingDurationWithTrigger);
@@ -664,12 +673,16 @@ base::Status TracingServiceImpl::EnableTracing(ConsumerEndpointImpl* consumer,
       }
       buf_size_sum += buf.size_kb();
     }
-    if (buf_size_sum > kGuardrailsMaxTracingBufferSizeKb) {
+
+    uint32_t max_tracing_buffer_size_kb =
+        std::max(kGuardrailsMaxTracingBufferSizeKb,
+                 cfg.guardrail_overrides().max_tracing_buffer_size_kb());
+    if (buf_size_sum > max_tracing_buffer_size_kb) {
       MaybeLogUploadEvent(
           cfg, PerfettoStatsdAtom::kTracedEnableTracingBufferSizeTooLarge);
       return PERFETTO_SVC_ERR("Requested too large trace buffer (%" PRIu64
                               "kB  > %" PRIu32 " kB)",
-                              buf_size_sum, kGuardrailsMaxTracingBufferSizeKb);
+                              buf_size_sum, max_tracing_buffer_size_kb);
     }
   }
 
@@ -940,6 +953,9 @@ base::Status TracingServiceImpl::EnableTracing(ConsumerEndpointImpl* consumer,
       tracing_session->config.set_duration_ms(
           cfg.trigger_config().trigger_timeout_ms());
       break;
+
+      // The case of unknown modes (coming from future versions of the service)
+      // is handled few lines above (search for TriggerMode_MAX).
   }
 
   tracing_session->state = TracingSession::CONFIGURED;
diff --git a/src/tracing/core/tracing_service_impl_unittest.cc b/src/tracing/core/tracing_service_impl_unittest.cc
index 00b7a3a16..bce99e7ff 100644
--- a/src/tracing/core/tracing_service_impl_unittest.cc
+++ b/src/tracing/core/tracing_service_impl_unittest.cc
@@ -425,6 +425,31 @@ TEST_F(TracingServiceImplTest, StartTracingTriggerTimeOut) {
   EXPECT_THAT(consumer->ReadBuffers(), IsEmpty());
 }
 
+// Regression test for b/274931668. An unkonwn trigger should not cause a trace
+// that runs indefinitely.
+TEST_F(TracingServiceImplTest, FailOnUnknownTrigger) {
+  std::unique_ptr<MockConsumer> consumer = CreateMockConsumer();
+  consumer->Connect(svc.get());
+
+  std::unique_ptr<MockProducer> producer = CreateMockProducer();
+  producer->Connect(svc.get(), "mock_producer");
+  producer->RegisterDataSource("ds_1");
+
+  TraceConfig trace_config;
+  trace_config.add_buffers()->set_size_kb(128);
+  trace_config.add_data_sources()->mutable_config()->set_name("ds_1");
+  auto* trigger_config = trace_config.mutable_trigger_config();
+  trigger_config->set_trigger_mode(
+      static_cast<TraceConfig::TriggerConfig::TriggerMode>(
+          TraceConfig::TriggerConfig::TriggerMode_MAX + 1));
+  auto* trigger = trigger_config->add_triggers();
+  trigger->set_name("trigger_from_the_future");
+  trigger_config->set_trigger_timeout_ms(1);
+
+  consumer->EnableTracing(trace_config);
+  consumer->WaitForTracingDisabled();
+}
+
 // Creates a tracing session with a START_TRACING trigger and checks that
 // the session is not started when the configured trigger producer is different
 // than the producer that sent the trigger.
diff --git a/src/tracing/ipc/service/service_ipc_host_impl.cc b/src/tracing/ipc/service/service_ipc_host_impl.cc
index 5618b3cb3..3598bf59e 100644
--- a/src/tracing/ipc/service/service_ipc_host_impl.cc
+++ b/src/tracing/ipc/service/service_ipc_host_impl.cc
@@ -31,6 +31,10 @@
 
 namespace perfetto {
 
+namespace {
+constexpr uint32_t kProducerSocketTxTimeoutMs = 10;
+}
+
 // TODO(fmayer): implement per-uid connection limit (b/69093705).
 
 // Implements the publicly exposed factory method declared in
@@ -85,6 +89,16 @@ bool ServiceIPCHostImpl::DoStart() {
     return false;
   }
 
+  // Lower the timeout for blocking socket sends to producers as we shouldn't
+  // normally exhaust the kernel send buffer unless the producer is
+  // unresponsive. We'll drop the connection if the timeout is hit (see
+  // UnixSocket::Send). Context in b/236813972, b/193234818.
+  // Consumer port continues using the default timeout (10s) as there are
+  // generally fewer consumer processes, and they're better behaved. Also the
+  // consumer port ipcs might exhaust the send buffer under normal operation
+  // due to large messages such as ReadBuffersResponse.
+  producer_ipc_port_->SetSocketSendTimeoutMs(kProducerSocketTxTimeoutMs);
+
   // TODO(fmayer): add a test that destroyes the ServiceIPCHostImpl soon after
   // Start() and checks that no spurious callbacks are issued.
   bool producer_service_exposed = producer_ipc_port_->ExposeService(