Merge "header-abi-diff: Add a flag to consider opaque types with different names structurally unequal."
diff --git a/vndk/tools/header-checker/header-abi-diff/src/abi_diff.cpp b/vndk/tools/header-checker/header-abi-diff/src/abi_diff.cpp
index 936d905..9d8acf5 100644
--- a/vndk/tools/header-checker/header-abi-diff/src/abi_diff.cpp
+++ b/vndk/tools/header-checker/header-abi-diff/src/abi_diff.cpp
@@ -360,7 +360,9 @@
}
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;
diff --git a/vndk/tools/header-checker/header-abi-diff/src/abi_diff.h b/vndk/tools/header-checker/header-abi-diff/src/abi_diff.h
index 18e785d..9dcedc0 100644
--- a/vndk/tools/header-checker/header-abi-diff/src/abi_diff.h
+++ b/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 @@
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 @@
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_;
diff --git a/vndk/tools/header-checker/header-abi-diff/src/abi_diff_wrappers.h b/vndk/tools/header-checker/header-abi-diff/src/abi_diff_wrappers.h
index 5376d2f..92c0475 100644
--- a/vndk/tools/header-checker/header-abi-diff/src/abi_diff_wrappers.h
+++ b/vndk/tools/header-checker/header-abi-diff/src/abi_diff_wrappers.h
@@ -42,9 +42,10 @@
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),
- oldp_(oldp), newp_(newp) { }
+ : AbiDiffHelper(old_types, new_types, diff_policy_options, type_cache,
+ ir_diff_dumper), oldp_(oldp), newp_(newp) { }
bool DumpDiff(abi_util::IRDiffDumper::DiffKind diff_kind);
diff --git a/vndk/tools/header-checker/header-abi-diff/src/header_abi_diff.cpp b/vndk/tools/header-checker/header-abi-diff/src/header_abi_diff.cpp
index f576470..9cb95c8 100644
--- a/vndk/tools/header-checker/header-abi-diff/src/header_abi_diff.cpp
+++ b/vndk/tools/header-checker/header-abi-diff/src/header_abi_diff.cpp
@@ -85,6 +85,14 @@
" 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 @@
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,
- text_format_new, text_format_diff);
+ ignored_symbols, diff_policy_options, check_all_apis,
+ text_format_old, text_format_new, text_format_diff);
abi_util::CompatibilityStatusIR status = judge.GenerateCompatibilityReport();
diff --git a/vndk/tools/header-checker/header-abi-util/include/abi_diff_helpers.h b/vndk/tools/header-checker/header-abi-util/include/abi_diff_helpers.h
index 6403332..839bac0 100644
--- a/vndk/tools/header-checker/header-abi-util/include/abi_diff_helpers.h
+++ b/vndk/tools/header-checker/header-abi-util/include/abi_diff_helpers.h
@@ -19,6 +19,8 @@
// 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 @@
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 @@
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;
diff --git a/vndk/tools/header-checker/header-abi-util/src/abi_diff_helpers.cpp b/vndk/tools/header-checker/header-abi-util/src/abi_diff_helpers.cpp
index 3d19d71..616f28f 100644
--- a/vndk/tools/header-checker/header-abi-util/src/abi_diff_helpers.cpp
+++ b/vndk/tools/header-checker/header-abi-util/src/abi_diff_helpers.cpp
@@ -786,6 +786,9 @@
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 @@
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;
}
diff --git a/vndk/tools/header-checker/header-abi-util/src/ir_representation.cpp b/vndk/tools/header-checker/header-abi-util/src/ir_representation.cpp
index 8ccb5be..cf1e2fb 100644
--- a/vndk/tools/header-checker/header-abi-util/src/ir_representation.cpp
+++ b/vndk/tools/header-checker/header-abi-util/src/ir_representation.cpp
@@ -123,7 +123,9 @@
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(),
diff --git a/vndk/tools/header-checker/tests/integration/c_and_cpp/include/c_include.h b/vndk/tools/header-checker/tests/integration/c_and_cpp/include/c_include.h
index 42fc6e0..d19ce8d 100644
--- a/vndk/tools/header-checker/tests/integration/c_and_cpp/include/c_include.h
+++ b/vndk/tools/header-checker/tests/integration/c_and_cpp/include/c_include.h
@@ -6,9 +6,17 @@
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);
diff --git a/vndk/tools/header-checker/tests/module.py b/vndk/tools/header-checker/tests/module.py
index 0df8b2a..b7f9cde 100755
--- a/vndk/tools/header-checker/tests/module.py
+++ b/vndk/tools/header-checker/tests/module.py
@@ -126,6 +126,28 @@
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',
'integration/c_and_cpp/source2.c',
diff --git a/vndk/tools/header-checker/tests/test.py b/vndk/tools/header-checker/tests/test.py
index ed186ad..8196a33 100755
--- a/vndk/tools/header-checker/tests/test.py
+++ b/vndk/tools/header-checker/tests/test.py
@@ -146,6 +146,13 @@
"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)
