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

am: 73ea62cf25

Change-Id: I26bc6587e730fad442fda69b88aa368dee30d099
This commit is contained in:
Jayant Chowdhary
2018-06-07 13:21:27 -07:00
committed by android-build-merger
10 changed files with 84 additions and 8 deletions

View File

@@ -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;

View File

@@ -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_;

View File

@@ -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),
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);

View File

@@ -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,
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();

View File

@@ -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;

View File

@@ -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;
}

View File

@@ -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(),

View File

@@ -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);

View File

@@ -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',

View File

@@ -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)