Do not allow extending pass-by-value parameters or return types

Resizing the parameters or return types changes the stack layout. It
is not an allowed extension to functions.
AbiDiffHelper::CompareFunctionTypes additionally checks the sizes of the
parameters and the return types. The difference cannot be ignored by
-allow-extensions. The user who intends to ignore the difference should
specify -ignore-symbols or -ignore-linker-set-key.

Test: ./test.py
Bug: 248418092
Change-Id: Ibef3b9260504afff3fc0260b0565736133b8e0dc
This commit is contained in:
Hsin-Yi Chen
2022-09-28 14:23:53 +08:00
parent efd3826863
commit d875d513f2
12 changed files with 416 additions and 27 deletions

View File

@@ -90,18 +90,14 @@ bool DiffWrapper<repr::FunctionIR>::DumpDiff(
std::deque<std::string> type_queue;
type_queue.push_back(oldp_->GetName());
DiffStatus param_diffs = CompareFunctionParameters(
oldp_->GetParameters(), newp_->GetParameters(), &type_queue, diff_kind);
DiffStatus return_type_diff = CompareAndDumpTypeDiff(
oldp_->GetReturnType(), newp_->GetReturnType(), &type_queue, diff_kind);
DiffStatus function_type_diff =
CompareFunctionTypes(oldp_, newp_, &type_queue, diff_kind);
CompareTemplateInfo(oldp_->GetTemplateElements(),
newp_->GetTemplateElements(),
&type_queue, diff_kind);
if ((param_diffs == DiffStatus::direct_diff ||
return_type_diff == DiffStatus::direct_diff) ||
if ((function_type_diff == DiffStatus::direct_diff) ||
(oldp_->GetAccess() != newp_->GetAccess())) {
repr::FunctionIR old_function = *oldp_;
repr::FunctionIR new_function = *newp_;

View File

@@ -260,13 +260,26 @@ bool AbiDiffHelper::CompareVTables(
return true;
}
bool AbiDiffHelper::CompareSizeAndAlignment(
const TypeIR *old_type,
const TypeIR *new_type) {
static bool CompareSizeAndAlignment(const TypeIR *old_type,
const TypeIR *new_type) {
return old_type->GetSize() == new_type->GetSize() &&
old_type->GetAlignment() == new_type->GetAlignment();
}
bool AbiDiffHelper::AreTypeSizeAndAlignmentEqual(
const std::string &old_type_id, const std::string &new_type_id) const {
AbiElementMap<const TypeIR *>::const_iterator old_it =
old_types_.find(old_type_id);
AbiElementMap<const TypeIR *>::const_iterator new_it =
new_types_.find(new_type_id);
if (old_it == old_types_.end() || new_it == new_types_.end()) {
return !diff_policy_options_.consider_opaque_types_different_;
}
return CompareSizeAndAlignment(old_it->second, new_it->second);
}
DiffStatusPair<std::unique_ptr<RecordFieldDiffIR>>
AbiDiffHelper::CompareCommonRecordFields(
const RecordFieldIR *old_field,
@@ -496,17 +509,14 @@ AbiDiffHelper::FixupDiffedFieldTypeIds(
}
DiffStatus AbiDiffHelper::CompareFunctionTypes(
const FunctionTypeIR *old_type,
const FunctionTypeIR *new_type,
std::deque<std::string> *type_queue,
DiffMessageIR::DiffKind diff_kind) {
const CFunctionLikeIR *old_type, const CFunctionLikeIR *new_type,
std::deque<std::string> *type_queue, DiffMessageIR::DiffKind diff_kind) {
DiffStatus param_diffs = CompareFunctionParameters(old_type->GetParameters(),
new_type->GetParameters(),
type_queue, diff_kind);
DiffStatus return_type_diff =
CompareAndDumpTypeDiff(old_type->GetReturnType(),
new_type->GetReturnType(),
type_queue, diff_kind);
DiffStatus return_type_diff = CompareParameterOrReturnType(
old_type->GetReturnType(), new_type->GetReturnType(), type_queue,
diff_kind);
if (param_diffs == DiffStatus::direct_diff ||
return_type_diff == DiffStatus::direct_diff) {
@@ -698,10 +708,10 @@ DiffStatus AbiDiffHelper::CompareFunctionParameters(
while (i < old_parameters_size) {
const ParamIR &old_parameter = old_parameters.at(i);
const ParamIR &new_parameter = new_parameters.at(i);
if ((CompareAndDumpTypeDiff(old_parameter.GetReferencedType(),
new_parameter.GetReferencedType(),
type_queue, diff_kind) ==
DiffStatus::direct_diff) ||
if (CompareParameterOrReturnType(old_parameter.GetReferencedType(),
new_parameter.GetReferencedType(),
type_queue,
diff_kind) == DiffStatus::direct_diff ||
(old_parameter.GetIsDefault() != new_parameter.GetIsDefault())) {
return DiffStatus::direct_diff;
}
@@ -710,6 +720,16 @@ DiffStatus AbiDiffHelper::CompareFunctionParameters(
return DiffStatus::no_diff;
}
DiffStatus AbiDiffHelper::CompareParameterOrReturnType(
const std::string &old_type_id, const std::string &new_type_id,
std::deque<std::string> *type_queue, DiffMessageIR::DiffKind diff_kind) {
if (!AreTypeSizeAndAlignmentEqual(old_type_id, new_type_id)) {
return DiffStatus::direct_diff;
}
return CompareAndDumpTypeDiff(old_type_id, new_type_id, type_queue,
diff_kind);
}
DiffStatus AbiDiffHelper::CompareAndDumpTypeDiff(
const TypeIR *old_type, const TypeIR *new_type,
LinkableMessageKind kind, std::deque<std::string> *type_queue,

View File

@@ -88,6 +88,9 @@ class AbiDiffHelper {
ignored_linker_set_keys_(ignored_linker_set_keys),
ir_diff_dumper_(ir_diff_dumper) {}
bool AreTypeSizeAndAlignmentEqual(const std::string &old_type_str,
const std::string &new_type_str) const;
DiffStatus CompareAndDumpTypeDiff(
const std::string &old_type_str, const std::string &new_type_str,
std::deque<std::string> *type_queue = nullptr,
@@ -110,8 +113,8 @@ class AbiDiffHelper {
std::deque<std::string> *type_queue,
IRDiffDumper::DiffKind diff_kind);
DiffStatus CompareFunctionTypes(const FunctionTypeIR *old_type,
const FunctionTypeIR *new_type,
DiffStatus CompareFunctionTypes(const CFunctionLikeIR *old_type,
const CFunctionLikeIR *new_type,
std::deque<std::string> *type_queue,
DiffMessageIR::DiffKind diff_kind);
@@ -121,6 +124,11 @@ class AbiDiffHelper {
std::deque<std::string> *type_queue,
IRDiffDumper::DiffKind diff_kind);
DiffStatus CompareParameterOrReturnType(const std::string &old_type_id,
const std::string &new_type_id,
std::deque<std::string> *type_queue,
IRDiffDumper::DiffKind diff_kind);
DiffStatus CompareTemplateInfo(
const std::vector<TemplateElementIR> &old_template_elements,
const std::vector<TemplateElementIR> &new_template_elements,
@@ -198,9 +206,6 @@ class AbiDiffHelper {
const VTableComponentIR &old_component,
const VTableComponentIR &new_component);
bool CompareSizeAndAlignment(const TypeIR *old_ti,
const TypeIR *new_ti);
template <typename DiffType, typename DiffElement>
bool AddToDiff(DiffType *mutable_diff, const DiffElement *oldp,
const DiffElement *newp,

View File

@@ -0,0 +1,10 @@
struct Parameter {
int member;
};
struct Return {
int member;
};
void PassByValue(Parameter);
Return ReturnByValue();

View File

@@ -0,0 +1,11 @@
struct Parameter {
int member;
int extra_member;
};
struct Return {
int member;
};
void PassByValue(Parameter);
Return ReturnByValue();

View File

@@ -0,0 +1,11 @@
struct Parameter {
int member;
};
struct Return {
int member;
int extra_member;
};
void PassByValue(Parameter);
Return ReturnByValue();

View File

@@ -0,0 +1,5 @@
libpass_by_value {
global:
_Z11PassByValue9Parameter;
_Z13ReturnByValuev;
};

View File

@@ -688,6 +688,33 @@ TEST_MODULES = [
linker_flags=['-output-format', 'Json'],
has_reference_dump=True,
),
LsdumpModule(
name='libpass_by_value',
arch='arm64',
srcs=['integration/pass_by_value/include/base.h'],
version_script='integration/pass_by_value/map.txt',
export_include_dirs=['integration/pass_by_value/include'],
linker_flags=['-output-format', 'Json'],
has_reference_dump=True,
),
LsdumpModule(
name='libparam_size_diff',
arch='arm64',
srcs=['integration/pass_by_value/include/param_size_diff.h'],
version_script='integration/pass_by_value/map.txt',
export_include_dirs=['integration/pass_by_value/include'],
linker_flags=['-output-format', 'Json'],
has_reference_dump=True,
),
LsdumpModule(
name='libreturn_size_diff',
arch='arm64',
srcs=['integration/pass_by_value/include/return_size_diff.h'],
version_script='integration/pass_by_value/map.txt',
export_include_dirs=['integration/pass_by_value/include'],
linker_flags=['-output-format', 'Json'],
has_reference_dump=True,
),
]
TEST_MODULES = {m.name: m for m in TEST_MODULES}

View File

@@ -0,0 +1,99 @@
{
"array_types" : [],
"builtin_types" :
[
{
"alignment" : 4,
"is_integral" : true,
"linker_set_key" : "_ZTIi",
"name" : "int",
"referenced_type" : "_ZTIi",
"self_type" : "_ZTIi",
"size" : 4
},
{
"linker_set_key" : "_ZTIv",
"name" : "void",
"referenced_type" : "_ZTIv",
"self_type" : "_ZTIv"
}
],
"elf_functions" :
[
{
"name" : "_Z11PassByValue9Parameter"
},
{
"name" : "_Z13ReturnByValuev"
}
],
"elf_objects" : [],
"enum_types" : [],
"function_types" : [],
"functions" :
[
{
"function_name" : "PassByValue",
"linker_set_key" : "_Z11PassByValue9Parameter",
"parameters" :
[
{
"referenced_type" : "_ZTI9Parameter"
}
],
"return_type" : "_ZTIv",
"source_file" : "development/vndk/tools/header-checker/tests/integration/pass_by_value/include/param_size_diff.h"
},
{
"function_name" : "ReturnByValue",
"linker_set_key" : "_Z13ReturnByValuev",
"return_type" : "_ZTI6Return",
"source_file" : "development/vndk/tools/header-checker/tests/integration/pass_by_value/include/param_size_diff.h"
}
],
"global_vars" : [],
"lvalue_reference_types" : [],
"pointer_types" : [],
"qualified_types" : [],
"record_types" :
[
{
"alignment" : 4,
"fields" :
[
{
"field_name" : "member",
"referenced_type" : "_ZTIi"
}
],
"linker_set_key" : "_ZTI6Return",
"name" : "Return",
"referenced_type" : "_ZTI6Return",
"self_type" : "_ZTI6Return",
"size" : 4,
"source_file" : "development/vndk/tools/header-checker/tests/integration/pass_by_value/include/param_size_diff.h"
},
{
"alignment" : 4,
"fields" :
[
{
"field_name" : "member",
"referenced_type" : "_ZTIi"
},
{
"field_name" : "extra_member",
"field_offset" : 32,
"referenced_type" : "_ZTIi"
}
],
"linker_set_key" : "_ZTI9Parameter",
"name" : "Parameter",
"referenced_type" : "_ZTI9Parameter",
"self_type" : "_ZTI9Parameter",
"size" : 8,
"source_file" : "development/vndk/tools/header-checker/tests/integration/pass_by_value/include/param_size_diff.h"
}
],
"rvalue_reference_types" : []
}

View File

@@ -0,0 +1,94 @@
{
"array_types" : [],
"builtin_types" :
[
{
"alignment" : 4,
"is_integral" : true,
"linker_set_key" : "_ZTIi",
"name" : "int",
"referenced_type" : "_ZTIi",
"self_type" : "_ZTIi",
"size" : 4
},
{
"linker_set_key" : "_ZTIv",
"name" : "void",
"referenced_type" : "_ZTIv",
"self_type" : "_ZTIv"
}
],
"elf_functions" :
[
{
"name" : "_Z11PassByValue9Parameter"
},
{
"name" : "_Z13ReturnByValuev"
}
],
"elf_objects" : [],
"enum_types" : [],
"function_types" : [],
"functions" :
[
{
"function_name" : "PassByValue",
"linker_set_key" : "_Z11PassByValue9Parameter",
"parameters" :
[
{
"referenced_type" : "_ZTI9Parameter"
}
],
"return_type" : "_ZTIv",
"source_file" : "development/vndk/tools/header-checker/tests/integration/pass_by_value/include/base.h"
},
{
"function_name" : "ReturnByValue",
"linker_set_key" : "_Z13ReturnByValuev",
"return_type" : "_ZTI6Return",
"source_file" : "development/vndk/tools/header-checker/tests/integration/pass_by_value/include/base.h"
}
],
"global_vars" : [],
"lvalue_reference_types" : [],
"pointer_types" : [],
"qualified_types" : [],
"record_types" :
[
{
"alignment" : 4,
"fields" :
[
{
"field_name" : "member",
"referenced_type" : "_ZTIi"
}
],
"linker_set_key" : "_ZTI6Return",
"name" : "Return",
"referenced_type" : "_ZTI6Return",
"self_type" : "_ZTI6Return",
"size" : 4,
"source_file" : "development/vndk/tools/header-checker/tests/integration/pass_by_value/include/base.h"
},
{
"alignment" : 4,
"fields" :
[
{
"field_name" : "member",
"referenced_type" : "_ZTIi"
}
],
"linker_set_key" : "_ZTI9Parameter",
"name" : "Parameter",
"referenced_type" : "_ZTI9Parameter",
"self_type" : "_ZTI9Parameter",
"size" : 4,
"source_file" : "development/vndk/tools/header-checker/tests/integration/pass_by_value/include/base.h"
}
],
"rvalue_reference_types" : []
}

View File

@@ -0,0 +1,99 @@
{
"array_types" : [],
"builtin_types" :
[
{
"alignment" : 4,
"is_integral" : true,
"linker_set_key" : "_ZTIi",
"name" : "int",
"referenced_type" : "_ZTIi",
"self_type" : "_ZTIi",
"size" : 4
},
{
"linker_set_key" : "_ZTIv",
"name" : "void",
"referenced_type" : "_ZTIv",
"self_type" : "_ZTIv"
}
],
"elf_functions" :
[
{
"name" : "_Z11PassByValue9Parameter"
},
{
"name" : "_Z13ReturnByValuev"
}
],
"elf_objects" : [],
"enum_types" : [],
"function_types" : [],
"functions" :
[
{
"function_name" : "PassByValue",
"linker_set_key" : "_Z11PassByValue9Parameter",
"parameters" :
[
{
"referenced_type" : "_ZTI9Parameter"
}
],
"return_type" : "_ZTIv",
"source_file" : "development/vndk/tools/header-checker/tests/integration/pass_by_value/include/return_size_diff.h"
},
{
"function_name" : "ReturnByValue",
"linker_set_key" : "_Z13ReturnByValuev",
"return_type" : "_ZTI6Return",
"source_file" : "development/vndk/tools/header-checker/tests/integration/pass_by_value/include/return_size_diff.h"
}
],
"global_vars" : [],
"lvalue_reference_types" : [],
"pointer_types" : [],
"qualified_types" : [],
"record_types" :
[
{
"alignment" : 4,
"fields" :
[
{
"field_name" : "member",
"referenced_type" : "_ZTIi"
},
{
"field_name" : "extra_member",
"field_offset" : 32,
"referenced_type" : "_ZTIi"
}
],
"linker_set_key" : "_ZTI6Return",
"name" : "Return",
"referenced_type" : "_ZTI6Return",
"self_type" : "_ZTI6Return",
"size" : 8,
"source_file" : "development/vndk/tools/header-checker/tests/integration/pass_by_value/include/return_size_diff.h"
},
{
"alignment" : 4,
"fields" :
[
{
"field_name" : "member",
"referenced_type" : "_ZTIi"
}
],
"linker_set_key" : "_ZTI9Parameter",
"name" : "Parameter",
"referenced_type" : "_ZTI9Parameter",
"self_type" : "_ZTI9Parameter",
"size" : 4,
"source_file" : "development/vndk/tools/header-checker/tests/integration/pass_by_value/include/return_size_diff.h"
}
],
"rvalue_reference_types" : []
}

View File

@@ -407,6 +407,18 @@ class HeaderCheckerTest(unittest.TestCase):
flags=["-input-format-new", "Json", "-input-format-old", "Json"],
create_old=False, create_new=False)
def test_param_size_diff(self):
self.prepare_and_run_abi_diff_all_archs(
"libpass_by_value", "libparam_size_diff", 8,
flags=["-input-format-new", "Json", "-input-format-old", "Json"],
create_old=False, create_new=False)
def test_return_size_diff(self):
self.prepare_and_run_abi_diff_all_archs(
"libpass_by_value", "libreturn_size_diff", 8,
flags=["-input-format-new", "Json", "-input-format-old", "Json"],
create_old=False, create_new=False)
if __name__ == '__main__':
unittest.main()