Donate to e Foundation | Murena handsets with /e/OS | Own a part of Murena! Learn more

Commit cd78febe authored by Ryan Mitchell's avatar Ryan Mitchell
Browse files

Recognize dynamic res ids as valid 

Shared libraries are assigned package id 0. Resource ids that start
with 0x00 are not invalid. This change changes is_valid to accept
dynamic resource ids and adds an is_valid_static method for when
an id must have a non-zero package id.

This also fixes an issue that made layouts in shared libraries that
use internal attributes exclude compiled resource ids from the binay
xml output.

Bug: 146491000
Test: Build a shared library with a layout that uses app attributes
      as well as android attribute and verify that all attributes
      have assigned resource ids using `aapt2 dump xmltree`

Change-Id: Ibc0407c610ffc98d7aaf233c37c065912ab0d516
parent 0cf243ea

tools/aapt2/Resource.h

+5 −4
Original line number Diff line number Diff line
@@ -147,10 +147,11 @@ struct ResourceId { 
  ResourceId(uint32_t res_id);  // NOLINT(google-explicit-constructor)

  ResourceId(uint8_t p, uint8_t t, uint16_t e);



  bool is_valid() const;

  // Returns true if the ID is a valid ID that is not dynamic (package ID cannot be 0)

  bool is_valid_static() const;



  // Returns true if the ID is a valid ID or dynamic ID (package ID can be 0).

  bool is_valid_dynamic() const;

  bool is_valid() const;



  uint8_t package_id() const;

  uint8_t type_id() const;
@@ -233,11 +234,11 @@ inline ResourceId::ResourceId(uint32_t res_id) : id(res_id) {} 
inline ResourceId::ResourceId(uint8_t p, uint8_t t, uint16_t e)

    : id((p << 24) | (t << 16) | e) {}



inline bool ResourceId::is_valid() const {

inline bool ResourceId::is_valid_static() const {

  return (id & 0xff000000u) != 0 && (id & 0x00ff0000u) != 0;

}



inline bool ResourceId::is_valid_dynamic() const {

inline bool ResourceId::is_valid() const {

  return (id & 0x00ff0000u) != 0;

}

tools/aapt2/ResourceTable.cpp

+8 −8
Original line number Diff line number Diff line
@@ -398,7 +398,7 @@ bool ResourceTable::AddResourceImpl(const ResourceNameRef& name, const ResourceI 


  // Check for package names appearing twice with two different package ids

  ResourceTablePackage* package = FindOrCreatePackage(name.package);

  if (res_id.is_valid_dynamic() && package->id && package->id.value() != res_id.package_id()) {

  if (res_id.is_valid() && package->id && package->id.value() != res_id.package_id()) {

    diag->Error(DiagMessage(source)

                    << "trying to add resource '" << name << "' with ID " << res_id

                    << " but package '" << package->name << "' already has ID "
@@ -407,9 +407,9 @@ bool ResourceTable::AddResourceImpl(const ResourceNameRef& name, const ResourceI 
  }



  // Whether or not to error on duplicate resources

  bool check_id = validate_resources_ && res_id.is_valid_dynamic();

  bool check_id = validate_resources_ && res_id.is_valid();

  // Whether or not to create a duplicate resource if the id does not match

  bool use_id = !validate_resources_ && res_id.is_valid_dynamic();

  bool use_id = !validate_resources_ && res_id.is_valid();



  ResourceTableType* type = package->FindOrCreateType(name.type, use_id ? res_id.type_id()

                                                                        : Maybe<uint8_t>());
@@ -463,7 +463,7 @@ bool ResourceTable::AddResourceImpl(const ResourceNameRef& name, const ResourceI 
    }

  }



  if (res_id.is_valid_dynamic()) {

  if (res_id.is_valid()) {

    package->id = res_id.package_id();

    type->id = res_id.type_id();

    entry->id = res_id.entry_id();
@@ -504,7 +504,7 @@ bool ResourceTable::SetVisibilityImpl(const ResourceNameRef& name, const Visibil 


  // Check for package names appearing twice with two different package ids

  ResourceTablePackage* package = FindOrCreatePackage(name.package);

  if (res_id.is_valid_dynamic() && package->id && package->id.value() != res_id.package_id()) {

  if (res_id.is_valid() && package->id && package->id.value() != res_id.package_id()) {

    diag->Error(DiagMessage(source)

                    << "trying to add resource '" << name << "' with ID " << res_id

                    << " but package '" << package->name << "' already has ID "
@@ -513,9 +513,9 @@ bool ResourceTable::SetVisibilityImpl(const ResourceNameRef& name, const Visibil 
  }



  // Whether or not to error on duplicate resources

  bool check_id = validate_resources_ && res_id.is_valid_dynamic();

  bool check_id = validate_resources_ && res_id.is_valid();

  // Whether or not to create a duplicate resource if the id does not match

  bool use_id = !validate_resources_ && res_id.is_valid_dynamic();

  bool use_id = !validate_resources_ && res_id.is_valid();



  ResourceTableType* type = package->FindOrCreateType(name.type, use_id ? res_id.type_id()

                                                                        : Maybe<uint8_t>());
@@ -541,7 +541,7 @@ bool ResourceTable::SetVisibilityImpl(const ResourceNameRef& name, const Visibil 
    return false;

  }



  if (res_id.is_valid_dynamic()) {

  if (res_id.is_valid()) {

    package->id = res_id.package_id();

    type->id = res_id.type_id();

    entry->id = res_id.entry_id();

tools/aapt2/ResourceUtils.cpp

+1 −1
Original line number Diff line number Diff line
@@ -516,7 +516,7 @@ Maybe<ResourceId> ParseResourceId(const StringPiece& str) { 
  if (android::ResTable::stringToInt(str16.data(), str16.size(), &value)) {

    if (value.dataType == android::Res_value::TYPE_INT_HEX) {

      ResourceId id(value.data);

      if (id.is_valid_dynamic()) {

      if (id.is_valid()) {

        return id;

      }

    }

tools/aapt2/ResourceValues.cpp

+3 −3
Original line number Diff line number Diff line
@@ -117,7 +117,7 @@ bool Reference::Equals(const Value* value) const { 


bool Reference::Flatten(android::Res_value* out_value) const {

  const ResourceId resid = id.value_or_default(ResourceId(0));

  const bool dynamic = resid.is_valid_dynamic() && is_dynamic;

  const bool dynamic = resid.is_valid() && is_dynamic;



  if (reference_type == Reference::Type::kResource) {

    if (dynamic) {
@@ -159,7 +159,7 @@ void Reference::Print(std::ostream* out) const { 
    *out << name.value();

  }



  if (id && id.value().is_valid_dynamic()) {

  if (id && id.value().is_valid()) {

    if (name) {

      *out << " ";

    }
@@ -196,7 +196,7 @@ static void PrettyPrintReferenceImpl(const Reference& ref, bool print_package, P 
      printer->Print("/");

      printer->Print(name.entry);

    }

  } else if (ref.id && ref.id.value().is_valid_dynamic()) {

  } else if (ref.id && ref.id.value().is_valid()) {

    printer->Print(ref.id.value().to_string());

  }

}

tools/aapt2/process/SymbolTable.cpp

+2 −2
Original line number Diff line number Diff line
@@ -340,7 +340,7 @@ std::unique_ptr<SymbolTable::Symbol> AssetManagerSymbolSource::FindByName( 
    }



    res_id = asset_manager_.GetResourceId(real_name.to_string());

    if (res_id.is_valid() && asset_manager_.GetResourceFlags(res_id.id, &type_spec_flags)) {

    if (res_id.is_valid_static() && asset_manager_.GetResourceFlags(res_id.id, &type_spec_flags)) {

      found = true;

      return false;

    }
@@ -379,7 +379,7 @@ static Maybe<ResourceName> GetResourceName(android::AssetManager2& am, 


std::unique_ptr<SymbolTable::Symbol> AssetManagerSymbolSource::FindById(

    ResourceId id) {

  if (!id.is_valid()) {

  if (!id.is_valid_static()) {

    // Exit early and avoid the error logs from AssetManager.

    return {};

  }