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
diff --git a/tools/aapt2/XmlDom.h b/tools/aapt2/XmlDom.h
index 9a46bcb..721bf5b 100644
--- a/tools/aapt2/XmlDom.h
+++ b/tools/aapt2/XmlDom.h
@@ -215,10 +215,13 @@
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 c70531b..a4cb596 100644
--- a/tools/aapt2/link/ReferenceLinkerVisitor.h
+++ b/tools/aapt2/link/ReferenceLinkerVisitor.h
@@ -80,7 +80,7 @@
return;
}
- DiagMessage errorMsg;
+ DiagMessage errorMsg(reference->getSource());
errorMsg << "reference to " << reference->name.value();
if (realName) {
errorMsg << " (aka " << realName.value() << ")";
@@ -92,7 +92,7 @@
}
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 147b9bf..caab9b8 100644
--- a/tools/aapt2/link/XmlReferenceLinker.cpp
+++ b/tools/aapt2/link/XmlReferenceLinker.cpp
@@ -33,6 +33,7 @@
private:
IAaptContext* mContext;
ISymbolTable* mSymbols;
+ Source mSource;
std::set<int>* mSdkLevelsFound;
ReferenceLinkerVisitor mReferenceLinkerVisitor;
bool mError = false;
@@ -40,13 +41,14 @@
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 @@
!(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 @@
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 @@
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 9a8b263..bb33ea7 100644
--- a/tools/aapt2/process/SymbolTable.cpp
+++ b/tools/aapt2/process/SymbolTable.cpp
@@ -77,16 +77,8 @@
}
-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 @@
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 @@
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();
diff --git a/tools/aapt2/unflatten/BinaryResourceParser.cpp b/tools/aapt2/unflatten/BinaryResourceParser.cpp
index 0d17e84..3048334 100644
--- a/tools/aapt2/unflatten/BinaryResourceParser.cpp
+++ b/tools/aapt2/unflatten/BinaryResourceParser.cpp
@@ -304,6 +304,11 @@
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())) {