public inbox for gcc@gcc.gnu.org
 help / color / mirror / Atom feed
* mudflap problem
@ 2004-09-28 21:38 Herman ten Brugge
  2004-09-29  1:57 ` Frank Ch. Eigler
  0 siblings, 1 reply; 21+ messages in thread
From: Herman ten Brugge @ 2004-09-28 21:38 UTC (permalink / raw)
  To: gcc

Hello,

I tried mudflap on some test programs and found that the following 
program works:

int a[1024];
int b[1024];

int
main(void)
{
        int i;

        for (i = 0 ; i < 2048 ; i++) {
                b[i] = 0;
        }
        return 0;
}

When compiled with -fmudflap the code runs with no error. In the memory map
the array b is before array a. So the code above first fills array b 
with 0 and then a.
I assumed mudflap should warn me about buffer overruns. It does not in 
this case.

If I replace 'b[i] = 0' with 'a[i] = 0' I get the expected errors.

    Herman.

^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: mudflap problem
  2004-09-28 21:38 mudflap problem Herman ten Brugge
@ 2004-09-29  1:57 ` Frank Ch. Eigler
  2004-09-29  4:59   ` Frank Ch. Eigler
  2004-09-29 23:01   ` Doug Graham
  0 siblings, 2 replies; 21+ messages in thread
From: Frank Ch. Eigler @ 2004-09-29  1:57 UTC (permalink / raw)
  To: Herman ten Brugge; +Cc: gcc


Herman ten Brugge <hermantenbrugge@home.nl> writes:

> I tried mudflap on some test programs and found that the following
> program works:
> [...]
> When compiled with -fmudflap the code runs with no error. In the memory map
> the array b is before array a. So the code above first fills array b
> with 0 and then a.

This should not be happening, unless some intermediate compiler pass
is lowering the ARRAY_REF (b[i]) to a plain INDIRECT_REF (* b+i) type
expression.  (If so, the instrumentation loses its limited awareness
of where the pointer value came from, and checks only that it refers
to *some* valid object.)  This sort of lowering transform should
likely be disabled in the -fmudflap case.  (It's possible that there
is a bug elsewhere instead.)

- FChE

^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: mudflap problem
  2004-09-29  1:57 ` Frank Ch. Eigler
@ 2004-09-29  4:59   ` Frank Ch. Eigler
  2004-09-29 23:01   ` Doug Graham
  1 sibling, 0 replies; 21+ messages in thread
From: Frank Ch. Eigler @ 2004-09-29  4:59 UTC (permalink / raw)
  To: Herman ten Brugge; +Cc: gcc


I wrote:

> This should not be happening, unless some intermediate compiler pass
> is lowering the ARRAY_REF (b[i]) to a plain INDIRECT_REF (* b+i)
> type expression.  [...]

It turns out that this lowering is occurring in the current
tree-mudflap.c code itself.  (See how ARRAY_REF is handled in
mf_build_check_statement_for().)  It used not to do that - I'm
investigating when/why this function became broken.

- FChE

^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: mudflap problem
  2004-09-29  1:57 ` Frank Ch. Eigler
  2004-09-29  4:59   ` Frank Ch. Eigler
@ 2004-09-29 23:01   ` Doug Graham
  2004-09-29 23:14     ` Frank Ch. Eigler
  1 sibling, 1 reply; 21+ messages in thread
From: Doug Graham @ 2004-09-29 23:01 UTC (permalink / raw)
  To: gcc

On Tue, Sep 28, 2004 at 05:07:53PM -0400, Frank Ch. Eigler wrote:
> 
> Herman ten Brugge <hermantenbrugge@home.nl> writes:
> 
> > I tried mudflap on some test programs and found that the following
> > program works:
> > [...]
> > When compiled with -fmudflap the code runs with no error. In the memory map
> > the array b is before array a. So the code above first fills array b
> > with 0 and then a.
> 
> This should not be happening, unless some intermediate compiler pass
> is lowering the ARRAY_REF (b[i]) to a plain INDIRECT_REF (* b+i) type
> expression.  (If so, the instrumentation loses its limited awareness
> of where the pointer value came from, and checks only that it refers
> to *some* valid object.)  This sort of lowering transform should
> likely be disabled in the -fmudflap case.  (It's possible that there
> is a bug elsewhere instead.)

My apologies if this is a question that would better be asked elsewhere.

Herman's question and your response got me to wondering what sort of
things the bounds-checking patches that Herman currently maintains would
catch that mudflap would not.  I know that all pointer arithmetic is
checked in his patches, and I'm fairly sure that mudflap doesn't check any
pointer arith, so I thought that there would be plenty of cases similar
(but not quite the same) to the one that he is asking about that would not
be caught by mudflap.  But then I tried this:

    int a[16];
    int b[16];
    
    static void foo(int *x)
    {
            *x = 42;
    }
    
    int main()
    {
            foo(b+16);
    }

I thought that since b+16 points to the base of a, mudflap would not
complain about the dereference in foo().  But it did somehow know that
the pointer passed in to foo() was derived from 'b' rather than 'a'.
OTOH, if I call foo(a) instead, which means that I'm passing exactly
the same pointer value to foo(), mudflap does not complain.

How did mudflap know in foo(), without using fat pointers, that the
pointer was derived from 'b'?  Herman's bounds-checking patches can catch
this as well, but they do so by adding some pad bytes between 'a' and 'b',
so that b+16 does not point to anything that can be legally dereferenced.

But then I changed main so that I could test both cases from the same
executable:

    int main(int argc, char **argv)
    {
        if (argc > 1)
                foo(b+16);
        else
                foo(a);
    }

and mudflap didn't catch the b+16 dereference???

My original intent was to suggest that maybe mudflap could (perhaps
optionally) check pointer arithmetic as well, but now I'm not sure
what advantages that would offer.

BTW, I'm using the 20040627 snapshot of GCC 3.5 (which doesn't exhibit
the problem that Herman mentioned).

--Doug.

^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: mudflap problem
  2004-09-29 23:01   ` Doug Graham
