[rfc] Overflow in transfer-rate

[rfc] Overflow in transfer-rate

[Denis PILAT]

In symfile.c/print_transfer_performance (), an overflow can occure when data_count (see below) is a large number.

I propose either the following 1st patch, or to pass to "%llu"into the ui_out_field_fmt functino call, see the next patch proposal.

Index: symfile.c
===================================================================
--- symfile.c   (revision 215)
+++ symfile.c   (working copy)
@@ -1821,7 +1821,7 @@ print_transfer_performance (struct ui_fi
   if (time_count > 0)
     {
       ui_out_field_fmt (uiout, "transfer-rate", "%lu",
-                       1000 * (data_count * 8) / time_count);
+                       ((data_count / time_count) * 8 * 1000);
       ui_out_text (uiout, " bits/sec");
     }
   else




This 2nd option would fix all possible overflow, but I'm not sure yet the %llu is supported under windows.

Index: symfile.c
===================================================================
--- symfile.c   (revision 215)
+++ symfile.c   (working copy)
@@ -1820,13 +1820,13 @@ print_transfer_performance (struct ui_fi
  ui_out_text (uiout, "Transfer rate: ");
  if (time_count > 0)
    {
-      ui_out_field_fmt (uiout, "transfer-rate", "%lu",
-                       1000 * (data_count * 8) / time_count);
+      ui_out_field_fmt (uiout, "transfer-rate", "%llu",
+                       1000ULL * (data_count * 8) / time_count);
      ui_out_text (uiout, " bits/sec");
    }
  else
    {
-      ui_out_field_fmt (uiout, "transferred-bits", "%lu", (data_count * 8));
+      ui_out_field_fmt (uiout, "transferred-bits", "%llu", (data_count * 8ULL));
      ui_out_text (uiout, " bits in <1 sec");
    }
  if (write_count > 0)


Please give me your opinion.

-- 
Denis

Re: [rfc] Overflow in transfer-rate

[Andrew STUBBS]

Denis PILAT wrote:
> In symfile.c/print_transfer_performance (), an overflow can occure when 
> data_count (see below) is a large number.
> 
> I propose either the following 1st patch, or to pass to "%llu"into the 
> ui_out_field_fmt functino call, see the next patch proposal.

I might also be worth taking a look at this proposal from some time ago 
http://www.cygwin.com/ml/gdb-patches/2006-10/msg00184.html

Re: [rfc] Overflow in transfer-rate

[Denis PILAT]

Andrew STUBBS wrote:
> Denis PILAT wrote:
>> In symfile.c/print_transfer_performance (), an overflow can occure 
>> when data_count (see below) is a large number.
>>
>> I propose either the following 1st patch, or to pass to "%llu"into 
>> the ui_out_field_fmt functino call, see the next patch proposal.
>
> I might also be worth taking a look at this proposal from some time 
> ago http://www.cygwin.com/ml/gdb-patches/2006-10/msg00184.html
>
Well, it sounds like an internal ST discussion but I prefer the patch in 
your link, it avoids overflow *and* prints transfer rate in a much 
clever way.

Denis

Re: [rfc] Overflow in transfer-rate

[Daniel Jacobowitz]

On Mon, Jun 11, 2007 at 03:15:41PM +0200, Denis PILAT wrote:
> Andrew STUBBS wrote:
> > Denis PILAT wrote:
> >> In symfile.c/print_transfer_performance (), an overflow can occure when 
> >> data_count (see below) is a large number.
> >>
> >> I propose either the following 1st patch, or to pass to "%llu"into the 
> >> ui_out_field_fmt functino call, see the next patch proposal.
> >
> > I might also be worth taking a look at this proposal from some time ago 
> > http://www.cygwin.com/ml/gdb-patches/2006-10/msg00184.html
> >
> Well, it sounds like an internal ST discussion but I prefer the patch in your 
> link, it avoids overflow *and* prints transfer rate in a much clever way.

How about this version, then?  I left GDB/MI behavior unchanged, so
that this does not break GUIs.

-- 
Daniel Jacobowitz
CodeSourcery

2007-07-01  Ilko Iliev  <iliev@ronetix.at>
	    Daniel Jacobowitz  <dan@codesourcery.com>

	* symfile.c (print_transfer_performance): Avoid integer overflow.
	Use larger units.

Index: symfile.c
===================================================================
RCS file: /cvs/src/src/gdb/symfile.c,v
retrieving revision 1.187
diff -u -p -r1.187 symfile.c
--- symfile.c	18 Jun 2007 15:46:38 -0000	1.187
+++ symfile.c	1 Jul 2007 22:08:46 -0000
@@ -1947,7 +1947,7 @@ print_transfer_performance (struct ui_fi
 			    const struct timeval *start_time,
 			    const struct timeval *end_time)
 {
-  unsigned long time_count;
+  ULONGEST time_count;
 
   /* Compute the elapsed time in milliseconds, as a tradeoff between
      accuracy and overflow.  */
@@ -1957,9 +1957,23 @@ print_transfer_performance (struct ui_fi
   ui_out_text (uiout, "Transfer rate: ");
   if (time_count > 0)
     {
-      ui_out_field_fmt (uiout, "transfer-rate", "%lu",
-			1000 * (data_count * 8) / time_count);
-      ui_out_text (uiout, " bits/sec");
+      unsigned long rate = ((ULONGEST) data_count * 1000) / time_count;
+
+      if (ui_out_is_mi_like_p (uiout))
+	{
+	  ui_out_field_fmt (uiout, "transfer-rate", "%lu", rate * 8);
+	  ui_out_text (uiout, " bits/sec");
+	}
+      else if (rate < 1024)
+	{
+	  ui_out_field_fmt (uiout, "transfer-rate", "%lu", rate);
+	  ui_out_text (uiout, " bytes/sec");
+	}
+      else
+	{
+	  ui_out_field_fmt (uiout, "transfer-rate", "%lu", rate / 1024);
+	  ui_out_text (uiout, " KB/sec");
+	}
     }
   else
     {

Re: [rfc] Overflow in transfer-rate

[Denis PILAT]

Daniel Jacobowitz wrote:
> On Mon, Jun 11, 2007 at 03:15:41PM +0200, Denis PILAT wrote:
>   
>> Andrew STUBBS wrote:
>>     
>>> Denis PILAT wrote:
>>>       
>>>> In symfile.c/print_transfer_performance (), an overflow can occure when 
>>>> data_count (see below) is a large number.
>>>>
>>>> I propose either the following 1st patch, or to pass to "%llu"into the 
>>>> ui_out_field_fmt functino call, see the next patch proposal.
>>>>         
>>> I might also be worth taking a look at this proposal from some time ago 
>>> http://www.cygwin.com/ml/gdb-patches/2006-10/msg00184.html
>>>
>>>       
>> Well, it sounds like an internal ST discussion but I prefer the patch in your 
>> link, it avoids overflow *and* prints transfer rate in a much clever way.
>>     
>
> How about this version, then?  I left GDB/MI behavior unchanged, so
> that this does not break GUIs.
>
>   
Hi Daniel,

You're right, we must not break MI based GUI, I'm fine with your proposal.
I guess it will be include in 6.8 , not 6.7 GDB version ?

Thanks,
Denis

Re: [rfc] Overflow in transfer-rate

[Daniel Jacobowitz]

On Mon, Jul 02, 2007 at 09:35:38AM +0200, Denis PILAT wrote:
> You're right, we must not break MI based GUI, I'm fine with your proposal.
> I guess it will be include in 6.8 , not 6.7 GDB version ?

I've committed it.  I don't know when Joel is planning to date the
branch, but I think it will be in 6.7.

-- 
Daniel Jacobowitz
CodeSourcery

Re: [rfc] Overflow in transfer-rate

[Joel Brobecker]

> I've committed it.  I don't know when Joel is planning to date the
> branch, but I think it will be in 6.7.

I am about to start working on the branch. I'm trying to catch up
a bit on the last patches that went in to see where it would be most
beneficial to branch. I have updated the Wiki to make sure that this
patch makes it to the branch.

-- 
Joel

Re: [rfc] Overflow in transfer-rate

[Mark Kettenis]

> Date: Wed, 4 Jul 2007 10:21:11 -0700
> From: Joel Brobecker <brobecker@adacore.com>
> 
> > I've committed it.  I don't know when Joel is planning to date the
> > branch, but I think it will be in 6.7.
> 
> I am about to start working on the branch. I'm trying to catch up
> a bit on the last patches that went in to see where it would be most
> beneficial to branch. I have updated the Wiki to make sure that this
> patch makes it to the branch.

Just a heads-up, but I found out yesterday, that this commit:

2007-07-01  Daniel Jacobowitz  <dan@codesourcery.com>

        PR symtab/2161
        * target.c (memory_xfer_partial): Do not continue past targets with
        all memory.

broke debugging threaded programs on OpenBSD.  I think I know why, but
I haven't actually fixed it yet.  I'd like to make sure this is fixed
before we make the release (but I can fix it on the branch if
necessary).

There also is a recently introduced problem with building opcodes
using BSD make.  I have a solution for it, but not mailed out the diff
yet.

Mark

Re: [rfc] Overflow in transfer-rate

[Daniel Jacobowitz]

On Wed, Jul 04, 2007 at 08:00:19PM +0200, Mark Kettenis wrote:
> Just a heads-up, but I found out yesterday, that this commit:
> 
> 2007-07-01  Daniel Jacobowitz  <dan@codesourcery.com>
> 
>         PR symtab/2161
>         * target.c (memory_xfer_partial): Do not continue past targets with
>         all memory.
> 
> broke debugging threaded programs on OpenBSD.  I think I know why, but
> I haven't actually fixed it yet.  I'd like to make sure this is fixed
> before we make the release (but I can fix it on the branch if
> necessary).

Eek!  I'm very sorry about that.  I can not see how, but please let me
know if you would like my help figuring it out.

-- 
Daniel Jacobowitz
CodeSourcery

Re: [rfc] Overflow in transfer-rate

[Joel Brobecker]

> Just a heads-up, but I found out yesterday, that this commit:
> 
> 2007-07-01  Daniel Jacobowitz  <dan@codesourcery.com>
> 
>         PR symtab/2161
>         * target.c (memory_xfer_partial): Do not continue past targets with
>         all memory.
> 
> broke debugging threaded programs on OpenBSD.

Thanks for the heads-up, Mark. I've added this to the 6.7 wiki page,
so I'll make sure we wait for a fix before we make a first pre-release.

-- 
Joel