* [patch] PR 12028 "GDB crashes on a double free during overload resolution"
@ 2010-09-18 9:25 sami wagiaalla
2010-09-19 17:39 ` Doug Evans
0 siblings, 1 reply; 7+ messages in thread
From: sami wagiaalla @ 2010-09-18 9:25 UTC (permalink / raw)
To: gdb-patches
[-- Attachment #1: Type: text/plain, Size: 245 bytes --]
old_cleanups was being set twice making the later call to
discard_cleanups ignore the first 'make_cleanup' request.
The patch is proposed for both head and the 7.2 branch.
This has been regression tested on x8664 with gcc-4.4.4-10.fc13
Sami
[-- Attachment #2: pr12028.patch --]
[-- Type: text/x-patch, Size: 2582 bytes --]
Fix PR 12028: "GDB crashes on a double free during overload resolution "
2010-09-16 Sami Wagiaalla <swagiaal@redhat.com>
PR C++/12028
* valops.c (find_oload_champ_namespace_loop): removed incorrect
'old_cleanups' reassignment.
2010-09-16 Sami Wagiaalla <swagiaal@redhat.com>
* gdb.cp/pr12028.cc: New.
* gdb.cp/pr12028.exp: New.
diff --git a/gdb/testsuite/gdb.cp/pr12028.cc b/gdb/testsuite/gdb.cp/pr12028.cc
new file mode 100644
index 0000000..0fcab6b
--- /dev/null
+++ b/gdb/testsuite/gdb.cp/pr12028.cc
@@ -0,0 +1,21 @@
+class A{};
+class B{};
+class C: public B {};
+
+namespace D{
+ int foo (A) { return 11; }
+ int foo (C) { return 12; }
+}
+
+int main()
+{
+ A a;
+ B b;
+ C c;
+
+ D::foo (a);
+ // D::foo (b);
+ D::foo (c);
+
+ return 0;
+}
diff --git a/gdb/testsuite/gdb.cp/pr12028.exp b/gdb/testsuite/gdb.cp/pr12028.exp
new file mode 100644
index 0000000..746c6b5
--- /dev/null
+++ b/gdb/testsuite/gdb.cp/pr12028.exp
@@ -0,0 +1,29 @@
+# Copyright 2008 Free Software Foundation, Inc.
+
+# This program is free software; you can redistribute it and/or modify
+# it under the terms of the GNU General Public License as published by
+# the Free Software Foundation; either version 3 of the License, or
+# (at your option) any later version.
+#
+# This program is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+# GNU General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with this program. If not, see <http://www.gnu.org/licenses/>.
+
+set testfile pr12028
+set srcfile ${testfile}.cc
+if { [prepare_for_testing ${testfile}.exp ${testfile} ${srcfile} {debug c++}] } {
+ return -1
+}
+
+############################################
+
+if ![runto_main] then {
+ perror "couldn't run to breakpoint main"
+ continue
+}
+
+gdb_test "p D::foo(b)" "Cannot resolve function foo to any overloaded instance"
diff --git a/gdb/valops.c b/gdb/valops.c
index 7fbad10..4e83a04 100644
--- a/gdb/valops.c
+++ b/gdb/valops.c
@@ -2715,7 +2715,7 @@ find_oload_champ_namespace_loop (struct type **arg_types, int nargs,
function symbol to start off with.) */
old_cleanups = make_cleanup (xfree, *oload_syms);
- old_cleanups = make_cleanup (xfree, *oload_champ_bv);
+ make_cleanup (xfree, *oload_champ_bv);
new_namespace = alloca (namespace_len + 1);
strncpy (new_namespace, qualified_name, namespace_len);
new_namespace[namespace_len] = '\0';
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [patch] PR 12028 "GDB crashes on a double free during overload resolution"
2010-09-18 9:25 [patch] PR 12028 "GDB crashes on a double free during overload resolution" sami wagiaalla
@ 2010-09-19 17:39 ` Doug Evans
2010-09-22 18:12 ` sami wagiaalla
0 siblings, 1 reply; 7+ messages in thread
From: Doug Evans @ 2010-09-19 17:39 UTC (permalink / raw)
To: sami wagiaalla; +Cc: gdb-patches
On Fri, Sep 17, 2010 at 7:41 AM, sami wagiaalla <swagiaal@redhat.com> wrote:
> old_cleanups was being set twice making the later call to discard_cleanups
> ignore the first 'make_cleanup' request.
>
> The patch is proposed for both head and the 7.2 branch.
>
> This has been regression tested on x8664 with gcc-4.4.4-10.fc13
>
> Sami
>
Hi. This patch is ok.
Looking at the function though, is this a memory leak?
i.e. you want to do_cleanups here, not discard them (right?).
else
{
*oload_syms = new_oload_syms;
*oload_champ = new_oload_champ;
*oload_champ_bv = new_oload_champ_bv;
discard_cleanups (old_cleanups);
return 0;
}
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [patch] PR 12028 "GDB crashes on a double free during overload resolution"
2010-09-19 17:39 ` Doug Evans
@ 2010-09-22 18:12 ` sami wagiaalla
2010-09-22 19:36 ` Tom Tromey
0 siblings, 1 reply; 7+ messages in thread
From: sami wagiaalla @ 2010-09-22 18:12 UTC (permalink / raw)
To: Doug Evans; +Cc: gdb-patches
[-- Attachment #1: Type: text/plain, Size: 888 bytes --]
On 09/18/2010 12:03 PM, Doug Evans wrote:
> On Fri, Sep 17, 2010 at 7:41 AM, sami wagiaalla<swagiaal@redhat.com> wrote:
>> old_cleanups was being set twice making the later call to discard_cleanups
>> ignore the first 'make_cleanup' request.
>>
>> The patch is proposed for both head and the 7.2 branch.
>>
>> This has been regression tested on x8664 with gcc-4.4.4-10.fc13
>>
>> Sami
>>
>
> Hi. This patch is ok.
>
Thanks for the review.
> Looking at the function though, is this a memory leak?
> i.e. you want to do_cleanups here, not discard them (right?).
>
> else
> {
> *oload_syms = new_oload_syms;
> *oload_champ = new_oload_champ;
> *oload_champ_bv = new_oload_champ_bv;
> discard_cleanups (old_cleanups);
> return 0;
> }
I agree. I have attached a patch which has been regression tested on
x8664 with gcc-4.4.4-10.fc13
Sami
[-- Attachment #2: oload_memory_leak.patch --]
[-- Type: text/x-patch, Size: 621 bytes --]
Fix find_oload_champ_namespace_loop memory leak.
2010-09-22 Sami Wagiaalla <swagiaal@redhat.com>
* valops.c (find_oload_champ_namespace_loop): replace incorrect
discard_cleanups do_cleanups.
diff --git a/gdb/valops.c b/gdb/valops.c
index bbbf66e..13c83ff 100644
--- a/gdb/valops.c
+++ b/gdb/valops.c
@@ -2760,7 +2760,7 @@ find_oload_champ_namespace_loop (struct type **arg_types, int nargs,
*oload_syms = new_oload_syms;
*oload_champ = new_oload_champ;
*oload_champ_bv = new_oload_champ_bv;
- discard_cleanups (old_cleanups);
+ do_cleanups (old_cleanups);
return 0;
}
}
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [patch] PR 12028 "GDB crashes on a double free during overload resolution"
2010-09-22 18:12 ` sami wagiaalla
@ 2010-09-22 19:36 ` Tom Tromey
2010-09-23 18:46 ` sami wagiaalla
0 siblings, 1 reply; 7+ messages in thread
From: Tom Tromey @ 2010-09-22 19:36 UTC (permalink / raw)
To: sami wagiaalla; +Cc: Doug Evans, gdb-patches
>>>>> "Sami" == sami wagiaalla <swagiaal@redhat.com> writes:
Sami> I agree. I have attached a patch which has been regression tested on
Sami> x8664 with gcc-4.4.4-10.fc13
This patch is also ok, assuming that it is still valgrind-clean for the
test in question.
Tom
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [patch] PR 12028 "GDB crashes on a double free during overload resolution"
2010-09-22 19:36 ` Tom Tromey
@ 2010-09-23 18:46 ` sami wagiaalla
2010-09-27 18:02 ` Tom Tromey
0 siblings, 1 reply; 7+ messages in thread
From: sami wagiaalla @ 2010-09-23 18:46 UTC (permalink / raw)
To: Tom Tromey; +Cc: Doug Evans, gdb-patches
On 09/22/2010 03:19 PM, Tom Tromey wrote:
>>>>>> "Sami" == sami wagiaalla<swagiaal@redhat.com> writes:
>
> Sami> I agree. I have attached a patch which has been regression tested on
> Sami> x8664 with gcc-4.4.4-10.fc13
>
> This patch is also ok, assuming that it is still valgrind-clean for the
> test in question.
>
I ran valgrind like this:
valgrind ./gdb/gdb ./gdb/testsuite/gdb.cp/pr12028 -ex 'sta' -ex 'p
D::foo(b)' -ex 'q'
There are no glaring errors. The original double free error is still
gone of course and there are no others.
Incidentally, there is this error:
==8467== ERROR SUMMARY: 0 errors from 0 contexts (suppressed: 27 from 9)
==8467== Use of uninitialised value of size 8
==8467== at 0x57C966: bcache_full (bcache.c:189)
==8467== by 0x52E94F: add_psymbol_to_list (psymtab.c:1358)
==8467== by 0x596634: add_partial_symbol (dwarf2read.c:3794)
==8467== by 0x596A7A: add_partial_subprogram (dwarf2read.c:3882)
Worth figuring out but it is unrelated to this patch series. So I am
committing the above patch.
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [patch] PR 12028 "GDB crashes on a double free during overload resolution"
2010-09-23 18:46 ` sami wagiaalla
@ 2010-09-27 18:02 ` Tom Tromey
2010-09-28 16:32 ` sami wagiaalla
0 siblings, 1 reply; 7+ messages in thread
From: Tom Tromey @ 2010-09-27 18:02 UTC (permalink / raw)
To: sami wagiaalla; +Cc: Doug Evans, gdb-patches
>>>>> "Sami" == sami wagiaalla <swagiaal@redhat.com> writes:
Sami> Incidentally, there is this error:
Sami> ==8467== ERROR SUMMARY: 0 errors from 0 contexts (suppressed: 27 from 9)
Sami> ==8467== Use of uninitialised value of size 8
Sami> ==8467== at 0x57C966: bcache_full (bcache.c:189)
Sami> ==8467== by 0x52E94F: add_psymbol_to_list (psymtab.c:1358)
Sami> ==8467== by 0x596634: add_partial_symbol (dwarf2read.c:3794)
Sami> ==8467== by 0x596A7A: add_partial_subprogram (dwarf2read.c:3882)
Sami> Worth figuring out but it is unrelated to this patch series.
This is fallout from your earlier bcache changes.
I'm regtesting the appended. I already ran it through valgrind, it
definitely fixes the above report. I will check it in once the regtest
reports no regressions.
Tom
2010-09-27 Tom Tromey <tromey@redhat.com>
* bcache.c (expand_hash_table): Use hash_function, not hash.
Index: bcache.c
===================================================================
RCS file: /cvs/src/src/gdb/bcache.c,v
retrieving revision 1.28
diff -u -r1.28 bcache.c
--- bcache.c 31 Aug 2010 17:26:08 -0000 1.28
+++ bcache.c 27 Sep 2010 17:16:41 -0000
@@ -184,7 +184,8 @@
struct bstring **new_bucket;
next = s->next;
- new_bucket = &new_buckets[(hash (&s->d.data, s->length)
+ new_bucket = &new_buckets[(bcache->hash_function (&s->d.data,
+ s->length)
% new_num_buckets)];
s->next = *new_bucket;
*new_bucket = s;
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [patch] PR 12028 "GDB crashes on a double free during overload resolution"
2010-09-27 18:02 ` Tom Tromey
@ 2010-09-28 16:32 ` sami wagiaalla
0 siblings, 0 replies; 7+ messages in thread
From: sami wagiaalla @ 2010-09-28 16:32 UTC (permalink / raw)
To: Tom Tromey; +Cc: Doug Evans, gdb-patches
On 09/27/2010 01:18 PM, Tom Tromey wrote:
>>>>>> "Sami" == sami wagiaalla<swagiaal@redhat.com> writes:
>
> Sami> Incidentally, there is this error:
> Sami> ==8467== ERROR SUMMARY: 0 errors from 0 contexts (suppressed: 27 from 9)
> Sami> ==8467== Use of uninitialised value of size 8
> Sami> ==8467== at 0x57C966: bcache_full (bcache.c:189)
> Sami> ==8467== by 0x52E94F: add_psymbol_to_list (psymtab.c:1358)
> Sami> ==8467== by 0x596634: add_partial_symbol (dwarf2read.c:3794)
> Sami> ==8467== by 0x596A7A: add_partial_subprogram (dwarf2read.c:3882)
>
> Sami> Worth figuring out but it is unrelated to this patch series.
>
> This is fallout from your earlier bcache changes.
>
Totally my bad, I should have guessed that and investigated further.
Thanks for tracking this down and for the patch.
^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2010-09-28 14:55 UTC | newest]
Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2010-09-18 9:25 [patch] PR 12028 "GDB crashes on a double free during overload resolution" sami wagiaalla
2010-09-19 17:39 ` Doug Evans
2010-09-22 18:12 ` sami wagiaalla
2010-09-22 19:36 ` Tom Tromey
2010-09-23 18:46 ` sami wagiaalla
2010-09-27 18:02 ` Tom Tromey
2010-09-28 16:32 ` sami wagiaalla
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).