@ 2004-09-29 23:14     ` Frank Ch. Eigler
  2004-09-29 23:35       ` Doug Graham
  0 siblings, 1 reply; 21+ messages in thread
From: Frank Ch. Eigler @ 2004-09-29 23:14 UTC (permalink / raw)
  To: Doug Graham; +Cc: gcc


"Doug Graham" <dgraham@nortelnetworks.com> writes:

> [...]  I'm fairly sure that mudflap doesn't check any pointer arith

That's correct - mudflap does not check pointer manipulation, except
the final dereference.

> so I thought that there would be plenty of cases similar
> (but not quite the same) to the one that he is asking about that would not
> be caught by mudflap.  But then I tried this:
> [...]
> I thought that since b+16 points to the base of a, mudflap would not
> complain about the dereference in foo().  But it did somehow know that
> the pointer passed in to foo() was derived from 'b' rather than 'a'.

I believe this is an optimization artifact.  With "-O2", the declaration of
"a" is marked somehow as unused (it is), and libmudflap does not get
a startup-time registration call for it.  Thus b[16] is checked and is found
to refer to an invalid address.

You may find running programs with
  env MUDFLAP_OPTIONS="-trace-calls" <your-program>
will clarify the actual behavior of the code.

> How did mudflap know in foo(), without using fat pointers, that the
> pointer was derived from 'b'?  

It didn't.

> Herman's bounds-checking patches can catch this as well, but they do
> so by adding some pad bytes between 'a' and 'b', so that b+16 does
> not point to anything that can be legally dereferenced.

Adding some padding between static objects would help mudflap catch
such off-by-one (but not off-by-many) accesses in the same way.


> [...]  My original intent was to suggest that maybe mudflap could
> (perhaps optionally) check pointer arithmetic as well [...]

It might help catch more problems earlier and with more specificity,
but this would require a lot more instrumentation code to be
run, and thus slow down execution even more.


- FChE

^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: mudflap problem
  2004-09-29 23:14     ` Frank Ch. Eigler
@ 2004-09-29 23:35       ` Doug Graham
  2004-10-06 10:15         ` Doug Graham
  0 siblings, 1 reply; 21+ messages in thread
From: Doug Graham @ 2004-09-29 23:35 UTC (permalink / raw)
  To: gcc

On Wed, Sep 29, 2004 at 06:05:56PM -0400, Frank Ch. Eigler wrote:
> 
> > I thought that since b+16 points to the base of a, mudflap would not
> > complain about the dereference in foo().  But it did somehow know that
> > the pointer passed in to foo() was derived from 'b' rather than 'a'.
> 
> I believe this is an optimization artifact.  With "-O2", the declaration of
> "a" is marked somehow as unused (it is), and libmudflap does not get
> a startup-time registration call for it.  Thus b[16] is checked and is found
> to refer to an invalid address.

Aha!  You're right, only 'b' is passed to __mf_register.  This happened
without -O2; my compile line was just: gcc -fmudflap -o foo foo.c.

> Adding some padding between static objects would help mudflap catch
> such off-by-one (but not off-by-many) accesses in the same way.
> 
> > [...]  My original intent was to suggest that maybe mudflap could
> > (perhaps optionally) check pointer arithmetic as well [...]
> 
> It might help catch more problems earlier and with more specificity,
> but this would require a lot more instrumentation code to be
> run, and thus slow down execution even more.

True, but without padding around static (and stack) objects, and without
any pointer arithmetic checking, there are a large number of common bugs
that it won't be able to catch.  It's hard to say, but I'd guess that
adding some padding around static and stack objects would catch most
of them.  The overhead of checking all pointer arithmetic may not be
worth it.

--Doug.

^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: mudflap problem
  2004-09-29 23:35       ` Doug Graham
