[Bug libstdc++/109891] New: Null pointer special handling in ostream's operator << for C-strings

[Bug libstdc++/109891] New: Null pointer special handling in ostream's operator << for C-strings

[mimomorin at gmail dot com]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=109891

            Bug ID: 109891
           Summary: Null pointer special handling in ostream's operator <<
                    for C-strings
           Product: gcc
           Version: 14.0
            Status: UNCONFIRMED
          Severity: normal
          Priority: P3
         Component: libstdc++
          Assignee: unassigned at gcc dot gnu.org
          Reporter: mimomorin at gmail dot com
  Target Milestone: ---

This code

    #include <iostream>
    int main() { std::cout << (char*)nullptr; }

does not cause any bad things (like SEGV), because libstdc++'s
operator<<(ostream, char const*) has special handling of null pointers: 

    template<typename _CharT, typename _Traits>
    inline basic_ostream<_CharT, _Traits>&
    operator<<(basic_ostream<_CharT, _Traits>& __out, const _CharT* __s)
    {
        if (!__s)
            __out.setstate(ios_base::badbit);
        else
            __ostream_insert(...);
        return __out;
    }

Passing a null pointer to this operator is a precondition violation, so the
current implementation perfectly conforms to the C++ standard. But, why don't
we remove this special handling? By doing so, we get
- better interoperability with toolings (i.e. sanitizers can find the bug
easily)
- unnoticeable performace improvement
and we lose
- deterministic behaviors (of poor codes) on a particular stdlib
I believe the first point makes more sense than the last point.

