[AI] feat: support feature flag structures for API 37 (cinnamon bun)

[iBotPeaches]

Adds support for the new feature flag related structures in the API 37 ResourceTypes.h spec:

• RES_TABLE_FLAGGED (0x0207) and RES_TABLE_FLAG_LIST (0x0208) chunk types. Flagged chunks are parsed and their contained types merged with the default values (with a warning), instead of being dropped as unexpected chunks.
• ResXMLTreeFlagExt in binary XML start elements. aapt2 strips the android:featureFlag attribute when flattening read/write flagged elements and stores it as a flag ext, so restore the attribute on decode and record the flag so it can be passed back to aapt on build via --feature-flags.

---

Figured giving AI AOSP and saying add the missing changes would help. It gave me this.

[iBotPeaches]

enum  ResXMLTreeExtDescriptor : uint8_t {
    PADDING = 0x00,
    FLAG_INFO = 0x01,
};

struct ResXMLTreeFlagExt {

    // defines the type of the extended element structure
    ResXMLTreeExtDescriptor descriptor;

    // if the flag condition is negated
    bool flag_negated;

    // a hole for 4-byte alignment
    uint16_t reserved;

    // The reference into the string pool that the flag name is stored at
    ResStringPool_ref flag_name;
};

/**
 * A container for other chunks all of whose values are behind a given flag.
 *
 * The flag_name_index is the index of the flag name in the value string pool.
 *
 * When the android runtime encounters this chunk it will check the flag against its current value.
 * If the flag is enabled and flag_negated is false or it is disabled and flag_negated is true, the
 * runtime will then process all of the chunks inside of it normally. Otherwise the entire chunk is
 * skipped.
 *
 * Currently this is chunk should be contained in a ResTable_typeSpec and contain any number of
 * ResTable_type.
 */
struct ResTable_flagged {
  struct ResChunk_header header;

  ResStringPool_ref flag_name_index;
  bool flag_negated;
  uint8_t padding[3];
};

/**
 * A chunk that contains a list of the names of all the read/write flags used by the
 * ResTable_flagged chunks in the file. Specifically, all data after the header is an array of
 * ResStringPool_ref objects for the flag names in no specific order. References use the global
 * values stringpool.
 */
struct ResTable_flag_list {
  struct ResChunk_header header;
};

[iBotPeaches]

The question/comments now is a few things.

1. Are these feature flags stamped at build time or run time? If build time - we don't really care about the feature, because that app was built with that flag set. However, if it was set at build time - AOSP would be smart enough to rip those components out of the APK. Which leads me to believe this is run time set. However, I can't yet any documentation regarding this.

2. It seems the new table entry is contained within ResTable_typeSpec so probably older devices skip it.

3. This code is not good

[iBotPeaches]

cc @IgorEisberg - been messing with this. It seems the XML Tree encodes the information about the flags. Can't really find any real documentation about this feature since its use-cases seem odd to me. If it was a build time flag - why even keep the resources? So it must be a runtime flag you can change that is only affected during launch/unpack of app?

Ever stumbled upon any docs/cases for this feature? Closest thing I found was this FlaggedResourcesTest test in AOSP.

[IgorEisberg]

I'm not sure about ResXMLTreeFlagExt yet, can't find an example.

As for ResTable_flagged and ResTable_flag_list, these chunks are used in ARSC when a RW flag (default) is used on a resource.
Example:
With the following passed to both aapt2 compile and aapt2 link:

--feature-flags test.package.falseFlag:ro=false,test.package.trueFlag:ro=true,test.package.readWriteFlag=false

In this case, test.package.falseFlag and test.package.trueFlag are read-only (basically used in compile-time only) while test.package.readWriteFlag is read-write (encoded and can be turn on/off in runtime).

res/values/bools.xml:

<resources xmlns:android="http://schemas.android.com/apk/res/android">
    <bool name="bool1" android:featureFlag="test.package.falseFlag">true</bool>
    <bool name="bool2" android:featureFlag="test.package.trueFlag">true</bool>
    <bool name="bool3" android:featureFlag="test.package.readWriteFlag">true</bool>
</resources>

