On 6/16/21 6:38 AM, David Malcolm wrote:
On Tue, 2021-06-15 at 19:48 -0600, Martin Sebor wrote:
Thanks for writing the patch.
While debugging locations I noticed the semi_embedded_vec template
in line-map.h doesn't declare a copy ctor or copy assignment, but
is being copied in a couple of places in the C++ parser (via
gcc_rich_location). It gets away with it most likely because it
never grows beyond the embedded buffer.
Where are these places? I wasn't aware of this.
They're in the attached file along with the diff to reproduce
the errors.
I was seeing strange behavior in my tests that led me to rich_location
and the m_ranges member. The problem turned out to be unrelated but
before I figured it out I noticed the missing copy ctor and deleted
it to see if it was being used. Since that's such a pervasive bug
in GCC code (and likely elsewhere as well) I'm thinking I should take
the time to develop the warning I've been thinking about to detect it.
The attached patch defines the copy ctor and also copy assignment
and adds the corresponding move functions.
Note that rich_location::m_fixit_hints "owns" the fixit_hint instances,
manually deleting them in rich_location's dtor, so simply doing a
shallow copy of it would be wrong.
Also, a rich_location stores other pointers (to range_labels and
diagnostic_path), which are borrowed pointers, where their lifetime is
assumed to outlive any (non-dtor) calls to the rich_location. So I'm
nervous about code that copies rich_location instances.
I think I'd prefer to forbid copying them; what's the use-case for
copying them? Am I missing something here?
I noticed and fixed just the one problem I uncovered by accident with
the missing copy ctor. If there are others I don't know about them.
Preventing code from copying rich_location might make sense
independently of fixing the vec class to be safely copyable.
Martin
Tested on x86_64-linux.
Martin
Thanks
Dave
/src/gcc/master/gcc/cp/parser.c: In function ‘tree_node* cp_parser_selection_statement(cp_parser*, bool*, vec<tree_node*>*)’:
/src/gcc/master/gcc/cp/parser.c:12358:39: error: use of deleted function ‘gcc_rich_location::gcc_rich_location(gcc_rich_location&&)’
gcc_rich_location richloc = tok->location;
^~~~~~~~
In file included from /src/gcc/master/gcc/cp/parser.c:44:
/src/gcc/master/gcc/gcc-rich-location.h:25:7: note: ‘gcc_rich_location::gcc_rich_location(gcc_rich_location&&)’ is implicitly deleted because the default definition would be ill-formed:
class gcc_rich_location : public rich_location
^~~~~~~~~~~~~~~~~
/src/gcc/master/gcc/gcc-rich-location.h:25:7: error: use of deleted function ‘rich_location::rich_location(rich_location&)’
In file included from /src/gcc/master/gcc/input.h:24,
from /src/gcc/master/gcc/coretypes.h:482,
from /src/gcc/master/gcc/cp/parser.c:24:
/src/gcc/master/gcc/../libcpp/include/line-map.h:1664:7: note: ‘rich_location::rich_location(rich_location&)’ is implicitly deleted because the default definition would be ill-formed:
class rich_location
^~~~~~~~~~~~~
/src/gcc/master/gcc/../libcpp/include/line-map.h:1664:7: error: use of deleted function ‘semi_embedded_vec<T, NUM_EMBEDDED>::semi_embedded_vec(semi_embedded_vec<T, NUM_EMBEDDED>&) [with T = location_range; int NUM_EMBEDDED = 3]’
/src/gcc/master/gcc/../libcpp/include/line-map.h:1379:3: note: declared here
semi_embedded_vec (semi_embedded_vec &) = delete;
^~~~~~~~~~~~~~~~~
/src/gcc/master/gcc/../libcpp/include/line-map.h:1664:7: error: use of deleted function ‘semi_embedded_vec<T, NUM_EMBEDDED>::semi_embedded_vec(semi_embedded_vec<T, NUM_EMBEDDED>&) [with T = fixit_hint*; int NUM_EMBEDDED = 2]’
class rich_location
^~~~~~~~~~~~~
/src/gcc/master/gcc/../libcpp/include/line-map.h:1379:3: note: declared here
semi_embedded_vec (semi_embedded_vec &) = delete;
^~~~~~~~~~~~~~~~~
In file included from /src/gcc/master/gcc/cp/parser.c:44:
/src/gcc/master/gcc/gcc-rich-location.h:31:3: note: after user-defined conversion: ‘gcc_rich_location::gcc_rich_location(location_t, const range_label*)’
gcc_rich_location (location_t loc, const range_label *label = NULL)
^~~~~~~~~~~~~~~~~
/src/gcc/master/gcc/cp/parser.c:12386:46: error: use of deleted function ‘gcc_rich_location::gcc_rich_location(gcc_rich_location&&)’
gcc_rich_location else_richloc = else_tok->location;
^~~~~~~~
In file included from /src/gcc/master/gcc/cp/parser.c:44:
/src/gcc/master/gcc/gcc-rich-location.h:31:3: note: after user-defined conversion: ‘gcc_rich_location::gcc_rich_location(location_t, const range_label*)’
gcc_rich_location (location_t loc, const range_label *label = NULL)
^~~~~~~~~~~~~~~~~
diff --git a/libcpp/include/line-map.h b/libcpp/include/line-map.h
index 7d964172469..5bedb5708ca 100644
--- a/libcpp/include/line-map.h
+++ b/libcpp/include/line-map.h
@@ -1376,6 +1376,9 @@ class semi_embedded_vec
semi_embedded_vec ();
~semi_embedded_vec ();
+ semi_embedded_vec (semi_embedded_vec &) = delete;
+ void operator= (semi_embedded_vec &) = delete;
+
unsigned int count () const { return m_num; }
T& operator[] (int idx);
const T& operator[] (int idx) const;