It seems that old special handling `if (s == NULL) s = "(null)";`
(https://github.com/gcc-mirror/gcc/blob/6599da0/libio/iostream.cc#L638) was
removed in GCC 3.0, but reintroduced (in the current form) in GCC 3.2 in
response to https://gcc.gnu.org/bugzilla/show_bug.cgi?id=6518 .

[Bug libstdc++/109891] Null pointer special handling in ostream's operator << for C-strings

[redi at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=109891

--- Comment #1 from Jonathan Wakely <redi at gcc dot gnu.org> ---
Adding more UB to the library doesn't seem wise.

We could make it abort in debug mode, instead of setting badbit, but I don't
think we should just make it UB.

[Bug libstdc++/109891] Null pointer special handling in ostream's operator << for C-strings

[redi at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=109891

--- Comment #2 from Jonathan Wakely <redi at gcc dot gnu.org> ---
--- a/libstdc++-v3/include/bits/ostream.tcc
+++ b/libstdc++-v3/include/bits/ostream.tcc
@@ -306,6 +306,7 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
     basic_ostream<_CharT, _Traits>&
     operator<<(basic_ostream<_CharT, _Traits>& __out, const char* __s)
     {
+      _GLIBCXX_DEBUG_PEDANTIC(__s != 0);
       if (!__s)
        __out.setstate(ios_base::badbit);
       else

[Bug libstdc++/109891] Null pointer special handling in ostream's operator << for C-strings

[mimomorin at gmail dot com]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=109891

--- Comment #3 from Michel Morin <mimomorin at gmail dot com> ---
From the safety point of view, I agree with you. But, at the same time, I
thought that detectable UB (with the help of sanitizers) is useful than silent
bug. 

How about `throw`ing as in std::string's constructor?

[Bug libstdc++/109891] Null pointer special handling in ostream's operator << for C-strings

[pinskia at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=109891

--- Comment #4 from Andrew Pinski <pinskia at gcc dot gnu.org> ---
IIRC this was to added to be similar to glibc's nullptr handling for %s:
printf("xyza %s\n", nullptr);

[Bug libstdc++/109891] Null pointer special handling in ostream's operator << for C-strings

[redi at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=109891

--- Comment #5 from Jonathan Wakely <redi at gcc dot gnu.org> ---
(In reply to Michel Morin from comment #3)
> From the safety point of view, I agree with you. But, at the same time, I
> thought that detectable UB (with the help of sanitizers) is useful than
> silent bug. 

Detectable UB doesn't guarantee detection. Sanitizers are not suitable for
production code. Introducing UB here would be strictly less safe, full stop.

And the bug isn't silent, it makes the stream unusable.

> How about `throw`ing as in std::string's constructor?

Set the exception flag on the stream and you get an exception when badbit is
set.

[Bug libstdc++/109891] Null pointer special handling in ostream's operator << for C-strings

[mimomorin at gmail dot com]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=109891

--- Comment #6 from Michel Morin <mimomorin at gmail dot com> ---
True. Detectable is not correct — that's "maybe-detectable" at most, and the
bug is not silent. In a code that I checked, the buggy code (`std::cout <<
NullCharPtr;`) is the last printing call to std::cout, so I failed to see the
side-effect.

The patchlet using `_GLIBCXX_DEBUG_PEDASSERT` works fine. Actually I would like
`_GLIBCXX_DEBUG_ASSERT` (because I've been using `_GLIBCXX_DEBUG` but never
`_GLIBCXX_DEBUG_PEDANTIC`), but I guess using `_GLIBCXX_DEBUG_PEDASSERT` rather
than `_GLIBCXX_DEBUG_ASSERT` in this case is a delibarate choice.

[Bug libstdc++/109891] Null pointer special handling in ostream's operator << for C-strings

[redi at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=109891

--- Comment #7 from Jonathan Wakely <redi at gcc dot gnu.org> ---
Oops, if I'd typed PEDASSERT not PEDANTIC, it would be a deliberate choice ;-)

Yes, I think PEDASSERT fits better, based on the documented meaning of it
(which even mentions the std::string((const char*)nullptr) case):
https://gcc.gnu.org/onlinedocs/libstdc++/manual/debug_mode_semantics.html

[Bug libstdc++/109891] Null pointer special handling in ostream's operator << for C-strings

[xry111 at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=109891

Xi Ruoyao <xry111 at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |xry111 at gcc dot gnu.org

--- Comment #8 from Xi Ruoyao <xry111 at gcc dot gnu.org> ---
(In reply to Jonathan Wakely from comment #7)
> Oops, if I'd typed PEDASSERT not PEDANTIC, it would be a deliberate choice
> ;-)
> 
> Yes, I think PEDASSERT fits better, based on the documented meaning of it
> (which even mentions the std::string((const char*)nullptr) case):
> https://gcc.gnu.org/onlinedocs/libstdc++/manual/debug_mode_semantics.html

Ccan we add a "pednonnull" attribute or something to produce a -Wnonnull
warning like the nonnull attribute but w/o affecting code generation as well? 
I remember we've discussed "adding nonnull attribute for
basic_ostream::operator<<" and the idea was rejected because the nonnull
attribute would throw away the null pointer check.

[Bug libstdc++/109891] Null pointer special handling in ostream's operator << for C-strings

[mimomorin at gmail dot com]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=109891

--- Comment #9 from Michel Morin <mimomorin at gmail dot com> ---
> (which even mentions the std::string((const char*)nullptr) case):
> https://gcc.gnu.org/onlinedocs/libstdc++/manual/debug_mode_semantics.html

Oh, that's good to know. Understood that PEDASSERT fits better.

> can we add a "pednonnull" attribute or something to produce a -Wnonnull 
> warning like the nonnull attribute but w/o affecting code generation as well?

I think such an attribute (like Clang's _Nonnull) would be a nice addition. So
I grepped Nonnull on libc++, but strangely there are __no__ uses of
_Nonnull/__nonnull. I only found a few __gnu__::__nonnull__ in
__memory_resource/memory_resource.h. In libc++, std::string constructors have
assertions for nullptr check, but there are no attributes.

[Bug libstdc++/109891] Null pointer special handling in ostream's operator << for C-strings

[pinskia at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=109891

Andrew Pinski <pinskia at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
         Resolution|---                         |DUPLICATE
             Status|UNCONFIRMED                 |RESOLVED

--- Comment #10 from Andrew Pinski <pinskia at gcc dot gnu.org> ---
Dup of bug 86130.

*** This bug has been marked as a duplicate of bug 86130 ***