* [RFA 1/3] PR symtab/20652 - fix psymbol_compare
2016-10-03 21:54 [RFA 0/3] Fix various bugs found by static analysis Tom Tromey
@ 2016-10-03 21:54 ` Tom Tromey
2016-10-04 10:55 ` Yao Qi
2016-10-04 18:37 ` Pedro Alves
2016-10-03 21:54 ` [RFA 3/3] PR remote/20655 - small fix in handle_tracepoint_bkpts Tom Tromey
` (2 subsequent siblings)
3 siblings, 2 replies; 14+ messages in thread
From: Tom Tromey @ 2016-10-03 21:54 UTC (permalink / raw)
To: gdb-patches; +Cc: Tom Tromey
This fixes an oversight in psymbol_compare.
2016-10-03 Tom Tromey <tom@tromey.com>
PR symtab/20652:
* psymtab.c (psymbol_compare): Correctly compare "ginfo.value"
fields.
---
gdb/ChangeLog | 6 ++++++
gdb/psymtab.c | 2 +-
2 files changed, 7 insertions(+), 1 deletion(-)
diff --git a/gdb/ChangeLog b/gdb/ChangeLog
index ffbdc3d..434f750 100644
--- a/gdb/ChangeLog
+++ b/gdb/ChangeLog
@@ -1,3 +1,9 @@
+2016-10-03 Tom Tromey <tom@tromey.com>
+
+ PR symtab/20652:
+ * psymtab.c (psymbol_compare): Correctly compare "ginfo.value"
+ fields.
+
2016-09-29 Jan Kratochvil <jan.kratochvil@redhat.com>
PR gdb/20609 - attach of JIT-debug-enabled inf 7.11.1 regression
diff --git a/gdb/psymtab.c b/gdb/psymtab.c
index a39c6e2..edcaa8b 100644
--- a/gdb/psymtab.c
+++ b/gdb/psymtab.c
@@ -1577,7 +1577,7 @@ psymbol_compare (const void *addr1, const void *addr2, int length)
struct partial_symbol *sym1 = (struct partial_symbol *) addr1;
struct partial_symbol *sym2 = (struct partial_symbol *) addr2;
- return (memcmp (&sym1->ginfo.value, &sym1->ginfo.value,
+ return (memcmp (&sym1->ginfo.value, &sym2->ginfo.value,
sizeof (sym1->ginfo.value)) == 0
&& sym1->ginfo.language == sym2->ginfo.language
&& PSYMBOL_DOMAIN (sym1) == PSYMBOL_DOMAIN (sym2)
--
2.7.4
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [RFA 1/3] PR symtab/20652 - fix psymbol_compare
2016-10-03 21:54 ` [RFA 1/3] PR symtab/20652 - fix psymbol_compare Tom Tromey
@ 2016-10-04 10:55 ` Yao Qi
2016-10-04 18:37 ` Pedro Alves
1 sibling, 0 replies; 14+ messages in thread
From: Yao Qi @ 2016-10-04 10:55 UTC (permalink / raw)
To: Tom Tromey; +Cc: gdb-patches
On Mon, Oct 3, 2016 at 10:54 PM, Tom Tromey <tom@tromey.com> wrote:
> This fixes an oversight in psymbol_compare.
>
> 2016-10-03 Tom Tromey <tom@tromey.com>
>
> PR symtab/20652:
> * psymtab.c (psymbol_compare): Correctly compare "ginfo.value"
> fields.
Patch is good to me.
--
Yao (齐尧)
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [RFA 1/3] PR symtab/20652 - fix psymbol_compare
2016-10-03 21:54 ` [RFA 1/3] PR symtab/20652 - fix psymbol_compare Tom Tromey
2016-10-04 10:55 ` Yao Qi
@ 2016-10-04 18:37 ` Pedro Alves
2016-10-04 18:52 ` Tom Tromey
1 sibling, 1 reply; 14+ messages in thread
From: Pedro Alves @ 2016-10-04 18:37 UTC (permalink / raw)
To: Tom Tromey, gdb-patches
On 10/03/2016 10:54 PM, Tom Tromey wrote:
> @@ -1577,7 +1577,7 @@ psymbol_compare (const void *addr1, const void *addr2, int length)
> struct partial_symbol *sym1 = (struct partial_symbol *) addr1;
> struct partial_symbol *sym2 = (struct partial_symbol *) addr2;
>
> - return (memcmp (&sym1->ginfo.value, &sym1->ginfo.value,
> + return (memcmp (&sym1->ginfo.value, &sym2->ginfo.value,
> sizeof (sym1->ginfo.value)) == 0
I wonder whether using memcpy is really the correct thing
here, considering ginfo.value is a union. I.e., thinking about
padding bits not part of the active member. Maybe we're zero
initializing the whole value and it works in practice because
of that.
> && sym1->ginfo.language == sym2->ginfo.language
> && PSYMBOL_DOMAIN (sym1) == PSYMBOL_DOMAIN (sym2)
Thanks,
Pedro Alves
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [RFA 1/3] PR symtab/20652 - fix psymbol_compare
2016-10-04 18:37 ` Pedro Alves
@ 2016-10-04 18:52 ` Tom Tromey
0 siblings, 0 replies; 14+ messages in thread
From: Tom Tromey @ 2016-10-04 18:52 UTC (permalink / raw)
To: Pedro Alves; +Cc: Tom Tromey, gdb-patches
>>>>> "Pedro" == Pedro Alves <palves@redhat.com> writes:
Pedro> I wonder whether using memcpy is really the correct thing
Pedro> here, considering ginfo.value is a union. I.e., thinking about
Pedro> padding bits not part of the active member. Maybe we're zero
Pedro> initializing the whole value and it works in practice because
Pedro> of that.
Yes, psymbols are fully zero-initialized so that they can be entered
into the bcache. See add_psymbol_to_bcache.
Tom
^ permalink raw reply [flat|nested] 14+ messages in thread
* [RFA 3/3] PR remote/20655 - small fix in handle_tracepoint_bkpts
2016-10-03 21:54 [RFA 0/3] Fix various bugs found by static analysis Tom Tromey
2016-10-03 21:54 ` [RFA 1/3] PR symtab/20652 - fix psymbol_compare Tom Tromey
@ 2016-10-03 21:54 ` Tom Tromey
2016-10-04 12:53 ` Yao Qi
2016-10-03 21:54 ` [RFA 2/3] PR gdb/20653 - small cleanup in string_to_explicit_location Tom Tromey
2016-10-04 15:14 ` [RFA 0/3] Fix various bugs found by static analysis Yao Qi
3 siblings, 1 reply; 14+ messages in thread
From: Tom Tromey @ 2016-10-03 21:54 UTC (permalink / raw)
To: gdb-patches; +Cc: Tom Tromey
handle_tracepoint_bkpts has two parallel "if"s. This changes the
second one to check ipa_error_tracepoint, which seems to be what was
intended.
2016-10-03 Tom Tromey <tom@tromey.com>
PR remote/20655:
* tracepoint.c (handle_tracepoint_bkpts): Check
ipa_error_tracepoint, not ipa_stopping_tracepoint.
---
gdb/gdbserver/ChangeLog | 6 ++++++
gdb/gdbserver/tracepoint.c | 2 +-
2 files changed, 7 insertions(+), 1 deletion(-)
diff --git a/gdb/gdbserver/ChangeLog b/gdb/gdbserver/ChangeLog
index 5c2cca9..afb0f78 100644
--- a/gdb/gdbserver/ChangeLog
+++ b/gdb/gdbserver/ChangeLog
@@ -1,3 +1,9 @@
+2016-10-03 Tom Tromey <tom@tromey.com>
+
+ PR remote/20655:
+ * tracepoint.c (handle_tracepoint_bkpts): Check
+ ipa_error_tracepoint, not ipa_stopping_tracepoint.
+
2016-09-30 Yao Qi <yao.qi@linaro.org>
PR gdbserver/20627
diff --git a/gdb/gdbserver/tracepoint.c b/gdb/gdbserver/tracepoint.c
index c07e525..7700ad1 100644
--- a/gdb/gdbserver/tracepoint.c
+++ b/gdb/gdbserver/tracepoint.c
@@ -4534,7 +4534,7 @@ handle_tracepoint_bkpts (struct thread_info *tinfo, CORE_ADDR stop_pc)
trace_debug ("lib stopped due to full buffer.");
if (ipa_stopping_tracepoint)
trace_debug ("lib stopped due to tpoint");
- if (ipa_stopping_tracepoint)
+ if (ipa_error_tracepoint)
trace_debug ("lib stopped due to error");
}
--
2.7.4
^ permalink raw reply [flat|nested] 14+ messages in thread
* [RFA 2/3] PR gdb/20653 - small cleanup in string_to_explicit_location
2016-10-03 21:54 [RFA 0/3] Fix various bugs found by static analysis Tom Tromey
2016-10-03 21:54 ` [RFA 1/3] PR symtab/20652 - fix psymbol_compare Tom Tromey
2016-10-03 21:54 ` [RFA 3/3] PR remote/20655 - small fix in handle_tracepoint_bkpts Tom Tromey
@ 2016-10-03 21:54 ` Tom Tromey
2016-10-03 22:27 ` Keith Seitz
2016-10-04 10:50 ` Yao Qi
2016-10-04 15:14 ` [RFA 0/3] Fix various bugs found by static analysis Yao Qi
3 siblings, 2 replies; 14+ messages in thread
From: Tom Tromey @ 2016-10-03 21:54 UTC (permalink / raw)
To: gdb-patches; +Cc: Tom Tromey
This bug points out that string_to_explicit_location compares a char*
against '\0'; whereas comparing against NULL is more normal.
2016-10-03 Tom Tromey <tom@tromey.com>
PR breakpoints/20653:
* location.c (string_to_explicit_location): Use NULL, not '\0'.
---
gdb/ChangeLog | 5 +++++
gdb/location.c | 2 +-
2 files changed, 6 insertions(+), 1 deletion(-)
diff --git a/gdb/ChangeLog b/gdb/ChangeLog
index 434f750..7a7ba1c 100644
--- a/gdb/ChangeLog
+++ b/gdb/ChangeLog
@@ -1,5 +1,10 @@
2016-10-03 Tom Tromey <tom@tromey.com>
+ PR breakpoints/20653:
+ * location.c (string_to_explicit_location): Use NULL, not '\0'.
+
+2016-10-03 Tom Tromey <tom@tromey.com>
+
PR symtab/20652:
* psymtab.c (psymbol_compare): Correctly compare "ginfo.value"
fields.
diff --git a/gdb/location.c b/gdb/location.c
index 65116c7..8dce21a 100644
--- a/gdb/location.c
+++ b/gdb/location.c
@@ -524,7 +524,7 @@ string_to_explicit_location (const char **argp,
character is an explicit location. "-p" is reserved, though,
for probe locations. */
if (argp == NULL
- || *argp == '\0'
+ || *argp == NULL
|| *argp[0] != '-'
|| !isalpha ((*argp)[1])
|| ((*argp)[0] == '-' && (*argp)[1] == 'p'))
--
2.7.4
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [RFA 2/3] PR gdb/20653 - small cleanup in string_to_explicit_location
2016-10-03 21:54 ` [RFA 2/3] PR gdb/20653 - small cleanup in string_to_explicit_location Tom Tromey
@ 2016-10-03 22:27 ` Keith Seitz
2016-10-04 10:50 ` Yao Qi
1 sibling, 0 replies; 14+ messages in thread
From: Keith Seitz @ 2016-10-03 22:27 UTC (permalink / raw)
To: Tom Tromey, gdb-patches
On 10/03/2016 02:54 PM, Tom Tromey wrote:
> This bug points out that string_to_explicit_location compares a char*
> against '\0'; whereas comparing against NULL is more normal.
>
> 2016-10-03 Tom Tromey <tom@tromey.com>
>
> PR breakpoints/20653:
> * location.c (string_to_explicit_location): Use NULL, not '\0'.
While I cannot approve changes to this code, having been the
under-caffeinated person who wrote it all, I'd have to say that your
patch is correct.
Thank you for taking care of this! [And hopefully a maintainer agrees
with us. :-)]
Keith
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [RFA 2/3] PR gdb/20653 - small cleanup in string_to_explicit_location
2016-10-03 21:54 ` [RFA 2/3] PR gdb/20653 - small cleanup in string_to_explicit_location Tom Tromey
2016-10-03 22:27 ` Keith Seitz
@ 2016-10-04 10:50 ` Yao Qi
1 sibling, 0 replies; 14+ messages in thread
From: Yao Qi @ 2016-10-04 10:50 UTC (permalink / raw)
To: Tom Tromey; +Cc: gdb-patches
On Mon, Oct 3, 2016 at 10:54 PM, Tom Tromey <tom@tromey.com> wrote:
> This bug points out that string_to_explicit_location compares a char*
> against '\0'; whereas comparing against NULL is more normal.
>
> 2016-10-03 Tom Tromey <tom@tromey.com>
>
> PR breakpoints/20653:
> * location.c (string_to_explicit_location): Use NULL, not '\0'.
Patch is good to me.
GCC trunk can capture it by a warning like this,
../../binutils-gdb/gdb/location.c: In function ‘event_location*
string_to_explicit_location(const char**, const language_defn*, int)’:
../../binutils-gdb/gdb/location.c:527:19: error: ISO C++ forbids
comparison between pointer and integer [-fpermissive]
|| *argp == '\0'
^~~~
make: *** [location.o] Error 1
--
Yao (齐尧)
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [RFA 0/3] Fix various bugs found by static analysis
2016-10-03 21:54 [RFA 0/3] Fix various bugs found by static analysis Tom Tromey
` (2 preceding siblings ...)
2016-10-03 21:54 ` [RFA 2/3] PR gdb/20653 - small cleanup in string_to_explicit_location Tom Tromey
@ 2016-10-04 15:14 ` Yao Qi
2016-10-04 18:31 ` Pedro Alves
3 siblings, 1 reply; 14+ messages in thread
From: Yao Qi @ 2016-10-04 15:14 UTC (permalink / raw)
To: Tom Tromey; +Cc: gdb-patches
On Mon, Oct 3, 2016 at 10:54 PM, Tom Tromey <tom@tromey.com> wrote:
>
> * PR 20654, incorrect code in java_value_print. Now that gcj has been
> removed, I think it's probably better to simply remove the Java
> language support. If this sounds ok, let me know, and I can provide
> a patch.
>
I am fine to remove java language support in GDB, but I'd like to hear
what other people think about this.
--
Yao (齐尧)
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [RFA 0/3] Fix various bugs found by static analysis
2016-10-04 15:14 ` [RFA 0/3] Fix various bugs found by static analysis Yao Qi
@ 2016-10-04 18:31 ` Pedro Alves
2016-10-04 19:05 ` Eli Zaretskii
0 siblings, 1 reply; 14+ messages in thread
From: Pedro Alves @ 2016-10-04 18:31 UTC (permalink / raw)
To: Yao Qi, Tom Tromey; +Cc: gdb-patches
On 10/04/2016 04:14 PM, Yao Qi wrote:
> On Mon, Oct 3, 2016 at 10:54 PM, Tom Tromey <tom@tromey.com> wrote:
>>
>> * PR 20654, incorrect code in java_value_print. Now that gcj has been
>> removed, I think it's probably better to simply remove the Java
>> language support. If this sounds ok, let me know, and I can provide
>> a patch.
>>
>
> I am fine to remove java language support in GDB, but I'd like to hear
> what other people think about this.
>
I'm fine with removing gcj support, but then again I never really
needed it personally.
Thanks,
Pedro Alves
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [RFA 0/3] Fix various bugs found by static analysis
2016-10-04 18:31 ` Pedro Alves
@ 2016-10-04 19:05 ` Eli Zaretskii
2016-10-04 20:43 ` Tom Tromey
0 siblings, 1 reply; 14+ messages in thread
From: Eli Zaretskii @ 2016-10-04 19:05 UTC (permalink / raw)
To: Pedro Alves; +Cc: qiyaoltc, tom, gdb-patches
> Cc: "gdb-patches@sourceware.org" <gdb-patches@sourceware.org>
> From: Pedro Alves <palves@redhat.com>
> Date: Tue, 4 Oct 2016 19:30:58 +0100
>
> On 10/04/2016 04:14 PM, Yao Qi wrote:
> > On Mon, Oct 3, 2016 at 10:54 PM, Tom Tromey <tom@tromey.com> wrote:
> >>
> >> * PR 20654, incorrect code in java_value_print. Now that gcj has been
> >> removed, I think it's probably better to simply remove the Java
> >> language support. If this sounds ok, let me know, and I can provide
> >> a patch.
> >>
> >
> > I am fine to remove java language support in GDB, but I'd like to hear
> > what other people think about this.
> >
>
> I'm fine with removing gcj support, but then again I never really
> needed it personally.
Shouldn't we wait for some time? I mean, gcj may have been removed,
but that doesn't mean all of its installations have been deleted, or
that no one out there uses the last release, and will use them for
some time.
How about just not making any significant maintenance efforts for it
from now on?
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [RFA 0/3] Fix various bugs found by static analysis
2016-10-04 19:05 ` Eli Zaretskii
@ 2016-10-04 20:43 ` Tom Tromey
0 siblings, 0 replies; 14+ messages in thread
From: Tom Tromey @ 2016-10-04 20:43 UTC (permalink / raw)
To: Eli Zaretskii; +Cc: Pedro Alves, qiyaoltc, tom, gdb-patches
>>>>> "Eli" == Eli Zaretskii <eliz@gnu.org> writes:
Eli> Shouldn't we wait for some time? I mean, gcj may have been removed,
Eli> but that doesn't mean all of its installations have been deleted, or
Eli> that no one out there uses the last release, and will use them for
Eli> some time.
Nobody has used it seriously in years. GCC dropping it is just the
final chapter, but really it died long ago. Fedora (not sure about
other distros) stopped shipping it a while ago and there's no critical
(or even useful) free software tool written using it. So, I think it's
safe to delete now. In the unlikely even that someone is using it, they
can debug with gdb 7.11.
Also, FWIW, it's not outrageously bad to debug gcj-compiled code as if
it were C++. You lose a little bit (String printing primarily) and you
have to use C++ names for things, but I think otherwise it works ok.
Tom
^ permalink raw reply [flat|nested] 14+ messages in thread