* [PATCH] Make dtor of mapped_index_base virtual
@ 2017-12-30 18:21 Павел Крюков
2017-12-31 1:28 ` Simon Marchi
0 siblings, 1 reply; 4+ messages in thread
From: Павел Крюков @ 2017-12-30 18:21 UTC (permalink / raw)
To: gdb-patches
gdb/Changelog:
2017-12-30 Pavel I. Kryukov <kryukov@frtk.ru>
* dwarf2read.c (mapped_index_base): Make dtor virtual
diff --git a/gdb/ChangeLog b/gdb/ChangeLog
index 8555b55..fc4c1b3 100644
--- a/gdb/ChangeLog
+++ b/gdb/ChangeLog
@@ -1,3 +1,7 @@
+2017-12-30 Pavel I. Kryukov <kryukov@frtk.ru>
+
+ * dwarf2read.c (mapped_index_base): Make dtor virtual
+
2017-12-28 Simon Marchi <simon.marchi@polymtl.ca>
* target.h (enum target_object) <TARGET_OBJECT_HPUX_UREGS,
diff --git a/gdb/dwarf2read.c b/gdb/dwarf2read.c
index 484cbce..3ac4380 100644
--- a/gdb/dwarf2read.c
+++ b/gdb/dwarf2read.c
@@ -272,7 +272,7 @@ struct mapped_index_base
/* Prevent deleting/destroying via a base class pointer. */
protected:
- ~mapped_index_base() = default;
+ virtual ~mapped_index_base() = default;
};
/* A description of the mapped index. The file format is described in
--
2.7.4
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH] Make dtor of mapped_index_base virtual
2017-12-30 18:21 [PATCH] Make dtor of mapped_index_base virtual Павел Крюков
@ 2017-12-31 1:28 ` Simon Marchi
2017-12-31 10:12 ` Павел Крюков
0 siblings, 1 reply; 4+ messages in thread
From: Simon Marchi @ 2017-12-31 1:28 UTC (permalink / raw)
To: Павел
Крюков
Cc: gdb-patches
On 2017-12-30 13:21, Ðавел ÐÑÑков wrote:
> gdb/Changelog:
> 2017-12-30 Pavel I. Kryukov <kryukov@frtk.ru>
>
> * dwarf2read.c (mapped_index_base): Make dtor virtual
>
> diff --git a/gdb/ChangeLog b/gdb/ChangeLog
> index 8555b55..fc4c1b3 100644
> --- a/gdb/ChangeLog
> +++ b/gdb/ChangeLog
> @@ -1,3 +1,7 @@
> +2017-12-30 Pavel I. Kryukov <kryukov@frtk.ru>
> +
> + * dwarf2read.c (mapped_index_base): Make dtor virtual
> +
> 2017-12-28 Simon Marchi <simon.marchi@polymtl.ca>
>
> * target.h (enum target_object) <TARGET_OBJECT_HPUX_UREGS,
> diff --git a/gdb/dwarf2read.c b/gdb/dwarf2read.c
> index 484cbce..3ac4380 100644
> --- a/gdb/dwarf2read.c
> +++ b/gdb/dwarf2read.c
> @@ -272,7 +272,7 @@ struct mapped_index_base
>
> /* Prevent deleting/destroying via a base class pointer. */
> protected:
> - ~mapped_index_base() = default;
> + virtual ~mapped_index_base() = default;
> };
>
> /* A description of the mapped index. The file format is described in
> --
> 2.7.4
Hi Pavel,
Can you clarify what you are trying to fix/improve with this patch?
Since the goal is that we don't delete through a pointer of this class,
does the destructor need to be virtual (not that it would hurt
anything)? Do you get a build error or something?
Simon
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH] Make dtor of mapped_index_base virtual
2017-12-31 1:28 ` Simon Marchi
@ 2017-12-31 10:12 ` Павел Крюков
2017-12-31 13:49 ` Simon Marchi
0 siblings, 1 reply; 4+ messages in thread
From: Павел Крюков @ 2017-12-31 10:12 UTC (permalink / raw)
To: Simon Marchi; +Cc: gdb-patches
Hi Simon
> Do you get a build error or something?
Yes, I get a build error with Clang:
dwarf2read.c:25409:5: error: destructor called on non-final 'mapped_index'
that
has virtual functions but non-virtual destructor
[-Werror,-Wdelete-non-virtual-dtor]
data->index_table->~mapped_index ();
> Since the goal is that we don't delete through a pointer of this class, does
the destructor need to be virtual (not that it would hurt anything)?
Just to handle the case if someone would delete through a pointer to
"mapped_index_base" class.
Thanks,
--
Pavel
2017-12-31 4:28 GMT+03:00 Simon Marchi <simon.marchi@polymtl.ca>:
> On 2017-12-30 13:21, Павел Крюков wrote:
>
>> gdb/Changelog:
>> 2017-12-30 Pavel I. Kryukov <kryukov@frtk.ru>
>>
>> * dwarf2read.c (mapped_index_base): Make dtor virtual
>>
>> diff --git a/gdb/ChangeLog b/gdb/ChangeLog
>> index 8555b55..fc4c1b3 100644
>> --- a/gdb/ChangeLog
>> +++ b/gdb/ChangeLog
>> @@ -1,3 +1,7 @@
>> +2017-12-30 Pavel I. Kryukov <kryukov@frtk.ru>
>> +
>> + * dwarf2read.c (mapped_index_base): Make dtor virtual
>> +
>> 2017-12-28 Simon Marchi <simon.marchi@polymtl.ca>
>>
>> * target.h (enum target_object) <TARGET_OBJECT_HPUX_UREGS,
>> diff --git a/gdb/dwarf2read.c b/gdb/dwarf2read.c
>> index 484cbce..3ac4380 100644
>> --- a/gdb/dwarf2read.c
>> +++ b/gdb/dwarf2read.c
>> @@ -272,7 +272,7 @@ struct mapped_index_base
>>
>> /* Prevent deleting/destroying via a base class pointer. */
>> protected:
>> - ~mapped_index_base() = default;
>> + virtual ~mapped_index_base() = default;
>> };
>>
>> /* A description of the mapped index. The file format is described in
>> --
>> 2.7.4
>>
>
> Hi Pavel,
>
> Can you clarify what you are trying to fix/improve with this patch? Since
> the goal is that we don't delete through a pointer of this class, does the
> destructor need to be virtual (not that it would hurt anything)? Do you
> get a build error or something?
>
> Simon
>
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH] Make dtor of mapped_index_base virtual
2017-12-31 10:12 ` Павел Крюков
@ 2017-12-31 13:49 ` Simon Marchi
0 siblings, 0 replies; 4+ messages in thread
From: Simon Marchi @ 2017-12-31 13:49 UTC (permalink / raw)
To: Павел
Крюков
Cc: gdb-patches
On 2017-12-31 05:12, Ðавел ÐÑÑков wrote:
> Hi Simon
>
>> Do you get a build error or something?
>
> Yes, I get a build error with Clang:
>
> dwarf2read.c:25409:5: error: destructor called on non-final
> 'mapped_index'
> that
> has virtual functions but non-virtual destructor
> [-Werror,-Wdelete-non-virtual-dtor]
> data->index_table->~mapped_index ();
>
>> Since the goal is that we don't delete through a pointer of this
>> class, does
> the destructor need to be virtual (not that it would hurt anything)?
>
> Just to handle the case if someone would delete through a pointer to
> "mapped_index_base" class.
>
> Thanks,
Hi Pavel,
I think this warning was fixed by the following commit:
https://sourceware.org/git/gitweb.cgi?p=binutils-gdb.git;a=commit;h=fc898b42e355fef58e6a029799fdd71b9dda5dc6
We don't want anything deleting through a pointer of the base class,
because (for now) instances of the two child classes (mapped_index and
mapped_debug_names) are allocated differently. One is allocated on the
objfile obstack (some kind of manual memory block allocation) and the
other is allocated with new. Trying to delete or manually call the
destructor through a pointer to mapped_index_base would do the wrong
thing for one of the two children. So it's on purpose that
mapped_index_base's destructor is hidden, and it doesn't matter whether
or not it is virtual.
Thanks,
Simon
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2017-12-31 13:49 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-12-30 18:21 [PATCH] Make dtor of mapped_index_base virtual Павел Крюков
2017-12-31 1:28 ` Simon Marchi
2017-12-31 10:12 ` Павел Крюков
2017-12-31 13:49 ` Simon Marchi
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).