Fix shared library bug in bag attributes

A ResTable_map entry has a name attribute, which
could be a dynamic reference if it comes from
a shared library. It was not being patched with
the correct package id.

Bug:16795890
Change-Id: Ia8df6a943269b2fefb2132c3ed74eb1997d7701b

Author: Adam Lesinski

core/jni/android_util_AssetManager.cpp

@@ -1021,7 +1021,7 @@ static jboolean android_content_AssetManager_resolveAttrs(JNIEnv* env, jobject c
         return JNI_FALSE;
     }
 
-    DEBUG_STYLES(LOGI("APPLY STYLE: theme=0x%x defStyleAttr=0x%x defStyleRes=0x%x",
+    DEBUG_STYLES(ALOGI("APPLY STYLE: theme=0x%x defStyleAttr=0x%x defStyleRes=0x%x",
         themeToken, defStyleAttr, defStyleRes));
 
     ResTable::Theme* theme = reinterpret_cast<ResTable::Theme*>(themeToken);
@@ -1089,7 +1089,7 @@ static jboolean android_content_AssetManager_resolveAttrs(JNIEnv* env, jobject c
     for (jsize ii=0; ii<NI; ii++) {
         const uint32_t curIdent = (uint32_t)src[ii];
 
-        DEBUG_STYLES(LOGI("RETRIEVING ATTR 0x%08x...", curIdent));
+        DEBUG_STYLES(ALOGI("RETRIEVING ATTR 0x%08x...", curIdent));
 
         // Try to find a value for this attribute...  we prioritize values
         // coming from, first XML attributes, then XML style, then default
@@ -1104,7 +1104,7 @@ static jboolean android_content_AssetManager_resolveAttrs(JNIEnv* env, jobject c
             block = -1;
             value.dataType = Res_value::TYPE_ATTRIBUTE;
             value.data = srcValues[ii];
-            DEBUG_STYLES(LOGI("-> From values: type=0x%x, data=0x%08x",
+            DEBUG_STYLES(ALOGI("-> From values: type=0x%x, data=0x%08x",
                     value.dataType, value.data));
         }
 
@@ -1118,7 +1118,7 @@ static jboolean android_content_AssetManager_resolveAttrs(JNIEnv* env, jobject c
                 block = defStyleEnt->stringBlock;
                 typeSetFlags = defStyleTypeSetFlags;
                 value = defStyleEnt->map.value;
-                DEBUG_STYLES(LOGI("-> From def style: type=0x%x, data=0x%08x",
+                DEBUG_STYLES(ALOGI("-> From def style: type=0x%x, data=0x%08x",
                         value.dataType, value.data));
             }
             defStyleEnt++;
@@ -1130,14 +1130,14 @@ static jboolean android_content_AssetManager_resolveAttrs(JNIEnv* env, jobject c
             ssize_t newBlock = theme->resolveAttributeReference(&value, block,
                     &resid, &typeSetFlags, &config);
             if (newBlock >= 0) block = newBlock;
-            DEBUG_STYLES(LOGI("-> Resolved attr: type=0x%x, data=0x%08x",
+            DEBUG_STYLES(ALOGI("-> Resolved attr: type=0x%x, data=0x%08x",
                     value.dataType, value.data));
         } else {
             // If we still don't have a value for this attribute, try to find
             // it in the theme!
             ssize_t newBlock = theme->getAttribute(curIdent, &value, &typeSetFlags);
             if (newBlock >= 0) {
-                DEBUG_STYLES(LOGI("-> From theme: type=0x%x, data=0x%08x",
+                DEBUG_STYLES(ALOGI("-> From theme: type=0x%x, data=0x%08x",
                         value.dataType, value.data));
                 newBlock = res.resolveReference(&value, block, &resid,
                         &typeSetFlags, &config);
@@ -1148,19 +1148,19 @@ static jboolean android_content_AssetManager_resolveAttrs(JNIEnv* env, jobject c
                 }
 #endif
                 if (newBlock >= 0) block = newBlock;
-                DEBUG_STYLES(LOGI("-> Resolved theme: type=0x%x, data=0x%08x",
+                DEBUG_STYLES(ALOGI("-> Resolved theme: type=0x%x, data=0x%08x",
                         value.dataType, value.data));
             }
         }
 
         // Deal with the special @null value -- it turns back to TYPE_NULL.
         if (value.dataType == Res_value::TYPE_REFERENCE && value.data == 0) {
-            DEBUG_STYLES(LOGI("-> Setting to @null!"));
+            DEBUG_STYLES(ALOGI("-> Setting to @null!"));
             value.dataType = Res_value::TYPE_NULL;
             block = -1;
         }
 
-        DEBUG_STYLES(LOGI("Attribute 0x%08x: type=0x%x, data=0x%08x",
+        DEBUG_STYLES(ALOGI("Attribute 0x%08x: type=0x%x, data=0x%08x",
                 curIdent, value.dataType, value.data));
 
         // Write the final value back to Java.
@@ -1215,7 +1215,7 @@ static jboolean android_content_AssetManager_applyStyle(JNIEnv* env, jobject cla
         return JNI_FALSE;
     }
 
-    DEBUG_STYLES(LOGI("APPLY STYLE: theme=0x%x defStyleAttr=0x%x defStyleRes=0x%x xml=0x%x",
+    DEBUG_STYLES(ALOGI("APPLY STYLE: theme=0x%x defStyleAttr=0x%x defStyleRes=0x%x xml=0x%x",
         themeToken, defStyleAttr, defStyleRes, xmlParserToken));
 
     ResTable::Theme* theme = reinterpret_cast<ResTable::Theme*>(themeToken);
@@ -1313,7 +1313,7 @@ static jboolean android_content_AssetManager_applyStyle(JNIEnv* env, jobject cla
     for (jsize ii=0; ii<NI; ii++) {
         const uint32_t curIdent = (uint32_t)src[ii];
 
-        DEBUG_STYLES(LOGI("RETRIEVING ATTR 0x%08x...", curIdent));
+        DEBUG_STYLES(ALOGI("RETRIEVING ATTR 0x%08x...", curIdent));
 
         // Try to find a value for this attribute...  we prioritize values
         // coming from, first XML attributes, then XML style, then default
@@ -1334,7 +1334,7 @@ static jboolean android_content_AssetManager_applyStyle(JNIEnv* env, jobject cla
             xmlParser->getAttributeValue(ix, &value);
             ix++;
             curXmlAttr = xmlParser->getAttributeNameResID(ix);
-            DEBUG_STYLES(LOGI("-> From XML: type=0x%x, data=0x%08x",
+            DEBUG_STYLES(ALOGI("-> From XML: type=0x%x, data=0x%08x",
                     value.dataType, value.data));
         }
 
@@ -1348,7 +1348,7 @@ static jboolean android_content_AssetManager_applyStyle(JNIEnv* env, jobject cla
                 block = styleEnt->stringBlock;
                 typeSetFlags = styleTypeSetFlags;
                 value = styleEnt->map.value;
-                DEBUG_STYLES(LOGI("-> From style: type=0x%x, data=0x%08x",
+                DEBUG_STYLES(ALOGI("-> From style: type=0x%x, data=0x%08x",
                         value.dataType, value.data));
             }
             styleEnt++;
@@ -1364,7 +1364,7 @@ static jboolean android_content_AssetManager_applyStyle(JNIEnv* env, jobject cla
                 block = defStyleEnt->stringBlock;
                 typeSetFlags = defStyleTypeSetFlags;
                 value = defStyleEnt->map.value;
-                DEBUG_STYLES(LOGI("-> From def style: type=0x%x, data=0x%08x",
+                DEBUG_STYLES(ALOGI("-> From def style: type=0x%x, data=0x%08x",
                         value.dataType, value.data));
             }
             defStyleEnt++;
@@ -1376,14 +1376,14 @@ static jboolean android_content_AssetManager_applyStyle(JNIEnv* env, jobject cla
             ssize_t newBlock = theme->resolveAttributeReference(&value, block,
                     &resid, &typeSetFlags, &config);
             if (newBlock >= 0) block = newBlock;
-            DEBUG_STYLES(LOGI("-> Resolved attr: type=0x%x, data=0x%08x",
+            DEBUG_STYLES(ALOGI("-> Resolved attr: type=0x%x, data=0x%08x",
                     value.dataType, value.data));
         } else {
             // If we still don't have a value for this attribute, try to find
             // it in the theme!
             ssize_t newBlock = theme->getAttribute(curIdent, &value, &typeSetFlags);
             if (newBlock >= 0) {
-                DEBUG_STYLES(LOGI("-> From theme: type=0x%x, data=0x%08x",
+                DEBUG_STYLES(ALOGI("-> From theme: type=0x%x, data=0x%08x",
                         value.dataType, value.data));
                 newBlock = res.resolveReference(&value, block, &resid,
                         &typeSetFlags, &config);
@@ -1394,19 +1394,19 @@ static jboolean android_content_AssetManager_applyStyle(JNIEnv* env, jobject cla
                 }
 #endif
                 if (newBlock >= 0) block = newBlock;
-                DEBUG_STYLES(LOGI("-> Resolved theme: type=0x%x, data=0x%08x",
+                DEBUG_STYLES(ALOGI("-> Resolved theme: type=0x%x, data=0x%08x",
                         value.dataType, value.data));
             }
         }
 
         // Deal with the special @null value -- it turns back to TYPE_NULL.
         if (value.dataType == Res_value::TYPE_REFERENCE && value.data == 0) {
-            DEBUG_STYLES(LOGI("-> Setting to @null!"));
+            DEBUG_STYLES(ALOGI("-> Setting to @null!"));
             value.dataType = Res_value::TYPE_NULL;
             block = kXmlBlock;
         }
 
-        DEBUG_STYLES(LOGI("Attribute 0x%08x: type=0x%x, data=0x%08x",
+        DEBUG_STYLES(ALOGI("Attribute 0x%08x: type=0x%x, data=0x%08x",
                 curIdent, value.dataType, value.data));
 
         // Write the final value back to Java.

libs/androidfw/ResourceTypes.cpp

@@ -3907,7 +3907,17 @@ ssize_t ResTable::getBagLocked(uint32_t resID, const bag_entry** outBag,
         map = (const ResTable_map*)(((const uint8_t*)entry.type) + curOff);
         N++;
 
-        const uint32_t newName = htodl(map->name.ident);
+        uint32_t newName = htodl(map->name.ident);
+        if (!Res_INTERNALID(newName)) {
+            // Attributes don't have a resource id as the name. They specify
+            // other data, which would be wrong to change via a lookup.
+            if (grp->dynamicRefTable.lookupResourceId(&newName) != NO_ERROR) {
+                ALOGE("Failed resolving ResTable_map name at %d with ident 0x%08x",
+                        (int) curOff, (int) newName);
+                return UNKNOWN_ERROR;
+            }
+        }
+
         bool isInside;
         uint32_t oldName = 0;
         while ((isInside=(curEntry < set->numAttrs))
@@ -5856,11 +5866,11 @@ status_t DynamicRefTable::lookupResourceId(uint32_t* resId) const {
     // Do a proper lookup.
     uint8_t translatedId = mLookupTable[packageId];
     if (translatedId == 0) {
-        ALOGV("DynamicRefTable(0x%02x): No mapping for build-time package ID 0x%02x.",
+        ALOGE("DynamicRefTable(0x%02x): No mapping for build-time package ID 0x%02x.",
                 (uint8_t)mAssignedPackageId, (uint8_t)packageId);
         for (size_t i = 0; i < 256; i++) {
             if (mLookupTable[i] != 0) {
-                ALOGV("e[0x%02x] -> 0x%02x", (uint8_t)i, mLookupTable[i]);
+                ALOGE("e[0x%02x] -> 0x%02x", (uint8_t)i, mLookupTable[i]);
             }
         }
         return UNKNOWN_ERROR;

libs/androidfw/tests/Idmap_test.cpp

@@ -19,7 +19,7 @@
 #include <utils/String8.h>
 #include <utils/String16.h>
 #include "TestHelpers.h"
-#include "data/R.h"
+#include "data/basic/R.h"
 
 #include <gtest/gtest.h>
 
@@ -70,7 +70,7 @@ TEST_F(IdmapTest, canLoadIdmap) {
 
 TEST_F(IdmapTest, overlayOverridesResourceValue) {
     Res_value val;
-    ssize_t block = mTargetTable.getResource(R::string::test2, &val, false);
+    ssize_t block = mTargetTable.getResource(base::R::string::test2, &val, false);
     ASSERT_GE(block, 0);
     ASSERT_EQ(Res_value::TYPE_STRING, val.dataType);
     const ResStringPool* pool = mTargetTable.getTableStringBlock(block);
@@ -84,7 +84,7 @@ TEST_F(IdmapTest, overlayOverridesResourceValue) {
 
     ASSERT_EQ(NO_ERROR, mTargetTable.add(overlay_arsc, overlay_arsc_len, mData, mDataSize));
 
-    ssize_t newBlock = mTargetTable.getResource(R::string::test2, &val, false);
+    ssize_t newBlock = mTargetTable.getResource(base::R::string::test2, &val, false);
     ASSERT_GE(newBlock, 0);
     ASSERT_NE(block, newBlock);
     ASSERT_EQ(Res_value::TYPE_STRING, val.dataType);
@@ -101,7 +101,7 @@ TEST_F(IdmapTest, overlaidResourceHasSameName) {
     ASSERT_EQ(NO_ERROR, mTargetTable.add(overlay_arsc, overlay_arsc_len, mData, mDataSize));
 
     ResTable::resource_name resName;
-    ASSERT_TRUE(mTargetTable.getResourceName(R::array::integerArray1, false, &resName));
+    ASSERT_TRUE(mTargetTable.getResourceName(base::R::array::integerArray1, false, &resName));
 
     ASSERT_TRUE(resName.package != NULL);
     ASSERT_TRUE(resName.type != NULL);

libs/androidfw/tests/ResTable_test.cpp

@@ -19,7 +19,8 @@
 #include <utils/String8.h>
 #include <utils/String16.h>
 #include "TestHelpers.h"
-#include "data/R.h"
+#include "data/basic/R.h"
+#include "data/lib/R.h"
 
 #include <gtest/gtest.h>
 
@@ -34,6 +35,8 @@ namespace {
  */
 #include "data/basic/basic_arsc.h"
 
+#include "data/lib/lib_arsc.h"
+
 enum { MAY_NOT_BE_BAG = false };
 
 TEST(ResTableTest, shouldLoadSuccessfully) {
@@ -46,7 +49,7 @@ TEST(ResTableTest, simpleTypeIsRetrievedCorrectly) {
     ASSERT_EQ(NO_ERROR, table.add(basic_arsc, basic_arsc_len));
 
     Res_value val;
-    ssize_t block = table.getResource(R::string::test1, &val, MAY_NOT_BE_BAG);
+    ssize_t block = table.getResource(base::R::string::test1, &val, MAY_NOT_BE_BAG);
 
     ASSERT_GE(block, 0);
     ASSERT_EQ(Res_value::TYPE_STRING, val.dataType);
@@ -66,76 +69,91 @@ TEST(ResTableTest, resourceNameIsResolved) {
                                              0, 0,
                                              defPackage.string(), defPackage.size());
     ASSERT_NE(uint32_t(0x00000000), resID);
-    ASSERT_EQ(R::string::test1, resID);
+    ASSERT_EQ(base::R::string::test1, resID);
 }
 
 TEST(ResTableTest, noParentThemeIsAppliedCorrectly) {
     ResTable table;
     ASSERT_EQ(NO_ERROR, table.add(basic_arsc, basic_arsc_len));
 
     ResTable::Theme theme(table);
-    ASSERT_EQ(NO_ERROR, theme.applyStyle(R::style::Theme1));
+    ASSERT_EQ(NO_ERROR, theme.applyStyle(base::R::style::Theme1));
 
     Res_value val;
     uint32_t specFlags = 0;
-    ssize_t index = theme.getAttribute(R::attr::attr1, &val, &specFlags);
+    ssize_t index = theme.getAttribute(base::R::attr::attr1, &val, &specFlags);
     ASSERT_GE(index, 0);
     ASSERT_EQ(Res_value::TYPE_INT_DEC, val.dataType);
     ASSERT_EQ(uint32_t(100), val.data);
 
-    index = theme.getAttribute(R::attr::attr2, &val, &specFlags);
+    index = theme.getAttribute(base::R::attr::attr2, &val, &specFlags);
     ASSERT_GE(index, 0);
     ASSERT_EQ(Res_value::TYPE_REFERENCE, val.dataType);
-    ASSERT_EQ(R::integer::number1, val.data);
+    ASSERT_EQ(base::R::integer::number1, val.data);
 }
 
 TEST(ResTableTest, parentThemeIsAppliedCorrectly) {
     ResTable table;
     ASSERT_EQ(NO_ERROR, table.add(basic_arsc, basic_arsc_len));
 
     ResTable::Theme theme(table);
-    ASSERT_EQ(NO_ERROR, theme.applyStyle(R::style::Theme2));
+    ASSERT_EQ(NO_ERROR, theme.applyStyle(base::R::style::Theme2));
 
     Res_value val;
     uint32_t specFlags = 0;
-    ssize_t index = theme.getAttribute(R::attr::attr1, &val, &specFlags);
+    ssize_t index = theme.getAttribute(base::R::attr::attr1, &val, &specFlags);
     ASSERT_GE(index, 0);
     ASSERT_EQ(Res_value::TYPE_INT_DEC, val.dataType);
     ASSERT_EQ(uint32_t(300), val.data);
 
-    index = theme.getAttribute(R::attr::attr2, &val, &specFlags);
+    index = theme.getAttribute(base::R::attr::attr2, &val, &specFlags);
     ASSERT_GE(index, 0);
     ASSERT_EQ(Res_value::TYPE_REFERENCE, val.dataType);
-    ASSERT_EQ(R::integer::number1, val.data);
+    ASSERT_EQ(base::R::integer::number1, val.data);
+}
+
+TEST(ResTableTest, libraryThemeIsAppliedCorrectly) {
+    ResTable table;
+    ASSERT_EQ(NO_ERROR, table.add(lib_arsc, lib_arsc_len));
+
+    ResTable::Theme theme(table);
+    ASSERT_EQ(NO_ERROR, theme.applyStyle(lib::R::style::Theme));
+
+    Res_value val;
+    uint32_t specFlags = 0;
+    ssize_t index = theme.getAttribute(lib::R::attr::attr1, &val, &specFlags);
+    ASSERT_GE(index, 0);
+    ASSERT_EQ(Res_value::TYPE_INT_DEC, val.dataType);
+    ASSERT_EQ(uint32_t(700), val.data);
 }
 
 TEST(ResTableTest, referenceToBagIsNotResolved) {
     ResTable table;
     ASSERT_EQ(NO_ERROR, table.add(basic_arsc, basic_arsc_len));
 
     Res_value val;
-    ssize_t block = table.getResource(R::integer::number2, &val, MAY_NOT_BE_BAG);
+    ssize_t block = table.getResource(base::R::integer::number2, &val, MAY_NOT_BE_BAG);
     ASSERT_GE(block, 0);
     ASSERT_EQ(Res_value::TYPE_REFERENCE, val.dataType);
-    ASSERT_EQ(R::array::integerArray1, val.data);
+    ASSERT_EQ(base::R::array::integerArray1, val.data);
 
     ssize_t newBlock = table.resolveReference(&val, block);
     EXPECT_EQ(block, newBlock);
     EXPECT_EQ(Res_value::TYPE_REFERENCE, val.dataType);
-    EXPECT_EQ(R::array::integerArray1, val.data);
+    EXPECT_EQ(base::R::array::integerArray1, val.data);
 }
 
 TEST(ResTableTest, resourcesStillAccessibleAfterParameterChange) {
     ResTable table;
     ASSERT_EQ(NO_ERROR, table.add(basic_arsc, basic_arsc_len));
 
     Res_value val;
-    ssize_t block = table.getResource(R::integer::number1, &val, MAY_NOT_BE_BAG);
+    ssize_t block = table.getResource(base::R::integer::number1, &val, MAY_NOT_BE_BAG);
     ASSERT_GE(block, 0);
     ASSERT_EQ(Res_value::TYPE_INT_DEC, val.dataType);
 
     const ResTable::bag_entry* entry;
-    ssize_t count = table.lockBag(R::array::integerArray1, &entry);
+    ssize_t count = table.lockBag(base::R::array::integerArray1, &entry);
     ASSERT_GE(count, 0);
     table.unlockBag(entry);
 
@@ -144,11 +162,11 @@ TEST(ResTableTest, resourcesStillAccessibleAfterParameterChange) {
     param.density = 320;
     table.setParameters(&param);
 
-    block = table.getResource(R::integer::number1, &val, MAY_NOT_BE_BAG);
+    block = table.getResource(base::R::integer::number1, &val, MAY_NOT_BE_BAG);
     ASSERT_GE(block, 0);
     ASSERT_EQ(Res_value::TYPE_INT_DEC, val.dataType);
 
-    count = table.lockBag(R::array::integerArray1, &entry);
+    count = table.lockBag(base::R::array::integerArray1, &entry);
     ASSERT_GE(count, 0);
     table.unlockBag(entry);
 }
@@ -158,7 +176,7 @@ TEST(ResTableTest, resourceIsOverridenWithBetterConfig) {
     ASSERT_EQ(NO_ERROR, table.add(basic_arsc, basic_arsc_len));
 
     Res_value val;
-    ssize_t block = table.getResource(R::integer::number1, &val, MAY_NOT_BE_BAG);
+    ssize_t block = table.getResource(base::R::integer::number1, &val, MAY_NOT_BE_BAG);
     ASSERT_GE(block, 0);
     ASSERT_EQ(Res_value::TYPE_INT_DEC, val.dataType);
     ASSERT_EQ(uint32_t(200), val.data);
@@ -171,7 +189,7 @@ TEST(ResTableTest, resourceIsOverridenWithBetterConfig) {
     param.country[1] = 'E';
     table.setParameters(&param);
 
-    block = table.getResource(R::integer::number1, &val, MAY_NOT_BE_BAG);
+    block = table.getResource(base::R::integer::number1, &val, MAY_NOT_BE_BAG);
     ASSERT_GE(block, 0);
     ASSERT_EQ(Res_value::TYPE_INT_DEC, val.dataType);
     ASSERT_EQ(uint32_t(400), val.data);