Merge "Fix the internal type of enum values" am: 74e23a1ec1

Original change: https://android-review.googlesource.com/c/platform/development/+/2530761

Change-Id: I6b5df899cf359e4898bf87d583012898f9832766
Signed-off-by: Automerger Merge Worker <android-build-automerger-merge-worker@system.gserviceaccount.com>

vndk/tools/header-checker/src/dumper/abi_wrappers.cpp

@@ -882,8 +882,12 @@ bool EnumDeclWrapper::SetupEnumFields(repr::EnumTypeIR *enump) {
   clang::EnumDecl::enumerator_iterator enum_it = enum_decl_->enumerator_begin();
   while (enum_it != enum_decl_->enumerator_end()) {
     std::string name = enum_it->getQualifiedNameAsString();
-    uint64_t field_value = enum_it->getInitVal().getExtValue();
+    const llvm::APSInt &value = enum_it->getInitVal();
-    enump->AddEnumField(repr::EnumFieldIR(name, field_value));
+    if (value.isUnsigned()) {
+      enump->AddEnumField(repr::EnumFieldIR(name, value.getZExtValue()));
+    } else {
+      enump->AddEnumField(repr::EnumFieldIR(name, value.getSExtValue()));
+    }
     enum_it++;
   }
   return true;

vndk/tools/header-checker/src/repr/abi_diff_helpers.cpp

@@ -147,9 +147,10 @@ void AbiDiffHelper::CompareEnumFields(
       utils::FindCommonElements(old_fields_map, new_fields_map);
   std::vector<EnumFieldDiffIR> enum_field_diffs;
   for (auto &&common_fields : cf) {
-    if (common_fields.first->GetValue() != common_fields.second->GetValue()) {
+    if (common_fields.first->GetSignedValue() !=
+        common_fields.second->GetSignedValue()) {
       EnumFieldDiffIR enum_field_diff_ir(common_fields.first,
-                                                   common_fields.second);
+                                         common_fields.second);
       enum_field_diffs.emplace_back(std::move(enum_field_diff_ir));
     }
   }

vndk/tools/header-checker/src/repr/ir_representation.h

@@ -447,20 +447,29 @@ class RecordTypeIR : public TypeIR, public TemplatedArtifactIR {
 
 class EnumFieldIR {
  public:
-  EnumFieldIR(const std::string &name, int value)
+  EnumFieldIR(const std::string &name, int64_t value)
-      : name_(name), value_(value) {}
+      : name_(name), signed_value_(value), is_signed_(true) {}
+
+  EnumFieldIR(const std::string &name, uint64_t value)
+      : name_(name), unsigned_value_(value), is_signed_(false) {}
 
   const std::string &GetName() const {
     return name_;
   }
 
-  int GetValue() const {
+  bool IsSigned() const { return is_signed_; }
-    return value_;
+
-  }
+  int64_t GetSignedValue() const { return signed_value_; }
+
+  uint64_t GetUnsignedValue() const { return unsigned_value_; }
 
  protected:
   std::string name_;
-  int value_ = 0;
+  union {
+    int64_t signed_value_;
+    uint64_t unsigned_value_;
+  };
+  bool is_signed_;
 };
 
 class EnumTypeIR : public TypeIR {

vndk/tools/header-checker/src/repr/json/ir_dumper.cpp

@@ -203,7 +203,12 @@ static JsonObject ConvertEnumFieldIR(const EnumFieldIR *enum_field_ir) {
   JsonObject enum_field;
   enum_field.Set("name", enum_field_ir->GetName());
   // Never omit enum values.
-  enum_field["enum_field_value"] = Json::Int64(enum_field_ir->GetValue());
+  Json::Value &enum_field_value = enum_field["enum_field_value"];
+  if (enum_field_ir->IsSigned()) {
+    enum_field_value = Json::Int64(enum_field_ir->GetSignedValue());
+  } else {
+    enum_field_value = Json::UInt64(enum_field_ir->GetUnsignedValue());
+  }
   return enum_field;
 }
 

vndk/tools/header-checker/src/repr/json/ir_reader.cpp

@@ -76,11 +76,16 @@ bool JsonObjectRef::GetBool(const std::string &key) const {
 }
 
 int64_t JsonObjectRef::GetInt(const std::string &key) const {
-  return Get(key, json_0, &Json::Value::isIntegral).asInt64();
+  return Get(key, json_0, &Json::Value::isInt64).asInt64();
 }
 
 uint64_t JsonObjectRef::GetUint(const std::string &key) const {
-  return Get(key, json_0, &Json::Value::isIntegral).asUInt64();
+  return Get(key, json_0, &Json::Value::isUInt64).asUInt64();
+}
+
+const Json::Value &JsonObjectRef::GetIntegralValue(
+    const std::string &key) const {
+  return Get(key, json_0, &Json::Value::isIntegral);
 }
 
 std::string JsonObjectRef::GetString(const std::string &key) const {
@@ -248,9 +253,13 @@ void JsonIRReader::ReadVTableLayout(const JsonObjectRef &record_type,
 void JsonIRReader::ReadEnumFields(const JsonObjectRef &enum_type,
                                   EnumTypeIR *enum_ir) {
   for (auto &&field : enum_type.GetObjects("enum_fields")) {
-    EnumFieldIR enum_field_ir(field.GetString("name"),
+    std::string name = field.GetString("name");
-                              field.GetInt("enum_field_value"));
+    const Json::Value &value = field.GetIntegralValue("enum_field_value");
-    enum_ir->AddEnumField(std::move(enum_field_ir));
+    if (value.isUInt64()) {
+      enum_ir->AddEnumField(EnumFieldIR(name, value.asUInt64()));
+    } else {
+      enum_ir->AddEnumField(EnumFieldIR(name, value.asInt64()));
+    }
   }
 }
 

vndk/tools/header-checker/src/repr/json/ir_reader.h

@@ -46,6 +46,9 @@ class JsonObjectRef {
   // Default to 0.
   uint64_t GetUint(const std::string &key) const;
 
+  // Default to 0.
+  const Json::Value &GetIntegralValue(const std::string &key) const;
+
   // Default to "".
   std::string GetString(const std::string &key) const;
 

vndk/tools/header-checker/src/repr/protobuf/converter.h

@@ -356,7 +356,11 @@ inline bool SetIRToProtobufEnumField(
     return true;
   }
   enum_field_protobuf->set_name(enum_field_ir->GetName());
-  enum_field_protobuf->set_enum_field_value(enum_field_ir->GetValue());
+  // The "enum_field_value" in the .proto is a signed 64-bit integer. An
+  // unsigned integer >= (1 << 63) is represented with a negative integer in the
+  // dump file. Despite the wrong representation, the diff result isn't affected
+  // because every integer has a unique representation.
+  enum_field_protobuf->set_enum_field_value(enum_field_ir->GetSignedValue());
   return true;
 }
 

vndk/tools/header-checker/tests/integration/enum/include/base.h

@@ -0,0 +1,23 @@
+#include <cstdint>
+
+enum Int8 : int8_t {
+  ZERO = 0,
+  MINUS_1 = (int8_t)-1,
+};
+
+enum Uint8 : uint8_t {
+  UNSIGNED_255 = UINT8_MAX,
+};
+
+enum Int64 : int64_t {
+  SIGNED_MAX = INT64_MAX,
+  SIGNED_MIN = INT64_MIN,
+};
+
+enum Uint64 : uint64_t {
+  UNSIGNED_MAX = UINT64_MAX,
+};
+
+extern "C" {
+void function(Int8, Uint8, Int64, Uint64);
+}

vndk/tools/header-checker/tests/integration/enum/map.txt

@@ -0,0 +1,4 @@
+libenum {
+  global:
+    function;
+};

vndk/tools/header-checker/tests/module.py

@@ -789,6 +789,16 @@ TEST_MODULES = [
         export_include_dirs=['integration/union/include'],
         linker_flags=['-output-format', 'Json'],
     ),
+    LsdumpModule(
+        name='libenum',
+        arch='arm64',
+        srcs=['integration/enum/include/base.h'],
+        version_script='integration/enum/map.txt',
+        export_include_dirs=['integration/enum/include'],
+        dumper_flags=['-output-format', 'Json'],
+        linker_flags=['-input-format', 'Json', '-output-format', 'Json'],
+        has_reference_dump=True,
+    ),
 ]
 
 TEST_MODULES = {m.name: m for m in TEST_MODULES}

vndk/tools/header-checker/tests/reference_dumps/arm64/libenum.so.lsdump

@@ -0,0 +1,167 @@
+{
+ "array_types" : [],
+ "builtin_types" :
+ [
+  {
+   "alignment" : 1,
+   "is_integral" : true,
+   "linker_set_key" : "_ZTIa",
+   "name" : "signed char",
+   "referenced_type" : "_ZTIa",
+   "self_type" : "_ZTIa",
+   "size" : 1
+  },
+  {
+   "alignment" : 1,
+   "is_integral" : true,
+   "is_unsigned" : true,
+   "linker_set_key" : "_ZTIh",
+   "name" : "unsigned char",
+   "referenced_type" : "_ZTIh",
+   "self_type" : "_ZTIh",
+   "size" : 1
+  },
+  {
+   "alignment" : 8,
+   "is_integral" : true,
+   "linker_set_key" : "_ZTIl",
+   "name" : "long",
+   "referenced_type" : "_ZTIl",
+   "self_type" : "_ZTIl",
+   "size" : 8
+  },
+  {
+   "alignment" : 8,
+   "is_integral" : true,
+   "is_unsigned" : true,
+   "linker_set_key" : "_ZTIm",
+   "name" : "unsigned long",
+   "referenced_type" : "_ZTIm",
+   "self_type" : "_ZTIm",
+   "size" : 8
+  },
+  {
+   "linker_set_key" : "_ZTIv",
+   "name" : "void",
+   "referenced_type" : "_ZTIv",
+   "self_type" : "_ZTIv"
+  }
+ ],
+ "elf_functions" :
+ [
+  {
+   "name" : "function"
+  }
+ ],
+ "elf_objects" : [],
+ "enum_types" :
+ [
+  {
+   "alignment" : 1,
+   "enum_fields" :
+   [
+    {
+     "enum_field_value" : 0,
+     "name" : "ZERO"
+    },
+    {
+     "enum_field_value" : -1,
+     "name" : "MINUS_1"
+    }
+   ],
+   "linker_set_key" : "_ZTI4Int8",
+   "name" : "Int8",
+   "referenced_type" : "_ZTI4Int8",
+   "self_type" : "_ZTI4Int8",
+   "size" : 1,
+   "source_file" : "development/vndk/tools/header-checker/tests/integration/enum/include/base.h",
+   "underlying_type" : "_ZTIa"
+  },
+  {
+   "alignment" : 8,
+   "enum_fields" :
+   [
+    {
+     "enum_field_value" : 9223372036854775807,
+     "name" : "SIGNED_MAX"
+    },
+    {
+     "enum_field_value" : -9223372036854775808,
+     "name" : "SIGNED_MIN"
+    }
+   ],
+   "linker_set_key" : "_ZTI5Int64",
+   "name" : "Int64",
+   "referenced_type" : "_ZTI5Int64",
+   "self_type" : "_ZTI5Int64",
+   "size" : 8,
+   "source_file" : "development/vndk/tools/header-checker/tests/integration/enum/include/base.h",
+   "underlying_type" : "_ZTIl"
+  },
+  {
+   "alignment" : 1,
+   "enum_fields" :
+   [
+    {
+     "enum_field_value" : 255,
+     "name" : "UNSIGNED_255"
+    }
+   ],
+   "linker_set_key" : "_ZTI5Uint8",
+   "name" : "Uint8",
+   "referenced_type" : "_ZTI5Uint8",
+   "self_type" : "_ZTI5Uint8",
+   "size" : 1,
+   "source_file" : "development/vndk/tools/header-checker/tests/integration/enum/include/base.h",
+   "underlying_type" : "_ZTIh"
+  },
+  {
+   "alignment" : 8,
+   "enum_fields" :
+   [
+    {
+     "enum_field_value" : 18446744073709551615,
+     "name" : "UNSIGNED_MAX"
+    }
+   ],
+   "linker_set_key" : "_ZTI6Uint64",
+   "name" : "Uint64",
+   "referenced_type" : "_ZTI6Uint64",
+   "self_type" : "_ZTI6Uint64",
+   "size" : 8,
+   "source_file" : "development/vndk/tools/header-checker/tests/integration/enum/include/base.h",
+   "underlying_type" : "_ZTIm"
+  }
+ ],
+ "function_types" : [],
+ "functions" :
+ [
+  {
+   "function_name" : "function",
+   "linker_set_key" : "function",
+   "parameters" :
+   [
+    {
+     "referenced_type" : "_ZTI4Int8"
+    },
+    {
+     "referenced_type" : "_ZTI5Uint8"
+    },
+    {
+     "referenced_type" : "_ZTI5Int64"
+    },
+    {
+     "referenced_type" : "_ZTI6Uint64"
+    }
+   ],
+   "return_type" : "_ZTIv",
+   "source_file" : "development/vndk/tools/header-checker/tests/integration/enum/include/base.h"
+  }
+ ],
+ "global_vars" : [],
+ "lvalue_reference_types" : [],
+ "pointer_types" : [],
+ "qualified_types" : [],
+ "record_types" : [],
+ "rvalue_reference_types" : []
+}

vndk/tools/header-checker/tests/test.py

@@ -485,6 +485,9 @@ class HeaderCheckerTest(unittest.TestCase):
         self.assertNotIn("fields_added", diff)
         self.assertNotIn("fields_removed", diff)
 
+    def test_enum_diff(self):
+        self.prepare_and_absolute_diff_all_archs("libenum", "libenum")
+
 
 if __name__ == '__main__':
     unittest.main()