Re: string::iterator should have more error checking

[Jonathan Wakely <jwakely@redhat.com>]

On Fri, 24 Jun 2022 at 11:11, Frederick Virchanza Gotham
<cauldwell.thomas@gmail.com> wrote:
>
> On Fri, Jun 24, 2022 at 10:35 AM Jonathan Wakely <jwakely@redhat.com> wrote:
>
> > That's expected. The contents of the string_view are a string literal,
> > which is in the program image, not on the heap.
>
>
> Valgrind is the best tool for troubleshooting heap problems (although
> it's useless against non-heap problems).
>
>
> > AddressSanitizer
> > doesn't check access to such memory.
> > The byte after the string literal "brush" is uninitialized, but it
> > does exist in a valid memory page.
>
>
> Check out how "-fsanitize" flags the following invalid access to
> static-duration constexpr arrays:
>
>         https://godbolt.org/z/fjnPTWj5P
>
> Having thought about this a little more, I realise that "brush" in memory is:
>
>       char constexpr brush[6u] = { 'b', 'r', 'u', 's', 'h', '\0' };
>
> And so therefore it is perfectly valid to access the null terminator
> located at brush[5u], which is why the access is not flagged by
> "-fsanitize".

But &*(s2.cend() + 1u) accesses the byte after the null, doesn't it?

The issue is more that the compiler turns &*p into just p, and then
there is no further *p dereference that can be seen by ASan. It's
still undefined though.

>
> > And it's not detected by Debug Mode because string_view iterators are
> > just pointers.
>
>
> I understand that. I think there should be "<debug/string_view>" and
> that it should be optionally configurable to barf when it encounters a
> null terminator. (It must be optionally configurable because a null
> char is actually a valid char inside a string_view). Something like:
>
>     #define _GLIBCXX_STRINGVIEW_DONT_ALLOW_NULL_CHAR
>     #include <debug/string_view>

A debug string_view is unlikely to happen unless you do the work to add it.

I don't think that extra check is appropriate though. As you say, it's
entirely valid according to the standard, so if your application wants
to disallow that, checking for it belongs in your application.