Skip to content

Conversation

laanwj
Copy link
Member

@laanwj laanwj commented Jun 5, 2014

&vch[vch.size()] and even &vch[0] on vectors can cause assertion errors with VC in debug mode. This is the problem mentioned in #4239. The deeper problem with this is that we rely on undefined behavior.

I added begin_ptr and end_ptr as separate functions as I imagine they will be useful in more places.

@LongShao007 this was your issue, can you help test?

@LongShao007
Copy link
Contributor

The test is passed.
I copy the files ( script.h and serialize.h ) to my VC project, modify the code, do the white testing, debug the program many times, don't cause assertion errors ( vector subscript out of range ) .

@laanwj
Copy link
Member Author

laanwj commented Jun 10, 2014

Thanks for testing @LongShao007

@laanwj laanwj added this to the 0.10.0 milestone Jun 19, 2014
@petertodd
Copy link
Contributor

ACK

@sipa
Copy link
Member

sipa commented Jun 21, 2014

Untested ACK.

@BitcoinPullTester
Copy link

Automatic sanity-testing: PASSED, see http://jenkins.bluematt.me/pull-tester/p4293_2ede43ebe0e87f982747feaec62aeefaf12c3a41/ for binaries and test log.
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/
Contact BlueMatt on freenode if something looks broken.

`&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.
@laanwj laanwj merged commit fa126ef into bitcoin:master Jun 23, 2014
laanwj added a commit that referenced this pull request Jun 23, 2014
fa126ef Avoid undefined behavior using CFlatData in CScript serialization (Wladimir J. van der Laan)
@bitcoin bitcoin locked as resolved and limited conversation to collaborators Sep 8, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants