public inbox for gcc-bugs@sourceware.org
help / color / mirror / Atom feed
* [Bug libstdc++/102064] New: Wrong assignable check in uninitialized_fill and uninitialized_fill_n
@ 2021-08-25 12:10 redi at gcc dot gnu.org
  2021-08-25 12:10 ` [Bug libstdc++/102064] " redi at gcc dot gnu.org
                   ` (2 more replies)
  0 siblings, 3 replies; 4+ messages in thread
From: redi at gcc dot gnu.org @ 2021-08-25 12:10 UTC (permalink / raw)
  To: gcc-bugs

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

            Bug ID: 102064
           Summary: Wrong assignable check in uninitialized_fill and
                    uninitialized_fill_n
           Product: gcc
           Version: 12.0
            Status: UNCONFIRMED
          Keywords: rejects-valid
          Severity: normal
          Priority: P3
         Component: libstdc++
          Assignee: unassigned at gcc dot gnu.org
          Reporter: redi at gcc dot gnu.org
  Target Milestone: ---

In r204615 I added assignable checks to various algos to fix PR
libstdc++/58982.

The checks in uninitialized_fill and uninitialized_fill_n use
is_copy_assignable, which is wrong because the value being filled doesn't have
to be the value type of the output range.

This valid code fails to compile with libstdc++:

#include <memory>
#include <algorithm>

struct X
{
  X() = default;
  X(int) { }
  X(const X&) = default;
  X& operator=(const X&) = default;
  X& operator=(int) = delete;
};

static_assert( std::is_trivial<X>::value, "" );

int main()
{
  unsigned char buf[sizeof(X)];
  std::uninitialized_fill_n((X*)buf, 1, 99);
}


/usr/include/c++/11/bits/stl_algobase.h:924:18: error: use of deleted function
‘X& X::operator=(int)’
  924 |         *__first = __tmp;
      |         ~~~~~~~~~^~~~~~~

The problem is that we decide it's OK to use std::fill_n because X is trivial
and is_copy_assignable<X> is true, but then std::fill_n assigns int to X.

We need to check is_assignable<X&, const int&> instead.

^ permalink raw reply	[flat|nested] 4+ messages in thread

* [Bug libstdc++/102064] Wrong assignable check in uninitialized_fill and uninitialized_fill_n
  2021-08-25 12:10 [Bug libstdc++/102064] New: Wrong assignable check in uninitialized_fill and uninitialized_fill_n redi at gcc dot gnu.org
@ 2021-08-25 12:10 ` redi at gcc dot gnu.org
  2021-08-25 13:25 ` redi at gcc dot gnu.org
  2021-08-25 21:29 ` cvs-commit at gcc dot gnu.org
  2 siblings, 0 replies; 4+ messages in thread
From: redi at gcc dot gnu.org @ 2021-08-25 12:10 UTC (permalink / raw)
  To: gcc-bugs

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

Jonathan Wakely <redi at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
   Last reconfirmed|                            |2021-08-25
             Status|UNCONFIRMED                 |ASSIGNED
     Ever confirmed|0                           |1
           Assignee|unassigned at gcc dot gnu.org      |redi at gcc dot gnu.org

^ permalink raw reply	[flat|nested] 4+ messages in thread

* [Bug libstdc++/102064] Wrong assignable check in uninitialized_fill and uninitialized_fill_n
  2021-08-25 12:10 [Bug libstdc++/102064] New: Wrong assignable check in uninitialized_fill and uninitialized_fill_n redi at gcc dot gnu.org
  2021-08-25 12:10 ` [Bug libstdc++/102064] " redi at gcc dot gnu.org
@ 2021-08-25 13:25 ` redi at gcc dot gnu.org
  2021-08-25 21:29 ` cvs-commit at gcc dot gnu.org
  2 siblings, 0 replies; 4+ messages in thread
From: redi at gcc dot gnu.org @ 2021-08-25 13:25 UTC (permalink / raw)
  To: gcc-bugs

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

--- Comment #1 from Jonathan Wakely <redi at gcc dot gnu.org> ---
The assignable checks are also missing for C++98, but there are types which can
be constructed via std::uninitialized_foo that cannot be assigned to by
std::foo

#include <memory>
#include <algorithm>

struct X;

struct Y
{
  operator X() const;
};

struct X
{
private:
  void operator=(const Y&);
};

Y::operator X() const { return X(); }

