Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix CodeGen V2 for zero-length array #453

Merged
merged 3 commits into from
Jan 10, 2024
Merged

Fix CodeGen V2 for zero-length array #453

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
649f34a
Fix static_assert failure for zero-length array
ttreyer Jan 10, 2024
4d8506d
Fix TreeBuilder processing of zero-length array
ttreyer Jan 10, 2024
6a26f9a
Enable test arrays_member_int0
ttreyer Jan 10, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 3 additions & 4 deletions oi/OITraceCode.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -160,15 +160,14 @@ template <typename T, int32_t Id>
struct DummyAllocator<T, 0, 1, Id>
: DummyAllocatorBase<DummyAllocator, T, 0, 1, Id> {};

template <typename Type, size_t ExpectedSize, size_t ActualSize = 0>
struct validate_size {
template <typename Type, size_t ExpectedSize, size_t ActualSize>
struct validate_size_eq {
static constexpr bool value = true;
static_assert(ExpectedSize == ActualSize);
};

template <typename Type, size_t ExpectedSize>
struct validate_size<Type, ExpectedSize>
: validate_size<Type, ExpectedSize, sizeof(Type)> {};
struct validate_size : validate_size_eq<Type, ExpectedSize, sizeof(Type)> {};

template <size_t ExpectedOffset, size_t ActualOffset>
struct validate_offset {
Expand Down
17 changes: 14 additions & 3 deletions oi/TreeBuilder.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -378,9 +378,20 @@ uint64_t TreeBuilder::next() {
}

bool TreeBuilder::isContainer(const Variable& variable) {
return th->containerTypeMap.contains(variable.type) ||
(drgn_type_kind(variable.type) == DRGN_TYPE_ARRAY &&
drgn_type_length(variable.type) > 0);
if (th->containerTypeMap.contains(variable.type)) {
return true;
}

if (drgn_type_kind(variable.type) == DRGN_TYPE_ARRAY) {
/* CodeGen v1 does not consider zero-length array as containers,
* but CodeGen v2 does. This discrepancy is handled here.
* TODO: Cleanup this workaround once CodeGen v1 is gone. See #453
*/
return config.features[Feature::TypeGraph] ||
drgn_type_length(variable.type) > 0;
}

return false;
}

bool TreeBuilder::isPrimitive(struct drgn_type* type) {
Expand Down
12 changes: 4 additions & 8 deletions test/integration/arrays.toml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -42,10 +42,6 @@ definitions = '''
"capacity":10
}]}]'''
[cases.member_int0]
oil_skip = 'zero length arrays fail codegen v2' # https://github.com/facebookexperimental/object-introspection/issues/295
# WARNING: zero-length arrays are handled differently to non-empty arrays.
# They end up not being treated as containers. This should probably change
# in the future.
param_types = ["const Foo0&"]
setup = "return {};"
expect_json = '''[{
Expand All @@ -56,13 +52,13 @@ definitions = '''
"dynamicSize":0
}]}]'''
expect_json_v2 = '''[{
"staticSize":1,
"exclusiveSize":1,
"size":1,
"staticSize":0,
"exclusiveSize":0,
"size":0,
"members":[{
"staticSize":0,
"exclusiveSize":0,
"size":1
"size":0
}]}]'''
[cases.multidim_legacy] # Test for legacy behaviour. Remove with OICodeGen
oil_disable = 'oil only runs on codegen v2'
Expand Down