Issue |
130213
|
Summary |
[clang-tidy] Check request: boost-avoid-dangling-in-any-range-and-transformed-range-cooperation
|
Labels |
clang-tidy
|
Assignees |
|
Reporter |
denzor200
|
Look at the snippet(you could play with the snippet [here](https://godbolt.org/z/ojPbs9vYf)):
```
#include <boost/range/adaptors.hpp>
#include <boost/range/any_range.hpp>
#include <vector>
#include <iostream>
using StringRange = boost::any_range<
std::string,
boost::forward_traversal_tag,
const std::string&,
std::ptrdiff_t
>;
StringRange create_view_to_vector(const std::vector<std::string>& vec) {
auto identity_func = [](const std::string& x) {
return x;
};
auto view_to_vector = vec | boost::adaptors::transformed(identity_func);
return StringRange(view_to_vector);
}
int main() {
std::vector<std::string> vec = {"first", "second", "third", "fourth", "fifth"};
auto range = create_view_to_vector(vec);
// Attempt to use range
for (const std::string& str : range) {
std::cout << str << " "; // BAD - UB reached
}
}
```
For the first seeing this code looks correct and innocent. But reality doesn't spare us and this code contains insidious **undefined behaviour UB** which related to `boost::any_range` and `boost::adaptors::transformed` using. So, lets figure out what's wrong here.
1. The lambda `identity_func` returns a copy of `std::string`, not a reference:
```
auto identity_func = [](const std::string& x) {
return x; // Here we return copy of a string
};
```
2. But `boost::any_range` in the mood for store a reference(`const std::string&`):
```
using IntRange = boost::any_range<
std::string,
boost::forward_traversal_tag,
const std::string&, // Here we "store" a reference
std::ptrdiff_t
>;
```
It leads to `boost::any_range` attempts to store a reference to a temporary object which deletes just after lambda's evaluation. When `view_to_vector` returns a value it's always a temporary object. That object deletes just after being used, but `boost::any_range` attemps to store a reference to it. It leads to dangling references.
### How does the problem appear itself?
In the `for` loop:
```
for (const std::string& str : range) {
std::cout << str << " ";
}
```
the variable x will refer to temporary objects that have already been destroyed. This leads to UB, and the program may output garbage, crash, or behave unpredictably.
### Fixing
To fix the problem, we need to reconcile the types. There are two options:
1. Use `boost::any_range` that stores values.
Let's change using of `boost::any_range` so that it stores values (`std::string`) instead of references:
```
#include <boost/range/adaptors.hpp>
#include <boost/range/any_range.hpp>
#include <vector>
#include <iostream>
using StringRange = boost::any_range<
std::string,
boost::forward_traversal_tag,
std::string, // Here we "store" values
std::ptrdiff_t
>;
StringRange create_view_to_vector(const std::vector<std::string>& vec) {
auto identity_func = [](const std::string& x) {
return x;
};
auto view_to_vector = vec | boost::adaptors::transformed(identity_func);
return StringRange(view_to_vector);
}
int main() {
std::vector<std::string> vec = {"first", "second", "third", "fourth", "fifth"};
auto range = create_view_to_vector(vec);
// Attempt to use range
for (const std::string& str : range) {
std::cout << str << " "; // OK - first second third fourth fiveth
}
}
```
Now `boost::any_range` stores values, and the dangling reference problem disappears.
2. Use references in lambda.
If you want `boost::any_range` to store references, the lambda should return references to objects that live longer than `boost::any_range`. For example:
```
#include <boost/range/adaptors.hpp>
#include <boost/range/any_range.hpp>
#include <vector>
#include <iostream>
using StringRange = boost::any_range<
std::string,
boost::forward_traversal_tag,
const std::string&,
std::ptrdiff_t
>;
StringRange create_view_to_vector(const std::vector<std::string>& vec) {
auto identity_func = [](const std::string& x) -> const std::string& {
return x; // Here we return a reference for an element of the vector
};
auto view_to_vector = vec | boost::adaptors::transformed(identity_func);
return StringRange(view_to_vector);
}
int main() {
std::vector<std::string> vec = {"first", "second", "third", "fourth", "fifth"};
auto range = create_view_to_vector(vec);
// Attempt to use range
for (const std::string& str : range) {
std::cout << str << " "; // OK - first second third fourth fiveth
}
}
```
Here the lambda returns references to the elements of the original vector vec, which lives longer than `boost::any_range`. This is safe.
### Resume
Needs a check that will find the problem described above in C++ code and point it out. I believe it will not be a hard task to detect it automatically(it looks like a really simple Clang Tidy check), but believe my words it's impossible to detect by simple reading - only hours in a flaky debugging :)
I suppose this check will not suggest a fixin hint(or at least 100% safe and simple suggesting to force ` boost::any_range` to store values).
_______________________________________________
llvm-bugs mailing list
llvm-bugs@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-bugs