Re: [RFA] tracepoint remote.c:remote_trace_set_readonly_regions give up some regions if it is number is too big

[Pedro Alves <pedro@codesourcery.com>]

On Sunday 12 June 2011 15:01:07, Hui Zhu wrote:
> Hi,
> 
> My GDB got crash with a ELF file when I use tracepoint with it.  I
> found that it have 12898 sections.
> So in remote_trace_set_readonly_regions:
>       sprintf (target_buf + strlen (target_buf),
> 	       ":%s,%s", tmp1, tmp2);
> It will over write other val, it make GDB crash.
> So I add a check before it to fix it:
>       if (strlen (target_buf) + strlen(tmp1) + strlen(tmp2) + 3 >
> target_buf_size)
> 	{
> 	  warning (_("Give up some read only regions."));
> 	  break;
> 	}
> 

Note that if your stub supports qXfer:traceframe-info:read, this packet
is no longer necessary to support, as GDB will handle reading from
readonly sections out of live memory itself.  I think this
means that the warning should only be output
if remote_protocol_packets[PACKET_qXfer_traceframe_info].support
is not PACKET_ENABLE?

> Please help me review it.
> 
> And this issue affect 7.3 too. Does it can check in to 7.3?
> 
> Thanks,
> Hui
> 
> 2011-06-12  Hui Zhu  <teawater@gmail.com>
> 
> 	* remote.c (remote_trace_set_readonly_regions): Add a check for
> 	target_buf_size.
> ---
>  remote.c |    5 +++++
>  1 file changed, 5 insertions(+)
> 
> --- a/remote.c
> +++ b/remote.c
> @@ -9996,6 +9996,11 @@ remote_trace_set_readonly_regions (void)
>        size = bfd_get_section_size (s);
>        sprintf_vma (tmp1, vma);
>        sprintf_vma (tmp2, vma + size);
     if (strlen (target_buf) + strlen(tmp1) + strlen(tmp2) + 3 > target_buf_size)

Space before `('.  Too long line, split the length calc
into a local out of the if predicate.

Should be simple to keep the current length of target_buf in
a variable and increment it on each iteration so you don't
need to compute it everytime.

        size = bfd_get_section_size (s);
        sprintf_vma (tmp1, vma);
        sprintf_vma (tmp2, vma + size);
		 sec_length = 1 + strlen (tmp1) + 1 + strlen (tmp2);
        if (offset + sec_length >= target_buf_size)
          {
            warning();
            break;
          }

        sprintf (target_buf + offset, ":%s,%s", tmp1, tmp2);
        offset += sec_length;

> +	{
> +	  warning (_("Give up some read only regions."));

Make that:

"Too many sections for read-only sections definition packet".

> +	  break;
> +	}
>        sprintf (target_buf + strlen (target_buf),
>  	       ":%s,%s", tmp1, tmp2);
>      }
> 

-- 
Pedro Alves