-
Notifications
You must be signed in to change notification settings - Fork 37.8k
vector subscript out of range #4239
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
when i debug it using VC, it will report "Expression: vector subscript out of range".
This change is not correct. |
Automatic sanity-testing: FAILED BUILD/TEST, see http://jenkins.bluematt.me/pull-tester/34cb1fc5d6c7293b5c135b85c3dd3668e05ac2b8 for binaries and test log. This could happen for one of several reasons:
If you believe this to be in error, please ping BlueMatt on freenode or TheBlueMatt here. This test script verifies pulls every time they are updated. It, however, dies sometimes and fails to test properly. If you are waiting on a test, please check timestamps to verify that the test.log is moving at http://jenkins.bluematt.me/pull-tester/current/ |
Shouldn't the correct fix be to still use &compr[compr.size()-1] while change CFlatData to use inclusive upper bound? By definition compr[compr.size()] is a out of bound read even though it may not cause noticeable errors in certain implementations. |
Inclusive upper bound is incredibly rare in computer science. Would be extremely confusing. It could be considered to pass a pointer and a size though. Note that taking the address of compr[compr.size()] is not an out-of-bounds access in itself. The memory is never accessed. |
Would using compr.end() work? It returns an iterator pointing to the same address as &compr[compr.size()]. |
Good point, I think compr.end() works the best in terms of satisfying the runtime as well as increasing readability. I personally don't have preference between inclusive and exclusive upper bounds, none of them is more confusing than the other :) |
The foremost problem with inclusive upper bounds is that they also require a record/type size. An exclusive upper bound is unambiguously the top of the address range encompassing the objects. |
Using .end() makes sense here. But you'd still need something like Seemingly, getting the beginning and end pointer of a vector is not trivial to do in a way that is guaranteed to be portable and guaranteed to be working for empty vectors. See also http://stackoverflow.com/a/1339767/216357 which introduces |
ok, i will try! |
Why not use pointer arithmetic? This would prevent all warning messages, produce the same results as it currently is, and look purposeful (since it is).
|
@brandondahler Did you check the link I gave? template <class T, class TAl>
inline T* begin_ptr(std::vector<T,TAl>& v)
{return v.empty() ? NULL : &v[0];}
template <class T, class TAl>
inline const T* begin_ptr(const std::vector<T,TAl>& v)
{return v.empty() ? NULL : &v[0];}
template <class T, class TAl>
inline T* end_ptr(std::vector<T,TAl>& v)
{return v.empty() ? NULL : (begin_ptr(v) + v.size());}
template <class T, class TAl>
inline const T* end_ptr(const std::vector<T,TAl>& v)
{return v.empty() ? NULL : (begin_ptr(v) + v.size());} |
`&vch[vch.size()]` and even `&vch[0]` on vectors can cause assertion errors with VC in debug mode. This is the problem mentioned in bitcoin#4239. The deeper problem with this is that we rely on undefined behavior. - Add `begin_ptr` and `end_ptr` functions that get the beginning and end pointer of vector in a reliable way that copes with empty vectors and doesn't reference outside the vector (see https://stackoverflow.com/questions/1339470/how-to-get-the-address-of-the-stdvector-buffer-start-most-elegantly/1339767#1339767). - Add a convenience constructor to CFlatData that wraps a vector. I added `begin_ptr` and `end_ptr` as separate functions as I imagine they will be useful in more places.
`&vch[vch.size()]` and even `&vch[0]` on vectors can cause assertion errors with VC in debug mode. This is the problem mentioned in bitcoin#4239. The deeper problem with this is that we rely on undefined behavior. - Add `begin_ptr` and `end_ptr` functions that get the beginning and end pointer of vector in a reliable way that copes with empty vectors and doesn't reference outside the vector (see https://stackoverflow.com/questions/1339470/how-to-get-the-address-of-the-stdvector-buffer-start-most-elegantly/1339767#1339767). - Add a convenience constructor to CFlatData that wraps a vector. I added `begin_ptr` and `end_ptr` as separate functions as I imagine they will be useful in more places.
`&vch[vch.size()]` and even `&vch[0]` on vectors can cause assertion errors with VC in debug mode. This is the problem mentioned in bitcoin#4239. The deeper problem with this is that we rely on undefined behavior. - Add `begin_ptr` and `end_ptr` functions that get the beginning and end pointer of vector in a reliable way that copes with empty vectors and doesn't reference outside the vector (see https://stackoverflow.com/questions/1339470/how-to-get-the-address-of-the-stdvector-buffer-start-most-elegantly/1339767#1339767). - Add a convenience constructor to CFlatData that wraps a vector. I added `begin_ptr` and `end_ptr` as separate functions as I imagine they will be useful in more places. (cherry picked from commit fa126ef)
when i debug it using VC, it will report "Expression: vector subscript out of range".