Here, bool1 will be stripped (because test.package.falseFlag:ro=false), bool2 will be preserved unflagged (because test.package.trueFlag:ro=true), while bool3 will also be preserved but encoded as flagged using those chunks (because it's an RW flag, i.e. can be dynamically changed in runtime).
Since RO flags are used in compile-time and not encoded, we don't support passing flags with :ro to aapt2, and all flags recorded during decompile are implicitly RW. We also don't pass feature flags to aapt2 compile yet, but it's an easy fix.

Verbose output when decompiling an empty APK built with the above bools.xml only:

I: Loading resource table...
D: Chunk at 0x00000000: RES_TABLE_TYPE (740 bytes)
D: Chunk at 0x0000000c: RES_STRING_POOL_TYPE (64 bytes)
D: Chunk at 0x0000004c: 0x0208 (12 bytes)
W: Skipping unexpected 0x0208 chunk of 12 bytes at 0x0000004c.
D: Chunk at 0x00000058: RES_TABLE_PACKAGE_TYPE (652 bytes)
D: Chunk at 0x00000178: RES_STRING_POOL_TYPE (44 bytes)
D: Chunk at 0x000001a4: RES_STRING_POOL_TYPE (52 bytes)
D: Chunk at 0x000001d8: RES_TABLE_TYPE_SPEC_TYPE (28 bytes)
D: Chunk at 0x000001f4: 0x0207 (128 bytes)
W: Skipping unexpected 0x0207 chunk of 128 bytes at 0x000001f4.
D: Chunk at 0x00000274: RES_TABLE_TYPE_TYPE (112 bytes)
D: End of chunks at 0x000002e4

Our current state: Those chunks and flagged resources are skpped.
Again, neither test.package.falseFlag nor test.package.trueFlag are ever mentioned in ARSC because they were passed as RO, and those "unexpected chunks" are solely due to the RW flag test.package.readWriteFlag.
A text visual of this ARSC:

[iBotPeaches]

Ah so thats an aspect I missed. You have control whether the feature flag is build-time or runtime (readonly). I'll have to make some POC samples to learn more.

[IgorEisberg]

Ah so thats an aspect I missed. You have control whether the feature flag is build-time or runtime (readonly). I'll have to make some POC samples to learn more.

• build-time (readonly) or runtime (read-write).

[iBotPeaches]

Yeah so a readonly flag makes no sense to persist the chunk if false. Just strip it at build time, but based on your image it survived build time.

[IgorEisberg]

Yeah so a readonly flag makes no sense to persist the chunk if false. Just strip it at build time, ...

Exactly.

... but based on your image it survived build time.

Nope. Based on my image, the only flag that survived was test.package.readWriteFlag, which is the only read-write flag, with its initial value set to false.

[IgorEisberg]

What's even more interesting is the nesting on this thing.
Each ResTable_type that has at least one flagged resource gets a secondary ResTable_type wrapped inside RES_TABLE_FLAGGED. This RES_TABLE_FLAGGED has all secondary ResTable_type chunks for the given type ID, including other configs. Shouldn't be a problem for me.

[ResTable_header] chunkSize: 740 headerSize: 12 Package count: 1
  [RES_STRING_POOL_TYPE] chunkSize: 64 headerSize: 28 strings: 1 styles 0 flags: UTF-8|NON-SORTED
  #0 : test.package.readWriteFlag
  [RES_TABLE_FLAG_LIST] chunkSize: 12 headerSize: 8 count: 1
    [0] flag name: 0 'test.package.readWriteFlag'
  [ResTable_package] chunkSize: 652 headerSize: 288 id: 0x7fname: com.igoreisberg.test typeStrings: 288 lastPublicType: 0 keyStrings: 332 lastPublicKey: 0 typeIdOffset: 0
    [RES_STRING_POOL_TYPE] chunkSize: 44 headerSize: 28 strings: 1 styles 0 flags: UTF-16|NON-SORTED
    #0 : bool
    [RES_STRING_POOL_TYPE] chunkSize: 52 headerSize: 28 strings: 2 styles 0 flags: UTF-8|NON-SORTED
    #0 : bool2
    #1 : bool3
    [RES_TABLE_TYPE_SPEC_TYPE] chunkSize: 28 headerSize: 16 id: 0x01 types: 1 entry configs: 3
    Entry qualifier masks:
      (all empty)
    [RES_TABLE_FLAGGED] chunkSize: 128 headerSize: 16 name: test.package.readWriteFlag negated: false
      [ResTable_type] chunkSize: 112 headerSize: 84 id: 0x01 name: bool flags: 0x00 (DENSE) entryCount: 3 entryStart: 96 config: 
        [ResTable_entry] id: 0x0002 name: bool3 keyIndex: 1 size: 8 flags: 0x0010
          [Res_value] size: 8 dataType: 0x12 data: 0xffffffff (true)
    [ResTable_type] chunkSize: 112 headerSize: 84 id: 0x01 name: bool flags: 0x00 (DENSE) entryCount: 3 entryStart: 96 config: 
      [ResTable_entry] id: 0x0001 name: bool2 keyIndex: 0 size: 8 flags: 0x0000
        [Res_value] size: 8 dataType: 0x12 data: 0xffffffff (true)
[End]

[IgorEisberg]

This is going to take a while to plan properly. The thing is that like feature flags create a duplicate type with the same typeId, under the same package, and entries with flags can have the same name as an unflagged one, or with a different flag name or value, like in this AOSP example:

<?xml version="1.0" encoding="utf-8"?>
<resources xmlns:android="http://schemas.android.com/apk/res/android">
    <bool name="bool1">true</bool>
    <bool name="bool1" android:featureFlag="test.package.falseFlag">false</bool>

    <bool name="bool2">false</bool>
    <bool name="bool2" android:featureFlag="test.package.trueFlag">true</bool>

    <bool name="bool3">false</bool>

    <bool name="bool4" android:featureFlag="test.package.falseFlag">true</bool>

    <bool name="bool5">false</bool>
    <bool name="bool5" android:featureFlag="!test.package.falseFlag">true</bool>

    <bool name="bool6">true</bool>
    <bool name="bool6" android:featureFlag="!test.package.trueFlag">false</bool>

    <bool name="bool7">true</bool>
    <bool name="bool8">false</bool>
    <bool name="bool9">true</bool>
    <bool name="bool10">false</bool>
</resources>

So this is a whole another ResType with the same ResConfig but a feature flag in addition to that.
Also, while I see how this is used for value resources, I'm not sure how it's done for file resources.
I also never seen an APK that uses featureFlags like that yet.

[IgorEisberg]

Support is ready in BinaryResourceParser, recording the flags to ApkInfo, and serializing as attributes to values XMLs.
Only adding support to BinaryXmlResourceParser remaining.
Example verbose logging of ARSC decoding:

I: Loading resource table...
D: Chunk at 0x00000000: RES_TABLE_TYPE (2128 bytes)
D: Chunk at 0x0000000c: RES_STRING_POOL_TYPE (164 bytes)
D: Chunk at 0x000000b0: RES_TABLE_FLAG_LIST (16 bytes)
D: Chunk at 0x000000c0: RES_TABLE_PACKAGE_TYPE (1936 bytes)
D: Chunk at 0x000001e0: RES_STRING_POOL_TYPE (104 bytes)
D: Chunk at 0x00000248: RES_STRING_POOL_TYPE (168 bytes)
D: Chunk at 0x000002f0: RES_TABLE_TYPE_SPEC_TYPE (24 bytes)
D: Chunk at 0x00000308: RES_TABLE_FLAGGED (160 bytes)
D: Chunk at 0x00000318: RES_TABLE_TYPE_TYPE (144 bytes)
D: Chunk at 0x000003a8: RES_TABLE_FLAGGED (160 bytes)
D: Chunk at 0x000003b8: RES_TABLE_TYPE_TYPE (144 bytes)
D: Chunk at 0x00000448: RES_TABLE_TYPE_TYPE (196 bytes)
D: Chunk at 0x0000050c: RES_TABLE_TYPE_SPEC_TYPE (28 bytes)
D: Chunk at 0x00000528: RES_TABLE_FLAGGED (128 bytes)
D: Chunk at 0x00000538: RES_TABLE_TYPE_TYPE (112 bytes)
D: Chunk at 0x000005a8: RES_TABLE_TYPE_TYPE (128 bytes)
D: Chunk at 0x00000628: RES_TABLE_TYPE_SPEC_TYPE (32 bytes)
D: Chunk at 0x00000648: RES_TABLE_FLAGGED (132 bytes)
D: Chunk at 0x00000658: RES_TABLE_TYPE_TYPE (116 bytes)
D: Chunk at 0x000006cc: RES_TABLE_FLAGGED (132 bytes)
D: Chunk at 0x000006dc: RES_TABLE_TYPE_TYPE (116 bytes)
D: Chunk at 0x00000750: RES_TABLE_TYPE_TYPE (132 bytes)
D: Chunk at 0x000007d4: RES_TABLE_TYPE_SPEC_TYPE (20 bytes)
D: Chunk at 0x000007e8: RES_TABLE_TYPE_TYPE (104 bytes)
D: End of chunks at 0x00000850

[iBotPeaches]

Its weird they nested more chunks within FLAGGED. I'm guessing older AOSP doesn't even see it and just skips the rest of that RES_TABLE_TYPE_SPEC_TYPE chunk.

[IgorEisberg]

Yeah, so the binary XML part was simple enough, I'm almost done, but I can't really reproduce a binary XML that strips android:featureFlag attribute and replaces it with a FlagExt block.
The test that I can't reproduce:
https://cs.android.com/android/platform/superproject/+/android-latest-release:frameworks/base/tools/aapt2/format/binary/XmlFlattener_test.cpp;l=543

The linking stage uses FlaggedXmlVersioner to make the replacement:
https://cs.android.com/android/platform/superproject/+/android-latest-release:frameworks/base/tools/aapt2/cmd/Link.cpp;l=507

That FlaggedXmlVersioner uses FlagMarkingVisitor to do the seek and replace loop over the nodes:
https://cs.android.com/android/platform/superproject/+/android-latest-release:frameworks/base/tools/aapt2/link/FlaggedXmlVersioner.cpp;l=77

• The sdkVersion checks here might be the key to understanding how to trigger this replacement.

The visitor Visit method:
https://cs.android.com/android/platform/superproject/+/android-latest-release:frameworks/base/tools/aapt2/link/FlaggedXmlVersioner.cpp;l=63

Then the FlagExt block is written to the binary XML here:
https://cs.android.com/android/platform/superproject/+/android-latest-release:frameworks/base/tools/aapt2/format/binary/XmlFlattener.cpp;l=130

[IgorEisberg]

Aha, so the problem is here:

Apktool/brut.apktool/apktool-lib/src/main/java/brut/androlib/res/AaptInvoker.java

Line 162 in 6234db2

cmd.add("--no-auto-version");

Which looks like an official aapt2 bug. The FlaggedXmlVersioner gets no chance to run, at least the non-versioning part:
https://cs.android.com/android/platform/superproject/+/android-latest-release:frameworks/base/tools/aapt2/cmd/Link.cpp;l=489-508
This block only does post-processing without any auto-versioning and should always run regardless of --no-auto-version:
https://cs.android.com/android/platform/superproject/+/android-latest-release:frameworks/base/tools/aapt2/link/FlaggedXmlVersioner.cpp;l=82-91
While this block does both post-processing and auto-versioning so it should consider --no-auto-version:
https://cs.android.com/android/platform/superproject/+/android-latest-release:frameworks/base/tools/aapt2/link/FlaggedXmlVersioner.cpp;l=93-103

Results with this option removed:
Important fields in yaml:

sdkInfo:
  minSdkVersion: 37
  targetSdkVersion: 37
featureFlags:
- test.package.flag1
- test.package.flag2
- test.package.flag3
- test.package.flag4

• This ensures static_cast<ApiVersion>(context->GetMinSdkVersion()) > SDK_BAKLAVA) qualifies and second block of FlaggedXmlVersioner::Process runs.

