From e92f2b5eef008a8617bcd6d878b09848adf6ea7f Mon Sep 17 00:00:00 2001 From: Lancelot SIX Date: Tue, 19 Oct 2021 21:51:40 +0000 Subject: Improve gdb::array_view ctor from contiguous containers While reading the interface of gdb::array_view, I realized that the constructor that builds an array_view on top of a contiguous container (such as std::vector, std::array or even gdb::array_view) can be missused. Lets consider the following code sample: struct Parent { Parent (int a): a { a } {} int a; }; std::ostream &operator<< (std::ostream& os, const Parent & p) { os << "Parent {a=" << p.a << "}"; return os; } struct Child : public Parent { Child (int a, int b): Parent { a }, b { b } {} int b; }; std::ostream &operator<< (std::ostream& os, const Child & p) { os << "Child {a=" << p.a << ", b=" << p.b << "}"; return os; } template void print (const gdb::array_view &p) { std::for_each (p.begin (), p.end (), [](const T &p) { std::cout << p << '\n'; }); } Then with the current interface nothinng prevents this usage of array_view to be done: const std::array elts = { Child {1, 2}, Child {3, 4}, Child {5, 6} }; print_all (elts); This compiles fine and produces the following output: Parent {a=1} Parent {a=2} Parent {a=3} which is obviously wrong. There is nowhere in memory a Parent-like object for which the A member is 2 and this call to print_all shold not compile at all (calling print_all is however fine). This comes down to the fact that a Child* is convertible into a Parent*, and that an array view is constructed to a pointer to the first element and a size. The valid type pointed to that can be used with this constructor are restricted using SFINAE, which requires that a pointer to a member into the underlying container can be converted into a pointer the array_view's data type. This patch proposes to change the constraints on the gdb::array_view ctor which accepts a container now requires that the (decayed) type of the elements in the container match the (decayed) type of the array_view being constructed. Applying this change required minimum adjustment in GDB codebase, which are also included in this patch. Tested by rebuilding. --- gdb/dwarf2/read.c | 2 +- gdb/maint.c | 2 +- gdb/unittests/array-view-selftests.c | 25 +++++++++++++++++++++++-- 3 files changed, 25 insertions(+), 4 deletions(-) (limited to 'gdb') diff --git a/gdb/dwarf2/read.c b/gdb/dwarf2/read.c index 00856b8..ed10123 100644 --- a/gdb/dwarf2/read.c +++ b/gdb/dwarf2/read.c @@ -14936,7 +14936,7 @@ add_variant_property (struct field_info *fip, struct type *type, offset_map[fip->fields[i].offset] = i; struct objfile *objfile = cu->per_objfile->objfile; - gdb::array_view parts + gdb::array_view parts = create_variant_parts (&objfile->objfile_obstack, offset_map, fip, fip->variant_parts); diff --git a/gdb/maint.c b/gdb/maint.c index bcc71aa..e03abfa 100644 --- a/gdb/maint.c +++ b/gdb/maint.c @@ -1158,7 +1158,7 @@ maintenance_selftest (const char *args, int from_tty) auto grp = make_maintenance_selftest_option_group (&opts); gdb::option::process_options (&args, gdb::option::PROCESS_OPTIONS_UNKNOWN_IS_ERROR, grp); - gdb_argv argv (args); + const gdb_argv argv (args); selftests::run_tests (argv.as_array_view (), opts.verbose); #else printf_filtered (_("\ diff --git a/gdb/unittests/array-view-selftests.c b/gdb/unittests/array-view-selftests.c index b62369b..43b7434 100644 --- a/gdb/unittests/array-view-selftests.c +++ b/gdb/unittests/array-view-selftests.c @@ -21,6 +21,7 @@ #include "gdbsupport/selftest.h" #include "gdbsupport/array-view.h" #include +#include namespace selftests { namespace array_view_tests { @@ -86,8 +87,8 @@ struct A { int i; }; struct B : A { int j; }; struct C : A { int l; }; -/* Check that there's no array->view conversion for arrays of derived - types or subclasses. */ +/* Check that there's no array->view conversion for arrays of derived types or + subclasses. */ static constexpr bool check () { @@ -116,9 +117,29 @@ check () && !is_convertible > ()); } +/* Check that there's no container->view conversion for containers of derived + types or subclasses. */ + +template typename Container> +static constexpr bool +check_ctor_from_container () +{ + using gdb::array_view; + + return ( is_convertible , array_view> () + && !is_convertible , array_view> () + && !is_convertible , array_view> () + + && !is_convertible , array_view> () + && is_convertible , array_view> () + && !is_convertible , array_view> ()); +} + } /* namespace no_slicing */ static_assert (no_slicing::check (), ""); +static_assert (no_slicing::check_ctor_from_container (), ""); +static_assert (no_slicing::check_ctor_from_container (), ""); /* Check that array_view implicitly converts from std::vector. */ -- cgit v1.1