Merge "header-abi-diff: Add a flag to consider opaque types with different names structurally unequal."

am: 73ea62cf25

Change-Id: I26bc6587e730fad442fda69b88aa368dee30d099

vndk/tools/header-checker/header-abi-diff/src/abi_diff.cpp

@@ -360,7 +360,9 @@ bool HeaderAbiDiff::DumpDiffElements(
     }
     abi_diff_wrappers::DiffWrapper<T> diff_wrapper(old_element, new_element,
                                                    ir_diff_dumper, old_types,
-                                                   new_types, &type_cache_);
+                                                   new_types,
+                                                   diff_policy_options_,
+                                                   &type_cache_);
     if (!diff_wrapper.DumpDiff(diff_kind)) {
       llvm::errs() << "Failed to diff elements\n";
       return false;

vndk/tools/header-checker/header-abi-diff/src/abi_diff.h

@@ -20,6 +20,7 @@
 #include <vector>
 
 using abi_util::AbiElementMap;
+using abi_util::DiffPolicyOptions;
 
 class HeaderAbiDiff {
  public:
@@ -27,12 +28,15 @@ class HeaderAbiDiff {
                 const std::string &old_dump, const std::string &new_dump,
                 const std::string &compatibility_report,
                 const std::set<std::string> &ignored_symbols,
+                const DiffPolicyOptions &diff_policy_options,
                 bool check_all_apis, abi_util::TextFormatIR text_format_old,
                 abi_util::TextFormatIR text_format_new,
                 abi_util::TextFormatIR text_format_diff)
       : lib_name_(lib_name), arch_(arch), old_dump_(old_dump),
         new_dump_(new_dump), cr_(compatibility_report),
-        ignored_symbols_(ignored_symbols), check_all_apis_(check_all_apis),
+        ignored_symbols_(ignored_symbols),
+        diff_policy_options_(diff_policy_options),
+        check_all_apis_(check_all_apis),
         text_format_old_(text_format_old), text_format_new_(text_format_new),
         text_format_diff_(text_format_diff) { }
 
@@ -134,6 +138,7 @@ class HeaderAbiDiff {
   const std::string &new_dump_;
   const std::string &cr_;
   const std::set<std::string> &ignored_symbols_;
+  const DiffPolicyOptions &diff_policy_options_;
   bool check_all_apis_;
   std::set<std::string> type_cache_;
   abi_util::TextFormatIR text_format_old_;

vndk/tools/header-checker/header-abi-diff/src/abi_diff_wrappers.h

@@ -42,9 +42,10 @@ class DiffWrapper : public AbiDiffHelper {
               abi_util::IRDiffDumper *ir_diff_dumper,
               const AbiElementMap<const abi_util::TypeIR *> &old_types,
               const AbiElementMap<const abi_util::TypeIR *> &new_types,
+              const abi_util::DiffPolicyOptions &diff_policy_options,
               std::set<std::string> *type_cache)
-      : AbiDiffHelper(old_types, new_types, type_cache, ir_diff_dumper),
+      : AbiDiffHelper(old_types, new_types, diff_policy_options, type_cache,
-        oldp_(oldp), newp_(newp) { }
+                      ir_diff_dumper), oldp_(oldp), newp_(newp) { }
 
   bool DumpDiff(abi_util::IRDiffDumper::DiffKind diff_kind);
 

vndk/tools/header-checker/header-abi-diff/src/header_abi_diff.cpp

@@ -85,6 +85,14 @@ static llvm::cl::opt<bool> allow_unreferenced_changes(
                    " APIs."),
     llvm::cl::Optional, llvm::cl::cat(header_checker_category));
 
+static llvm::cl::opt<bool> consider_opaque_types_different(
+    "consider-opaque-types-different",
+    llvm::cl::desc("Consider opaque types with different names as different"
+                   " .This should not be used while comparing C++ library"
+                   " ABIs"),
+    llvm::cl::Optional, llvm::cl::cat(header_checker_category));
+
+
 static llvm::cl::opt<abi_util::TextFormatIR> text_format_old(
     "text-format-old", llvm::cl::desc("Specify text format of old abi dump"),
     llvm::cl::values(clEnumValN(abi_util::TextFormatIR::ProtobufTextFormat,
@@ -142,9 +150,11 @@ int main(int argc, const char **argv) {
   if (llvm::sys::fs::exists(ignore_symbol_list)) {
     ignored_symbols = LoadIgnoredSymbols(ignore_symbol_list);
   }
+  abi_util::DiffPolicyOptions diff_policy_options(
+      consider_opaque_types_different);
   HeaderAbiDiff judge(lib_name, arch, old_dump, new_dump, compatibility_report,
-                      ignored_symbols, check_all_apis, text_format_old,
+                      ignored_symbols, diff_policy_options, check_all_apis,
-                      text_format_new, text_format_diff);
+                      text_format_old, text_format_new, text_format_diff);
 
   abi_util::CompatibilityStatusIR status = judge.GenerateCompatibilityReport();
 

vndk/tools/header-checker/header-abi-util/include/abi_diff_helpers.h

@@ -19,6 +19,8 @@ enum DiffStatus {
   // There was a diff found, however it need not be added as a part of a diff
   // message, since it would have already been noted elsewhere.
   indirect_diff = 2,
+
+  opaque_diff = 3,
 };
 
 static inline DiffStatus operator| (DiffStatus f,DiffStatus s) {
@@ -46,16 +48,24 @@ struct GenericFieldDiffInfo {
 
 std::string Unwind(const std::deque<std::string> *type_queue);
 
+struct DiffPolicyOptions {
+  DiffPolicyOptions(bool consider_opaque_types_different) :
+    consider_opaque_types_different_(consider_opaque_types_different) { }
+  bool consider_opaque_types_different_ = false;
+};
+
 class AbiDiffHelper {
  public:
   AbiDiffHelper(
       const AbiElementMap<const abi_util::TypeIR *> &old_types,
       const AbiElementMap<const abi_util::TypeIR *> &new_types,
+      const DiffPolicyOptions &diff_policy_options,
       std::set<std::string> *type_cache,
       abi_util::IRDiffDumper *ir_diff_dumper = nullptr,
       AbiElementMap<MergeStatus> *local_to_global_type_id_map = nullptr)
       : old_types_(old_types), new_types_(new_types),
-        type_cache_(type_cache), ir_diff_dumper_(ir_diff_dumper),
+        diff_policy_options_(diff_policy_options), type_cache_(type_cache),
+        ir_diff_dumper_(ir_diff_dumper),
         local_to_global_type_id_map_(local_to_global_type_id_map) { }
 
   DiffStatus CompareAndDumpTypeDiff(
@@ -175,6 +185,7 @@ class AbiDiffHelper {
  protected:
   const AbiElementMap<const abi_util::TypeIR *> &old_types_;
   const AbiElementMap<const abi_util::TypeIR *> &new_types_;
+  const DiffPolicyOptions &diff_policy_options_;
   std::set<std::string> *type_cache_ = nullptr;
   abi_util::IRDiffDumper *ir_diff_dumper_ = nullptr;
   AbiElementMap<MergeStatus> *local_to_global_type_id_map_ = nullptr;

vndk/tools/header-checker/header-abi-util/src/abi_diff_helpers.cpp

@@ -786,6 +786,9 @@ DiffStatus AbiDiffHelper::CompareAndDumpTypeDiff(
   if (old_it == old_types_.end() || new_it == new_types_.end()) {
     TypeQueueCheckAndPop(type_queue);
     // One of the types were hidden, we cannot compare further.
+    if (diff_policy_options_.consider_opaque_types_different_) {
+      return DiffStatus::opaque_diff;
+    }
     return DiffStatus::no_diff;
   }
   abi_util::LinkableMessageKind old_kind =
@@ -800,6 +803,11 @@ DiffStatus AbiDiffHelper::CompareAndDumpTypeDiff(
                                          old_kind, type_queue, diff_kind);
   }
   TypeQueueCheckAndPop(type_queue);
+  if (diff_policy_options_.consider_opaque_types_different_ &&
+      diff_status == DiffStatus::opaque_diff &&
+      (old_it->second->GetName() != new_it->second->GetName())) {
+    return DiffStatus::direct_diff;
+  }
   return diff_status;
 }
 

vndk/tools/header-checker/header-abi-util/src/ir_representation.cpp

@@ -123,7 +123,9 @@ MergeStatus TextFormatToIRReader::DoesUDTypeODRViolationExist(
     return MergeStatus(true, "");
   }
   std::set<std::string> type_cache;
-  AbiDiffHelper diff_helper(type_graph_, addend.type_graph_, &type_cache,
+  DiffPolicyOptions diff_policy_options(false) ;
+  AbiDiffHelper diff_helper(type_graph_, addend.type_graph_,
+                            diff_policy_options, &type_cache,
                             nullptr, local_to_global_type_id_map_);
   for (auto &contender_ud : it->second) {
     if (diff_helper.CompareAndDumpTypeDiff(contender_ud->GetSelfType(),

vndk/tools/header-checker/tests/integration/c_and_cpp/include/c_include.h

@@ -6,9 +6,17 @@ struct Cinner {
   int c;
 };
 
+struct opaque_a;
+struct opaque_b;
+
 struct Cstruct {
   int a;
   struct Cinner *b;
+#ifdef OPAQUE_STRUCT_A
+  struct opaque_a *op;
+#elif OPAQUE_STRUCT_B
+  struct opaque_b *op;
+#endif
 };
 
 void CFunction(struct Cstruct **cstruct);

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

@@ -125,6 +125,28 @@ TEST_MODULES = [
         arch = '',
         api = 'current',
     ),
+    Module(
+        name = 'libc_and_cpp_with_opaque_ptr_a',
+        srcs = ['integration/c_and_cpp/source1.cpp',
+                'integration/c_and_cpp/source2.c',
+                ],
+        version_script = 'integration/c_and_cpp/map.txt',
+        export_include_dirs = ['integration/c_and_cpp/include'],
+        cflags = ['-DOPAQUE_STRUCT_A=1'],
+        arch = '',
+        api = 'current',
+    ),
+    Module(
+        name = 'libc_and_cpp_with_opaque_ptr_b',
+        srcs = ['integration/c_and_cpp/source1.cpp',
+                'integration/c_and_cpp/source2.c',
+                ],
+        version_script = 'integration/c_and_cpp/map.txt',
+        export_include_dirs = ['integration/c_and_cpp/include'],
+        cflags = ['-DOPAQUE_STRUCT_B=1'],
+        arch = '',
+        api = 'current',
+    ),
     Module(
         name = 'libc_and_cpp_with_unused_struct',
         srcs = ['integration/c_and_cpp/source1.cpp',

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

@@ -146,6 +146,13 @@ class MyTest(unittest.TestCase):
             "libc_and_cpp", "libc_and_cpp_with_unused_struct", 0,
             ['-check-all-apis', '-advice-only'])
 
+    def test_libc_and_cpp_opaque_pointer_diff(self):
+        self.prepare_and_run_abi_diff_all_archs(
+            "libc_and_cpp_with_opaque_ptr_a",
+            "libc_and_cpp_with_opaque_ptr_b", 8,
+            ['-consider-opaque-types-different'], True,
+            True)
+
     def test_libgolden_cpp_return_type_diff(self):
         self.prepare_and_run_abi_diff_all_archs(
             "libgolden_cpp", "libgolden_cpp_return_type_diff", 8)