Re: PATCH: (update from gcc-bugs post) post b-i-b mergereport/patch

[Zack Weinberg <zack at codesourcery dot com>]

Loren James Rittle <rittle@latour.rsch.comm.mot.com> writes:

> While walking up the stack of validate_switches() calls, it generated
> pointers as high as 0x806af3d (each pop of the call stack wants an
> extra \0 to point to in order to unwind properly).  However, the
> problem in much worse than that.  If one looks at the memory
> allocation related to spec handling, we can plainly see that, in this
> case, it allocated 0xf1a bytes of memory starting at 0x806a000; then
> it moved the contents of the read file to a region starting at
> 0x806b000 and freed the memory at 0x806a000.  Later spec file parsing
> allocated another chunk of memory at 0x806a000 (however, sized only
> ~600 bytes).  The validation routines may skip into a region of memory
> completely unrelated to the one it should be processing...

I'm confused.  Why is validate_switches walking past the end of the
array in the first place?

I *think* you are saying we should need only one '\0' terminator, and
that's what your patch accomplishes.  If that's correct I agree with
it, but I don't have authority to approve patches to gcc.c.  And I
hesitate to comment on the patch itself, since I'm not sure what it
does just looking at the code.

If the problem is that validate_switches returns a pointer incremented
once past the '\0' that stopped it and further processing picks up at
that position, I think I would rather that we decrement the pointer
before returning it, instead of introducing if (*p) all over the
place.  But that may not be feasible given the structure of the rest
of the function, which IIRC is rather long and complicated.
 
zw