[FR] [simpleperf] ./app_profiler.py --system_wide should work on rooted "user" phones

[mstange]

Description

I have a rooted Android phone.

When I run ./app_profiler.py --system_wide, I get the following output:

simpleperf E cmd_record.cpp:1256] System wide profiling needs root privilege.
Failed to record profiling data.

After debugging, I found out that the scripts have a way to use adb root + adb unroot if the ro.build.type is not user. But my phone's build type is user, and adb root prints adbd cannot run as root in production builds .

However, I can run simpleperf as root manually using adb shell su -c "...".

Could we adjust the scripts so that they do this?

--- /Users/mstange/Downloads/simpleperf_utils.py	2024-06-05 14:57:51
+++ /Users/mstange/.mozbuild/android-ndk-r26c/simpleperf/simpleperf_utils.py	2024-06-05 15:13:13
@@ -336,16 +336,20 @@
             return True
         build_type = self.get_property('ro.build.type')
         if build_type == 'user':
             return False
         self.run(['root'])
         time.sleep(1)
         self.run(['wait-for-device'])
         result, stdoutdata = self.run_and_return_output(['shell', 'whoami'])
+        return result and 'root' in stdoutdata
+
+    def check_su(self) -> bool:
+        result, stdoutdata = self.run_and_return_output(['shell', 'su', '-c', 'whoami'])
         return result and 'root' in stdoutdata
 
     def get_property(self, name: str) -> Optional[str]:
         result, stdoutdata = self.run_and_return_output(['shell', 'getprop', name])
         return stdoutdata.strip() if result else None
 
     def set_property(self, name: str, value: str) -> bool:
         return self.run(['shell', 'setprop', name, value])

--- /Users/mstange/Downloads/app_profiler.py	2024-06-05 14:57:50
+++ /Users/mstange/.mozbuild/android-ndk-r26c/simpleperf/app_profiler.py	2024-06-05 15:14:35
@@ -23,16 +23,17 @@
 
 import logging
 import os
 import os.path
 import re
 import subprocess
 import sys
 import time
+import shlex
 
 from simpleperf_utils import (
     AdbHelper, BaseArgumentParser, bytes_to_str, extant_dir, get_script_dir, get_target_binary_path,
     log_exit, ReadElf, remove, str_to_bytes)
 
 NATIVE_LIBS_DIR_ON_DEVICE = '/data/local/tmp/native_libs/'
 
 SHELL_PS_UID_PATTERN = re.compile(r'USER.*\nu(\d+)_.*')
@@ -190,16 +191,17 @@
 
 class ProfilerBase(object):
     """Base class of all Profilers."""
 
     def __init__(self, args):
         self.args = args
         self.adb = AdbHelper(enable_switch_to_root=not args.disable_adb_root)
         self.is_root_device = self.adb.switch_to_root()
+        self.have_su = self.adb.check_su()
         self.android_version = self.adb.get_android_version()
         if self.android_version < 7:
             log_exit("""app_profiler.py isn't supported on Android < N, please switch to use
                         simpleperf binary directly.""")
         self.device_arch = self.adb.get_device_arch()
         self.record_subproc = None
 
     def profile(self):
@@ -235,17 +237,20 @@
     def start_profiling(self, target_args):
         """Start simpleperf reocrd process on device."""
         args = ['/data/local/tmp/simpleperf', 'record', '-o', '/data/local/tmp/perf.data',
                 self.args.record_options]
         if self.adb.run(['shell', 'ls', NATIVE_LIBS_DIR_ON_DEVICE]):
             args += ['--symfs', NATIVE_LIBS_DIR_ON_DEVICE]
         args += ['--log', self.args.log]
         args += target_args
-        adb_args = [self.adb.adb_path, 'shell'] + args
+        if self.have_su:
+            adb_args = [self.adb.adb_path, 'shell', 'su', '-c', ' '.join(shlex.quote(arg) for arg in args)]
+        else:
+            adb_args = [self.adb.adb_path, 'shell'] + args
         logging.info('run adb cmd: %s' % adb_args)
         self.record_subproc = subprocess.Popen(adb_args)
 
     def wait_profiling(self):
         """Wait until profiling finishes, or stop profiling when user presses Ctrl-C."""
         returncode = None
         try:
             returncode = self.record_subproc.wait()
@@ -266,17 +271,27 @@
         while True:
             (result, _) = self.adb.run_and_return_output(['shell', 'pidof', 'simpleperf'])
             if not result:
                 break
             if not has_killed:
                 has_killed = True
                 self.adb.run_and_return_output(['shell', 'pkill', '-l', '2', 'simpleperf'])
             time.sleep(1)
+        self.make_perf_data_owned_by_user()
 
+    def make_perf_data_owned_by_user(self):
+        if not self.have_su:
+            return
+        result, shell_user = self.adb.run_and_return_output(['shell', 'whoami'])
+        if not result:
+            return
+        logging.info('adb shell user: %s', shell_user)
+        self.adb.run(['shell', 'su', '-c', 'chown', shell_user, '/data/local/tmp/perf.data'])
+
     def collect_profiling_data(self):
         self.adb.check_run_and_return_output(['pull', '/data/local/tmp/perf.data',
                                               self.args.perf_data_path])
         if not self.args.skip_collect_binaries:
             binary_cache_args = [sys.executable,
                                  os.path.join(get_script_dir(), 'binary_cache_builder.py')]
             binary_cache_args += ['-i', self.args.perf_data_path, '--log', self.args.log]
             if self.args.native_lib_dir:

cc @yabinc

[mstange]

Oh, I just learned that su isn't something that's provided by Android, it's provided by the rooting mechanism.

[yabinc]

I think it's fine to modify SystemWideProfiler class in app_profiler.py to support your case "having su, but not adb root".
It would be great if you'd like send a patch for review. Some instructions are in https://android.googlesource.com/platform/system/extras/+/master/simpleperf/doc/README.md#bugs-and-contribution.
Otherwise, I can make the change for you.