int main()
{
  unsigned char buf[sizeof(X)];
  Y y;
  std::uninitialized_fill_n((X*)buf, 1, y);
}

This should copy-construct an X from the result of the conversion operator. But
we optimize it to std::fill_n which tries to use the inaccessible assignment.

/usr/include/c++/11/bits/stl_algobase.h:912:18: error: ‘void X::operator=(const
Y&)’ is private within this context
  912 |         *__first = __value;
      |         ~~~~~~~~~^~~~~~~~~

^ permalink raw reply	[flat|nested] 4+ messages in thread

* [Bug libstdc++/102064] Wrong assignable check in uninitialized_fill and uninitialized_fill_n
  2021-08-25 12:10 [Bug libstdc++/102064] New: Wrong assignable check in uninitialized_fill and uninitialized_fill_n redi at gcc dot gnu.org
  2021-08-25 12:10 ` [Bug libstdc++/102064] " redi at gcc dot gnu.org
  2021-08-25 13:25 ` redi at gcc dot gnu.org
@ 2021-08-25 21:29 ` cvs-commit at gcc dot gnu.org
  2 siblings, 0 replies; 4+ messages in thread
From: cvs-commit at gcc dot gnu.org @ 2021-08-25 21:29 UTC (permalink / raw)
  To: gcc-bugs

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

--- Comment #2 from CVS 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:ead408529d7a69873a7c14dd12fa043cd5862253

commit r12-3147-gead408529d7a69873a7c14dd12fa043cd5862253
Author: Jonathan Wakely <jwakely@redhat.com>
Date:   Wed Aug 25 21:10:48 2021 +0100

    libstdc++: Fix conditions for optimizing uninitialized algos [PR102064]

    While laying some groundwork for constexpr std::vector, I noticed some
    bugs in the std::uninitialized_xxx algorithms. The conditions being
    checked for optimizing trivial cases were not quite right, as shown in
    the examples in the PR.

    This consolidates the checks into a single macro. The macro has
    appropriate definitions for C++98 or for later standards, to avoid a #if
    everywhere the checks are used. For C++11 and later the check makes a
    call to a new function doing a static_assert to ensure we don't use
    assignment in cases where construction would have been invalid.
    Extracting that check to a separate function will be useful for
    constexpr std::vector, as that can't use std::uninitialized_copy
    directly because it isn't constexpr).

    The consolidated checks mean that some slight variations in static
    assert message are gone, as there is only one place that does the assert
    now. That required adjusting some tests. As part of that the redundant
    89164_c++17.cc test was merged into 89164.cc which is compiled as C++17
    by default now, but can also use other -std options if the
    C++17-specific error is made conditional with a target selector.

    Signed-off-by: Jonathan Wakely <jwakely@redhat.com>

    libstdc++-v3/ChangeLog:

            PR libstdc++/102064
            * include/bits/stl_uninitialized.h (_GLIBCXX_USE_ASSIGN_FOR_INIT):
            Define macro to check conditions for optimizing trivial cases.
            (__check_constructible): New function to do static assert.
            (uninitialized_copy, uninitialized_fill, uninitialized_fill_n):
            Use new macro.
            * testsuite/20_util/specialized_algorithms/uninitialized_copy/1.cc:
            Adjust dg-error pattern.
            * testsuite/23_containers/vector/cons/89164.cc: Likewise. Add
            C++17-specific checks from 89164_c++17.cc.
            * testsuite/23_containers/vector/cons/89164_c++17.cc: Removed.
            *
testsuite/20_util/specialized_algorithms/uninitialized_copy/102064.cc:
            New test.
            *
testsuite/20_util/specialized_algorithms/uninitialized_copy_n/102064.cc:
            New test.
            *
testsuite/20_util/specialized_algorithms/uninitialized_fill/102064.cc:
            New test.
            *
testsuite/20_util/specialized_algorithms/uninitialized_fill_n/102064.cc:
            New test.

^ permalink raw reply	[flat|nested] 4+ messages in thread

end of thread, other threads:[~2021-08-25 21:29 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-08-25 12:10 [Bug libstdc++/102064] New: Wrong assignable check in uninitialized_fill and uninitialized_fill_n redi at gcc dot gnu.org
2021-08-25 12:10 ` [Bug libstdc++/102064] " redi at gcc dot gnu.org
2021-08-25 13:25 ` redi at gcc dot gnu.org
2021-08-25 21:29 ` cvs-commit at gcc dot gnu.org

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).