Snap for 9068117 from 7d360b3bc2e2ed19e5d7f64fe2e2b20e2dbcdee9 to mainline-neuralnetworks-release
Change-Id: Icc8fddaa768e4d1993f820a5b5fba1d07419f415
diff --git a/libnativeloader/library_namespaces.cpp b/libnativeloader/library_namespaces.cpp
index 96d4dde..f3c93a0 100644
--- a/libnativeloader/library_namespaces.cpp
+++ b/libnativeloader/library_namespaces.cpp
@@ -81,16 +81,22 @@
constexpr const char* kAlwaysPermittedDirectories = "/data:/mnt/expand";
constexpr const char* kVendorLibPath = "/vendor/" LIB;
+// TODO(mast): It's unlikely that both paths are necessary for kProductLibPath
+// below, because they can't be two separate directories - either one has to be
+// a symlink to the other.
constexpr const char* kProductLibPath = "/product/" LIB ":/system/product/" LIB;
+constexpr const char* kSystemLibPath = "/system/" LIB ":/system_ext/" LIB;
-const std::regex kVendorDexPathRegex("(^|:)/vendor/");
+const std::regex kVendorDexPathRegex("(^|:)(/system)?/vendor/");
const std::regex kProductDexPathRegex("(^|:)(/system)?/product/");
+const std::regex kSystemDexPathRegex("(^|:)/system(_ext)?/"); // MUST be tested last.
-// Define origin of APK if it is from vendor partition or product partition
+// Define origin partition of APK
using ApkOrigin = enum {
APK_ORIGIN_DEFAULT = 0,
- APK_ORIGIN_VENDOR = 1,
- APK_ORIGIN_PRODUCT = 2,
+ APK_ORIGIN_VENDOR = 1, // Includes both /vendor and /system/vendor
+ APK_ORIGIN_PRODUCT = 2, // Includes both /product and /system/product
+ APK_ORIGIN_SYSTEM = 3, // Includes both /system and /system_ext but not /system/{vendor,product}
};
jobject GetParentClassLoader(JNIEnv* env, jobject class_loader) {
@@ -113,6 +119,9 @@
apk_origin = APK_ORIGIN_PRODUCT;
}
+ if (apk_origin == APK_ORIGIN_DEFAULT && std::regex_search(dex_path, kSystemDexPathRegex)) {
+ apk_origin = APK_ORIGIN_SYSTEM;
+ }
return apk_origin;
}
@@ -234,7 +243,18 @@
const char* apk_origin_msg = "other apk"; // Only for debug logging.
if (!is_shared) {
- if (apk_origin == APK_ORIGIN_VENDOR) {
+ if (apk_origin == APK_ORIGIN_SYSTEM) {
+ // System apps commonly get shared namespaces and hence don't need this.
+ // In practice it's necessary for shared system libraries (i.e. JARs
+ // rather than actual APKs) that are loaded by ordinary apps which don't
+ // get shared namespaces.
+ apk_origin_msg = "system apk";
+
+ // Give access to all libraries in the system and system_ext partitions
+ // (they can freely access each other's private APIs).
+ library_path = library_path + ":" + kSystemLibPath;
+ permitted_path = permitted_path + ":" + kSystemLibPath;
+ } else if (apk_origin == APK_ORIGIN_VENDOR) {
unbundled_app_origin = APK_ORIGIN_VENDOR;
apk_origin_msg = "unbundled vendor apk";
@@ -288,8 +308,7 @@
// they are to other apps, including those in system, system_ext, and
// product partitions. The reason is that when GSI is used, the system
// partition may get replaced, and then vendor apps may fail. It's fine for
- // product (and system_ext) apps, because those partitions aren't mounted in
- // GSI tests.
+ // product apps, because that partition isn't mounted in GSI tests.
auto libs =
filter_public_libraries(target_sdk_version, uses_libraries, extended_public_libraries());
if (!libs.empty()) {
diff --git a/libnativeloader/test/Android.bp b/libnativeloader/test/Android.bp
index dd5e969..1d3a07a 100644
--- a/libnativeloader/test/Android.bp
+++ b/libnativeloader/test/Android.bp
@@ -30,8 +30,7 @@
}
// This app is just an intermediate container to be able to include the .so
-// library as a java resource in the host test. It's not actually installed or
-// started.
+// library in the host test. It's not actually installed or started.
android_test_helper_app {
name: "library_container_app",
defaults: ["art_module_source_build_java_defaults"],
@@ -40,19 +39,40 @@
jni_libs: ["libnativeloader_testlib"],
}
-java_defaults {
- name: "loadlibrarytest_app_defaults",
- defaults: ["art_module_source_build_java_defaults"],
-
- // TODO(mast): Use old target SDK to avoid filtering on uses_library lists.
- // Figure out what we need to do to make <uses-native-library> work in the
- // test apps so we can use that instead.
- sdk_version: "30",
-
+java_library {
+ name: "loadlibrarytest_test_utils",
static_libs: [
"androidx.test.ext.junit",
"androidx.test.ext.truth",
+ ],
+ srcs: ["src/android/test/lib/TestUtils.java"],
+}
+
+// Test fixture that represents a shared library in /system/framework.
+java_library {
+ name: "libnativeloader_system_shared_lib",
+ installable: true,
+ srcs: ["src/android/test/systemsharedlib/SystemSharedLib.java"],
+}
+
+// Test fixture that represents a shared library in /system_ext/framework.
+java_library {
+ name: "libnativeloader_system_ext_shared_lib",
+ installable: true,
+ srcs: ["src/android/test/systemextsharedlib/SystemExtSharedLib.java"],
+}
+
+java_defaults {
+ name: "loadlibrarytest_app_defaults",
+ defaults: ["art_module_source_build_java_defaults"],
+ static_libs: [
+ "androidx.test.ext.junit",
"androidx.test.rules",
+ "loadlibrarytest_test_utils",
+ ],
+ libs: [
+ "libnativeloader_system_shared_lib",
+ "libnativeloader_system_ext_shared_lib",
],
}
@@ -74,6 +94,7 @@
android_test_helper_app {
name: "loadlibrarytest_system_ext_app",
defaults: ["loadlibrarytest_app_defaults"],
+ system_ext_specific: true,
manifest: "loadlibrarytest_system_ext_app_manifest.xml",
// /system_ext should behave the same as /system, so use the same test class there.
srcs: ["src/android/test/app/SystemAppTest.java"],
@@ -82,6 +103,7 @@
android_test_helper_app {
name: "loadlibrarytest_product_app",
defaults: ["loadlibrarytest_app_defaults"],
+ product_specific: true,
manifest: "loadlibrarytest_product_app_manifest.xml",
srcs: ["src/android/test/app/ProductAppTest.java"],
}
@@ -89,22 +111,37 @@
android_test_helper_app {
name: "loadlibrarytest_vendor_app",
defaults: ["loadlibrarytest_app_defaults"],
+ vendor: true,
manifest: "loadlibrarytest_vendor_app_manifest.xml",
srcs: ["src/android/test/app/VendorAppTest.java"],
}
+// A normal app installed in /data.
+android_test_helper_app {
+ name: "loadlibrarytest_data_app",
+ defaults: ["loadlibrarytest_app_defaults"],
+ manifest: "loadlibrarytest_data_app_manifest.xml",
+ srcs: ["src/android/test/app/DataAppTest.java"],
+}
+
java_test_host {
name: "libnativeloader_e2e_tests",
defaults: ["art_module_source_build_java_defaults"],
srcs: ["src/android/test/hostside/*.java"],
- libs: ["tradefed"],
- java_resources: [
+ libs: [
+ "compatibility-tradefed",
+ "tradefed",
+ ],
+ data: [
":library_container_app",
+ ":libnativeloader_system_shared_lib",
+ ":libnativeloader_system_ext_shared_lib",
":loadlibrarytest_system_priv_app",
":loadlibrarytest_system_app",
":loadlibrarytest_system_ext_app",
":loadlibrarytest_product_app",
":loadlibrarytest_vendor_app",
+ ":loadlibrarytest_data_app",
],
test_config: "libnativeloader_e2e_tests.xml",
test_suites: ["general-tests"],
diff --git a/libnativeloader/test/loadlibrarytest_data_app_manifest.xml b/libnativeloader/test/loadlibrarytest_data_app_manifest.xml
new file mode 100644
index 0000000..2af0af4
--- /dev/null
+++ b/libnativeloader/test/loadlibrarytest_data_app_manifest.xml
@@ -0,0 +1,33 @@
+<?xml version="1.0" encoding="utf-8"?>
+<!--
+ * Copyright (C) 2022 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.
+ -->
+
+<manifest xmlns:android="http://schemas.android.com/apk/res/android"
+ package="android.test.app.data">
+ <instrumentation android:name="androidx.test.runner.AndroidJUnitRunner"
+ android:targetPackage="android.test.app.data" />
+ <application>
+ <uses-library android:name="android.test.systemsharedlib" />
+ <uses-library android:name="android.test.systemextsharedlib" />
+ <uses-native-library android:required="false" android:name="libfoo.oem1.so" />
+ <uses-native-library android:required="false" android:name="libbar.oem1.so" />
+ <uses-native-library android:required="false" android:name="libfoo.oem2.so" />
+ <!--libbar.oem2.so is left out -->
+ <uses-native-library android:required="false" android:name="libfoo.product1.so" />
+ <uses-native-library android:required="false" android:name="libbar.product1.so" />
+ </application>
+</manifest>
+
diff --git a/libnativeloader/test/loadlibrarytest_product_app_manifest.xml b/libnativeloader/test/loadlibrarytest_product_app_manifest.xml
index a791795..614f33f 100644
--- a/libnativeloader/test/loadlibrarytest_product_app_manifest.xml
+++ b/libnativeloader/test/loadlibrarytest_product_app_manifest.xml
@@ -19,5 +19,15 @@
package="android.test.app.product">
<instrumentation android:name="androidx.test.runner.AndroidJUnitRunner"
android:targetPackage="android.test.app.product" />
+ <application>
+ <uses-library android:name="android.test.systemsharedlib" />
+ <uses-library android:name="android.test.systemextsharedlib" />
+ <uses-native-library android:required="false" android:name="libfoo.oem1.so" />
+ <uses-native-library android:required="false" android:name="libbar.oem1.so" />
+ <uses-native-library android:required="false" android:name="libfoo.oem2.so" />
+ <!--libbar.oem2.so is left out -->
+ <uses-native-library android:required="false" android:name="libfoo.product1.so" />
+ <uses-native-library android:required="false" android:name="libbar.product1.so" />
+ </application>
</manifest>
diff --git a/libnativeloader/test/loadlibrarytest_system_app_manifest.xml b/libnativeloader/test/loadlibrarytest_system_app_manifest.xml
index 3cbd054..5711f65 100644
--- a/libnativeloader/test/loadlibrarytest_system_app_manifest.xml
+++ b/libnativeloader/test/loadlibrarytest_system_app_manifest.xml
@@ -19,5 +19,13 @@
package="android.test.app.system">
<instrumentation android:name="androidx.test.runner.AndroidJUnitRunner"
android:targetPackage="android.test.app.system" />
+ <application>
+ <uses-library android:name="android.test.systemsharedlib" />
+ <uses-library android:name="android.test.systemextsharedlib" />
+ <!-- System apps get a shared classloader namespace, so they don't need
+ uses-native-library entries for anything in /system. -->
+ <uses-native-library android:required="false" android:name="libfoo.product1.so" />
+ <uses-native-library android:required="false" android:name="libbar.product1.so" />
+ </application>
</manifest>
diff --git a/libnativeloader/test/loadlibrarytest_system_ext_app_manifest.xml b/libnativeloader/test/loadlibrarytest_system_ext_app_manifest.xml
index 83ca779..8aa3fa9 100644
--- a/libnativeloader/test/loadlibrarytest_system_ext_app_manifest.xml
+++ b/libnativeloader/test/loadlibrarytest_system_ext_app_manifest.xml
@@ -19,5 +19,13 @@
package="android.test.app.system_ext">
<instrumentation android:name="androidx.test.runner.AndroidJUnitRunner"
android:targetPackage="android.test.app.system_ext" />
+ <application>
+ <uses-library android:name="android.test.systemsharedlib" />
+ <uses-library android:name="android.test.systemextsharedlib" />
+ <!-- System apps get a shared classloader namespace, so they don't need
+ uses-native-library entries for anything in /system. -->
+ <uses-native-library android:required="false" android:name="libfoo.product1.so" />
+ <uses-native-library android:required="false" android:name="libbar.product1.so" />
+ </application>
</manifest>
diff --git a/libnativeloader/test/loadlibrarytest_system_priv_app_manifest.xml b/libnativeloader/test/loadlibrarytest_system_priv_app_manifest.xml
index 0ad1aa5..126453c 100644
--- a/libnativeloader/test/loadlibrarytest_system_priv_app_manifest.xml
+++ b/libnativeloader/test/loadlibrarytest_system_priv_app_manifest.xml
@@ -19,5 +19,13 @@
package="android.test.app.system_priv">
<instrumentation android:name="androidx.test.runner.AndroidJUnitRunner"
android:targetPackage="android.test.app.system_priv" />
+ <application>
+ <uses-library android:name="android.test.systemsharedlib" />
+ <uses-library android:name="android.test.systemextsharedlib" />
+ <!-- System apps get a shared classloader namespace, so they don't need
+ uses-native-library entries for anything in /system. -->
+ <uses-native-library android:required="false" android:name="libfoo.product1.so" />
+ <uses-native-library android:required="false" android:name="libbar.product1.so" />
+ </application>
</manifest>
diff --git a/libnativeloader/test/loadlibrarytest_vendor_app_manifest.xml b/libnativeloader/test/loadlibrarytest_vendor_app_manifest.xml
index b7073c6..a2a9f64 100644
--- a/libnativeloader/test/loadlibrarytest_vendor_app_manifest.xml
+++ b/libnativeloader/test/loadlibrarytest_vendor_app_manifest.xml
@@ -19,5 +19,15 @@
package="android.test.app.vendor">
<instrumentation android:name="androidx.test.runner.AndroidJUnitRunner"
android:targetPackage="android.test.app.vendor" />
+ <application>
+ <uses-library android:name="android.test.systemsharedlib" />
+ <uses-library android:name="android.test.systemextsharedlib" />
+ <uses-native-library android:required="false" android:name="libfoo.oem1.so" />
+ <uses-native-library android:required="false" android:name="libbar.oem1.so" />
+ <uses-native-library android:required="false" android:name="libfoo.oem2.so" />
+ <!--libbar.oem2.so is left out -->
+ <uses-native-library android:required="false" android:name="libfoo.product1.so" />
+ <uses-native-library android:required="false" android:name="libbar.product1.so" />
+ </application>
</manifest>
diff --git a/libnativeloader/test/src/android/test/app/DataAppTest.java b/libnativeloader/test/src/android/test/app/DataAppTest.java
new file mode 100644
index 0000000..767a7b1
--- /dev/null
+++ b/libnativeloader/test/src/android/test/app/DataAppTest.java
@@ -0,0 +1,64 @@
+/*
+ * Copyright (C) 2022 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.
+ */
+
+package android.test.app;
+
+import android.test.lib.TestUtils;
+import android.test.systemextsharedlib.SystemExtSharedLib;
+import android.test.systemsharedlib.SystemSharedLib;
+import androidx.test.filters.SmallTest;
+import androidx.test.runner.AndroidJUnit4;
+import org.junit.Test;
+import org.junit.runner.RunWith;
+
+@SmallTest
+@RunWith(AndroidJUnit4.class)
+public class DataAppTest {
+ @Test
+ public void testLoadExtendedPublicLibraries() {
+ System.loadLibrary("foo.oem1");
+ System.loadLibrary("bar.oem1");
+ System.loadLibrary("foo.oem2");
+ TestUtils.assertLinkerNamespaceError(
+ () -> System.loadLibrary("bar.oem2")); // Missing <uses-native-library>.
+ System.loadLibrary("foo.product1");
+ System.loadLibrary("bar.product1");
+ }
+
+ @Test
+ public void testLoadPrivateLibraries() {
+ TestUtils.assertLinkerNamespaceError(() -> System.loadLibrary("system_private1"));
+ TestUtils.assertLinkerNamespaceError(() -> System.loadLibrary("systemext_private1"));
+ TestUtils.assertLibraryNotFound(() -> System.loadLibrary("product_private1"));
+ TestUtils.assertLibraryNotFound(() -> System.loadLibrary("vendor_private1"));
+ }
+
+ @Test
+ public void testLoadPrivateLibrariesViaSystemSharedLib() {
+ SystemSharedLib.loadLibrary("system_private2");
+ SystemSharedLib.loadLibrary("systemext_private2");
+ TestUtils.assertLibraryNotFound(() -> SystemSharedLib.loadLibrary("product_private2"));
+ TestUtils.assertLibraryNotFound(() -> SystemSharedLib.loadLibrary("vendor_private2"));
+ }
+
+ @Test
+ public void testLoadPrivateLibrariesViaSystemExtSharedLib() {
+ SystemExtSharedLib.loadLibrary("system_private3");
+ SystemExtSharedLib.loadLibrary("systemext_private3");
+ TestUtils.assertLibraryNotFound(() -> SystemExtSharedLib.loadLibrary("product_private3"));
+ TestUtils.assertLibraryNotFound(() -> SystemExtSharedLib.loadLibrary("vendor_private3"));
+ }
+}
diff --git a/libnativeloader/test/src/android/test/app/ProductAppTest.java b/libnativeloader/test/src/android/test/app/ProductAppTest.java
index 243e2c7..1f36798 100644
--- a/libnativeloader/test/src/android/test/app/ProductAppTest.java
+++ b/libnativeloader/test/src/android/test/app/ProductAppTest.java
@@ -16,9 +16,9 @@
package android.test.app;
-import static com.google.common.truth.Truth.assertThat;
-import static org.junit.Assert.assertThrows;
-
+import android.test.lib.TestUtils;
+import android.test.systemextsharedlib.SystemExtSharedLib;
+import android.test.systemsharedlib.SystemSharedLib;
import androidx.test.filters.SmallTest;
import androidx.test.runner.AndroidJUnit4;
import org.junit.Test;
@@ -28,12 +28,37 @@
@RunWith(AndroidJUnit4.class)
public class ProductAppTest {
@Test
- public void testLoadLibraries() {
+ public void testLoadExtendedPublicLibraries() {
System.loadLibrary("foo.oem1");
System.loadLibrary("bar.oem1");
System.loadLibrary("foo.oem2");
- System.loadLibrary("bar.oem2");
+ TestUtils.assertLinkerNamespaceError(
+ () -> System.loadLibrary("bar.oem2")); // Missing <uses-native-library>.
System.loadLibrary("foo.product1");
System.loadLibrary("bar.product1");
}
+
+ @Test
+ public void testLoadPrivateLibraries() {
+ TestUtils.assertLinkerNamespaceError(() -> System.loadLibrary("system_private1"));
+ TestUtils.assertLinkerNamespaceError(() -> System.loadLibrary("systemext_private1"));
+ System.loadLibrary("product_private1");
+ TestUtils.assertLibraryNotFound(() -> System.loadLibrary("vendor_private1"));
+ }
+
+ @Test
+ public void testLoadPrivateLibrariesViaSystemSharedLib() {
+ SystemSharedLib.loadLibrary("system_private2");
+ SystemSharedLib.loadLibrary("systemext_private2");
+ TestUtils.assertLibraryNotFound(() -> SystemSharedLib.loadLibrary("product_private2"));
+ TestUtils.assertLibraryNotFound(() -> SystemSharedLib.loadLibrary("vendor_private2"));
+ }
+
+ @Test
+ public void testLoadPrivateLibrariesViaSystemExtSharedLib() {
+ SystemExtSharedLib.loadLibrary("system_private3");
+ SystemExtSharedLib.loadLibrary("systemext_private3");
+ TestUtils.assertLibraryNotFound(() -> SystemExtSharedLib.loadLibrary("product_private3"));
+ TestUtils.assertLibraryNotFound(() -> SystemExtSharedLib.loadLibrary("vendor_private3"));
+ }
}
diff --git a/libnativeloader/test/src/android/test/app/SystemAppTest.java b/libnativeloader/test/src/android/test/app/SystemAppTest.java
index dd680f5..197a40c 100644
--- a/libnativeloader/test/src/android/test/app/SystemAppTest.java
+++ b/libnativeloader/test/src/android/test/app/SystemAppTest.java
@@ -16,20 +16,20 @@
package android.test.app;
-import static com.google.common.truth.Truth.assertThat;
-import static org.junit.Assert.assertThrows;
-
+import android.test.lib.TestUtils;
+import android.test.systemextsharedlib.SystemExtSharedLib;
+import android.test.systemsharedlib.SystemSharedLib;
import androidx.test.filters.SmallTest;
import androidx.test.runner.AndroidJUnit4;
import org.junit.Test;
import org.junit.runner.RunWith;
-// These tests are run in both /system and /system_ext.
+// These tests are run from /system/app, /system/priv-app, and /system_ext/app.
@SmallTest
@RunWith(AndroidJUnit4.class)
public class SystemAppTest {
@Test
- public void testLoadLibraries() {
+ public void testLoadExtendedPublicLibraries() {
System.loadLibrary("foo.oem1");
System.loadLibrary("bar.oem1");
System.loadLibrary("foo.oem2");
@@ -37,4 +37,28 @@
System.loadLibrary("foo.product1");
System.loadLibrary("bar.product1");
}
+
+ @Test
+ public void testLoadPrivateLibraries() {
+ System.loadLibrary("system_private1");
+ System.loadLibrary("systemext_private1");
+ TestUtils.assertLibraryNotFound(() -> System.loadLibrary("product_private1"));
+ TestUtils.assertLibraryNotFound(() -> System.loadLibrary("vendor_private1"));
+ }
+
+ @Test
+ public void testLoadPrivateLibrariesViaSystemSharedLib() {
+ SystemSharedLib.loadLibrary("system_private2");
+ SystemSharedLib.loadLibrary("systemext_private2");
+ TestUtils.assertLibraryNotFound(() -> SystemSharedLib.loadLibrary("product_private2"));
+ TestUtils.assertLibraryNotFound(() -> SystemSharedLib.loadLibrary("vendor_private2"));
+ }
+
+ @Test
+ public void testLoadPrivateLibrariesViaSystemExtSharedLib() {
+ SystemExtSharedLib.loadLibrary("system_private3");
+ SystemExtSharedLib.loadLibrary("systemext_private3");
+ TestUtils.assertLibraryNotFound(() -> SystemExtSharedLib.loadLibrary("product_private3"));
+ TestUtils.assertLibraryNotFound(() -> SystemExtSharedLib.loadLibrary("vendor_private3"));
+ }
}
diff --git a/libnativeloader/test/src/android/test/app/VendorAppTest.java b/libnativeloader/test/src/android/test/app/VendorAppTest.java
index a830d06..c9ce8db 100644
--- a/libnativeloader/test/src/android/test/app/VendorAppTest.java
+++ b/libnativeloader/test/src/android/test/app/VendorAppTest.java
@@ -16,9 +16,9 @@
package android.test.app;
-import static com.google.common.truth.Truth.assertThat;
-import static org.junit.Assert.assertThrows;
-
+import android.test.lib.TestUtils;
+import android.test.systemextsharedlib.SystemExtSharedLib;
+import android.test.systemsharedlib.SystemSharedLib;
import androidx.test.filters.SmallTest;
import androidx.test.runner.AndroidJUnit4;
import org.junit.Test;
@@ -28,19 +28,40 @@
@RunWith(AndroidJUnit4.class)
public class VendorAppTest {
@Test
- public void testLoadLibraries() {
- assertLinkerNamespaceError("foo.oem1");
- assertLinkerNamespaceError("bar.oem1");
- assertLinkerNamespaceError("foo.oem2");
- assertLinkerNamespaceError("bar.oem2");
+ public void testLoadExtendedPublicLibraries() {
+ TestUtils.assertLinkerNamespaceError(() -> System.loadLibrary("foo.oem1"));
+ TestUtils.assertLinkerNamespaceError(() -> System.loadLibrary("bar.oem1"));
+ TestUtils.assertLinkerNamespaceError(() -> System.loadLibrary("foo.oem2"));
+ TestUtils.assertLinkerNamespaceError(() -> System.loadLibrary("bar.oem2"));
System.loadLibrary("foo.product1");
System.loadLibrary("bar.product1");
}
- private void assertLinkerNamespaceError(String libraryName) {
- Throwable t =
- assertThrows(UnsatisfiedLinkError.class, () -> System.loadLibrary(libraryName));
- assertThat(t.getMessage())
- .containsMatch("dlopen failed: .* is not accessible for the namespace");
+ @Test
+ public void testLoadPrivateLibraries() {
+ TestUtils.assertLinkerNamespaceError(() -> System.loadLibrary("system_private1"));
+ TestUtils.assertLinkerNamespaceError(() -> System.loadLibrary("systemext_private1"));
+ TestUtils.assertLibraryNotFound(() -> System.loadLibrary("product_private1"));
+ // TODO(mast): The vendor app fails to load a private vendor library because it gets
+ // classified as untrusted_app in SELinux, which doesn't have access to vendor_file. Even an
+ // app in /vendor/priv-app, which gets classified as priv_app, still doesn't have access to
+ // vendor_file. Check that the test setup is correct and if this is WAI.
+ TestUtils.assertLibraryNotFound(() -> System.loadLibrary("vendor_private1"));
+ }
+
+ @Test
+ public void testLoadPrivateLibrariesViaSystemSharedLib() {
+ SystemSharedLib.loadLibrary("system_private2");
+ SystemSharedLib.loadLibrary("systemext_private2");
+ TestUtils.assertLibraryNotFound(() -> SystemSharedLib.loadLibrary("product_private2"));
+ TestUtils.assertLibraryNotFound(() -> SystemSharedLib.loadLibrary("vendor_private2"));
+ }
+
+ @Test
+ public void testLoadPrivateLibrariesViaSystemExtSharedLib() {
+ SystemExtSharedLib.loadLibrary("system_private3");
+ SystemExtSharedLib.loadLibrary("systemext_private3");
+ TestUtils.assertLibraryNotFound(() -> SystemExtSharedLib.loadLibrary("product_private3"));
+ TestUtils.assertLibraryNotFound(() -> SystemExtSharedLib.loadLibrary("vendor_private3"));
}
}
diff --git a/libnativeloader/test/src/android/test/hostside/LibnativeloaderTest.java b/libnativeloader/test/src/android/test/hostside/LibnativeloaderTest.java
index 7e2d9de..c929037 100644
--- a/libnativeloader/test/src/android/test/hostside/LibnativeloaderTest.java
+++ b/libnativeloader/test/src/android/test/hostside/LibnativeloaderTest.java
@@ -19,6 +19,8 @@
import static com.google.common.truth.Truth.assertThat;
import static com.google.common.truth.Truth.assertWithMessage;
+import com.android.compatibility.common.tradefed.build.CompatibilityBuildHelper;
+import com.android.tradefed.build.IBuildInfo;
import com.android.tradefed.device.DeviceNotAvailableException;
import com.android.tradefed.device.ITestDevice;
import com.android.tradefed.invoker.IInvocationContext;
@@ -54,19 +56,28 @@
@BeforeClassWithInfo
public static void beforeClassWithDevice(TestInformation testInfo) throws Exception {
- DeviceContext ctx = new DeviceContext(testInfo.getContext(), testInfo.getDevice());
+ DeviceContext ctx = new DeviceContext(
+ testInfo.getContext(), testInfo.getDevice(), testInfo.getBuildInfo());
// A soft reboot is slow, so do setup for all tests and reboot once.
ctx.mDevice.remountSystemWritable();
- try (ZipFile libApk = openLibContainerApk()) {
- ctx.pushSystemOemLibs(libApk);
- ctx.pushProductLibs(libApk);
- }
- // "Install" apps in various partitions through plain adb push. We need them in these
- // locations to test library loading restrictions, so we cannot use
- // ITestDevice.installPackage for it since it only installs in /data.
+ File libContainerApk = ctx.mBuildHelper.getTestFile("library_container_app.apk");
+ try (ZipFile libApk = new ZipFile(libContainerApk)) {
+ ctx.pushExtendedPublicSystemOemLibs(libApk);
+ ctx.pushExtendedPublicProductLibs(libApk);
+ ctx.pushPrivateLibs(libApk);
+ }
+ ctx.pushSystemSharedLib("/system/framework", "android.test.systemsharedlib",
+ "libnativeloader_system_shared_lib.jar");
+ ctx.pushSystemSharedLib("/system_ext/framework", "android.test.systemextsharedlib",
+ "libnativeloader_system_ext_shared_lib.jar");
+
+ // "Install" apps in various partitions through plain adb push followed by a soft reboot. We
+ // need them in these locations to test library loading restrictions, so for all except
+ // loadlibrarytest_data_app we cannot use ITestDevice.installPackage for it since it only
+ // installs in /data.
// For testSystemPrivApp
ctx.pushApk("loadlibrarytest_system_priv_app", "/system/priv-app");
@@ -85,13 +96,21 @@
ctx.softReboot();
+ // For testDataApp. Install this the normal way after the system server restart.
+ ctx.installPackage("loadlibrarytest_data_app");
+
testInfo.properties().put(CLEANUP_PATHS_KEY, ctx.mCleanup.getPathList());
}
@AfterClassWithInfo
public static void afterClassWithDevice(TestInformation testInfo) throws Exception {
+ ITestDevice device = testInfo.getDevice();
+
+ // Uninstall loadlibrarytest_data_app.
+ device.uninstallPackage("android.test.app.data");
+
String cleanupPathList = testInfo.properties().get(CLEANUP_PATHS_KEY);
- CleanupPaths cleanup = new CleanupPaths(testInfo.getDevice(), cleanupPathList);
+ CleanupPaths cleanup = new CleanupPaths(device, cleanupPathList);
cleanup.cleanup();
}
@@ -123,6 +142,11 @@
runDeviceTests("android.test.app.vendor", "android.test.app.VendorAppTest");
}
+ @Test
+ public void testDataApp() throws Exception {
+ runDeviceTests("android.test.app.data", "android.test.app.DataAppTest");
+ }
+
// Utility class that keeps track of a set of paths the need to be deleted after testing.
private static class CleanupPaths {
private ITestDevice mDevice;
@@ -171,18 +195,20 @@
private static class DeviceContext implements AutoCloseable {
IInvocationContext mContext;
ITestDevice mDevice;
+ CompatibilityBuildHelper mBuildHelper;
CleanupPaths mCleanup;
private String mTestArch;
- DeviceContext(IInvocationContext context, ITestDevice device) {
+ DeviceContext(IInvocationContext context, ITestDevice device, IBuildInfo buildInfo) {
mContext = context;
mDevice = device;
+ mBuildHelper = new CompatibilityBuildHelper(buildInfo);
mCleanup = new CleanupPaths(mDevice);
}
public void close() throws DeviceNotAvailableException { mCleanup.cleanup(); }
- void pushSystemOemLibs(ZipFile libApk) throws Exception {
+ void pushExtendedPublicSystemOemLibs(ZipFile libApk) throws Exception {
pushNativeTestLib(libApk, "/system/${LIB}/libfoo.oem1.so");
pushNativeTestLib(libApk, "/system/${LIB}/libbar.oem1.so");
pushString("libfoo.oem1.so\n"
@@ -196,7 +222,7 @@
"/system/etc/public.libraries-oem2.txt");
}
- void pushProductLibs(ZipFile libApk) throws Exception {
+ void pushExtendedPublicProductLibs(ZipFile libApk) throws Exception {
pushNativeTestLib(libApk, "/product/${LIB}/libfoo.product1.so");
pushNativeTestLib(libApk, "/product/${LIB}/libbar.product1.so");
pushString("libfoo.product1.so\n"
@@ -204,6 +230,27 @@
"/product/etc/public.libraries-product1.txt");
}
+ void pushPrivateLibs(ZipFile libApk) throws Exception {
+ // Push the libraries once for each test. Since we cannot unload them, we need a fresh
+ // never-before-loaded library in each loadLibrary call.
+ for (int i = 1; i <= 3; ++i) {
+ pushNativeTestLib(libApk, "/system/${LIB}/libsystem_private" + i + ".so");
+ pushNativeTestLib(libApk, "/system_ext/${LIB}/libsystemext_private" + i + ".so");
+ pushNativeTestLib(libApk, "/product/${LIB}/libproduct_private" + i + ".so");
+ pushNativeTestLib(libApk, "/vendor/${LIB}/libvendor_private" + i + ".so");
+ }
+ }
+
+ void pushSystemSharedLib(String packageDir, String packageName, String buildJarName)
+ throws Exception {
+ String path = packageDir + "/" + packageName + ".jar";
+ pushFile(buildJarName, path);
+ pushString("<permissions>\n"
+ + "<library name=\"" + packageName + "\" file=\"" + path + "\" />\n"
+ + "</permissions>\n",
+ "system/etc/permissions/" + packageName + ".xml");
+ }
+
void softReboot() throws DeviceNotAvailableException {
assertCommandSucceeds("setprop dev.bootcomplete 0");
assertCommandSucceeds("stop");
@@ -228,15 +275,14 @@
assertThat(mDevice.pushString(fileContents, destPath)).isTrue();
}
- // Like pushString, but extracts a Java resource and pushes that.
- void pushResource(String resourceName, String destPath) throws Exception {
- File hostTempFile = extractResourceToTempFile(resourceName);
+ // Like pushString, but pushes a data file included in the host test.
+ void pushFile(String fileName, String destPath) throws Exception {
mCleanup.addPath(destPath);
- assertThat(mDevice.pushFile(hostTempFile, destPath)).isTrue();
+ assertThat(mDevice.pushFile(mBuildHelper.getTestFile(fileName), destPath)).isTrue();
}
void pushApk(String apkBaseName, String destPath) throws Exception {
- pushResource("/" + apkBaseName + ".apk",
+ pushFile(apkBaseName + ".apk",
destPath + "/" + apkBaseName + "/" + apkBaseName + ".apk");
}
@@ -261,6 +307,12 @@
assertThat(mDevice.pushFile(libraryTempFile, destPath)).isTrue();
}
+ void installPackage(String apkBaseName) throws Exception {
+ assertThat(mDevice.installPackage(mBuildHelper.getTestFile(apkBaseName + ".apk"),
+ false /* reinstall */))
+ .isNull();
+ }
+
String assertCommandSucceeds(String command) throws DeviceNotAvailableException {
CommandResult result = mDevice.executeShellV2Command(command);
assertWithMessage(result.toString()).that(result.getExitCode()).isEqualTo(0);
@@ -269,20 +321,6 @@
}
}
- static private ZipFile openLibContainerApk() throws Exception {
- return new ZipFile(extractResourceToTempFile("/library_container_app.apk"));
- }
-
- static private File extractResourceToTempFile(String resourceName) throws Exception {
- assertThat(resourceName).startsWith("/");
- try (InputStream inStream = LibnativeloaderTest.class.getResourceAsStream(resourceName)) {
- assertWithMessage("Failed to extract resource " + resourceName)
- .that(inStream)
- .isNotNull();
- return writeStreamToTempFile(resourceName.substring(1), inStream);
- }
- }
-
static private File writeStreamToTempFile(String tempFileBaseName, InputStream inStream)
throws Exception {
File hostTempFile = File.createTempFile(tempFileBaseName, null);
diff --git a/libnativeloader/test/src/android/test/lib/TestUtils.java b/libnativeloader/test/src/android/test/lib/TestUtils.java
new file mode 100644
index 0000000..eda9993
--- /dev/null
+++ b/libnativeloader/test/src/android/test/lib/TestUtils.java
@@ -0,0 +1,35 @@
+/*
+ * Copyright (C) 2022 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.
+ */
+
+package android.test.lib;
+
+import static com.google.common.truth.Truth.assertThat;
+import static org.junit.Assert.assertThrows;
+
+import org.junit.function.ThrowingRunnable;
+
+public final class TestUtils {
+ public static void assertLibraryNotFound(ThrowingRunnable loadLibrary) {
+ Throwable t = assertThrows(UnsatisfiedLinkError.class, loadLibrary);
+ assertThat(t.getMessage()).containsMatch("dlopen failed: library .* not found");
+ }
+
+ public static void assertLinkerNamespaceError(ThrowingRunnable loadLibrary) {
+ Throwable t = assertThrows(UnsatisfiedLinkError.class, loadLibrary);
+ assertThat(t.getMessage())
+ .containsMatch("dlopen failed: .* is not accessible for the namespace");
+ }
+}
diff --git a/libnativeloader/test/src/android/test/systemextsharedlib/SystemExtSharedLib.java b/libnativeloader/test/src/android/test/systemextsharedlib/SystemExtSharedLib.java
new file mode 100644
index 0000000..1240e12
--- /dev/null
+++ b/libnativeloader/test/src/android/test/systemextsharedlib/SystemExtSharedLib.java
@@ -0,0 +1,21 @@
+/*
+ * Copyright (C) 2022 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.
+ */
+
+package android.test.systemextsharedlib;
+
+public final class SystemExtSharedLib {
+ public static void loadLibrary(String name) { System.loadLibrary(name); }
+}
diff --git a/libnativeloader/test/src/android/test/systemsharedlib/SystemSharedLib.java b/libnativeloader/test/src/android/test/systemsharedlib/SystemSharedLib.java
new file mode 100644
index 0000000..8e2af9f
--- /dev/null
+++ b/libnativeloader/test/src/android/test/systemsharedlib/SystemSharedLib.java
@@ -0,0 +1,21 @@
+/*
+ * Copyright (C) 2022 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.
+ */
+
+package android.test.systemsharedlib;
+
+public final class SystemSharedLib {
+ public static void loadLibrary(String name) { System.loadLibrary(name); }
+}
diff --git a/odrefresh/odr_metrics.cc b/odrefresh/odr_metrics.cc
index 4bddb17..a2ce51f 100644
--- a/odrefresh/odr_metrics.cc
+++ b/odrefresh/odr_metrics.cc
@@ -64,16 +64,16 @@
}
}
-void OdrMetrics::SetCompilationTime(int32_t seconds) {
+void OdrMetrics::SetCompilationTime(int32_t millis) {
switch (stage_) {
case Stage::kPrimaryBootClasspath:
- primary_bcp_compilation_seconds_ = seconds;
+ primary_bcp_compilation_millis_ = millis;
break;
case Stage::kSecondaryBootClasspath:
- secondary_bcp_compilation_seconds_ = seconds;
+ secondary_bcp_compilation_millis_ = millis;
break;
case Stage::kSystemServerClasspath:
- system_server_compilation_seconds_ = seconds;
+ system_server_compilation_millis_ = millis;
break;
case Stage::kCheck:
case Stage::kComplete:
@@ -119,28 +119,31 @@
if (!trigger_.has_value()) {
return false;
}
+ record->odrefresh_metrics_version = kOdrefreshMetricsVersion;
record->art_apex_version = art_apex_version_;
record->trigger = static_cast<uint32_t>(trigger_.value());
record->stage_reached = static_cast<uint32_t>(stage_);
record->status = static_cast<uint32_t>(status_);
- record->primary_bcp_compilation_seconds = primary_bcp_compilation_seconds_;
- record->secondary_bcp_compilation_seconds = secondary_bcp_compilation_seconds_;
- record->system_server_compilation_seconds = system_server_compilation_seconds_;
record->cache_space_free_start_mib = cache_space_free_start_mib_;
record->cache_space_free_end_mib = cache_space_free_end_mib_;
+ record->primary_bcp_compilation_millis = primary_bcp_compilation_millis_;
+ record->secondary_bcp_compilation_millis = secondary_bcp_compilation_millis_;
+ record->system_server_compilation_millis = system_server_compilation_millis_;
return true;
}
void OdrMetrics::WriteToFile(const std::string& path, const OdrMetrics* metrics) {
- OdrMetricsRecord record;
+ OdrMetricsRecord record{};
if (!metrics->ToRecord(&record)) {
LOG(ERROR) << "Attempting to report metrics without a compilation trigger.";
return;
}
- // Preserve order from frameworks/proto_logging/stats/atoms.proto in metrics file written.
- std::ofstream ofs(path);
- ofs << record;
+ const android::base::Result<void>& result = record.WriteToFile(path);
+ if (!result.ok()) {
+ LOG(ERROR) << "Failed to report metrics to file: " << path
+ << ", error: " << result.error().message();
+ }
}
} // namespace odrefresh
diff --git a/odrefresh/odr_metrics.h b/odrefresh/odr_metrics.h
index cd80bef..7ebc148 100644
--- a/odrefresh/odr_metrics.h
+++ b/odrefresh/odr_metrics.h
@@ -126,7 +126,7 @@
static int32_t GetFreeSpaceMiB(const std::string& path);
static void WriteToFile(const std::string& path, const OdrMetrics* metrics);
- void SetCompilationTime(int32_t seconds);
+ void SetCompilationTime(int32_t millis);
const std::string cache_directory_;
const std::string metrics_file_;
@@ -137,11 +137,11 @@
Stage stage_ = Stage::kUnknown;
Status status_ = Status::kUnknown;
- int32_t primary_bcp_compilation_seconds_ = 0;
- int32_t secondary_bcp_compilation_seconds_ = 0;
- int32_t system_server_compilation_seconds_ = 0;
int32_t cache_space_free_start_mib_ = 0;
int32_t cache_space_free_end_mib_ = 0;
+ int32_t primary_bcp_compilation_millis_ = 0;
+ int32_t secondary_bcp_compilation_millis_ = 0;
+ int32_t system_server_compilation_millis_ = 0;
friend class ScopedOdrCompilationTimer;
};
@@ -155,8 +155,8 @@
~ScopedOdrCompilationTimer() {
auto elapsed_time = std::chrono::steady_clock::now() - start_;
- auto elapsed_seconds = std::chrono::duration_cast<std::chrono::seconds>(elapsed_time);
- metrics_.SetCompilationTime(static_cast<int32_t>(elapsed_seconds.count()));
+ auto elapsed_millis = std::chrono::duration_cast<std::chrono::milliseconds>(elapsed_time);
+ metrics_.SetCompilationTime(static_cast<int32_t>(elapsed_millis.count()));
}
private:
diff --git a/odrefresh/odr_metrics_record.cc b/odrefresh/odr_metrics_record.cc
index fc135d3..b8c2f80 100644
--- a/odrefresh/odr_metrics_record.cc
+++ b/odrefresh/odr_metrics_record.cc
@@ -14,59 +14,110 @@
* limitations under the License.
*/
+#include "android-base/logging.h"
#include "odr_metrics_record.h"
+#include "tinyxml2.h"
#include <iosfwd>
-#include <istream>
-#include <ostream>
-#include <streambuf>
#include <string>
namespace art {
namespace odrefresh {
-std::istream& operator>>(std::istream& is, OdrMetricsRecord& record) {
- // Block I/O related exceptions
- auto saved_exceptions = is.exceptions();
- is.exceptions(std::ios_base::iostate {});
+namespace {
+android::base::Result<int64_t> ReadInt64(tinyxml2::XMLElement* parent, const char* name) {
+ tinyxml2::XMLElement* element = parent->FirstChildElement(name);
+ if (element == nullptr) {
+ return Errorf("Expected Odrefresh metric {} not found", name);
+ }
- // The order here matches the field order of MetricsRecord.
- is >> record.art_apex_version >> std::ws;
- is >> record.trigger >> std::ws;
- is >> record.stage_reached >> std::ws;
- is >> record.status >> std::ws;
- is >> record.primary_bcp_compilation_seconds >> std::ws;
- is >> record.secondary_bcp_compilation_seconds >> std::ws;
- is >> record.system_server_compilation_seconds >> std::ws;
- is >> record.cache_space_free_start_mib >> std::ws;
- is >> record.cache_space_free_end_mib >> std::ws;
-
- // Restore I/O related exceptions
- is.exceptions(saved_exceptions);
- return is;
+ int64_t metric;
+ tinyxml2::XMLError result = element->QueryInt64Text(&metric);
+ if (result == tinyxml2::XML_SUCCESS) {
+ return metric;
+ } else {
+ return Errorf("Odrefresh metric {} is not an int64", name);
+ }
}
-std::ostream& operator<<(std::ostream& os, const OdrMetricsRecord& record) {
- static const char kSpace = ' ';
+android::base::Result<int32_t> ReadInt32(tinyxml2::XMLElement* parent, const char* name) {
+ tinyxml2::XMLElement* element = parent->FirstChildElement(name);
+ if (element == nullptr) {
+ return Errorf("Expected Odrefresh metric {} not found", name);
+ }
- // Block I/O related exceptions
- auto saved_exceptions = os.exceptions();
- os.exceptions(std::ios_base::iostate {});
+ int32_t metric;
+ tinyxml2::XMLError result = element->QueryIntText(&metric);
+ if (result == tinyxml2::XML_SUCCESS) {
+ return metric;
+ } else {
+ return Errorf("Odrefresh metric {} is not an int32", name);
+ }
+}
+
+template <typename T>
+void AddMetric(tinyxml2::XMLElement* parent, const char* name, const T& value) {
+ parent->InsertNewChildElement(name)->SetText(value);
+}
+} // namespace
+
+android::base::Result<void> OdrMetricsRecord::ReadFromFile(const std::string& filename) {
+ tinyxml2::XMLDocument xml_document;
+ tinyxml2::XMLError result = xml_document.LoadFile(filename.data());
+ if (result != tinyxml2::XML_SUCCESS) {
+ return android::base::Error() << xml_document.ErrorStr();
+ }
+
+ tinyxml2::XMLElement* metrics = xml_document.FirstChildElement("odrefresh_metrics");
+ if (metrics == nullptr) {
+ return Errorf("odrefresh_metrics element not found in {}", filename);
+ }
+
+ odrefresh_metrics_version = OR_RETURN(ReadInt32(metrics, "odrefresh_metrics_version"));
+ if (odrefresh_metrics_version != kOdrefreshMetricsVersion) {
+ return Errorf("odrefresh_metrics_version {} is different than expected ({})",
+ odrefresh_metrics_version,
+ kOdrefreshMetricsVersion);
+ }
+
+ art_apex_version = OR_RETURN(ReadInt64(metrics, "art_apex_version"));
+ trigger = OR_RETURN(ReadInt32(metrics, "trigger"));
+ stage_reached = OR_RETURN(ReadInt32(metrics, "stage_reached"));
+ status = OR_RETURN(ReadInt32(metrics, "status"));
+ cache_space_free_start_mib = OR_RETURN(ReadInt32(metrics, "cache_space_free_start_mib"));
+ cache_space_free_end_mib = OR_RETURN(ReadInt32(metrics, "cache_space_free_end_mib"));
+ primary_bcp_compilation_millis = OR_RETURN(
+ ReadInt32(metrics, "primary_bcp_compilation_millis"));
+ secondary_bcp_compilation_millis = OR_RETURN(
+ ReadInt32(metrics, "secondary_bcp_compilation_millis"));
+ system_server_compilation_millis = OR_RETURN(
+ ReadInt32(metrics, "system_server_compilation_millis"));
+ return {};
+}
+
+android::base::Result<void> OdrMetricsRecord::WriteToFile(const std::string& filename) const {
+ tinyxml2::XMLDocument xml_document;
+ tinyxml2::XMLElement* metrics = xml_document.NewElement("odrefresh_metrics");
+ xml_document.InsertEndChild(metrics);
// The order here matches the field order of MetricsRecord.
- os << record.art_apex_version << kSpace;
- os << record.trigger << kSpace;
- os << record.stage_reached << kSpace;
- os << record.status << kSpace;
- os << record.primary_bcp_compilation_seconds << kSpace;
- os << record.secondary_bcp_compilation_seconds << kSpace;
- os << record.system_server_compilation_seconds << kSpace;
- os << record.cache_space_free_start_mib << kSpace;
- os << record.cache_space_free_end_mib << std::endl;
+ AddMetric(metrics, "odrefresh_metrics_version", odrefresh_metrics_version);
+ AddMetric(metrics, "art_apex_version", art_apex_version);
+ AddMetric(metrics, "trigger", trigger);
+ AddMetric(metrics, "stage_reached", stage_reached);
+ AddMetric(metrics, "status", status);
+ AddMetric(metrics, "cache_space_free_start_mib", cache_space_free_start_mib);
+ AddMetric(metrics, "cache_space_free_end_mib", cache_space_free_end_mib);
+ AddMetric(metrics, "primary_bcp_compilation_millis", primary_bcp_compilation_millis);
+ AddMetric(metrics, "secondary_bcp_compilation_millis", secondary_bcp_compilation_millis);
+ AddMetric(metrics, "system_server_compilation_millis", system_server_compilation_millis);
- // Restore I/O related exceptions
- os.exceptions(saved_exceptions);
- return os;
+ tinyxml2::XMLError result = xml_document.SaveFile(filename.data(), /*compact=*/true);
+ if (result == tinyxml2::XML_SUCCESS) {
+ return {};
+ } else {
+ return android::base::Error() << xml_document.ErrorStr();
+ }
}
} // namespace odrefresh
diff --git a/odrefresh/odr_metrics_record.h b/odrefresh/odr_metrics_record.h
index 9dd51a6..fa95337 100644
--- a/odrefresh/odr_metrics_record.h
+++ b/odrefresh/odr_metrics_record.h
@@ -20,43 +20,42 @@
#include <cstdint>
#include <iosfwd> // For forward-declaration of std::string.
+#include "android-base/result.h"
+#include "tinyxml2.h"
+
namespace art {
namespace odrefresh {
// Default location for storing metrics from odrefresh.
-constexpr const char* kOdrefreshMetricsFile = "/data/misc/odrefresh/odrefresh-metrics.txt";
+constexpr const char* kOdrefreshMetricsFile = "/data/misc/odrefresh/odrefresh-metrics.xml";
+
+// Initial OdrefreshMetrics version
+static constexpr int32_t kOdrefreshMetricsVersion = 2;
// MetricsRecord is a simpler container for Odrefresh metric values reported to statsd. The order
// and types of fields here mirror definition of `OdrefreshReported` in
// frameworks/proto_logging/stats/atoms.proto.
struct OdrMetricsRecord {
+ int32_t odrefresh_metrics_version;
int64_t art_apex_version;
int32_t trigger;
int32_t stage_reached;
int32_t status;
- int32_t primary_bcp_compilation_seconds;
- int32_t secondary_bcp_compilation_seconds;
- int32_t system_server_compilation_seconds;
int32_t cache_space_free_start_mib;
int32_t cache_space_free_end_mib;
+ int32_t primary_bcp_compilation_millis;
+ int32_t secondary_bcp_compilation_millis;
+ int32_t system_server_compilation_millis;
+
+ // Reads a `MetricsRecord` from an XML file.
+ // Returns an error if the XML document was not found or parsed correctly.
+ android::base::Result<void> ReadFromFile(const std::string& filename);
+
+ // Writes a `MetricsRecord` to an XML file.
+ // Returns an error if the XML document was not saved correctly.
+ android::base::Result<void> WriteToFile(const std::string& filename) const;
};
-// Read a `MetricsRecord` from an `istream`.
-//
-// This method blocks istream related exceptions, the caller should check `is.fail()` is false after
-// calling.
-//
-// Returns `is`.
-std::istream& operator>>(std::istream& is, OdrMetricsRecord& record);
-
-// Write a `MetricsRecord` to an `ostream`.
-//
-// This method blocks ostream related exceptions, the caller should check `os.fail()` is false after
-// calling.
-//
-// Returns `os`
-std::ostream& operator<<(std::ostream& os, const OdrMetricsRecord& record);
-
} // namespace odrefresh
} // namespace art
diff --git a/odrefresh/odr_metrics_record_test.cc b/odrefresh/odr_metrics_record_test.cc
index dd739d6..fbb5b99 100644
--- a/odrefresh/odr_metrics_record_test.cc
+++ b/odrefresh/odr_metrics_record_test.cc
@@ -20,6 +20,8 @@
#include <fstream>
+#include "android-base/result-gmock.h"
+#include "android-base/stringprintf.h"
#include "base/common_art_test.h"
namespace art {
@@ -27,86 +29,124 @@
class OdrMetricsRecordTest : public CommonArtTest {};
+using android::base::testing::Ok;
+using android::base::testing::HasError;
+using android::base::testing::WithMessage;
+
TEST_F(OdrMetricsRecordTest, HappyPath) {
- const OdrMetricsRecord expected {
+ const OdrMetricsRecord expected{
+ .odrefresh_metrics_version = art::odrefresh::kOdrefreshMetricsVersion,
.art_apex_version = 0x01233456'789abcde,
.trigger = 0x01020304,
.stage_reached = 0x11121314,
.status = 0x21222324,
- .primary_bcp_compilation_seconds = 0x31323334,
- .secondary_bcp_compilation_seconds = 0x41424344,
- .system_server_compilation_seconds = 0x51525354,
.cache_space_free_start_mib = 0x61626364,
- .cache_space_free_end_mib = 0x71727374
+ .cache_space_free_end_mib = 0x71727374,
+ .primary_bcp_compilation_millis = 0x31323334,
+ .secondary_bcp_compilation_millis = 0x41424344,
+ .system_server_compilation_millis = 0x51525354
};
ScratchDir dir(/*keep_files=*/false);
- std::string file_path = dir.GetPath() + "/metrics-record.txt";
-
- {
- std::ofstream ofs(file_path);
- ofs << expected;
- ASSERT_FALSE(ofs.fail());
- ofs.close();
- }
+ std::string file_path = dir.GetPath() + "/metrics-record.xml";
+ ASSERT_THAT(expected.WriteToFile(file_path), Ok());
OdrMetricsRecord actual {};
- {
- std::ifstream ifs(file_path);
- ifs >> actual;
- ASSERT_TRUE(ifs.eof());
- }
+ ASSERT_THAT(actual.ReadFromFile(file_path), Ok());
+ ASSERT_EQ(expected.odrefresh_metrics_version, actual.odrefresh_metrics_version);
ASSERT_EQ(expected.art_apex_version, actual.art_apex_version);
ASSERT_EQ(expected.trigger, actual.trigger);
ASSERT_EQ(expected.stage_reached, actual.stage_reached);
ASSERT_EQ(expected.status, actual.status);
- ASSERT_EQ(expected.primary_bcp_compilation_seconds, actual.primary_bcp_compilation_seconds);
- ASSERT_EQ(expected.secondary_bcp_compilation_seconds, actual.secondary_bcp_compilation_seconds);
- ASSERT_EQ(expected.system_server_compilation_seconds, actual.system_server_compilation_seconds);
ASSERT_EQ(expected.cache_space_free_start_mib, actual.cache_space_free_start_mib);
ASSERT_EQ(expected.cache_space_free_end_mib, actual.cache_space_free_end_mib);
+ ASSERT_EQ(expected.primary_bcp_compilation_millis, actual.primary_bcp_compilation_millis);
+ ASSERT_EQ(expected.secondary_bcp_compilation_millis, actual.secondary_bcp_compilation_millis);
+ ASSERT_EQ(expected.system_server_compilation_millis, actual.system_server_compilation_millis);
ASSERT_EQ(0, memcmp(&expected, &actual, sizeof(expected)));
}
TEST_F(OdrMetricsRecordTest, EmptyInput) {
ScratchDir dir(/*keep_files=*/false);
- std::string file_path = dir.GetPath() + "/metrics-record.txt";
+ std::string file_path = dir.GetPath() + "/metrics-record.xml";
- std::ifstream ifs(file_path);
- OdrMetricsRecord record;
- ifs >> record;
-
- ASSERT_TRUE(ifs.fail());
- ASSERT_TRUE(!ifs);
+ OdrMetricsRecord record{};
+ ASSERT_THAT(record.ReadFromFile(file_path), testing::Not(Ok()));
}
-TEST_F(OdrMetricsRecordTest, ClosedInput) {
+TEST_F(OdrMetricsRecordTest, UnexpectedInput) {
ScratchDir dir(/*keep_files=*/false);
- std::string file_path = dir.GetPath() + "/metrics-record.txt";
-
- std::ifstream ifs(file_path);
- ifs.close();
-
- OdrMetricsRecord record;
- ifs >> record;
-
- ASSERT_TRUE(ifs.fail());
- ASSERT_TRUE(!ifs);
-}
-
-TEST_F(OdrMetricsRecordTest, ClosedOutput) {
- ScratchDir dir(/*keep_files=*/false);
- std::string file_path = dir.GetPath() + "/metrics-record.txt";
+ std::string file_path = dir.GetPath() + "/metrics-record.xml";
std::ofstream ofs(file_path);
+ ofs << "<not_odrefresh_metrics></not_odrefresh_metrics>";
ofs.close();
- OdrMetricsRecord record {};
- ofs << record;
+ OdrMetricsRecord record{};
+ ASSERT_THAT(
+ record.ReadFromFile(file_path),
+ HasError(WithMessage("odrefresh_metrics element not found in " + file_path)));
+}
- ASSERT_TRUE(ofs.fail());
- ASSERT_TRUE(!ofs.good());
+TEST_F(OdrMetricsRecordTest, ExpectedElementNotFound) {
+ ScratchDir dir(/*keep_files=*/false);
+ std::string file_path = dir.GetPath() + "/metrics-record.xml";
+
+ std::ofstream ofs(file_path);
+ ofs << "<odrefresh_metrics>";
+ ofs << "<not_valid_metric>25</not_valid_metric>";
+ ofs << "</odrefresh_metrics>";
+ ofs.close();
+
+ OdrMetricsRecord record{};
+ ASSERT_THAT(
+ record.ReadFromFile(file_path),
+ HasError(WithMessage("Expected Odrefresh metric odrefresh_metrics_version not found")));
+}
+
+TEST_F(OdrMetricsRecordTest, UnexpectedOdrefreshMetricsVersion) {
+ ScratchDir dir(/*keep_files=*/false);
+ std::string file_path = dir.GetPath() + "/metrics-record.xml";
+
+ std::ofstream ofs(file_path);
+ ofs << "<odrefresh_metrics>";
+ ofs << "<odrefresh_metrics_version>0</odrefresh_metrics_version>";
+ ofs << "</odrefresh_metrics>";
+ ofs.close();
+
+ OdrMetricsRecord record{};
+ std::string expected_error = android::base::StringPrintf(
+ "odrefresh_metrics_version 0 is different than expected (%d)",
+ kOdrefreshMetricsVersion);
+ ASSERT_THAT(record.ReadFromFile(file_path),
+ HasError(WithMessage(expected_error)));
+}
+
+TEST_F(OdrMetricsRecordTest, UnexpectedType) {
+ ScratchDir dir(/*keep_files=*/false);
+ std::string file_path = dir.GetPath() + "/metrics-record.xml";
+
+ std::ofstream ofs(file_path);
+ ofs << "<odrefresh_metrics>";
+ ofs << "<odrefresh_metrics_version>" << kOdrefreshMetricsVersion
+ << "</odrefresh_metrics_version>";
+ ofs << "<art_apex_version>81966764218039518</art_apex_version>";
+ ofs << "<trigger>16909060</trigger>";
+ ofs << "<stage_reached>286397204</stage_reached>";
+ ofs << "<status>abcd</status>"; // It should be an int32.
+ ofs << "<cache_space_free_start_mib>1633837924</cache_space_free_start_mib>";
+ ofs << "<cache_space_free_end_mib>1903326068</cache_space_free_end_mib>";
+ ofs << "<primary_bcp_compilation_millis>825373492</primary_bcp_compilation_millis>";
+ ofs << "<secondary_bcp_compilation_millis>1094861636</secondary_bcp_compilation_millis>";
+ ofs << "<system_server_compilation_millis>1364349780</system_server_compilation_millis>";
+ ofs << "</odrefresh_metrics>";
+ ofs.close();
+
+ OdrMetricsRecord record{};
+ ASSERT_THAT(
+ record.ReadFromFile(file_path),
+ HasError(WithMessage("Odrefresh metric status is not an int32")));
}
} // namespace odrefresh
diff --git a/odrefresh/odr_metrics_test.cc b/odrefresh/odr_metrics_test.cc
index 4519f00..f222caa 100644
--- a/odrefresh/odr_metrics_test.cc
+++ b/odrefresh/odr_metrics_test.cc
@@ -24,6 +24,7 @@
#include <memory>
#include <string>
+#include "android-base/result-gmock.h"
#include "base/common_art_test.h"
namespace art {
@@ -35,7 +36,7 @@
CommonArtTest::SetUp();
scratch_dir_ = std::make_unique<ScratchDir>();
- metrics_file_path_ = scratch_dir_->GetPath() + "/metrics.txt";
+ metrics_file_path_ = scratch_dir_->GetPath() + "/metrics.xml";
cache_directory_ = scratch_dir_->GetPath() + "/dir";
mkdir(cache_directory_.c_str(), S_IRWXU);
}
@@ -158,10 +159,10 @@
EXPECT_TRUE(metrics.ToRecord(&record));
EXPECT_EQ(OdrMetrics::Stage::kPrimaryBootClasspath,
enum_cast<OdrMetrics::Stage>(record.stage_reached));
- EXPECT_NE(0, record.primary_bcp_compilation_seconds);
- EXPECT_GT(10, record.primary_bcp_compilation_seconds);
- EXPECT_EQ(0, record.secondary_bcp_compilation_seconds);
- EXPECT_EQ(0, record.system_server_compilation_seconds);
+ EXPECT_NE(0, record.primary_bcp_compilation_millis);
+ EXPECT_GT(10'000, record.primary_bcp_compilation_millis);
+ EXPECT_EQ(0, record.secondary_bcp_compilation_millis);
+ EXPECT_EQ(0, record.system_server_compilation_millis);
// Secondary boot classpath compilation time.
{
@@ -172,10 +173,10 @@
EXPECT_TRUE(metrics.ToRecord(&record));
EXPECT_EQ(OdrMetrics::Stage::kSecondaryBootClasspath,
enum_cast<OdrMetrics::Stage>(record.stage_reached));
- EXPECT_NE(0, record.primary_bcp_compilation_seconds);
- EXPECT_NE(0, record.secondary_bcp_compilation_seconds);
- EXPECT_GT(10, record.secondary_bcp_compilation_seconds);
- EXPECT_EQ(0, record.system_server_compilation_seconds);
+ EXPECT_NE(0, record.primary_bcp_compilation_millis);
+ EXPECT_NE(0, record.secondary_bcp_compilation_millis);
+ EXPECT_GT(10'000, record.secondary_bcp_compilation_millis);
+ EXPECT_EQ(0, record.system_server_compilation_millis);
// system_server classpath compilation time.
{
@@ -186,10 +187,10 @@
EXPECT_TRUE(metrics.ToRecord(&record));
EXPECT_EQ(OdrMetrics::Stage::kSystemServerClasspath,
enum_cast<OdrMetrics::Stage>(record.stage_reached));
- EXPECT_NE(0, record.primary_bcp_compilation_seconds);
- EXPECT_NE(0, record.secondary_bcp_compilation_seconds);
- EXPECT_NE(0, record.system_server_compilation_seconds);
- EXPECT_GT(10, record.system_server_compilation_seconds);
+ EXPECT_NE(0, record.primary_bcp_compilation_millis);
+ EXPECT_NE(0, record.secondary_bcp_compilation_millis);
+ EXPECT_NE(0, record.system_server_compilation_millis);
+ EXPECT_GT(10'000, record.system_server_compilation_millis);
}
TEST_F(OdrMetricsTest, CacheSpaceValuesAreUpdated) {
@@ -207,11 +208,8 @@
EXPECT_EQ(0, snap.cache_space_free_end_mib);
}
- OdrMetricsRecord on_disk;
- std::ifstream ifs(GetMetricsFilePath());
- EXPECT_TRUE(ifs);
- ifs >> on_disk;
- EXPECT_TRUE(ifs);
+ OdrMetricsRecord on_disk{};
+ EXPECT_THAT(on_disk.ReadFromFile(GetMetricsFilePath()), android::base::testing::Ok());
EXPECT_EQ(snap.cache_space_free_start_mib, on_disk.cache_space_free_start_mib);
EXPECT_NE(0, on_disk.cache_space_free_end_mib);
}
diff --git a/odrefresh/odr_statslog_android.cc b/odrefresh/odr_statslog_android.cc
index 7db348e..ee173d4 100644
--- a/odrefresh/odr_statslog_android.cc
+++ b/odrefresh/odr_statslog_android.cc
@@ -103,16 +103,11 @@
bool ReadValues(const char* metrics_file,
/*out*/ OdrMetricsRecord* record,
/*out*/ std::string* error_msg) {
- std::ifstream ifs(metrics_file);
- if (!ifs) {
- *error_msg = android::base::StringPrintf(
- "metrics file '%s' could not be opened: %s", metrics_file, strerror(errno));
- return false;
- }
-
- ifs >> *record;
- if (!ifs) {
- *error_msg = "file parsing error.";
+ const android::base::Result<void>& result = record->ReadFromFile(metrics_file);
+ if (!result.ok()) {
+ *error_msg = android::base::StringPrintf("Unable to open or parse metrics file %s (error: %s)",
+ metrics_file,
+ result.error().message().data());
return false;
}
@@ -151,16 +146,20 @@
// Write values to statsd. The order of values passed is the same as the order of the
// fields in OdrMetricsRecord.
- int bytes_written = art::metrics::statsd::stats_write(metrics::statsd::ODREFRESH_REPORTED,
- record.art_apex_version,
- record.trigger,
- record.stage_reached,
- record.status,
- record.primary_bcp_compilation_seconds,
- record.secondary_bcp_compilation_seconds,
- record.system_server_compilation_seconds,
- record.cache_space_free_start_mib,
- record.cache_space_free_end_mib);
+ int bytes_written = art::metrics::statsd::stats_write(
+ metrics::statsd::ODREFRESH_REPORTED,
+ record.art_apex_version,
+ record.trigger,
+ record.stage_reached,
+ record.status,
+ record.primary_bcp_compilation_millis / 1000,
+ record.secondary_bcp_compilation_millis / 1000,
+ record.system_server_compilation_millis / 1000,
+ record.cache_space_free_start_mib,
+ record.cache_space_free_end_mib,
+ record.primary_bcp_compilation_millis,
+ record.secondary_bcp_compilation_millis,
+ record.system_server_compilation_millis);
if (bytes_written <= 0) {
*error_msg = android::base::StringPrintf("stats_write returned %d", bytes_written);
return false;
diff --git a/test/Android.bp b/test/Android.bp
index 0679bd6..4289c18 100644
--- a/test/Android.bp
+++ b/test/Android.bp
@@ -1121,6 +1121,8 @@
"2007-virtual-structural-finalizable/src-art/art/Test2007.java",
],
sdk_version: "core_platform",
+ // Make sure that this will be added to the sdk snapshot for S.
+ min_sdk_version: "S",
// Some ART run-tests contain constructs which break ErrorProne checks;
// disable `errorprone` builds.
errorprone: {
@@ -1284,6 +1286,8 @@
"expected_cts_outputs_gen",
],
sdk_version: "core_current",
+ // Make sure that this will be added to the sdk snapshot for S.
+ min_sdk_version: "S",
}
art_cc_test {
