From e352b990e1dca7857a4d170f411ddad2065670ca Mon Sep 17 00:00:00 2001 From: Adam Lesinski Date: Mon, 16 Nov 2015 11:59:14 -0800 Subject: [PATCH] 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 | 7 ++- tools/aapt2/link/ReferenceLinkerVisitor.h | 4 +- tools/aapt2/link/XmlReferenceLinker.cpp | 23 +++++---- tools/aapt2/process/SymbolTable.cpp | 48 ++++++++++++++----- .../aapt2/unflatten/BinaryResourceParser.cpp | 5 ++ 5 files changed, 61 insertions(+), 26 deletions(-) diff --git a/tools/aapt2/XmlDom.h b/tools/aapt2/XmlDom.h index 9a46bcb9fc5ce..721bf5bd6320d 100644 --- a/tools/aapt2/XmlDom.h +++ b/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 {}; diff --git a/tools/aapt2/link/ReferenceLinkerVisitor.h b/tools/aapt2/link/ReferenceLinkerVisitor.h index c70531ba82967..a4cb596eb5ec5 100644 --- a/tools/aapt2/link/ReferenceLinkerVisitor.h +++ b/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; diff --git a/tools/aapt2/link/XmlReferenceLinker.cpp b/tools/aapt2/link/XmlReferenceLinker.cpp index 147b9bf162485..caab9b8a4684c 100644 --- a/tools/aapt2/link/XmlReferenceLinker.cpp +++ b/tools/aapt2/link/XmlReferenceLinker.cpp @@ -33,6 +33,7 @@ class XmlReferenceLinkerVisitor : public xml::PackageAwareVisitor { private: IAaptContext* mContext; ISymbolTable* mSymbols; + Source mSource; std::set* 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* 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 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(); diff --git a/tools/aapt2/process/SymbolTable.cpp b/tools/aapt2/process/SymbolTable.cpp index 9a8b263f5f97b..bb33ea78f4962 100644 --- a/tools/aapt2/process/SymbolTable.cpp +++ b/tools/aapt2/process/SymbolTable.cpp @@ -77,16 +77,8 @@ const ISymbolTable::Symbol* SymbolTableWrapper::findByName(const ResourceName& n } -static std::shared_ptr 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 s = std::make_shared(); - s->id = id; - return s; - } - +static std::shared_ptr 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 s = lookupIdInTable(table, resId); + std::shared_ptr s; + if (name.type == ResourceType::kAttr) { + s = lookupAttributeInTable(table, resId); + } else { + s = std::make_shared(); + 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 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 s; + if (isAttr) { + s = lookupAttributeInTable(table, id); + } else { + s = std::make_shared(); + s->id = id; + } + if (s) { mIdCache.put(id, s); return s.get(); diff --git a/tools/aapt2/unflatten/BinaryResourceParser.cpp b/tools/aapt2/unflatten/BinaryResourceParser.cpp index 0d17e8467d32f..304833481be08 100644 --- a/tools/aapt2/unflatten/BinaryResourceParser.cpp +++ b/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())) {