Source res/layout/flags.xml:

<?xml version="1.0" encoding="utf-8"?>
<LinearLayout
  xmlns:android="http://schemas.android.com/apk/res/android">
    <TextView android:featureFlag="test.package.flag1" />
    <TextView android:featureFlag="!test.package.flag2" />
    <TextView android:featureFlag="test.package.flag3" />
    <LinearLayout>
        <TextView />
        <TextView android:featureFlag="test.package.flag4" />
    </LinearLayout>
</LinearLayout>

Verbose log of decoding the built APK:
(the warnings were added for this temporary debugging)

D: Decoding file res/layout/flags.xml to res/layout/flags.xml
D: Chunk at 0x00000000: RES_XML_TYPE (744 bytes)
D: Chunk at 0x00000008: RES_STRING_POOL_TYPE (228 bytes)
D: Chunk at 0x000000ec: RES_XML_RESOURCE_MAP_TYPE (8 bytes)
D: Chunk at 0x000000f4: RES_XML_START_NAMESPACE_TYPE (24 bytes)
D: Chunk at 0x0000010c: RES_XML_START_ELEMENT_TYPE (36 bytes)
D: Chunk at 0x00000130: RES_XML_START_ELEMENT_TYPE (44 bytes)
W: ResXMLTreeFlagExt at 0x00000154
D: Chunk at 0x0000015c: RES_XML_END_ELEMENT_TYPE (24 bytes)
D: Chunk at 0x00000174: RES_XML_START_ELEMENT_TYPE (44 bytes)
W: ResXMLTreeFlagExt at 0x00000198
D: Chunk at 0x000001a0: RES_XML_END_ELEMENT_TYPE (24 bytes)
D: Chunk at 0x000001b8: RES_XML_START_ELEMENT_TYPE (44 bytes)
W: ResXMLTreeFlagExt at 0x000001dc
D: Chunk at 0x000001e4: RES_XML_END_ELEMENT_TYPE (24 bytes)
D: Chunk at 0x000001fc: RES_XML_START_ELEMENT_TYPE (36 bytes)
D: Chunk at 0x00000220: RES_XML_START_ELEMENT_TYPE (36 bytes)
D: Chunk at 0x00000244: RES_XML_END_ELEMENT_TYPE (24 bytes)
D: Chunk at 0x0000025c: RES_XML_START_ELEMENT_TYPE (44 bytes)
W: ResXMLTreeFlagExt at 0x00000280
D: Chunk at 0x00000288: RES_XML_END_ELEMENT_TYPE (24 bytes)
D: Chunk at 0x000002a0: RES_XML_END_ELEMENT_TYPE (24 bytes)
D: Chunk at 0x000002b8: RES_XML_END_ELEMENT_TYPE (24 bytes)
D: Chunk at 0x000002d0: RES_XML_END_NAMESPACE_TYPE (24 bytes)
D: End of chunks at 0x000002e8

