Merge "Avoid integer underflow involving maxFreqCount."
diff --git a/cmds/atrace/atrace.rc b/cmds/atrace/atrace.rc
index 994375b..db3a314 100644
--- a/cmds/atrace/atrace.rc
+++ b/cmds/atrace/atrace.rc
@@ -220,6 +220,67 @@
chmod 0666 /sys/kernel/debug/tracing/per_cpu/cpu15/trace
chmod 0666 /sys/kernel/tracing/per_cpu/cpu15/trace
+on post-fs-data
+# Create MM Events Tracing Instance for Kmem Activity Trigger
+ mkdir /sys/kernel/debug/tracing/instances/mm_events 0755 system system
+ mkdir /sys/kernel/tracing/instances/mm_events 0755 system system
+
+# Read and set per CPU buffer size
+ chmod 0666 /sys/kernel/debug/tracing/instances/mm_events/buffer_size_kb
+ chmod 0666 /sys/kernel/tracing/instances/mm_events/buffer_size_kb
+
+# Read and enable tracing
+ chmod 0666 /sys/kernel/debug/tracing/instances/mm_events/tracing_on
+ chmod 0666 /sys/kernel/tracing/instances/mm_events/tracing_on
+
+# Read and truncate kernel trace
+ chmod 0666 /sys/kernel/debug/tracing/instances/mm_events/trace
+ chmod 0666 /sys/kernel/tracing/instances/mm_events/trace
+
+# Enable trace events
+ chmod 0666 /sys/kernel/debug/tracing/instances/mm_events/events/vmscan/mm_vmscan_direct_reclaim_begin/enable
+ chmod 0666 /sys/kernel/tracing/instances/mm_events/events/vmscan/mm_vmscan_direct_reclaim_begin/enable
+ chmod 0666 /sys/kernel/debug/tracing/instances/mm_events/events/vmscan/mm_vmscan_kswapd_wake/enable
+ chmod 0666 /sys/kernel/tracing/instances/mm_events/events/vmscan/mm_vmscan_kswapd_wake/enable
+ chmod 0666 /sys/kernel/debug/tracing/instances/mm_events/events/compaction/mm_compaction_begin/enable
+ chmod 0666 /sys/kernel/tracing/instances/mm_events/events/compaction/mm_compaction_begin/enable
+
+# Read and clear per-CPU raw kernel trace
+# Cannot use wildcards in .rc files. Update this if there is a phone with
+# more CPUs.
+ chmod 0666 /sys/kernel/debug/tracing/instances/mm_events/per_cpu/cpu0/trace
+ chmod 0666 /sys/kernel/tracing/instances/mm_events/per_cpu/cpu0/trace
+ chmod 0666 /sys/kernel/debug/tracing/instances/mm_events/per_cpu/cpu1/trace
+ chmod 0666 /sys/kernel/tracing/instances/mm_events/per_cpu/cpu1/trace
+ chmod 0666 /sys/kernel/debug/tracing/instances/mm_events/per_cpu/cpu2/trace
+ chmod 0666 /sys/kernel/tracing/instances/mm_events/per_cpu/cpu2/trace
+ chmod 0666 /sys/kernel/debug/tracing/instances/mm_events/per_cpu/cpu3/trace
+ chmod 0666 /sys/kernel/tracing/instances/mm_events/per_cpu/cpu3/trace
+ chmod 0666 /sys/kernel/debug/tracing/instances/mm_events/per_cpu/cpu4/trace
+ chmod 0666 /sys/kernel/tracing/instances/mm_events/per_cpu/cpu4/trace
+ chmod 0666 /sys/kernel/debug/tracing/instances/mm_events/per_cpu/cpu5/trace
+ chmod 0666 /sys/kernel/tracing/instances/mm_events/per_cpu/cpu5/trace
+ chmod 0666 /sys/kernel/debug/tracing/instances/mm_events/per_cpu/cpu6/trace
+ chmod 0666 /sys/kernel/tracing/instances/mm_events/per_cpu/cpu6/trace
+ chmod 0666 /sys/kernel/debug/tracing/instances/mm_events/per_cpu/cpu7/trace
+ chmod 0666 /sys/kernel/tracing/instances/mm_events/per_cpu/cpu7/trace
+ chmod 0666 /sys/kernel/debug/tracing/instances/mm_events/per_cpu/cpu8/trace
+ chmod 0666 /sys/kernel/tracing/instances/mm_events/per_cpu/cpu8/trace
+ chmod 0666 /sys/kernel/debug/tracing/instances/mm_events/per_cpu/cpu9/trace
+ chmod 0666 /sys/kernel/tracing/instances/mm_events/per_cpu/cpu9/trace
+ chmod 0666 /sys/kernel/debug/tracing/instances/mm_events/per_cpu/cpu10/trace
+ chmod 0666 /sys/kernel/tracing/instances/mm_events/per_cpu/cpu10/trace
+ chmod 0666 /sys/kernel/debug/tracing/instances/mm_events/per_cpu/cpu11/trace
+ chmod 0666 /sys/kernel/tracing/instances/mm_events/per_cpu/cpu11/trace
+ chmod 0666 /sys/kernel/debug/tracing/instances/mm_events/per_cpu/cpu12/trace
+ chmod 0666 /sys/kernel/tracing/instances/mm_events/per_cpu/cpu12/trace
+ chmod 0666 /sys/kernel/debug/tracing/instances/mm_events/per_cpu/cpu13/trace
+ chmod 0666 /sys/kernel/tracing/instances/mm_events/per_cpu/cpu13/trace
+ chmod 0666 /sys/kernel/debug/tracing/instances/mm_events/per_cpu/cpu14/trace
+ chmod 0666 /sys/kernel/tracing/instances/mm_events/per_cpu/cpu14/trace
+ chmod 0666 /sys/kernel/debug/tracing/instances/mm_events/per_cpu/cpu15/trace
+ chmod 0666 /sys/kernel/tracing/instances/mm_events/per_cpu/cpu15/trace
+
on property:persist.debug.atrace.boottrace=1
start boottrace
diff --git a/cmds/dumpstate/DumpstateService.cpp b/cmds/dumpstate/DumpstateService.cpp
index bfcc058..ba25a5a 100644
--- a/cmds/dumpstate/DumpstateService.cpp
+++ b/cmds/dumpstate/DumpstateService.cpp
@@ -39,8 +39,13 @@
std::string calling_package;
};
-static binder::Status exception(uint32_t code, const std::string& msg) {
- MYLOGE("%s (%d) ", msg.c_str(), code);
+static binder::Status exception(uint32_t code, const std::string& msg,
+ const std::string& extra_msg = "") {
+ if (extra_msg.empty()) {
+ MYLOGE("%s (%d) ", msg.c_str(), code);
+ } else {
+ MYLOGE("%s %s (%d) ", msg.c_str(), extra_msg.c_str(), code);
+ }
return binder::Status::fromExceptionCode(code, String8(msg.c_str()));
}
@@ -60,7 +65,7 @@
} // namespace
-DumpstateService::DumpstateService() : ds_(nullptr) {
+DumpstateService::DumpstateService() : ds_(nullptr), calling_uid_(-1), calling_package_() {
}
char const* DumpstateService::getServiceName() {
@@ -131,6 +136,10 @@
ds_->SetOptions(std::move(options));
ds_->listener_ = listener;
+ // Track caller info for cancellation purposes.
+ calling_uid_ = calling_uid;
+ calling_package_ = calling_package;
+
DumpstateInfo* ds_info = new DumpstateInfo();
ds_info->ds = ds_;
ds_info->calling_uid = calling_uid;
@@ -149,8 +158,20 @@
return binder::Status::ok();
}
-binder::Status DumpstateService::cancelBugreport() {
+binder::Status DumpstateService::cancelBugreport(int32_t calling_uid,
+ const std::string& calling_package) {
std::lock_guard<std::mutex> lock(lock_);
+ if (calling_uid != calling_uid_ || calling_package != calling_package_) {
+ // Note: we use a SecurityException to prevent BugreportManagerServiceImpl from killing the
+ // report in progress (from another caller).
+ return exception(
+ binder::Status::EX_SECURITY,
+ StringPrintf("Cancellation requested by %d/%s does not match report in "
+ "progress",
+ calling_uid, calling_package.c_str()),
+ // Sharing the owner of the BR is a (minor) leak, so leave it out of the app's exception
+ StringPrintf("started by %d/%s", calling_uid_, calling_package_.c_str()));
+ }
ds_->Cancel();
return binder::Status::ok();
}
diff --git a/cmds/dumpstate/DumpstateService.h b/cmds/dumpstate/DumpstateService.h
index ac8d3ac..3ec8471 100644
--- a/cmds/dumpstate/DumpstateService.h
+++ b/cmds/dumpstate/DumpstateService.h
@@ -44,8 +44,7 @@
const sp<IDumpstateListener>& listener,
bool is_screenshot_requested) override;
- // No-op
- binder::Status cancelBugreport();
+ binder::Status cancelBugreport(int32_t calling_uid, const std::string& calling_package);
private:
// Dumpstate object which contains all the bugreporting logic.
@@ -53,6 +52,8 @@
// one bugreport.
// This service does not own this object.
Dumpstate* ds_;
+ int32_t calling_uid_;
+ std::string calling_package_;
std::mutex lock_;
};
diff --git a/cmds/dumpstate/DumpstateUtil.cpp b/cmds/dumpstate/DumpstateUtil.cpp
index eeaa5a3..c833d0e 100644
--- a/cmds/dumpstate/DumpstateUtil.cpp
+++ b/cmds/dumpstate/DumpstateUtil.cpp
@@ -124,6 +124,12 @@
return *this;
}
+CommandOptions::CommandOptionsBuilder&
+CommandOptions::CommandOptionsBuilder::CloseAllFileDescriptorsOnExec() {
+ values.close_all_fds_on_exec_ = true;
+ return *this;
+}
+
CommandOptions::CommandOptionsBuilder& CommandOptions::CommandOptionsBuilder::Log(
const std::string& message) {
values.logging_message_ = message;
@@ -137,6 +143,7 @@
CommandOptions::CommandOptionsValues::CommandOptionsValues(int64_t timeout_ms)
: timeout_ms_(timeout_ms),
always_(false),
+ close_all_fds_on_exec_(false),
account_mode_(DONT_DROP_ROOT),
output_mode_(NORMAL_OUTPUT),
logging_message_("") {
@@ -157,6 +164,10 @@
return values.always_;
}
+bool CommandOptions::ShouldCloseAllFileDescriptorsOnExec() const {
+ return values.close_all_fds_on_exec_;
+}
+
PrivilegeMode CommandOptions::PrivilegeMode() const {
return values.account_mode_;
}
@@ -277,7 +288,8 @@
MYLOGI(logging_message.c_str(), command_string.c_str());
}
- bool silent = (options.OutputMode() == REDIRECT_TO_STDERR);
+ bool silent = (options.OutputMode() == REDIRECT_TO_STDERR ||
+ options.ShouldCloseAllFileDescriptorsOnExec());
bool redirecting_to_fd = STDOUT_FILENO != fd;
if (PropertiesHelper::IsDryRun() && !options.Always()) {
@@ -314,7 +326,27 @@
return -1;
}
- if (silent) {
+ if (options.ShouldCloseAllFileDescriptorsOnExec()) {
+ int devnull_fd = TEMP_FAILURE_RETRY(open("/dev/null", O_RDONLY));
+ TEMP_FAILURE_RETRY(dup2(devnull_fd, STDIN_FILENO));
+ close(devnull_fd);
+ devnull_fd = TEMP_FAILURE_RETRY(open("/dev/null", O_WRONLY));
+ TEMP_FAILURE_RETRY(dup2(devnull_fd, STDOUT_FILENO));
+ TEMP_FAILURE_RETRY(dup2(devnull_fd, STDERR_FILENO));
+ close(devnull_fd);
+ // This is to avoid leaking FDs that, accidentally, have not been
+ // marked as O_CLOEXEC. Leaking FDs across exec can cause failures
+ // when execing a process that has a SELinux auto_trans rule.
+ // Here we assume that the dumpstate process didn't open more than
+ // 1000 FDs. In theory we could iterate through /proc/self/fd/, but
+ // doing that in a fork-safe way is too complex and not worth it
+ // (opendir()/readdir() do heap allocations and take locks).
+ for (int i = 0; i < 1000; i++) {
+ if (i != STDIN_FILENO && i!= STDOUT_FILENO && i != STDERR_FILENO) {
+ close(i);
+ }
+ }
+ } else if (silent) {
// Redirects stdout to stderr
TEMP_FAILURE_RETRY(dup2(STDERR_FILENO, STDOUT_FILENO));
} else if (redirecting_to_fd) {
diff --git a/cmds/dumpstate/DumpstateUtil.h b/cmds/dumpstate/DumpstateUtil.h
index b099443..b00c46e 100644
--- a/cmds/dumpstate/DumpstateUtil.h
+++ b/cmds/dumpstate/DumpstateUtil.h
@@ -80,6 +80,7 @@
int64_t timeout_ms_;
bool always_;
+ bool close_all_fds_on_exec_;
PrivilegeMode account_mode_;
OutputMode output_mode_;
std::string logging_message_;
@@ -112,6 +113,13 @@
CommandOptionsBuilder& DropRoot();
/* Sets the command's OutputMode as `REDIRECT_TO_STDERR` */
CommandOptionsBuilder& RedirectStderr();
+ /* Closes all file descriptors before exec-ing the target process. This
+ * includes also stdio pipes, which are dup-ed on /dev/null. It prevents
+ * leaking opened FDs to the target process, which in turn can hit
+ * selinux denials in presence of auto_trans rules.
+ */
+ CommandOptionsBuilder& CloseAllFileDescriptorsOnExec();
+
/* When not empty, logs a message before executing the command.
* Must contain a `%s`, which will be replaced by the full command line, and end on `\n`. */
CommandOptionsBuilder& Log(const std::string& message);
@@ -130,6 +138,8 @@
int64_t TimeoutInMs() const;
/* Checks whether the command should always be run, even on dry-run mode. */
bool Always() const;
+ /* Checks whether all FDs should be closed prior to the exec() calls. */
+ bool ShouldCloseAllFileDescriptorsOnExec() const;
/** Gets the PrivilegeMode of the command. */
PrivilegeMode PrivilegeMode() const;
/** Gets the OutputMode of the command. */
diff --git a/cmds/dumpstate/binder/android/os/IDumpstate.aidl b/cmds/dumpstate/binder/android/os/IDumpstate.aidl
index ba008bb..0793f0b 100644
--- a/cmds/dumpstate/binder/android/os/IDumpstate.aidl
+++ b/cmds/dumpstate/binder/android/os/IDumpstate.aidl
@@ -1,4 +1,4 @@
-/**
+/*
* Copyright (c) 2016, The Android Open Source Project
*
* Licensed under the Apache License, Version 2.0 (the "License");
@@ -19,9 +19,9 @@
import android.os.IDumpstateListener;
/**
- * Binder interface for the currently running dumpstate process.
- * {@hide}
- */
+ * Binder interface for the currently running dumpstate process.
+ * {@hide}
+ */
interface IDumpstate {
// NOTE: If you add to or change these modes, please also change the corresponding enums
@@ -49,10 +49,10 @@
// Default mode.
const int BUGREPORT_MODE_DEFAULT = 6;
- /*
+ /**
* Starts a bugreport in the background.
*
- *<p>Shows the user a dialog to get consent for sharing the bugreport with the calling
+ * <p>Shows the user a dialog to get consent for sharing the bugreport with the calling
* application. If they deny {@link IDumpstateListener#onError} will be called. If they
* consent and bugreport generation is successful artifacts will be copied to the given fds and
* {@link IDumpstateListener#onFinished} will be called. If there
@@ -71,8 +71,15 @@
int bugreportMode, IDumpstateListener listener,
boolean isScreenshotRequested);
- /*
+ /**
* Cancels the bugreport currently in progress.
+ *
+ * <p>The caller must match the original caller of {@link #startBugreport} in order for the
+ * report to actually be cancelled. A {@link SecurityException} is reported if a mismatch is
+ * detected.
+ *
+ * @param callingUid UID of the original application that requested the cancellation.
+ * @param callingPackage package of the original application that requested the cancellation.
*/
- void cancelBugreport();
+ void cancelBugreport(int callingUid, @utf8InCpp String callingPackage);
}
diff --git a/cmds/dumpstate/binder/android/os/IDumpstateListener.aidl b/cmds/dumpstate/binder/android/os/IDumpstateListener.aidl
index 85e6969..50c1624 100644
--- a/cmds/dumpstate/binder/android/os/IDumpstateListener.aidl
+++ b/cmds/dumpstate/binder/android/os/IDumpstateListener.aidl
@@ -21,8 +21,6 @@
*
* <p>When bugreport creation is complete one of {@code onError} or {@code onFinished} is called.
*
- * <p>These methods are synchronous by design in order to make dumpstate's lifecycle simpler
- * to handle.
*
* {@hide}
*/
@@ -54,10 +52,8 @@
/**
* Called on an error condition with one of the error codes listed above.
- * This is not an asynchronous method since it can race with dumpstate exiting, thus triggering
- * death recipient.
*/
- void onError(int errorCode);
+ oneway void onError(int errorCode);
/**
* Called when taking bugreport finishes successfully.
diff --git a/cmds/dumpstate/dumpstate.cpp b/cmds/dumpstate/dumpstate.cpp
index 9c5b883..fbb0a18 100644
--- a/cmds/dumpstate/dumpstate.cpp
+++ b/cmds/dumpstate/dumpstate.cpp
@@ -28,6 +28,7 @@
#include <stdio.h>
#include <stdlib.h>
#include <string.h>
+#include <sys/mount.h>
#include <sys/poll.h>
#include <sys/prctl.h>
#include <sys/resource.h>
@@ -174,6 +175,7 @@
#define SNAPSHOTCTL_LOG_DIR "/data/misc/snapshotctl_log"
#define LINKERCONFIG_DIR "/linkerconfig"
#define PACKAGE_DEX_USE_LIST "/data/system/package-dex-usage.list"
+#define SYSTEM_TRACE_SNAPSHOT "/data/misc/perfetto-traces/bugreport/systrace.pftrace"
// TODO(narayan): Since this information has to be kept in sync
// with tombstoned, we should just put it in a common header.
@@ -1052,6 +1054,24 @@
}
}
+static void MaybeAddSystemTraceToZip() {
+ // This function copies into the .zip the system trace that was snapshotted
+ // by the early call to MaybeSnapshotSystemTrace(), if any background
+ // tracing was happening.
+ if (!ds.IsZipping()) {
+ MYLOGD("Not dumping system trace because it's not a zipped bugreport\n");
+ return;
+ }
+ if (!ds.has_system_trace_) {
+ // No background trace was happening at the time dumpstate was invoked.
+ return;
+ }
+ ds.AddZipEntry(
+ ZIP_ROOT_DIR + SYSTEM_TRACE_SNAPSHOT,
+ SYSTEM_TRACE_SNAPSHOT);
+ android::os::UnlinkAndLogOnError(SYSTEM_TRACE_SNAPSHOT);
+}
+
static void DumpVisibleWindowViews() {
if (!ds.IsZipping()) {
MYLOGD("Not dumping visible views because it's not a zipped bugreport\n");
@@ -1548,7 +1568,7 @@
dprintf(out_fd, "========================================================\n");
RunDumpsys("APP PROVIDERS PLATFORM", {"activity", "provider", "all-platform"},
- DUMPSYS_COMPONENTS_OPTIONS, out_fd);
+ DUMPSYS_COMPONENTS_OPTIONS, 0, out_fd);
dprintf(out_fd, "========================================================\n");
dprintf(out_fd, "== Running Application Providers (non-platform)\n");
@@ -1648,6 +1668,8 @@
AddAnrTraceFiles();
+ MaybeAddSystemTraceToZip();
+
// NOTE: tombstones are always added as separate entries in the zip archive
// and are not interspersed with the main report.
const bool tombstones_dumped = AddDumps(ds.tombstone_data_.begin(), ds.tombstone_data_.end(),
@@ -2156,6 +2178,22 @@
return;
}
+ /*
+ * mount debugfs for non-user builds which launch with S and unmount it
+ * after invoking dumpstateBoard_* methods. This is to enable debug builds
+ * to not have debugfs mounted during runtime. It will also ensure that
+ * debugfs is only accessed by the dumpstate HAL.
+ */
+ auto api_level = android::base::GetIntProperty("ro.product.first_api_level", 0);
+ bool mount_debugfs = !PropertiesHelper::IsUserBuild() && api_level >= 31;
+
+ if (mount_debugfs) {
+ RunCommand("mount debugfs", {"mount", "-t", "debugfs", "debugfs", "/sys/kernel/debug"},
+ AS_ROOT_20);
+ RunCommand("chmod debugfs", {"chmod", "0755", "/sys/kernel/debug"},
+ AS_ROOT_20);
+ }
+
std::vector<std::string> paths;
std::vector<android::base::ScopeGuard<std::function<void()>>> remover;
for (int i = 0; i < NUM_OF_DUMPS; i++) {
@@ -2255,6 +2293,10 @@
"there might be racing in content\n", killing_timeout_sec);
}
+ if (mount_debugfs) {
+ RunCommand("unmount debugfs", {"umount", "/sys/kernel/debug"}, AS_ROOT_20);
+ }
+
auto file_sizes = std::make_unique<ssize_t[]>(paths.size());
for (size_t i = 0; i < paths.size(); i++) {
struct stat s;
@@ -2865,6 +2907,13 @@
RunDumpsysCritical();
}
MaybeTakeEarlyScreenshot();
+
+ if (!is_dumpstate_restricted) {
+ // Snapshot the system trace now (if running) to avoid that dumpstate's
+ // own activity pushes out interesting data from the trace ring buffer.
+ // The trace file is added to the zip by MaybeAddSystemTraceToZip().
+ MaybeSnapshotSystemTrace();
+ }
onUiIntensiveBugreportDumpsFinished(calling_uid);
MaybeCheckUserConsent(calling_uid, calling_package);
if (options_->telephony_only) {
@@ -2955,6 +3004,26 @@
TakeScreenshot();
}
+void Dumpstate::MaybeSnapshotSystemTrace() {
+ // If a background system trace is happening and is marked as "suitable for
+ // bugreport" (i.e. bugreport_score > 0 in the trace config), this command
+ // will stop it and serialize into SYSTEM_TRACE_SNAPSHOT. In the (likely)
+ // case that no trace is ongoing, this command is a no-op.
+ // Note: this should not be enqueued as we need to freeze the trace before
+ // dumpstate starts. Otherwise the trace ring buffers will contain mostly
+ // the dumpstate's own activity which is irrelevant.
+ int res = RunCommand(
+ "SERIALIZE PERFETTO TRACE",
+ {"perfetto", "--save-for-bugreport"},
+ CommandOptions::WithTimeout(10)
+ .DropRoot()
+ .CloseAllFileDescriptorsOnExec()
+ .Build());
+ has_system_trace_ = res == 0;
+ // MaybeAddSystemTraceToZip() will take care of copying the trace in the zip
+ // file in the later stages.
+}
+
void Dumpstate::onUiIntensiveBugreportDumpsFinished(int32_t calling_uid) {
if (calling_uid == AID_SHELL || !CalledByApi()) {
return;
diff --git a/cmds/dumpstate/dumpstate.h b/cmds/dumpstate/dumpstate.h
index 255243f..f83968b 100644
--- a/cmds/dumpstate/dumpstate.h
+++ b/cmds/dumpstate/dumpstate.h
@@ -458,6 +458,11 @@
// Whether it should take an screenshot earlier in the process.
bool do_early_screenshot_ = false;
+ // This is set to true when the trace snapshot request in the early call to
+ // MaybeSnapshotSystemTrace(). When this is true, the later stages of
+ // dumpstate will append the trace to the zip archive.
+ bool has_system_trace_ = false;
+
std::unique_ptr<Progress> progress_;
// When set, defines a socket file-descriptor use to report progress to bugreportz
@@ -543,6 +548,7 @@
RunStatus DumpstateDefaultAfterCritical();
void MaybeTakeEarlyScreenshot();
+ void MaybeSnapshotSystemTrace();
void onUiIntensiveBugreportDumpsFinished(int32_t calling_uid);
diff --git a/cmds/dumpstate/tests/dumpstate_smoke_test.cpp b/cmds/dumpstate/tests/dumpstate_smoke_test.cpp
index fe6a34a..0e366cb 100644
--- a/cmds/dumpstate/tests/dumpstate_smoke_test.cpp
+++ b/cmds/dumpstate/tests/dumpstate_smoke_test.cpp
@@ -319,6 +319,16 @@
*/
class BugreportSectionTest : public Test {
public:
+ ZipArchiveHandle handle;
+
+ void SetUp() {
+ ASSERT_EQ(OpenArchive(ZippedBugreportGenerationTest::getZipFilePath().c_str(), &handle), 0);
+ }
+
+ void TearDown() {
+ CloseArchive(handle);
+ }
+
static void SetUpTestCase() {
ParseSections(ZippedBugreportGenerationTest::getZipFilePath().c_str(),
ZippedBugreportGenerationTest::sections.get());
@@ -343,6 +353,19 @@
}
FAIL() << sectionName << " not found.";
}
+
+ /**
+ * Whether or not the content of the section is injected by other commands.
+ */
+ bool IsContentInjectedByOthers(const std::string& line) {
+ // Command header such as `------ APP ACTIVITIES (/system/bin/dumpsys activity -v) ------`.
+ static const std::regex kCommandHeader = std::regex{"------ .+ \\(.+\\) ------"};
+ std::smatch match;
+ if (std::regex_match(line, match, kCommandHeader)) {
+ return true;
+ }
+ return false;
+ }
};
TEST_F(BugreportSectionTest, Atleast3CriticalDumpsysSectionsGenerated) {
@@ -400,6 +423,28 @@
SectionExists("wifi", /* bytes= */ 100000);
}
+TEST_F(BugreportSectionTest, NoInjectedContentByOtherCommand) {
+ // Extract the main entry to a temp file
+ TemporaryFile tmp_binary;
+ ASSERT_NE(-1, tmp_binary.fd);
+ ExtractBugreport(&handle, tmp_binary.fd);
+
+ // Read line by line and identify sections
+ std::ifstream ifs(tmp_binary.path, std::ifstream::in);
+ std::string line;
+ std::string current_section_name;
+ while (std::getline(ifs, line)) {
+ std::string section_name;
+ if (IsSectionStart(line, §ion_name)) {
+ current_section_name = section_name;
+ } else if (IsSectionEnd(line)) {
+ current_section_name = "";
+ } else if (!current_section_name.empty()) {
+ EXPECT_FALSE(IsContentInjectedByOthers(line));
+ }
+ }
+}
+
class DumpstateBinderTest : public Test {
protected:
void SetUp() override {
diff --git a/cmds/servicemanager/main.cpp b/cmds/servicemanager/main.cpp
index b1bc6dc..627dfe6 100644
--- a/cmds/servicemanager/main.cpp
+++ b/cmds/servicemanager/main.cpp
@@ -45,10 +45,6 @@
IPCThreadState::self()->setupPolling(&binder_fd);
LOG_ALWAYS_FATAL_IF(binder_fd < 0, "Failed to setupPolling: %d", binder_fd);
- // Flush after setupPolling(), to make sure the binder driver
- // knows about this thread handling commands.
- IPCThreadState::self()->flushCommands();
-
int ret = looper->addFd(binder_fd,
Looper::POLL_CALLBACK,
Looper::EVENT_INPUT,
diff --git a/libs/binder/Android.bp b/libs/binder/Android.bp
index 08b984e..e754d74 100644
--- a/libs/binder/Android.bp
+++ b/libs/binder/Android.bp
@@ -181,15 +181,26 @@
"--header-filter=^.*frameworks/native/libs/binder/.*.h$",
],
tidy_checks_as_errors: [
- "*",
+ // Explicitly list the checks that should not occur in this module.
+ "abseil-*",
+ "android-*",
+ "bugprone-*",
+ "cert-*",
+ "clang-analyzer-*",
"-clang-analyzer-core.CallAndMessage",
"-clang-analyzer-core.uninitialized.Assign",
- "-clang-analyzer-unix.Malloc,",
+ "-clang-analyzer-unix.Malloc",
"-clang-analyzer-deadcode.DeadStores",
"-clang-analyzer-optin.cplusplus.UninitializedObject",
+ "google-*",
+ "-google-readability-*",
+ "-google-runtime-references",
+ "misc-*",
"-misc-no-recursion",
"-misc-redundant-expression",
"-misc-unused-using-decls",
+ "performance*",
+ "portability*",
],
}
diff --git a/libs/binder/IPCThreadState.cpp b/libs/binder/IPCThreadState.cpp
index 7d01e0b..5c34069 100644
--- a/libs/binder/IPCThreadState.cpp
+++ b/libs/binder/IPCThreadState.cpp
@@ -486,15 +486,30 @@
}
}
+bool IPCThreadState::flushIfNeeded()
+{
+ if (mIsLooper || mServingStackPointer != nullptr) {
+ return false;
+ }
+ // In case this thread is not a looper and is not currently serving a binder transaction,
+ // there's no guarantee that this thread will call back into the kernel driver any time
+ // soon. Therefore, flush pending commands such as BC_FREE_BUFFER, to prevent them from getting
+ // stuck in this thread's out buffer.
+ flushCommands();
+ return true;
+}
+
void IPCThreadState::blockUntilThreadAvailable()
{
pthread_mutex_lock(&mProcess->mThreadCountLock);
+ mProcess->mWaitingForThreads++;
while (mProcess->mExecutingThreadsCount >= mProcess->mMaxThreads) {
ALOGW("Waiting for thread to be free. mExecutingThreadsCount=%lu mMaxThreads=%lu\n",
static_cast<unsigned long>(mProcess->mExecutingThreadsCount),
static_cast<unsigned long>(mProcess->mMaxThreads));
pthread_cond_wait(&mProcess->mThreadCountDecrement, &mProcess->mThreadCountLock);
}
+ mProcess->mWaitingForThreads--;
pthread_mutex_unlock(&mProcess->mThreadCountLock);
}
@@ -534,7 +549,12 @@
}
mProcess->mStarvationStartTimeMs = 0;
}
- pthread_cond_broadcast(&mProcess->mThreadCountDecrement);
+
+ // Cond broadcast can be expensive, so don't send it every time a binder
+ // call is processed. b/168806193
+ if (mProcess->mWaitingForThreads > 0) {
+ pthread_cond_broadcast(&mProcess->mThreadCountDecrement);
+ }
pthread_mutex_unlock(&mProcess->mThreadCountLock);
}
@@ -597,6 +617,7 @@
mOut.writeInt32(isMain ? BC_ENTER_LOOPER : BC_REGISTER_LOOPER);
+ mIsLooper = true;
status_t result;
do {
processPendingDerefs();
@@ -619,6 +640,7 @@
(void*)pthread_self(), getpid(), result);
mOut.writeInt32(BC_EXIT_LOOPER);
+ mIsLooper = false;
talkWithDriver(false);
}
@@ -629,6 +651,7 @@
}
mOut.writeInt32(BC_ENTER_LOOPER);
+ flushCommands();
*fd = mProcess->mDriverFD;
return 0;
}
@@ -731,9 +754,11 @@
LOG_REMOTEREFS("IPCThreadState::incStrongHandle(%d)\n", handle);
mOut.writeInt32(BC_ACQUIRE);
mOut.writeInt32(handle);
- // Create a temp reference until the driver has handled this command.
- proxy->incStrong(mProcess.get());
- mPostWriteStrongDerefs.push(proxy);
+ if (!flushIfNeeded()) {
+ // Create a temp reference until the driver has handled this command.
+ proxy->incStrong(mProcess.get());
+ mPostWriteStrongDerefs.push(proxy);
+ }
}
void IPCThreadState::decStrongHandle(int32_t handle)
@@ -741,6 +766,7 @@
LOG_REMOTEREFS("IPCThreadState::decStrongHandle(%d)\n", handle);
mOut.writeInt32(BC_RELEASE);
mOut.writeInt32(handle);
+ flushIfNeeded();
}
void IPCThreadState::incWeakHandle(int32_t handle, BpBinder *proxy)
@@ -748,9 +774,11 @@
LOG_REMOTEREFS("IPCThreadState::incWeakHandle(%d)\n", handle);
mOut.writeInt32(BC_INCREFS);
mOut.writeInt32(handle);
- // Create a temp reference until the driver has handled this command.
- proxy->getWeakRefs()->incWeak(mProcess.get());
- mPostWriteWeakDerefs.push(proxy->getWeakRefs());
+ if (!flushIfNeeded()) {
+ // Create a temp reference until the driver has handled this command.
+ proxy->getWeakRefs()->incWeak(mProcess.get());
+ mPostWriteWeakDerefs.push(proxy->getWeakRefs());
+ }
}
void IPCThreadState::decWeakHandle(int32_t handle)
@@ -758,6 +786,7 @@
LOG_REMOTEREFS("IPCThreadState::decWeakHandle(%d)\n", handle);
mOut.writeInt32(BC_DECREFS);
mOut.writeInt32(handle);
+ flushIfNeeded();
}
status_t IPCThreadState::attemptIncStrongHandle(int32_t handle)
@@ -813,6 +842,7 @@
mServingStackPointer(nullptr),
mWorkSource(kUnsetWorkSource),
mPropagateWorkSource(false),
+ mIsLooper(false),
mStrictModePolicy(0),
mLastTransactionBinderFlags(0),
mCallRestriction(mProcess->mCallRestriction)
@@ -1393,6 +1423,7 @@
IPCThreadState* state = self();
state->mOut.writeInt32(BC_FREE_BUFFER);
state->mOut.writePointer((uintptr_t)data);
+ state->flushIfNeeded();
}
} // namespace android
diff --git a/libs/binder/LazyServiceRegistrar.cpp b/libs/binder/LazyServiceRegistrar.cpp
index 325e204..21a9251 100644
--- a/libs/binder/LazyServiceRegistrar.cpp
+++ b/libs/binder/LazyServiceRegistrar.cpp
@@ -14,6 +14,7 @@
* limitations under the License.
*/
+#include "log/log_main.h"
#define LOG_TAG "AidlLazyServiceRegistrar"
#include <binder/LazyServiceRegistrar.h>
@@ -41,6 +42,12 @@
*/
void forcePersist(bool persist);
+ void setActiveServicesCallback(const std::function<bool(bool)>& activeServicesCallback);
+
+ bool tryUnregister();
+
+ void reRegister();
+
protected:
Status onClients(const sp<IBinder>& service, bool clients) override;
@@ -53,6 +60,7 @@
// whether, based on onClients calls, we know we have a client for this
// service or not
bool clients = false;
+ bool registered = true;
};
/**
@@ -66,13 +74,27 @@
*/
void tryShutdown();
+ /**
+ * Try to shutdown the process, unless:
+ * - 'forcePersist' is 'true', or
+ * - The active services count callback returns 'true', or
+ * - Some services have clients.
+ */
+ void maybeTryShutdown();
+
// count of services with clients
size_t mNumConnectedServices;
+ // previous value passed to the active services callback
+ std::optional<bool> mPreviousHasClients;
+
// map of registered names and services
std::map<std::string, Service> mRegisteredServices;
bool mForcePersist;
+
+ // Callback used to report if there are services with clients
+ std::function<bool(bool)> mActiveServicesCallback;
};
bool ClientCounterCallback::registerService(const sp<IBinder>& service, const std::string& name,
@@ -119,8 +141,64 @@
void ClientCounterCallback::forcePersist(bool persist) {
mForcePersist = persist;
- if(!mForcePersist) {
+ if (!mForcePersist) {
// Attempt a shutdown in case the number of clients hit 0 while the flag was on
+ maybeTryShutdown();
+ }
+}
+
+bool ClientCounterCallback::tryUnregister() {
+ auto manager = interface_cast<AidlServiceManager>(asBinder(defaultServiceManager()));
+
+ for (auto& [name, entry] : mRegisteredServices) {
+ bool success = manager->tryUnregisterService(name, entry.service).isOk();
+
+ if (!success) {
+ ALOGI("Failed to unregister service %s", name.c_str());
+ return false;
+ }
+ entry.registered = false;
+ }
+
+ return true;
+}
+
+void ClientCounterCallback::reRegister() {
+ for (auto& [name, entry] : mRegisteredServices) {
+ // re-register entry if not already registered
+ if (entry.registered) {
+ continue;
+ }
+
+ if (!registerService(entry.service, name, entry.allowIsolated,
+ entry.dumpFlags)) {
+ // Must restart. Otherwise, clients will never be able to get a hold of this service.
+ LOG_ALWAYS_FATAL("Bad state: could not re-register services");
+ }
+
+ entry.registered = true;
+ }
+}
+
+void ClientCounterCallback::maybeTryShutdown() {
+ if (mForcePersist) {
+ ALOGI("Shutdown prevented by forcePersist override flag.");
+ return;
+ }
+
+ bool handledInCallback = false;
+ if (mActiveServicesCallback != nullptr) {
+ bool hasClients = mNumConnectedServices != 0;
+ if (hasClients != mPreviousHasClients) {
+ handledInCallback = mActiveServicesCallback(hasClients);
+ mPreviousHasClients = hasClients;
+ }
+ }
+
+ // If there is no callback defined or the callback did not handle this
+ // client count change event, try to shutdown the process if its services
+ // have no clients.
+ if (!handledInCallback && mNumConnectedServices == 0) {
tryShutdown();
}
}
@@ -150,54 +228,25 @@
ALOGI("Process has %zu (of %zu available) client(s) in use after notification %s has clients: %d",
mNumConnectedServices, mRegisteredServices.size(), name.c_str(), clients);
- tryShutdown();
+ maybeTryShutdown();
+
return Status::ok();
}
void ClientCounterCallback::tryShutdown() {
- if(mNumConnectedServices > 0) {
- // Should only shut down if there are no clients
- return;
- }
-
- if(mForcePersist) {
- ALOGI("Shutdown prevented by forcePersist override flag.");
- return;
- }
-
ALOGI("Trying to shut down the service. No clients in use for any service in process.");
- auto manager = interface_cast<AidlServiceManager>(asBinder(defaultServiceManager()));
-
- auto unRegisterIt = mRegisteredServices.begin();
- for (; unRegisterIt != mRegisteredServices.end(); ++unRegisterIt) {
- auto& entry = (*unRegisterIt);
-
- bool success = manager->tryUnregisterService(entry.first, entry.second.service).isOk();
-
-
- if (!success) {
- ALOGI("Failed to unregister service %s", entry.first.c_str());
- break;
- }
- }
-
- if (unRegisterIt == mRegisteredServices.end()) {
+ if (tryUnregister()) {
ALOGI("Unregistered all clients and exiting");
exit(EXIT_SUCCESS);
}
- for (auto reRegisterIt = mRegisteredServices.begin(); reRegisterIt != unRegisterIt;
- reRegisterIt++) {
- auto& entry = (*reRegisterIt);
+ reRegister();
+}
- // re-register entry
- if (!registerService(entry.second.service, entry.first, entry.second.allowIsolated,
- entry.second.dumpFlags)) {
- // Must restart. Otherwise, clients will never be able to get a hold of this service.
- ALOGE("Bad state: could not re-register services");
- }
- }
+void ClientCounterCallback::setActiveServicesCallback(const std::function<bool(bool)>&
+ activeServicesCallback) {
+ mActiveServicesCallback = activeServicesCallback;
}
} // namespace internal
@@ -223,5 +272,18 @@
mClientCC->forcePersist(persist);
}
+void LazyServiceRegistrar::setActiveServicesCallback(const std::function<bool(bool)>&
+ activeServicesCallback) {
+ mClientCC->setActiveServicesCallback(activeServicesCallback);
+}
+
+bool LazyServiceRegistrar::tryUnregister() {
+ return mClientCC->tryUnregister();
+}
+
+void LazyServiceRegistrar::reRegister() {
+ mClientCC->reRegister();
+}
+
} // namespace hardware
} // namespace android
diff --git a/libs/binder/Parcel.cpp b/libs/binder/Parcel.cpp
index 9bba369..8087443 100644
--- a/libs/binder/Parcel.cpp
+++ b/libs/binder/Parcel.cpp
@@ -1477,6 +1477,31 @@
goto data_sorted;
}
+status_t Parcel::readVectorSizeWithCoarseBoundCheck(int32_t *size) const {
+ int32_t requestedSize;
+ const status_t status = readInt32(&requestedSize);
+ if (status != NO_ERROR) return status;
+
+ // We permit negative sizes, which indicate presence of a nullable vector,
+ // i.e. a vector embedded in std::optional, std::unique_ptr, or std::shared_ptr.
+ if (requestedSize > 0) {
+ // Check if there are fewer bytes than vector elements.
+ // A lower bound is 1 byte per element, satisfied by some enum and int8_t and uint8_t.
+ const size_t availableBytes = dataAvail();
+ if (static_cast<size_t>(requestedSize) > availableBytes) {
+ // We have a size that is greater than the number of bytes available.
+ // On bounds failure we do not 'rewind' position by 4 bytes of the size already read.
+ ALOGW("%s: rejecting out of bounds vector size (requestedSize):%d "
+ "Parcel{dataAvail:%zu mDataSize:%zu mDataPos:%zu mDataCapacity:%zu}",
+ __func__, requestedSize, availableBytes, mDataSize, mDataPos, mDataCapacity);
+ return BAD_VALUE;
+ }
+ }
+
+ *size = requestedSize;
+ return NO_ERROR;
+}
+
status_t Parcel::read(void* outData, size_t len) const
{
if (len > INT32_MAX) {
@@ -1699,7 +1724,7 @@
status_t Parcel::readBoolVector(std::optional<std::vector<bool>>* val) const {
const int32_t start = dataPosition();
int32_t size;
- status_t status = readInt32(&size);
+ status_t status = readVectorSizeWithCoarseBoundCheck(&size);
val->reset();
if (status != OK || size < 0) {
@@ -1721,7 +1746,7 @@
status_t Parcel::readBoolVector(std::unique_ptr<std::vector<bool>>* val) const {
const int32_t start = dataPosition();
int32_t size;
- status_t status = readInt32(&size);
+ status_t status = readVectorSizeWithCoarseBoundCheck(&size);
val->reset();
if (status != OK || size < 0) {
@@ -1742,7 +1767,7 @@
status_t Parcel::readBoolVector(std::vector<bool>* val) const {
int32_t size;
- status_t status = readInt32(&size);
+ status_t status = readVectorSizeWithCoarseBoundCheck(&size);
if (status != OK) {
return status;
diff --git a/libs/binder/ParcelableHolder.cpp b/libs/binder/ParcelableHolder.cpp
index b2b8671..2e86b74 100644
--- a/libs/binder/ParcelableHolder.cpp
+++ b/libs/binder/ParcelableHolder.cpp
@@ -37,7 +37,7 @@
size_t sizePos = p->dataPosition();
RETURN_ON_FAILURE(p->writeInt32(0));
size_t dataStartPos = p->dataPosition();
- RETURN_ON_FAILURE(p->writeUtf8AsUtf16(this->mParcelableName));
+ RETURN_ON_FAILURE(p->writeString16(this->mParcelableName));
this->mParcelable->writeToParcel(p);
size_t dataSize = p->dataPosition() - dataStartPos;
diff --git a/libs/binder/ProcessState.cpp b/libs/binder/ProcessState.cpp
index b5e4dfe..c38249e 100644
--- a/libs/binder/ProcessState.cpp
+++ b/libs/binder/ProcessState.cpp
@@ -399,6 +399,7 @@
, mThreadCountLock(PTHREAD_MUTEX_INITIALIZER)
, mThreadCountDecrement(PTHREAD_COND_INITIALIZER)
, mExecutingThreadsCount(0)
+ , mWaitingForThreads(0)
, mMaxThreads(DEFAULT_MAX_BINDER_THREADS)
, mStarvationStartTimeMs(0)
, mThreadPoolStarted(false)
diff --git a/libs/binder/TEST_MAPPING b/libs/binder/TEST_MAPPING
index 97e282e..2e90142 100644
--- a/libs/binder/TEST_MAPPING
+++ b/libs/binder/TEST_MAPPING
@@ -10,6 +10,9 @@
"name": "binderAllocationLimits"
},
{
+ "name": "binderClearBufTest"
+ },
+ {
"name": "binderDriverInterfaceTest"
},
{
@@ -62,6 +65,9 @@
},
{
"name": "binderRustNdkInteropTest"
+ },
+ {
+ "name": "rustBinderSerializationTest"
}
]
}
diff --git a/libs/binder/include/binder/IPCThreadState.h b/libs/binder/include/binder/IPCThreadState.h
index 4da8aa1..0183324 100644
--- a/libs/binder/include/binder/IPCThreadState.h
+++ b/libs/binder/include/binder/IPCThreadState.h
@@ -110,6 +110,7 @@
status_t setupPolling(int* fd);
status_t handlePolledCommands();
void flushCommands();
+ bool flushIfNeeded();
void joinThreadPool(bool isMain = true);
@@ -204,6 +205,7 @@
int32_t mWorkSource;
// Whether the work source should be propagated.
bool mPropagateWorkSource;
+ bool mIsLooper;
int32_t mStrictModePolicy;
int32_t mLastTransactionBinderFlags;
CallRestriction mCallRestriction;
diff --git a/libs/binder/include/binder/LazyServiceRegistrar.h b/libs/binder/include/binder/LazyServiceRegistrar.h
index d18c88e..9659732 100644
--- a/libs/binder/include/binder/LazyServiceRegistrar.h
+++ b/libs/binder/include/binder/LazyServiceRegistrar.h
@@ -16,6 +16,8 @@
#pragma once
+#include <functional>
+
#include <binder/IServiceManager.h>
#include <binder/Status.h>
#include <utils/StrongPointer.h>
@@ -53,6 +55,39 @@
*/
void forcePersist(bool persist);
+ /**
+ * Set a callback that is invoked when the active service count (i.e. services with clients)
+ * registered with this process drops to zero (or becomes nonzero).
+ * The callback takes a boolean argument, which is 'true' if there is
+ * at least one service with clients.
+ *
+ * Callback return value:
+ * - false: Default behavior for lazy services (shut down the process if there
+ * are no clients).
+ * - true: Don't shut down the process even if there are no clients.
+ *
+ * This callback gives a chance to:
+ * 1 - Perform some additional operations before exiting;
+ * 2 - Prevent the process from exiting by returning "true" from the
+ * callback.
+ *
+ * This method should be called before 'registerService' to avoid races.
+ */
+ void setActiveServicesCallback(const std::function<bool(bool)>& activeServicesCallback);
+
+ /**
+ * Try to unregister all services previously registered with 'registerService'.
+ * Returns 'true' if successful.
+ */
+ bool tryUnregister();
+
+ /**
+ * Re-register services that were unregistered by 'tryUnregister'.
+ * This method should be called in the case 'tryUnregister' fails
+ * (and should be called on the same thread).
+ */
+ void reRegister();
+
private:
std::shared_ptr<internal::ClientCounterCallback> mClientCC;
LazyServiceRegistrar();
diff --git a/libs/binder/include/binder/Parcel.h b/libs/binder/include/binder/Parcel.h
index b49951b..54c49e4 100644
--- a/libs/binder/include/binder/Parcel.h
+++ b/libs/binder/include/binder/Parcel.h
@@ -517,6 +517,11 @@
void initState();
void scanForFds() const;
status_t validateReadData(size_t len) const;
+
+ // Reads an int32 size and does a coarse bounds check against the number
+ // of available bytes in the Parcel.
+ status_t readVectorSizeWithCoarseBoundCheck(int32_t *size) const;
+
void updateWorkSourceRequestHeaderPosition() const;
status_t finishFlattenBinder(const sp<IBinder>& binder);
@@ -787,6 +792,7 @@
template<typename T>
status_t Parcel::resizeOutVector(std::vector<T>* val) const {
int32_t size;
+ // used for allocating 'out' vector args, do not use readVectorSizeWithCoarseBoundCheck() here
status_t err = readInt32(&size);
if (err != NO_ERROR) {
return err;
@@ -802,6 +808,7 @@
template<typename T>
status_t Parcel::resizeOutVector(std::optional<std::vector<T>>* val) const {
int32_t size;
+ // used for allocating 'out' vector args, do not use readVectorSizeWithCoarseBoundCheck() here
status_t err = readInt32(&size);
if (err != NO_ERROR) {
return err;
@@ -818,6 +825,7 @@
template<typename T>
status_t Parcel::resizeOutVector(std::unique_ptr<std::vector<T>>* val) const {
int32_t size;
+ // used for allocating 'out' vector args, do not use readVectorSizeWithCoarseBoundCheck() here
status_t err = readInt32(&size);
if (err != NO_ERROR) {
return err;
@@ -834,7 +842,7 @@
template<typename T>
status_t Parcel::reserveOutVector(std::vector<T>* val, size_t* size) const {
int32_t read_size;
- status_t err = readInt32(&read_size);
+ status_t err = readVectorSizeWithCoarseBoundCheck(&read_size);
if (err != NO_ERROR) {
return err;
}
@@ -850,7 +858,7 @@
template<typename T>
status_t Parcel::reserveOutVector(std::optional<std::vector<T>>* val, size_t* size) const {
int32_t read_size;
- status_t err = readInt32(&read_size);
+ status_t err = readVectorSizeWithCoarseBoundCheck(&read_size);
if (err != NO_ERROR) {
return err;
}
@@ -870,7 +878,7 @@
status_t Parcel::reserveOutVector(std::unique_ptr<std::vector<T>>* val,
size_t* size) const {
int32_t read_size;
- status_t err = readInt32(&read_size);
+ status_t err = readVectorSizeWithCoarseBoundCheck(&read_size);
if (err != NO_ERROR) {
return err;
}
@@ -923,7 +931,7 @@
std::vector<T>* val,
status_t(Parcel::*read_func)(U*) const) const {
int32_t size;
- status_t status = this->readInt32(&size);
+ status_t status = this->readVectorSizeWithCoarseBoundCheck(&size);
if (status != OK) {
return status;
@@ -965,7 +973,7 @@
status_t(Parcel::*read_func)(T*) const) const {
const size_t start = dataPosition();
int32_t size;
- status_t status = readInt32(&size);
+ status_t status = readVectorSizeWithCoarseBoundCheck(&size);
val->reset();
if (status != OK || size < 0) {
@@ -989,7 +997,7 @@
status_t(Parcel::*read_func)(T*) const) const {
const size_t start = dataPosition();
int32_t size;
- status_t status = readInt32(&size);
+ status_t status = readVectorSizeWithCoarseBoundCheck(&size);
val->reset();
if (status != OK || size < 0) {
@@ -1093,7 +1101,7 @@
status_t Parcel::readParcelableVector(std::optional<std::vector<std::optional<T>>>* val) const {
const size_t start = dataPosition();
int32_t size;
- status_t status = readInt32(&size);
+ status_t status = readVectorSizeWithCoarseBoundCheck(&size);
val->reset();
if (status != OK || size < 0) {
@@ -1117,7 +1125,7 @@
status_t Parcel::readParcelableVector(std::unique_ptr<std::vector<std::unique_ptr<T>>>* val) const {
const size_t start = dataPosition();
int32_t size;
- status_t status = readInt32(&size);
+ status_t status = readVectorSizeWithCoarseBoundCheck(&size);
val->reset();
if (status != OK || size < 0) {
diff --git a/libs/binder/include/binder/ParcelableHolder.h b/libs/binder/include/binder/ParcelableHolder.h
index ff0a686..9e4475c 100644
--- a/libs/binder/include/binder/ParcelableHolder.h
+++ b/libs/binder/include/binder/ParcelableHolder.h
@@ -18,6 +18,7 @@
#include <binder/Parcel.h>
#include <binder/Parcelable.h>
+#include <utils/String16.h>
#include <mutex>
#include <optional>
#include <tuple>
@@ -72,7 +73,7 @@
template <typename T>
status_t getParcelable(std::shared_ptr<T>* ret) const {
static_assert(std::is_base_of<Parcelable, T>::value, "T must be derived from Parcelable");
- const std::string& parcelableDesc = T::getParcelableDescriptor();
+ const String16& parcelableDesc = T::getParcelableDescriptor();
if (!this->mParcelPtr) {
if (!this->mParcelable || !this->mParcelableName) {
ALOGD("empty ParcelableHolder");
@@ -80,7 +81,7 @@
return android::OK;
} else if (parcelableDesc != *mParcelableName) {
ALOGD("extension class name mismatch expected:%s actual:%s",
- mParcelableName->c_str(), parcelableDesc.c_str());
+ String8(*mParcelableName).c_str(), String8(parcelableDesc).c_str());
*ret = nullptr;
return android::BAD_VALUE;
}
@@ -88,7 +89,7 @@
return android::OK;
}
this->mParcelPtr->setDataPosition(0);
- status_t status = this->mParcelPtr->readUtf8FromUtf16(&this->mParcelableName);
+ status_t status = this->mParcelPtr->readString16(&this->mParcelableName);
if (status != android::OK || parcelableDesc != this->mParcelableName) {
this->mParcelableName = std::nullopt;
*ret = nullptr;
@@ -130,7 +131,7 @@
private:
mutable std::shared_ptr<Parcelable> mParcelable;
- mutable std::optional<std::string> mParcelableName;
+ mutable std::optional<String16> mParcelableName;
mutable std::unique_ptr<Parcel> mParcelPtr;
Stability mStability;
};
diff --git a/libs/binder/include/binder/ProcessState.h b/libs/binder/include/binder/ProcessState.h
index bab6469..2405ab6 100644
--- a/libs/binder/include/binder/ProcessState.h
+++ b/libs/binder/include/binder/ProcessState.h
@@ -107,11 +107,14 @@
int mDriverFD;
void* mVMStart;
- // Protects thread count variable below.
+ // Protects thread count and wait variables below.
pthread_mutex_t mThreadCountLock;
+ // Broadcast whenever mWaitingForThreads > 0
pthread_cond_t mThreadCountDecrement;
// Number of binder threads current executing a command.
size_t mExecutingThreadsCount;
+ // Number of threads calling IPCThreadState::blockUntilThreadAvailable()
+ size_t mWaitingForThreads;
// Maximum number for binder threads allowed for this process.
size_t mMaxThreads;
// Time when thread pool was emptied
diff --git a/libs/binder/ndk/Android.bp b/libs/binder/ndk/Android.bp
index bdb74dc..82f3882 100644
--- a/libs/binder/ndk/Android.bp
+++ b/libs/binder/ndk/Android.bp
@@ -104,15 +104,28 @@
"--header-filter=^.*frameworks/native/libs/binder/.*.h$",
],
tidy_checks_as_errors: [
- "*",
+ // Explicitly list the checks that should not occur in this module.
+ "abseil-*",
+ "android-*",
+ "bugprone-*",
+ "cert-*",
+ "clang-analyzer-*",
"-clang-analyzer-core.CallAndMessage",
"-clang-analyzer-core.uninitialized.Assign",
- "-clang-analyzer-unix.Malloc,",
+ "-clang-analyzer-unix.Malloc",
"-clang-analyzer-deadcode.DeadStores",
"-clang-analyzer-optin.cplusplus.UninitializedObject",
+ "google-*",
+ "-google-readability-*",
+ "-google-runtime-references",
+ "misc-*",
"-misc-no-recursion",
+ "-misc-non-private-member-variables-in-classes",
"-misc-redundant-expression",
+ "-misc-unused-parameters",
"-misc-unused-using-decls",
+ "performance*",
+ "portability*",
],
}
diff --git a/libs/binder/ndk/ibinder.cpp b/libs/binder/ndk/ibinder.cpp
index 350c658..454fbd0 100644
--- a/libs/binder/ndk/ibinder.cpp
+++ b/libs/binder/ndk/ibinder.cpp
@@ -301,6 +301,26 @@
return binder.get();
}
+AIBinder_Weak* AIBinder_Weak_clone(const AIBinder_Weak* weak) {
+ if (weak == nullptr) {
+ return nullptr;
+ }
+
+ return new AIBinder_Weak{weak->binder};
+}
+
+bool AIBinder_lt(const AIBinder* lhs, const AIBinder* rhs) {
+ if (lhs == nullptr || rhs == nullptr) return lhs < rhs;
+
+ return const_cast<AIBinder*>(lhs)->getBinder() < const_cast<AIBinder*>(rhs)->getBinder();
+}
+
+bool AIBinder_Weak_lt(const AIBinder_Weak* lhs, const AIBinder_Weak* rhs) {
+ if (lhs == nullptr || rhs == nullptr) return lhs < rhs;
+
+ return lhs->binder < rhs->binder;
+}
+
AIBinder_Class::AIBinder_Class(const char* interfaceDescriptor, AIBinder_Class_onCreate onCreate,
AIBinder_Class_onDestroy onDestroy,
AIBinder_Class_onTransact onTransact)
diff --git a/libs/binder/ndk/include_cpp/android/binder_auto_utils.h b/libs/binder/ndk/include_cpp/android/binder_auto_utils.h
index 53871f2..0ad400b 100644
--- a/libs/binder/ndk/include_cpp/android/binder_auto_utils.h
+++ b/libs/binder/ndk/include_cpp/android/binder_auto_utils.h
@@ -259,10 +259,24 @@
const char* getMessage() const { return AStatus_getMessage(get()); }
std::string getDescription() const {
- const char* cStr = AStatus_getDescription(get());
- std::string ret = cStr;
- AStatus_deleteDescription(cStr);
- return ret;
+ if (__builtin_available(android 30, *)) {
+ const char* cStr = AStatus_getDescription(get());
+ std::string ret = cStr;
+ AStatus_deleteDescription(cStr);
+ return ret;
+ }
+ binder_exception_t exception = getExceptionCode();
+ std::string desc = std::to_string(exception);
+ if (exception == EX_SERVICE_SPECIFIC) {
+ desc += " (" + std::to_string(getServiceSpecificError()) + ")";
+ } else if (exception == EX_TRANSACTION_FAILED) {
+ desc += " (" + std::to_string(getStatus()) + ")";
+ }
+ if (const char* msg = getMessage(); msg != nullptr) {
+ desc += ": ";
+ desc += msg;
+ }
+ return desc;
}
/**
diff --git a/libs/binder/ndk/include_cpp/android/binder_interface_utils.h b/libs/binder/ndk/include_cpp/android/binder_interface_utils.h
index a4f4441..05eb64b 100644
--- a/libs/binder/ndk/include_cpp/android/binder_interface_utils.h
+++ b/libs/binder/ndk/include_cpp/android/binder_interface_utils.h
@@ -83,7 +83,8 @@
template <class T, class... Args>
static std::shared_ptr<T> make(Args&&... args) {
T* t = new T(std::forward<Args>(args)...);
- return t->template ref<T>();
+ // warning: Potential leak of memory pointed to by 't' [clang-analyzer-unix.Malloc]
+ return t->template ref<T>(); // NOLINT(clang-analyzer-unix.Malloc)
}
static void operator delete(void* p) { std::free(p); }
@@ -246,7 +247,7 @@
// ourselves. The defaults are harmless.
AIBinder_Class_setOnDump(clazz, ICInterfaceData::onDump);
#ifdef HAS_BINDER_SHELL_COMMAND
- if (AIBinder_Class_setHandleShellCommand != nullptr) {
+ if (__builtin_available(android 30, *)) {
AIBinder_Class_setHandleShellCommand(clazz, ICInterfaceData::handleShellCommand);
}
#endif
diff --git a/libs/binder/ndk/include_cpp/android/binder_parcelable_utils.h b/libs/binder/ndk/include_cpp/android/binder_parcelable_utils.h
index 6636a41..2277148 100644
--- a/libs/binder/ndk/include_cpp/android/binder_parcelable_utils.h
+++ b/libs/binder/ndk/include_cpp/android/binder_parcelable_utils.h
@@ -51,14 +51,27 @@
binder_status_t writeToParcel(AParcel* parcel) const {
RETURN_ON_FAILURE(AParcel_writeInt32(parcel, static_cast<int32_t>(this->mStability)));
- RETURN_ON_FAILURE(AParcel_writeInt32(parcel, AParcel_getDataSize(this->mParcel.get())));
- RETURN_ON_FAILURE(AParcel_appendFrom(this->mParcel.get(), parcel, 0,
- AParcel_getDataSize(this->mParcel.get())));
+ if (__builtin_available(android 31, *)) {
+ int32_t size = AParcel_getDataSize(this->mParcel.get());
+ RETURN_ON_FAILURE(AParcel_writeInt32(parcel, size));
+ } else {
+ return STATUS_INVALID_OPERATION;
+ }
+ if (__builtin_available(android 31, *)) {
+ int32_t size = AParcel_getDataSize(this->mParcel.get());
+ RETURN_ON_FAILURE(AParcel_appendFrom(this->mParcel.get(), parcel, 0, size));
+ } else {
+ return STATUS_INVALID_OPERATION;
+ }
return STATUS_OK;
}
binder_status_t readFromParcel(const AParcel* parcel) {
- AParcel_reset(mParcel.get());
+ if (__builtin_available(android 31, *)) {
+ AParcel_reset(mParcel.get());
+ } else {
+ return STATUS_INVALID_OPERATION;
+ }
RETURN_ON_FAILURE(AParcel_readInt32(parcel, &this->mStability));
int32_t dataSize;
@@ -74,7 +87,11 @@
return STATUS_BAD_VALUE;
}
- status = AParcel_appendFrom(parcel, mParcel.get(), dataStartPos, dataSize);
+ if (__builtin_available(android 31, *)) {
+ status = AParcel_appendFrom(parcel, mParcel.get(), dataStartPos, dataSize);
+ } else {
+ status = STATUS_INVALID_OPERATION;
+ }
if (status != STATUS_OK) {
return status;
}
@@ -86,7 +103,11 @@
if (this->mStability > T::_aidl_stability) {
return STATUS_BAD_VALUE;
}
- AParcel_reset(mParcel.get());
+ if (__builtin_available(android 31, *)) {
+ AParcel_reset(mParcel.get());
+ } else {
+ return STATUS_INVALID_OPERATION;
+ }
AParcel_writeString(mParcel.get(), T::descriptor, strlen(T::descriptor));
p.writeToParcel(mParcel.get());
return STATUS_OK;
@@ -96,9 +117,13 @@
binder_status_t getParcelable(std::optional<T>* ret) const {
const std::string parcelableDesc(T::descriptor);
AParcel_setDataPosition(mParcel.get(), 0);
- if (AParcel_getDataSize(mParcel.get()) == 0) {
- *ret = std::nullopt;
- return STATUS_OK;
+ if (__builtin_available(android 31, *)) {
+ if (AParcel_getDataSize(mParcel.get()) == 0) {
+ *ret = std::nullopt;
+ return STATUS_OK;
+ }
+ } else {
+ return STATUS_INVALID_OPERATION;
}
std::string parcelableDescInParcel;
binder_status_t status = AParcel_readString(mParcel.get(), &parcelableDescInParcel);
@@ -115,7 +140,11 @@
return STATUS_OK;
}
- void reset() { AParcel_reset(mParcel.get()); }
+ void reset() {
+ if (__builtin_available(android 31, *)) {
+ AParcel_reset(mParcel.get());
+ }
+ }
inline bool operator!=(const AParcelableHolder& rhs) const { return this != &rhs; }
inline bool operator<(const AParcelableHolder& rhs) const { return this < &rhs; }
diff --git a/libs/binder/ndk/include_ndk/android/binder_ibinder.h b/libs/binder/ndk/include_ndk/android/binder_ibinder.h
index 5e1ed46..bb70588 100644
--- a/libs/binder/ndk/include_ndk/android/binder_ibinder.h
+++ b/libs/binder/ndk/include_ndk/android/binder_ibinder.h
@@ -657,6 +657,77 @@
*/
const char* AIBinder_Class_getDescriptor(const AIBinder_Class* clazz) __INTRODUCED_IN(31);
+/**
+ * Whether AIBinder is less than another.
+ *
+ * This provides a per-process-unique total ordering of binders where a null
+ * AIBinder* object is considered to be before all other binder objects.
+ * For instance, two binders refer to the same object in a local or remote
+ * process when both AIBinder_lt(a, b) and AIBinder(b, a) are false. This API
+ * might be used to insert and lookup binders in binary search trees.
+ *
+ * AIBinder* pointers themselves actually also create a per-process-unique total
+ * ordering. However, this ordering is inconsistent with AIBinder_Weak_lt for
+ * remote binders. So, in general, this function should be preferred.
+ *
+ * Available since API level 31.
+ *
+ * \param lhs comparison object
+ * \param rhs comparison object
+ *
+ * \return whether "lhs < rhs" is true
+ */
+bool AIBinder_lt(const AIBinder* lhs, const AIBinder* rhs);
+
+/**
+ * Clone an AIBinder_Weak. Useful because even if a weak binder promotes to a
+ * null value, after further binder transactions, it may no longer promote to a
+ * null value.
+ *
+ * Available since API level 31.
+ *
+ * \param weak Object to clone
+ *
+ * \return clone of the input parameter. This must be deleted with
+ * AIBinder_Weak_delete. Null if weak input parameter is also null.
+ */
+AIBinder_Weak* AIBinder_Weak_clone(const AIBinder_Weak* weak);
+
+/**
+ * Whether AIBinder_Weak is less than another.
+ *
+ * This provides a per-process-unique total ordering of binders which is exactly
+ * the same as AIBinder_lt. Similarly, a null AIBinder_Weak* is considered to be
+ * ordered before all other weak references.
+ *
+ * This function correctly distinguishes binders even if one is deallocated. So,
+ * for instance, an AIBinder_Weak* entry representing a deleted binder will
+ * never compare as equal to an AIBinder_Weak* entry which represents a
+ * different allocation of a binder, even if the two binders were originally
+ * allocated at the same address. That is:
+ *
+ * AIBinder* a = ...; // imagine this has address 0x8
+ * AIBinder_Weak* bWeak = AIBinder_Weak_new(a);
+ * AIBinder_decStrong(a); // a may be deleted, if this is the last reference
+ * AIBinder* b = ...; // imagine this has address 0x8 (same address as b)
+ * AIBinder_Weak* bWeak = AIBinder_Weak_new(b);
+ *
+ * Then when a/b are compared with other binders, their order will be preserved,
+ * and it will either be the case that AIBinder_Weak_lt(aWeak, bWeak) OR
+ * AIBinder_Weak_lt(bWeak, aWeak), but not both.
+ *
+ * Unlike AIBinder*, the AIBinder_Weak* addresses themselves have nothing to do
+ * with the underlying binder.
+ *
+ * Available since API level 31.
+ *
+ * \param lhs comparison object
+ * \param rhs comparison object
+ *
+ * \return whether "lhs < rhs" is true
+ */
+bool AIBinder_Weak_lt(const AIBinder_Weak* lhs, const AIBinder_Weak* rhs);
+
#endif //__ANDROID_API__ >= 31
__END_DECLS
diff --git a/libs/binder/ndk/include_ndk/android/binder_parcel.h b/libs/binder/ndk/include_ndk/android/binder_parcel.h
index 93c3f32..ab67017 100644
--- a/libs/binder/ndk/include_ndk/android/binder_parcel.h
+++ b/libs/binder/ndk/include_ndk/android/binder_parcel.h
@@ -1120,7 +1120,6 @@
// @END-PRIMITIVE-READ-WRITE
#endif //__ANDROID_API__ >= 29
-#if __ANDROID_API__ >= 31
/**
* Reset the parcel to the initial status.
*
@@ -1166,7 +1165,6 @@
* \return A parcel which is not related to any IBinder objects.
*/
AParcel* AParcel_create() __INTRODUCED_IN(31);
-#endif //__ANDROID_API__ >= 31
__END_DECLS
/** @} */
diff --git a/libs/binder/ndk/libbinder_ndk.map.txt b/libs/binder/ndk/libbinder_ndk.map.txt
index e233ffd..9a93bf3 100644
--- a/libs/binder/ndk/libbinder_ndk.map.txt
+++ b/libs/binder/ndk/libbinder_ndk.map.txt
@@ -122,6 +122,9 @@
AServiceManager_waitForService; # apex llndk
AIBinder_Class_getDescriptor;
+ AIBinder_Weak_clone;
+ AIBinder_Weak_lt;
+ AIBinder_lt;
AParcel_appendFrom;
AParcel_create;
AParcel_getDataSize;
diff --git a/libs/binder/ndk/tests/libbinder_ndk_unit_test.cpp b/libs/binder/ndk/tests/libbinder_ndk_unit_test.cpp
index b7df115..0d1989e 100644
--- a/libs/binder/ndk/tests/libbinder_ndk_unit_test.cpp
+++ b/libs/binder/ndk/tests/libbinder_ndk_unit_test.cpp
@@ -375,8 +375,7 @@
AIBinder_decStrong(binder);
- // assert because would need to decStrong if non-null and we shouldn't need to add a no-op here
- ASSERT_NE(nullptr, AIBinder_Weak_promote(wBinder));
+ ASSERT_EQ(nullptr, AIBinder_Weak_promote(wBinder));
AIBinder_Weak_delete(wBinder);
}
diff --git a/libs/binder/rust/src/binder.rs b/libs/binder/rust/src/binder.rs
index ed3b9ec..42c1e0a 100644
--- a/libs/binder/rust/src/binder.rs
+++ b/libs/binder/rust/src/binder.rs
@@ -613,7 +613,7 @@
impl $crate::parcel::Serialize for dyn $interface + '_
where
- $interface: $crate::Interface
+ dyn $interface: $crate::Interface
{
fn serialize(&self, parcel: &mut $crate::parcel::Parcel) -> $crate::Result<()> {
let binder = $crate::Interface::as_binder(self);
diff --git a/libs/binder/rust/src/parcel/parcelable.rs b/libs/binder/rust/src/parcel/parcelable.rs
index 138b360..8d18fb4 100644
--- a/libs/binder/rust/src/parcel/parcelable.rs
+++ b/libs/binder/rust/src/parcel/parcelable.rs
@@ -15,15 +15,17 @@
*/
use crate::binder::{AsNative, FromIBinder};
-use crate::error::{status_result, Result, Status, StatusCode};
+use crate::error::{status_result, status_t, Result, Status, StatusCode};
use crate::parcel::Parcel;
use crate::proxy::SpIBinder;
use crate::sys;
use std::convert::TryInto;
use std::ffi::c_void;
-use std::os::raw::c_char;
+use std::os::raw::{c_char, c_ulong};
+use std::mem::{self, MaybeUninit};
use std::ptr;
+use std::slice;
/// A struct whose instances can be written to a [`Parcel`].
// Might be able to hook this up as a serde backend in the future?
@@ -49,38 +51,109 @@
pub trait SerializeArray: Serialize + Sized {
/// Serialize an array of this type into the given [`Parcel`].
fn serialize_array(slice: &[Self], parcel: &mut Parcel) -> Result<()> {
- parcel.write_slice_size(Some(slice))?;
-
- for item in slice {
- parcel.write(item)?;
- }
-
- Ok(())
+ let res = unsafe {
+ // Safety: Safe FFI, slice will always be a safe pointer to pass.
+ sys::AParcel_writeParcelableArray(
+ parcel.as_native_mut(),
+ slice.as_ptr() as *const c_void,
+ slice.len().try_into().or(Err(StatusCode::BAD_VALUE))?,
+ Some(serialize_element::<Self>),
+ )
+ };
+ status_result(res)
}
}
+/// Callback to serialize an element of a generic parcelable array.
+///
+/// Safety: We are relying on binder_ndk to not overrun our slice. As long as it
+/// doesn't provide an index larger than the length of the original slice in
+/// serialize_array, this operation is safe. The index provided is zero-based.
+unsafe extern "C" fn serialize_element<T: Serialize>(
+ parcel: *mut sys::AParcel,
+ array: *const c_void,
+ index: c_ulong,
+) -> status_t {
+ // c_ulong and usize are the same, but we need the explicitly sized version
+ // so the function signature matches what bindgen generates.
+ let index = index as usize;
+
+ let slice: &[T] = slice::from_raw_parts(array.cast(), index+1);
+
+ let mut parcel = match Parcel::borrowed(parcel) {
+ None => return StatusCode::UNEXPECTED_NULL as status_t,
+ Some(p) => p,
+ };
+
+ slice[index].serialize(&mut parcel)
+ .err()
+ .unwrap_or(StatusCode::OK)
+ as status_t
+}
+
/// Helper trait for types that can be deserialized as arrays.
/// Defaults to calling Deserialize::deserialize() manually for every element,
/// but can be overridden for custom implementations like `readByteArray`.
pub trait DeserializeArray: Deserialize {
/// Deserialize an array of type from the given [`Parcel`].
fn deserialize_array(parcel: &Parcel) -> Result<Option<Vec<Self>>> {
- let len: i32 = parcel.read()?;
- if len < 0 {
- return Ok(None);
- }
-
- // TODO: Assumes that usize is at least 32 bits
- let mut vec = Vec::with_capacity(len as usize);
-
- for _ in 0..len {
- vec.push(parcel.read()?);
- }
-
- Ok(Some(vec))
+ let mut vec: Option<Vec<MaybeUninit<Self>>> = None;
+ let res = unsafe {
+ // Safety: Safe FFI, vec is the correct opaque type expected by
+ // allocate_vec and deserialize_element.
+ sys::AParcel_readParcelableArray(
+ parcel.as_native(),
+ &mut vec as *mut _ as *mut c_void,
+ Some(allocate_vec::<Self>),
+ Some(deserialize_element::<Self>),
+ )
+ };
+ status_result(res)?;
+ let vec: Option<Vec<Self>> = unsafe {
+ // Safety: We are assuming that the NDK correctly initialized every
+ // element of the vector by now, so we know that all the
+ // MaybeUninits are now properly initialized. We can transmute from
+ // Vec<MaybeUninit<T>> to Vec<T> because MaybeUninit<T> has the same
+ // alignment and size as T, so the pointer to the vector allocation
+ // will be compatible.
+ mem::transmute(vec)
+ };
+ Ok(vec)
}
}
+/// Callback to deserialize a parcelable element.
+///
+/// The opaque array data pointer must be a mutable pointer to an
+/// `Option<Vec<MaybeUninit<T>>>` with at least enough elements for `index` to be valid
+/// (zero-based).
+unsafe extern "C" fn deserialize_element<T: Deserialize>(
+ parcel: *const sys::AParcel,
+ array: *mut c_void,
+ index: c_ulong,
+) -> status_t {
+ // c_ulong and usize are the same, but we need the explicitly sized version
+ // so the function signature matches what bindgen generates.
+ let index = index as usize;
+
+ let vec = &mut *(array as *mut Option<Vec<MaybeUninit<T>>>);
+ let vec = match vec {
+ Some(v) => v,
+ None => return StatusCode::BAD_INDEX as status_t,
+ };
+
+ let parcel = match Parcel::borrowed(parcel as *mut _) {
+ None => return StatusCode::UNEXPECTED_NULL as status_t,
+ Some(p) => p,
+ };
+ let element = match parcel.read() {
+ Ok(e) => e,
+ Err(code) => return code as status_t,
+ };
+ ptr::write(vec[index].as_mut_ptr(), element);
+ StatusCode::OK as status_t
+}
+
/// Helper trait for types that can be nullable when serialized.
// We really need this trait instead of implementing `Serialize for Option<T>`
// because of the Rust orphan rule which prevents us from doing
@@ -115,28 +188,54 @@
/// Callback to allocate a vector for parcel array read functions.
///
+/// This variant is for APIs which use an out buffer pointer.
+///
/// # Safety
///
/// The opaque data pointer passed to the array read function must be a mutable
-/// pointer to an `Option<Vec<T>>`. `buffer` will be assigned a mutable pointer
+/// pointer to an `Option<Vec<MaybeUninit<T>>>`. `buffer` will be assigned a mutable pointer
/// to the allocated vector data if this function returns true.
-unsafe extern "C" fn allocate_vec<T: Clone + Default>(
+unsafe extern "C" fn allocate_vec_with_buffer<T>(
data: *mut c_void,
len: i32,
buffer: *mut *mut T,
) -> bool {
- let vec = &mut *(data as *mut Option<Vec<T>>);
+ let res = allocate_vec::<T>(data, len);
+ let vec = &mut *(data as *mut Option<Vec<MaybeUninit<T>>>);
+ if let Some(new_vec) = vec {
+ *buffer = new_vec.as_mut_ptr() as *mut T;
+ }
+ res
+}
+
+/// Callback to allocate a vector for parcel array read functions.
+///
+/// # Safety
+///
+/// The opaque data pointer passed to the array read function must be a mutable
+/// pointer to an `Option<Vec<MaybeUninit<T>>>`.
+unsafe extern "C" fn allocate_vec<T>(
+ data: *mut c_void,
+ len: i32,
+) -> bool {
+ let vec = &mut *(data as *mut Option<Vec<MaybeUninit<T>>>);
if len < 0 {
*vec = None;
return true;
}
- let mut new_vec: Vec<T> = Vec::with_capacity(len as usize);
- new_vec.resize_with(len as usize, Default::default);
- *buffer = new_vec.as_mut_ptr();
- *vec = Some(new_vec);
+ let mut new_vec: Vec<MaybeUninit<T>> = Vec::with_capacity(len as usize);
+
+ // Safety: We are filling the vector with uninitialized data here, but this
+ // is safe because the vector contains MaybeUninit elements which can be
+ // uninitialized. We're putting off the actual unsafe bit, transmuting the
+ // vector to a Vec<T> until the contents are initialized.
+ new_vec.set_len(len as usize);
+
+ ptr::write(vec, Some(new_vec));
true
}
+
macro_rules! parcelable_primitives {
{
$(
@@ -204,19 +303,29 @@
{DeserializeArray, $ty:ty, $read_array_fn:path} => {
impl DeserializeArray for $ty {
fn deserialize_array(parcel: &Parcel) -> Result<Option<Vec<Self>>> {
- let mut vec: Option<Vec<Self>> = None;
+ let mut vec: Option<Vec<MaybeUninit<Self>>> = None;
let status = unsafe {
// Safety: `Parcel` always contains a valid pointer to an
// `AParcel`. `allocate_vec<T>` expects the opaque pointer to
- // be of type `*mut Option<Vec<T>>`, so `&mut vec` is
+ // be of type `*mut Option<Vec<MaybeUninit<T>>>`, so `&mut vec` is
// correct for it.
$read_array_fn(
parcel.as_native(),
&mut vec as *mut _ as *mut c_void,
- Some(allocate_vec),
+ Some(allocate_vec_with_buffer),
)
};
status_result(status)?;
+ let vec: Option<Vec<Self>> = unsafe {
+ // Safety: We are assuming that the NDK correctly
+ // initialized every element of the vector by now, so we
+ // know that all the MaybeUninits are now properly
+ // initialized. We can transmute from Vec<MaybeUninit<T>> to
+ // Vec<T> because MaybeUninit<T> has the same alignment and
+ // size as T, so the pointer to the vector allocation will
+ // be compatible.
+ mem::transmute(vec)
+ };
Ok(vec)
}
}
@@ -414,7 +523,7 @@
sys::AParcel_readString(
parcel.as_native(),
&mut vec as *mut _ as *mut c_void,
- Some(allocate_vec),
+ Some(allocate_vec_with_buffer),
)
};
diff --git a/libs/binder/rust/src/proxy.rs b/libs/binder/rust/src/proxy.rs
index 17af099..e9e74c0 100644
--- a/libs/binder/rust/src/proxy.rs
+++ b/libs/binder/rust/src/proxy.rs
@@ -70,6 +70,20 @@
ptr.as_mut().map(|p| Self(p))
}
+ /// Extract a raw `AIBinder` pointer from this wrapper.
+ ///
+ /// This method should _only_ be used for testing. Do not try to use the NDK
+ /// interface directly for anything else.
+ ///
+ /// # Safety
+ ///
+ /// The resulting pointer is valid only as long as the SpIBinder is alive.
+ /// The SpIBinder object retains ownership of the AIBinder and the caller
+ /// should not attempt to free the returned pointer.
+ pub unsafe fn as_raw(&self) -> *mut sys::AIBinder {
+ self.0
+ }
+
/// Return true if this binder object is hosted in a different process than
/// the current one.
pub fn is_remote(&self) -> bool {
diff --git a/libs/binder/rust/tests/Android.bp b/libs/binder/rust/tests/Android.bp
index 5ae9c53..8810b5d 100644
--- a/libs/binder/rust/tests/Android.bp
+++ b/libs/binder/rust/tests/Android.bp
@@ -79,3 +79,50 @@
"IBinderRustNdkInteropTest-rust",
],
}
+
+cc_test {
+ name: "rustBinderSerializationTest",
+ shared_libs: [
+ "libbinder",
+ "libbinder_ndk",
+ "libutils",
+ "libbase",
+ ],
+ static_libs: [
+ "libbinder_rs_serialization_test"
+ ],
+ srcs: [
+ "serialization.cpp",
+ ],
+ auto_gen_config: true,
+ test_suites: ["general-tests"],
+}
+
+rust_bindgen {
+ name: "libbinder_rs_serialization_bindgen",
+ crate_name: "binder_rs_serialization_bindgen",
+ wrapper_src: "serialization.hpp",
+ source_stem: "bindings",
+ cpp_std: "gnu++17",
+ bindgen_flags: [
+ "--whitelist-type", "Transaction",
+ "--whitelist-var", "TESTDATA_.*",
+ ],
+
+ shared_libs: [
+ "libbinder",
+ "libc++",
+ ],
+}
+
+rust_ffi_static {
+ name: "libbinder_rs_serialization_test",
+ crate_name: "binder_rs_serialization_test",
+ srcs: [
+ "serialization.rs",
+ ":libbinder_rs_serialization_bindgen",
+ ],
+ rustlibs: [
+ "libbinder_rs",
+ ],
+}
diff --git a/libs/binder/rust/tests/serialization.cpp b/libs/binder/rust/tests/serialization.cpp
new file mode 100644
index 0000000..ec780f2
--- /dev/null
+++ b/libs/binder/rust/tests/serialization.cpp
@@ -0,0 +1,454 @@
+/*
+ * Copyright (C) 2020 The Android Open Source Project
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+#include <android/binder_ibinder_platform.h>
+#include <android/binder_libbinder.h>
+#include <binder/IServiceManager.h>
+#include <binder/Parcel.h>
+#include <binder/ParcelFileDescriptor.h>
+#include <binder/ProcessState.h>
+#include <binder/Status.h>
+#include <gtest/gtest.h>
+#include <utils/Errors.h>
+#include <utils/String16.h>
+#include "android-base/file.h"
+#include "serialization.hpp"
+
+#include <cmath>
+#include <cstdint>
+#include <iostream>
+#include <optional>
+
+using namespace std;
+using namespace android;
+using android::base::unique_fd;
+using android::os::ParcelFileDescriptor;
+
+// defined in Rust
+extern "C" AIBinder *rust_service();
+
+
+const int8_t TESTDATA_I8[4] = {-128, 0, 117, 127};
+const uint8_t TESTDATA_U8[4] = {0, 42, 117, 255};
+const char16_t TESTDATA_CHARS[4] = {0, 42, 117, numeric_limits<char16_t>::max()};
+const int32_t TESTDATA_I32[4] = {numeric_limits<int32_t>::min(), 0, 117, numeric_limits<int32_t>::max()};
+const int64_t TESTDATA_I64[4] = {numeric_limits<int64_t>::min(), 0, 117, numeric_limits<int64_t>::max()};
+const uint64_t TESTDATA_U64[4] = {0, 42, 117, numeric_limits<uint64_t>::max()};
+const float TESTDATA_FLOAT[4] = {
+ numeric_limits<float>::quiet_NaN(),
+ -numeric_limits<float>::infinity(),
+ 117.0,
+ numeric_limits<float>::infinity(),
+};
+const double TESTDATA_DOUBLE[4] = {
+ numeric_limits<double>::quiet_NaN(),
+ -numeric_limits<double>::infinity(),
+ 117.0,
+ numeric_limits<double>::infinity(),
+};
+const bool TESTDATA_BOOL[4] = {true, false, false, true};
+const char* const TESTDATA_STRS[4] = {"", nullptr, "test", ""};
+
+static ::testing::Environment* gEnvironment;
+
+class SerializationEnvironment : public ::testing::Environment {
+public:
+ void SetUp() override {
+ m_server = AIBinder_toPlatformBinder(rust_service());
+ }
+
+ sp<IBinder> getServer(void) { return m_server; }
+
+private:
+ sp<IBinder> m_server;
+};
+
+
+class SerializationTest : public ::testing::Test {
+protected:
+ void SetUp() override {
+ ASSERT_NE(gEnvironment, nullptr);
+ m_server = static_cast<SerializationEnvironment *>(gEnvironment)->getServer();
+ }
+
+ sp<IBinder> m_server;
+};
+
+
+TEST_F(SerializationTest, SerializeBool) {
+ android::Parcel data;
+ data.writeInterfaceToken(String16("read_parcel_test"));
+
+ vector<bool> bools(begin(TESTDATA_BOOL), end(TESTDATA_BOOL));
+ ASSERT_EQ(data.writeBool(true), OK);
+ ASSERT_EQ(data.writeBool(false), OK);
+ ASSERT_EQ(data.writeBoolVector(bools), OK);
+ ASSERT_EQ(data.writeBoolVector(nullopt), OK);
+
+ android::Parcel reply;
+ ASSERT_EQ(m_server->transact(TEST_BOOL, data, &reply), OK);
+
+ vector<bool> read_bools;
+ optional<vector<bool>> maybe_bools;
+ ASSERT_EQ(reply.readBool(), true);
+ ASSERT_EQ(reply.readBool(), false);
+ ASSERT_EQ(reply.readBoolVector(&read_bools), OK);
+ ASSERT_EQ(read_bools, bools);
+ ASSERT_EQ(reply.readBoolVector(&maybe_bools), OK);
+ ASSERT_EQ(maybe_bools, nullopt);
+
+ int32_t end;
+ ASSERT_EQ(reply.readInt32(&end), NOT_ENOUGH_DATA);
+}
+
+TEST_F(SerializationTest, SerializeByte) {
+ android::Parcel data;
+ data.writeInterfaceToken(String16("read_parcel_test"));
+
+ vector<int8_t> i8s(begin(TESTDATA_I8), end(TESTDATA_I8));
+ vector<uint8_t> u8s(begin(TESTDATA_U8), end(TESTDATA_U8));
+ data.writeByte(0);
+ data.writeByte(1);
+ data.writeByte(numeric_limits<int8_t>::max());
+ data.writeByteVector(i8s);
+ data.writeByteVector(u8s);
+ data.writeByteVector(optional<vector<int8_t>>({}));
+
+ android::Parcel reply;
+ ASSERT_EQ(m_server->transact(TEST_BYTE, data, &reply), OK);
+
+ vector<int8_t> read_i8s;
+ vector<uint8_t> read_u8s;
+ optional<vector<int8_t>> maybe_i8s;
+ ASSERT_EQ(reply.readByte(), 0);
+ ASSERT_EQ(reply.readByte(), 1);
+ ASSERT_EQ(reply.readByte(), numeric_limits<int8_t>::max());
+ ASSERT_EQ(reply.readByteVector(&read_i8s), OK);
+ ASSERT_EQ(read_i8s, i8s);
+ ASSERT_EQ(reply.readByteVector(&read_u8s), OK);
+ ASSERT_EQ(read_u8s, u8s);
+ ASSERT_EQ(reply.readByteVector(&maybe_i8s), OK);
+ ASSERT_EQ(maybe_i8s, nullopt);
+
+ int32_t end;
+ ASSERT_EQ(reply.readInt32(&end), NOT_ENOUGH_DATA);
+}
+
+TEST_F(SerializationTest, SerializeU16) {
+ android::Parcel data;
+ data.writeInterfaceToken(String16("read_parcel_test"));
+
+ vector<char16_t> chars(begin(TESTDATA_CHARS), end(TESTDATA_CHARS));
+ data.writeChar(0);
+ data.writeChar(1);
+ data.writeChar(numeric_limits<char16_t>::max());
+ data.writeCharVector(chars);
+ data.writeCharVector(nullopt);
+
+ android::Parcel reply;
+ ASSERT_EQ(m_server->transact(TEST_U16, data, &reply), OK);
+
+ vector<char16_t> read_chars;
+ optional<vector<char16_t>> maybe_chars;
+ ASSERT_EQ(reply.readChar(), 0);
+ ASSERT_EQ(reply.readChar(), 1);
+ ASSERT_EQ(reply.readChar(), numeric_limits<char16_t>::max());
+ ASSERT_EQ(reply.readCharVector(&read_chars), OK);
+ ASSERT_EQ(read_chars, chars);
+ ASSERT_EQ(reply.readCharVector(&maybe_chars), OK);
+ ASSERT_EQ(maybe_chars, nullopt);
+
+ int32_t end;
+ ASSERT_EQ(reply.readInt32(&end), NOT_ENOUGH_DATA);
+}
+
+TEST_F(SerializationTest, SerializeI32) {
+ android::Parcel data;
+ data.writeInterfaceToken(String16("read_parcel_test"));
+
+ vector<int32_t> i32s(begin(TESTDATA_I32), end(TESTDATA_I32));
+ data.writeInt32(0);
+ data.writeInt32(1);
+ data.writeInt32(numeric_limits<int32_t>::max());
+ data.writeInt32Vector(i32s);
+ data.writeInt32Vector(nullopt);
+
+ android::Parcel reply;
+ ASSERT_EQ(m_server->transact(TEST_I32, data, &reply), OK);
+
+ vector<int32_t> read_i32s;
+ optional<vector<int32_t>> maybe_i32s;
+ ASSERT_EQ(reply.readInt32(), 0);
+ ASSERT_EQ(reply.readInt32(), 1);
+ ASSERT_EQ(reply.readInt32(), numeric_limits<int32_t>::max());
+ ASSERT_EQ(reply.readInt32Vector(&read_i32s), OK);
+ ASSERT_EQ(read_i32s, i32s);
+ ASSERT_EQ(reply.readInt32Vector(&maybe_i32s), OK);
+ ASSERT_EQ(maybe_i32s, nullopt);
+
+ int32_t end;
+ ASSERT_EQ(reply.readInt32(&end), NOT_ENOUGH_DATA);
+}
+
+TEST_F(SerializationTest, SerializeI64) {
+ android::Parcel data;
+ data.writeInterfaceToken(String16("read_parcel_test"));
+
+ vector<int64_t> i64s(begin(TESTDATA_I64), end(TESTDATA_I64));
+ data.writeInt64(0);
+ data.writeInt64(1);
+ data.writeInt64(numeric_limits<int64_t>::max());
+ data.writeInt64Vector(i64s);
+ data.writeInt64Vector(nullopt);
+
+ android::Parcel reply;
+ ASSERT_EQ(m_server->transact(TEST_I64, data, &reply), OK);
+
+ vector<int64_t> read_i64s;
+ optional<vector<int64_t>> maybe_i64s;
+ ASSERT_EQ(reply.readInt64(), 0);
+ ASSERT_EQ(reply.readInt64(), 1);
+ ASSERT_EQ(reply.readInt64(), numeric_limits<int64_t>::max());
+ ASSERT_EQ(reply.readInt64Vector(&read_i64s), OK);
+ ASSERT_EQ(read_i64s, i64s);
+ ASSERT_EQ(reply.readInt64Vector(&maybe_i64s), OK);
+ ASSERT_EQ(maybe_i64s, nullopt);
+
+ int32_t end;
+ ASSERT_EQ(reply.readInt32(&end), NOT_ENOUGH_DATA);
+}
+
+TEST_F(SerializationTest, SerializeU64) {
+ android::Parcel data;
+ data.writeInterfaceToken(String16("read_parcel_test"));
+
+ vector<uint64_t> u64s(begin(TESTDATA_U64), end(TESTDATA_U64));
+ data.writeUint64(0);
+ data.writeUint64(1);
+ data.writeUint64(numeric_limits<uint64_t>::max());
+ data.writeUint64Vector(u64s);
+ data.writeUint64Vector(nullopt);
+
+ android::Parcel reply;
+ ASSERT_EQ(m_server->transact(TEST_U64, data, &reply), OK);
+
+ vector<uint64_t> read_u64s;
+ optional<vector<uint64_t>> maybe_u64s;
+ ASSERT_EQ(reply.readUint64(), 0);
+ ASSERT_EQ(reply.readUint64(), 1);
+ ASSERT_EQ(reply.readUint64(), numeric_limits<uint64_t>::max());
+ ASSERT_EQ(reply.readUint64Vector(&read_u64s), OK);
+ ASSERT_EQ(read_u64s, u64s);
+ ASSERT_EQ(reply.readUint64Vector(&maybe_u64s), OK);
+ ASSERT_EQ(maybe_u64s, nullopt);
+
+ int32_t end;
+ ASSERT_EQ(reply.readInt32(&end), NOT_ENOUGH_DATA);
+}
+
+TEST_F(SerializationTest, SerializeF32) {
+ android::Parcel data;
+ data.writeInterfaceToken(String16("read_parcel_test"));
+
+ vector<float> floats(begin(TESTDATA_FLOAT), end(TESTDATA_FLOAT));
+ data.writeFloat(0);
+ data.writeFloatVector(floats);
+ data.writeFloatVector(nullopt);
+
+ android::Parcel reply;
+ ASSERT_EQ(m_server->transact(TEST_F32, data, &reply), OK);
+
+ vector<float> read_floats;
+ optional<vector<float>> maybe_floats;
+ ASSERT_EQ(reply.readFloat(), 0);
+ ASSERT_EQ(reply.readFloatVector(&read_floats), OK);
+ ASSERT_TRUE(isnan(read_floats[0]));
+ ASSERT_EQ(read_floats[1], floats[1]);
+ ASSERT_EQ(read_floats[2], floats[2]);
+ ASSERT_EQ(read_floats[3], floats[3]);
+ ASSERT_EQ(reply.readFloatVector(&maybe_floats), OK);
+ ASSERT_EQ(maybe_floats, nullopt);
+
+ int32_t end;
+ ASSERT_EQ(reply.readInt32(&end), NOT_ENOUGH_DATA);
+}
+
+TEST_F(SerializationTest, SerializeF64) {
+ android::Parcel data;
+ data.writeInterfaceToken(String16("read_parcel_test"));
+
+ vector<double> doubles(begin(TESTDATA_DOUBLE), end(TESTDATA_DOUBLE));
+ data.writeDouble(0);
+ data.writeDoubleVector(doubles);
+ data.writeDoubleVector(nullopt);
+
+ android::Parcel reply;
+ ASSERT_EQ(m_server->transact(TEST_F64, data, &reply), OK);
+
+ vector<double> read_doubles;
+ optional<vector<double>> maybe_doubles;
+ ASSERT_EQ(reply.readDouble(), 0);
+ ASSERT_EQ(reply.readDoubleVector(&read_doubles), OK);
+ ASSERT_TRUE(isnan(read_doubles[0]));
+ ASSERT_EQ(read_doubles[1], doubles[1]);
+ ASSERT_EQ(read_doubles[2], doubles[2]);
+ ASSERT_EQ(read_doubles[3], doubles[3]);
+ ASSERT_EQ(reply.readDoubleVector(&maybe_doubles), OK);
+ ASSERT_EQ(maybe_doubles, nullopt);
+
+ int32_t end;
+ ASSERT_EQ(reply.readInt32(&end), NOT_ENOUGH_DATA);
+}
+
+TEST_F(SerializationTest, SerializeString) {
+ android::Parcel data;
+ data.writeInterfaceToken(String16("read_parcel_test"));
+
+ vector<optional<String16>> strings;
+ for (auto I = begin(TESTDATA_STRS), E = end(TESTDATA_STRS); I != E; ++I) {
+ if (*I == nullptr) {
+ strings.push_back(optional<String16>());
+ } else {
+ strings.emplace_back(*I);
+ }
+ }
+ data.writeUtf8AsUtf16(string("testing"));
+ data.writeString16(nullopt);
+ data.writeString16Vector(strings);
+ data.writeString16Vector(nullopt);
+
+ android::Parcel reply;
+ ASSERT_EQ(m_server->transact(TEST_STRING, data, &reply), OK);
+
+ optional<String16> maybe_string;
+ optional<vector<optional<String16>>> read_strings;
+ ASSERT_EQ(reply.readString16(), String16("testing"));
+ ASSERT_EQ(reply.readString16(&maybe_string), OK);
+ ASSERT_EQ(maybe_string, nullopt);
+ ASSERT_EQ(reply.readString16Vector(&read_strings), OK);
+ ASSERT_EQ(read_strings, strings);
+ ASSERT_EQ(reply.readString16Vector(&read_strings), OK);
+ ASSERT_EQ(read_strings, nullopt);
+
+ int32_t end;
+ ASSERT_EQ(reply.readInt32(&end), NOT_ENOUGH_DATA);
+}
+
+TEST_F(SerializationTest, SerializeFileDescriptor) {
+ unique_fd out_file, in_file;
+ ASSERT_TRUE(base::Pipe(&out_file, &in_file));
+
+ vector<ParcelFileDescriptor> file_descriptors;
+ file_descriptors.push_back(ParcelFileDescriptor(std::move(out_file)));
+ file_descriptors.push_back(ParcelFileDescriptor(std::move(in_file)));
+
+ android::Parcel data;
+ data.writeInterfaceToken(String16("read_parcel_test"));
+
+ data.writeParcelable(file_descriptors[0]);
+ data.writeParcelable(file_descriptors[1]);
+ data.writeParcelableVector(file_descriptors);
+
+ android::Parcel reply;
+ ASSERT_EQ(m_server->transact(TEST_FILE_DESCRIPTOR, data, &reply), OK);
+
+ ParcelFileDescriptor returned_fd1, returned_fd2;
+ vector<ParcelFileDescriptor> returned_file_descriptors;
+ ASSERT_EQ(reply.readParcelable(&returned_fd1), OK);
+ ASSERT_EQ(reply.readParcelable(&returned_fd2), OK);
+ ASSERT_EQ(reply.readParcelableVector(&returned_file_descriptors), OK);
+
+ int32_t end;
+ ASSERT_EQ(reply.readInt32(&end), NOT_ENOUGH_DATA);
+
+ base::WriteStringToFd("Testing", returned_fd2.get());
+ base::WriteStringToFd("File", returned_file_descriptors[1].get());
+ base::WriteStringToFd("Descriptors", file_descriptors[1].get());
+
+ string expected = "TestingFileDescriptors";
+ vector<char> buf(expected.length());
+ base::ReadFully(file_descriptors[0].release(), buf.data(), buf.size());
+ ASSERT_EQ(expected, string(buf.data()));
+}
+
+TEST_F(SerializationTest, SerializeIBinder) {
+ android::Parcel data;
+ data.writeInterfaceToken(String16("read_parcel_test"));
+
+ data.writeStrongBinder(m_server);
+ data.writeStrongBinder(nullptr);
+ data.writeStrongBinderVector({m_server, nullptr});
+ data.writeStrongBinderVector(nullopt);
+
+ android::Parcel reply;
+ ASSERT_EQ(m_server->transact(TEST_IBINDER, data, &reply), OK);
+
+ optional<vector<sp<IBinder>>> binders;
+ ASSERT_TRUE(reply.readStrongBinder());
+ ASSERT_FALSE(reply.readStrongBinder());
+ ASSERT_EQ(reply.readStrongBinderVector(&binders), OK);
+ ASSERT_EQ(binders->size(), 2);
+ ASSERT_TRUE((*binders)[0]);
+ ASSERT_FALSE((*binders)[1]);
+ ASSERT_EQ(reply.readStrongBinderVector(&binders), OK);
+ ASSERT_FALSE(binders);
+
+ int32_t end;
+ ASSERT_EQ(reply.readInt32(&end), NOT_ENOUGH_DATA);
+}
+
+TEST_F(SerializationTest, SerializeStatus) {
+ android::Parcel data;
+ data.writeInterfaceToken(String16("read_parcel_test"));
+
+ binder::Status::ok().writeToParcel(&data);
+ binder::Status::fromExceptionCode(binder::Status::EX_NULL_POINTER, "a status message")
+ .writeToParcel(&data);
+ binder::Status::fromServiceSpecificError(42, "a service-specific error").writeToParcel(&data);
+
+ android::Parcel reply;
+ ASSERT_EQ(m_server->transact(TEST_STATUS, data, &reply), OK);
+
+ binder::Status status;
+
+ ASSERT_EQ(status.readFromParcel(reply), OK);
+ ASSERT_TRUE(status.isOk());
+
+ ASSERT_EQ(status.readFromParcel(reply), OK);
+ ASSERT_EQ(status.exceptionCode(), binder::Status::EX_NULL_POINTER);
+ ASSERT_EQ(status.exceptionMessage(), "a status message");
+
+ ASSERT_EQ(status.readFromParcel(reply), OK);
+ ASSERT_EQ(status.serviceSpecificErrorCode(), 42);
+ ASSERT_EQ(status.exceptionMessage(), "a service-specific error");
+
+ int32_t end;
+ ASSERT_EQ(reply.readInt32(&end), NOT_ENOUGH_DATA);
+}
+
+// Test that failures from Rust properly propagate to C++
+TEST_F(SerializationTest, SerializeRustFail) {
+ android::Parcel data;
+ data.writeInterfaceToken(String16("read_parcel_test"));
+ ASSERT_EQ(m_server->transact(TEST_FAIL, data, nullptr), FAILED_TRANSACTION);
+}
+
+int main(int argc, char **argv) {
+ ::testing::InitGoogleTest(&argc, argv);
+ gEnvironment = AddGlobalTestEnvironment(new SerializationEnvironment());
+ ProcessState::self()->startThreadPool();
+ return RUN_ALL_TESTS();
+}
diff --git a/libs/binder/rust/tests/serialization.hpp b/libs/binder/rust/tests/serialization.hpp
new file mode 100644
index 0000000..0041608
--- /dev/null
+++ b/libs/binder/rust/tests/serialization.hpp
@@ -0,0 +1,48 @@
+/*
+ * Copyright (C) 2020 The Android Open Source Project
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+#pragma once
+
+#include <binder/IBinder.h>
+
+using namespace android;
+
+enum Transaction {
+ TEST_BOOL = IBinder::FIRST_CALL_TRANSACTION,
+ TEST_BYTE,
+ TEST_U16,
+ TEST_I32,
+ TEST_I64,
+ TEST_U64,
+ TEST_F32,
+ TEST_F64,
+ TEST_STRING,
+ TEST_FILE_DESCRIPTOR,
+ TEST_IBINDER,
+ TEST_STATUS,
+ TEST_FAIL,
+};
+
+extern const int8_t TESTDATA_I8[4];
+extern const uint8_t TESTDATA_U8[4];
+extern const char16_t TESTDATA_CHARS[4];
+extern const int32_t TESTDATA_I32[4];
+extern const int64_t TESTDATA_I64[4];
+extern const uint64_t TESTDATA_U64[4];
+extern const float TESTDATA_FLOAT[4];
+extern const double TESTDATA_DOUBLE[4];
+extern const bool TESTDATA_BOOL[4];
+extern const char* const TESTDATA_STRS[4];
diff --git a/libs/binder/rust/tests/serialization.rs b/libs/binder/rust/tests/serialization.rs
new file mode 100644
index 0000000..2ae13f4
--- /dev/null
+++ b/libs/binder/rust/tests/serialization.rs
@@ -0,0 +1,306 @@
+/*
+ * Copyright (C) 2020 The Android Open Source Project
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+//! Included as a module in the binder crate internal tests for internal API
+//! access.
+
+use binder::declare_binder_interface;
+use binder::{
+ Binder, ExceptionCode, Interface, Parcel, Result, SpIBinder, Status,
+ StatusCode, TransactionCode,
+};
+use binder::parcel::ParcelFileDescriptor;
+
+use std::ffi::{c_void, CStr, CString};
+use std::panic::{self, AssertUnwindSafe};
+use std::sync::Once;
+
+#[allow(
+ non_camel_case_types,
+ non_snake_case,
+ non_upper_case_globals,
+ unused,
+ improper_ctypes,
+ missing_docs,
+ clippy::all
+)]
+mod bindings {
+ include!(concat!(env!("OUT_DIR"), "/bindings.rs"));
+}
+
+static SERVICE_ONCE: Once = Once::new();
+static mut SERVICE: Option<SpIBinder> = None;
+
+/// Start binder service and return a raw AIBinder pointer to it.
+///
+/// Safe to call multiple times, only creates the service once.
+#[no_mangle]
+pub extern "C" fn rust_service() -> *mut c_void {
+ unsafe {
+ SERVICE_ONCE.call_once(|| {
+ SERVICE = Some(BnReadParcelTest::new_binder(()).as_binder());
+ });
+ SERVICE.as_ref().unwrap().as_raw().cast()
+ }
+}
+
+/// Empty interface just to use the declare_binder_interface macro
+pub trait ReadParcelTest: Interface {}
+
+declare_binder_interface! {
+ ReadParcelTest["read_parcel_test"] {
+ native: BnReadParcelTest(on_transact),
+ proxy: BpReadParcelTest,
+ }
+}
+
+impl ReadParcelTest for Binder<BnReadParcelTest> {}
+
+impl ReadParcelTest for BpReadParcelTest {}
+
+impl ReadParcelTest for () {}
+
+fn on_transact(
+ _service: &dyn ReadParcelTest,
+ code: TransactionCode,
+ parcel: &Parcel,
+ reply: &mut Parcel,
+) -> Result<()> {
+ panic::catch_unwind(AssertUnwindSafe(|| transact_inner(code, parcel, reply))).unwrap_or_else(
+ |e| {
+ eprintln!("Failure in Rust: {:?}", e.downcast_ref::<String>());
+ Err(StatusCode::FAILED_TRANSACTION)
+ },
+ )
+}
+
+#[allow(clippy::float_cmp)]
+fn transact_inner(code: TransactionCode, parcel: &Parcel, reply: &mut Parcel) -> Result<()> {
+ match code {
+ bindings::Transaction_TEST_BOOL => {
+ assert_eq!(parcel.read::<bool>()?, true);
+ assert_eq!(parcel.read::<bool>()?, false);
+ assert_eq!(parcel.read::<Vec<bool>>()?, unsafe {
+ bindings::TESTDATA_BOOL
+ });
+ assert_eq!(parcel.read::<Option<Vec<bool>>>()?, None);
+
+ reply.write(&true)?;
+ reply.write(&false)?;
+ reply.write(&unsafe { bindings::TESTDATA_BOOL }[..])?;
+ reply.write(&(None as Option<Vec<bool>>))?;
+ }
+ bindings::Transaction_TEST_BYTE => {
+ assert_eq!(parcel.read::<i8>()?, 0);
+ assert_eq!(parcel.read::<i8>()?, 1);
+ assert_eq!(parcel.read::<i8>()?, i8::max_value());
+ assert_eq!(parcel.read::<Vec<i8>>()?, unsafe { bindings::TESTDATA_I8 });
+ assert_eq!(parcel.read::<Vec<u8>>()?, unsafe { bindings::TESTDATA_U8 });
+ assert_eq!(parcel.read::<Option<Vec<i8>>>()?, None);
+
+ reply.write(&0i8)?;
+ reply.write(&1i8)?;
+ reply.write(&i8::max_value())?;
+ reply.write(&unsafe { bindings::TESTDATA_I8 }[..])?;
+ reply.write(&unsafe { bindings::TESTDATA_U8 }[..])?;
+ reply.write(&(None as Option<Vec<i8>>))?;
+ }
+ bindings::Transaction_TEST_U16 => {
+ assert_eq!(parcel.read::<u16>()?, 0);
+ assert_eq!(parcel.read::<u16>()?, 1);
+ assert_eq!(parcel.read::<u16>()?, u16::max_value());
+ assert_eq!(parcel.read::<Vec<u16>>()?, unsafe {
+ bindings::TESTDATA_CHARS
+ });
+ assert_eq!(parcel.read::<Option<Vec<u16>>>()?, None);
+
+ reply.write(&0u16)?;
+ reply.write(&1u16)?;
+ reply.write(&u16::max_value())?;
+ reply.write(&unsafe { bindings::TESTDATA_CHARS }[..])?;
+ reply.write(&(None as Option<Vec<u16>>))?;
+ }
+ bindings::Transaction_TEST_I32 => {
+ assert_eq!(parcel.read::<i32>()?, 0);
+ assert_eq!(parcel.read::<i32>()?, 1);
+ assert_eq!(parcel.read::<i32>()?, i32::max_value());
+ assert_eq!(parcel.read::<Vec<i32>>()?, unsafe {
+ bindings::TESTDATA_I32
+ });
+ assert_eq!(parcel.read::<Option<Vec<i32>>>()?, None);
+
+ reply.write(&0i32)?;
+ reply.write(&1i32)?;
+ reply.write(&i32::max_value())?;
+ reply.write(&unsafe { bindings::TESTDATA_I32 }[..])?;
+ reply.write(&(None as Option<Vec<i32>>))?;
+ }
+ bindings::Transaction_TEST_I64 => {
+ assert_eq!(parcel.read::<i64>()?, 0);
+ assert_eq!(parcel.read::<i64>()?, 1);
+ assert_eq!(parcel.read::<i64>()?, i64::max_value());
+ assert_eq!(parcel.read::<Vec<i64>>()?, unsafe {
+ bindings::TESTDATA_I64
+ });
+ assert_eq!(parcel.read::<Option<Vec<i64>>>()?, None);
+
+ reply.write(&0i64)?;
+ reply.write(&1i64)?;
+ reply.write(&i64::max_value())?;
+ reply.write(&unsafe { bindings::TESTDATA_I64 }[..])?;
+ reply.write(&(None as Option<Vec<i64>>))?;
+ }
+ bindings::Transaction_TEST_U64 => {
+ assert_eq!(parcel.read::<u64>()?, 0);
+ assert_eq!(parcel.read::<u64>()?, 1);
+ assert_eq!(parcel.read::<u64>()?, u64::max_value());
+ assert_eq!(parcel.read::<Vec<u64>>()?, unsafe {
+ bindings::TESTDATA_U64
+ });
+ assert_eq!(parcel.read::<Option<Vec<u64>>>()?, None);
+
+ reply.write(&0u64)?;
+ reply.write(&1u64)?;
+ reply.write(&u64::max_value())?;
+ reply.write(&unsafe { bindings::TESTDATA_U64 }[..])?;
+ reply.write(&(None as Option<Vec<u64>>))?;
+ }
+ bindings::Transaction_TEST_F32 => {
+ assert_eq!(parcel.read::<f32>()?, 0f32);
+ let floats = parcel.read::<Vec<f32>>()?;
+ assert!(floats[0].is_nan());
+ assert_eq!(floats[1..], unsafe { bindings::TESTDATA_FLOAT }[1..]);
+ assert_eq!(parcel.read::<Option<Vec<f32>>>()?, None);
+
+ reply.write(&0f32)?;
+ reply.write(&unsafe { bindings::TESTDATA_FLOAT }[..])?;
+ reply.write(&(None as Option<Vec<f32>>))?;
+ }
+ bindings::Transaction_TEST_F64 => {
+ assert_eq!(parcel.read::<f64>()?, 0f64);
+ let doubles = parcel.read::<Vec<f64>>()?;
+ assert!(doubles[0].is_nan());
+ assert_eq!(doubles[1..], unsafe { bindings::TESTDATA_DOUBLE }[1..]);
+ assert_eq!(parcel.read::<Option<Vec<f64>>>()?, None);
+
+ reply.write(&0f64)?;
+ reply.write(&unsafe { bindings::TESTDATA_DOUBLE }[..])?;
+ reply.write(&(None as Option<Vec<f64>>))?;
+ }
+ bindings::Transaction_TEST_STRING => {
+ let s: Option<String> = parcel.read()?;
+ assert_eq!(s.as_deref(), Some("testing"));
+ let s: Option<String> = parcel.read()?;
+ assert_eq!(s, None);
+ let s: Option<Vec<Option<String>>> = parcel.read()?;
+ for (s, expected) in s
+ .unwrap()
+ .iter()
+ .zip(unsafe { bindings::TESTDATA_STRS }.iter())
+ {
+ let expected = unsafe {
+ expected
+ .as_ref()
+ .and_then(|e| CStr::from_ptr(e).to_str().ok())
+ };
+ assert_eq!(s.as_deref(), expected);
+ }
+ let s: Option<Vec<Option<String>>> = parcel.read()?;
+ assert_eq!(s, None);
+
+ let strings: Vec<Option<String>> = unsafe {
+ bindings::TESTDATA_STRS
+ .iter()
+ .map(|s| {
+ s.as_ref().map(|s| {
+ CStr::from_ptr(s)
+ .to_str()
+ .expect("String was not UTF-8")
+ .to_owned()
+ })
+ })
+ .collect()
+ };
+
+ reply.write("testing")?;
+ reply.write(&(None as Option<String>))?;
+ reply.write(&strings)?;
+ reply.write(&(None as Option<Vec<String>>))?;
+ }
+ bindings::Transaction_TEST_FILE_DESCRIPTOR => {
+ let file1 = parcel.read::<ParcelFileDescriptor>()?;
+ let file2 = parcel.read::<ParcelFileDescriptor>()?;
+ let files = parcel.read::<Vec<Option<ParcelFileDescriptor>>>()?;
+
+ reply.write(&file1)?;
+ reply.write(&file2)?;
+ reply.write(&files)?;
+ }
+ bindings::Transaction_TEST_IBINDER => {
+ assert!(parcel.read::<Option<SpIBinder>>()?.is_some());
+ assert!(parcel.read::<Option<SpIBinder>>()?.is_none());
+ let ibinders = parcel.read::<Option<Vec<Option<SpIBinder>>>>()?.unwrap();
+ assert_eq!(ibinders.len(), 2);
+ assert!(ibinders[0].is_some());
+ assert!(ibinders[1].is_none());
+ assert!(parcel.read::<Option<Vec<Option<SpIBinder>>>>()?.is_none());
+
+ let service = unsafe {
+ SERVICE
+ .as_ref()
+ .expect("Global binder service not initialized")
+ .clone()
+ };
+ reply.write(&service)?;
+ reply.write(&(None as Option<&SpIBinder>))?;
+ reply.write(&[Some(&service), None][..])?;
+ reply.write(&(None as Option<Vec<Option<&SpIBinder>>>))?;
+ }
+ bindings::Transaction_TEST_STATUS => {
+ let status: Status = parcel.read()?;
+ assert!(status.is_ok());
+ let status: Status = parcel.read()?;
+ assert_eq!(status.exception_code(), ExceptionCode::NULL_POINTER);
+ assert_eq!(
+ status.get_description(),
+ "Status(-4, EX_NULL_POINTER): 'a status message'"
+ );
+ let status: Status = parcel.read()?;
+ assert_eq!(status.service_specific_error(), 42);
+ assert_eq!(
+ status.get_description(),
+ "Status(-8, EX_SERVICE_SPECIFIC): '42: a service-specific error'"
+ );
+
+ reply.write(&Status::ok())?;
+ reply.write(&Status::new_exception(
+ ExceptionCode::NULL_POINTER,
+ Some(&CString::new("a status message").unwrap()),
+ ))?;
+ reply.write(&Status::new_service_specific_error(
+ 42,
+ Some(&CString::new("a service-specific error").unwrap()),
+ ))?;
+ }
+ bindings::Transaction_TEST_FAIL => {
+ panic!("Testing expected failure");
+ }
+ _ => return Err(StatusCode::UNKNOWN_TRANSACTION),
+ }
+
+ assert_eq!(parcel.read::<i32>(), Err(StatusCode::NOT_ENOUGH_DATA));
+ Ok(())
+}
diff --git a/libs/binder/tests/Android.bp b/libs/binder/tests/Android.bp
index 87f1d45..988f7f3 100644
--- a/libs/binder/tests/Android.bp
+++ b/libs/binder/tests/Android.bp
@@ -157,6 +157,24 @@
require_root: true,
}
+cc_test {
+ name: "binderClearBufTest",
+ defaults: ["binder_test_defaults"],
+ srcs: [
+ "binderClearBufTest.cpp",
+ ],
+
+ shared_libs: [
+ "libbase",
+ "libbinder",
+ "liblog",
+ "libutils",
+ ],
+
+ test_suites: ["general-tests"],
+ require_root: true,
+}
+
aidl_interface {
name: "binderStabilityTestIface",
unstable: true,
diff --git a/libs/binder/tests/binderClearBufTest.cpp b/libs/binder/tests/binderClearBufTest.cpp
new file mode 100644
index 0000000..2d30c8d
--- /dev/null
+++ b/libs/binder/tests/binderClearBufTest.cpp
@@ -0,0 +1,130 @@
+/*
+ * Copyright (C) 2020 The Android Open Source Project
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+#include <android-base/logging.h>
+#include <binder/Binder.h>
+#include <binder/IBinder.h>
+#include <binder/IPCThreadState.h>
+#include <binder/IServiceManager.h>
+#include <binder/Parcel.h>
+#include <binder/Stability.h>
+#include <gtest/gtest.h>
+
+#include <sys/prctl.h>
+#include <thread>
+
+using namespace android;
+
+const String16 kServerName = String16("binderClearBuf");
+
+std::string hexString(const void* bytes, size_t len) {
+ if (bytes == nullptr) return "<null>";
+
+ const uint8_t* bytes8 = static_cast<const uint8_t*>(bytes);
+ char chars[] = "0123456789abcdef";
+ std::string result;
+ result.resize(len * 2);
+
+ for (size_t i = 0; i < len; i++) {
+ result[2 * i] = chars[bytes8[i] >> 4];
+ result[2 * i + 1] = chars[bytes8[i] & 0xf];
+ }
+
+ return result;
+}
+
+class FooBar : public BBinder {
+ public:
+ enum {
+ TRANSACTION_REPEAT_STRING = IBinder::FIRST_CALL_TRANSACTION,
+ };
+
+ std::mutex foo;
+ std::string last;
+
+ status_t onTransact(uint32_t code, const Parcel& data, Parcel* reply, uint32_t flags) {
+ // not checking data, since there is no hook at the time this test is
+ // written to check values there are set to zero. Instead, we only check
+ // the reply parcel.
+
+ switch (code) {
+ case TRANSACTION_REPEAT_STRING: {
+ const char* str = data.readCString();
+ return reply->writeCString(str == nullptr ? "<null>" : str);
+ }
+ }
+ return BBinder::onTransact(code, data, reply, flags);
+ }
+ static std::string RepeatString(const sp<IBinder> binder,
+ const std::string& repeat,
+ std::string* outBuffer) {
+ Parcel data;
+ data.writeCString(repeat.c_str());
+ std::string result;
+ const uint8_t* lastReply;
+ size_t lastReplySize;
+ {
+ Parcel reply;
+ binder->transact(TRANSACTION_REPEAT_STRING, data, &reply, FLAG_CLEAR_BUF);
+ result = reply.readCString();
+ lastReply = reply.data();
+ lastReplySize = reply.dataSize();
+ }
+ *outBuffer = hexString(lastReply, lastReplySize);
+ return result;
+ }
+};
+
+TEST(BinderClearBuf, ClearKernelBuffer) {
+ sp<IBinder> binder = defaultServiceManager()->getService(kServerName);
+ ASSERT_NE(nullptr, binder);
+
+ std::string replyBuffer;
+ std::string result = FooBar::RepeatString(binder, "foo", &replyBuffer);
+ EXPECT_EQ("foo", result);
+
+ // the buffer must have at least some length for the string, but we will
+ // just check it has some length, to avoid assuming anything about the
+ // format
+ EXPECT_GT(replyBuffer.size(), 0);
+
+ for (size_t i = 0; i < replyBuffer.size(); i++) {
+ EXPECT_EQ(replyBuffer[i], '0') << "reply buffer at " << i;
+ }
+}
+
+int main(int argc, char** argv) {
+ ::testing::InitGoogleTest(&argc, argv);
+
+ if (fork() == 0) {
+ prctl(PR_SET_PDEATHSIG, SIGHUP);
+
+ sp<IBinder> server = new FooBar;
+ android::defaultServiceManager()->addService(kServerName, server);
+
+ IPCThreadState::self()->joinThreadPool(true);
+ exit(1); // should not reach
+ }
+
+ // This is not racey. Just giving these services some time to register before we call
+ // getService which sleeps for much longer. One alternative would be to
+ // start a threadpool + use waitForService, but we want to have as few
+ // binder things going on in this test as possible, since we are checking
+ // memory is zero'd which the kernel has a right to change.
+ usleep(100000);
+
+ return RUN_ALL_TESTS();
+}
diff --git a/libs/binder/tests/fuzzers/Android.bp b/libs/binder/tests/fuzzers/Android.bp
index c465bed..5531296 100644
--- a/libs/binder/tests/fuzzers/Android.bp
+++ b/libs/binder/tests/fuzzers/Android.bp
@@ -69,3 +69,18 @@
defaults: ["binder_fuzz_defaults"],
srcs: ["TextOutputFuzz.cpp"],
}
+
+cc_fuzz {
+ name: "binder_bufferedTextOutputFuzz",
+ include_dirs: [
+ "frameworks/native/libs/binder",
+ ],
+ defaults: ["binder_fuzz_defaults"],
+ srcs: ["BufferedTextOutputFuzz.cpp"],
+}
+
+cc_fuzz {
+ name: "binder_memoryDealerFuzz",
+ defaults: ["binder_fuzz_defaults"],
+ srcs: ["MemoryDealerFuzz.cpp"],
+}
diff --git a/libs/binder/tests/fuzzers/BufferedTextOutputFuzz.cpp b/libs/binder/tests/fuzzers/BufferedTextOutputFuzz.cpp
new file mode 100644
index 0000000..09cb216
--- /dev/null
+++ b/libs/binder/tests/fuzzers/BufferedTextOutputFuzz.cpp
@@ -0,0 +1,72 @@
+/*
+ * Copyright (C) 2020 The Android Open Source Project
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+#include <commonFuzzHelpers.h>
+#include <fuzzer/FuzzedDataProvider.h>
+#include <string>
+#include <vector>
+#include "BufferedTextOutput.h"
+
+namespace android {
+
+class FuzzBufferedTextOutput : public BufferedTextOutput {
+public:
+ FuzzBufferedTextOutput(uint32_t flags) : BufferedTextOutput(flags) {}
+ virtual status_t writeLines(const struct iovec& buf, size_t) {
+ size_t len = buf.iov_len;
+ void* tmp_buf = malloc(len);
+
+ if (tmp_buf == NULL) {
+ return status_t();
+ }
+
+ // This will attempt to read data from iov_base to ensure valid params were passed.
+ memcpy(tmp_buf, buf.iov_base, len);
+ free(tmp_buf);
+ return status_t();
+ }
+};
+
+// Fuzzer entry point.
+extern "C" int LLVMFuzzerTestOneInput(const uint8_t* data, size_t size) {
+ FuzzedDataProvider fdp(data, size);
+ uint32_t flags = fdp.ConsumeIntegral<uint32_t>();
+ size_t push_count = 0;
+ std::shared_ptr<BufferedTextOutput> bTextOutput(new FuzzBufferedTextOutput(flags));
+
+ while (fdp.remaining_bytes() > 0) {
+ fdp.PickValueInArray<std::function<void()>>({
+ [&]() -> void {
+ bTextOutput->pushBundle();
+ push_count++;
+ },
+ [&]() -> void {
+ std::string txt = fdp.ConsumeRandomLengthString(fdp.remaining_bytes());
+ size_t len = fdp.ConsumeIntegralInRange<size_t>(0, txt.length());
+ bTextOutput->print(txt.c_str(), len);
+ },
+ [&]() -> void {
+ if (push_count == 0) return;
+
+ bTextOutput->popBundle();
+ push_count--;
+ },
+ })();
+ }
+
+ return 0;
+}
+} // namespace android
diff --git a/libs/binder/tests/fuzzers/MemoryDealerFuzz.cpp b/libs/binder/tests/fuzzers/MemoryDealerFuzz.cpp
new file mode 100644
index 0000000..f9dda8c
--- /dev/null
+++ b/libs/binder/tests/fuzzers/MemoryDealerFuzz.cpp
@@ -0,0 +1,77 @@
+/*
+ * Copyright (C) 2020 The Android Open Source Project
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+#include <binder/MemoryDealer.h>
+#include <commonFuzzHelpers.h>
+#include <fuzzer/FuzzedDataProvider.h>
+#include <string>
+#include <unordered_set>
+
+namespace android {
+
+static constexpr size_t kMaxBufferSize = 10000;
+static constexpr size_t kMaxDealerSize = 1024 * 512;
+static constexpr size_t kMaxAllocSize = 1024;
+
+// Fuzzer entry point.
+extern "C" int LLVMFuzzerTestOneInput(const uint8_t* data, size_t size) {
+ if (size > kMaxBufferSize) {
+ return 0;
+ }
+
+ FuzzedDataProvider fdp(data, size);
+ size_t dSize = fdp.ConsumeIntegralInRange<size_t>(0, kMaxDealerSize);
+ std::string name = fdp.ConsumeRandomLengthString(fdp.remaining_bytes());
+ uint32_t flags = fdp.ConsumeIntegral<uint32_t>();
+ sp<MemoryDealer> dealer = new MemoryDealer(dSize, name.c_str(), flags);
+
+ // This is used to track offsets that have been freed already to avoid an expected fatal log.
+ std::unordered_set<size_t> free_list;
+
+ while (fdp.remaining_bytes() > 0) {
+ fdp.PickValueInArray<std::function<void()>>({
+ [&]() -> void { dealer->getAllocationAlignment(); },
+ [&]() -> void { dealer->getMemoryHeap(); },
+ [&]() -> void {
+ size_t offset = fdp.ConsumeIntegral<size_t>();
+
+ // Offset has already been freed, so return instead.
+ if (free_list.find(offset) != free_list.end()) return;
+
+ dealer->deallocate(offset);
+ free_list.insert(offset);
+ },
+ [&]() -> void {
+ std::string randString = fdp.ConsumeRandomLengthString(fdp.remaining_bytes());
+ dealer->dump(randString.c_str());
+ },
+ [&]() -> void {
+ size_t allocSize = fdp.ConsumeIntegralInRange<size_t>(0, kMaxAllocSize);
+ sp<IMemory> allocated = dealer->allocate(allocSize);
+ // If the allocation was successful, try to write to it
+ if (allocated != nullptr && allocated->unsecurePointer() != nullptr) {
+ memset(allocated->unsecurePointer(), 1, allocated->size());
+
+ // Clear the address from freelist since it has been allocated over again.
+ free_list.erase(allocated->offset());
+ }
+ },
+ })();
+ }
+
+ return 0;
+}
+} // namespace android
diff --git a/libs/cputimeinstate/cputimeinstate.cpp b/libs/cputimeinstate/cputimeinstate.cpp
index 22103d5..462f0db 100644
--- a/libs/cputimeinstate/cputimeinstate.cpp
+++ b/libs/cputimeinstate/cputimeinstate.cpp
@@ -56,6 +56,7 @@
static std::vector<std::vector<uint32_t>> gPolicyFreqs;
static std::vector<std::vector<uint32_t>> gPolicyCpus;
static std::set<uint32_t> gAllFreqs;
+static unique_fd gTisTotalMapFd;
static unique_fd gTisMapFd;
static unique_fd gConcurrentMapFd;
static unique_fd gUidLastUpdateMapFd;
@@ -129,6 +130,10 @@
gPolicyCpus.emplace_back(*cpus);
}
+ gTisTotalMapFd =
+ unique_fd{bpf_obj_get(BPF_FS_PATH "map_time_in_state_total_time_in_state_map")};
+ if (gTisTotalMapFd < 0) return false;
+
gTisMapFd = unique_fd{bpf_obj_get(BPF_FS_PATH "map_time_in_state_uid_time_in_state_map")};
if (gTisMapFd < 0) return false;
@@ -239,6 +244,31 @@
return gPolicyFreqs;
}
+std::optional<std::vector<std::vector<uint64_t>>> getTotalCpuFreqTimes() {
+ if (!gInitialized && !initGlobals()) return {};
+
+ std::vector<std::vector<uint64_t>> out;
+ uint32_t maxFreqCount = 0;
+ for (const auto &freqList : gPolicyFreqs) {
+ if (freqList.size() > maxFreqCount) maxFreqCount = freqList.size();
+ out.emplace_back(freqList.size(), 0);
+ }
+
+ std::vector<uint64_t> vals(gNCpus);
+ const uint32_t freqCount = maxFreqCount <= MAX_FREQS_FOR_TOTAL ? maxFreqCount :
+ MAX_FREQS_FOR_TOTAL;
+ for (uint32_t freqIdx = 0; freqIdx < freqCount; ++freqIdx) {
+ if (findMapEntry(gTisTotalMapFd, &freqIdx, vals.data())) return {};
+ for (uint32_t policyIdx = 0; policyIdx < gNPolicies; ++policyIdx) {
+ if (freqIdx >= gPolicyFreqs[policyIdx].size()) continue;
+ for (const auto &cpu : gPolicyCpus[policyIdx]) {
+ out[policyIdx][freqIdx] += vals[cpu];
+ }
+ }
+ }
+
+ return out;
+}
// Retrieve the times in ns that uid spent running at each CPU frequency.
// Return contains no value on error, otherwise it contains a vector of vectors using the format:
// [[t0_0, t0_1, ...],
diff --git a/libs/cputimeinstate/cputimeinstate.h b/libs/cputimeinstate/cputimeinstate.h
index 87a328a..46de669 100644
--- a/libs/cputimeinstate/cputimeinstate.h
+++ b/libs/cputimeinstate/cputimeinstate.h
@@ -23,6 +23,7 @@
namespace bpf {
bool startTrackingUidTimes();
+std::optional<std::vector<std::vector<uint64_t>>> getTotalCpuFreqTimes();
std::optional<std::vector<std::vector<uint64_t>>> getUidCpuFreqTimes(uint32_t uid);
std::optional<std::unordered_map<uint32_t, std::vector<std::vector<uint64_t>>>>
getUidsCpuFreqTimes();
diff --git a/libs/cputimeinstate/testtimeinstate.cpp b/libs/cputimeinstate/testtimeinstate.cpp
index 519689b..d25b2e9 100644
--- a/libs/cputimeinstate/testtimeinstate.cpp
+++ b/libs/cputimeinstate/testtimeinstate.cpp
@@ -40,6 +40,12 @@
using std::vector;
+TEST(TimeInStateTest, TotalTimeInState) {
+ auto times = getTotalCpuFreqTimes();
+ ASSERT_TRUE(times.has_value());
+ EXPECT_FALSE(times->empty());
+}
+
TEST(TimeInStateTest, SingleUidTimeInState) {
auto times = getUidCpuFreqTimes(0);
ASSERT_TRUE(times.has_value());
@@ -186,6 +192,31 @@
}
}
+TEST(TimeInStateTest, TotalAndAllUidTimeInStateConsistent) {
+ auto allUid = getUidsCpuFreqTimes();
+ auto total = getTotalCpuFreqTimes();
+
+ ASSERT_TRUE(allUid.has_value() && total.has_value());
+
+ // Check the number of policies.
+ ASSERT_EQ(allUid->at(0).size(), total->size());
+
+ for (uint32_t policyIdx = 0; policyIdx < total->size(); ++policyIdx) {
+ std::vector<uint64_t> totalTimes = total->at(policyIdx);
+ uint32_t totalFreqsCount = totalTimes.size();
+ std::vector<uint64_t> allUidTimes(totalFreqsCount, 0);
+ for (auto const &[uid, uidTimes]: *allUid) {
+ for (uint32_t freqIdx = 0; freqIdx < uidTimes[policyIdx].size(); ++freqIdx) {
+ allUidTimes[std::min(freqIdx, totalFreqsCount - 1)] += uidTimes[policyIdx][freqIdx];
+ }
+ }
+
+ for (uint32_t freqIdx = 0; freqIdx < totalFreqsCount; ++freqIdx) {
+ ASSERT_LE(allUidTimes[freqIdx], totalTimes[freqIdx]);
+ }
+ }
+}
+
TEST(TimeInStateTest, SingleAndAllUidTimeInStateConsistent) {
uint64_t zero = 0;
auto maps = {getUidsCpuFreqTimes(), getUidsUpdatedCpuFreqTimes(&zero)};
@@ -292,6 +323,22 @@
ASSERT_LE(after - before, NSEC_PER_SEC * 2 * get_nprocs_conf());
}
+TEST(TimeInStateTest, TotalTimeInStateMonotonic) {
+ auto before = getTotalCpuFreqTimes();
+ ASSERT_TRUE(before.has_value());
+ sleep(1);
+ auto after = getTotalCpuFreqTimes();
+ ASSERT_TRUE(after.has_value());
+
+ for (uint32_t policyIdx = 0; policyIdx < after->size(); ++policyIdx) {
+ auto timesBefore = before->at(policyIdx);
+ auto timesAfter = after->at(policyIdx);
+ for (uint32_t freqIdx = 0; freqIdx < timesAfter.size(); ++freqIdx) {
+ ASSERT_NO_FATAL_FAILURE(TestCheckDelta(timesBefore[freqIdx], timesAfter[freqIdx]));
+ }
+ }
+}
+
TEST(TimeInStateTest, AllUidTimeInStateMonotonic) {
auto map1 = getUidsCpuFreqTimes();
ASSERT_TRUE(map1.has_value());
diff --git a/services/gpuservice/tests/unittests/Android.bp b/services/gpuservice/tests/unittests/Android.bp
index 940a26b..9606daa 100644
--- a/services/gpuservice/tests/unittests/Android.bp
+++ b/services/gpuservice/tests/unittests/Android.bp
@@ -20,6 +20,7 @@
},
srcs: [
"GpuMemTest.cpp",
+ "GpuMemTracerTest.cpp",
"GpuStatsTest.cpp",
],
shared_libs: [
@@ -29,14 +30,19 @@
"libcutils",
"libgfxstats",
"libgpumem",
+ "libgpumemtracer",
"libgraphicsenv",
"liblog",
+ "libprotobuf-cpp-lite",
+ "libprotoutil",
"libstatslog",
"libstatspull",
"libutils",
],
static_libs: [
"libgmock",
+ "libperfetto_client_experimental",
+ "perfetto_trace_protos",
],
require_root: true,
}
diff --git a/services/gpuservice/tests/unittests/GpuMemTest.cpp b/services/gpuservice/tests/unittests/GpuMemTest.cpp
index c5f8859..e916221 100644
--- a/services/gpuservice/tests/unittests/GpuMemTest.cpp
+++ b/services/gpuservice/tests/unittests/GpuMemTest.cpp
@@ -59,7 +59,6 @@
}
void SetUp() override {
- SKIP_IF_BPF_NOT_SUPPORTED;
bpf::setrlimitForTest();
mGpuMem = std::make_unique<GpuMem>();
@@ -87,8 +86,6 @@
};
TEST_F(GpuMemTest, validGpuMemTotalBpfPaths) {
- SKIP_IF_BPF_NOT_SUPPORTED;
-
EXPECT_EQ(mTestableGpuMem.getGpuMemTraceGroup(), "gpu_mem");
EXPECT_EQ(mTestableGpuMem.getGpuMemTotalTracepoint(), "gpu_mem_total");
EXPECT_EQ(mTestableGpuMem.getGpuMemTotalProgPath(),
@@ -97,20 +94,16 @@
}
TEST_F(GpuMemTest, bpfInitializationFailed) {
- SKIP_IF_BPF_NOT_SUPPORTED;
-
EXPECT_EQ(dumpsys(), "Failed to initialize GPU memory eBPF\n");
}
TEST_F(GpuMemTest, gpuMemTotalMapEmpty) {
- SKIP_IF_BPF_NOT_SUPPORTED;
mTestableGpuMem.setGpuMemTotalMap(mTestMap);
EXPECT_EQ(dumpsys(), "GPU memory total usage map is empty\n");
}
TEST_F(GpuMemTest, globalMemTotal) {
- SKIP_IF_BPF_NOT_SUPPORTED;
ASSERT_RESULT_OK(mTestMap.writeValue(TEST_GLOBAL_KEY, TEST_GLOBAL_VAL, BPF_ANY));
mTestableGpuMem.setGpuMemTotalMap(mTestMap);
@@ -118,7 +111,6 @@
}
TEST_F(GpuMemTest, missingGlobalMemTotal) {
- SKIP_IF_BPF_NOT_SUPPORTED;
ASSERT_RESULT_OK(mTestMap.writeValue(TEST_PROC_KEY_1, TEST_PROC_VAL_1, BPF_ANY));
mTestableGpuMem.setGpuMemTotalMap(mTestMap);
@@ -126,7 +118,6 @@
}
TEST_F(GpuMemTest, procMemTotal) {
- SKIP_IF_BPF_NOT_SUPPORTED;
ASSERT_RESULT_OK(mTestMap.writeValue(TEST_PROC_KEY_1, TEST_PROC_VAL_1, BPF_ANY));
ASSERT_RESULT_OK(mTestMap.writeValue(TEST_PROC_KEY_2, TEST_PROC_VAL_2, BPF_ANY));
mTestableGpuMem.setGpuMemTotalMap(mTestMap);
@@ -146,7 +137,6 @@
}
TEST_F(GpuMemTest, traverseGpuMemTotals) {
- SKIP_IF_BPF_NOT_SUPPORTED;
ASSERT_RESULT_OK(mTestMap.writeValue(TEST_GLOBAL_KEY, TEST_GLOBAL_VAL, BPF_ANY));
ASSERT_RESULT_OK(mTestMap.writeValue(TEST_PROC_KEY_1, TEST_PROC_VAL_1, BPF_ANY));
ASSERT_RESULT_OK(mTestMap.writeValue(TEST_PROC_KEY_2, TEST_PROC_VAL_2, BPF_ANY));
diff --git a/services/gpuservice/tests/unittests/GpuMemTracerTest.cpp b/services/gpuservice/tests/unittests/GpuMemTracerTest.cpp
new file mode 100644
index 0000000..d76f039
--- /dev/null
+++ b/services/gpuservice/tests/unittests/GpuMemTracerTest.cpp
@@ -0,0 +1,196 @@
+/*
+ * Copyright 2020 The Android Open Source Project
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+#undef LOG_TAG
+#define LOG_TAG "gpuservice_unittest"
+
+#include <bpf/BpfMap.h>
+#include <gpumem/GpuMem.h>
+#include <gtest/gtest.h>
+#include <perfetto/trace/trace.pb.h>
+#include <tracing/GpuMemTracer.h>
+
+#include "TestableGpuMem.h"
+
+namespace android {
+
+constexpr uint32_t TEST_MAP_SIZE = 10;
+constexpr uint64_t TEST_GLOBAL_KEY = 0;
+constexpr uint32_t TEST_GLOBAL_PID = 0;
+constexpr uint64_t TEST_GLOBAL_VAL = 123;
+constexpr uint32_t TEST_GLOBAL_GPU_ID = 0;
+constexpr uint64_t TEST_PROC_KEY_1 = 1;
+constexpr uint32_t TEST_PROC_PID_1 = 1;
+constexpr uint64_t TEST_PROC_VAL_1 = 234;
+constexpr uint32_t TEST_PROC_1_GPU_ID = 0;
+constexpr uint64_t TEST_PROC_KEY_2 = 4294967298; // (1 << 32) + 2
+constexpr uint32_t TEST_PROC_PID_2 = 2;
+constexpr uint64_t TEST_PROC_VAL_2 = 345;
+constexpr uint32_t TEST_PROC_2_GPU_ID = 1;
+
+class GpuMemTracerTest : public testing::Test {
+public:
+ GpuMemTracerTest() {
+ const ::testing::TestInfo* const test_info =
+ ::testing::UnitTest::GetInstance()->current_test_info();
+ ALOGD("**** Setting up for %s.%s\n", test_info->test_case_name(), test_info->name());
+ }
+
+ ~GpuMemTracerTest() {
+ const ::testing::TestInfo* const test_info =
+ ::testing::UnitTest::GetInstance()->current_test_info();
+ ALOGD("**** Tearing down after %s.%s\n", test_info->test_case_name(), test_info->name());
+ }
+
+ void SetUp() override {
+ bpf::setrlimitForTest();
+
+ mGpuMem = std::make_shared<GpuMem>();
+ mGpuMemTracer = std::make_unique<GpuMemTracer>();
+ mGpuMemTracer->initializeForTest(mGpuMem);
+ mTestableGpuMem = TestableGpuMem(mGpuMem.get());
+
+ errno = 0;
+ mTestMap = bpf::BpfMap<uint64_t, uint64_t>(BPF_MAP_TYPE_HASH, TEST_MAP_SIZE,
+ BPF_F_NO_PREALLOC);
+
+ EXPECT_EQ(0, errno);
+ EXPECT_LE(0, mTestMap.getMap().get());
+ EXPECT_TRUE(mTestMap.isValid());
+ }
+
+ int getTracerThreadCount() { return mGpuMemTracer->tracerThreadCount; }
+
+ std::vector<perfetto::protos::TracePacket> readGpuMemTotalPacketsBlocking(
+ perfetto::TracingSession* tracingSession) {
+ std::vector<char> raw_trace = tracingSession->ReadTraceBlocking();
+ perfetto::protos::Trace trace;
+ trace.ParseFromArray(raw_trace.data(), int(raw_trace.size()));
+
+ std::vector<perfetto::protos::TracePacket> packets;
+ for (const auto& packet : trace.packet()) {
+ if (!packet.has_gpu_mem_total_event()) {
+ continue;
+ }
+ packets.emplace_back(packet);
+ }
+ return packets;
+ }
+
+ std::shared_ptr<GpuMem> mGpuMem;
+ TestableGpuMem mTestableGpuMem;
+ std::unique_ptr<GpuMemTracer> mGpuMemTracer;
+ bpf::BpfMap<uint64_t, uint64_t> mTestMap;
+};
+
+static constexpr uint64_t getSizeForPid(uint32_t pid) {
+ switch (pid) {
+ case TEST_GLOBAL_PID:
+ return TEST_GLOBAL_VAL;
+ case TEST_PROC_PID_1:
+ return TEST_PROC_VAL_1;
+ case TEST_PROC_PID_2:
+ return TEST_PROC_VAL_2;
+ }
+ return 0;
+}
+
+static constexpr uint32_t getGpuIdForPid(uint32_t pid) {
+ switch (pid) {
+ case TEST_GLOBAL_PID:
+ return TEST_GLOBAL_GPU_ID;
+ case TEST_PROC_PID_1:
+ return TEST_PROC_1_GPU_ID;
+ case TEST_PROC_PID_2:
+ return TEST_PROC_2_GPU_ID;
+ }
+ return 0;
+}
+
+TEST_F(GpuMemTracerTest, traceInitialCountersAfterGpuMemInitialize) {
+ ASSERT_RESULT_OK(mTestMap.writeValue(TEST_GLOBAL_KEY, TEST_GLOBAL_VAL, BPF_ANY));
+ ASSERT_RESULT_OK(mTestMap.writeValue(TEST_PROC_KEY_1, TEST_PROC_VAL_1, BPF_ANY));
+ ASSERT_RESULT_OK(mTestMap.writeValue(TEST_PROC_KEY_2, TEST_PROC_VAL_2, BPF_ANY));
+ mTestableGpuMem.setGpuMemTotalMap(mTestMap);
+ mTestableGpuMem.setInitialized();
+
+ // Only 1 tracer thread should be existing for test.
+ EXPECT_EQ(getTracerThreadCount(), 1);
+ auto tracingSession = mGpuMemTracer->getTracingSessionForTest();
+
+ tracingSession->StartBlocking();
+ // Sleep for a short time to let the tracer thread finish its work
+ sleep(1);
+ tracingSession->StopBlocking();
+
+ // The test tracer thread should have finished its execution by now.
+ EXPECT_EQ(getTracerThreadCount(), 0);
+
+ auto packets = readGpuMemTotalPacketsBlocking(tracingSession.get());
+ EXPECT_EQ(packets.size(), 3);
+
+ const auto& packet0 = packets[0];
+ ASSERT_TRUE(packet0.has_timestamp());
+ ASSERT_TRUE(packet0.has_gpu_mem_total_event());
+ const auto& gpuMemEvent0 = packet0.gpu_mem_total_event();
+ ASSERT_TRUE(gpuMemEvent0.has_pid());
+ const auto& pid0 = gpuMemEvent0.pid();
+ ASSERT_TRUE(gpuMemEvent0.has_size());
+ EXPECT_EQ(gpuMemEvent0.size(), getSizeForPid(pid0));
+ ASSERT_TRUE(gpuMemEvent0.has_gpu_id());
+ EXPECT_EQ(gpuMemEvent0.gpu_id(), getGpuIdForPid(pid0));
+
+ const auto& packet1 = packets[1];
+ ASSERT_TRUE(packet1.has_timestamp());
+ ASSERT_TRUE(packet1.has_gpu_mem_total_event());
+ const auto& gpuMemEvent1 = packet1.gpu_mem_total_event();
+ ASSERT_TRUE(gpuMemEvent1.has_pid());
+ const auto& pid1 = gpuMemEvent1.pid();
+ ASSERT_TRUE(gpuMemEvent1.has_size());
+ EXPECT_EQ(gpuMemEvent1.size(), getSizeForPid(pid1));
+ ASSERT_TRUE(gpuMemEvent1.has_gpu_id());
+ EXPECT_EQ(gpuMemEvent1.gpu_id(), getGpuIdForPid(pid1));
+
+ const auto& packet2 = packets[2];
+ ASSERT_TRUE(packet2.has_timestamp());
+ ASSERT_TRUE(packet2.has_gpu_mem_total_event());
+ const auto& gpuMemEvent2 = packet2.gpu_mem_total_event();
+ ASSERT_TRUE(gpuMemEvent2.has_pid());
+ const auto& pid2 = gpuMemEvent2.pid();
+ ASSERT_TRUE(gpuMemEvent2.has_size());
+ EXPECT_EQ(gpuMemEvent2.size(), getSizeForPid(pid2));
+ ASSERT_TRUE(gpuMemEvent2.has_gpu_id());
+ EXPECT_EQ(gpuMemEvent2.gpu_id(), getGpuIdForPid(pid2));
+}
+
+TEST_F(GpuMemTracerTest, noTracingWithoutGpuMemInitialize) {
+ // Only 1 tracer thread should be existing for test.
+ EXPECT_EQ(getTracerThreadCount(), 1);
+
+ auto tracingSession = mGpuMemTracer->getTracingSessionForTest();
+
+ tracingSession->StartBlocking();
+ // Sleep for a short time to let the tracer thread finish its work
+ sleep(1);
+ tracingSession->StopBlocking();
+
+ // The test tracer thread should have finished its execution by now.
+ EXPECT_EQ(getTracerThreadCount(), 0);
+
+ auto packets = readGpuMemTotalPacketsBlocking(tracingSession.get());
+ EXPECT_EQ(packets.size(), 0);
+}
+} // namespace android
diff --git a/services/gpuservice/tracing/GpuMemTracer.cpp b/services/gpuservice/tracing/GpuMemTracer.cpp
index 6366e1d..44a30ea 100644
--- a/services/gpuservice/tracing/GpuMemTracer.cpp
+++ b/services/gpuservice/tracing/GpuMemTracer.cpp
@@ -44,9 +44,35 @@
args.backends = perfetto::kSystemBackend;
perfetto::Tracing::Initialize(args);
registerDataSource();
- std::thread tracerThread(&GpuMemTracer::threadLoop, this);
+ std::thread tracerThread(&GpuMemTracer::threadLoop, this, true);
pthread_setname_np(tracerThread.native_handle(), "GpuMemTracerThread");
tracerThread.detach();
+ tracerThreadCount++;
+}
+
+void GpuMemTracer::initializeForTest(std::shared_ptr<GpuMem> gpuMem) {
+ mGpuMem = gpuMem;
+ perfetto::TracingInitArgs args;
+ args.backends = perfetto::kInProcessBackend;
+ perfetto::Tracing::Initialize(args);
+ registerDataSource();
+ std::thread tracerThread(&GpuMemTracer::threadLoop, this, false);
+ pthread_setname_np(tracerThread.native_handle(), "GpuMemTracerThreadForTest");
+ tracerThread.detach();
+ tracerThreadCount++;
+}
+
+// Each tracing session can be used for a single block of Start -> Stop.
+std::unique_ptr<perfetto::TracingSession> GpuMemTracer::getTracingSessionForTest() {
+ perfetto::TraceConfig cfg;
+ cfg.set_duration_ms(500);
+ cfg.add_buffers()->set_size_kb(1024);
+ auto* ds_cfg = cfg.add_data_sources()->mutable_config();
+ ds_cfg->set_name(GpuMemTracer::kGpuMemDataSource);
+
+ auto tracingSession = perfetto::Tracing::NewTrace(perfetto::kInProcessBackend);
+ tracingSession->Setup(cfg);
+ return tracingSession;
}
void GpuMemTracer::registerDataSource() {
@@ -55,8 +81,8 @@
GpuMemDataSource::Register(dsd);
}
-void GpuMemTracer::threadLoop() {
- while (true) {
+void GpuMemTracer::threadLoop(bool infiniteLoop) {
+ do {
{
std::unique_lock<std::mutex> lock(GpuMemTracer::sTraceMutex);
while (!sTraceStarted) {
@@ -68,7 +94,11 @@
std::lock_guard<std::mutex> lock(GpuMemTracer::sTraceMutex);
sTraceStarted = false;
}
- }
+ } while (infiniteLoop);
+
+ // Thread loop is exiting. Reduce the tracerThreadCount to reflect the number of active threads
+ // in the wait loop.
+ tracerThreadCount--;
}
void GpuMemTracer::traceInitialCounters() {
diff --git a/services/gpuservice/tracing/include/tracing/GpuMemTracer.h b/services/gpuservice/tracing/include/tracing/GpuMemTracer.h
index 40deb4c..ae871f1 100644
--- a/services/gpuservice/tracing/include/tracing/GpuMemTracer.h
+++ b/services/gpuservice/tracing/include/tracing/GpuMemTracer.h
@@ -20,6 +20,10 @@
#include <mutex>
+namespace perfetto::protos {
+class TracePacket;
+}
+
namespace android {
class GpuMem;
@@ -45,16 +49,37 @@
// perfetto::kInProcessBackend in tests.
void registerDataSource();
+ // TODO(b/175904796): Refactor gpuservice lib to include perfetto lib and move the test
+ // functions into the unittests.
+ // Functions only used for testing with in-process backend. These functions require the static
+ // perfetto lib to be linked. If the tests have a perfetto linked, while libgpumemtracer.so also
+ // has one linked, they will both use different static states maintained in perfetto. Since the
+ // static perfetto states are not shared, tracing sessions created in the unit test are not
+ // recognized by GpuMemTracer. As a result, we cannot use any of the perfetto functions from
+ // this class, which defeats the purpose of the unit test. To solve this, we restrict all
+ // tracing functionality to this class, while the unit test validates the data.
+ // Sets up the perfetto in-process backend and calls into registerDataSource.
+ void initializeForTest(std::shared_ptr<GpuMem>);
+ // Creates a tracing session with in process backend, for testing.
+ std::unique_ptr<perfetto::TracingSession> getTracingSessionForTest();
+ // Read and filter the gpu memory packets from the created trace.
+ std::vector<perfetto::protos::TracePacket> readGpuMemTotalPacketsForTestBlocking(
+ perfetto::TracingSession* tracingSession);
+
static constexpr char kGpuMemDataSource[] = "android.gpu.memory";
static std::condition_variable sCondition;
static std::mutex sTraceMutex;
static bool sTraceStarted;
private:
- void traceInitialCounters();
- void threadLoop();
+ // Friend class for testing
+ friend class GpuMemTracerTest;
+ void threadLoop(bool infiniteLoop);
+ void traceInitialCounters();
std::shared_ptr<GpuMem> mGpuMem;
+ // Count of how many tracer threads are currently active. Useful for testing.
+ std::atomic<int32_t> tracerThreadCount = 0;
};
} // namespace android
diff --git a/services/inputflinger/reader/mapper/TouchInputMapper.cpp b/services/inputflinger/reader/mapper/TouchInputMapper.cpp
index 0b541fb..cbdb1d0 100644
--- a/services/inputflinger/reader/mapper/TouchInputMapper.cpp
+++ b/services/inputflinger/reader/mapper/TouchInputMapper.cpp
@@ -3673,6 +3673,9 @@
const float xScaled = float(x - mRawPointerAxes.x.minValue) * mXScale;
const float yScaled = float(y - mRawPointerAxes.y.minValue) * mYScale;
+ const float xScaledMax = float(mRawPointerAxes.x.maxValue - x) * mXScale;
+ const float yScaledMax = float(mRawPointerAxes.y.maxValue - y) * mYScale;
+
// Rotate to surface coordinate.
// 0 - no swap and reverse.
// 90 - swap x/y and reverse y.
@@ -3684,16 +3687,16 @@
y = yScaled + mYTranslate;
break;
case DISPLAY_ORIENTATION_90:
- y = mSurfaceRight - xScaled;
+ y = xScaledMax - (mRawSurfaceWidth - mSurfaceRight);
x = yScaled + mYTranslate;
break;
case DISPLAY_ORIENTATION_180:
- x = mSurfaceRight - xScaled;
- y = mSurfaceBottom - yScaled;
+ x = xScaledMax - (mRawSurfaceWidth - mSurfaceRight);
+ y = yScaledMax - (mRawSurfaceHeight - mSurfaceBottom);
break;
case DISPLAY_ORIENTATION_270:
y = xScaled + mXTranslate;
- x = mSurfaceBottom - yScaled;
+ x = yScaledMax - (mRawSurfaceHeight - mSurfaceBottom);
break;
default:
assert(false);
diff --git a/services/inputflinger/tests/InputReader_test.cpp b/services/inputflinger/tests/InputReader_test.cpp
index 58f83b5..e9bb169 100644
--- a/services/inputflinger/tests/InputReader_test.cpp
+++ b/services/inputflinger/tests/InputReader_test.cpp
@@ -7449,8 +7449,8 @@
configureDevice(InputReaderConfiguration::CHANGE_DISPLAY_INFO);
}
- void processPositionAndVerify(MultiTouchInputMapper& mapper, int32_t xInside, int32_t yInside,
- int32_t xOutside, int32_t yOutside, int32_t xExpected,
+ void processPositionAndVerify(MultiTouchInputMapper& mapper, int32_t xOutside, int32_t yOutside,
+ int32_t xInside, int32_t yInside, int32_t xExpected,
int32_t yExpected) {
// touch on outside area should not work.
processPosition(mapper, toRawX(xOutside), toRawY(yOutside));
@@ -7532,6 +7532,34 @@
processPositionAndVerify(mapper, x - 1, y, x + 1, y, xExpected, yExpected);
}
+TEST_F(MultiTouchInputMapperTest_SurfaceRange, Viewports_SurfaceRange_Corner) {
+ addConfigurationProperty("touch.deviceType", "touchScreen");
+ prepareDisplay(DISPLAY_ORIENTATION_0);
+ prepareAxes(POSITION);
+ MultiTouchInputMapper& mapper = addMapperAndConfigure<MultiTouchInputMapper>();
+
+ const int32_t x = 0;
+ const int32_t y = 0;
+
+ const int32_t xExpected = x;
+ const int32_t yExpected = y;
+ processPositionAndVerify(mapper, x - 1, y, x, y, xExpected, yExpected);
+
+ clearViewports();
+ prepareDisplay(DISPLAY_ORIENTATION_90);
+ // expect x/y = swap x/y then reverse y.
+ const int32_t xExpected90 = y;
+ const int32_t yExpected90 = DISPLAY_WIDTH - 1;
+ processPositionAndVerify(mapper, x - 1, y, x, y, xExpected90, yExpected90);
+
+ clearViewports();
+ prepareDisplay(DISPLAY_ORIENTATION_270);
+ // expect x/y = swap x/y then reverse x.
+ const int32_t xExpected270 = DISPLAY_HEIGHT - 1;
+ const int32_t yExpected270 = x;
+ processPositionAndVerify(mapper, x - 1, y, x, y, xExpected270, yExpected270);
+}
+
TEST_F(MultiTouchInputMapperTest, Process_TouchpadCapture) {
// we need a pointer controller for mouse mode of touchpad (start pointer at 0,0)
std::shared_ptr<FakePointerController> fakePointerController =
diff --git a/services/surfaceflinger/Android.bp b/services/surfaceflinger/Android.bp
index bdd04db..2734ee9 100644
--- a/services/surfaceflinger/Android.bp
+++ b/services/surfaceflinger/Android.bp
@@ -20,7 +20,6 @@
"-DEGL_EGLEXT_PROTOTYPES",
],
shared_libs: [
- "android.frameworks.vr.composer@2.0",
"android.hardware.configstore-utils",
"android.hardware.configstore@1.0",
"android.hardware.configstore@1.1",
@@ -58,21 +57,12 @@
"libutils",
"libSurfaceFlingerProp",
],
- // VrComposer is not used when building surfaceflinger for vendors
- target: {
- vendor: {
- exclude_shared_libs: [
- "android.frameworks.vr.composer@2.0",
- ],
- },
- },
static_libs: [
"libcompositionengine",
"libperfetto_client_experimental",
"librenderengine",
"libserviceutils",
"libtrace_proto",
- "libvrflinger",
],
header_libs: [
"android.hardware.graphics.composer@2.1-command-buffer",
@@ -198,17 +188,6 @@
// can be easily replaced.
"SurfaceFlingerFactory.cpp",
],
- cflags: [
- "-DUSE_VR_COMPOSER=1",
- ],
- // VrComposer is not used when building surfaceflinger for vendors
- target: {
- vendor: {
- cflags: [
- "-DUSE_VR_COMPOSER=0",
- ],
- },
- },
logtags: ["EventLog/EventLogTags.logtags"],
}
diff --git a/services/surfaceflinger/BufferLayer.cpp b/services/surfaceflinger/BufferLayer.cpp
index f0b0200..df26a3d 100644
--- a/services/surfaceflinger/BufferLayer.cpp
+++ b/services/surfaceflinger/BufferLayer.cpp
@@ -176,7 +176,14 @@
if (!holes.isEmpty()) {
targetSettings.clearRegion.orSelf(holes);
}
- return std::nullopt;
+
+ if (mSidebandStream != nullptr) {
+ // For surfaceview of tv sideband, there is no activeBuffer
+ // in bufferqueue, we need return LayerSettings.
+ return result;
+ } else {
+ return std::nullopt;
+ }
}
bool blackOutLayer = (isProtected() && !targetSettings.supportsProtectedContent) ||
(isSecure() && !targetSettings.isSecure);
diff --git a/services/surfaceflinger/CompositionEngine/Android.bp b/services/surfaceflinger/CompositionEngine/Android.bp
index b37ca33..4863297 100644
--- a/services/surfaceflinger/CompositionEngine/Android.bp
+++ b/services/surfaceflinger/CompositionEngine/Android.bp
@@ -6,7 +6,6 @@
"-DATRACE_TAG=ATRACE_TAG_GRAPHICS",
],
shared_libs: [
- "android.frameworks.vr.composer@2.0",
"android.hardware.graphics.allocator@2.0",
"android.hardware.graphics.composer@2.1",
"android.hardware.graphics.composer@2.2",
diff --git a/services/surfaceflinger/CompositionEngine/include/compositionengine/LayerFECompositionState.h b/services/surfaceflinger/CompositionEngine/include/compositionengine/LayerFECompositionState.h
index b4ed92f..77400eb 100644
--- a/services/surfaceflinger/CompositionEngine/include/compositionengine/LayerFECompositionState.h
+++ b/services/surfaceflinger/CompositionEngine/include/compositionengine/LayerFECompositionState.h
@@ -130,16 +130,6 @@
Rect geomContentCrop;
Rect geomCrop;
- /*
- * Extra metadata
- */
-
- // The type for this layer
- int type{0};
-
- // The appId for this layer
- int appId{0};
-
GenericLayerMetadataMap metadata;
/*
diff --git a/services/surfaceflinger/CompositionEngine/src/LayerFECompositionState.cpp b/services/surfaceflinger/CompositionEngine/src/LayerFECompositionState.cpp
index 02e3a45..1338538 100644
--- a/services/surfaceflinger/CompositionEngine/src/LayerFECompositionState.cpp
+++ b/services/surfaceflinger/CompositionEngine/src/LayerFECompositionState.cpp
@@ -75,10 +75,6 @@
dumpVal(out, "alpha", alpha);
dumpVal(out, "backgroundBlurRadius", backgroundBlurRadius);
- out.append("\n ");
- dumpVal(out, "type", type);
- dumpVal(out, "appId", appId);
-
if (!metadata.empty()) {
out.append("\n metadata {");
for (const auto& [key, entry] : metadata) {
diff --git a/services/surfaceflinger/CompositionEngine/src/Output.cpp b/services/surfaceflinger/CompositionEngine/src/Output.cpp
index 34dc536..d3247e1 100644
--- a/services/surfaceflinger/CompositionEngine/src/Output.cpp
+++ b/services/surfaceflinger/CompositionEngine/src/Output.cpp
@@ -835,6 +835,8 @@
needsProtected == renderEngine.isProtected()) {
mRenderSurface->setProtected(needsProtected);
}
+ } else if (!outputState.isSecure && renderEngine.isProtected()) {
+ renderEngine.useProtectedContext(false);
}
base::unique_fd fd;
diff --git a/services/surfaceflinger/CompositionEngine/src/OutputLayer.cpp b/services/surfaceflinger/CompositionEngine/src/OutputLayer.cpp
index 1faf775..3aed7f5 100644
--- a/services/surfaceflinger/CompositionEngine/src/OutputLayer.cpp
+++ b/services/surfaceflinger/CompositionEngine/src/OutputLayer.cpp
@@ -402,13 +402,6 @@
outputIndependentState.alpha, to_string(error).c_str(), static_cast<int32_t>(error));
}
- if (auto error = hwcLayer->setInfo(static_cast<uint32_t>(outputIndependentState.type),
- static_cast<uint32_t>(outputIndependentState.appId));
- error != hal::Error::NONE) {
- ALOGE("[%s] Failed to set info %s (%d)", getLayerFE().getDebugName(),
- to_string(error).c_str(), static_cast<int32_t>(error));
- }
-
for (const auto& [name, entry] : outputIndependentState.metadata) {
if (auto error = hwcLayer->setLayerGenericMetadata(name, entry.mandatory, entry.value);
error != hal::Error::NONE) {
diff --git a/services/surfaceflinger/CompositionEngine/tests/DisplayTest.cpp b/services/surfaceflinger/CompositionEngine/tests/DisplayTest.cpp
index 09f37fb..29fd4b9 100644
--- a/services/surfaceflinger/CompositionEngine/tests/DisplayTest.cpp
+++ b/services/surfaceflinger/CompositionEngine/tests/DisplayTest.cpp
@@ -159,6 +159,7 @@
EXPECT_CALL(mCompositionEngine, getHwComposer()).WillRepeatedly(ReturnRef(mHwComposer));
EXPECT_CALL(mCompositionEngine, getRenderEngine()).WillRepeatedly(ReturnRef(mRenderEngine));
EXPECT_CALL(mRenderEngine, supportsProtectedContent()).WillRepeatedly(Return(false));
+ EXPECT_CALL(mRenderEngine, isProtected()).WillRepeatedly(Return(false));
}
DisplayCreationArgs getDisplayCreationArgsForPhysicalHWCDisplay() {
diff --git a/services/surfaceflinger/CompositionEngine/tests/MockHWC2.h b/services/surfaceflinger/CompositionEngine/tests/MockHWC2.h
index d21b97e..87911cc 100644
--- a/services/surfaceflinger/CompositionEngine/tests/MockHWC2.h
+++ b/services/surfaceflinger/CompositionEngine/tests/MockHWC2.h
@@ -66,7 +66,6 @@
MOCK_METHOD1(setTransform, Error(hal::Transform));
MOCK_METHOD1(setVisibleRegion, Error(const android::Region&));
MOCK_METHOD1(setZOrder, Error(uint32_t));
- MOCK_METHOD2(setInfo, Error(uint32_t, uint32_t));
MOCK_METHOD1(setColorTransform, Error(const android::mat4&));
MOCK_METHOD3(setLayerGenericMetadata,
diff --git a/services/surfaceflinger/CompositionEngine/tests/OutputLayerTest.cpp b/services/surfaceflinger/CompositionEngine/tests/OutputLayerTest.cpp
index 020f93a..c32ef11 100644
--- a/services/surfaceflinger/CompositionEngine/tests/OutputLayerTest.cpp
+++ b/services/surfaceflinger/CompositionEngine/tests/OutputLayerTest.cpp
@@ -676,8 +676,6 @@
static constexpr Hwc2::IComposerClient::BlendMode kBlendMode =
static_cast<Hwc2::IComposerClient::BlendMode>(41);
static constexpr float kAlpha = 51.f;
- static constexpr uint32_t kType = 61u;
- static constexpr uint32_t kAppId = 62u;
static constexpr ui::Dataspace kDataspace = static_cast<ui::Dataspace>(71);
static constexpr int kSupportedPerFrameMetadata = 101;
static constexpr int kExpectedHwcSlot = 0;
@@ -711,8 +709,6 @@
mLayerFEState.blendMode = kBlendMode;
mLayerFEState.alpha = kAlpha;
- mLayerFEState.type = kType;
- mLayerFEState.appId = kAppId;
mLayerFEState.colorTransform = kColorTransform;
mLayerFEState.color = kColor;
mLayerFEState.surfaceDamage = kSurfaceDamage;
@@ -746,7 +742,6 @@
EXPECT_CALL(*mHwcLayer, setBlendMode(kBlendMode)).WillOnce(Return(kError));
EXPECT_CALL(*mHwcLayer, setPlaneAlpha(kAlpha)).WillOnce(Return(kError));
- EXPECT_CALL(*mHwcLayer, setInfo(kType, kAppId)).WillOnce(Return(kError));
}
void expectPerFrameCommonCalls(SimulateUnsupported unsupported = SimulateUnsupported::None) {
diff --git a/services/surfaceflinger/CompositionEngine/tests/OutputTest.cpp b/services/surfaceflinger/CompositionEngine/tests/OutputTest.cpp
index 7a06400..8f76afa 100644
--- a/services/surfaceflinger/CompositionEngine/tests/OutputTest.cpp
+++ b/services/surfaceflinger/CompositionEngine/tests/OutputTest.cpp
@@ -2871,6 +2871,7 @@
mOutput.mState.usesClientComposition = false;
EXPECT_CALL(mRenderEngine, supportsProtectedContent()).WillRepeatedly(Return(false));
+ EXPECT_CALL(mRenderEngine, isProtected()).WillRepeatedly(Return(false));
EXPECT_CALL(mOutput, setExpensiveRenderingExpected(false));
@@ -2883,6 +2884,7 @@
mOutput.mState.flipClientTarget = true;
EXPECT_CALL(mRenderEngine, supportsProtectedContent()).WillRepeatedly(Return(false));
+ EXPECT_CALL(mRenderEngine, isProtected()).WillRepeatedly(Return(false));
EXPECT_CALL(*mRenderSurface, dequeueBuffer(_)).WillOnce(Return(mOutputBuffer));
EXPECT_CALL(mOutput, setExpensiveRenderingExpected(false));
@@ -2892,6 +2894,7 @@
TEST_F(OutputComposeSurfacesTest, doesMinimalWorkIfDequeueBufferFailsForClientComposition) {
EXPECT_CALL(mRenderEngine, supportsProtectedContent()).WillRepeatedly(Return(false));
+ EXPECT_CALL(mRenderEngine, isProtected()).WillRepeatedly(Return(false));
EXPECT_CALL(*mRenderSurface, dequeueBuffer(_)).WillOnce(Return(nullptr));
@@ -2904,6 +2907,7 @@
mOutput.mState.flipClientTarget = true;
EXPECT_CALL(mRenderEngine, supportsProtectedContent()).WillRepeatedly(Return(false));
+ EXPECT_CALL(mRenderEngine, isProtected()).WillRepeatedly(Return(false));
EXPECT_CALL(*mRenderSurface, dequeueBuffer(_)).WillOnce(Return(nullptr));
@@ -2914,6 +2918,7 @@
EXPECT_CALL(mOutput, getSkipColorTransform()).WillRepeatedly(Return(false));
EXPECT_CALL(*mDisplayColorProfile, hasWideColorGamut()).WillRepeatedly(Return(true));
EXPECT_CALL(mRenderEngine, supportsProtectedContent()).WillRepeatedly(Return(false));
+ EXPECT_CALL(mRenderEngine, isProtected()).WillRepeatedly(Return(false));
EXPECT_CALL(mOutput, generateClientCompositionRequests(_, _, kDefaultOutputDataspace))
.WillRepeatedly(Return(std::vector<LayerFE::LayerSettings>{}));
EXPECT_CALL(mOutput, appendRegionFlashRequests(RegionEq(kDebugRegion), _))
@@ -2936,6 +2941,7 @@
EXPECT_CALL(mOutput, getSkipColorTransform()).WillRepeatedly(Return(false));
EXPECT_CALL(*mDisplayColorProfile, hasWideColorGamut()).WillRepeatedly(Return(true));
EXPECT_CALL(mRenderEngine, supportsProtectedContent()).WillRepeatedly(Return(false));
+ EXPECT_CALL(mRenderEngine, isProtected()).WillRepeatedly(Return(false));
EXPECT_CALL(mOutput, generateClientCompositionRequests(_, _, kDefaultOutputDataspace))
.WillRepeatedly(Return(std::vector<LayerFE::LayerSettings>{r1}));
EXPECT_CALL(mOutput, appendRegionFlashRequests(RegionEq(kDebugRegion), _))
@@ -2963,6 +2969,7 @@
EXPECT_CALL(mOutput, getSkipColorTransform()).WillRepeatedly(Return(false));
EXPECT_CALL(*mDisplayColorProfile, hasWideColorGamut()).WillRepeatedly(Return(true));
EXPECT_CALL(mRenderEngine, supportsProtectedContent()).WillRepeatedly(Return(false));
+ EXPECT_CALL(mRenderEngine, isProtected()).WillRepeatedly(Return(false));
EXPECT_CALL(mOutput, generateClientCompositionRequests(_, _, kDefaultOutputDataspace))
.WillRepeatedly(Return(std::vector<LayerFE::LayerSettings>{r1, r2}));
EXPECT_CALL(mOutput, appendRegionFlashRequests(RegionEq(kDebugRegion), _))
@@ -2991,6 +2998,7 @@
EXPECT_CALL(mOutput, getSkipColorTransform()).WillRepeatedly(Return(false));
EXPECT_CALL(*mDisplayColorProfile, hasWideColorGamut()).WillRepeatedly(Return(true));
EXPECT_CALL(mRenderEngine, supportsProtectedContent()).WillRepeatedly(Return(false));
+ EXPECT_CALL(mRenderEngine, isProtected()).WillRepeatedly(Return(false));
EXPECT_CALL(mOutput, generateClientCompositionRequests(_, _, kDefaultOutputDataspace))
.WillRepeatedly(Return(std::vector<LayerFE::LayerSettings>{r1, r2}));
EXPECT_CALL(mOutput, appendRegionFlashRequests(RegionEq(kDebugRegion), _))
@@ -3019,6 +3027,7 @@
EXPECT_CALL(mOutput, getSkipColorTransform()).WillRepeatedly(Return(false));
EXPECT_CALL(*mDisplayColorProfile, hasWideColorGamut()).WillRepeatedly(Return(true));
EXPECT_CALL(mRenderEngine, supportsProtectedContent()).WillRepeatedly(Return(false));
+ EXPECT_CALL(mRenderEngine, isProtected()).WillRepeatedly(Return(false));
EXPECT_CALL(mOutput, generateClientCompositionRequests(_, _, kDefaultOutputDataspace))
.WillRepeatedly(Return(std::vector<LayerFE::LayerSettings>{r1, r2}));
EXPECT_CALL(mOutput, appendRegionFlashRequests(RegionEq(kDebugRegion), _))
@@ -3050,6 +3059,7 @@
EXPECT_CALL(mOutput, getSkipColorTransform()).WillRepeatedly(Return(false));
EXPECT_CALL(*mDisplayColorProfile, hasWideColorGamut()).WillRepeatedly(Return(true));
EXPECT_CALL(mRenderEngine, supportsProtectedContent()).WillRepeatedly(Return(false));
+ EXPECT_CALL(mRenderEngine, isProtected()).WillRepeatedly(Return(false));
EXPECT_CALL(mOutput, generateClientCompositionRequests(_, _, kDefaultOutputDataspace))
.WillOnce(Return(std::vector<LayerFE::LayerSettings>{r1, r2}))
.WillOnce(Return(std::vector<LayerFE::LayerSettings>{r1, r3}));
@@ -3072,6 +3082,7 @@
struct OutputComposeSurfacesTest_UsesExpectedDisplaySettings : public OutputComposeSurfacesTest {
OutputComposeSurfacesTest_UsesExpectedDisplaySettings() {
EXPECT_CALL(mRenderEngine, supportsProtectedContent()).WillRepeatedly(Return(false));
+ EXPECT_CALL(mRenderEngine, isProtected()).WillRepeatedly(Return(false));
EXPECT_CALL(mOutput, generateClientCompositionRequests(_, _, kDefaultOutputDataspace))
.WillRepeatedly(Return(std::vector<LayerFE::LayerSettings>{}));
EXPECT_CALL(mOutput, appendRegionFlashRequests(RegionEq(kDebugRegion), _))
@@ -3219,6 +3230,8 @@
mOutput.mState.isSecure = false;
mLayer2.mLayerFEState.hasProtectedContent = true;
EXPECT_CALL(mRenderEngine, supportsProtectedContent()).WillRepeatedly(Return(true));
+ EXPECT_CALL(mRenderEngine, isProtected).WillOnce(Return(true));
+ EXPECT_CALL(mRenderEngine, useProtectedContext(false)).WillOnce(Return(true));
mOutput.composeSurfaces(kDebugRegion, kDefaultRefreshArgs);
}
@@ -3311,6 +3324,7 @@
EXPECT_CALL(mOutput, getSkipColorTransform()).WillRepeatedly(Return(false));
EXPECT_CALL(*mDisplayColorProfile, hasWideColorGamut()).WillRepeatedly(Return(true));
EXPECT_CALL(mRenderEngine, supportsProtectedContent()).WillRepeatedly(Return(false));
+ EXPECT_CALL(mRenderEngine, isProtected()).WillRepeatedly(Return(false));
EXPECT_CALL(mOutput, appendRegionFlashRequests(RegionEq(kDebugRegion), _))
.WillRepeatedly(Return());
EXPECT_CALL(*mRenderSurface, dequeueBuffer(_)).WillRepeatedly(Return(mOutputBuffer));
diff --git a/services/surfaceflinger/DisplayHardware/ComposerHal.cpp b/services/surfaceflinger/DisplayHardware/ComposerHal.cpp
index a3f1b52..1bf43da 100644
--- a/services/surfaceflinger/DisplayHardware/ComposerHal.cpp
+++ b/services/surfaceflinger/DisplayHardware/ComposerHal.cpp
@@ -117,63 +117,7 @@
namespace impl {
-#if defined(USE_VR_COMPOSER) && USE_VR_COMPOSER
-Composer::CommandWriter::CommandWriter(uint32_t initialMaxSize)
- : CommandWriterBase(initialMaxSize) {}
-
-Composer::CommandWriter::~CommandWriter()
-{
-}
-
-void Composer::CommandWriter::setLayerInfo(uint32_t type, uint32_t appId)
-{
- constexpr uint16_t kSetLayerInfoLength = 2;
- beginCommand(static_cast<V2_1::IComposerClient::Command>(
- IVrComposerClient::VrCommand::SET_LAYER_INFO),
- kSetLayerInfoLength);
- write(type);
- write(appId);
- endCommand();
-}
-
-void Composer::CommandWriter::setClientTargetMetadata(
- const IVrComposerClient::BufferMetadata& metadata)
-{
- constexpr uint16_t kSetClientTargetMetadataLength = 7;
- beginCommand(static_cast<V2_1::IComposerClient::Command>(
- IVrComposerClient::VrCommand::SET_CLIENT_TARGET_METADATA),
- kSetClientTargetMetadataLength);
- writeBufferMetadata(metadata);
- endCommand();
-}
-
-void Composer::CommandWriter::setLayerBufferMetadata(
- const IVrComposerClient::BufferMetadata& metadata)
-{
- constexpr uint16_t kSetLayerBufferMetadataLength = 7;
- beginCommand(static_cast<V2_1::IComposerClient::Command>(
- IVrComposerClient::VrCommand::SET_LAYER_BUFFER_METADATA),
- kSetLayerBufferMetadataLength);
- writeBufferMetadata(metadata);
- endCommand();
-}
-
-void Composer::CommandWriter::writeBufferMetadata(
- const IVrComposerClient::BufferMetadata& metadata)
-{
- write(metadata.width);
- write(metadata.height);
- write(metadata.stride);
- write(metadata.layerCount);
- writeSigned(static_cast<int32_t>(metadata.format));
- write64(metadata.usage);
-}
-#endif // defined(USE_VR_COMPOSER) && USE_VR_COMPOSER
-
-Composer::Composer(const std::string& serviceName)
- : mWriter(kWriterInitialSize),
- mIsUsingVrComposer(serviceName == std::string("vr"))
-{
+Composer::Composer(const std::string& serviceName) : mWriter(kWriterInitialSize) {
mComposer = V2_1::IComposer::getService(serviceName);
if (mComposer == nullptr) {
@@ -215,15 +159,6 @@
if (mClient == nullptr) {
LOG_ALWAYS_FATAL("failed to create composer client");
}
-
-#if defined(USE_VR_COMPOSER) && USE_VR_COMPOSER
- if (mIsUsingVrComposer) {
- sp<IVrComposerClient> vrClient = IVrComposerClient::castFrom(mClient);
- if (vrClient == nullptr) {
- LOG_ALWAYS_FATAL("failed to create vr composer client");
- }
- }
-#endif // defined(USE_VR_COMPOSER) && USE_VR_COMPOSER
}
Composer::~Composer() = default;
@@ -262,10 +197,6 @@
}
}
-bool Composer::isRemote() {
- return mClient->isRemote();
-}
-
void Composer::resetCommands() {
mWriter.reset();
}
@@ -587,20 +518,6 @@
{
mWriter.selectDisplay(display);
-#if defined(USE_VR_COMPOSER) && USE_VR_COMPOSER
- if (mIsUsingVrComposer && target.get()) {
- IVrComposerClient::BufferMetadata metadata = {
- .width = target->getWidth(),
- .height = target->getHeight(),
- .stride = target->getStride(),
- .layerCount = target->getLayerCount(),
- .format = static_cast<types::V1_2::PixelFormat>(target->getPixelFormat()),
- .usage = target->getUsage(),
- };
- mWriter.setClientTargetMetadata(metadata);
- }
-#endif // defined(USE_VR_COMPOSER) && USE_VR_COMPOSER
-
const native_handle_t* handle = nullptr;
if (target.get()) {
handle = target->getNativeBuffer()->handle;
@@ -720,20 +637,6 @@
mWriter.selectDisplay(display);
mWriter.selectLayer(layer);
-#if defined(USE_VR_COMPOSER) && USE_VR_COMPOSER
- if (mIsUsingVrComposer && buffer.get()) {
- IVrComposerClient::BufferMetadata metadata = {
- .width = buffer->getWidth(),
- .height = buffer->getHeight(),
- .stride = buffer->getStride(),
- .layerCount = buffer->getLayerCount(),
- .format = static_cast<types::V1_2::PixelFormat>(buffer->getPixelFormat()),
- .usage = buffer->getUsage(),
- };
- mWriter.setLayerBufferMetadata(metadata);
- }
-#endif // defined(USE_VR_COMPOSER) && USE_VR_COMPOSER
-
const native_handle_t* handle = nullptr;
if (buffer.get()) {
handle = buffer->getNativeBuffer()->handle;
@@ -850,27 +753,6 @@
return Error::NONE;
}
-#if defined(USE_VR_COMPOSER) && USE_VR_COMPOSER
-Error Composer::setLayerInfo(Display display, Layer layer, uint32_t type,
- uint32_t appId)
-{
- if (mIsUsingVrComposer) {
- mWriter.selectDisplay(display);
- mWriter.selectLayer(layer);
- mWriter.setLayerInfo(type, appId);
- }
- return Error::NONE;
-}
-#else
-Error Composer::setLayerInfo(Display display, Layer layer, uint32_t, uint32_t) {
- if (mIsUsingVrComposer) {
- mWriter.selectDisplay(display);
- mWriter.selectLayer(layer);
- }
- return Error::NONE;
-}
-#endif // defined(USE_VR_COMPOSER) && USE_VR_COMPOSER
-
Error Composer::execute()
{
// prepare input command queue
diff --git a/services/surfaceflinger/DisplayHardware/ComposerHal.h b/services/surfaceflinger/DisplayHardware/ComposerHal.h
index 00ef782..5b66809 100644
--- a/services/surfaceflinger/DisplayHardware/ComposerHal.h
+++ b/services/surfaceflinger/DisplayHardware/ComposerHal.h
@@ -27,9 +27,6 @@
#pragma clang diagnostic push
#pragma clang diagnostic ignored "-Wconversion"
-#if defined(USE_VR_COMPOSER) && USE_VR_COMPOSER
-#include <android/frameworks/vr/composer/2.0/IVrComposerClient.h>
-#endif // defined(USE_VR_COMPOSER) && USE_VR_COMPOSER
#include <android/hardware/graphics/common/1.1/types.h>
#include <android/hardware/graphics/composer/2.4/IComposer.h>
#include <android/hardware/graphics/composer/2.4/IComposerClient.h>
@@ -47,10 +44,6 @@
namespace Hwc2 {
-#if defined(USE_VR_COMPOSER) && USE_VR_COMPOSER
-using frameworks::vr::composer::V2_0::IVrComposerClient;
-#endif // defined(USE_VR_COMPOSER) && USE_VR_COMPOSER
-
namespace types = hardware::graphics::common;
namespace V2_1 = hardware::graphics::composer::V2_1;
@@ -91,11 +84,6 @@
virtual void registerCallback(const sp<IComposerCallback>& callback) = 0;
- // Returns true if the connected composer service is running in a remote
- // process, false otherwise. This will return false if the service is
- // configured in passthrough mode, for example.
- virtual bool isRemote() = 0;
-
// Reset all pending commands in the command buffer. Useful if you want to
// skip a frame but have already queued some commands.
virtual void resetCommands() = 0;
@@ -104,7 +92,6 @@
virtual Error executeCommands() = 0;
virtual uint32_t getMaxVirtualDisplayCount() = 0;
- virtual bool isUsingVrComposer() const = 0;
virtual Error createVirtualDisplay(uint32_t width, uint32_t height, PixelFormat* format,
Display* outDisplay) = 0;
virtual Error destroyVirtualDisplay(Display display) = 0;
@@ -188,7 +175,6 @@
virtual Error setLayerVisibleRegion(Display display, Layer layer,
const std::vector<IComposerClient::Rect>& visible) = 0;
virtual Error setLayerZOrder(Display display, Layer layer, uint32_t z) = 0;
- virtual Error setLayerInfo(Display display, Layer layer, uint32_t type, uint32_t appId) = 0;
// Composer HAL 2.2
virtual Error setLayerPerFrameMetadata(
@@ -344,11 +330,6 @@
void registerCallback(const sp<IComposerCallback>& callback) override;
- // Returns true if the connected composer service is running in a remote
- // process, false otherwise. This will return false if the service is
- // configured in passthrough mode, for example.
- bool isRemote() override;
-
// Reset all pending commands in the command buffer. Useful if you want to
// skip a frame but have already queued some commands.
void resetCommands() override;
@@ -357,7 +338,6 @@
Error executeCommands() override;
uint32_t getMaxVirtualDisplayCount() override;
- bool isUsingVrComposer() const override { return mIsUsingVrComposer; }
Error createVirtualDisplay(uint32_t width, uint32_t height, PixelFormat* format,
Display* outDisplay) override;
Error destroyVirtualDisplay(Display display) override;
@@ -436,7 +416,6 @@
Error setLayerVisibleRegion(Display display, Layer layer,
const std::vector<IComposerClient::Rect>& visible) override;
Error setLayerZOrder(Display display, Layer layer, uint32_t z) override;
- Error setLayerInfo(Display display, Layer layer, uint32_t type, uint32_t appId) override;
// Composer HAL 2.2
Error setLayerPerFrameMetadata(
@@ -490,29 +469,11 @@
IComposerClient::ClientTargetProperty* outClientTargetProperty) override;
private:
-#if defined(USE_VR_COMPOSER) && USE_VR_COMPOSER
- class CommandWriter : public CommandWriterBase {
- public:
- explicit CommandWriter(uint32_t initialMaxSize);
- ~CommandWriter() override;
-
- void setLayerInfo(uint32_t type, uint32_t appId);
- void setClientTargetMetadata(
- const IVrComposerClient::BufferMetadata& metadata);
- void setLayerBufferMetadata(
- const IVrComposerClient::BufferMetadata& metadata);
-
- private:
- void writeBufferMetadata(
- const IVrComposerClient::BufferMetadata& metadata);
- };
-#else
class CommandWriter : public CommandWriterBase {
public:
explicit CommandWriter(uint32_t initialMaxSize) : CommandWriterBase(initialMaxSize) {}
~CommandWriter() override {}
};
-#endif // defined(USE_VR_COMPOSER) && USE_VR_COMPOSER
// Many public functions above simply write a command into the command
// queue to batch the calls. validateDisplay and presentDisplay will call
@@ -531,10 +492,6 @@
64 * 1024 / sizeof(uint32_t) - 16;
CommandWriter mWriter;
CommandReader mReader;
-
- // When true, the we attach to the vr_hwcomposer service instead of the
- // hwcomposer. This allows us to redirect surfaces to 3d surfaces in vr.
- const bool mIsUsingVrComposer;
};
} // namespace impl
diff --git a/services/surfaceflinger/DisplayHardware/HWC2.cpp b/services/surfaceflinger/DisplayHardware/HWC2.cpp
index 08559bd..e6bff04 100644
--- a/services/surfaceflinger/DisplayHardware/HWC2.cpp
+++ b/services/surfaceflinger/DisplayHardware/HWC2.cpp
@@ -977,12 +977,6 @@
return static_cast<Error>(intError);
}
-Error Layer::setInfo(uint32_t type, uint32_t appId)
-{
- auto intError = mComposer.setLayerInfo(mDisplayId, mId, type, appId);
- return static_cast<Error>(intError);
-}
-
// Composer HAL 2.3
Error Layer::setColorTransform(const android::mat4& matrix) {
if (matrix == mColorMatrix) {
diff --git a/services/surfaceflinger/DisplayHardware/HWC2.h b/services/surfaceflinger/DisplayHardware/HWC2.h
index 6819ff4..8bd6ea8 100644
--- a/services/surfaceflinger/DisplayHardware/HWC2.h
+++ b/services/surfaceflinger/DisplayHardware/HWC2.h
@@ -380,7 +380,6 @@
[[clang::warn_unused_result]] virtual hal::Error setVisibleRegion(
const android::Region& region) = 0;
[[clang::warn_unused_result]] virtual hal::Error setZOrder(uint32_t z) = 0;
- [[clang::warn_unused_result]] virtual hal::Error setInfo(uint32_t type, uint32_t appId) = 0;
// Composer HAL 2.3
[[clang::warn_unused_result]] virtual hal::Error setColorTransform(
@@ -422,7 +421,6 @@
hal::Error setTransform(hal::Transform transform) override;
hal::Error setVisibleRegion(const android::Region& region) override;
hal::Error setZOrder(uint32_t z) override;
- hal::Error setInfo(uint32_t type, uint32_t appId) override;
// Composer HAL 2.3
hal::Error setColorTransform(const android::mat4& matrix) override;
diff --git a/services/surfaceflinger/DisplayHardware/HWComposer.cpp b/services/surfaceflinger/DisplayHardware/HWComposer.cpp
index 7a2f0f3..7db9359 100644
--- a/services/surfaceflinger/DisplayHardware/HWComposer.cpp
+++ b/services/surfaceflinger/DisplayHardware/HWComposer.cpp
@@ -815,10 +815,6 @@
});
}
-bool HWComposer::isUsingVrComposer() const {
- return getComposer()->isUsingVrComposer();
-}
-
status_t HWComposer::setAutoLowLatencyMode(DisplayId displayId, bool on) {
RETURN_IF_INVALID_DISPLAY(displayId, BAD_INDEX);
const auto error = mDisplayData[displayId].hwcDisplay->setAutoLowLatencyMode(on);
@@ -883,11 +879,6 @@
bool HWComposer::shouldIgnoreHotplugConnect(hal::HWDisplayId hwcDisplayId,
bool hasDisplayIdentificationData) const {
- if (isUsingVrComposer() && mInternalHwcDisplayId) {
- ALOGE("Ignoring connection of external display %" PRIu64 " in VR mode", hwcDisplayId);
- return true;
- }
-
if (mHasMultiDisplaySupport && !hasDisplayIdentificationData) {
ALOGE("Ignoring connection of display %" PRIu64 " without identification data",
hwcDisplayId);
diff --git a/services/surfaceflinger/DisplayHardware/HWComposer.h b/services/surfaceflinger/DisplayHardware/HWComposer.h
index c355ebd..e05e41a 100644
--- a/services/surfaceflinger/DisplayHardware/HWComposer.h
+++ b/services/surfaceflinger/DisplayHardware/HWComposer.h
@@ -195,8 +195,6 @@
virtual status_t setActiveColorMode(DisplayId displayId, ui::ColorMode mode,
ui::RenderIntent renderIntent) = 0;
- virtual bool isUsingVrComposer() const = 0;
-
// Composer 2.4
virtual DisplayConnectionType getDisplayConnectionType(DisplayId) const = 0;
virtual bool isVsyncPeriodSwitchSupported(DisplayId displayId) const = 0;
@@ -336,8 +334,6 @@
status_t setActiveColorMode(DisplayId displayId, ui::ColorMode mode,
ui::RenderIntent renderIntent) override;
- bool isUsingVrComposer() const override;
-
// Composer 2.4
DisplayConnectionType getDisplayConnectionType(DisplayId) const override;
bool isVsyncPeriodSwitchSupported(DisplayId displayId) const override;
diff --git a/services/surfaceflinger/Layer.cpp b/services/surfaceflinger/Layer.cpp
index 55af849..3282a5a 100644
--- a/services/surfaceflinger/Layer.cpp
+++ b/services/surfaceflinger/Layer.cpp
@@ -492,9 +492,6 @@
compositionState->geomUsesSourceCrop = usesSourceCrop();
compositionState->isSecure = isSecure();
- compositionState->type = type;
- compositionState->appId = appId;
-
compositionState->metadata.clear();
const auto& supportedMetadata = mFlinger->getHwComposer().getSupportedLayerGenericMetadata();
for (const auto& [key, mandatory] : supportedMetadata) {
diff --git a/services/surfaceflinger/Scheduler/VSyncPredictor.cpp b/services/surfaceflinger/Scheduler/VSyncPredictor.cpp
index 61f3fbb..708a5b8 100644
--- a/services/surfaceflinger/Scheduler/VSyncPredictor.cpp
+++ b/services/surfaceflinger/Scheduler/VSyncPredictor.cpp
@@ -78,8 +78,12 @@
if (!validate(timestamp)) {
// VSR could elect to ignore the incongruent timestamp or resetModel(). If ts is ignored,
- // don't insert this ts into mTimestamps ringbuffer.
- if (!mTimestamps.empty()) {
+ // don't insert this ts into mTimestamps ringbuffer. If we are still
+ // in the learning phase we should just clear all timestamps and start
+ // over.
+ if (mTimestamps.size() < kMinimumSamplesForPrediction) {
+ clearTimestamps();
+ } else if (!mTimestamps.empty()) {
mKnownTimestamp =
std::max(timestamp, *std::max_element(mTimestamps.begin(), mTimestamps.end()));
} else {
diff --git a/services/surfaceflinger/StartPropertySetThread.cpp b/services/surfaceflinger/StartPropertySetThread.cpp
index db82772..f42cd53 100644
--- a/services/surfaceflinger/StartPropertySetThread.cpp
+++ b/services/surfaceflinger/StartPropertySetThread.cpp
@@ -31,6 +31,7 @@
property_set(kTimestampProperty, mTimestampPropertyValue ? "1" : "0");
// Clear BootAnimation exit flag
property_set("service.bootanim.exit", "0");
+ property_set("service.bootanim.progress", "0");
// Start BootAnimation if not started
property_set("ctl.start", "bootanim");
// Exit immediately
diff --git a/services/surfaceflinger/SurfaceFlinger.cpp b/services/surfaceflinger/SurfaceFlinger.cpp
index 39f923f..261722d 100644
--- a/services/surfaceflinger/SurfaceFlinger.cpp
+++ b/services/surfaceflinger/SurfaceFlinger.cpp
@@ -46,7 +46,6 @@
#include <cutils/compiler.h>
#include <cutils/properties.h>
#include <dlfcn.h>
-#include <dvr/vr_flinger.h>
#include <errno.h>
#include <gui/BufferQueue.h>
#include <gui/DebugEGLImageTracker.h>
@@ -267,7 +266,6 @@
bool SurfaceFlinger::useHwcForRgbToYuv;
uint64_t SurfaceFlinger::maxVirtualDisplaySize;
bool SurfaceFlinger::hasSyncFramework;
-bool SurfaceFlinger::useVrFlinger;
int64_t SurfaceFlinger::maxFrameBufferAcquiredBuffers;
uint32_t SurfaceFlinger::maxGraphicsWidth;
uint32_t SurfaceFlinger::maxGraphicsHeight;
@@ -332,9 +330,6 @@
maxVirtualDisplaySize = max_virtual_display_dimension(0);
- // Vr flinger is only enabled on Daydream ready devices.
- useVrFlinger = use_vr_flinger(false);
-
maxFrameBufferAcquiredBuffers = max_frame_buffer_acquired_buffers(2);
maxGraphicsWidth = std::max(max_graphics_width(0), 0);
@@ -609,10 +604,6 @@
mInputFlinger = interface_cast<IInputFlinger>(input);
}
- if (mVrFlinger) {
- mVrFlinger->OnBootFinished();
- }
-
// stop boot animation
// formerly we would just kill the process, but we now ask it to exit so it
// can choose where to stop the animation.
@@ -688,9 +679,6 @@
: renderengine::RenderEngine::ContextPriority::MEDIUM)
.build()));
mCompositionEngine->setTimeStats(mTimeStats);
-
- LOG_ALWAYS_FATAL_IF(mVrFlingerRequestsDisplay,
- "Starting with vr flinger active is not currently supported.");
mCompositionEngine->setHwComposer(getFactory().createHWComposer(getBE().mHwcServiceName));
mCompositionEngine->getHwComposer().setConfiguration(this, getBE().mComposerSequenceId);
// Process any initial hotplug and resulting display changes.
@@ -700,30 +688,6 @@
LOG_ALWAYS_FATAL_IF(!getHwComposer().isConnected(*display->getId()),
"Internal display is disconnected.");
- if (useVrFlinger) {
- auto vrFlingerRequestDisplayCallback = [this](bool requestDisplay) {
- // This callback is called from the vr flinger dispatch thread. We
- // need to call signalTransaction(), which requires holding
- // mStateLock when we're not on the main thread. Acquiring
- // mStateLock from the vr flinger dispatch thread might trigger a
- // deadlock in surface flinger (see b/66916578), so post a message
- // to be handled on the main thread instead.
- static_cast<void>(schedule([=] {
- ALOGI("VR request display mode: requestDisplay=%d", requestDisplay);
- mVrFlingerRequestsDisplay = requestDisplay;
- signalTransaction();
- }));
- };
- mVrFlinger = dvr::VrFlinger::Create(getHwComposer().getComposer(),
- getHwComposer()
- .fromPhysicalDisplayId(*display->getId())
- .value_or(0),
- vrFlingerRequestDisplayCallback);
- if (!mVrFlinger) {
- ALOGE("Failed to start vrflinger");
- }
- }
-
// initialize our drawing state
mDrawingState = mCurrentState;
@@ -1700,98 +1664,6 @@
}
}
-void SurfaceFlinger::resetDisplayState() {
- mScheduler->disableHardwareVsync(true);
- // Clear the drawing state so that the logic inside of
- // handleTransactionLocked will fire. It will determine the delta between
- // mCurrentState and mDrawingState and re-apply all changes when we make the
- // transition.
- mDrawingState.displays.clear();
- mDisplays.clear();
-}
-
-void SurfaceFlinger::updateVrFlinger() {
- ATRACE_CALL();
- if (!mVrFlinger)
- return;
- bool vrFlingerRequestsDisplay = mVrFlingerRequestsDisplay;
- if (vrFlingerRequestsDisplay == getHwComposer().isUsingVrComposer()) {
- return;
- }
-
- if (vrFlingerRequestsDisplay && !getHwComposer().getComposer()->isRemote()) {
- ALOGE("Vr flinger is only supported for remote hardware composer"
- " service connections. Ignoring request to transition to vr"
- " flinger.");
- mVrFlingerRequestsDisplay = false;
- return;
- }
-
- Mutex::Autolock _l(mStateLock);
-
- sp<DisplayDevice> display = getDefaultDisplayDeviceLocked();
- LOG_ALWAYS_FATAL_IF(!display);
-
- const hal::PowerMode currentDisplayPowerMode = display->getPowerMode();
-
- // Clear out all the output layers from the composition engine for all
- // displays before destroying the hardware composer interface. This ensures
- // any HWC layers are destroyed through that interface before it becomes
- // invalid.
- for (const auto& [token, displayDevice] : mDisplays) {
- displayDevice->getCompositionDisplay()->clearOutputLayers();
- }
-
- // This DisplayDevice will no longer be relevant once resetDisplayState() is
- // called below. Clear the reference now so we don't accidentally use it
- // later.
- display.clear();
-
- if (!vrFlingerRequestsDisplay) {
- mVrFlinger->SeizeDisplayOwnership();
- }
-
- resetDisplayState();
- // Delete the current instance before creating the new one
- mCompositionEngine->setHwComposer(std::unique_ptr<HWComposer>());
- mCompositionEngine->setHwComposer(getFactory().createHWComposer(
- vrFlingerRequestsDisplay ? "vr" : getBE().mHwcServiceName));
- mCompositionEngine->getHwComposer().setConfiguration(this, ++getBE().mComposerSequenceId);
-
- LOG_ALWAYS_FATAL_IF(!getHwComposer().getComposer()->isRemote(),
- "Switched to non-remote hardware composer");
-
- if (vrFlingerRequestsDisplay) {
- mVrFlinger->GrantDisplayOwnership();
- }
-
- mVisibleRegionsDirty = true;
- invalidateHwcGeometry();
-
- // Re-enable default display.
- display = getDefaultDisplayDeviceLocked();
- LOG_ALWAYS_FATAL_IF(!display);
- setPowerModeInternal(display, currentDisplayPowerMode);
-
- // Reset the timing values to account for the period of the swapped in HWC
- const nsecs_t vsyncPeriod = mRefreshRateConfigs->getCurrentRefreshRate().getVsyncPeriod();
- mAnimFrameTracker.setDisplayRefreshPeriod(vsyncPeriod);
-
- // The present fences returned from vr_hwc are not an accurate
- // representation of vsync times.
- mScheduler->setIgnorePresentFences(getHwComposer().isUsingVrComposer() || !hasSyncFramework);
-
- // Use phase of 0 since phase is not known.
- // Use latency of 0, which will snap to the ideal latency.
- DisplayStatInfo stats{0 /* vsyncTime */, vsyncPeriod};
- setCompositorTimingSnapped(stats, 0);
-
- mScheduler->resyncToHardwareVsync(false, vsyncPeriod);
-
- mRepaintEverything = true;
- setTransactionFlags(eDisplayTransactionNeeded);
-}
-
sp<Fence> SurfaceFlinger::previousFrameFence() {
// We are storing the last 2 present fences. If sf's phase offset is to be
// woken up before the actual vsync but targeting the next vsync, we need to check
@@ -1960,11 +1832,6 @@
}
}
- // Now that we're going to make it to the handleMessageTransaction()
- // call below it's safe to call updateVrFlinger(), which will
- // potentially trigger a display handoff.
- updateVrFlinger();
-
if (mTracingEnabledChanged) {
mTracingEnabled = mTracing.isEnabled();
mTracingEnabledChanged = false;
@@ -2611,7 +2478,7 @@
builder.setIsSecure(state.isSecure);
builder.setLayerStackId(state.layerStack);
builder.setPowerAdvisor(&mPowerAdvisor);
- builder.setUseHwcVirtualDisplays(mUseHwcVirtualDisplays || getHwComposer().isUsingVrComposer());
+ builder.setUseHwcVirtualDisplays(mUseHwcVirtualDisplays);
builder.setName(state.displayName);
const auto compositionDisplay = getCompositionEngine().createDisplay(builder.build());
@@ -4501,12 +4368,9 @@
void SurfaceFlinger::dumpFrameEventsLocked(std::string& result) {
result.append("Layer frame timestamps:\n");
-
- const LayerVector& currentLayers = mCurrentState.layersSortedByZ;
- const size_t count = currentLayers.size();
- for (size_t i=0 ; i<count ; i++) {
- currentLayers[i]->dumpFrameEvents(result);
- }
+ // Traverse all layers to dump frame-events for each layer
+ mCurrentState.traverseInZOrder(
+ [&] (Layer* layer) { layer->dumpFrameEvents(result); });
}
void SurfaceFlinger::dumpBufferingStats(std::string& result) const {
@@ -4835,15 +4699,6 @@
const GraphicBufferAllocator& alloc(GraphicBufferAllocator::get());
alloc.dump(result);
- /*
- * Dump VrFlinger state if in use.
- */
- if (mVrFlingerRequestsDisplay && mVrFlinger) {
- result.append("VrFlinger state:\n");
- result.append(mVrFlinger->Dump());
- result.append("\n");
- }
-
result.append(mTimeStats->miniDump());
result.append("\n");
}
@@ -5226,11 +5081,8 @@
}
return NO_ERROR;
}
- // Is VrFlinger active?
- case 1028: {
- Mutex::Autolock _l(mStateLock);
- reply->writeBool(getHwComposer().isUsingVrComposer());
- return NO_ERROR;
+ case 1028: { // Unused.
+ return NAME_NOT_FOUND;
}
// Set buffer size for SF tracing (value in KB)
case 1029: {
@@ -6209,9 +6061,6 @@
// on the work to remove the table in that bug rather than adding more to
// it.
static const std::unordered_map<std::string, uint32_t> genericLayerMetadataKeyMap{
- // Note: METADATA_OWNER_UID and METADATA_WINDOW_TYPE are officially
- // supported, and exposed via the
- // IVrComposerClient::VrCommand::SET_LAYER_INFO command.
{"org.chromium.arc.V1_0.TaskId", METADATA_TASK_ID},
{"org.chromium.arc.V1_0.CursorInfo", METADATA_MOUSE_CURSOR},
};
diff --git a/services/surfaceflinger/SurfaceFlinger.h b/services/surfaceflinger/SurfaceFlinger.h
index c727574..61bd020 100644
--- a/services/surfaceflinger/SurfaceFlinger.h
+++ b/services/surfaceflinger/SurfaceFlinger.h
@@ -109,10 +109,6 @@
class RenderEngine;
} // namespace renderengine
-namespace dvr {
-class VrFlinger;
-} // namespace dvr
-
enum {
eTransactionNeeded = 0x01,
eTraversalNeeded = 0x02,
@@ -798,8 +794,7 @@
// The following thread safety rules apply when accessing mHwc, either
// directly or via getHwComposer():
//
- // 1. When recreating mHwc, acquire mStateLock. We currently recreate mHwc
- // only when switching into and out of vr. Recreating mHwc must only be
+ // 1. When recreating mHwc, acquire mStateLock. Recreating mHwc must only be
// done on the main thread.
//
// 2. When accessing mHwc on the main thread, it's not necessary to acquire
@@ -983,14 +978,6 @@
void onFrameRateFlexibilityTokenReleased();
- /* ------------------------------------------------------------------------
- * VrFlinger
- */
- void resetDisplayState() REQUIRES(mStateLock);
-
- // Check to see if we should handoff to vr flinger.
- void updateVrFlinger();
-
void updateColorMatrixLocked();
/* ------------------------------------------------------------------------
@@ -1175,9 +1162,6 @@
// to mWindowManager or mInputFlinger
std::atomic<bool> mBootFinished = false;
- std::unique_ptr<dvr::VrFlinger> mVrFlinger;
- std::atomic<bool> mVrFlingerRequestsDisplay = false;
- static bool useVrFlinger;
std::thread::id mMainThreadId = std::this_thread::get_id();
DisplayColorSetting mDisplayColorSetting = DisplayColorSetting::kEnhanced;
diff --git a/services/surfaceflinger/tests/unittests/CompositionTest.cpp b/services/surfaceflinger/tests/unittests/CompositionTest.cpp
index 32d722e..faa619e 100644
--- a/services/surfaceflinger/tests/unittests/CompositionTest.cpp
+++ b/services/surfaceflinger/tests/unittests/CompositionTest.cpp
@@ -577,8 +577,6 @@
.Times(1);
// TODO: Coverage of other values
EXPECT_CALL(*test->mComposer, setLayerZOrder(HWC_DISPLAY, HWC_LAYER, 0u)).Times(1);
- // TODO: Coverage of other values
- EXPECT_CALL(*test->mComposer, setLayerInfo(HWC_DISPLAY, HWC_LAYER, 0u, 0u)).Times(1);
// These expectations retire on saturation as the code path these
// expectations are for appears to make an extra call to them.
diff --git a/services/surfaceflinger/tests/unittests/DisplayTransactionTest.cpp b/services/surfaceflinger/tests/unittests/DisplayTransactionTest.cpp
index 06bdcdc..0323778 100644
--- a/services/surfaceflinger/tests/unittests/DisplayTransactionTest.cpp
+++ b/services/surfaceflinger/tests/unittests/DisplayTransactionTest.cpp
@@ -1298,55 +1298,6 @@
}
/* ------------------------------------------------------------------------
- * SurfaceFlinger::resetDisplayState
- */
-
-TEST_F(DisplayTransactionTest, resetDisplayStateClearsState) {
- using Case = NonHwcVirtualDisplayCase;
-
- // --------------------------------------------------------------------
- // Preconditions
-
- // vsync is enabled and available
- mFlinger.scheduler()->mutablePrimaryHWVsyncEnabled() = true;
- mFlinger.scheduler()->mutableHWVsyncAvailable() = true;
-
- // A display exists
- auto existing = Case::Display::makeFakeExistingDisplayInjector(this);
- existing.inject();
-
- // --------------------------------------------------------------------
- // Call Expectations
-
- // The call disable vsyncs
- EXPECT_CALL(*mEventControlThread, setVsyncEnabled(false)).Times(1);
-
- // The call ends any display resyncs
- EXPECT_CALL(*mPrimaryDispSync, endResync()).Times(1);
-
- // --------------------------------------------------------------------
- // Invocation
-
- mFlinger.resetDisplayState();
-
- // --------------------------------------------------------------------
- // Postconditions
-
- // vsyncs should be off and not available.
- EXPECT_FALSE(mFlinger.scheduler()->mutablePrimaryHWVsyncEnabled());
- EXPECT_FALSE(mFlinger.scheduler()->mutableHWVsyncAvailable());
-
- // The display should have been removed from the display map.
- EXPECT_FALSE(hasDisplayDevice(existing.token()));
-
- // The display should still exist in the current state
- EXPECT_TRUE(hasCurrentDisplayState(existing.token()));
-
- // The display should have been removed from the drawing state
- EXPECT_FALSE(hasDrawingDisplayState(existing.token()));
-}
-
-/* ------------------------------------------------------------------------
* DisplayDevice::GetBestColorMode
*/
class GetBestColorModeTest : public DisplayTransactionTest {
@@ -2019,8 +1970,6 @@
// --------------------------------------------------------------------
// Call Expectations
- EXPECT_CALL(*mComposer, isUsingVrComposer()).WillOnce(Return(false));
-
setupCommonCallExpectationsForConnectProcessing<Case>();
// --------------------------------------------------------------------
@@ -2082,7 +2031,6 @@
// --------------------------------------------------------------------
// Call Expectations
- EXPECT_CALL(*mComposer, isUsingVrComposer()).WillRepeatedly(Return(false));
EXPECT_CALL(*mComposer, getDisplayIdentificationData(Case::Display::HWC_DISPLAY_ID, _, _))
.Times(0);
@@ -2139,20 +2087,9 @@
SetArgPointee<2>(TertiaryDisplay::GET_IDENTIFICATION_DATA()),
Return(Error::NONE)));
- EXPECT_CALL(*mComposer, isUsingVrComposer()).WillRepeatedly(Return(false));
-
ignoresHotplugConnectCommon<SimpleTertiaryDisplayCase>();
}
-TEST_F(HandleTransactionLockedTest, ignoresHotplugConnectIfExternalForVrComposer) {
- // Inject a primary display.
- PrimaryDisplayVariant::injectHwcDisplay(this);
-
- EXPECT_CALL(*mComposer, isUsingVrComposer()).WillRepeatedly(Return(true));
-
- ignoresHotplugConnectCommon<SimpleExternalDisplayCase>();
-}
-
TEST_F(HandleTransactionLockedTest, processesHotplugDisconnectPrimaryDisplay) {
processesHotplugDisconnectCommon<SimplePrimaryDisplayCase>();
}
@@ -2177,8 +2114,6 @@
// --------------------------------------------------------------------
// Call Expectations
- EXPECT_CALL(*mComposer, isUsingVrComposer()).WillRepeatedly(Return(false));
-
setupCommonCallExpectationsForConnectProcessing<Case>();
setupCommonCallExpectationsForDisconnectProcessing<Case>();
@@ -2225,8 +2160,6 @@
// --------------------------------------------------------------------
// Call Expectations
- EXPECT_CALL(*mComposer, isUsingVrComposer()).WillRepeatedly(Return(false));
-
setupCommonCallExpectationsForConnectProcessing<Case>();
setupCommonCallExpectationsForDisconnectProcessing<Case>();
@@ -2385,11 +2318,6 @@
mFlinger.mutableCurrentState().displays.removeItem(existing.token());
// --------------------------------------------------------------------
- // Call Expectations
-
- EXPECT_CALL(*mComposer, isUsingVrComposer()).WillRepeatedly(Return(false));
-
- // --------------------------------------------------------------------
// Invocation
mFlinger.handleTransactionLocked(eDisplayTransactionNeeded);
diff --git a/services/surfaceflinger/tests/unittests/TestableSurfaceFlinger.h b/services/surfaceflinger/tests/unittests/TestableSurfaceFlinger.h
index cbf264d..cd2a482 100644
--- a/services/surfaceflinger/tests/unittests/TestableSurfaceFlinger.h
+++ b/services/surfaceflinger/tests/unittests/TestableSurfaceFlinger.h
@@ -289,8 +289,6 @@
return mFlinger->destroyDisplay(displayToken);
}
- auto resetDisplayState() NO_THREAD_SAFETY_ANALYSIS { return mFlinger->resetDisplayState(); }
-
auto setupNewDisplayDeviceInternal(
const wp<IBinder>& displayToken,
std::shared_ptr<compositionengine::Display> compositionDisplay,
diff --git a/services/surfaceflinger/tests/unittests/VSyncPredictorTest.cpp b/services/surfaceflinger/tests/unittests/VSyncPredictorTest.cpp
index d4cd11d..5e5d51c 100644
--- a/services/surfaceflinger/tests/unittests/VSyncPredictorTest.cpp
+++ b/services/surfaceflinger/tests/unittests/VSyncPredictorTest.cpp
@@ -450,6 +450,20 @@
EXPECT_THAT(intercept, Eq(0));
}
+TEST_F(VSyncPredictorTest, InconsistentVsyncValueIsFlushedEventually) {
+ EXPECT_TRUE(tracker.addVsyncTimestamp(600));
+ EXPECT_TRUE(tracker.needsMoreSamples());
+
+ EXPECT_FALSE(tracker.addVsyncTimestamp(mNow += mPeriod));
+
+ for (auto i = 0u; i < kMinimumSamplesForPrediction; i++) {
+ EXPECT_TRUE(tracker.needsMoreSamples());
+ EXPECT_TRUE(tracker.addVsyncTimestamp(mNow += mPeriod));
+ }
+
+ EXPECT_FALSE(tracker.needsMoreSamples());
+}
+
} // namespace android::scheduler
// TODO(b/129481165): remove the #pragma below and fix conversion issues
diff --git a/services/surfaceflinger/tests/unittests/mock/DisplayHardware/MockComposer.h b/services/surfaceflinger/tests/unittests/mock/DisplayHardware/MockComposer.h
index c2c5072..cd9b87a 100644
--- a/services/surfaceflinger/tests/unittests/mock/DisplayHardware/MockComposer.h
+++ b/services/surfaceflinger/tests/unittests/mock/DisplayHardware/MockComposer.h
@@ -52,11 +52,9 @@
MOCK_METHOD0(getCapabilities, std::vector<IComposer::Capability>());
MOCK_METHOD0(dumpDebugInfo, std::string());
MOCK_METHOD1(registerCallback, void(const sp<IComposerCallback>&));
- MOCK_METHOD0(isRemote, bool());
MOCK_METHOD0(resetCommands, void());
MOCK_METHOD0(executeCommands, Error());
MOCK_METHOD0(getMaxVirtualDisplayCount, uint32_t());
- MOCK_CONST_METHOD0(isUsingVrComposer, bool());
MOCK_METHOD4(createVirtualDisplay, Error(uint32_t, uint32_t, PixelFormat*, Display*));
MOCK_METHOD1(destroyVirtualDisplay, Error(Display));
MOCK_METHOD1(acceptDisplayChanges, Error(Display));
@@ -110,7 +108,6 @@
MOCK_METHOD3(setLayerVisibleRegion,
Error(Display, Layer, const std::vector<IComposerClient::Rect>&));
MOCK_METHOD3(setLayerZOrder, Error(Display, Layer, uint32_t));
- MOCK_METHOD4(setLayerInfo, Error(Display, Layer, uint32_t, uint32_t));
MOCK_METHOD3(getRenderIntents, Error(Display, ColorMode, std::vector<RenderIntent>*));
MOCK_METHOD3(setLayerColorTransform, Error(Display, Layer, const float*));
MOCK_METHOD4(getDisplayedContentSamplingAttributes,