AAPT2: Minor fixes to SymbolTable and diagnostic output

The SymbolTable lookup relied on the configuration, which we NEVER
want. Now we rely solely on the symbols defined in the ResTable
and no specific configuration or value of that symbol.

Also added some better source logging.

Change-Id: I983855c283493e924b2e92a9fd8e4cb841769349

tools/aapt2/XmlDom.h

@@ -215,10 +215,13 @@ public:
         for (auto iter = mPackageDecls.rbegin(); iter != rend; ++iter) {
             if (name.package == iter->prefix) {
                 if (iter->package.empty()) {
-                    return ResourceName{ localPackage.toString(), name.type, name.entry };
-                } else {
+                    if (localPackage != name.package) {
+                        return ResourceName{ localPackage.toString(), name.type, name.entry };
+                    }
+                } else if (iter->package != name.package) {
                     return ResourceName{ iter->package, name.type, name.entry };
                 }
+                break;
             }
         }
         return {};

tools/aapt2/link/ReferenceLinkerVisitor.h

@@ -80,7 +80,7 @@ public:
                 return;
             }
 
-            DiagMessage errorMsg;
+            DiagMessage errorMsg(reference->getSource());
             errorMsg << "reference to " << reference->name.value();
             if (realName) {
                 errorMsg << " (aka " << realName.value() << ")";
@@ -92,7 +92,7 @@ public:
         }
 
         if (!mSymbols->findById(reference->id.value())) {
-            mContext->getDiagnostics()->error(DiagMessage()
+            mContext->getDiagnostics()->error(DiagMessage(reference->getSource())
                                               << "reference to " << reference->id.value()
                                               << " was not found");
             mError = true;

tools/aapt2/link/XmlReferenceLinker.cpp

@@ -33,6 +33,7 @@ class XmlReferenceLinkerVisitor : public xml::PackageAwareVisitor {
 private:
     IAaptContext* mContext;
     ISymbolTable* mSymbols;
+    Source mSource;
     std::set<int>* mSdkLevelsFound;
     ReferenceLinkerVisitor mReferenceLinkerVisitor;
     bool mError = false;
@@ -40,13 +41,14 @@ private:
 public:
     using xml::PackageAwareVisitor::visit;
 
-    XmlReferenceLinkerVisitor(IAaptContext* context, ISymbolTable* symbols,
+    XmlReferenceLinkerVisitor(IAaptContext* context, ISymbolTable* symbols, const Source& source,
                               std::set<int>* sdkLevelsFound) :
-            mContext(context), mSymbols(symbols), mSdkLevelsFound(sdkLevelsFound),
+            mContext(context), mSymbols(symbols), mSource(source), mSdkLevelsFound(sdkLevelsFound),
             mReferenceLinkerVisitor(context, symbols, this) {
     }
 
     void visit(xml::Element* el) override {
+        const Source source = mSource.withLine(el->lineNumber);
         for (xml::Attribute& attr : el->attributes) {
             Maybe<std::u16string> maybePackage =
                     util::extractPackageFromNamespace(attr.namespaceUri);
@@ -76,15 +78,16 @@ public:
                             !(attribute->typeMask & android::ResTable_map::TYPE_STRING)) {
                         // We won't be able to encode this as a string.
                         mContext->getDiagnostics()->error(
-                                DiagMessage() << "'" << attr.value << "' "
-                                              << "is incompatible with attribute "
-                                              << package << ":" << attr.name << " " << *attribute);
+                                DiagMessage(source) << "'" << attr.value << "' "
+                                                    << "is incompatible with attribute "
+                                                    << package << ":" << attr.name << " "
+                                                    << *attribute);
                         mError = true;
                     }
                 } else {
-                    mContext->getDiagnostics()->error(
-                            DiagMessage() << "attribute '" << package << ":" << attr.name
-                                          << "' was not found");
+                    mContext->getDiagnostics()->error(DiagMessage(source)
+                                                      << "attribute '" << package << ":"
+                                                      << attr.name << "' was not found");
                     mError = true;
 
                 }
@@ -95,6 +98,7 @@ public:
 
             if (attr.compiledValue) {
                 // With a compiledValue, we must resolve the reference and assign it an ID.
+                attr.compiledValue->setSource(source);
                 attr.compiledValue->accept(&mReferenceLinkerVisitor);
             }
         }
@@ -123,7 +127,8 @@ public:
 
 bool XmlReferenceLinker::consume(IAaptContext* context, XmlResource* resource) {
     mSdkLevelsFound.clear();
-    XmlReferenceLinkerVisitor visitor(context, context->getExternalSymbols(), &mSdkLevelsFound);
+    XmlReferenceLinkerVisitor visitor(context, context->getExternalSymbols(), resource->file.source,
+                                      &mSdkLevelsFound);
     if (resource->root) {
         resource->root->accept(&visitor);
         return !visitor.hasError();

tools/aapt2/process/SymbolTable.cpp

@@ -77,16 +77,8 @@ const ISymbolTable::Symbol* SymbolTableWrapper::findByName(const ResourceName& n
 }
 
 
-static std::shared_ptr<ISymbolTable::Symbol> lookupIdInTable(const android::ResTable& table,
-                                                             ResourceId id) {
-    android::Res_value val = {};
-    ssize_t block = table.getResource(id.id, &val, true);
-    if (block >= 0) {
-        std::shared_ptr<ISymbolTable::Symbol> s = std::make_shared<ISymbolTable::Symbol>();
-        s->id = id;
-        return s;
-    }
-
+static std::shared_ptr<ISymbolTable::Symbol> lookupAttributeInTable(const android::ResTable& table,
+                                                                    ResourceId id) {
     // Try as a bag.
     const android::ResTable::bag_entry* entry;
     ssize_t count = table.lockBag(id.id, &entry);
@@ -148,14 +140,23 @@ const ISymbolTable::Symbol* AssetManagerSymbolTableBuilder::AssetManagerSymbolTa
     for (const auto& asset : mAssets) {
         const android::ResTable& table = asset->getResources(false);
         StringPiece16 typeStr = toString(name.type);
+        uint32_t typeSpecFlags = 0;
         ResourceId resId = table.identifierForName(name.entry.data(), name.entry.size(),
                                                    typeStr.data(), typeStr.size(),
-                                                   name.package.data(), name.package.size());
+                                                   name.package.data(), name.package.size(),
+                                                   &typeSpecFlags);
         if (!resId.isValid()) {
             continue;
         }
 
-        std::shared_ptr<Symbol> s = lookupIdInTable(table, resId);
+        std::shared_ptr<Symbol> s;
+        if (name.type == ResourceType::kAttr) {
+            s = lookupAttributeInTable(table, resId);
+        } else {
+            s = std::make_shared<Symbol>();
+            s->id = resId;
+        }
+
         if (s) {
             mCache.put(name, s);
             return s.get();
@@ -173,7 +174,28 @@ const ISymbolTable::Symbol* AssetManagerSymbolTableBuilder::AssetManagerSymbolTa
     for (const auto& asset : mAssets) {
         const android::ResTable& table = asset->getResources(false);
 
-        std::shared_ptr<Symbol> s = lookupIdInTable(table, id);
+        android::ResTable::resource_name name;
+        if (!table.getResourceName(id.id, true, &name)) {
+            continue;
+        }
+
+        bool isAttr = false;
+        if (name.type) {
+            if (const ResourceType* t = parseResourceType(StringPiece16(name.type, name.typeLen))) {
+                isAttr = (*t == ResourceType::kAttr);
+            }
+        } else if (name.type8) {
+            isAttr = (StringPiece(name.type8, name.typeLen) == "attr");
+        }
+
+        std::shared_ptr<Symbol> s;
+        if (isAttr) {
+            s = lookupAttributeInTable(table, id);
+        } else {
+            s = std::make_shared<Symbol>();
+            s->id = id;
+        }
+
         if (s) {
             mIdCache.put(id, s);
             return s.get();

tools/aapt2/unflatten/BinaryResourceParser.cpp

@@ -304,6 +304,11 @@ bool BinaryResourceParser::parsePackage(const ResChunk_header* chunk) {
         return false;
     }
 
+    // There can be multiple packages in a table, so
+    // clear the type and key pool in case they were set from a previous package.
+    mTypePool.uninit();
+    mKeyPool.uninit();
+
     ResChunkPullParser parser(getChunkData(&packageHeader->header),
                               getChunkDataLen(&packageHeader->header));
     while (ResChunkPullParser::isGoodEvent(parser.next())) {