Re: [committed] libstdc++: Disable over-zealous warnings about std::string copies [PR103332]

[Jonathan Wakely <jwakely.gcc@gmail.com>]

On Fri, 10 Dec 2021 at 16:35, Martin Sebor <msebor@gmail.com> wrote:
>
> On 12/10/21 3:12 AM, Jonathan Wakely wrote:
> > On Fri, 10 Dec 2021 at 01:50, Martin Sebor via Libstdc++
> > <libstdc++@gcc.gnu.org> wrote:
> >>
> >> On 12/9/21 5:38 PM, Martin Sebor wrote:
> >>> On 12/9/21 4:24 PM, Jonathan Wakely via Gcc-patches wrote:
> >>>> These warnings are triggered by perfectly valid code using std::string.
> >>>> They're particularly bad when --enable-fully-dynamic-string is used,
> >>>> because even std::string().begin() will give a warning.
> >>>>
> >>>> Use pragmas to stop the troublesome warnings for copies done by
> >>>> std::char_traits.
> >>>
> >>> I'm still experimenting with some of the approaches we discussed
> >>> last week, but based on my findings so far this was going to be
> >>> my suggestion at lest for now, until or unless the problem turns
> >>> out to affect more code than just std::string.
> >>
> >> Just minutes after I wrote this I tried following the clue
> >> in the note printed for the test case from PR 103534 with
> >> an enhancement I'm experimenting with:
> >>
> >> /build/gcc-master/x86_64-pc-linux-gnu/libstdc++-v3/include/bits/char_traits.h:426:56:
> >> warning: ‘void* __builtin_memcpy(void*, const void*, long unsigned int)’
> >> specified size between 18446744073709551600 and 18446744073709551615
> >> exceeds maximum object size 9223372036854775807 [-Wstringop-overflow=]
> >>     426 |         return static_cast<char_type*>(__builtin_memcpy(__s1,
> >> __s2, __n));
> >>         |
> >> ~~~~~~~~~~~~~~~~^~~~~~~~~~~~~~~~~
> >> /build/gcc-master/x86_64-pc-linux-gnu/libstdc++-v3/include/bits/char_traits.h:426:56:
> >> note: when
> >> ‘(<anonymous>.std::__cxx11::basic_string<char>::_M_string_length >
> >> 18446744073709551599)’
> >>
> >> and adding an assert to string::size():
> >>
> >>         constexpr
> >>         size_type
> >>         size() const noexcept
> >>         {
> >>           if (_M_string_length >= -1LU >> 1)
> >
> > I think that will be wrong for 64-bit Windows, where long is 32-bit,
> > but the string's max size is closer to -1LLU >> 2, and we don't want
> > to just use -1LLU because that's wrong for 32-bit targets.
> >
> > Technically the max size is (N - 1)/2 where N is the allocator's max
> > size, which is PTRDIFF_MAX for std::allocator, SIZE_MAX for allocators
> > that use the default value from std::allocator_traits, or some other
> > value that the allocator defines. And the value is reduced
> > appropriately if sizeof(char_type) > 1.
> >
> > But if we call this->max_size() which calls _Alloc_traits::max_size()
> > which potentially calls allocator_type::max_size(), will the compiler
> > actually make those calls to mark the path unreachable, or will it
> > just ignore the unreachable if it can't fold all those calls at
> > compile time?
>
> The above was just a quick proof of concept experiment.  You're
> of course right that the final solution can't be so crude(*).
> But if the required functions are always_inline (I think member
> functions defined in the body of the class implicitly are) then

They're implicitly inline, but not implicitly always_inline.

> I'd expect them to be folded at compile time at all optimization
> levels.  That's what makes -Winvalid-memory-order work in C++
> even at -O0 in the patch I posted earlier this week.
>
> If I still remember my C++-library-fu, even though the standard
> requires containers to call max_size() etc., since std::string
> is (or can be) an explicit specialization, there shouldn't be
> a way for a conforming program to find out if it does, so it
> could take shortcuts.  That makes me wonder if it could even
> call malloc instead of operator new to get around the problem
> with the operator's replaceability.

std::string is required to use std::allocator<char> and users can't
specialize that to observe its allocations, but it does require:

The storage for the array is obtained by calling ::operator new
(17.6.3), but it is unspecified when or how often this function is
called.

Users can certainly observe whether std::string *never* calls their
replaced operator new (and if they've replaced it specifically to
ensure allocations use their own source, and not malloc, they wouldn't
be happy).