[PATCH c++ 0/6] A few C++ patches

[PATCH c++ 0/6] A few C++ patches

[Simon Marchi]

A few one liners.

Simon Marchi (6):
  dwarf2read.c: Add cast
  mdebugread.c: Add cast
  Change type of write_qxfer_response parameter
  gdbserver/server.c: Cast return value of memmem
  Cast gdb_dlsym return value
  Add a cast in jit_target_read_impl

 gdb/dwarf2read.c       | 2 +-
 gdb/gdbserver/server.c | 5 +++--
 gdb/jit.c              | 5 +++--
 gdb/mdebugread.c       | 2 +-
 4 files changed, 8 insertions(+), 6 deletions(-)

-- 
2.6.1

[PATCH c++ 1/6] dwarf2read.c: Add cast

[Simon Marchi]

There is no enum value representing 0.  It seems like the value of the
name field is irrelevant here.

gdb/ChangeLog:

	* dwarf2read.c (partial_die_full_name): Add cast.
---
 gdb/dwarf2read.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/gdb/dwarf2read.c b/gdb/dwarf2read.c
index d43c42e..baa288c 100644
--- a/gdb/dwarf2read.c
+++ b/gdb/dwarf2read.c
@@ -6854,7 +6854,7 @@ partial_die_full_name (struct partial_die_info *pdi,
 	  struct dwarf2_cu *ref_cu = cu;
 
 	  /* DW_FORM_ref_addr is using section offset.  */
-	  attr.name = 0;
+	  attr.name = (dwarf_attribute) 0;
 	  attr.form = DW_FORM_ref_addr;
 	  attr.u.unsnd = pdi->offset.sect_off;
 	  die = follow_die_ref (NULL, &attr, &ref_cu);
-- 
2.6.1

Re: [PATCH c++ 1/6] dwarf2read.c: Add cast

[Simon Marchi]

On 29 October 2015 at 09:02, Simon Marchi <simon.marchi@polymtl.ca> wrote:
> There is no enum value representing 0.  It seems like the value of the
> name field is irrelevant here.
>
> gdb/ChangeLog:
>
>         * dwarf2read.c (partial_die_full_name): Add cast.
> ---
>  gdb/dwarf2read.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/gdb/dwarf2read.c b/gdb/dwarf2read.c
> index d43c42e..baa288c 100644
> --- a/gdb/dwarf2read.c
> +++ b/gdb/dwarf2read.c
> @@ -6854,7 +6854,7 @@ partial_die_full_name (struct partial_die_info *pdi,
>           struct dwarf2_cu *ref_cu = cu;
>
>           /* DW_FORM_ref_addr is using section offset.  */
> -         attr.name = 0;
> +         attr.name = (dwarf_attribute) 0;

Oops, here it should say (enum dwarf_attribute), so it works in C too.

>           attr.form = DW_FORM_ref_addr;
>           attr.u.unsnd = pdi->offset.sect_off;
>           die = follow_die_ref (NULL, &attr, &ref_cu);
> --
> 2.6.1
>

Re: [PATCH c++ 1/6] dwarf2read.c: Add cast

[Pedro Alves]

On 10/29/2015 02:57 PM, Simon Marchi wrote:

>> --- a/gdb/dwarf2read.c
>> +++ b/gdb/dwarf2read.c
>> @@ -6854,7 +6854,7 @@ partial_die_full_name (struct partial_die_info *pdi,
>>           struct dwarf2_cu *ref_cu = cu;
>>
>>           /* DW_FORM_ref_addr is using section offset.  */
>> -         attr.name = 0;
>> +         attr.name = (dwarf_attribute) 0;
> 
> Oops, here it should say (enum dwarf_attribute), so it works in C too.

OK with that change.

Thanks,
Pedro Alves

Re: [PATCH c++ 1/6] dwarf2read.c: Add cast

[Simon Marchi]

On 29 October 2015 at 13:14, Pedro Alves <palves@redhat.com> wrote:
> On 10/29/2015 02:57 PM, Simon Marchi wrote:
>
>>> --- a/gdb/dwarf2read.c
>>> +++ b/gdb/dwarf2read.c
>>> @@ -6854,7 +6854,7 @@ partial_die_full_name (struct partial_die_info *pdi,
>>>           struct dwarf2_cu *ref_cu = cu;
>>>
>>>           /* DW_FORM_ref_addr is using section offset.  */
>>> -         attr.name = 0;
>>> +         attr.name = (dwarf_attribute) 0;
>>
>> Oops, here it should say (enum dwarf_attribute), so it works in C too.
>
> OK with that change.
>
> Thanks,
> Pedro Alves

Pushed everything except patch 2, thanks!

[PATCH c++ 4/6] gdbserver/server.c: Cast return value of memmem

[Simon Marchi]

gdb/gdbserver/ChangeLog:

	* server.c (handle_search_memory_1): Cast return value of
	memmem.
---
 gdb/gdbserver/server.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/gdb/gdbserver/server.c b/gdb/gdbserver/server.c
index 1312760..f258e90 100644
--- a/gdb/gdbserver/server.c
+++ b/gdb/gdbserver/server.c
@@ -891,7 +891,8 @@ handle_search_memory_1 (CORE_ADDR start_addr, CORE_ADDR search_space_len,
 				  ? search_space_len
 				  : search_buf_size);
 
-      found_ptr = memmem (search_buf, nr_search_bytes, pattern, pattern_len);
+      found_ptr = (gdb_byte *) memmem (search_buf, nr_search_bytes, pattern,
+				       pattern_len);
 
       if (found_ptr != NULL)
 	{
-- 
2.6.1

Re: [PATCH c++ 4/6] gdbserver/server.c: Cast return value of memmem

[Pedro Alves]

On 10/29/2015 01:02 PM, Simon Marchi wrote:
> gdb/gdbserver/ChangeLog:
> 
> 	* server.c (handle_search_memory_1): Cast return value of
> 	memmem.

OK.

Thanks,
Pedro Alves

[PATCH c++ 2/6] mdebugread.c: Add cast

[Simon Marchi]

I spent some time looking at the code trying to understand what happens.
I'm still not totally sure, but if it's been there all that time it must
be ok.  If anybody wants to take a deeper look, please do.

gdb/ChangeLog:

	* mdebugread.c (parse_symbol): Add cast.
---
 gdb/mdebugread.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/gdb/mdebugread.c b/gdb/mdebugread.c
index 03c1ff8..af55823 100644
--- a/gdb/mdebugread.c
+++ b/gdb/mdebugread.c
@@ -651,7 +651,7 @@ parse_symbol (SYMR *sh, union aux_ext *ax, char *ext_sh, int bigend,
       s = new_symbol (name);
       SYMBOL_VALUE (s) = svalue;
       if (sh->sc == scRegister)
-	theclass = mdebug_register_index;
+	theclass = (enum address_class) mdebug_register_index;
       else
 	theclass = LOC_LOCAL;
 
-- 
2.6.1

Re: [PATCH c++ 2/6] mdebugread.c: Add cast

[Pedro Alves]

On 10/29/2015 01:02 PM, Simon Marchi wrote:
> I spent some time looking at the code trying to understand what happens.
> I'm still not totally sure, but if it's been there all that time it must
> be ok.  If anybody wants to take a deeper look, please do.

Ah, a challenge!  Must ... resist ...

How about this instead?

From: Pedro Alves <palves@redhat.com>
Subject: [PATCH] mdebugread.c: Address class -> address class index

This fixes this error in C++ mode:

 /home/pedro/gdb/mygit/cxx-convertion/src/gdb/mdebugread.c:654:11: error: invalid conversion from ‘int’ to ‘address_class’ [-fpermissive]
   theclass = mdebug_register_index;
	    ^

The "theclass" local is of type enum address_class, however, what it
really holds is an address class index.  Class index values by design
match the address class values up until LOC_FINAL_VALUE, but extend
beyond that, so it's not really right to store an address class index
in an enum address_class.

The fix is really the same as making the 'theclass' local be of type int,
but while we're at it, we get rid of the goto, and thus the local
becomes the 'aclass_index' parameter in the new add_data_symbol
function.

gdb/ChangeLog:
2015-10-29  Pedro Alves  <palves@redhat.com>

	* mdebugread.c (add_data_symbol): New function, factored out from
	...
	(parse_symbol): ... here.  Delete 'theclass' local.
---
 gdb/mdebugread.c | 48 ++++++++++++++++++++++++++++--------------------
 1 file changed, 28 insertions(+), 20 deletions(-)

diff --git a/gdb/mdebugread.c b/gdb/mdebugread.c
index 03c1ff8..da5ec60 100644
--- a/gdb/mdebugread.c
+++ b/gdb/mdebugread.c
@@ -567,6 +567,26 @@ static const struct symbol_register_ops mdebug_register_funcs = {
 static int mdebug_register_index;
 static int mdebug_regparm_index;
 
+/* Common code for symbols describing data.  */
+
+static int
+add_data_symbol (SYMR *sh, union aux_ext *ax, int bigend,
+		 struct symbol *s, int aclass_index, struct block *b,
+		 struct objfile *objfile, char *name)
+{
+  SYMBOL_DOMAIN (s) = VAR_DOMAIN;
+  SYMBOL_ACLASS_INDEX (s) = aclass_index;
+  add_symbol (s, top_stack->cur_st, b);
+
+  /* Type could be missing if file is compiled without debugging info.  */
+  if (SC_IS_UNDEF (sh->sc)
+      || sh->sc == scNil || sh->index == indexNil)
+    SYMBOL_TYPE (s) = objfile_type (objfile)->nodebug_data_symbol;
+  else
+    SYMBOL_TYPE (s) = parse_type (cur_fd, ax, sh->index, 0, bigend, name);
+  /* Value of a data symbol is its memory address.  */
+}
+
 static int
 parse_symbol (SYMR *sh, union aux_ext *ax, char *ext_sh, int bigend,
 	      struct section_offsets *section_offsets, struct objfile *objfile)
@@ -581,7 +601,6 @@ parse_symbol (SYMR *sh, union aux_ext *ax, char *ext_sh, int bigend,
   struct type *t;
   struct field *f;
   int count = 1;
-  enum address_class theclass;
   TIR tir;
   long svalue = sh->value;
   int bitsize;
@@ -622,15 +641,14 @@ parse_symbol (SYMR *sh, union aux_ext *ax, char *ext_sh, int bigend,
       break;
 
     case stGlobal:		/* External symbol, goes into global block.  */
-      theclass = LOC_STATIC;
       b = BLOCKVECTOR_BLOCK (SYMTAB_BLOCKVECTOR (top_stack->cur_st),
 			     GLOBAL_BLOCK);
       s = new_symbol (name);
       SYMBOL_VALUE_ADDRESS (s) = (CORE_ADDR) sh->value;
-      goto data;
+      add_data_symbol (sh, ax, bigend, s, LOC_STATIC, b, objfile, name);
+      break;
 
     case stStatic:		/* Static data, goes into current block.  */
-      theclass = LOC_STATIC;
       b = top_stack->cur_block;
       s = new_symbol (name);
       if (SC_IS_COMMON (sh->sc))
@@ -644,29 +662,19 @@ parse_symbol (SYMR *sh, union aux_ext *ax, char *ext_sh, int bigend,
 	}
       else
 	SYMBOL_VALUE_ADDRESS (s) = (CORE_ADDR) sh->value;
-      goto data;
+      add_data_symbol (sh, ax, bigend, s, LOC_STATIC, b, objfile, name);
+      break;
 
     case stLocal:		/* Local variable, goes into current block.  */
       b = top_stack->cur_block;
       s = new_symbol (name);
       SYMBOL_VALUE (s) = svalue;
       if (sh->sc == scRegister)
-	theclass = mdebug_register_index;
-      else
-	theclass = LOC_LOCAL;
-
-    data:			/* Common code for symbols describing data.  */
-      SYMBOL_DOMAIN (s) = VAR_DOMAIN;
-      SYMBOL_ACLASS_INDEX (s) = theclass;
-      add_symbol (s, top_stack->cur_st, b);
-
-      /* Type could be missing if file is compiled without debugging info.  */
-      if (SC_IS_UNDEF (sh->sc)
-	  || sh->sc == scNil || sh->index == indexNil)
-	SYMBOL_TYPE (s) = objfile_type (objfile)->nodebug_data_symbol;
+	add_data_symbol (sh, ax, bigend, s, mdebug_register_index,
+			 b, objfile, name);
       else
-	SYMBOL_TYPE (s) = parse_type (cur_fd, ax, sh->index, 0, bigend, name);
-      /* Value of a data symbol is its memory address.  */
+	add_data_symbol (sh, ax, bigend, s, LOC_LOCAL,
+			 b, objfile, name);
       break;
 
     case stParam:		/* Arg to procedure, goes into current
-- 
1.9.3

[PATCH c++ 5/6] Cast gdb_dlsym return value

[Simon Marchi]

gdb/ChangeLog:

	* jit.c (jit_reader_load): Add cast.
---
 gdb/jit.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/gdb/jit.c b/gdb/jit.c
index 7c267d7..ddc223d 100644
--- a/gdb/jit.c
+++ b/gdb/jit.c
@@ -177,7 +177,7 @@ jit_reader_load (const char *file_name)
   so = gdb_dlopen (file_name);
   old_cleanups = make_cleanup_dlclose (so);
 
-  init_fn = gdb_dlsym (so, reader_init_fn_sym);
+  init_fn = (reader_init_fn_type *) gdb_dlsym (so, reader_init_fn_sym);
   if (!init_fn)
     error (_("Could not locate initialization function: %s."),
           reader_init_fn_sym);
-- 
2.6.1

Re: [PATCH c++ 5/6] Cast gdb_dlsym return value

[Pedro Alves]

On 10/29/2015 01:02 PM, Simon Marchi wrote:
> gdb/ChangeLog:
> 
> 	* jit.c (jit_reader_load): Add cast.

OK.

Thanks,
Pedro Alves

[PATCH c++ 3/6] Change type of write_qxfer_response parameter

[Simon Marchi]

Fixes:

/home/simark/src/binutils-gdb/gdb/gdbserver/server.c: In function ‘int write_qxfer_response(char*, const void*, int, int)’:
/home/simark/src/binutils-gdb/gdb/gdbserver/server.c:398:32: error: invalid conversion from ‘const void*’ to ‘const gdb_byte* {aka const unsigned char*}’ [-fpermissive]
           &out_len, PBUFSIZ - 2) + 1;
                                ^
gdb/gdbserver/ChangeLog:

	* server.c (write_qxfer_response): Change type of data to
	gdb_byte *.
---
 gdb/gdbserver/server.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/gdb/gdbserver/server.c b/gdb/gdbserver/server.c
index ad6626e..1312760 100644
--- a/gdb/gdbserver/server.c
+++ b/gdb/gdbserver/server.c
@@ -385,7 +385,7 @@ decode_xfer (char *buf, char **object, char **rw, char **annex, char **offset)
    to as much of DATA/LEN as we could fit.  IS_MORE controls
    the first character of the response.  */
 static int
-write_qxfer_response (char *buf, const void *data, int len, int is_more)
+write_qxfer_response (char *buf, const gdb_byte *data, int len, int is_more)
 {
   int out_len;
 
-- 
2.6.1

Re: [PATCH c++ 3/6] Change type of write_qxfer_response parameter

[Pedro Alves]

On 10/29/2015 01:02 PM, Simon Marchi wrote:
> Fixes:
> 
> /home/simark/src/binutils-gdb/gdb/gdbserver/server.c: In function ‘int write_qxfer_response(char*, const void*, int, int)’:
> /home/simark/src/binutils-gdb/gdb/gdbserver/server.c:398:32: error: invalid conversion from ‘const void*’ to ‘const gdb_byte* {aka const unsigned char*}’ [-fpermissive]
>            &out_len, PBUFSIZ - 2) + 1;
>                                 ^
> gdb/gdbserver/ChangeLog:
> 
> 	* server.c (write_qxfer_response): Change type of data to
> 	gdb_byte *.

OK.

Thanks,
Pedro Alves

[PATCH c++ 6/6] Add a cast in jit_target_read_impl

[Simon Marchi]

We could change the signature of the function.  However, it would
require changing gdb_target_read in jit-reader.h, which is an exported
interface.  It's probably better to just add a cast in our code than to
break other people's code.

gdb/ChangeLog:

	* jit.c (jit_target_read_impl): Add cast.
---
 gdb/jit.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/gdb/jit.c b/gdb/jit.c
index ddc223d..7c6aede 100644
--- a/gdb/jit.c
+++ b/gdb/jit.c
@@ -491,7 +491,8 @@ typedef CORE_ADDR jit_dbg_reader_data;
 static enum gdb_status
 jit_target_read_impl (GDB_CORE_ADDR target_mem, void *gdb_buf, int len)
 {
-  int result = target_read_memory ((CORE_ADDR) target_mem, gdb_buf, len);
+  int result = target_read_memory ((CORE_ADDR) target_mem,
+				   (gdb_byte *) gdb_buf, len);
   if (result == 0)
     return GDB_SUCCESS;
   else
-- 
2.6.1

Re: [PATCH c++ 6/6] Add a cast in jit_target_read_impl

[Pedro Alves]

On 10/29/2015 01:02 PM, Simon Marchi wrote:
> We could change the signature of the function.  However, it would
> require changing gdb_target_read in jit-reader.h, which is an exported
> interface.  It's probably better to just add a cast in our code than to
> break other people's code.

Agreed.

> 
> gdb/ChangeLog:
> 
> 	* jit.c (jit_target_read_impl): Add cast.

OK.

Thanks,
Pedro Alves