diff mbox

[RFA,1/3] PR symtab/20652 - fix psymbol_compare

Message ID 1475531646-18049-2-git-send-email-tom@tromey.com
State New, archived
Headers

Commit Message

Tom Tromey Oct. 3, 2016, 9:54 p.m. UTC 
  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(-)

Comments

Yao Qi Oct. 4, 2016, 10:54 a.m. UTC | #1 
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.
Pedro Alves Oct. 4, 2016, 6:37 p.m. UTC | #2 
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
Tom Tromey Oct. 4, 2016, 6:48 p.m. UTC | #3 
>>>>> "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
diff mbox

Patch

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)