This time, exclude NDK libraries instead of LLNDK libraries from the
package. This is necessary because there are libraries such as libvndksupport
which are LLNDK but are not accessible to apps.
Original commit message:
> Previously, android_app targets for which a.shouldEmbedJnis(ctx) = true
> (e.g. CtsSelinuxTargetSdk25TestCases) would need to specify all of their
> recursive library dependencies, including for example libc++ when depending
> on the platform libc++. This means unnecessary churn when we add a new
> dependency to libc++ (e.g. libunwind [1]). To avoid the churn and allow
> jni_libs clauses to be simplified, make the build system search for the
> recursive dependencies and automatically include them.
>
> This change allows us to remove code that was previously adding NDK libc++
> as a special case, as it is now covered by the generic code.
>
> Also fix some improper quoting that was exposed as a result of this change
> causing more files to be packaged than before.
>
> [1] https://android-review.googlesource.com/q/topic:%22libunwind-so%22
Bug: 144430859
Test: atest CtsAppOpsTestCases
Test: atest FrameworksNetSmokeTests
Change-Id: I8311ede0b44d7e50b9f272912ead8ef07e82b074
Run the imageMutator between osMutator and archMutator so that
different arch variants can be set for the different partitions.
Bug: 142286466
Test: m checkbuild
Change-Id: I65d05714b75aa462bf9816da60fdc2deda4de593
Merged-In: I65d05714b75aa462bf9816da60fdc2deda4de593
(cherry picked from commit 9c8f687584)
Run the imageMutator between osMutator and archMutator so that
different arch variants can be set for the different partitions.
Bug: 142286466
Test: m checkbuild
Change-Id: I65d05714b75aa462bf9816da60fdc2deda4de593
Previously, android_app targets for which a.shouldEmbedJnis(ctx) = true
(e.g. CtsSelinuxTargetSdk25TestCases) would need to specify all of their
recursive library dependencies, including for example libc++ when depending
on the platform libc++. This means unnecessary churn when we add a new
dependency to libc++ (e.g. libunwind [1]). To avoid the churn and allow
jni_libs clauses to be simplified, make the build system search for the
recursive dependencies and automatically include them.
This change allows us to remove code that was previously adding NDK libc++
as a special case, as it is now covered by the generic code.
Also fix some improper quoting that was exposed as a result of this change
causing more files to be packaged than before.
[1] https://android-review.googlesource.com/q/topic:%22libunwind-so%22
Bug: 144430859
Change-Id: I3d6fbcce75bc108a982eb7483992a4b202056339
The clang-r370808 upgrade contains a change to LLD allow PT_LOAD
segments to reside at non-multiples of the page size in the resulting
object file. https://reviews.llvm.org/rL369344
While this helps reduce the alignment waste and resulting image size, it
has interesting implications for execute only memory (XOM): The runtime
loader will now load code or data from other segments into pages with
different protections than intended.
This would partially defeat execute only (XOM) text sections as the
segment could now overlap with previous and following sections. This
might allow for code or data from the preceding and following sections
(like .eh_frame, and .data.rel.ro) to be executable, and either ends of
.text to be readable.
When the runtime loader (linker[64]) `mmap`s segments from *.so files,
the file offset parameter (see `man 2 mmap`) MUST be a multiple of the
page size. Since the updated LLD can now pack segments in a file (which
helps minimize resulting object file size) (previously, the segment
offsets were page aligned), this has interesting implications.
To appreciate the current bug, consider the following output from
`readelf` before this patch is applied, but after the toolchain upgrade:
```
$ readelf -lSW $OUT/symbols/apex/com.android.runtime/lib64/bionic/libc.so
...
[Nr] Name Type Address Off Size ES Flg Lk Inf Al
...
[13] .eh_frame PROGBITS 000000000002e7c0 02e7c0 013374 00 A 0 0 8
[14] .text PROGBITS 0000000000042b40 041b40 09ecb4 00 AX 0 0 64
[15] .plt PROGBITS 00000000000e1800 0e0800 001f30 00 AX 0 0 16
[16] .data.rel.ro PROGBITS 00000000000e4740 0e2740 005208 00 WA 0 0 32
...
Type Offset VirtAddr PhysAddr FileSiz MemSiz Flg Align
PHDR 0x000040 0x0000000000000040 0x0000000000000040 0x000230 0x000230 R 0x8
LOAD 0x000000 0x0000000000000000 0x0000000000000000 0x041b34 0x041b34 R 0x1000
LOAD 0x041b40 0x0000000000042b40 0x0000000000042b40 0x0a0bf0 0x0a0bf0 E 0x1000
LOAD 0x0e2740 0x00000000000e4740 0x00000000000e4740 0x006720 0x006720 RW 0x1000
...
01 .note.android.ident .note.gnu.build-id .dynsym .gnu.version .gnu.version_d .gnu.version_r .gnu.hash .dynstr .rela.dyn .rela.plt .rodata .eh_frame_hdr .eh_frame
02 .text .plt
03 .data.rel.ro .fini_array .init_array .dynamic .got .got.plt
...
The above output tells us:
1. .text will wind up in the third (02) segment.
2. The third segment will be (LOAD)'ed as (E)xecutable.
3. Because the file (Offset) of the first segment (0x41b40) is NOT a
multiple of the page size, it cannot be passed as the `offset` to
`mmap`. As such it will be rounded down to the first multiple of the
page size, 0x41000.
4. The preceding section (.eh_frame) will be loaded in the preceding
segment (01). It occupies file (Off)set range [(0x2e7c0):0x41b34].
0x41b34 is not explicit in the output, instead you must use the
formula:
Off + Size == End
ie.
0x2e7c0 + 0x13374 == 0x41b34
(This happens to match (FileSiz) of the second segment, which makes
sense as .eh_frame is the final section in the second segment.)
5. mmap'ing file offset 0x41000 when loading the second segment will
include 0x4c0 bytes (0x42000 - 0x41b40) from .text, now mapped as
readable (oops). Suddenly code from .text is now readable (and thus
scannable for gadgets for ROP chains).
6. mmap'ing file offset 0x41000 when loading the third segment will
include 0xb34 bytes (0x41b34 - 0x41000) from .eh_frame, now mapped as
executable (oops). Suddenly data from .eh_frame is now executable
(and thus a potential gadget for ROP chains).
7. mmap'ing file offset 0xe2000 when loading the third segment will
include 0x8CO bytes (0xe3000 - 0xe2740) from .data.rel.ro, now mapped
as executable (oops). Suddenly data from .data.rel.ro is now
executable (and thus a potential gadget for ROP chains).
8. mmap'ing file offset 0xe2000 when loading the fourth segment will
include 0x730 bytes (0xe0800 + 0x1f30 - 0xe2000) from .plt, now
mapped as readable (oops). Suddenly data from .plt is now readable
(and thus scannable for gadgets for ROP chains).
All these oops' could be avoided if the linker placed .text+.plt at page
size aligned file offsets, which is what `-Wl,-z,separate-code` code
does. After this patch, we have:
```
$ readelf -lSW $OUT/symbols/apex/com.android.runtime/lib64/bionic/libc.so
...
Type Offset VirtAddr PhysAddr FileSiz MemSiz Flg Align
PHDR 0x000040 0x0000000000000040 0x0000000000000040 0x000230 0x000230 R 0x8
LOAD 0x000000 0x0000000000000000 0x0000000000000000 0x041b34 0x041b34 R 0x1000
LOAD 0x042000 0x0000000000042000 0x0000000000042000 0x0a0be0 0x0a0be0 E 0x1000
LOAD 0x0e3000 0x00000000000e3000 0x00000000000e3000 0x006720 0x006720 RW 0x1000
```
In the future, we could go back to tightly packing segments in the
binary if the runtime loader was improved to detect the previously
stated problem, and `memset` over the problematic ranges of the freshly
`mmap`ed pages (implying additional startup cost for reduced binary
size). This might save ~6 KB from each native binary, which adds up to
~17 MB for an AOSP image.
Also, prefer
-Wl,--execute-only
rather than
-Wl,-execute-only
Bug: 139945549
Bug: 146144180
Test: readelf -lSW $OUT/symbols/apex/com.android.runtime/lib64/bionic/libc.so
Change-Id: I64527e034ca3c71565ea52ed06f81f75d5216627
Reported-by: Ryan Prichard <rprichard@google.com>
Suggested-by: Fangrui Song <maskray@google.com>
Signed-off-by: Nick Desaulniers <ndesaulniers@google.com>
Use the empty string for the core image variant so that modules
added to imageMutator do not change their build directory.
Bug: 142286466
Test: m checkbuild
Change-Id: Ida4534d9a4d6176236aaa480fed359ce27acfaa1
Merged-In: Ida4534d9a4d6176236aaa480fed359ce27acfaa1
(cherry picked from commit 72d685ee7f45e5393be44ae4159edf083ac918de)
Use the empty string for the core image variant so that modules
added to imageMutator do not change their build directory.
Bug: 142286466
Test: m checkbuild
Change-Id: Ida4534d9a4d6176236aaa480fed359ce27acfaa1
This reverts commit 862eb4648a.
Re-upgrades the compiler to clang-r370808, after first dealing with
regressions in ndk_translate and execute only pages.
Bug: 139945549
Bug: 145807809
Bug: 145827049
Bug: 145825270
Test: atest \
CtsSelinuxTargetSdk27TestCases:android.security.SELinuxTargetSdkTest#testNoExecuteOnly
Test: m ndk_translation_host_unit_tests && \
./out/host/linux-x86/nativetest/ndk_translation_host_unit_tests/ndk_translation_host_unit_tests
Signed-off-by: Nick Desaulniers <ndesaulniers@google.com>
Change-Id: I714b582faffa8c92384818a7b12338d621968548
Previously, adding a new SdkMemberType would require adding a new
sdkMemberListProperty instance to the sdkMemberListProperties as well
as adding a new property into the sdkProperties struct. They are
potential sources of conflict and couple the sdk code with all the
packages that add members to it. This change switched to a
registration model that allows each package to register its sdk
member types decoupling them from the sdk code.
Adds an SdkPropertyName() method to SdkMemberType that specifies the
name of the property to use in the sdk/sdk_snapshot. Also provides
an SdkMemberTypeBase struct to be used by providers of SdkMemberType
implementations.
SdkMemberType instances are registered using the
RegisterSdkMemberType() func which sorts the registered instances
by their SdkPropertyName() to ensure the behavior is consistent and
not affected by order of registration.
When creating a new sdk module a dynamicSdkMemberTypes instance is
created that contains the following:
* A properties struct is created dynamically that contains a field for
each registered SdkMemberType, corresponding to that type's
SdkPropertyName().
* A list of sdkMemberListProperty instances is also created, one for
each registered SdkMemberType.
The dynamicSdkMemberTypes instance is cached using a key that uniquely
identifies the set of registered types just in case new types are
registered after one has been created, e.g. by tests.
Bug: 142918168
Test: m checkbuild
Change-Id: I4bf2bf56a2a49025aa41454048bc1e8ccc6baca2
The exported include dirs includes both source and generated
directories (e.g. containing aidl generated headers). The latter are
always arch specific so if they are present they make all the include
directories arch specific.
This change separates the source and generated include dirs so that
the source include dirs (which are probably not arch specific) can be
optimized separately from the arch specific generated include dirs.
The FilterPathList() func was refactored to extract the more general
FilterPathListPredicate() func.
A number of tests needed to be updated to reflect the more optimal
snapshot creation.
Bug: 142918168
Test: m checkbuild
Change-Id: Id1a23d35a45b250ae2168834f9c2a65c86a5fd77
Previously, code that attempted to optimize the generated .bp rules
treated the properties structure as a single entity. So, a single arch
specific value would cause all properties to be treated as arch
specific. Also, that code was specific to one structure type.
This generalizes the optimization to work with any properties structure
which will be helpful for other multi-variant module types. It also
treats each property separately.
The hasArchSpecificFlags field has been removed from nativeLibInfo and
a commonProperties field has been added instead into which the common
values will be found. File path creation that conditionally prefixed a
path with archType has been replaced with general code that relies on
archType being "" for common properties and filepath.Join(..) ignoring
empty string components.
The common and arch variant properties are always processed. The first
within the context of the .bp module's property set and the latter
within an arch specific property set. There are always some properties
that are arch specific, e.g. outputFile, so there is no need to worry
about an empty arch property set being created.
The archSpecificNativeLibInfo type was renamed nativeLibInfoProperties
as it may not be arch specific.
The printExportedDirCopyCommandsForNativeLibs variable was renamed to
addExportedDirCopyCommandsForNativeLibs as it no longer does any
printing.
Bug: 142918168
Test: m checkbuild
Change-Id: Iad45913299c37fd76fe03ed0ca68bdc68ed76431
Parameterized the cc.librarySdkMemberType to allow it to support
both static and shared libraries. Created two instances, one for shared
and one for static libraries. A follow up change will add support for
libraries that can be both.
Added *librarySdkMemberType to nativeMemberInfo as information from
there is needed when generating the snapshot.
Made organizeVariants() func a method of *librarySdkMemberType so that
it can initialize the new field. Moved it to be with all the other
methods of that type.
Added host and device tests for the new module type.
Bug: 142918168
Test: m nothing
Change-Id: I00b1e8424b9d81f7d15edc4883971d10668ec2cc
AndroidMkEntries now returns multiple AndroidMkEntires so that a module
can emit multiple Make modules if needed.
Bug: 128708192
Test: m
Change-Id: I56b6f76d22943b80329951c5acb80a1b932441ad
Build fuzzer-sanitized libraries with an $ORIGIN DT_RUNPATH. Android's linker
uses DT_RUNPATH, not DT_RPATH. When we deploy cc_fuzz targets and their
libraries to /data/fuzz/<arch>/lib, any transient shared library gets the
DT_RUNPATH from the shared library above it, and not the executable, meaning
that the lookup falls back to the system. Adding the $ORIGIN to the DT_RUNPATH
here means that transient shared libraries can be found colocated with their
parents.
This may have some interesting consequences if:
1. Your fuzz target depends on a shared library which has `sanitize.fuzzer:
false` (as the DT_RUNPATH won't have `$ORIGIN`, and so you may get missing
libraries).
2. A `SANITIZE_TARGET=fuzzer` platform has a shared object in two
different directories (like system vs. ndk) and is depending on the linker
implementation details to resolve this in some manner.
I don't believe either of these instances should reasonably happen in
practise.
Fixes: 145973404
Fixes: 145988908
Test: m example_fuzzer
Change-Id: I94cbf628fc1ce15c43283d72bdabd9817de1fef8
