Previously, Looper internally kept track of the requests to add fds
using the fd value itself. It used an internal sequence number
associated with each request to add a callback to avoid a situation
where a callback is unexpectedly called. However, since it used the fd
value rather than the sequence number to register events into epoll,
there was still a way where unintended hangups could occur.
This exact sequence of events caused unintended behavior in Looper:
- An fd (FD) is added to the looper.
- Looper registers FD into epoll using its FD number.
- FD is closed.
- A hangup event arrives from epoll_wait while the Looper is polling.
Looper is waiting for the lock to process the callback for FD, because
it is blocked by:
- A new fd is created and added to the looper. Since the lowest number
fd is reused, this new fd has the same value as FD.
- The poll request for Looper is now unblocked, so it looks up the
callback associated with FD to process the hangup.
- Since FD is already associated with the new callback, the new callback
is called unintentionally.
This CL uses the sequence number to register fds into epoll. That way,
when we get a hangup from epoll that is associated with a sequence
number, there is no way an unexpected callback will called.
This CL also adds a test to verify this behavior. Due to the
nondeterministic nature of this multi-thread scenario, the test verifies
this scenario repeatedly. Without the fix in Looper, the test is flaky,
but should never fail after the fix.
Bug: 195020232
Bug: 189135695
Test: atest libutils_test --rerun-until-failure
Ignore-AOSP-First: Topic CL aosp/1799831 has a merge conflict with
internal master, resolved in ag/15613419.
Change-Id: Ib4edab7f2407adaef6a1708b29bc52634f25dbb6
A lot of things had moved out of system/core/ without their TEST_MAPPING
entries having gone with them, reducing the amount of presubmit coverage
for those things.
In order to reduce the likelihood of that happening again, I've pushed
all that remained in the system/core/ TEST_MAPPING down into the
individual subdirectories.
Test: treehugger
Change-Id: Ib75d65f9200fa64ae1552471da6fbe5b7023cf94
This reverts commit 70d9fb63e6.
Reason for revert: Outstanding usage of this method removed internally
Change-Id: Idcc00ec261aa1d97f11e47abdb08b10a37b5d20f
Test: Local build; treehugger (which I'll manually confirm runs on the appropriate targets)
String16's ctors already handle static strings, so we don't need a
specialized constructor which accepts StaticString16.
Bug: n/a
Test: libutils_test
Change-Id: I93a1ba70d743ff9c73f113d53ffba73cef6adade
String16 is still in use by AIDL compiler. Because String16 is not
noexcept-move-constructible, the C++ compiler will complain when it is
used with non-copyable types (such as ParcelFileDescriptor).
For example, when vector<Foo> is resized, copy-ctor of Foo is called,
which is not available.
parcelable Foo {
String s;
ParcelFileDescriptor[] pfds;
}
By providing noexcept move-ctor for String16, vector<Foo> can be resized
with no problem.
Btw, copy from StaticString16 is specialized for efficiency and move
from StaticString16 don't need to be different from copy.
Bug: 192136980
Test: libutils_test
Change-Id: I13744a2ceebf5781c3ef7f3a04237a6750b0db0a
Fixes a compiler warning for implicit conversion changes from signed
to unsigned which surfaced when refactoring native input libraries.
Add an explicit cast to avoid adding -Wno-sign-conversion compile
flags.
Test: b libsurfaceflinger_unittest
Bug: b/188792659
Change-Id: I1b9309c3530d6063d5cefcaa31af84d9cebc3f5d
Merged-In: I8866aef7f09ca5173604abe18c586b68bbf12ed6
Fixes a compiler warning for implicit conversion changes from signed
to unsigned which surfaced when refactoring native input libraries.
Add an explicit cast to avoid adding -Wno-sign-conversion compile
flags.
Test: b libsurfaceflinger_unittest
Change-Id: I8866aef7f09ca5173604abe18c586b68bbf12ed6
This function, ironically, is being removed.
Even more amusing, it was never "remove" anyway --- it literally did
the opposite, and removed everything *except* the range you passed to
it, and should probably have been called "keep"!
I'm looking at reimplementing much of libutils, but first I'm improving
test coverage, and literally every test I wrote for this failed. And
then when I fixed the "obvious bugs" in the implementation, I found
there actually were a couple of existing unit tests --- that mostly
served to demonstrate just how counter-intuitive this function was.
Bug: http://b/156999009
Test: treehugger
Change-Id: I41fd85f7c0988070f4039f607d2e57523d862ed9
These tests are only in internal right now, preventing me from removing
this function entirely. So let's get them out of the way...
Test: treehugger
Ignore-AOSP-First: workaround for presubmit merge conflict
Bug: http://b/35267372
Change-Id: Ib5de178ffad833c87e8e93bfd33e85a0ff504ac0
It's not tested, and it's not used. Also remove the fuzzer which is just
wasting CPU cycles.
This gets us to 100% function coverage, 100% line coverage, and N/A
branch coverage for StopWatch.
Test: treehugger
Change-Id: Ib5e08510ef1046a6f2af3f0b8a1c317a8bb39fd4
This dates from when libutils was still part of frameworks, and talks
about stuff that *didn't* move out of frameworks.
Test: treehugger
Change-Id: Icf22eebc417bcb37abdbfdc89e9a3f14894e18c2
Allow LightRefBase to be used with
ANDROID_UTILS_REF_BASE_DISABLE_IMPLICIT_CONSTRUCTION, mainly for
libhwui.
Bug: N/A
Test: libutils_test
Change-Id: I251c874a80f0a069572bc51da45f8f8e74ba6f5b
This test previously expected failure - a machine which does not have
2GiB of memory. However, while today is becoming the past,
2GiB allocations working is no longer a dream of the future!
Fixes: 186569165
Test: libutils_test
Change-Id: I6a9ed608c0989d415b4e7735b8a451b8928b4083
Make it easier to see reference to usage documentation, as requested in
review.
Bug: 184190315
Test: libutils_test
Change-Id: If9056e35b1c7a779dd78f2b986ad10d02f25eaf3
Actually, it looks like it's only toLower() that's used, so let's remove
toUpper() separately, since it's so easy.
Test: treehugger
Change-Id: I8fae9fa513b2a34d5bd6b3f64e9305a1ee3c1ec4
I want to remove these bad ASCII-only APIs completely, but it might be
easier to remove the range variants first.
Test: treehugger
Change-Id: I4c11f959a7bd8e670708cc03281ea72e9c461ff7
If you need to do a case transformation for a Unicode string, you need
to use icu4c. This only worked for ASCII, which is just silly. Luckily
it doesn't seem to be used anywhere.
Test: treehugger
Change-Id: I4a864823ec35a0b57b50909587cc3efac3f531a7
Merged-In: I4a864823ec35a0b57b50909587cc3efac3f531a7
(cherry picked from commit c74d97c854)
This API was tested before, but it wasn't used until it is needed in
libbinder. Previously it passed the tests because wp::operator==
compares weak_ptrs which are never deleted, but it doesn't check the
value of m_ptr as well. This assumes that the RefBase implementation is
self-consistent.
Future considerations:
- add additional CHECK (perf?)
- add an additional optional CHECK?
- update all refbase tests to use an embellished form of this operator
Bug: 184190315
Test: libutils_test, boot and kill process when libbinder is using this
API
Change-Id: I66c97386d769529d5efae48e06775d4b4a344025
If you need to do a case transformation for a Unicode string, you need
to use icu4c. This only worked for ASCII, which is just silly. Luckily
it doesn't seem to be used anywhere.
Test: treehugger
Change-Id: I4a864823ec35a0b57b50909587cc3efac3f531a7
Merged-In: I4a864823ec35a0b57b50909587cc3efac3f531a7
Previously, sp::sp(T*) internally had a static cast, and people
frequently wrote code like this:
sp<A> a = ...;
sp<B> b(a.get()); // implicit static cast
Luckily, none of the other sp constructors have this implicit cast. So,
for explicit code, rather than making those use static_cast internally,
adding an sp::cast function.
Bug: 184190315
Test: use in libbinder
Change-Id: Id205c88d03e16cf85ccb8f493ce88b4bbc65a688
In form, inspired by ANDROID_BASE_UNIQUE_FD_DISABLE_IMPLICIT_CONVERSION.
We get occasional bugs about sp double-ownership. When this flag is
enabled, we have:
- you must construct RefBase objects using sp<>::make
- you must construct wp<> objects by converting them to sp<>
- if you want to convert a raw pointer to an sp<> object (this is
possible since the refcount is used internally, and is used commonly
on this*), then you must use 'assertStrongRefExists' semantics which
aborts if there is no strong ref held. That is, if a client uses
std::make_shared and then calls a function which internally used to
call `sp<T>(this)`, you would now call
`sp<T>::assertStrongRefExists(this)`, and the double ownership
problem would become a runtime error.
Bug: 184190315
Test: libutils_test
Change-Id: Ie18d3146420df1808e3733027070ec234dda4e9d
vendor_available modules were available to product modules.
However, not all vendor_available modules are required to be
available to product modules. Some modules want to be available only
to product modules but not vendor modules.
To cover the requirement, we separate product_available from
vendor_available.
vendor_available will not provide product available module.
Rename libcutils/include_vndk to libcutils/include_outside_system as
it provides the headers to the modules outside the system partition
including /product.
Bug: 150902910
Test: build
Change-Id: I791b5a6a1dc99442065debfce6fa0f54a335fcea
Restrucures this fuzzer to be far more robust and less brittle.
Fix: 163727995
Fix: 163449137
Test: libutils_fuzz_refbase clusterfuzz-testcase-minimized-libutils_fuzz_refbase-5674315436261376
Test: libutils_fuzz_refbase clusterfuzz-testcase-minimized-libutils_fuzz_refbase-5731662044069888
Test: libutils_fuzz_refbase clusterfuzz-testcase-minimized-libutils_fuzz_refbase-5081777218256896
Signed-off-by: Dylan Katz <dylan.katz@leviathansecurity.com>
Change-Id: I239298dc2895a06af5a126e9ca2ae452579e5cc0
* changes:
libprocessgroup_headers: make vendor_ramdisk_available.
libdebuggerd_handler_fallback: make vendor_ramdisk_available.
libcutils: make vendor_ramdisk_available.
libpropertyinfoparser: make vendor_ramdisk_available.
This requires a few other changes to support building libutils
properly. It does appear the windows versions of libutils is
referencing CallStack code, but it doesn't seem to cause any problems.
However, I removed those references completely for the windows build.
Also removed a few extra spaces that seem to have accumulated in the
RefBase.cpp.
Bug: 170465278
Test: Builds and libutils unit tests pass.
Change-Id: Ibeee7791b13636b34bdd592c5420fd91620f752a
Allocating the entire address space is sufficiently unreasonable that
hwasan blows you out of the water if you try to do that. Half the
address space feels like it's large enough to be guaranteed to fail
(which is all I think this intended to test) without being quite so
obviously incorrect that tools like hwasan are going to object...
...except hwasan actually has a 2GiB limit. So let's keep the cleanup,
but also just disable these tests if they're running under hwasan for
now.
Bug: https://issuetracker.google.com/171054277
Test: treehugger
Change-Id: I41f3def5c3a43aa7f9ca3a130b4306ebed659d6a
Missed a review point made, added the license header to FuzzFormatTypes
Test: Compiled code
Signed-off-by: Dylan Katz <dylan.katz@leviathansecurity.com>
Change-Id: Ib8f32b802ec207d0483772c38a0a741e0dd1f151
The class being fuzzed here was too simple for a fuzzer to be effective at finding crashes that were not working as intended.
To resolve the issue noted in 163775285 would require the fuzzer to simply use RWLock exactly as intended, defeating the point of fuzzing it.
Because of this, we have made the decision to remove this class. It should have a fairly small impact on coverage. In the future,
perhaps a better approach would be fuzzing pthread's rwlock methods directly.
Test: Made sure the project still builds without RWLock_fuzz.
Signed-off-by: Dylan Katz <dylan.katz@leviathansecurity.com>
Change-Id: Id5e77d053dc800a982176802dc506d26b54d810f
The input code is the only customer of PropertyMap. For easier
maintenance and eventual removal of it, move it to libinput.
Currently, the caller is responsible for managing the lifecycle of the
returned outMap when calling PropertyMap::load. However, the fact that
the function call allocates new memory is not obvious from the function
signature.
In a separate commit, I will refactor the function to return
Result<unique_ptr<>> to make it less errorprone.
In this commit, only move the files around to make code reviews easier.
Bug: 163171599
Test: atest inputflinger_tests
Change-Id: I316084886c3f09a1776fdb449d2f03d0563b66c1
In https://android-review.googlesource.com/c/platform/system/core/+/1224544
when we introduced a default argument to androidSetThreadPriority(),
we broke C compatibility with this API, and with the header file
in general.
We fix this up by instead introducing a new method that takes
three arguments. This gets this header file compiling for C again,
and keeps this particular API C compatible.
Any C++ callers of the three argument version of
androidSetThreadPriority() will need to switch to using
androidSetThreadPriorityAndPolicy(). Although since this was
a recent change, we believe there is only one such user, which
we are fixing at the same time.
Test: TreeHugger
Bug: 165009705
Merged-In: Iab0b7e6c91a8e32a17ba1b186fd0c2fe96b601e4
Change-Id: Iab0b7e6c91a8e32a17ba1b186fd0c2fe96b601e4
In https://android-review.googlesource.com/c/platform/system/core/+/1224544
when we introduced a default argument to androidSetThreadPriority(),
we broke C compatibility with this API, and with the header file
in general.
We fix this up by instead introducing a new method that takes
three arguments. This gets this header file compiling for C again,
and keeps this particular API C compatible.
Any C++ callers of the three argument version of
androidSetThreadPriority() will need to switch to using
androidSetThreadPriorityAndPolicy(). Although since this was
a recent change, we believe there is only one such user, which
we are fixing at the same time.
Test: TreeHugger
Bug: 165009705
Change-Id: Iab0b7e6c91a8e32a17ba1b186fd0c2fe96b601e4
I stumbled across this because of the `#ifdef __cplusplus` inside an
unguarded `namespace`, but this whole file should be deprecated and
removed in favor of static_assert.
Test: treehugger
Change-Id: I2b65db67e694115e915f0cc9d762b0fd41a7d2af
For encoding errors, this function will return a negative value which
causes problems down the line. Check for an error and return. Also,
integer overflows are guarded.
Bug: 161894517
Test: fuzzer test case
Change-Id: Ia85067d4258bde4b875c832d6223db5dd26b8838
Merged-In: Ia85067d4258bde4b875c832d6223db5dd26b8838
(cherry picked from commit ee22384c54)
For encoding errors, this function will return a negative value which
causes problems down the line. Check for an error and return. Also,
integer overflows are guarded.
Bug: 161894517
Test: fuzzer test case
Change-Id: Ia85067d4258bde4b875c832d6223db5dd26b8838
Merged-In: Ia85067d4258bde4b875c832d6223db5dd26b8838
For encoding errors, this function will return a negative value which
causes problems down the line. Check for an error and return. Also,
integer overflows are guarded.
Bug: 161894517
Test: fuzzer test case
Change-Id: Ia85067d4258bde4b875c832d6223db5dd26b8838
std::bitset<> doesn't have exact replacements for all of the
functionality of BitSet32/64, so there are still places we want to use
these classes.
Bug: 160010896
Test: test BitSet_test.cpp
Change-Id: Ica59a138e014d82139fb6dcea0597a207faecc2a
Since tolower/toupper take and return integer arguments, ascii chars in
the extended range will be converted from positive int values to
negative char values. In order to silence an error here, which was added
recently with integer sanitization here, casting explicitly.
Fixes: 160831549
Test: w/ libutils_fuzz_string8
Change-Id: Iedcd6643f95f84ce662a80e38931d918a200f508
Most of the built-in functions return signed values but BitSet deals
only in unsigned values. We can safely cast these away though as they're
always indices, which can't be negative.
Bug: 160010896
Test: atest inputflinger_tests, atest libinput_tests
Change-Id: I6e0b33972fabcd41ad1c269ea0e2a07b13b33c12
Use the new "data_libs:" to specify test library dependency. This is
like "data:" and it treats compiled libraries as test data files.
The result is libutils_test_singleton{1,2} is picked up by libutils_test
and installed next to the test binary.
Mark the test libs libutils_test_singleton{1,2} as uninstallable. This
prevents installing libutils_test_singleton{1,2} directly and only
permits installing via "data_libs:".
```
$ m libutils_test
testcases
└── libutils_test
├── arm
│ ├── libutils_test
│ ├── libutils_test_singleton1.so
│ └── libutils_test_singleton2.so
├── arm64
│ ├── libutils_test
│ ├── libutils_test_singleton1.so
│ └── libutils_test_singleton2.so
└── libutils_test.config
```
Bug: 124838889
Test: atest libutils_test
Change-Id: I432135e128fc9eedb1b8c18a331957e271d8b0f0
Instead of aborting when FileMap::create detects an overflow, detect the
overflow directly and fail the call.
Bug: 156997193
Test: Ran unit tests, including new unit test that aborted before.
Change-Id: Ie49975b8949fd12bbde14346ec9bbb774ef88a51
Merged-In: Ie49975b8949fd12bbde14346ec9bbb774ef88a51
(cherry picked from commit 68604b9c29)
Instead of aborting when FileMap::create detects an overflow, detect the
overflow directly and fail the call.
Bug: 156997193
Test: Ran unit tests, including new unit test that aborted before.
Change-Id: Ie49975b8949fd12bbde14346ec9bbb774ef88a51
Merged-In: Ie49975b8949fd12bbde14346ec9bbb774ef88a51
(cherry picked from commit 68604b9c29)
Instead of aborting when FileMap::create detects an overflow, detect the
overflow directly and fail the call.
Bug: 156997193
Test: Ran unit tests, including new unit test that aborted before.
Merged-In: Ie49975b8949fd12bbde14346ec9bbb774ef88a51
Change-Id: Ie49975b8949fd12bbde14346ec9bbb774ef88a51
(cherry picked from commit 68604b9c29)
