Hello, On 23/12/2024 17:55, Patrick Palka wrote:
Just to reiterate: if the projection returns an rvalue, it would be wrong for the predicate to actually move from it, as it would violate the equality-preserving semantic requirements of regular_invocable? I'll add the missing forward then.IIUC yes. I just opened https://gcc.gnu.org/PR118185 to track a similar missing forward in our ranges::clamp implementation.
Ok. I've added a mention to https://gcc.gnu.org/bugzilla/show_bug.cgi?id=100249 in the changelog, as it seems relevant.
+static_assert(test04);Missing (), otherwise LGTM!
D'oh. Fourth time the charm... -- Giuseppe D'Angelo
From 5d5e7186b927ef85f9e360d3bf9d2a98b9133251 Mon Sep 17 00:00:00 2001
From: Giuseppe D'Angelo <giuseppe.dang...@kdab.com>
Date: Fri, 20 Dec 2024 12:55:01 +0100
Subject: [PATCH] libstdc++: fix a dangling reference crash in
ranges::is_permutation [PR118160]
The code was caching the result of `invoke(proj, *it)` in a local
`auto &&` variable. The problem is that this may create dangling
references, for instance in case `proj` is `std::identity` (the common
case) and `*it` produces a prvalue: lifetime extension does not
apply here due to the expressions involved.
Instead, store (and lifetime-extend) the result of `*it` in a separate
variable, then project that variable. While at it, also forward the
result of the projection to the predicate, so that the predicate can
act on the proper value category.
libstdc++-v3/ChangeLog:
PR libstdc++/118160
PR libstdc++/100249
* include/bits/ranges_algo.h (__is_permutation_fn): Avoid a
dangling reference by storing the result of the iterator
dereference and the result of the projection in two distinct
variables, in order to lifetime-extend each one.
Forward the projected value to the predicate.
* testsuite/25_algorithms/is_permutation/constrained.cc: Add a
test with a range returning prvalues. Test it in a constexpr
context, in order to rely on the compiler to catch UB.
Signed-off-by: Giuseppe D'Angelo <giuseppe.dang...@kdab.com>
---
libstdc++-v3/include/bits/ranges_algo.h | 7 +++++--
.../25_algorithms/is_permutation/constrained.cc | 13 +++++++++++++
2 files changed, 18 insertions(+), 2 deletions(-)
diff --git a/libstdc++-v3/include/bits/ranges_algo.h b/libstdc++-v3/include/bits/ranges_algo.h
index 772bf4dd997..80d8123443f 100644
--- a/libstdc++-v3/include/bits/ranges_algo.h
+++ b/libstdc++-v3/include/bits/ranges_algo.h
@@ -567,9 +567,12 @@ namespace ranges
for (auto __scan = __first1; __scan != __last1; ++__scan)
{
- auto&& __proj_scan = std::__invoke(__proj1, *__scan);
+ auto&& __scan_deref = *__scan;
+ auto&& __proj_scan =
+ std::__invoke(__proj1, std::forward<decltype(__scan_deref)>(__scan_deref));
auto __comp_scan = [&] <typename _Tp> (_Tp&& __arg) -> bool {
- return std::__invoke(__pred, __proj_scan,
+ return std::__invoke(__pred,
+ std::forward<decltype(__proj_scan)>(__proj_scan),
std::forward<_Tp>(__arg));
};
if (__scan != ranges::find_if(__first1, __scan,
diff --git a/libstdc++-v3/testsuite/25_algorithms/is_permutation/constrained.cc b/libstdc++-v3/testsuite/25_algorithms/is_permutation/constrained.cc
index 2fbebe37609..7266aff5b17 100644
--- a/libstdc++-v3/testsuite/25_algorithms/is_permutation/constrained.cc
+++ b/libstdc++-v3/testsuite/25_algorithms/is_permutation/constrained.cc
@@ -19,6 +19,7 @@
#include <algorithm>
#include <iterator>
+#include <ranges>
#include <testsuite_hooks.h>
#include <testsuite_iterators.h>
@@ -76,10 +77,22 @@ test03()
while (std::next_permutation(std::begin(cx), std::end(cx)));
}
+constexpr
+bool
+test04() // PR118160, do not create dangling references
+{
+ int x[] = { 4, 3, 2, 1 };
+ auto y = std::views::iota(1, 5);
+ return ranges::is_permutation(x, y) && ranges::is_permutation(y, x);
+}
+
+static_assert(test04());
+
int
main()
{
test01();
test02();
test03();
+ VERIFY( test04() );
}
--
2.34.1
smime.p7s
Description: S/MIME Cryptographic Signature