While versioning happens only if I set minSdkVersion to lower than 37 in yaml (technically, should be lower than 36.1, but we can't pass minor version to aapt2), and splits into res/layout/flags.xml (featureFlags attributes stripped, elements with negated flags are kept and non-negated removed, basically treats the flags as if they were read-only and their value set to false) and res/layout-v36.1/flags.xml (processed with FlagExt blocks).

[iBotPeaches]

Nice find. I vaguely remember seeing an aosp change to actually stamp the minor version into arsc, since it had been 0 forever. So the version constraint to a minor may be possible on 37+

[IgorEisberg]

Nice find. I vaguely remember seeing an aosp change to actually stamp the minor version into arsc, since it had been 0 forever. So the version constraint to a minor may be possible on 37+

Yeah, it works now and it's one of the changes I made in ResConfig.

So, this FlagExt block injection doesn't get a chance to run as long as --no-auto-version exits early.
https://cs.android.com/android/platform/superproject/+/android-latest-release:frameworks/base/tools/aapt2/cmd/Link.cpp;l=489-508

This can only be changed by patching aapt2. Other than that, my changes work as expected, now that I got the chance to test it.

[iBotPeaches]

aapt/2 team was always pretty responsive on bugs, just not maybe in the speed of fixing them :) - https://issuetracker.google.com/issues?q=componentid:190923%2B%20status:open%20aapt2

[iBotPeaches]

Closing for #4220