From: Lancelot SIX <lsix@lancelotsix.com>
Date: Tue, 19 Oct 2021 21:51:40 +0000 (+0000)
Subject: Improve gdb::array_view ctor from contiguous containers
X-Git-Url: https://git.libre-soc.org/?a=commitdiff_plain;h=e92f2b5eef008a8617bcd6d878b09848adf6ea7f;p=binutils-gdb.git

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 <typename T>
	void print (const gdb::array_view<const T> &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<Child, 3> elts = {
	  Child {1, 2},
	  Child {3, 4},
	  Child {5, 6}
	};
	print_all<Parent> (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<Parent>
shold not compile at all (calling print_all<Child> 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.
---

diff --git a/gdb/dwarf2/read.c b/gdb/dwarf2/read.c
index 00856b86483..ed101237587 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<variant_part> parts
+  gdb::array_view<const variant_part> parts
     = create_variant_parts (&objfile->objfile_obstack, offset_map, fip,
 			    fip->variant_parts);
 
diff --git a/gdb/maint.c b/gdb/maint.c
index bcc71aab579..e03abfacdae 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 b62369bc8cd..43b7434d10f 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 <array>
+#include <vector>
 
 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 <C, array_view<B>> ());
 }
 
+/* Check that there's no container->view conversion for containers of derived
+   types or subclasses.  */
+
+template<template<typename ...> typename Container>
+static constexpr bool
+check_ctor_from_container ()
+{
+  using gdb::array_view;
+
+  return (    is_convertible <Container<A>, array_view<A>> ()
+	  && !is_convertible <Container<B>, array_view<A>> ()
+	  && !is_convertible <Container<C>, array_view<A>> ()
+
+	  && !is_convertible <Container<A>, array_view<B>> ()
+	  &&  is_convertible <Container<B>, array_view<B>> ()
+	  && !is_convertible <Container<C>, array_view<B>> ());
+}
+
 } /* namespace no_slicing */
 
 static_assert (no_slicing::check (), "");
+static_assert (no_slicing::check_ctor_from_container<std::vector> (), "");
+static_assert (no_slicing::check_ctor_from_container<gdb::array_view> (), "");
 
 /* Check that array_view implicitly converts from std::vector.  */
 
diff --git a/gdbsupport/array-view.h b/gdbsupport/array-view.h
index ab1d032c60e..6f7e45a979c 100644
--- a/gdbsupport/array-view.h
+++ b/gdbsupport/array-view.h
@@ -139,9 +139,10 @@ public:
   template<typename Container,
 	   typename = Requires<gdb::Not<IsDecayedT<Container>>>,
 	   typename
-	     = Requires<std::is_convertible
-			<decltype (std::declval<Container> ().data ()),
-			 T *>>,
+	     = Requires<DecayedConvertible
+			<typename std::remove_pointer
+			 <decltype (std::declval<Container> ().data ())
+			 >::type>>,
 	   typename
 	     = Requires<std::is_convertible
 			<decltype (std::declval<Container> ().size ()),