@ 2004-10-06 10:15         ` Doug Graham
  2004-10-06 15:00           ` Frank Ch. Eigler
  0 siblings, 1 reply; 21+ messages in thread
From: Doug Graham @ 2004-10-06 10:15 UTC (permalink / raw)
  To: gcc

On Wed, Sep 29, 2004 at 06:50:40PM -0400, Graham, Doug [CAR:QT96:EXCH] wrote:
> On Wed, Sep 29, 2004 at 06:05:56PM -0400, Frank Ch. Eigler wrote:
> > 
> > > I thought that since b+16 points to the base of a, mudflap would not
> > > complain about the dereference in foo().  But it did somehow know that
> > > the pointer passed in to foo() was derived from 'b' rather than 'a'.
> > 
> > I believe this is an optimization artifact.  With "-O2", the declaration of
> > "a" is marked somehow as unused (it is), and libmudflap does not get
> > a startup-time registration call for it.  Thus b[16] is checked and is found
> > to refer to an invalid address.
> 
> Aha!  You're right, only 'b' is passed to __mf_register.  This happened
> without -O2; my compile line was just: gcc -fmudflap -o foo foo.c.

Hey wait a minute.  Isn't this a bug?  a[16] has external linkage,
so mudflap can't know that it isn't going to be accessed from some
other compilation unit.  When I add a second source file that
does this:

  extern int a[];
  void baz() { a[0] = 42; }

mudflap complains during compilation of this new module that it can't
track the lifetime of `a', and then complains at runtime when legal
accesses to a[] are made.  I'm not sure what the compilation warning
is all about, as a[]'s lifetime is obviously the lifetime of the program,
but the runtime warning occurs because a[] wasn't registered.

--Doug.


^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: mudflap problem
  2004-10-06 10:15         ` Doug Graham
@ 2004-10-06 15:00           ` Frank Ch. Eigler
  0 siblings, 0 replies; 21+ messages in thread
From: Frank Ch. Eigler @ 2004-10-06 15:00 UTC (permalink / raw)
  To: Doug Graham; +Cc: gcc


dgraham wrote:

> [...]  Hey wait a minute.  Isn't this a bug?  a[16] has external
> linkage, so mudflap can't know that it isn't going to be accessed
> from some other compilation unit.  [...]

You're right.  Mudflap should instrument such extern declarations, and
omit them only for unaddressed static ones.  (I think it once did
that.)

> [...]  I'm not sure what the ["cannot track lifetime"] compilation warning
> is all about [...]

I'll reword that warning message.  It's trying to say that it realizes
that the extern variable should have its lifetime tracked via
mf_register/mf_unregister, but it doesn't know the actual size (since
it's declared as "int a [];") so can't do this itself in this compilation unit.

- FChE

^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: mudflap problem
  2005-02-07 20:32                     ` Frank Ch. Eigler
@ 2005-02-07 22:23                       ` Doug Graham
  0 siblings, 0 replies; 21+ messages in thread
From: Doug Graham @ 2005-02-07 22:23 UTC (permalink / raw)
  To: gcc

On Sun, Feb 06, 2005 at 08:14:22PM -0500, Frank Ch. Eigler wrote:
> 
> dgraham wrote:
> 
> > [...]  I suspect that most of the cost is in backtrace_symbols().
> > Couldn't the conversion to symbolic names just be done when an error
> > occurs, rather than every time a backtrace is captured?
> 
> That is a possibility.  It might lose some information, should the
> program layout change due to e.g. shared libraries, but that's a small
> price to pay for the probable speed increase.  You could test the
> theory approximately by disabling HAVE_BACKTRACE_SYMBOLS.

That's not so bad:

  10.86s user 0.68s system 98% cpu 11.711 total

and if I remove the alternative code, the stuff that does all the
sprintfs:

  10.00s user 0.35s system 100% cpu 10.343 total

It looks like backtrace() itself isn't all the swift either.  A test
program that just captures a million 10 deep backtraces takes about 3s
using glibc's backtrace(), and about 0.1s using a homerolled backtrace().
I guess glibc's version might have to handle things like code compiled
without frame pointers (which mine doesn't), but I wonder too if it
turns an O(n) algorithm into an O(n**2) algorithm by looping over
__builtin_return_address(n).  Presumably, __builtin_return_address(n)
is O(n) itself.

Also, I don't think backtrace_symbols() even works the way it's supposed
to (in GLIBC 2.3.4 at least).  It seems to find some symbol names for code
in GLIBC itself, but it's not finding any for code in the main executable.
Haven't looked at the source for it, and there's no documentation for it
that I can find, but I assume that it's supposed to search for symbols
in the main executable too.

--Doug.

^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: mudflap problem
  2005-02-07  6:41                   ` Doug Graham
