[Bug libstdc++/116369] New: [DEBUG MODE,C++20] access to iterators from global empty container defined by "const&" results in SIGSEGV

[Bug libstdc++/116369] New: [DEBUG MODE,C++20] access to iterators from global empty container defined by "const&" results in SIGSEGV

[piotrwn1 at gmail dot com]

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

            Bug ID: 116369
           Summary: [DEBUG MODE,C++20] access to iterators from global
                    empty container defined by "const&" results in SIGSEGV
           Product: gcc
           Version: 14.1.1
            Status: UNCONFIRMED
          Severity: normal
          Priority: P3
         Component: libstdc++
          Assignee: unassigned at gcc dot gnu.org
          Reporter: piotrwn1 at gmail dot com
  Target Milestone: ---

Starting from C++20 (C++23 too) and gcc14.1, with STL debug mode ON (-std=c++20
-D_GLIBCXX_DEBUG) the following code starts producing SIGSEGV:

    #include <vector>

    const std::vector<int>& a{};
    int main() {
        (void)a.begin();
    }

The obvious workaround, changing to "const std::vector<int> a{};" works.
Also the problem is not visible with local vector, only for globals.
And only visible in GLIBC debug mode.
Not also visible in clang - so maybe this is the gcc compiler issue.

Compiler Explorer link: https://godbolt.org/z/PKnbavGe6

[Bug libstdc++/116369] [DEBUG MODE,C++20] access to iterators from global empty container defined by "const&" results in SIGSEGV

[piotrwn1 at gmail dot com]

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

