From mboxrd@z Thu Jan  1 00:00:00 1970
Return-Path: <gcc-bugzilla@gcc.gnu.org>
Received: by sourceware.org (Postfix, from userid 48)
	id 284EC3852C52; Wed, 30 Nov 2022 14:29:55 +0000 (GMT)
DKIM-Filter: OpenDKIM Filter v2.11.0 sourceware.org 284EC3852C52
DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gcc.gnu.org;
	s=default; t=1669818595;
	bh=Z3yJEIPG+PDWcDqedFahdN50eMmk2AIwTWWjfcAmbyU=;
	h=From:To:Subject:Date:In-Reply-To:References:From;
	b=tFZ5MzaVM6uS9UXXqqZvuJhEyD87QRYSrOVZobTOvvcXeO4jEv3wOBlbUuMQs7xmx
	 Bm9lCu8YnKcsSVQ2ciJrhzRf19EQJUvEP+R+gUPlfJ6X7Jk7538RJG7f5DbVW4j/kh
	 YkuacIpgdvXAZ81C3+G2JvhSqP9uKicvE6styc8Q=
From: "redi at gcc dot gnu.org" <gcc-bugzilla@gcc.gnu.org>
To: gcc-bugs@gcc.gnu.org
Subject: [Bug c++/106434] [12/13 Regression] Spurious -Wnull-dereference when
 using std::unique_copy() since r12-5187-g1ae8edf5f73ca5c3
Date: Wed, 30 Nov 2022 14:29:54 +0000
X-Bugzilla-Reason: CC
X-Bugzilla-Type: changed
X-Bugzilla-Watch-Reason: None
X-Bugzilla-Product: gcc
X-Bugzilla-Component: c++
X-Bugzilla-Version: 12.1.0
X-Bugzilla-Keywords: diagnostic
X-Bugzilla-Severity: normal
X-Bugzilla-Who: redi at gcc dot gnu.org
X-Bugzilla-Status: NEW
X-Bugzilla-Resolution: 
X-Bugzilla-Priority: P2
X-Bugzilla-Assigned-To: unassigned at gcc dot gnu.org
X-Bugzilla-Target-Milestone: 12.3
X-Bugzilla-Flags: 
X-Bugzilla-Changed-Fields: 
Message-ID: <bug-106434-4-Cm2fsiVJag@http.gcc.gnu.org/bugzilla/>
In-Reply-To: <bug-106434-4@http.gcc.gnu.org/bugzilla/>
References: <bug-106434-4@http.gcc.gnu.org/bugzilla/>
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: quoted-printable
X-Bugzilla-URL: http://gcc.gnu.org/bugzilla/
Auto-Submitted: auto-generated
MIME-Version: 1.0
List-Id: <gcc-bugs.sourceware.org>

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

--- Comment #8 from Jonathan Wakely <redi at gcc dot gnu.org> ---
So initially iin.iter._M_current is indeed null, because unique.begin() ret=
urns
_M_start (which is null) when the vector is empty:

  std::insert_iterator iin(unique, unique.begin());

But when the vector is empty, this condition is always false:

      if (this->_M_impl._M_finish !=3D this->_M_impl._M_end_of_storage)
        if (__position =3D=3D end())

It's impossible for _M_start to be null unless _M_finish and _M_end_of_stor=
age
are also null.

After the first insertion into the vector all three of _M_start, _M_finish =
and
_M_end_of_storage are non-null. But after the first insertion
iin.iter._M_current is also non-null.

So we need to add a hint so the compiler knows that the jump threaded
"__position is null but finish !=3D end_of_storage" case is nonsense. It's =
not
_impossible_, because a dumb user could make it happen, but it violates the
function precondition so is UB.

i.e. this would take that code path:

std::vector<int> v{1,2,3}; // non-empty vector
std::vector<int>::const_iterator null;
v.insert(null, 1); // try to insert at invalid position

But that's UB.


This seems to work (the __glibcxx_assert isn't needed to stop the warning, =
but
might be a useful assertion).


--- a/libstdc++-v3/include/bits/vector.tcc
+++ b/libstdc++-v3/include/bits/vector.tcc
@@ -137,8 +137,14 @@ _GLIBCXX_BEGIN_NAMESPACE_CONTAINER
     insert(iterator __position, const value_type& __x)
 #endif
     {
+      __glibcxx_assert(capacity() =3D=3D 0 || __position !=3D const_iterat=
or());
+
       const size_type __n =3D __position - begin();
       if (this->_M_impl._M_finish !=3D this->_M_impl._M_end_of_storage)
+        {
+          if (__position =3D=3D const_iterator())
+            __builtin_unreachable();
+
        if (__position =3D=3D end())
          {
            _GLIBCXX_ASAN_ANNOTATE_GROW(1);
@@ -159,6 +165,7 @@ _GLIBCXX_BEGIN_NAMESPACE_CONTAINER
            _M_insert_aux(__position, __x);
 #endif
          }
+      }
       else
 #if __cplusplus >=3D 201103L
        _M_realloc_insert(begin() + (__position - cbegin()), __x);=