[PATCH] make rich_location safe to copy

Martin Sebor msebor@gmail.com
Wed Jun 16 14:52:53 GMT 2021

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.


>> Tested on x86_64-linux.
>> Martin
> Thanks
> Dave

-------------- next part --------------
A non-text attachment was scrubbed...
Name: error.diff
Type: text/x-patch
Size: 3381 bytes
Desc: not available
URL: <https://gcc.gnu.org/pipermail/gcc-patches/attachments/20210616/e71b0b64/attachment.bin>

More information about the Gcc-patches mailing list