From 7c4beee9f9ea17fb896ebcdd4789c60ccfb17fab Mon Sep 17 00:00:00 2001 From: Michael Hoisie Date: Fri, 12 Jan 2024 19:18:19 +0000 Subject: [PATCH] Add support for ashmem-host for host Windows Migrate to tmpfile and fileno for temp file operations. These calls are supported on MinGW, and the temp files are automatically cleaned up. A Windows variant of ashmem-host is needed to support CursorWindows on host Windows. In Windows, it is not possible to unlink an open file, so the nlink check in ashmem_validate_stat must be made Unix-only. Test: SQLiteDatabaseTest in Google3 Test: libcutils_test_static on Windows Bug: 317884162 Change-Id: I7fc0f1f49406b01549b7f4d7e138cb3e4d79be72 --- libcutils/Android.bp | 39 +++++++++++---------- libcutils/ashmem-host.cpp | 55 ++++++++++++++++++----------- libcutils/ashmem_base_test.cpp | 63 ++++++++++++++++++++++++++++++++++ libcutils/ashmem_test.cpp | 22 ------------ libcutils/sockets_test.cpp | 2 -- 5 files changed, 119 insertions(+), 62 deletions(-) create mode 100644 libcutils/ashmem_base_test.cpp diff --git a/libcutils/Android.bp b/libcutils/Android.bp index b7752d902..862af841c 100644 --- a/libcutils/Android.bp +++ b/libcutils/Android.bp @@ -156,23 +156,18 @@ cc_library { "fs_config.cpp", ], }, - not_windows: { - srcs: libcutils_nonwindows_sources + [ - "ashmem-host.cpp", - "trace-host.cpp", - ], - }, - windows: { - host_ldlibs: ["-lws2_32"], - + host: { srcs: [ "trace-host.cpp", + "ashmem-host.cpp", ], - + }, + not_windows: { + srcs: libcutils_nonwindows_sources, + }, + windows: { enabled: true, - cflags: [ - "-D_GNU_SOURCE", - ], + host_ldlibs: ["-lws2_32"], }, android: { sanitize: { @@ -241,6 +236,7 @@ cc_library { cc_defaults { name: "libcutils_test_default", srcs: [ + "ashmem_base_test.cpp", "native_handle_test.cpp", "properties_test.cpp", "sockets_test.cpp", @@ -299,20 +295,26 @@ cc_test { cc_defaults { name: "libcutils_test_static_defaults", defaults: ["libcutils_test_default"], - static_libs: [ - "libc", - "libcgrouprc_format", - ] + test_libraries + always_static_test_libraries, stl: "libc++_static", require_root: true, target: { android: { static_executable: true, + static_libs: [ + "libcgrouprc_format", + ] + test_libraries + always_static_test_libraries, + }, + not_windows: { + static_libs: test_libraries + always_static_test_libraries, }, windows: { + static_libs: [ + "libbase", + "libcutils", + "libcutils_sockets", + ], host_ldlibs: ["-lws2_32"], - enabled: true, }, }, @@ -320,6 +322,7 @@ cc_defaults { cc_test { name: "libcutils_test_static", + host_supported: true, test_suites: ["device-tests"], defaults: ["libcutils_test_static_defaults"], } diff --git a/libcutils/ashmem-host.cpp b/libcutils/ashmem-host.cpp index 2ba1eb0c3..9003b76c9 100644 --- a/libcutils/ashmem-host.cpp +++ b/libcutils/ashmem-host.cpp @@ -17,10 +17,13 @@ #include /* - * Implementation of the user-space ashmem API for the simulator, which lacks - * an ashmem-enabled kernel. See ashmem-dev.c for the real ashmem-based version. + * Implementation of the user-space ashmem API for the simulator, which lacks an + * ashmem-enabled kernel. See ashmem-dev.c for the real ashmem-based version. A + * disk-backed temp file is the best option that is consistently supported + * across all host platforms. */ +#include #include #include #include @@ -31,8 +34,10 @@ #include #include #include - #include +#include + +using android::base::unique_fd; static bool ashmem_validate_stat(int fd, struct stat* buf) { int result = fstat(fd, buf); @@ -40,15 +45,20 @@ static bool ashmem_validate_stat(int fd, struct stat* buf) { return false; } - /* - * Check if this is an "ashmem" region. - * TODO: This is very hacky, and can easily break. - * We need some reliable indicator. - */ - if (!(buf->st_nlink == 0 && S_ISREG(buf->st_mode))) { + // Check if this is an ashmem region. Since there's no such thing on the host, + // we can't actually implement that. Check that it's at least a regular file. + if (!S_ISREG(buf->st_mode)) { errno = ENOTTY; return false; } + // In Win32, unlike Unix, the temp file is not unlinked immediately after + // creation. +#if !defined(_WIN32) + if (buf->st_nlink != 0) { + errno = ENOTTY; + return false; + } +#endif return true; } @@ -58,19 +68,24 @@ int ashmem_valid(int fd) { } int ashmem_create_region(const char* /*ignored*/, size_t size) { - char pattern[PATH_MAX]; - snprintf(pattern, sizeof(pattern), "/tmp/android-ashmem-%d-XXXXXXXXX", getpid()); - int fd = mkstemp(pattern); - if (fd == -1) return -1; + // Files returned by tmpfile are automatically removed. + std::unique_ptr tmp(tmpfile(), &fclose); - unlink(pattern); - - if (TEMP_FAILURE_RETRY(ftruncate(fd, size)) == -1) { - close(fd); - return -1; + if (!tmp) { + return -1; } - - return fd; + int fd = fileno(tmp.get()); + if (fd == -1) { + return -1; + } + unique_fd dupfd = unique_fd(dup(fd)); + if (dupfd == -1) { + return -1; + } + if (TEMP_FAILURE_RETRY(ftruncate(dupfd, size)) == -1) { + return -1; + } + return dupfd.release(); } int ashmem_set_prot_region(int /*fd*/, int /*prot*/) { diff --git a/libcutils/ashmem_base_test.cpp b/libcutils/ashmem_base_test.cpp new file mode 100644 index 000000000..c9b14e583 --- /dev/null +++ b/libcutils/ashmem_base_test.cpp @@ -0,0 +1,63 @@ +/* + * Copyright (C) 2024 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 + +#include + +#include +#include +#include + +/* + * Tests in AshmemBaseTest are designed to run on Android as well as host + * platforms (Linux, Mac, Windows). + */ + +#if defined(_WIN32) +static inline size_t getpagesize() { + return 4096; +} +#endif + +using android::base::unique_fd; + +TEST(AshmemBaseTest, BasicTest) { + const size_t size = getpagesize(); + std::vector data(size); + std::generate(data.begin(), data.end(), [n = 0]() mutable { return n++ & 0xFF; }); + + unique_fd fd = unique_fd(ashmem_create_region(nullptr, size)); + ASSERT_TRUE(fd >= 0); + ASSERT_TRUE(ashmem_valid(fd)); + ASSERT_EQ(size, static_cast(ashmem_get_size_region(fd))); + + std::unique_ptr mapped = + android::base::MappedFile::FromFd(fd, 0, size, PROT_READ | PROT_WRITE); + EXPECT_TRUE(mapped.get() != nullptr); + void* region1 = mapped->data(); + EXPECT_TRUE(region1 != nullptr); + + memcpy(region1, data.data(), size); + ASSERT_EQ(0, memcmp(region1, data.data(), size)); + + std::unique_ptr mapped2 = + android::base::MappedFile::FromFd(fd, 0, size, PROT_READ | PROT_WRITE); + EXPECT_TRUE(mapped2.get() != nullptr); + void* region2 = mapped2->data(); + EXPECT_TRUE(region2 != nullptr); + ASSERT_EQ(0, memcmp(region2, data.data(), size)); +} diff --git a/libcutils/ashmem_test.cpp b/libcutils/ashmem_test.cpp index 571b41000..ccbb8c977 100644 --- a/libcutils/ashmem_test.cpp +++ b/libcutils/ashmem_test.cpp @@ -69,28 +69,6 @@ void FillData(std::vector& data) { } } -TEST(AshmemTest, BasicTest) { - const size_t size = getpagesize(); - std::vector data(size); - FillData(data); - - unique_fd fd; - ASSERT_NO_FATAL_FAILURE(TestCreateRegion(size, fd, PROT_READ | PROT_WRITE)); - - void* region1 = nullptr; - ASSERT_NO_FATAL_FAILURE(TestMmap(fd, size, PROT_READ | PROT_WRITE, ®ion1)); - - memcpy(region1, data.data(), size); - ASSERT_EQ(0, memcmp(region1, data.data(), size)); - - EXPECT_EQ(0, munmap(region1, size)); - - void *region2; - ASSERT_NO_FATAL_FAILURE(TestMmap(fd, size, PROT_READ, ®ion2)); - ASSERT_EQ(0, memcmp(region2, data.data(), size)); - EXPECT_EQ(0, munmap(region2, size)); -} - TEST(AshmemTest, ForkTest) { const size_t size = getpagesize(); std::vector data(size); diff --git a/libcutils/sockets_test.cpp b/libcutils/sockets_test.cpp index 1fa40bc6a..f6f9c3628 100644 --- a/libcutils/sockets_test.cpp +++ b/libcutils/sockets_test.cpp @@ -19,8 +19,6 @@ // should be the case for loopback communication, but is not guaranteed. #include -#include -#include #include #include