@ 2005-02-07 20:32                     ` Frank Ch. Eigler
  2005-02-07 22:23                       ` Doug Graham
  0 siblings, 1 reply; 21+ messages in thread
From: Frank Ch. Eigler @ 2005-02-07 20:32 UTC (permalink / raw)
  To: Doug Graham; +Cc: gcc


dgraham wrote:

> [...]  I suspect that most of the cost is in backtrace_symbols().
> Couldn't the conversion to symbolic names just be done when an error
> occurs, rather than every time a backtrace is captured?

That is a possibility.  It might lose some information, should the
program layout change due to e.g. shared libraries, but that's a small
price to pay for the probable speed increase.  You could test the
theory approximately by disabling HAVE_BACKTRACE_SYMBOLS.

- FChE

^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: mudflap problem
  2005-02-06 12:12               ` Doug Graham
  2005-02-06 17:33                 ` Doug Graham
@ 2005-02-07 15:39                 ` Frank Ch. Eigler
  2005-02-07  6:41                   ` Doug Graham
  1 sibling, 1 reply; 21+ messages in thread
From: Frank Ch. Eigler @ 2005-02-07 15:39 UTC (permalink / raw)
  To: Doug Graham; +Cc: gcc


Doug Graham <dgraham@nortel.com> writes:

> [...]  AFAIK, the only extra system CPU that would be used by
> mudflap are the frequent calls to gettimeofday.  So I tried a run
> after setting -no-timestamps in MUDFLAP_OPTIONS and got this:
>     mudflap: 19.51s user 0.56s system 99% cpu 20.075 total
> Saved a bunch of user CPU too.
> [...]

Oh yea, and add "-backtrace=0" too.

- FChE

^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: mudflap problem
  2005-02-07 15:39                 ` Frank Ch. Eigler
@ 2005-02-07  6:41                   ` Doug Graham
  2005-02-07 20:32                     ` Frank Ch. Eigler
  0 siblings, 1 reply; 21+ messages in thread
From: Doug Graham @ 2005-02-07  6:41 UTC (permalink / raw)
  To: gcc

On Sun, Feb 06, 2005 at 04:30:06PM -0500, Frank Ch. Eigler wrote:
> 
> Doug Graham <dgraham@nortel.com> writes:
> 
> > [...]  AFAIK, the only extra system CPU that would be used by
> > mudflap are the frequent calls to gettimeofday.  So I tried a run
> > after setting -no-timestamps in MUDFLAP_OPTIONS and got this:
> >     mudflap: 19.51s user 0.56s system 99% cpu 20.075 total
> > Saved a bunch of user CPU too.
> > [...]
> 
> Oh yea, and add "-backtrace=0" too.

Yow.  Capturing backtraces sure ain't cheap.

MUDFLAP_OPTIONS=-no-timestamps -backtrace=0
6.24s user 0.46s system 100% cpu 6.698 total

There are about 700K mallocs and 50K frees, and almost no reallocs,
callocs or strdups done in this test.  I'm surprised that capturing
a backtrace appears to be so relatively expensive in comparison with
malloc and free themselves (as far as I can tell, mudflap only captures a
backtrace when heap objects are created or deleted).

HAVE_BACKTRACE and HAVE_BACKTRACE_SYMBOLS are set, so __mf_backtrace is
calling backtrace() and backtrace_symbols() from glibc.  I suspect that
most of the cost is in backtrace_symbols().  Couldn't the conversion to
symbolic names just be done when an error occurs, rather than every time
a backtrace is captured?

--Doug.

^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: mudflap problem
  2005-02-01 18:55       ` Doug Graham
@ 2005-02-06 18:23         ` Frank Ch. Eigler
  2005-02-06 18:21           ` Doug Graham
  0 siblings, 1 reply; 21+ messages in thread
From: Frank Ch. Eigler @ 2005-02-06 18:23 UTC (permalink / raw)
  To: Doug Graham; +Cc: gcc


Doug Graham <dgraham@nortel.com> writes:

> [...]
> > That's right.  Like the other old bounded-pointers gcc patch (Hi Gary!),
> > the sourceforge-hosted bounds-checker patch uses "fat" pointers for
> > some local pointer variables.  This enables this particular check.
> 
> Do you mean the patches at http://sourceforge.net/projects/boundschecking?
> If so, I don't think they use fat pointers for anything.  

You could be right.  I based my comment on a glance at documentation
embedded within the latest patch.

> But they are able to detect a lot of problems that mudflap can't,
> because they 1) add padding between objects, and 2) check all
> pointer arithmetic. [...]

Right: all those extra function calls into the runtime pay for the
extra checking capability.

> I'm trying to figure out how to say this in a way that doesn't make
> me sound ungrateful, but IMO, mudflap just won't be very useful
> until it incorporates a feature like [adding gaps / fattening up
> local pointers].  [...]

It's a fair observation.  BTW, how does mudflap's performance compare
to the boundchecker (not having the latter built lately)?


- FChE

^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: mudflap problem
  2005-02-06 18:23         ` Frank Ch. Eigler
@ 2005-02-06 18:21           ` Doug Graham
  2005-02-06 12:12             ` Doug Graham
  0 siblings, 1 reply; 21+ messages in thread
From: Doug Graham @ 2005-02-06 18:21 UTC (permalink / raw)
  To: gcc

On Thu, Feb 03, 2005 at 03:58:13PM -0500, Frank Ch. Eigler wrote:
> > I'm trying to figure out how to say this in a way that doesn't make
> > me sound ungrateful, but IMO, mudflap just won't be very useful
> > until it incorporates a feature like [adding gaps / fattening up
> > local pointers].  [...]
> 
> It's a fair observation.  BTW, how does mudflap's performance compare
> to the boundchecker (not having the latter built lately)?

I'm in the opposite boat.  I haven't built a GCC that supports mudflap
lately.  Last time I did a performance comparison, the sourceforge patches
came out ahead, I think by a factor of about two.  This despite the
fact that many more calls were made into its runtime (to do pointer
arithmetic checking) than mudflap made into its runtime.

I'll grab the latest gcc 3.5 snapshot and compare mudflap against
the bounds-checking patches in 3.4.3 and let you know.

--Doug.

^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: mudflap problem
  2005-02-06 12:12             ` Doug Graham
  2005-02-06 12:12               ` Doug Graham
@ 2005-02-06 17:34               ` Doug Graham
  1 sibling, 0 replies; 21+ messages in thread
From: Doug Graham @ 2005-02-06 17:34 UTC (permalink / raw)
  To: gcc

On Thu, Feb 03, 2005 at 04:40:12PM -0500, Doug Graham wrote:
> On Thu, Feb 03, 2005 at 03:58:13PM -0500, Frank Ch. Eigler wrote:
> > > I'm trying to figure out how to say this in a way that doesn't make
> > > me sound ungrateful, but IMO, mudflap just won't be very useful
> > > until it incorporates a feature like [adding gaps / fattening up
> > > local pointers].  [...]
> > 
> > It's a fair observation.  BTW, how does mudflap's performance compare
> > to the boundchecker (not having the latter built lately)?
>
> I'll grab the latest gcc 3.5 snapshot and compare mudflap against
> the bounds-checking patches in 3.4.3 and let you know.

I guess 3.5 turned into 4.0 while I wasn't paying attention.

I compared mudflap in the gcc-4.0-20050130 snapshot against the bounds
checking patches for 3.4.3.  Compile lines were:

  gcc-4.0 -g -O2 -Wall parse.c -o t.neither
  gcc-3.4.3 -g -O2 -Wall -fbounds-checking parse.c -o t.bounds
  gcc-4.0 -g -O2 -Wall -fmudflap parse.c -lmudflap -o t.mudflap

The test program parse.c is the same one I sent you a while back.
Here are the numbers:

  neither:  0.43s user 0.13s system 101% cpu  0.553 total
  bounds:   7.01s user 0.28s system  99% cpu  7.297 total
  mudflap: 25.27s user 5.08s system 100% cpu 30.339 total

With MUDFLAP_OPTIONS set to -collect-stats, it prints this:

  mudflap stats:
  calls to __mf_check: 3709518
           __mf_register: 2538319 [524294B, 17919030B, 57738B, 16324113B, 47745B]
           __mf_unregister: 1879528 [16324108B]
           __mf_violation: [0, 0, 0, 0, 0]
  calls with reentrancy: 764057
  lookup cache slots used: 65536  unused: 0  peak-reuse: 80529
  number of live objects: 658791
          zombie objects: 204