--- Comment #1 from Piotr Nycz <piotrwn1 at gmail dot com> ---
(In reply to Piotr Nycz from comment #0)
> Starting from C++20 (C++23 too) and gcc14.1, with STL debug mode ON
> (-std=c++20 -D_GLIBCXX_DEBUG) the following code starts producing SIGSEGV:
> 
>     #include <vector>
> 
>     const std::vector<int>& a{};
>     int main() {
>         (void)a.begin();
>     }
> 
> The obvious workaround, changing to "const std::vector<int> a{};" works.
> Also the problem is not visible with local vector, only for globals.
> And only visible in GLIBC debug mode.
> Not also visible in clang - so maybe this is the gcc compiler issue.
> 
> Compiler Explorer link: https://godbolt.org/z/PKnbavGe6

proper link: https://godbolt.org/z/1EjrY8vaM

[Bug libstdc++/116369] [DEBUG MODE,C++20] access to iterators from global empty container defined by "const&" results in SIGSEGV

[pinskia at gcc dot gnu.org]

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

--- Comment #2 from Andrew Pinski <pinskia at gcc dot gnu.org> ---
It worked with `14.0.0 20231109`

[Bug libstdc++/116369] [DEBUG MODE,C++20] access to iterators from global empty container defined by "const&" results in SIGSEGV

[pinskia at gcc dot gnu.org]

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

--- Comment #3 from Andrew Pinski <pinskia at gcc dot gnu.org> ---
The difference between the two versions (and even the difference between the
refernece and non reference) is the underlying variable for a1234 is in the
rodata section for the one with the temporary bounded to the reference.

I am not sure this is a libstdc++ issue but maybe a front-end one ...

[Bug c++/116369] [12/13/14/15 Regression] temporary variable bounded to const reference with mutable field incorrectly marked as rodata

[pinskia at gcc dot gnu.org]

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

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

           What    |Removed                     |Added
----------------------------------------------------------------------------
          Component|libstdc++                   |c++
     Ever confirmed|0                           |1
   Last reconfirmed|                            |2024-08-14
   Target Milestone|---                         |12.5
      Known to fail|                            |9.1.0
            Summary|[DEBUG MODE,C++20] access   |[12/13/14/15 Regression]
                   |to iterators from global    |temporary variable bounded
                   |empty container defined by  |to const reference with
                   |"const&" results in SIGSEGV |mutable field incorrectly
                   |                            |marked as rodata
           Keywords|                            |wrong-code
             Status|UNCONFIRMED                 |NEW
      Known to work|                            |5.1.0, 7.1.0

--- Comment #4 from Andrew Pinski <pinskia at gcc dot gnu.org> ---
Note I think there is a front-end (and a library) issue too.

In libstdc++-v3/include/debug/safe_base.h we have:
```
    /// The list of mutable iterators that reference this container
    _Safe_iterator_base* _M_iterators;

    /// The list of constant iterators that reference this container
    _Safe_iterator_base* _M_const_iterators;

    /// The container version number. This number may never be 0.
    mutable unsigned int _M_version;

```

The segfault is the store to _M_const_iterators. Which should have been marked
as mutable but since _M_version is marked as mutable the temporary variable
should NOT have gone in rodata anyways.

Reduced testcase for the front-end issue:
```
struct f{
  int t;
};

const f &g = {1};
```

[Bug c++/116369] [12/13/14/15 Regression] temporary variable bounded to const reference with mutable field incorrectly marked as rodata

[pinskia at gcc dot gnu.org]

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

--- Comment #5 from Andrew Pinski <pinskia at gcc dot gnu.org> ---
Run test:
```
struct f{
  mutable int t;
};

const f &g = {1};

int main()
{
  g.t++;
}
```

[Bug c++/116369] [12/13/14/15 Regression] temporary variable bounded to const reference with mutable field incorrectly marked as rodata

[ppalka at gcc dot gnu.org]

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

Patrick Palka <ppalka at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |jason at gcc dot gnu.org,
                   |                            |ppalka at gcc dot gnu.org
           Keywords|needs-bisection             |

--- Comment #6 from Patrick Palka <ppalka at gcc dot gnu.org> ---
Started with r9-869-g5603790dbf233c

[Bug c++/116369] [12/13/14/15 Regression] temporary variable bounded to const reference with mutable field incorrectly marked as rodata since r9-869-g5603790dbf233c

[mpolacek at gcc dot gnu.org]

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

Marek Polacek <mpolacek at gcc dot gnu.org> changed:

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

--- Comment #7 from Marek Polacek <mpolacek at gcc dot gnu.org> ---
Interesting, r14-1785-g47c3144c2f6b00 already added the !TYPE_HAS_MUTABLE_P
check.

[Bug c++/116369] [12/13/14/15 Regression] temporary variable bounded to const reference with mutable field incorrectly marked as rodata since r9-869-g5603790dbf233c

[redi at gcc dot gnu.org]

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

--- Comment #8 from Jonathan Wakely <redi at gcc dot gnu.org> ---
(In reply to Andrew Pinski from comment #4)
> Note I think there is a front-end (and a library) issue too.
> 
> In libstdc++-v3/include/debug/safe_base.h we have:
> ```
>     /// The list of mutable iterators that reference this container
>     _Safe_iterator_base* _M_iterators;
> 
>     /// The list of constant iterators that reference this container
>     _Safe_iterator_base* _M_const_iterators;
> 
>     /// The container version number. This number may never be 0.
>     mutable unsigned int _M_version;
> 
> ```
> 
> The segfault is the store to _M_const_iterators. Which should have been
> marked as mutable

Why? It's only ever written to in the constructor and in non-const member
functions.

One of those functions is _M_detach_all() which is called from the destructor,
but that's fine because 'this' is non-const during construction and
destruction. (This makes me wonder how any object with a non-trivial destructor
that calls some non-inline function can ever be placed in rodata, because the
destructor could perform arbitrary writes to data members which aren't
necessarily removed by DSE.)

[Bug c++/116369] [12/13/14/15 Regression] temporary variable bounded to const reference with mutable field incorrectly marked as rodata since r9-869-g5603790dbf233c

[redi at gcc dot gnu.org]

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

--- Comment #9 from Jonathan Wakely <redi at gcc dot gnu.org> ---
Ah no, during iterator construction we do:

        this->_M_attach(const_cast<_Safe_sequence_base*>(__seq), __constant);

So that is a library bug.

[Bug c++/116369] [12/13/14/15 Regression] temporary variable bounded to const reference with mutable field incorrectly marked as rodata since r9-869-g5603790dbf233c

[pinskia at gcc dot gnu.org]

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

--- Comment #10 from Andrew Pinski <pinskia at gcc dot gnu.org> ---
(In reply to Jonathan Wakely from comment #9)
> Ah no, during iterator construction we do:
> 
> 	this->_M_attach(const_cast<_Safe_sequence_base*>(__seq), __constant);
> 
> So that is a library bug.

Sorry I should have mentioned that line beforehand. After I noticed the mutable
on one of the fields, I was more focused on that and the front-end side of
things and getting a testcase there rather than describing the full sequence of
code for the library side of things.

[Bug c++/116369] [12/13/14/15 Regression] temporary variable bounded to const reference with mutable field incorrectly marked as rodata since r9-869-g5603790dbf233c

[cvs-commit at gcc dot gnu.org]

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

--- Comment #11 from GCC Commits <cvs-commit at gcc dot gnu.org> ---
The master branch has been updated by Jonathan Wakely <redi@gcc.gnu.org>:

https://gcc.gnu.org/g:a35dd276cbf6236e08bcf6e56e62c2be41cf6e3c

commit r15-3127-ga35dd276cbf6236e08bcf6e56e62c2be41cf6e3c
Author: Jonathan Wakely <jwakely@redhat.com>
Date:   Wed Aug 21 12:29:32 2024 +0100

    libstdc++: Make debug sequence members mutable [PR116369]

    We need to be able to attach debug mode iterators to const containers,
    so the safe iterator constructor uses const_cast to get a modifiable
    pointer to the container. If the container was defined as const, that
    const_cast to access its members results in undefined behaviour.  PR
    116369 shows a case where it results in a segfault because the container
    is in a rodata section (which shouldn't have happened, but the undefined
    behaviour in the library still exists in any case).

    This makes the _M_iterators and _M_const_iterators data members mutable,
    so that it's safe to modify them even if the declared type of the
    container is a const type.

    Ideally we would not need the const_cast at all. Instead, the _M_attach
    member (and everything it calls) should be const-qualified. That would
    work fine now, because the members that it ends up modifying are
    mutable. Making that change would require a number of new exports from
    the shared library, and would require retaining the old non-const member
    functions (maybe as symbol aliases) for backwards compatibility. That
    might be worth changing at some point, but isn't done here.

    libstdc++-v3/ChangeLog:

            PR c++/116369
            * include/debug/safe_base.h (_Safe_sequence_base::_M_iterators):
            Add mutable specifier.
            (_Safe_sequence_base::_M_const_iterators): Likewise.

[Bug c++/116369] [12/13/14/15 Regression] temporary variable bounded to const reference with mutable field incorrectly marked as rodata since r9-869-g5603790dbf233c

[mpolacek at gcc dot gnu.org]

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

Marek Polacek <mpolacek at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|NEW                         |ASSIGNED
           Assignee|unassigned at gcc dot gnu.org      |mpolacek at gcc dot gnu.org
           Priority|P3                          |P2

[Bug c++/116369] [12/13/14/15 Regression] temporary variable bounded to const reference with mutable field incorrectly marked as rodata since r9-869-g5603790dbf233c

[cvs-commit at gcc dot gnu.org]

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

--- Comment #12 from GCC Commits <cvs-commit at gcc dot gnu.org> ---
The trunk branch has been updated by Marek Polacek <mpolacek@gcc.gnu.org>:

https://gcc.gnu.org/g:2801a49d1144bce5568b527d1972952ad3420f66

commit r15-3572-g2801a49d1144bce5568b527d1972952ad3420f66
Author: Marek Polacek <polacek@redhat.com>
Date:   Thu Aug 29 15:13:03 2024 -0400

    c++: mutable temps in rodata [PR116369]

    Here we wrongly mark the reference temporary for g TREE_READONLY,
    so it's put in .rodata and so we can't modify its subobject even
    when the subobject is marked mutable.  This is so since r9-869.
    r14-1785 fixed a similar problem, but not in set_up_extended_ref_temp.

            PR c++/116369

    gcc/cp/ChangeLog:

            * call.cc (set_up_extended_ref_temp): Don't mark a temporary
            TREE_READONLY if its type is TYPE_HAS_MUTABLE_P.

    gcc/testsuite/ChangeLog:

            * g++.dg/tree-ssa/initlist-opt7.C: New test.

[Bug c++/116369] [12/13/14 Regression] temporary variable bounded to const reference with mutable field incorrectly marked as rodata since r9-869-g5603790dbf233c

[mpolacek at gcc dot gnu.org]

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

Marek Polacek <mpolacek at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
            Summary|[12/13/14/15 Regression]    |[12/13/14 Regression]
                   |temporary variable bounded  |temporary variable bounded
                   |to const reference with     |to const reference with
                   |mutable field incorrectly   |mutable field incorrectly
                   |marked as rodata since      |marked as rodata since
                   |r9-869-g5603790dbf233c      |r9-869-g5603790dbf233c

--- Comment #13 from Marek Polacek <mpolacek at gcc dot gnu.org> ---
Fixed on trunk so far.