core: updating from upstream Followed instructions from go/nnapi-dep-instructions BUG=b:211342927 TEST=FEATURES=test emerge-amd64-generic nnapi aosp-frameworks-ml-nn Exempt-From-Owner-Approval: this is a fork / mirror of an Android repository and we don't want to modify OWNERS files Change-Id: I8720939ad2818157e33b8f39096b99f494f8878f NOKEYCHECK=True GitOrigin-RevId: cb38e42b0b0301cfd309caae66755a724b04a5a2
diff --git a/Android.bp b/Android.bp index 13e4c02..58af8e4 100644 --- a/Android.bp +++ b/Android.bp
@@ -28,16 +28,18 @@ min_sdk_version: "apex_inherit", header_libs: [ - "liblog_headers", - "libsystem_headers", + "libbase_headers", "libcutils_headers", + "liblog_headers", "libprocessgroup_headers", + "libsystem_headers", ], export_header_lib_headers: [ - "liblog_headers", - "libsystem_headers", + "libbase_headers", "libcutils_headers", + "liblog_headers", "libprocessgroup_headers", + "libsystem_headers", ], export_include_dirs: ["include"], @@ -98,8 +100,6 @@ cflags: ["-fvisibility=protected"], shared_libs: [ - "libprocessgroup", - "libdl", "libvndksupport", ], @@ -130,6 +130,9 @@ enabled: true, }, }, + fuzz_config: { + cc: ["[email protected]"], + }, } cc_library { @@ -178,6 +181,8 @@ "//apex_available:platform", ], min_sdk_version: "apex_inherit", + + afdo: true, } cc_library { @@ -298,13 +303,14 @@ srcs: [ "BitSet_test.cpp", + "Errors_test.cpp", "FileMap_test.cpp", "LruCache_test.cpp", "Mutex_test.cpp", "SharedBuffer_test.cpp", "Singleton_test.cpp", - "String8_test.cpp", "String16_test.cpp", + "String8_test.cpp", "StrongPointer_test.cpp", "Timers_test.cpp", "Unicode_test.cpp", @@ -363,6 +369,7 @@ "-Wall", "-Werror", ], + header_libs: ["libutils_headers"], } cc_test_library { @@ -375,6 +382,7 @@ "-Werror", ], shared_libs: ["libutils_test_singleton1"], + header_libs: ["libutils_headers"], } cc_benchmark {
diff --git a/Errors_test.cpp b/Errors_test.cpp new file mode 100644 index 0000000..0d13bb0 --- /dev/null +++ b/Errors_test.cpp
@@ -0,0 +1,172 @@ +/* + * Copyright (C) 2021 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 "utils/ErrorsMacros.h" + +#include <android-base/result.h> + +#include <gtest/gtest.h> + +using namespace android; + +using android::base::Error; +using android::base::Result; + +status_t success_or_fail(bool success) { + if (success) + return OK; + else + return PERMISSION_DENIED; +} + +TEST(errors, unwrap_or_return) { + auto f = [](bool success, int* val) -> status_t { + OR_RETURN(success_or_fail(success)); + *val = 10; + return OK; + }; + + int val; + status_t s = f(true, &val); + EXPECT_EQ(OK, s); + EXPECT_EQ(10, val); + + val = 0; // reset + status_t q = f(false, &val); + EXPECT_EQ(PERMISSION_DENIED, q); + EXPECT_EQ(0, val); +} + +TEST(errors, unwrap_or_return_result) { + auto f = [](bool success) -> Result<std::string, StatusT> { + OR_RETURN(success_or_fail(success)); + return "hello"; + }; + + auto r = f(true); + EXPECT_TRUE(r.ok()); + EXPECT_EQ("hello", *r); + + auto s = f(false); + EXPECT_FALSE(s.ok()); + EXPECT_EQ(PERMISSION_DENIED, s.error().code()); + EXPECT_EQ("PERMISSION_DENIED", s.error().message()); +} + +TEST(errors, unwrap_or_return_result_int) { + auto f = [](bool success) -> Result<int, StatusT> { + OR_RETURN(success_or_fail(success)); + return 10; + }; + + auto r = f(true); + EXPECT_TRUE(r.ok()); + EXPECT_EQ(10, *r); + + auto s = f(false); + EXPECT_FALSE(s.ok()); + EXPECT_EQ(PERMISSION_DENIED, s.error().code()); + EXPECT_EQ("PERMISSION_DENIED", s.error().message()); +} + +TEST(errors, unwrap_or_fatal) { + OR_FATAL(success_or_fail(true)); + + EXPECT_DEATH(OR_FATAL(success_or_fail(false)), "PERMISSION_DENIED"); +} + +TEST(errors, result_in_status) { + auto f = [](bool success) -> Result<std::string, StatusT> { + if (success) + return "OK"; + else + return Error<StatusT>(PERMISSION_DENIED) << "custom error message"; + }; + + auto g = [&](bool success) -> status_t { + std::string val = OR_RETURN(f(success)); + EXPECT_EQ("OK", val); + return OK; + }; + + status_t a = g(true); + EXPECT_EQ(OK, a); + + status_t b = g(false); + EXPECT_EQ(PERMISSION_DENIED, b); +} + +TEST(errors, conversion_promotion) { + constexpr size_t successVal = 10ull; + auto f = [&](bool success) -> Result<size_t, StatusT> { + OR_RETURN(success_or_fail(success)); + return successVal; + }; + auto s = f(true); + ASSERT_TRUE(s.ok()); + EXPECT_EQ(s.value(), successVal); + auto r = f(false); + EXPECT_TRUE(!r.ok()); + EXPECT_EQ(PERMISSION_DENIED, r.error().code()); +} + +TEST(errors, conversion_promotion_bool) { + constexpr size_t successVal = true; + auto f = [&](bool success) -> Result<bool, StatusT> { + OR_RETURN(success_or_fail(success)); + return successVal; + }; + auto s = f(true); + ASSERT_TRUE(s.ok()); + EXPECT_EQ(s.value(), successVal); + auto r = f(false); + EXPECT_TRUE(!r.ok()); + EXPECT_EQ(PERMISSION_DENIED, r.error().code()); +} + +TEST(errors, conversion_promotion_char) { + constexpr char successVal = 'a'; + auto f = [&](bool success) -> Result<unsigned char, StatusT> { + OR_RETURN(success_or_fail(success)); + return successVal; + }; + auto s = f(true); + ASSERT_TRUE(s.ok()); + EXPECT_EQ(s.value(), successVal); + auto r = f(false); + EXPECT_TRUE(!r.ok()); + EXPECT_EQ(PERMISSION_DENIED, r.error().code()); +} + +struct IntContainer { + // Implicit conversion from int is desired + IntContainer(int val) : val_(val) {} + int val_; +}; + +TEST(errors, conversion_construct) { + constexpr int successVal = 10; + auto f = [&](bool success) -> Result<IntContainer, StatusT> { + OR_RETURN(success_or_fail(success)); + return successVal; + }; + auto s = f(true); + ASSERT_TRUE(s.ok()); + EXPECT_EQ(s.value().val_, successVal); + auto r = f(false); + EXPECT_TRUE(!r.ok()); + EXPECT_EQ(PERMISSION_DENIED, r.error().code()); +}
diff --git a/Looper.cpp b/Looper.cpp index 14e3e35..292425a 100644 --- a/Looper.cpp +++ b/Looper.cpp
@@ -20,6 +20,16 @@ namespace android { +namespace { + +constexpr uint64_t WAKE_EVENT_FD_SEQ = 1; + +epoll_event createEpollEvent(uint32_t events, uint64_t seq) { + return {.events = events, .data = {.u64 = seq}}; +} + +} // namespace + // --- WeakMessageHandler --- WeakMessageHandler::WeakMessageHandler(const wp<MessageHandler>& handler) : @@ -64,7 +74,7 @@ mSendingMessage(false), mPolling(false), mEpollRebuildRequired(false), - mNextRequestSeq(0), + mNextRequestSeq(WAKE_EVENT_FD_SEQ + 1), mResponseIndex(0), mNextMessageUptime(LLONG_MAX) { mWakeEventFd.reset(eventfd(0, EFD_NONBLOCK | EFD_CLOEXEC)); @@ -137,22 +147,17 @@ mEpollFd.reset(); } - // Allocate the new epoll instance and register the wake pipe. + // Allocate the new epoll instance and register the WakeEventFd. mEpollFd.reset(epoll_create1(EPOLL_CLOEXEC)); LOG_ALWAYS_FATAL_IF(mEpollFd < 0, "Could not create epoll instance: %s", strerror(errno)); - struct epoll_event eventItem; - memset(& eventItem, 0, sizeof(epoll_event)); // zero out unused members of data field union - eventItem.events = EPOLLIN; - eventItem.data.fd = mWakeEventFd.get(); - int result = epoll_ctl(mEpollFd.get(), EPOLL_CTL_ADD, mWakeEventFd.get(), &eventItem); + epoll_event wakeEvent = createEpollEvent(EPOLLIN, WAKE_EVENT_FD_SEQ); + int result = epoll_ctl(mEpollFd.get(), EPOLL_CTL_ADD, mWakeEventFd.get(), &wakeEvent); LOG_ALWAYS_FATAL_IF(result != 0, "Could not add wake event fd to epoll instance: %s", strerror(errno)); - for (size_t i = 0; i < mRequests.size(); i++) { - const Request& request = mRequests.valueAt(i); - struct epoll_event eventItem; - request.initEventItem(&eventItem); + for (const auto& [seq, request] : mRequests) { + epoll_event eventItem = createEpollEvent(request.getEpollEvents(), seq); int epollResult = epoll_ctl(mEpollFd.get(), EPOLL_CTL_ADD, request.fd, &eventItem); if (epollResult < 0) { @@ -276,26 +281,28 @@ #endif for (int i = 0; i < eventCount; i++) { - int fd = eventItems[i].data.fd; + const SequenceNumber seq = eventItems[i].data.u64; uint32_t epollEvents = eventItems[i].events; - if (fd == mWakeEventFd.get()) { + if (seq == WAKE_EVENT_FD_SEQ) { if (epollEvents & EPOLLIN) { awoken(); } else { ALOGW("Ignoring unexpected epoll events 0x%x on wake event fd.", epollEvents); } } else { - ssize_t requestIndex = mRequests.indexOfKey(fd); - if (requestIndex >= 0) { + const auto& request_it = mRequests.find(seq); + if (request_it != mRequests.end()) { + const auto& request = request_it->second; int events = 0; if (epollEvents & EPOLLIN) events |= EVENT_INPUT; if (epollEvents & EPOLLOUT) events |= EVENT_OUTPUT; if (epollEvents & EPOLLERR) events |= EVENT_ERROR; if (epollEvents & EPOLLHUP) events |= EVENT_HANGUP; - pushResponse(events, mRequests.valueAt(requestIndex)); + mResponses.push({.seq = seq, .events = events, .request = request}); } else { - ALOGW("Ignoring unexpected epoll events 0x%x on fd %d that is " - "no longer registered.", epollEvents, fd); + ALOGW("Ignoring unexpected epoll events 0x%x for sequence number %" PRIu64 + " that is no longer registered.", + epollEvents, seq); } } } @@ -354,7 +361,8 @@ // we need to be a little careful when removing the file descriptor afterwards. int callbackResult = response.request.callback->handleEvent(fd, events, data); if (callbackResult == 0) { - removeFd(fd, response.request.seq); + AutoMutex _l(mLock); + removeSequenceNumberLocked(response.seq); } // Clear the callback reference in the response structure promptly because we @@ -416,13 +424,6 @@ TEMP_FAILURE_RETRY(read(mWakeEventFd.get(), &counter, sizeof(uint64_t))); } -void Looper::pushResponse(int events, const Request& request) { - Response response; - response.events = events; - response.request = request; - mResponses.push(response); -} - int Looper::addFd(int fd, int ident, int events, Looper_callbackFunc callback, void* data) { return addFd(fd, ident, events, callback ? new SimpleLooperCallback(callback) : nullptr, data); } @@ -449,27 +450,27 @@ { // acquire lock AutoMutex _l(mLock); + // There is a sequence number reserved for the WakeEventFd. + if (mNextRequestSeq == WAKE_EVENT_FD_SEQ) mNextRequestSeq++; + const SequenceNumber seq = mNextRequestSeq++; Request request; request.fd = fd; request.ident = ident; request.events = events; - request.seq = mNextRequestSeq++; request.callback = callback; request.data = data; - if (mNextRequestSeq == -1) mNextRequestSeq = 0; // reserve sequence number -1 - struct epoll_event eventItem; - request.initEventItem(&eventItem); - - ssize_t requestIndex = mRequests.indexOfKey(fd); - if (requestIndex < 0) { + epoll_event eventItem = createEpollEvent(request.getEpollEvents(), seq); + auto seq_it = mSequenceNumberByFd.find(fd); + if (seq_it == mSequenceNumberByFd.end()) { int epollResult = epoll_ctl(mEpollFd.get(), EPOLL_CTL_ADD, fd, &eventItem); if (epollResult < 0) { ALOGE("Error adding epoll events for fd %d: %s", fd, strerror(errno)); return -1; } - mRequests.add(fd, request); + mRequests.emplace(seq, request); + mSequenceNumberByFd.emplace(fd, seq); } else { int epollResult = epoll_ctl(mEpollFd.get(), EPOLL_CTL_MOD, fd, &eventItem); if (epollResult < 0) { @@ -486,7 +487,7 @@ // set from scratch because it may contain an old file handle that we are // now unable to remove since its file descriptor is no longer valid. // No such problem would have occurred if we were using the poll system - // call instead, but that approach carries others disadvantages. + // call instead, but that approach carries other disadvantages. #if DEBUG_CALLBACKS ALOGD("%p ~ addFd - EPOLL_CTL_MOD failed due to file descriptor " "being recycled, falling back on EPOLL_CTL_ADD: %s", @@ -504,71 +505,69 @@ return -1; } } - mRequests.replaceValueAt(requestIndex, request); + const SequenceNumber oldSeq = seq_it->second; + mRequests.erase(oldSeq); + mRequests.emplace(seq, request); + seq_it->second = seq; } } // release lock return 1; } int Looper::removeFd(int fd) { - return removeFd(fd, -1); + AutoMutex _l(mLock); + const auto& it = mSequenceNumberByFd.find(fd); + if (it == mSequenceNumberByFd.end()) { + return 0; + } + return removeSequenceNumberLocked(it->second); } -int Looper::removeFd(int fd, int seq) { +int Looper::removeSequenceNumberLocked(SequenceNumber seq) { #if DEBUG_CALLBACKS - ALOGD("%p ~ removeFd - fd=%d, seq=%d", this, fd, seq); + ALOGD("%p ~ removeFd - fd=%d, seq=%u", this, fd, seq); #endif - { // acquire lock - AutoMutex _l(mLock); - ssize_t requestIndex = mRequests.indexOfKey(fd); - if (requestIndex < 0) { - return 0; - } + const auto& request_it = mRequests.find(seq); + if (request_it == mRequests.end()) { + return 0; + } + const int fd = request_it->second.fd; - // Check the sequence number if one was given. - if (seq != -1 && mRequests.valueAt(requestIndex).seq != seq) { + // Always remove the FD from the request map even if an error occurs while + // updating the epoll set so that we avoid accidentally leaking callbacks. + mRequests.erase(request_it); + mSequenceNumberByFd.erase(fd); + + int epollResult = epoll_ctl(mEpollFd.get(), EPOLL_CTL_DEL, fd, nullptr); + if (epollResult < 0) { + if (errno == EBADF || errno == ENOENT) { + // Tolerate EBADF or ENOENT because it means that the file descriptor was closed + // before its callback was unregistered. This error may occur naturally when a + // callback has the side-effect of closing the file descriptor before returning and + // unregistering itself. + // + // Unfortunately due to kernel limitations we need to rebuild the epoll + // set from scratch because it may contain an old file handle that we are + // now unable to remove since its file descriptor is no longer valid. + // No such problem would have occurred if we were using the poll system + // call instead, but that approach carries other disadvantages. #if DEBUG_CALLBACKS - ALOGD("%p ~ removeFd - sequence number mismatch, oldSeq=%d", - this, mRequests.valueAt(requestIndex).seq); + ALOGD("%p ~ removeFd - EPOLL_CTL_DEL failed due to file descriptor " + "being closed: %s", + this, strerror(errno)); #endif - return 0; + scheduleEpollRebuildLocked(); + } else { + // Some other error occurred. This is really weird because it means + // our list of callbacks got out of sync with the epoll set somehow. + // We defensively rebuild the epoll set to avoid getting spurious + // notifications with nowhere to go. + ALOGE("Error removing epoll events for fd %d: %s", fd, strerror(errno)); + scheduleEpollRebuildLocked(); + return -1; } - - // Always remove the FD from the request map even if an error occurs while - // updating the epoll set so that we avoid accidentally leaking callbacks. - mRequests.removeItemsAt(requestIndex); - - int epollResult = epoll_ctl(mEpollFd.get(), EPOLL_CTL_DEL, fd, nullptr); - if (epollResult < 0) { - if (seq != -1 && (errno == EBADF || errno == ENOENT)) { - // Tolerate EBADF or ENOENT when the sequence number is known because it - // means that the file descriptor was closed before its callback was - // unregistered. This error may occur naturally when a callback has the - // side-effect of closing the file descriptor before returning and - // unregistering itself. - // - // Unfortunately due to kernel limitations we need to rebuild the epoll - // set from scratch because it may contain an old file handle that we are - // now unable to remove since its file descriptor is no longer valid. - // No such problem would have occurred if we were using the poll system - // call instead, but that approach carries others disadvantages. -#if DEBUG_CALLBACKS - ALOGD("%p ~ removeFd - EPOLL_CTL_DEL failed due to file descriptor " - "being closed: %s", this, strerror(errno)); -#endif - scheduleEpollRebuildLocked(); - } else { - // Some other error occurred. This is really weird because it means - // our list of callbacks got out of sync with the epoll set somehow. - // We defensively rebuild the epoll set to avoid getting spurious - // notifications with nowhere to go. - ALOGE("Error removing epoll events for fd %d: %s", fd, strerror(errno)); - scheduleEpollRebuildLocked(); - return -1; - } - } - } // release lock + } return 1; } @@ -656,14 +655,11 @@ return mPolling; } -void Looper::Request::initEventItem(struct epoll_event* eventItem) const { - int epollEvents = 0; +uint32_t Looper::Request::getEpollEvents() const { + uint32_t epollEvents = 0; if (events & EVENT_INPUT) epollEvents |= EPOLLIN; if (events & EVENT_OUTPUT) epollEvents |= EPOLLOUT; - - memset(eventItem, 0, sizeof(epoll_event)); // zero out unused members of data field union - eventItem->events = epollEvents; - eventItem->data.fd = fd; + return epollEvents; } MessageHandler::~MessageHandler() { }
diff --git a/Looper_test.cpp b/Looper_test.cpp index 34f424b..c859f9c 100644 --- a/Looper_test.cpp +++ b/Looper_test.cpp
@@ -8,6 +8,9 @@ #include <utils/Looper.h> #include <utils/StopWatch.h> #include <utils/Timers.h> +#include <thread> +#include <unordered_map> +#include <utility> #include "Looper_test_pipe.h" #include <utils/threads.h> @@ -710,4 +713,123 @@ << "no more messages to handle"; } +class LooperEventCallback : public LooperCallback { + public: + using Callback = std::function<int(int fd, int events)>; + explicit LooperEventCallback(Callback callback) : mCallback(std::move(callback)) {} + int handleEvent(int fd, int events, void* /*data*/) override { return mCallback(fd, events); } + + private: + Callback mCallback; +}; + +// A utility class that allows for pipes to be added and removed from the looper, and polls the +// looper from a different thread. +class ThreadedLooperUtil { + public: + explicit ThreadedLooperUtil(const sp<Looper>& looper) : mLooper(looper), mRunning(true) { + mThread = std::thread([this]() { + while (mRunning) { + static constexpr std::chrono::milliseconds POLL_TIMEOUT(500); + mLooper->pollOnce(POLL_TIMEOUT.count()); + } + }); + } + + ~ThreadedLooperUtil() { + mRunning = false; + mThread.join(); + } + + // Create a new pipe, and return the write end of the pipe and the id used to track the pipe. + // The read end of the pipe is added to the looper. + std::pair<int /*id*/, base::unique_fd> createPipe() { + int pipeFd[2]; + if (pipe(pipeFd)) { + ADD_FAILURE() << "pipe() failed."; + return {}; + } + const int readFd = pipeFd[0]; + const int writeFd = pipeFd[1]; + + int id; + { // acquire lock + std::scoped_lock l(mLock); + + id = mNextId++; + mFds.emplace(id, readFd); + + auto removeCallback = [this, id, readFd](int fd, int events) { + EXPECT_EQ(readFd, fd) << "Received callback for incorrect fd."; + if ((events & Looper::EVENT_HANGUP) == 0) { + return 1; // Not a hangup, keep the callback. + } + removePipe(id); + return 0; // Remove the callback. + }; + + mLooper->addFd(readFd, 0, Looper::EVENT_INPUT, + new LooperEventCallback(std::move(removeCallback)), nullptr); + } // release lock + + return {id, base::unique_fd(writeFd)}; + } + + // Remove the pipe with the given id. + void removePipe(int id) { + std::scoped_lock l(mLock); + if (mFds.find(id) == mFds.end()) { + return; + } + mLooper->removeFd(mFds[id].get()); + mFds.erase(id); + } + + // Check if the pipe with the given id exists and has not been removed. + bool hasPipe(int id) { + std::scoped_lock l(mLock); + return mFds.find(id) != mFds.end(); + } + + private: + sp<Looper> mLooper; + std::atomic<bool> mRunning; + std::thread mThread; + + std::mutex mLock; + std::unordered_map<int, base::unique_fd> mFds GUARDED_BY(mLock); + int mNextId GUARDED_BY(mLock) = 0; +}; + +TEST_F(LooperTest, MultiThreaded_NoUnexpectedFdRemoval) { + ThreadedLooperUtil util(mLooper); + + // Iterate repeatedly to try to recreate a flaky instance. + for (int i = 0; i < 1000; i++) { + auto [firstPipeId, firstPipeFd] = util.createPipe(); + const int firstFdNumber = firstPipeFd.get(); + + // Close the first pipe's fd, causing a fd hangup. + firstPipeFd.reset(); + + // Request to remove the pipe from this test thread. This causes a race for pipe removal + // between the hangup in the looper's thread and this remove request from the test thread. + util.removePipe(firstPipeId); + + // Create the second pipe. Since the fds for the first pipe are closed, this pipe should + // have the same fd numbers as the first pipe because the lowest unused fd number is used. + const auto [secondPipeId, fd] = util.createPipe(); + EXPECT_EQ(firstFdNumber, fd.get()) + << "The first and second fds must match for the purposes of this test."; + + // Wait for unexpected hangup to occur. + std::this_thread::sleep_for(std::chrono::milliseconds(1)); + + ASSERT_TRUE(util.hasPipe(secondPipeId)) << "The second pipe was removed unexpectedly."; + + util.removePipe(secondPipeId); + } + SUCCEED() << "No unexpectedly removed fds."; +} + } // namespace android
diff --git a/RefBase.cpp b/RefBase.cpp index 0518927..99fefee 100644 --- a/RefBase.cpp +++ b/RefBase.cpp
@@ -50,11 +50,6 @@ // log all reference counting operations #define PRINT_REFS 0 -// Continue after logging a stack trace if ~RefBase discovers that reference -// count has never been incremented. Normally we conspicuously crash in that -// case. -#define DEBUG_REFBASE_DESTRUCTION 1 - #if !defined(_WIN32) && !defined(__APPLE__) // CallStack is only supported on linux type platforms. #define CALLSTACK_ENABLED 1 @@ -751,17 +746,19 @@ } } else if (mRefs->mStrong.load(std::memory_order_relaxed) == INITIAL_STRONG_VALUE) { // We never acquired a strong reference on this object. -#if DEBUG_REFBASE_DESTRUCTION - // Treating this as fatal is prone to causing boot loops. For debugging, it's - // better to treat as non-fatal. - ALOGD("RefBase: Explicit destruction, weak count = %d (in %p)", mRefs->mWeak.load(), this); + + // TODO: make this fatal, but too much code in Android manages RefBase with + // new/delete manually (or using other mechanisms such as std::make_unique). + // However, this is dangerous because it's also common for code to use the + // sp<T>(T*) constructor, assuming that if the object is around, it is already + // owned by an sp<>. + ALOGW("RefBase: Explicit destruction, weak count = %d (in %p). Use sp<> to manage this " + "object.", + mRefs->mWeak.load(), this); #if CALLSTACK_ENABLED CallStack::logStack(LOG_TAG); #endif -#else - LOG_ALWAYS_FATAL("RefBase: Explicit destruction, weak count = %d", mRefs->mWeak.load()); -#endif } // For debugging purposes, clear mRefs. Ineffective against outstanding wp's. const_cast<weakref_impl*&>(mRefs) = nullptr;
diff --git a/String16.cpp b/String16.cpp index c42cada..68642d8 100644 --- a/String16.cpp +++ b/String16.cpp
@@ -199,99 +199,59 @@ return NO_MEMORY; } -status_t String16::append(const String16& other) -{ - const size_t myLen = size(); - const size_t otherLen = other.size(); - if (myLen == 0) { - setTo(other); - return OK; - } else if (otherLen == 0) { - return OK; - } - - if (myLen >= SIZE_MAX / sizeof(char16_t) - otherLen) { - android_errorWriteLog(0x534e4554, "73826242"); - abort(); - } - - SharedBuffer* buf = - static_cast<SharedBuffer*>(editResize((myLen + otherLen + 1) * sizeof(char16_t))); - if (buf) { - char16_t* str = (char16_t*)buf->data(); - memcpy(str+myLen, other, (otherLen+1)*sizeof(char16_t)); - mString = str; - return OK; - } - return NO_MEMORY; +status_t String16::append(const String16& other) { + return append(other.string(), other.size()); } -status_t String16::append(const char16_t* chrs, size_t otherLen) -{ +status_t String16::append(const char16_t* chrs, size_t otherLen) { const size_t myLen = size(); - if (myLen == 0) { - setTo(chrs, otherLen); - return OK; - } else if (otherLen == 0) { - return OK; - } - if (myLen >= SIZE_MAX / sizeof(char16_t) - otherLen) { - android_errorWriteLog(0x534e4554, "73826242"); - abort(); - } + if (myLen == 0) return setTo(chrs, otherLen); - SharedBuffer* buf = - static_cast<SharedBuffer*>(editResize((myLen + otherLen + 1) * sizeof(char16_t))); - if (buf) { - char16_t* str = (char16_t*)buf->data(); - memcpy(str+myLen, chrs, otherLen*sizeof(char16_t)); - str[myLen+otherLen] = 0; - mString = str; - return OK; - } - return NO_MEMORY; + if (otherLen == 0) return OK; + + size_t size = myLen; + if (__builtin_add_overflow(size, otherLen, &size) || + __builtin_add_overflow(size, 1, &size) || + __builtin_mul_overflow(size, sizeof(char16_t), &size)) return NO_MEMORY; + + SharedBuffer* buf = static_cast<SharedBuffer*>(editResize(size)); + if (!buf) return NO_MEMORY; + + char16_t* str = static_cast<char16_t*>(buf->data()); + memcpy(str + myLen, chrs, otherLen * sizeof(char16_t)); + str[myLen + otherLen] = 0; + mString = str; + return OK; } -status_t String16::insert(size_t pos, const char16_t* chrs) -{ +status_t String16::insert(size_t pos, const char16_t* chrs) { return insert(pos, chrs, strlen16(chrs)); } -status_t String16::insert(size_t pos, const char16_t* chrs, size_t len) -{ +status_t String16::insert(size_t pos, const char16_t* chrs, size_t otherLen) { const size_t myLen = size(); - if (myLen == 0) { - return setTo(chrs, len); - return OK; - } else if (len == 0) { - return OK; - } + + if (myLen == 0) return setTo(chrs, otherLen); + + if (otherLen == 0) return OK; if (pos > myLen) pos = myLen; - #if 0 - printf("Insert in to %s: pos=%d, len=%d, myLen=%d, chrs=%s\n", - String8(*this).string(), pos, - len, myLen, String8(chrs, len).string()); - #endif + size_t size = myLen; + if (__builtin_add_overflow(size, otherLen, &size) || + __builtin_add_overflow(size, 1, &size) || + __builtin_mul_overflow(size, sizeof(char16_t), &size)) return NO_MEMORY; - SharedBuffer* buf = - static_cast<SharedBuffer*>(editResize((myLen + len + 1) * sizeof(char16_t))); - if (buf) { - char16_t* str = (char16_t*)buf->data(); - if (pos < myLen) { - memmove(str+pos+len, str+pos, (myLen-pos)*sizeof(char16_t)); - } - memcpy(str+pos, chrs, len*sizeof(char16_t)); - str[myLen+len] = 0; - mString = str; - #if 0 - printf("Result (%d chrs): %s\n", size(), String8(*this).string()); - #endif - return OK; - } - return NO_MEMORY; + SharedBuffer* buf = static_cast<SharedBuffer*>(editResize(size)); + if (!buf) return NO_MEMORY; + + char16_t* str = static_cast<char16_t*>(buf->data()); + if (pos < myLen) memmove(str + pos + otherLen, str + pos, (myLen - pos) * sizeof(char16_t)); + memcpy(str + pos, chrs, otherLen * sizeof(char16_t)); + str[myLen + otherLen] = 0; + mString = str; + return OK; } ssize_t String16::findFirst(char16_t c) const
diff --git a/String16_test.cpp b/String16_test.cpp index 7d7230e..c6e6f74 100644 --- a/String16_test.cpp +++ b/String16_test.cpp
@@ -19,7 +19,7 @@ #include <gtest/gtest.h> -namespace android { +using namespace android; ::testing::AssertionResult Char16_tStringEquals(const char16_t* a, const char16_t* b) { if (strcmp16(a, b) != 0) { @@ -224,4 +224,36 @@ EXPECT_STR16EQ(another, u"abcdef"); } -} // namespace android +TEST(String16Test, append) { + String16 s; + EXPECT_EQ(OK, s.append(String16(u"foo"))); + EXPECT_STR16EQ(u"foo", s); + EXPECT_EQ(OK, s.append(String16(u"bar"))); + EXPECT_STR16EQ(u"foobar", s); + EXPECT_EQ(OK, s.append(u"baz", 0)); + EXPECT_STR16EQ(u"foobar", s); + EXPECT_EQ(NO_MEMORY, s.append(u"baz", SIZE_MAX)); + EXPECT_STR16EQ(u"foobar", s); +} + +TEST(String16Test, insert) { + String16 s; + + // Inserting into the empty string inserts at the start. + EXPECT_EQ(OK, s.insert(123, u"foo")); + EXPECT_STR16EQ(u"foo", s); + + // Inserting zero characters at any position is okay, but won't expand the string. + EXPECT_EQ(OK, s.insert(123, u"foo", 0)); + EXPECT_STR16EQ(u"foo", s); + + // Inserting past the end of a non-empty string appends. + EXPECT_EQ(OK, s.insert(123, u"bar")); + EXPECT_STR16EQ(u"foobar", s); + + EXPECT_EQ(OK, s.insert(3, u"!")); + EXPECT_STR16EQ(u"foo!bar", s); + + EXPECT_EQ(NO_MEMORY, s.insert(3, u"", SIZE_MAX)); + EXPECT_STR16EQ(u"foo!bar", s); +}
diff --git a/String8.cpp b/String8.cpp index 8511da9..419b2de 100644 --- a/String8.cpp +++ b/String8.cpp
@@ -313,8 +313,8 @@ if (n > 0) { size_t oldLength = length(); - if ((size_t)n > SIZE_MAX - 1 || - oldLength > SIZE_MAX - (size_t)n - 1) { + if (n > std::numeric_limits<size_t>::max() - 1 || + oldLength > std::numeric_limits<size_t>::max() - n - 1) { return NO_MEMORY; } char* buf = lockBuffer(oldLength + n); @@ -327,21 +327,23 @@ return result; } -status_t String8::real_append(const char* other, size_t otherLen) -{ +status_t String8::real_append(const char* other, size_t otherLen) { const size_t myLen = bytes(); - SharedBuffer* buf = SharedBuffer::bufferFromData(mString) - ->editResize(myLen+otherLen+1); - if (buf) { - char* str = (char*)buf->data(); - mString = str; - str += myLen; - memcpy(str, other, otherLen); - str[otherLen] = '\0'; - return OK; + SharedBuffer* buf; + size_t newLen; + if (__builtin_add_overflow(myLen, otherLen, &newLen) || + __builtin_add_overflow(newLen, 1, &newLen) || + (buf = SharedBuffer::bufferFromData(mString)->editResize(newLen)) == nullptr) { + return NO_MEMORY; } - return NO_MEMORY; + + char* str = (char*)buf->data(); + mString = str; + str += myLen; + memcpy(str, other, otherLen); + str[otherLen] = '\0'; + return OK; } char* String8::lockBuffer(size_t size)
diff --git a/String8_test.cpp b/String8_test.cpp index 9efcc6f..1356cd0 100644 --- a/String8_test.cpp +++ b/String8_test.cpp
@@ -15,13 +15,14 @@ */ #define LOG_TAG "String8_test" + #include <utils/Log.h> #include <utils/String8.h> #include <utils/String16.h> #include <gtest/gtest.h> -namespace android { +using namespace android; class String8Test : public testing::Test { protected: @@ -101,4 +102,15 @@ String8 valid = String8(String16(tmp)); EXPECT_STREQ(valid, "abcdef"); } + +TEST_F(String8Test, append) { + String8 s; + EXPECT_EQ(OK, s.append("foo")); + EXPECT_STREQ("foo", s); + EXPECT_EQ(OK, s.append("bar")); + EXPECT_STREQ("foobar", s); + EXPECT_EQ(OK, s.append("baz", 0)); + EXPECT_STREQ("foobar", s); + EXPECT_EQ(NO_MEMORY, s.append("baz", SIZE_MAX)); + EXPECT_STREQ("foobar", s); }
diff --git a/Threads.cpp b/Threads.cpp index 540dcf4..4dacdc6 100644 --- a/Threads.cpp +++ b/Threads.cpp
@@ -84,12 +84,6 @@ delete t; setpriority(PRIO_PROCESS, 0, prio); - // A new thread will be in its parent's sched group by default, - // so we just need to handle the background case. - if (prio >= ANDROID_PRIORITY_BACKGROUND) { - SetTaskProfiles(0, {"SCHED_SP_BACKGROUND"}, true); - } - if (name) { androidSetThreadName(name); free(name); @@ -305,30 +299,16 @@ int androidSetThreadPriority(pid_t tid, int pri) { int rc = 0; - int lasterr = 0; int curr_pri = getpriority(PRIO_PROCESS, tid); if (curr_pri == pri) { return rc; } - if (pri >= ANDROID_PRIORITY_BACKGROUND) { - rc = SetTaskProfiles(tid, {"SCHED_SP_BACKGROUND"}, true) ? 0 : -1; - } else if (curr_pri >= ANDROID_PRIORITY_BACKGROUND) { - SchedPolicy policy = SP_FOREGROUND; - // Change to the sched policy group of the process. - get_sched_policy(getpid(), &policy); - rc = SetTaskProfiles(tid, {get_sched_policy_profile_name(policy)}, true) ? 0 : -1; - } - - if (rc) { - lasterr = errno; - } - if (setpriority(PRIO_PROCESS, tid, pri) < 0) { rc = INVALID_OPERATION; } else { - errno = lasterr; + errno = 0; } return rc;
diff --git a/Unicode_test.cpp b/Unicode_test.cpp index b92eef8..8b994d9 100644 --- a/Unicode_test.cpp +++ b/Unicode_test.cpp
@@ -100,7 +100,7 @@ 0xF0, 0x90, 0x80, 0x80, // U+10000, 2 UTF-16 character }; - char16_t output[1 + 1 + 1 + 2 + 1]; // Room for NULL + char16_t output[1 + 1 + 1 + 2 + 1]; // Room for null utf8_to_utf16(str, sizeof(str), output, sizeof(output) / sizeof(output[0])); @@ -114,8 +114,7 @@ << "should be first half of surrogate U+10000"; EXPECT_EQ(0xDC00, output[4]) << "should be second half of surrogate U+10000"; - EXPECT_EQ(NULL, output[5]) - << "should be NULL terminated"; + EXPECT_EQ(0, output[5]) << "should be null terminated"; } TEST_F(UnicodeTest, strstr16EmptyTarget) {
diff --git a/include/utils/ErrorsMacros.h b/include/utils/ErrorsMacros.h new file mode 100644 index 0000000..fdc46e6 --- /dev/null +++ b/include/utils/ErrorsMacros.h
@@ -0,0 +1,186 @@ +/* + * Copyright (C) 2021 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 "Errors.h" + +// It would have been better if this file (ErrorsMacros.h) is entirely in utils/Errors.h. However +// that is infeasible as some (actually many) are using utils/Errors.h via the implicit include path +// `system/core/include` [1]. Since such users are not guaranteed to specify the dependency to +// libbase_headers, the following headers from libbase_headers can't be found. +// [1] build/soong/cc/config/global.go#commonGlobalIncludes +#include <android-base/errors.h> +#include <android-base/result.h> +#include <log/log_main.h> + +#include <assert.h> + +namespace android { + +// StatusT is a wrapper class for status_t. Use this type instead of status_t when instantiating +// Result<T, E> and Error<E> template classes. This is required to distinguish status_t from +// other integer-based error code types like errno, and also to provide utility functions like +// print(). +struct StatusT { + StatusT() : val_(OK) {} + StatusT(status_t s) : val_(s) {} + const status_t& value() const { return val_; } + operator status_t() const { return val_; } + std::string print() const { return statusToString(val_); } + + status_t val_; +}; + + +namespace base { +// TODO(b/221235365) StatusT fulfill ResultError contract and cleanup. + +// Unlike typical ResultError types, the underlying code should be a status_t +// instead of a StatusT. We also special-case message generation. +template<> +struct ResultError<StatusT, false> { + ResultError(status_t s) : val_(s) { + LOG_FATAL_IF(s == OK, "Result error should not hold success"); + } + + template <typename T> + operator expected<T, ResultError<StatusT, false>>() const { + return unexpected(*this); + } + + std::string message() const { return statusToString(val_); } + status_t code() const { return val_; } + + private: + const status_t val_; +}; + +template<> +struct ResultError<StatusT, true> { + template <typename T> + ResultError(T&& message, status_t s) : val_(s), message_(std::forward<T>(message)) { + LOG_FATAL_IF(s == OK, "Result error should not hold success"); + } + + ResultError(status_t s) : val_(s) {} + + template <typename T> + operator expected<T, ResultError<StatusT, true>>() const { + return unexpected(*this); + } + + status_t code() const { return val_; } + + std::string message() const { return statusToString(val_) + message_; } + private: + const status_t val_; + std::string message_; +}; + +// Specialization of android::base::OkOrFail<V> for V = status_t. This is used to use the OR_RETURN +// and OR_FATAL macros with statements that yields a value of status_t. See android-base/errors.h +// for the detailed contract. +template <> +struct OkOrFail<status_t> { + static_assert(std::is_same_v<status_t, int>); + // Tests if status_t is a success value of not. + static bool IsOk(const status_t& s) { return s == OK; } + + // Unwrapping status_t in the success case is just asserting that it is actually a success. + // We don't return OK because it would be redundant. + static void Unwrap([[maybe_unused]] status_t&& s) { assert(IsOk(s)); } + + // Consumes status_t when it's a fail value + static OkOrFail<status_t> Fail(status_t&& s) { + assert(!IsOk(s)); + return OkOrFail<status_t>{s}; + } + status_t val_; + + // And converts back into status_t. This is used when OR_RETURN is used in a function whose + // return type is status_t. + operator status_t() && { return val_; } + + // Or converts into Result<T, StatusT>. This is used when OR_RETURN is used in a function whose + // return type is Result<T, StatusT>. + + template <typename T> + operator Result<T, StatusT>() && { + return ResultError<StatusT>(std::move(val_)); + } + + template<typename T> + operator Result<T, StatusT, false>() && { + return ResultError<StatusT, false>(std::move(val_)); + } + + // Since user defined conversion can be followed by numeric conversion, + // we have to specialize all conversions to results holding numeric types to + // avoid conversion ambiguities with the constructor of expected. +#pragma push_macro("SPECIALIZED_CONVERSION") +#define SPECIALIZED_CONVERSION(type)\ + operator Result<type, StatusT>() && { return ResultError<StatusT>(std::move(val_)); }\ + operator Result<type, StatusT, false>() && { return ResultError<StatusT, false>(std::move(val_));} + + SPECIALIZED_CONVERSION(int) + SPECIALIZED_CONVERSION(short int) + SPECIALIZED_CONVERSION(unsigned short int) + SPECIALIZED_CONVERSION(unsigned int) + SPECIALIZED_CONVERSION(long int) + SPECIALIZED_CONVERSION(unsigned long int) + SPECIALIZED_CONVERSION(long long int) + SPECIALIZED_CONVERSION(unsigned long long int) + SPECIALIZED_CONVERSION(bool) + SPECIALIZED_CONVERSION(char) + SPECIALIZED_CONVERSION(unsigned char) + SPECIALIZED_CONVERSION(signed char) + SPECIALIZED_CONVERSION(wchar_t) + SPECIALIZED_CONVERSION(char16_t) + SPECIALIZED_CONVERSION(char32_t) + SPECIALIZED_CONVERSION(float) + SPECIALIZED_CONVERSION(double) + SPECIALIZED_CONVERSION(long double) +#undef SPECIALIZED_CONVERSION +#pragma pop_macro("SPECIALIZED_CONVERSION") + // String representation of the error value. + static std::string ErrorMessage(const status_t& s) { return statusToString(s); } +}; +} // namespace base + + +// These conversions make StatusT directly comparable to status_t in order to +// avoid calling code whenever comparisons are desired. + +template <bool include_message> +bool operator==(const base::ResultError<StatusT, include_message>& l, const status_t& r) { + return (l.code() == r); +} +template <bool include_message> +bool operator==(const status_t& l, const base::ResultError<StatusT, include_message>& r) { + return (l == r.code()); +} + +template <bool include_message> +bool operator!=(const base::ResultError<StatusT, include_message>& l, const status_t& r) { + return (l.code() != r); +} +template <bool include_message> +bool operator!=(const status_t& l, const base::ResultError<StatusT, include_message>& r) { + return (l != r.code()); +} + +} // namespace android
diff --git a/include/utils/Looper.h b/include/utils/Looper.h index 466fbb7..b387d68 100644 --- a/include/utils/Looper.h +++ b/include/utils/Looper.h
@@ -17,15 +17,16 @@ #ifndef UTILS_LOOPER_H #define UTILS_LOOPER_H -#include <utils/threads.h> #include <utils/RefBase.h> -#include <utils/KeyedVector.h> #include <utils/Timers.h> +#include <utils/Vector.h> +#include <utils/threads.h> #include <sys/epoll.h> #include <android-base/unique_fd.h> +#include <unordered_map> #include <utility> namespace android { @@ -421,18 +422,20 @@ static sp<Looper> getForThread(); private: - struct Request { - int fd; - int ident; - int events; - int seq; - sp<LooperCallback> callback; - void* data; + using SequenceNumber = uint64_t; - void initEventItem(struct epoll_event* eventItem) const; - }; + struct Request { + int fd; + int ident; + int events; + sp<LooperCallback> callback; + void* data; + + uint32_t getEpollEvents() const; + }; struct Response { + SequenceNumber seq; int events; Request request; }; @@ -463,9 +466,14 @@ android::base::unique_fd mEpollFd; // guarded by mLock but only modified on the looper thread bool mEpollRebuildRequired; // guarded by mLock - // Locked list of file descriptor monitoring requests. - KeyedVector<int, Request> mRequests; // guarded by mLock - int mNextRequestSeq; + // Locked maps of fds and sequence numbers monitoring requests. + // Both maps must be kept in sync at all times. + std::unordered_map<SequenceNumber, Request> mRequests; // guarded by mLock + std::unordered_map<int /*fd*/, SequenceNumber> mSequenceNumberByFd; // guarded by mLock + + // The sequence number to use for the next fd that is added to the looper. + // The sequence number 0 is reserved for the WakeEventFd. + SequenceNumber mNextRequestSeq; // guarded by mLock // This state is only used privately by pollOnce and does not require a lock since // it runs on a single thread. @@ -474,9 +482,8 @@ nsecs_t mNextMessageUptime; // set to LLONG_MAX when none int pollInner(int timeoutMillis); - int removeFd(int fd, int seq); + int removeSequenceNumberLocked(SequenceNumber seq); // requires mLock void awoken(); - void pushResponse(int events, const Request& request); void rebuildEpollLocked(); void scheduleEpollRebuildLocked();