This is the mail archive of the gcc-patches@gcc.gnu.org mailing list for the GCC project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

Re: [PATCH] Updated buffer overflow protection patch


On Wed, 2004-10-27 at 04:18, Jakub Jelinek wrote:

> E.g. glibc checks gets (if requested via -D_FORTIFY_SOURCE):
> extern char *__gets_chk (char *__str, size_t);
> #define gets(__str) \
>   ((__bos (__str) == (size_t) -1)                               \
>    ? (gets) (__str) : __gets_chk (__str, __bos (__str)))
> 
Why this hackery in the header files?  Given -ffortify-string-ops, we
could just expand to 'gets_chk' if the length of __str is known at
compile time.

I agree with providing __builtin_object_size for users to add to their
code.  But for the handful of builtins already known by GCC, we
shouldn't need to force glibc to do this.  I think I'm missing something
here.


> that are not easily doable from macros/inlines.
> But I think it is better when the interface is the same.
>
Right, that's why I was thinking that we not force glibc to explicitly
choose between memcpy and memcpy_chk when we can do it inside the
compiler.


> > Also, the propagation of these lengths is only extending the hack we had
> > for strlen and friends.  We need to propagate string attributes such as
> > length using the generic propagator.
> 
> I briefly looked at using the generic propagator but couldn't see how
> it matches the needs of object size propagation.
> First of all, I'd like to understand whether you want to propagate all
> attributes in one pass or have separate passes for different attributes
> (say VRP, string length, CCP, object size) and just use the generic
> propagator for that.
>
Object size is a generic value that will be useful in other contexts
(string length, ranges, mudflap), so I don't think it's a good idea
computing it on demand out of __bos calls.

Once you have propagated the known object sizes for 'char *' and 'char
[]' objects, folding __bos is trivial.  

> The advantages current tree-object-size.c propagator IMHO has is that
> it doesn't compute object sizes that aren't needed and it is easier to handle
> cycles with DU chain loops.
>
You don't need to compute object size for things that you are not
interested in.  DU chains are handled naturally by the value
propagator.  Your PHI visit function will even be able to take advantage
of the executable flag of edges.

> tree-object-size.c propagation works in 3 separate passes.
> The first pass walks the DU chains from __builtin_object_size first
> arguments and computes everything unless there were DU chain loops
> that matter for __builtin_object_size calls.
>
This is the initialization of every propagation pass.

> The second pass deals with __builtin_object_size computing minimum
> within DU chain loops.  For
> char buf[64], *p;
> p = &buf[2];
> for (i = 0; i < x; ++i)
>   p += 2;
> return __builtin_object_size (p, N);
> if N is 0 or 1 (i.e. when computing maximum) 62 should be returned, as
> pointer arithmetic must be within the object bounds (+ one past the last
> byte).  If N is 2 or 3 (computing minimum), 0 should be returned, but
> getting there through pass 3 could take too long.
>
Hmm?  I thought the second argument for __bos was a boolean?  We have
the capability of counting number of iterations in a loop.  If the loop
is countable, you could easily deduce what would be the value of p at
the end (Sebastian, Zdenek, is this correct?)

> Pass 3 is propagation of the object sizes that still need to be
> computed, until no more changes happen.
> 
These two are dealt with in the statement and PHI node visit functions. 
What you have here is a classical value propagation problem.  The
generic propagator is a perfect fit.  You are duplicating much of the
propagation engine.  I'll try and come up with a prototype for this.  I
want to make sure we're trying to solve the same problem.


Diego.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]