Bounds checking prints this:

  Bounds library call frequency statistics:
    Calls to push, pop, param function:        9075792, 9075791, 2
    Calls to add, delete stack:                6470885, 6470882
    Calls to add, delete heap:                 711363, 52693
    Calls to check pointer +/- integer:        2474875
    Calls to check array references:           4119089
    Calls to check pointer differences:        926394
    Calls to check object references:          86383858
    Calls to check component references:       3955305
    Calls to check truth, falsity of pointers: 4021139, 516427
    Calls to check <, >, <=, >= of pointers:   0
    Calls to check ==, != of pointers:         8058643
    Calls to check p++, ++p, p--, --p:         39612694, 1395, 0, 0
    Calls to add, find, delete oob pointers:   0, 0, 0
    References to unchecked static, stack:     0, 0

Neither found any bugs :-)

I thought I remembered mudflap being only about twice as slow as
bounds checking, but now it seems to be about 4 times slower.

--Doug.

^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: mudflap problem
  2005-02-06 12:12               ` Doug Graham
@ 2005-02-06 17:33                 ` Doug Graham
  2005-02-07 15:39                 ` Frank Ch. Eigler
  1 sibling, 0 replies; 21+ messages in thread
From: Doug Graham @ 2005-02-06 17:33 UTC (permalink / raw)
  To: gcc

On Fri, Feb 04, 2005 at 01:05:52AM -0500, Doug Graham wrote:
> On Thu, Feb 03, 2005 at 04:40:12PM -0500, Doug Graham wrote:
> The test program parse.c is the same one I sent you a while back.
> Here are the numbers:
> 
>   neither:  0.43s user 0.13s system 101% cpu  0.553 total
>   bounds:   7.01s user 0.28s system  99% cpu  7.297 total
>   mudflap: 25.27s user 5.08s system 100% cpu 30.339 total

Forgot to mention: this is on a Fedora Core 3 system running on an Athlon
XP 3200+.  I find it interesting that mudflap used almost as much system
CPU as bounds checking used total CPU.  AFAIK, the only extra system CPU
that would be used by mudflap are the frequent calls to gettimeofday.
So I tried a run after setting -no-timestamps in MUDFLAP_OPTIONS and
got this:

    mudflap: 19.51s user 0.56s system 99% cpu 20.075 total

Saved a bunch of user CPU too.

Getting closer.

--Doug.

^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: mudflap problem
  2005-02-06 12:12             ` Doug Graham
@ 2005-02-06 12:12               ` Doug Graham
  2005-02-06 17:33                 ` Doug Graham
  2005-02-07 15:39                 ` Frank Ch. Eigler
  2005-02-06 17:34               ` Doug Graham
  1 sibling, 2 replies; 21+ messages in thread
From: Doug Graham @ 2005-02-06 12:12 UTC (permalink / raw)
  To: gcc

On Fri, Feb 04, 2005 at 01:05:52AM -0500, Doug Graham wrote:
> On Thu, Feb 03, 2005 at 04:40:12PM -0500, Doug Graham wrote:
> The test program parse.c is the same one I sent you a while back.
> Here are the numbers:
> 
>   neither:  0.43s user 0.13s system 101% cpu  0.553 total
>   bounds:   7.01s user 0.28s system  99% cpu  7.297 total
>   mudflap: 25.27s user 5.08s system 100% cpu 30.339 total

Forgot to mention: this is on a Fedora Core 3 system running on an Athlon
XP 3200+.  I find it interesting that mudflap used almost as much system
CPU as bounds checking used total CPU.  AFAIK, the only extra system CPU
that would be used by mudflap are the frequent calls to gettimeofday.
So I tried a run after setting -no-timestamps in MUDFLAP_OPTIONS and
got this:

    mudflap: 19.51s user 0.56s system 99% cpu 20.075 total

Saved a bunch of user CPU too.

Getting closer.

--Doug.

^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: mudflap problem
  2005-02-06 18:21           ` Doug Graham
@ 2005-02-06 12:12             ` Doug Graham
  2005-02-06 12:12               ` Doug Graham
  2005-02-06 17:34               ` Doug Graham
  0 siblings, 2 replies; 21+ messages in thread
From: Doug Graham @ 2005-02-06 12:12 UTC (permalink / raw)
  To: gcc

On Thu, Feb 03, 2005 at 04:40:12PM -0500, Doug Graham wrote:
> On Thu, Feb 03, 2005 at 03:58:13PM -0500, Frank Ch. Eigler wrote:
> > > I'm trying to figure out how to say this in a way that doesn't make
> > > me sound ungrateful, but IMO, mudflap just won't be very useful
> > > until it incorporates a feature like [adding gaps / fattening up
> > > local pointers].  [...]
> > 
> > It's a fair observation.  BTW, how does mudflap's performance compare
> > to the boundchecker (not having the latter built lately)?
>
> I'll grab the latest gcc 3.5 snapshot and compare mudflap against
> the bounds-checking patches in 3.4.3 and let you know.

