Fix map key in header-abi-diff while extracting user defined types.
The key being added to respective maps which facilitate diffing of
unreferenced user defined types was earlier the linker set key. The
unique id should be used instead. This is because linker set keys for C
and C++ types can be the same, but their unique id's cannot. Thus,
having linker set keys as a key leaves open the possibility of false
type aliasing, whereas the unique id does not.
Test: python3 tests/test.py
Test: python3 tests/test.py \
MyTest.test_libc_and_cpp_with_unused_struct_and_libc_and_cpp_with_unused_cstruct;
before this change, this test fails; after this change, the test
passes.
Change-Id: I91816167a48308a8c812d01e0a09e9bb70c60b50
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 ee3e229..eeacf3c 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
@@ -98,7 +98,7 @@
AbiElementMap<const abi_util::RecordTypeIR *> record_types;
// Iterate through the ODRListMap, if there is more than 1 element in the
// list, we cannot really unique the type by name, so skip it. If not, add a
- // map entry ODRListMapKey -> const Record(Enum)TypeIR *.
+ // map entry UniqueId -> const Record(Enum)TypeIR *.
for (auto &it : tu->GetODRListMap()) {
auto &odr_list = it.second;
if (odr_list.size() != 1) {
@@ -107,12 +107,14 @@
const abi_util::TypeIR *type = *(odr_list.begin());
switch (type->GetKind()) {
case abi_util::RecordTypeKind:
- record_types.emplace(type->GetLinkerSetKey(),
- static_cast<const abi_util::RecordTypeIR *>(type));
+ record_types.emplace(
+ static_cast<const abi_util::RecordTypeIR *>(type)->GetUniqueId(),
+ static_cast<const abi_util::RecordTypeIR *>(type));
break;
case abi_util::EnumTypeKind:
- enum_types.emplace(type->GetLinkerSetKey(),
- static_cast<const abi_util::EnumTypeIR *>(type));
+ enum_types.emplace(
+ static_cast<const abi_util::EnumTypeIR *>(type)->GetUniqueId(),
+ static_cast<const abi_util::EnumTypeIR *>(type));
break;
default:
// Only user defined types should have ODR list entries.
diff --git a/vndk/tools/header-checker/tests/gen_all.py b/vndk/tools/header-checker/tests/gen_all.py
index b0b3ed6..78ba2b3 100755
--- a/vndk/tools/header-checker/tests/gen_all.py
+++ b/vndk/tools/header-checker/tests/gen_all.py
@@ -16,12 +16,9 @@
INPUT_DIR = os.path.join(SCRIPT_DIR, 'input')
EXPECTED_DIR = os.path.join(SCRIPT_DIR, 'expected')
REFERENCE_DUMP_DIR = os.path.join(SCRIPT_DIR, 'reference_dumps')
-
-DEFAULT_CFLAGS = ['-x', 'c++', '-std=c++11']
-
FILE_EXTENSIONS = ['h', 'hpp', 'hxx', 'cpp', 'cc', 'c']
-def make_and_copy_reference_dumps(module, default_cflags,
+def make_and_copy_reference_dumps(module, default_cflags=[],
reference_dump_dir=REFERENCE_DUMP_DIR):
lsdump_content = module.make_lsdump(default_cflags)
return copy_reference_dump_content(module.get_name(), lsdump_content,
@@ -45,15 +42,14 @@
output_path = os.path.join(EXPECTED_DIR, input_rel_path)
print('generating', output_path, '...')
- output_content = run_header_abi_dumper(input_path, True,
- DEFAULT_CFLAGS)
+ output_content = run_header_abi_dumper(input_path, True)
os.makedirs(os.path.dirname(output_path), exist_ok=True)
with open(output_path, 'w') as f:
f.write(output_content)
modules = Module.get_test_modules()
for module in modules:
- make_and_copy_reference_dumps(module, DEFAULT_CFLAGS)
+ make_and_copy_reference_dumps(module)
return 0
diff --git a/vndk/tools/header-checker/tests/integration/c_and_cpp/include/c_and_cpp.h b/vndk/tools/header-checker/tests/integration/c_and_cpp/include/c_and_cpp.h
index e852a4e..c4a7fcd 100644
--- a/vndk/tools/header-checker/tests/integration/c_and_cpp/include/c_and_cpp.h
+++ b/vndk/tools/header-checker/tests/integration/c_and_cpp/include/c_and_cpp.h
@@ -1,7 +1,13 @@
#if INCLUDE_UNUSED_STRUCTS
+#if MAKE_UNUSED_STRUCT_C
+extern "C" {
+#endif
struct UnusedStruct {
int mUnusedMember;
};
+#if MAKE_UNUSED_STRUCT_C
+}
+#endif
#endif
class Foo {
diff --git a/vndk/tools/header-checker/tests/integration/c_and_cpp/source1.cpp b/vndk/tools/header-checker/tests/integration/c_and_cpp/source1.cpp
index 8fa6700..a90e98e 100644
--- a/vndk/tools/header-checker/tests/integration/c_and_cpp/source1.cpp
+++ b/vndk/tools/header-checker/tests/integration/c_and_cpp/source1.cpp
@@ -1,4 +1,5 @@
#include <c_and_cpp.h>
+#include <c_include.h>
Foo foo(int *a, int *b) {
// This does not make sense
diff --git a/vndk/tools/header-checker/tests/module.py b/vndk/tools/header-checker/tests/module.py
index 7c472aa..056f12e 100755
--- a/vndk/tools/header-checker/tests/module.py
+++ b/vndk/tools/header-checker/tests/module.py
@@ -137,6 +137,17 @@
api = 'current',
),
Module(
+ name = 'libc_and_cpp_with_unused_cstruct',
+ 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 = ['-DINCLUDE_UNUSED_STRUCTS=1', '-DMAKE_UNUSED_STRUCT_C=1'],
+ arch = '',
+ api = 'current',
+ ),
+ Module(
name = 'libgolden_cpp',
srcs = ['integration/cpp/gold/golden_1.cpp',
'integration/cpp/gold/high_volume_speaker.cpp',
diff --git a/vndk/tools/header-checker/tests/reference_dumps/arm/libreproducability.so.lsdump b/vndk/tools/header-checker/tests/reference_dumps/arm/libreproducability.so.lsdump
index 406c66c..673efb5 100644
--- a/vndk/tools/header-checker/tests/reference_dumps/arm/libreproducability.so.lsdump
+++ b/vndk/tools/header-checker/tests/reference_dumps/arm/libreproducability.so.lsdump
@@ -17,7 +17,7 @@
access: public_access
record_kind: struct_kind
tag_info {
- unique_id: "_ZTS11ShouldRepro"
+ unique_id: "ShouldRepro"
}
}
builtin_types {
diff --git a/vndk/tools/header-checker/tests/reference_dumps/arm64/libreproducability.so.lsdump b/vndk/tools/header-checker/tests/reference_dumps/arm64/libreproducability.so.lsdump
index 406c66c..673efb5 100644
--- a/vndk/tools/header-checker/tests/reference_dumps/arm64/libreproducability.so.lsdump
+++ b/vndk/tools/header-checker/tests/reference_dumps/arm64/libreproducability.so.lsdump
@@ -17,7 +17,7 @@
access: public_access
record_kind: struct_kind
tag_info {
- unique_id: "_ZTS11ShouldRepro"
+ unique_id: "ShouldRepro"
}
}
builtin_types {
diff --git a/vndk/tools/header-checker/tests/reference_dumps/mips/libreproducability.so.lsdump b/vndk/tools/header-checker/tests/reference_dumps/mips/libreproducability.so.lsdump
index 406c66c..673efb5 100644
--- a/vndk/tools/header-checker/tests/reference_dumps/mips/libreproducability.so.lsdump
+++ b/vndk/tools/header-checker/tests/reference_dumps/mips/libreproducability.so.lsdump
@@ -17,7 +17,7 @@
access: public_access
record_kind: struct_kind
tag_info {
- unique_id: "_ZTS11ShouldRepro"
+ unique_id: "ShouldRepro"
}
}
builtin_types {
diff --git a/vndk/tools/header-checker/tests/reference_dumps/mips64/libreproducability.so.lsdump b/vndk/tools/header-checker/tests/reference_dumps/mips64/libreproducability.so.lsdump
index 406c66c..673efb5 100644
--- a/vndk/tools/header-checker/tests/reference_dumps/mips64/libreproducability.so.lsdump
+++ b/vndk/tools/header-checker/tests/reference_dumps/mips64/libreproducability.so.lsdump
@@ -17,7 +17,7 @@
access: public_access
record_kind: struct_kind
tag_info {
- unique_id: "_ZTS11ShouldRepro"
+ unique_id: "ShouldRepro"
}
}
builtin_types {
diff --git a/vndk/tools/header-checker/tests/reference_dumps/x86/libreproducability.so.lsdump b/vndk/tools/header-checker/tests/reference_dumps/x86/libreproducability.so.lsdump
index 406c66c..673efb5 100644
--- a/vndk/tools/header-checker/tests/reference_dumps/x86/libreproducability.so.lsdump
+++ b/vndk/tools/header-checker/tests/reference_dumps/x86/libreproducability.so.lsdump
@@ -17,7 +17,7 @@
access: public_access
record_kind: struct_kind
tag_info {
- unique_id: "_ZTS11ShouldRepro"
+ unique_id: "ShouldRepro"
}
}
builtin_types {
diff --git a/vndk/tools/header-checker/tests/reference_dumps/x86_64/libreproducability.so.lsdump b/vndk/tools/header-checker/tests/reference_dumps/x86_64/libreproducability.so.lsdump
index 406c66c..673efb5 100644
--- a/vndk/tools/header-checker/tests/reference_dumps/x86_64/libreproducability.so.lsdump
+++ b/vndk/tools/header-checker/tests/reference_dumps/x86_64/libreproducability.so.lsdump
@@ -17,7 +17,7 @@
access: public_access
record_kind: struct_kind
tag_info {
- unique_id: "_ZTS11ShouldRepro"
+ unique_id: "ShouldRepro"
}
}
builtin_types {
diff --git a/vndk/tools/header-checker/tests/test.py b/vndk/tools/header-checker/tests/test.py
index a43c227..f6a7a1f 100755
--- a/vndk/tools/header-checker/tests/test.py
+++ b/vndk/tools/header-checker/tests/test.py
@@ -14,7 +14,6 @@
run_abi_diff, run_header_abi_dumper)
from module import Module
from gen_all import make_and_copy_reference_dumps
-from gen_all import DEFAULT_CFLAGS
SCRIPT_DIR = os.path.abspath(os.path.dirname(__file__))
INPUT_DIR = os.path.join(SCRIPT_DIR, 'input')
@@ -64,7 +63,7 @@
def create_ref_dump(self, name, dir_name, target_arch):
module_bare = Module.get_test_module_by_name(name)
module = Module.mutate_module_for_arch(module_bare, target_arch)
- return make_and_copy_reference_dumps(module, DEFAULT_CFLAGS,
+ return make_and_copy_reference_dumps(module, [],
dir_name)
def get_or_create_ref_dump(self, name, target_arch, dir_name, create):
@@ -135,6 +134,12 @@
"libc_and_cpp", "libc_and_cpp_with_unused_struct", 1,
['-check-all-apis'])
+ def test_libc_and_cpp_with_unused_struct_and_libc_and_cpp_with_unused_cstruct(self):
+ self.prepare_and_run_abi_diff_all_archs(
+ "libc_and_cpp_with_unused_struct",
+ "libc_and_cpp_with_unused_cstruct", 0,
+ ['-check-all-apis', '-allow-unreferenced-changes'])
+
def test_libc_and_cpp_and_libc_and_cpp_with_unused_struct_check_all_advice(
self):
self.prepare_and_run_abi_diff_all_archs(
diff --git a/vndk/tools/header-checker/utils/utils.py b/vndk/tools/header-checker/utils/utils.py
index 83cd8ee..e199a5b 100644
--- a/vndk/tools/header-checker/utils/utils.py
+++ b/vndk/tools/header-checker/utils/utils.py
@@ -22,6 +22,9 @@
SOURCE_ABI_DUMP_EXT_END = '.lsdump'
SOURCE_ABI_DUMP_EXT = SO_EXT + SOURCE_ABI_DUMP_EXT_END
+DEFAULT_CPPFLAGS = ['-x', 'c++', '-std=c++11']
+DEFAULT_CFLAGS = ['-std=gnu99']
+
TARGET_ARCHS = ['arm', 'arm64', 'x86', 'x86_64', 'mips', 'mips64']
def get_reference_dump_dir(reference_dump_dir_stem,
@@ -85,11 +88,17 @@
def run_header_abi_dumper_on_file(input_path, output_path,
export_include_dirs = [], cflags =[]):
+ input_name, input_ext = os.path.splitext(input_path)
cmd = ['header-abi-dumper', '-o', output_path, input_path,]
for dir in export_include_dirs:
cmd += ['-I', dir]
cmd += ['--']
cmd += cflags
+ if input_ext == '.cpp' or input_ext == '.cc' or input_ext == '.h':
+ cmd += DEFAULT_CPPFLAGS
+ else:
+ cmd += DEFAULT_CFLAGS
+
for dir in BUILTIN_HEADERS_DIR:
cmd += ['-isystem', dir]
# export includes imply local includes
