[SOLVED]CXTPHexEdit crash when replacing Data buff |
Post Reply |
Author | |
grumat
Newbie Joined: 02 September 2021 Location: Austria Status: Offline Points: 4 |
Post Options
Thanks(0)
Posted: 02 September 2021 at 4:19am |
Hi, I had a interesting scenario here:
Reload Data does actually:
Problem is: The first command shrinks the buffer to 0. The current implementation of CXTPHexEdit::SetData() does:
Issue: SetSel causes repaint, and OnDraw ends up accessing invalid memory because buffer bounds refers to the old buffer size. My current fix was to command: ctrl..m_nLength = 0; // OMG! access to internal stuff!! before the new SetData(). Surely that a definitive fix will needs more accuracy and tests, but hoping to see it on the next release. Best regards, M. Gruber
|
|
agontarenko
Admin Group Joined: 25 March 2016 Status: Offline Points: 294 |
Post Options
Thanks(0)
|
Hello,
Unfortunately it is not clear what exactly is going wrong. Can you please provide more detailed information. If it cannot be re-produced in any of our sample applications provided I would appreciate you sending a sample application so that we could debug it. Regards, Artem Gontarenko |
|
grumat
Newbie Joined: 02 September 2021 Location: Austria Status: Offline Points: 4 |
Post Options
Thanks(0)
|
Just see your source code: XTPHexEdit.cpp: void CXTPHexEdit::SetData(LPBYTE p, int nLength, int nMaxLength){ if (m_pData) free(m_pData); m_pData = (LPBYTE)malloc(nLength); MEMCPY_S(m_pData, p, nLength); SetSel(-1, -1); m_nMaxLength = nMaxLength < 0 ? -1 : __max(nMaxLength, nLength); m_nLength = nLength; ... See the bold lines: You free the old buffer, then allocates the new buffer with the new length. Before updating the new buffer bound (m_nLength) you issue a SetSel, which "may" cause a WM_PAINT. void CXTPHexEdit::SetSel(int nSelStart, int nSelEnd) { ... RedrawWindow(); ... }The OnDraw checks m_nLength to keep access within the buffer bounds, but since the order of the previous code has a flaw, the m_nLength is not valid for the new buffer. This is critical if the new size of the buffer decreases. The use case requires an initial "big buffer" and also that we "scroll to the end of that buffer". So when a tiny new buffer is newly applied, it causes that all indexes for the current scroll position are outside of the new buffer bound, which will cause a CPU exception in the OnDraw: void CXTPHexEdit::OnDraw(CDC* pDC) ... if (m_pData) ... if (m_bShowHex) ... if (m_nSelStart != -1 && (m_eEditMode == editHigh || m_eEditMode == editLow)) ... else ... for (i = m_nTopIndex; (i < m_nLength) && (rcd.TopLeft().y < height);) ... TOHEX(m_pData[ i ], p);This is the flow up to the access exception. Although m_nLength correctly guards the array bounds for the initial state, it actually doesn't during the SetData, because it refers to the "old control state". Note: during my tests I had situations where a WM_PAINT did not happen (maybe Z-ordering), so the bug did not happen. Also CPU memory protection has a granularity with a minimum block size (4K, AFAIK), so the invalid access may not cause the exception in a 100% reproducible case. But the logic above has a clear flaw. Hope this helps.
|
|
agontarenko
Admin Group Joined: 25 March 2016 Status: Offline Points: 294 |
Post Options
Thanks(0)
|
Hello,
I understood that you have problem. For a solution, I need to reproduce this bug. Regards,Can you change HexEdit sample that the problem is reproducible, and attach it to topic. Also, in v20.0 or 20.1 has been some changes, may be this problem already fixed. Artem Gontarenko |
|
grumat
Newbie Joined: 02 September 2021 Location: Austria Status: Offline Points: 4 |
Post Options
Thanks(0)
|
As you don't seem to understand: I have a fix for your bug and I am sharing it to you. There are enough information for a regular experienced programmer understand and fix the issue. If you are not interested in the "continuous product improvement", then please close the issue. I won't annoy you anymore.
|
|
agontarenko
Admin Group Joined: 25 March 2016 Status: Offline Points: 294 |
Post Options
Thanks(0)
|
Ok, I changed our code. You right. void CXTPHexEdit::SetData(LPBYTE p, int nLength, int nMaxLength) { if (m_pData) { free(m_pData); m_nLength = 0; } Thanks.
|
|
Post Reply | |
Tweet
|
Forum Jump | Forum Permissions You cannot post new topics in this forum You cannot reply to topics in this forum You cannot delete your posts in this forum You cannot edit your posts in this forum You cannot create polls in this forum You cannot vote in polls in this forum |