Fix wp and sp comparison bugs
Make clear() actually clear wp m_refs, so that nulls compare equal.
Make equality consistent with < and >, ensuring that a weak pointer
cannot be both equal to and greater than another.
Don't rely on the built-in < and > operators to correctly order
different objects. The standard does not guarantee that, and there is
a risk of compiler relying on that lack of guarantee.
Remove unnecessary comparison overloads, especially those
comparing a wp<> to an sp<>.
Change the remaining wp<> to sp<> comparisons to check for equivalence
of the mRefs pointer instead of the object address, thus eliminating
the dubious equal comparison result for a dead wp<> and an sp<> that
happen to point to the same object address.
Add comparison tests.
This reverts commit a2a2ad8057.
The original code, and my original CL, both failed to initialize m_refs
in various wp<> constructors. This now became more important, since
comparisons now rely more on m_refs. However I believe it was always
a bug, since some comparisons always relied on m_refs.
Test: Treehugger, boot AOSP, atest RefBase
Bug: 126922090
This reverts commit a2a2ad8057.
Reason for revert: Reapply after constructor fixes.
Change-Id: I2c8917416a2306e36d2b6bb7b397f653020e5688
This reverts commit 029b12ebde.
Reason for revert: There appear to be problems with null comparisons. Reported failure in HwcBufferCacheTest.
Change-Id: I19745bb281dabe8b05c2df3fe95e7be7a49dcd51
Make clear() actually clear wp m_refs, so that nulls compare equal.
Make equality consistent with < and >, ensuring that a weak pointer
cannot be both equal to and greater than another.
Don't rely on the built-in < and > operators to correctly order
different objects. The standard does not guarantee that, and there is
a risk of compiler relying on that lack of guarantee.
Remove unnecessary comparison overloads, especially those
comparing a wp<> to an sp<>.
Change the remaining wp<> to sp<> comparisons to check for equivalence
of the mRefs pointer instead of the object address, thus eliminating
the dubious equal comparison result for a dead wp<> and an sp<> that
happen to point to the same object address.
Add comparison tests.
Test: Treehugger, boot AOSP, atest RefBase
Bug: 126922090
Change-Id: I15911150e0fc85ace2c4b77d337826e12793c690
Also move the singleton test out because our infrastructure can't cope
with tests that have their own test libraries.
Bug: http://b/124838889
Test: atest
Change-Id: Iddce3325a56d44a4288ace2a29921a3e02367413
Move tests in the same directory as the corresponding code, so it's
easier to see what is/isn't tested.
Fix naming of libcutils_tests (plural) to match the singular that's more
common (even though the plural makes more sense to me).
Add these two to system/core/'s TEST_MAPPING.
Remove obsolete AndroidTest.xml.
Fix a flaky (timing-dependent) libcutils test.
Test: ran tests
Change-Id: I7e0a31ff45c8a152562bf66fc97161594249366e
* changes:
DO NOT MERGE: Revert "Revert "Add dependencies on libprocessgroup for sched_policy users""
DO NOT MERGE: Revert "Revert "libcutils: Move sched_policy functions into libprocessgroup""
DO NOT MERGE: Revert "Revert "Add libprocessgroup into VNDK""
This reverts commit 1bef8c550c.
Reason for revert: AOSP is fixed with new vendor image
Change-Id: Ib341ac80e2f88c13a7815a490ea2d9422ebdf55f
Signed-off-by: Suren Baghdasaryan <surenb@google.com>
If this was strlcpy16 it wouldn't be such a bad idea, but strncpy16 is
just an accident waiting to happen...
Test: N/A
Change-Id: Id296fdeadfb9f1f70ddc8fb6d31b3b6b5178a12c
The lint rule google-default-arguments ensures that virtual or override
methods do not have default arguments, because different default values
across the hierarchy chain (e.g. Base::foo(int a=0) v.s.
Derived::foo(int a=10)) can cause confusions.
However, since the uses of the default arguments in libbinder don't lead
to such problem, suppress the warnings.
Test: WITH_TIDY=true WITH_TIDY_CHECKS=google-default-arguments m
libbinder does not show any warning about google-default-arguments
Change-Id: Ica41034ab0ad1037a0facc447ee47e0c77fa9c55
Add FALLTHROUGH_INTENDED for clang compiler.
Bug: 112564944
Test: build with global -Wimplicit-fallthrough.
Change-Id: I40f8bbf94e207c9dd90921e9b762ba51abab5777
sp<>::clear() presents the same risks of heap corruption in the presence
of data races as does assignment. Add the same data race check.
Bug: 112651574
Test: Build and boot AOSP
Change-Id: I75d4eedd756d521920e61ff9187509f9145d4235
This reverts commit b9d0753d2b.
Reason for revert: Re-land with MacOS workaround.
Test: Build (on Linux) and boot AOSP, with weak symbols enabled and disabled.
Change-Id: I5150cd90367178f3b039761dca3bccc9c2987df1
This reverts commit 9d3146af22.
Reason for revert: It appears that weak symbols don't work as expected on MacOS, breaking the MacOS aapt build.
Change-Id: Ica0955106485a7bf2e2c3f09ff7910e230eb4139
Move towards crashing if a normally configured RefBase object is
destroyed without ever incrementing the reference count. We've been
threatening to do this for a long time. The previously last known
violation had been fixed.
This also fixes stack trace printing from RefBase, which had previously
been broken, and which we found necessary to track down further
violations of this rule.
Unfortunately, we found several more violations with the aid of
that fix. After existing CLs are submitted, there are
still some failures, but they are no longer numerous. Thus this CL
doesn't actually crash in the event of a violation, but does log a
verbose stack trace if it encounters one.
Bugs have been filed against the remaining known RefBase client offenders.
We plan to enable crashing on usage violations once those are fixed.
The fix for the stack trace printing breakage unfortunately requires
the use of weak symbols in order to avoid a circular build dependency.
We expect to eventually replace this with execinfo.h functionality.
Some random reformatting, driven by consistency with current formatting
requirements.
Add missing include to BacktraceMap.h.
Bug: 79112958
Bug: 30292291
Test: Boot AOSP, Master
Change-Id: I8151c54560c3b6f75ffc4c48229f0388a2066958
Switch Looper to using unique_fd for its owned file descriptors, to
benefit from fdsan.
Bug: http://b/111560345
Test: treehugger
Change-Id: I8efff7741ed19fd71f82f7e604b4f1c66fc5ea2b
adbd has been built as a static executable since the same binary was
copied to the recovery partition where shared library is not supported.
However, since we now support shared library in the recovery partition,
adbd is built as a dynamic executable.
In addition, the dependency from adbd to libdebuggerd_handler is removed
as debuggerd is handled by the dynamic linker.
A few more modules in /system/core are marked as recovery_available:
true as they are transitive dependencies of the dynamic linker.
This change also includes ld.config.recovery.txt which is the linker
config file for the recovery mode. It is installed to /etc/ld.config.txt
and contains linker namespace config for the dynamic binaries under
/sbin.
Bug: 63673171
Test: `adb reboot recovery; adb devices` shows the device ID
Test: Select 'mount /system' in the recovery mode, then `adb shell`.
$ lsof -p `pidof adbd` shows that libm.so, libc.so, etc. are loaded from
the /lib directory.
Change-Id: I363d5a787863f1677ee40afb5d5841321ddaae77
Libraries that are direct or indirect dependencies of modules installed
to recovery partition (e.g. toybox) are marked as recovery_available:
true. This allows a recovery variant of the lib is created when it is
depended by other recovery or recovery_available modules.
Bug: 67916654
Bug: 64960723
Test: m -j
Change-Id: Ie59155c08890e96ce1893fa3687afcf763d7aea3
These warnings are triggered by -Wextra-semi (and not -Weverything, as
incorrectly mentioned in I49b6e6af483e011632e6a34c0663c93e5c385aa6).
This warning is added to Hidl-generated libs.
To appease clang-format, this patch also fixes some extra newlines.
Test: Build
Change-Id: I63cf5d8ecba46ad87876ff21848bfff04b12ec6e
Upcoming clang update to r328903 adds a new warning:
warning: extra ';' outside of a function is incompatible with C++98
[-Wc++98-compat-extra-semi]
which is included in -Weverything.
We can just delete the extra semicolon (even though we use gnu99), and
save the extra byte.
Test: Build
Change-Id: I49b6e6af483e011632e6a34c0663c93e5c385aa6
CallStack.cpp was part of libutils, but has been separated into a new
library libutilscallstack [1] and then made invisible to vendors [2].
However, this is causing problem to the vendors who have been using
the CallStack class from libutils, because the class is no longer
available to them.
In order to support them, marking libutilscallstack as
'vendor_available: true'.
This reverts commit a32678df55.
[1] https://android-review.googlesource.com/c/platform/system/core/+/591954
[2] https://android-review.googlesource.com/c/platform/system/core/+/599754
Bug: 74600214
Test: m -j
Test: 2016/2017 pixel devices boots to the UI and Photo Editing works
Change-Id: Iaa232ed978378b566a4710e2320ee6ed04572e14
The library isn't intented to be exposed to vendors. It is a platform
private library.
Bug: 72471487
Test: Pixel 2016/2017 boots to the UI. Photo editing works.
Change-Id: Ib0479a43d66d988c2f882688240746bed3213dad
Providing alternative suggestions for using C++ stdlib types
instead of libutils types:
- higher interoperability
- fewer "legacy" quirks
- ability to use stl algorithms
- high optimization levels
Test: none
Change-Id: If81aa9982ca0ad229fa13c8142387906981b054d
This commit removes unused class declaration for SharedBuffer and
TextOutput. SharedBuffer has become internal implementation details
since 282efae9c. TextOutput usages have been removed since 9eb2a3b1.
Test: AOSP and master build w/o problems
Change-Id: I1871c4919a46f1ea8f41fb7eb79b4dc800b6f6f4
In the original code when target is an empty string
strlen16() would start reading the memory until a
"terminating null" (that is, zero) character is found.
This may happen because "*target++", at line 300,
would increment the pointer beyond the actual string.
Signed-off-by: Branislav Rankov <branislav.rankov@arm.com>
Signed-off-by: Tamas Petz <tamas.petz@arm.com>
Test: libutils_tests --gtest_filter=UnicodeTest.strstr16*
Change-Id: I213ffe061057c7fa8f34b68881e106a709557dcd
- make liblog dependency for all library builds (this is required
for files like String8.cpp)
- export liblog headers (because they are used in many header files).
Test: less libraries fail with BOARD_VNDK_VERSION := current
Merged-In: Iecb9cd00deb3a9056ea63c4a087afdb80a51a2b8
Change-Id: Iecb9cd00deb3a9056ea63c4a087afdb80a51a2b8
(cherry picked from commit 43e20cac7b)
Now in Android.bp files, target.linux applies to all targets running a
linux kernel (android, linux_glibc, linux_bionic). So common
flags/sources/etc can be combined instead of copying them to each
target.
Test: m
Change-Id: If7ad138ea1c540c160731f86b6ccc0daa5c69b83
- make liblog dependency for all library builds (this is required
for files like String8.cpp)
- export liblog headers (because they are used in many header files).
Test: less libraries fail with BOARD_VNDK_VERSION := current
Merged-In: Iecb9cd00deb3a9056ea63c4a087afdb80a51a2b8
Change-Id: Iecb9cd00deb3a9056ea63c4a087afdb80a51a2b8
(cherry picked from commit 43e20cac7b)
The analyzer is known to be very conservative in the face of atomic
operations (e.g. https://bugs.llvm.org/show_bug.cgi?id=34365); this case
is no different.
It's concerned that `delete this;` might read a different value for
`flags`, and proceed to delete `refs`. Since there are many comments
explaining why this won't happen (and it all looks sane to me), use a
NOLINT to silence this warning.
Analyzer warning:
system/core/libutils/RefBase.cpp:445:5: warning: Use of memory after it
is freed
Bug: 27101951
Test: mma. Use-after-free warning is gone.
Change-Id: Ic1623971bd1bad546fbb12a79439116c89a6762d
In the future, target.linux will apply to all targets running a linux kernel
(android, linux_glibc, linux_bionic). So move all current users to the specific
linux_glibc.
There will be another cleanup pass later that will move some instances back to
target.linux if the properties should be shared with target.android and
target.linux_bionic, but target.linux needs to be removed first.
Test: out/soong/build.ninja identical before/after
Change-Id: I72ef34689c60ce547cab2898e354b027e335f6a1
Exempt-From-Owner-Approval: build system cleanup
libdl is part of system_shared_libs now. -ldl -lpthread -lm are now defaults
for host_ldlibs on Linux and Darwin. -lrt is a default for host_ldlibs on
Linux.
Test: m host
Change-Id: I0b3c147b00a8ab6ff289b85db55b88836c905f5c
Exempt-From-Owner-Approval: build system cleanup
Without an explicit check, the return value can wrap around and return
a value that is far too small to hold the data from the resulting
conversion.
No CTS test is provided because it would need to allocate at least
SSIZE_MAX / 2 bytes of UTF-16 data, which is unreasonable on 64-bit
devices.
Bug: 37723026
Test: run cts -p android.security
Change-Id: I56ba5e31657633b7f33685dd8839d4b3b998e586
As a VNDK-SP module, Android.bp must have 'vndk' tag as well as
'vendor_available: true'.
The 'vndk' tag for VNDK-SP formated as follows:
vndk: {
enabled: true,
support_system_process: true,
},
VNDK-SP modules will be installed both in system/lib(64) as normal
and in system/lib(64)/vndk-sp as a vendor variant.
Bug: 63866913
Test: build and boot with BOARD_VNDK_VERSION=current
Merged-In: I51fe0859f63ad58b7b91909e7d7d4206443228cd
Change-Id: I51fe0859f63ad58b7b91909e7d7d4206443228cd
(cherry picked from commit aeb68e86e4)
Add NOLINT comment to work around clang-tidy
error in checking macro arguments used in
type expressions.
Bug: 28705665
Test: make with WITH_TIDY=1 WITH_TIDY_CHECKS=-*,misc-macro-* \
WITH_TIDY_FLAGS=-header-filter=system/core/.*
Change-Id: I7619978c1804e151a11a8b0477e80076bcf21cab
This is baked into too many prebuilts. Perhaps
eventually it can be removed, but this is very
unlikely.
Change-Id: Ie3f0095a7b48c8b60e548cf2d32d2d95c108b5fb
Fixes: 35363681