I guess 3.5 turned into 4.0 while I wasn't paying attention.

I compared mudflap in the gcc-4.0-20050130 snapshot against the bounds
checking patches for 3.4.3.  Compile lines were:

  gcc-4.0 -g -O2 -Wall parse.c -o t.neither
  gcc-3.4.3 -g -O2 -Wall -fbounds-checking parse.c -o t.bounds
  gcc-4.0 -g -O2 -Wall -fmudflap parse.c -lmudflap -o t.mudflap

The test program parse.c is the same one I sent you a while back.
Here are the numbers:

  neither:  0.43s user 0.13s system 101% cpu  0.553 total
  bounds:   7.01s user 0.28s system  99% cpu  7.297 total
  mudflap: 25.27s user 5.08s system 100% cpu 30.339 total

With MUDFLAP_OPTIONS set to -collect-stats, it prints this:

  mudflap stats:
  calls to __mf_check: 3709518
           __mf_register: 2538319 [524294B, 17919030B, 57738B, 16324113B, 47745B]
           __mf_unregister: 1879528 [16324108B]
           __mf_violation: [0, 0, 0, 0, 0]
  calls with reentrancy: 764057
  lookup cache slots used: 65536  unused: 0  peak-reuse: 80529
  number of live objects: 658791
          zombie objects: 204

Bounds checking prints this:

  Bounds library call frequency statistics:
    Calls to push, pop, param function:        9075792, 9075791, 2
    Calls to add, delete stack:                6470885, 6470882
    Calls to add, delete heap:                 711363, 52693
    Calls to check pointer +/- integer:        2474875
    Calls to check array references:           4119089
    Calls to check pointer differences:        926394
    Calls to check object references:          86383858
    Calls to check component references:       3955305
    Calls to check truth, falsity of pointers: 4021139, 516427
    Calls to check <, >, <=, >= of pointers:   0
    Calls to check ==, != of pointers:         8058643
    Calls to check p++, ++p, p--, --p:         39612694, 1395, 0, 0
    Calls to add, find, delete oob pointers:   0, 0, 0
    References to unchecked static, stack:     0, 0

Neither found any bugs :-)

I thought I remembered mudflap being only about twice as slow as
bounds checking, but now it seems to be about 4 times slower.

--Doug.

^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: mudflap problem
  2005-01-28 21:43     ` Frank Ch. Eigler
@ 2005-02-01 18:55       ` Doug Graham
  2005-02-06 18:23         ` Frank Ch. Eigler
  0 siblings, 1 reply; 21+ messages in thread
From: Doug Graham @ 2005-02-01 18:55 UTC (permalink / raw)
  To: gcc

On Fri, Jan 28, 2005 at 03:57:05PM -0500, Frank Ch. Eigler wrote:
> On Fri, Jan 28, 2005 at 03:20:33PM -0500, Chris Scott wrote:
> > [...]  A thought about this. Adding padding 
> > would catch iterations, but it would not catch discrete references that 
> > step outside the bounds of an object. We discovered the sourceforge 
> > bounds checker does not rely on its padding to make this work. [...]
> > This is because the sourceforge checker creates an 
> > "out of bounds" pointer for i when i goes outside of b. [...]
> 
> That's right.  Like the other old bounded-pointers gcc patch (Hi Gary!),
> the sourceforge-hosted bounds-checker patch uses "fat" pointers for
> some local pointer variables.  This enables this particular check.

Do you mean the patches at http://sourceforge.net/projects/boundschecking?
If so, I don't think they use fat pointers for anything.  But they are
able to detect a lot of problems that mudflap can't, because they 1) add
padding between objects, and 2) check all pointer arithmetic.  AFAIK,
it's still necessary to include padding even though pointer arithmetic
is checked, because the C language allows a pointer to point one past
the end of an array.  Padding is needed to disambiguate that case from
case of a pointer pointing to the immediately following object.

The only mention of work done on gcc fat pointers that I remember seeing
is that done by Greg McGary a few years back, but I don't think those
were ever made widely available.

> Mudflap could be extended transparently to make use of the technique,
> and it might make a performance difference (no need to check the lookup
> cache) the as well as catching a different (larger?) space of errors.
> The necessary transformations could fit well into the tree-ssa framework.

I'm trying to figure out how to say this in a way that doesn't make me
sound ungrateful, but IMO, mudflap just won't be very useful until it
incorporates a feature like this.  Mudflap has the advantage that it
works on C++, but aside from that, I think that the sourceforge patches
are quite a bit more useful when working with C code.

--Doug.

^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: mudflap problem
       [not found]   ` <41FA9E91.4030801@ll.mit.edu>
