User forums > Nightly builds
The 08 February 2014 build (9639) is out.
ollydbg:
--- Quote from: osdt on February 12, 2014, 03:01:21 pm ---
--- Quote from: ollydbg on February 12, 2014, 02:48:24 pm ---@morten, revert rev9369 can avoid the hang issue, so I'm going to commit the change now. (This is a workaround, not a true fix).
--- End quote ---
@ollydbg: reverting r9638 fixes this issue too!
I can confirm that r9639 has this issue while r9619 works flawlessly. Between this revisions, CC-related changes are r9636 and r9638.
- osdt
--- End quote ---
Dear osdt, thanks for the reply. Rev9638 is a solid fix for rev 9601, because when in rev 9601, I leave a logic error.
See what rev9638 does:
--- Code: --- src/plugins/codecompletion/parser/tokenizer.cpp | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/src/plugins/codecompletion/parser/tokenizer.cpp b/src/plugins/codecompletion/parser/tokenizer.cpp
index 6761574..25b0a6d 100644
--- a/src/plugins/codecompletion/parser/tokenizer.cpp
+++ b/src/plugins/codecompletion/parser/tokenizer.cpp
@@ -1985,7 +1985,7 @@ bool Tokenizer::GetMacroExpendedText(const Token* tk, wxString& expandedText)
// sanity check if we have such macro definition that #define AAA(x,y) x+y+AAA
// which means a macro name is exists in its definition, which will cause a infinite expansion loop
- if (tk->m_FullType.Find(tk->m_Name) ==wxNOT_FOUND)
+ if (tk->m_FullType.Find(tk->m_Name) != wxNOT_FOUND)
return false;
// Now, tk is a function like macro definition we are going to expand, it's m_Args contains the
--- End code ---
You see, if you leave the if (tk->m_FullType.Find(tk->m_Name) ==wxNOT_FOUND) here, than you just disable the macro expansion feature now, so it should definitely be != for this sanity check.
ollydbg:
--- Quote from: ollydbg on February 12, 2014, 02:48:24 pm ---@morten, revert rev9369 can avoid the hang issue, so I'm going to commit the change now. (This is a workaround, not a true fix).
--- End quote ---
Done in r9647, sorry to all who have the hang issue.
osdt:
--- Quote from: ollydbg on February 12, 2014, 03:08:36 pm ---Dear osdt, thanks for the reply. Rev9638 is a solid fix for rev 9601, because when in rev 9601, I leave a logic error.
--- End quote ---
Ok. I was just bitsecting this issue and came across r9638.
-osdt
Huki:
--- Quote from: ollydbg on February 12, 2014, 03:51:52 am ---I debugged a little, I think it was a regression after a patch by Huki,
So, bug is here:
--- Code: --- // Check whether a ReplaceBufferText() was done in DoGetToken().
// We assume m_Undo... have already been reset in ReplaceBufferText().
if (m_IsReplaceParsing && savedReplaceCount != (int)m_RepeatReplaceCount)
{
m_TokenIndex = m_UndoTokenIndex; //*************bug here******************
m_LineNumber = m_UndoLineNumber;
m_NestLevel = m_UndoNestLevel;
}
--- End code ---
m_TokenIndex is always remain/reset to the same value, so Tokenizer won't go forward any more.
--- End quote ---
Are we dealing with really complex nested macros in wx3.0? I'm assuming that is the problem...
A quick explanation of my patch for PeekToken(): the goal is to reset m_TokenIndex every time a macro replacement occurs inside ReplaceBufferText(). So we use the test "if (m_IsReplaceParsing && savedReplaceCount != (int)m_RepeatReplaceCount)", to see if m_RepeatReplaceCount has increased after calling DoGetToken(). This should be safe because there is a limit on the number of nested macro replacements allowed (s_MaxRepeatReplaceCount), so infinite loop is impossible.
But seeing the concerned code, I think the problem is obvious: (in tokenizer.cpp)
--- Code: ---bool Tokenizer::ReplaceBufferText(const wxString& target, bool updatePeekToken)
{
if (target.IsEmpty())
return false;
if (m_IsReplaceParsing && ++m_RepeatReplaceCount > s_MaxRepeatReplaceCount)
{
m_TokenIndex = m_BufferLen - m_FirstRemainingLength;
m_PeekAvailable = false;
SkipToEOL(false);
return false;
}
...
--- End code ---
We can see that the m_RepeatReplaceCount will keep increasing even after s_MaxRepeatReplaceCount is reached, even though no more replacement occurs. We should change it to something like this:
--- Code: ---bool Tokenizer::ReplaceBufferText(const wxString& target, bool updatePeekToken)
{
if (target.IsEmpty())
return false;
if (m_IsReplaceParsing && m_RepeatReplaceCount >= s_MaxRepeatReplaceCount)
return false;
++m_RepeatReplaceCount;
...
--- End code ---
@ollydbg: Does my patch still cause the hang with the above fix?
ollydbg:
--- Quote from: Huki on February 13, 2014, 05:36:24 pm ---...
@ollydbg: Does my patch still cause the hang with the above fix?
--- End quote ---
@Huki, many thanks, fairly clear explanation, I just test the code and no hang now.
This is the code I use:
--- Code: ---bool Tokenizer::ReplaceBufferText(const wxString& target, bool updatePeekToken)
{
if (target.IsEmpty())
return false;
if (m_IsReplaceParsing)
{
if (m_RepeatReplaceCount >= s_MaxRepeatReplaceCount)
{
m_TokenIndex = m_BufferLen - m_FirstRemainingLength;
m_PeekAvailable = false;
SkipToEOL(false);
return false;
}
else
++m_RepeatReplaceCount;
}
.....
--- End code ---
BTW: I originally thought the test condition in PeekToken() can be changed to
--- Quote --- if (m_IsReplaceParsing && savedReplaceCount < (int)m_RepeatReplaceCount)
--- End quote ---
Because in some cases, m_RepeatReplaceCount will reset to zero, but finally I found that is not necessary, because both m_IsReplaceParsing and m_RepeatReplaceCount will be reset to zero in the same time.
--- Code: --- if (m_FirstRemainingLength != 0 && m_BufferLen - m_FirstRemainingLength < m_TokenIndex)
{
m_FirstRemainingLength = 0;
m_IsReplaceParsing = false;
m_RepeatReplaceCount = 0;
}
--- End code ---
BTW2: Why we need two variables? I think m_IsReplaceParsing is a redundant variable of m_RepeatReplaceCount. Checking all the source code, I realize that
--- Code: ---RepeatReplaceCount > 0 means m_IsReplaceParsing == true
RepeatReplaceCount == 0 means m_IsReplaceParsing == false
--- End code ---
So, if we remove the m_IsReplaceParsing, we can change the test to
--- Quote --- if (savedReplaceCount < m_RepeatReplaceCount)
--- End quote ---
But the code to initialize savedReplaceCount can be simply
--- Code: ---size_t savedReplaceCount = m_RepeatReplaceCount;
--- End code ---
Then
--- Code: ---if (m_IsReplaceParsing)
--- End code ---
can change to
--- Code: ---if (m_RepeatReplaceCount > 0)
--- End code ---
Also need to adjust the code below
--- Code: --- // Set replace parsing state, and save first replace token index
if (!m_IsReplaceParsing)
{
m_FirstRemainingLength = m_BufferLen - m_TokenIndex;
m_IsReplaceParsing = true;
}
--- End code ---
I will prepare two commits:
1, fix the hang issue as you suggest.
2, code refactoring by remove the m_IsReplaceParsing variable.
Finally, thanks for your help!
Navigation
[0] Message Index
[#] Next page
[*] Previous page
Go to full version