I can't help thinking that is not enough for the C++ critics: they want it to be impossible to do badly. |
cppreference wrote: |
---|
Notes views::counted does not check if the range is long enough to provide all count elements: use views::take if that check is necessary. |
cppreferenceOutput wrote: |
---|
Output: 1 2 3 4 5 6 7 0 181290128 32764 2 3 4 5 0 4196509 0 1 2 3 |
seeplus wrote: |
---|
The philosophy behind c and C++ is to be run-time efficient. Thus things like bounds checking etc aren't performed as they are in other languages. The basic assumption with c/C++ is that the programmer knows what they are doing so let them get on with it. |
it might be changing to prefer code that is safe by default |
3) [...] n is not equal to ranges::distance(i, s) explicitly converted to its type. 5) [...] n is not equal to ranges::distance(ranges::begin(r), ranges::end(r)) explicitly converted to its type. |
Peter87 wrote: |
---|
Since views::counted takes only an iterator there is no way to bounds check that without knowing what the "full range" is. |
views::counted
does take a count, see the example on cppreference. IMO the count n
is the whole idea of a counted range. I gather that it works by adding n
to the iterator and seeing if the result is valid. I mentioned earlier that I altered the example to be out of bounds with a count that was too large, it did produce warnings. That is all fine, but it does produce erroneous output.Peter87 wrote: |
---|
There is generally no way to check if two iterators is a valid range. You just have to assume. |
I
is some iterator between begin()
and end()
, and n
is the unsigned
count, then std::distance(begin, I + n)
would have to <= to std::distance(begin, end)
, otherwise I + n
is invalid. Maybe I am misunderstanding that somehow, I haven't written any code to test that.Peter87 wrote: |
---|
I don't fully understand the purpose of n. It seems like you don't need to supply it. So maybe the purpose is just for optimization purposes? If you do supply it it needs to be correct. If it would have to verify that it was correct it would probably defeat its purpose. |
n
is an argument for the constructors 3 and 5, it doesn't seem to be defaulted. I think the purpose of ranges::subrange is also to be able to specify a new range somewhere inside an existing range, with a particular size. And it does seem to check if that spec is invalid, and produces a warning.seeplus wrote: |
---|
It's certainly the case that more compile-time checks are implemented - which is a good thing. The more issues found at compile time the better - even at the cost of slower compile times. However I contend that for c/c++ run-time safety is a programmer issue and not for the compiler. Although I would not disagree with more run-time checks when code is compiled as a 'debug' build, but 'release' build shouldn't have these IMO. |
TheIdeasMan wrote: |
---|
views::counted does take a count, see the example on cppreference. IMO the count n is the whole idea of a counted range. |
TheIdeasMan wrote: |
---|
I gather that it works by adding n to the iterator and seeing if the result is valid. |
TheIdeasMan wrote: |
---|
If I is some iterator between begin() and end() , and n is the unsigned count, then std::distance(begin, I + n) would have to <= to std::distance(begin, end) , otherwise I + n is invalid. |
TheIdeasMan wrote: |
---|
n is an argument for the constructors 3 and 5, it doesn't seem to be defaulted. |
TheIdeasMan wrote: |
---|
With the cost of doing checks, It seems to me that these checks are at compile time, done once, and are only simple pointer arithmetic internally (std::distance and the like). |
Peter87 wrote: |
---|
It's cheap for random-access iterators like std::vector<T>::iterator but for something like std::list::iterator it will have to call the increment operator until the first iterator reaches the second iterator which means it's an O(n) operation. |
cppreference wrote: |
---|
2) Constructs a subrange from an iterator-sentinel pair. Initializes the stored iterator and sentinel with std::move(i) and s respectively. The behavior is undefined if [i, s) is not a valid range. |
jonnin wrote: |
---|
as long as the STL uses pointers, its going to be possible to UB it, is my take on all this. A simplified answer, sure, but out of bounds (array or pointer either one, same difference) or pointer fubars are easy to do. .... |
HerbSutter wrote: |
---|
The immediate problem “is” that it’s Too Easy By Default™ to write security and safety vulnerabilities in C++ that would have been caught by stricter enforcement of known rules for type, bounds, initialization, and lifetime language safety |
std::distance
et al is shown in the implementation of some of the constructors and elsewhere. Do they happen at runtime? I am in the middle of writing some code to investigate these things.
TheIdeasMan wrote: |
---|
However, it seems able to do the check, cppreference states: >2) Constructs a subrange from an iterator-sentinel pair. Initializes the stored iterator and sentinel with std::move(i) and s respectively. The behavior is undefined if [i, s) is not a valid range. Emphasis mine. |
TheIdeasMan wrote: |
---|
With cost, maybe it is worth it, especially at compile time, if the iterator/range is invalid to start with? And it does do it because it produces a compiler warning. |
TheIdeasMan wrote: |
---|
All this discussion so far has been about compile time UB, in my mind that is a problem because a program could have a const container. |
jonnin wrote: |
---|
as long as the STL uses pointers, its going to be possible to UB it, is my take on all this. |
Peter87 wrote: |
---|
But the compiler warning is from the compiler, right? It's not the subrange code that is generating it. ... I think this is about "quality of implementation" (QoI). Compilers might warn about these things if they can but it's not required by the standard. |
Peter87 wrote: |
---|
And by "compile time UB" you mean code that can be proved, at compile-time, to generate UB if it was executed? |
|
|
/opt/compiler-explorer/gcc-13.2.0/include/c++/13.2.0/debug/safe_iterator.h:934: In function: gnu_debug::_Safe_iterator<gnu_cxx::normal_iterator<int*, std:: vector<int, std::allocator<int> > >, std::debug::vector<int>, std::random_access_iterator_tag>::_Self gnu_debug::operator+(const _Safe_iterator<gnu_cxx::normal_iterator<int*, std::vector<int, std::allocator<int> > >, std::debug::vector<int>, std::random_access_iterator_tag>::_Self&, _Safe_iterator<gnu_cxx:: normal_iterator<int*, std::vector<int, std::allocator<int> > >, std:: debug::vector<int>, std::random_access_iterator_tag>::difference_type) Error: attempt to advance a dereferenceable (start-of-sequence) iterator 10 steps, which falls outside its valid range. Objects involved in the operation: iterator @ 0x7ffd42e688f0 { type = gnu_cxx::normal_iterator<int*, std::vector<int, std::allocator<int> > > (mutable iterator); state = dereferenceable (start-of-sequence); references sequence with type 'std::debug::vector<int, std::allocator<int> >' @ 0x7ffd42e688b0 } Program terminated with signal: SIGSEGV |
================================================================= ==1==ERROR: AddressSanitizer: heap-buffer-overflow on address 0x60300000007c at pc 0x000000402e96 bp 0x7fff5e9546b0 sp 0x7fff5e9546a8 READ of size 4 at 0x60300000007c thread T0 #0 0x402e95 in main /app/example.cpp:37 #1 0x7a23c8229d8f (/lib/x86_64-linux-gnu/libc.so.6+0x29d8f) (BuildId: 962015aa9d133c6cbcfb31ec300596d7f44d3348) #2 0x7a23c8229e3f in __libc_start_main (/lib/x86_64-linux-gnu/libc.so.6+0x29e3f) (BuildId: 962015aa9d133c6cbcfb31ec300596d7f44d3348) #3 0x402254 in _start (/app/output.s+0x402254) (BuildId: cdf900c382eefa53660f7474e41ea8f9a90ad251) 0x60300000007c is located 28 bytes after 32-byte region [0x603000000040,0x603000000060) allocated by thread T0 here: #0 0x7a23c8f50b78 in operator new(unsigned long) (/opt/compiler-explorer/gcc-13.2.0/lib64/libasan.so.8+0xdcb78) (BuildId: 53df075b42b04e0fd573977feeb6ac6e330cfaaa) #1 0x407bf3 in std::__new_allocator<int>::allocate(unsigned long, void const*) /opt/compiler-explorer/gcc-13.2.0/include/c++/13.2.0/bits/new_allocator.h:147 #2 0x40737f in std::allocator<int>::allocate(unsigned long) /opt/compiler-explorer/gcc-13.2.0/include/c++/13.2.0/bits/allocator.h:198 #3 0x40737f in std::allocator_traits<std::allocator<int> >::allocate(std::allocator<int>&, unsigned long) /opt/compiler-explorer/gcc-13.2.0/include/c++/13.2.0/bits/alloc_traits.h:482 #4 0x40737f in std::_Vector_base<int, std::allocator<int> >::_M_allocate(unsigned long) /opt/compiler-explorer/gcc-13.2.0/include/c++/13.2.0/bits/stl_vector.h:378 #5 0x4066d9 in void std::vector<int, std::allocator<int> >::_M_realloc_insert<int const&>(__gnu_cxx::__normal_iterator<int*, std::vector<int, std::allocator<int> > >, int const&) /opt/compiler-explorer/gcc-13.2.0/include/c++/13.2.0/bits/vector.tcc:459 #6 0x405157 in std::vector<int, std::allocator<int> >::push_back(int const&) /opt/compiler-explorer/gcc-13.2.0/include/c++/13.2.0/bits/stl_vector.h:1289 #7 0x4026f7 in main /app/example.cpp:22 #8 0x7a23c8229d8f (/lib/x86_64-linux-gnu/libc.so.6+0x29d8f) (BuildId: 962015aa9d133c6cbcfb31ec300596d7f44d3348) SUMMARY: AddressSanitizer: heap-buffer-overflow /app/example.cpp:37 in main Shadow bytes around the buggy address: 0x602ffffffd80: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 0x602ffffffe00: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 0x602ffffffe80: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 0x602fffffff00: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 0x602fffffff80: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 =>0x603000000000: fa fa 00 00 00 fa fa fa 00 00 00 00 fa fa fa[fa] 0x603000000080: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa 0x603000000100: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa 0x603000000180: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa 0x603000000200: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa 0x603000000280: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa Shadow byte legend (one shadow byte represents 8 application bytes): Addressable: 00 Partially addressable: 01 02 03 04 05 06 07 Heap left redzone: fa Freed heap region: fd Stack left redzone: f1 Stack mid redzone: f2 Stack right redzone: f3 Stack after return: f5 Stack use after scope: f8 Global redzone: f9 Global init order: f6 Poisoned by user: f7 Container overflow: fc Array cookie: ac Intra object redzone: bb ASan internal: fe Left alloca redzone: ca Right alloca redzone: cb ==1==ABORTING |
==2037== Invalid read of size 4 ==2037== at 0x10948F: main (test.cpp:37) ==2037== Address 0x4d7e1ec is 28 bytes after a block of size 32 in arena "client" |
std::ranges::take_view
does the right thing, why can't the others do it too?HerbSutter wrote: |
---|
The immediate problem “is” that it’s Too Easy By Default™ to write security and safety vulnerabilities in C++ that would have been caught by stricter enforcement of known rules for type, bounds, initialization, and lifetime language safety |