@ 2005-01-28 21:43     ` Frank Ch. Eigler
  2005-02-01 18:55       ` Doug Graham
  0 siblings, 1 reply; 21+ messages in thread
From: Frank Ch. Eigler @ 2005-01-28 21:43 UTC (permalink / raw)
  To: Chris Scott; +Cc: gcc, mikez

[-- Attachment #1: Type: text/plain, Size: 977 bytes --]

Hi -


On Fri, Jan 28, 2005 at 03:20:33PM -0500, Chris Scott wrote:
> [...]  A thought about this. Adding padding 
> would catch iterations, but it would not catch discrete references that 
> step outside the bounds of an object. We discovered the sourceforge 
> bounds checker does not rely on its padding to make this work. [...]
> This is because the sourceforge checker creates an 
> "out of bounds" pointer for i when i goes outside of b. [...]

That's right.  Like the other old bounded-pointers gcc patch (Hi Gary!),
the sourceforge-hosted bounds-checker patch uses "fat" pointers for
some local pointer variables.  This enables this particular check.

Mudflap could be extended transparently to make use of the technique,
and it might make a performance difference (no need to check the lookup
cache) the as well as catching a different (larger?) space of errors.
The necessary transformations could fit well into the tree-ssa framework.


- FChE

[-- Attachment #2: Type: application/pgp-signature, Size: 189 bytes --]

^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: mudflap problem
       [not found] <41FA851F.9080802@ll.mit.edu>
@ 2005-01-28 19:58 ` Frank Ch. Eigler
       [not found]   ` <41FA9E91.4030801@ll.mit.edu>
  0 siblings, 1 reply; 21+ messages in thread
From: Frank Ch. Eigler @ 2005-01-28 19:58 UTC (permalink / raw)
  To: Chris Scott; +Cc: gcc, mikez

[-- Attachment #1: Type: text/plain, Size: 978 bytes --]

Hi -


[adding cc: gcc mailing list]

On Fri, Jan 28, 2005 at 01:31:59PM -0500, Chris Scott wrote:
> We've been doing some testing with mudflap. For some reason, mudflap 
> does not detect an overflow here, but the sourceforge bounds checker does:
> 
> int a[1024];
> int b[1024];
> 
> [...]
>        int *i;
>        for (i = &b[0]  ; i < &a[1024] ; i++) {
>              *i = 0;
> [...]
> How come mudflap does not catch this [when sourceforge bounds checker
> does]?

That is because mudflap works differently.  It only checks that each
individual pointer dereference points to a valid object: which for
each value of i in this particular case, it does.  It does not
track the fact that i was originally assigned from &b[0].

A straightforward improvement to mudflap would be to emit padding
between adjacent static (or even auto) objects, and mark those
regions as unaccessible.  Then such simple iteration patterns would
be caught.


- FChE

[-- Attachment #2: Type: application/pgp-signature, Size: 189 bytes --]

^ permalink raw reply	[flat|nested] 21+ messages in thread

end of thread, other threads:[~2005-02-07  4:21 UTC | newest]

Thread overview: 21+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2004-09-28 21:38 mudflap problem Herman ten Brugge
2004-09-29  1:57 ` Frank Ch. Eigler
2004-09-29  4:59   ` Frank Ch. Eigler
2004-09-29 23:01   ` Doug Graham
2004-09-29 23:14     ` Frank Ch. Eigler
2004-09-29 23:35       ` Doug Graham
2004-10-06 10:15         ` Doug Graham
2004-10-06 15:00           ` Frank Ch. Eigler
     [not found] <41FA851F.9080802@ll.mit.edu>
2005-01-28 19:58 ` Frank Ch. Eigler
     [not found]   ` <41FA9E91.4030801@ll.mit.edu>
2005-01-28 21:43     ` Frank Ch. Eigler
2005-02-01 18:55       ` Doug Graham
2005-02-06 18:23         ` Frank Ch. Eigler
2005-02-06 18:21           ` Doug Graham
2005-02-06 12:12             ` Doug Graham
2005-02-06 12:12               ` Doug Graham
2005-02-06 17:33                 ` Doug Graham
2005-02-07 15:39                 ` Frank Ch. Eigler
2005-02-07  6:41                   ` Doug Graham
2005-02-07 20:32                     ` Frank Ch. Eigler
2005-02-07 22:23                       ` Doug Graham
2005-02-06 17:34               ` Doug Graham

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).