Re: Fix writing beyond array bounds in codecvt/unshift/char/1.cc

[Alexander Monakov <amonakov at ispras dot ru>]

On Thu, 22 Apr 2010, Jonathan Wakely wrote:

> On 22 April 2010 14:14, Alexander Monakov wrote:
> > [adding libstdc++@gcc.gnu.org to Cc:]
> >
> > Alexander Monakov <amonakov@ispras.ru> writes:
> >> > 'size' definition seemingly does not account for a zero byte, and thus strcpy
> >> > call near the end of the test overwrites one byte beyond allocated memory,
> >> > which may lead to spurious test failures. ?The patch simply bumps size to 24.
> >
> > On Tue, 13 Apr 2010, Andreas Schwab wrote:
> >
> >> Alexander Monakov <amonakov@ispras.ru> writes:
> >>
> >> > diff --git a/libstdc++-v3/testsuite/22_locale/codecvt/unshift/char/1.cc b/libstdc++-v3/testsuite/22_locale/codecvt/unshift/char/1.cc
> >> > index ba417af..8a45efc 100644
> >> > --- a/libstdc++-v3/testsuite/22_locale/codecvt/unshift/char/1.cc
> >> > +++ b/libstdc++-v3/testsuite/22_locale/codecvt/unshift/char/1.cc
> >> > @@ -35,7 +35,7 @@ void test01()
> >> > ? ?bool test __attribute__((unused)) = true;
> >> > ? ?const char* ? ? ? ? ? ? ?c_lit = "black pearl jasmine tea";
> >> > ? ?const char* ? ? ? ? ? ? ?from_next;
> >> > - ?int ? ? ? ? ? ? ? ? ? ? ?size = 23;
> >> > + ?int ? ? ? ? ? ? ? ? ? ? ?size = 24;
> >>
> >> How about using strlen(c_lit)?
> >>
> >> > ? ?char* ? ? ? ? ? ?c_arr = new char[size];
> >> > ? ?char* ? ? ? ? ? ? ? ? c_ref = new char[size];
> >>
> >> I think you rather want to use size+1 for the allocations.
> >>
> >> Andreas.
> >>
> >>
> >
> > I'd rather keep allocations as is, since size is passed as an argument into
> > various functions in the test, and we probably want to notice them access one
> > element beyond array bounds. ?Thus, I'm changing size to strlen(c_lit) + 1.
> >
> > OK to commit?
> >
> > 2010-04-22 ?Alexander Monakov ?<amonakov@ispras.ru>
> >
> > ? ? ? ?* 22_locale/codecvt/unshift/char/1.cc (test01): Correct definition of size.
> >
> > diff --git a/libstdc++-v3/testsuite/22_locale/codecvt/unshift/char/1.cc b/libstdc++-v3/testsuite/22_locale/codecvt/unshift/char/1.cc
> > index ba417af..6e851ff 100644
> > --- a/libstdc++-v3/testsuite/22_locale/codecvt/unshift/char/1.cc
> > +++ b/libstdc++-v3/testsuite/22_locale/codecvt/unshift/char/1.cc
> > @@ -35,7 +35,7 @@ void test01()
> > ? bool test __attribute__((unused)) = true;
> > ? const char* ? ? ? ? ?c_lit = "black pearl jasmine tea";
> > ? const char* ? ? ? ? ?from_next;
> > - ?int ? ? ? ? ? ? ? ? ?size = 23;
> > + ?int ? ? ? ? ? ? ? ? ?size = strlen(c_lit) + 1;
> > ? char* ? ? ? ? ? ? ? ?c_arr = new char[size];
> > ? char* ? ? ? ? ? ? ? ? c_ref = new char[size];
> > ? char* ? ? ? ? ? ? ? ? ? ? ? ?to_next;
> 
> But now when c_lit+size is passed to cvt->in() and cvt->out() it will
> be too large.

Sorry, I do not agree.  Those are past-the-end iterators used to define a
sequence [c_lit, c_lit+size) (right end not inclusive) and must be legal.
Or am I missing something?

Thanks.

Alexander