Author Topic: Macro expansion infinite loop.  (Read 27257 times)

Offline ollydbg

  • Developer
  • Lives here!
  • *****
  • Posts: 5371
  • OpenCV and Robotics
    • Chinese OpenCV forum moderator
Re: Macro expansion infinite loop.
« Reply #30 on: March 05, 2015, 12:48:16 am »
Any progress with this branch?
Do you plan committing fixes any time soon?
This is a big change, but I don't have much time to test this branch, sorry, so I think it won't happen soon.
If some piece of memory should be reused, turn them to variables (or const variables).
If some piece of operations should be reused, turn them to functions.
If they happened together, then turn them to classes.

Offline oBFusCATed

  • Developer
  • Lives here!
  • *****
  • Posts: 13438
    • Travis build status
Re: Macro expansion infinite loop.
« Reply #31 on: March 05, 2015, 01:15:20 am »
I'm testing it every day. For me it is better than the old infinite-loop version.
(most of the time I ignore long posts)
[strangers don't send me private messages, I'll ignore them; post a topic in the forum, but first read the rules!]

Offline MortenMacFly

  • Administrator
  • Lives here!
  • *****
  • Posts: 9615
Re: Macro expansion infinite loop.
« Reply #32 on: March 05, 2015, 05:38:21 am »
I'm testing it every day. For me it is better than the old infinite-loop version.
Same for me until the changes in trunk were no longer compatible with the branch.
A rebase and merge would help to continue testing. However, what I noticed was that many methods of base classes were missing. That's what the wx replacements were for, for example.
Compiler logging: Settings->Compiler & Debugger->tab "Other"->Compiler logging="Full command line"
C::B Manual: http://www.codeblocks.org/docs/main_codeblocks_en.html
C::B FAQ: http://wiki.codeblocks.org/index.php?title=FAQ

Offline Huki

  • Multiple posting newcomer
  • *
  • Posts: 95
Re: Macro expansion infinite loop.
« Reply #33 on: March 06, 2015, 10:22:15 am »
I see infinite loop too. :(

Bug located. Patch here:
Code
 src/plugins/codecompletion/parser/tokenizer.cpp | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/src/plugins/codecompletion/parser/tokenizer.cpp b/src/plugins/codecompletion/parser/tokenizer.cpp
index 764bcbe..6202eda 100644
--- a/src/plugins/codecompletion/parser/tokenizer.cpp
+++ b/src/plugins/codecompletion/parser/tokenizer.cpp
@@ -1794,6 +1794,10 @@ bool Tokenizer::ReplaceBufferText(const wxString& target)
             m_TokenIndex = m_BufferLen - m_FirstRemainingLength;
             m_PeekAvailable = false;
             SkipToEOL(false);
+            SkipToOneOfChars(_T(",;}"), true, false, false);
+            m_SavedTokenIndex   = m_UndoTokenIndex = m_TokenIndex;
+            m_SavedLineNumber   = m_UndoLineNumber = m_LineNumber;
+            m_SavedNestingLevel = m_UndoNestLevel  = m_NestLevel;
             return false;
         }
         else
It is a workaround patch and will cause parsing the Test class body wrongly, you will notice one extra member in the symbol tree :). But it works around a bug that macro replacement cause infinite loop when parsing. The fundamental solution is to ONLY do macro replacement in the Tokenizer level. (We currently do in both ParserThread and Tokenizer).
From this patch I see the bug is due to incorrect handling when the expansion limit (s_MaxRepeatReplaceCount) is reached. I remember we added this commit in r9933 (see diff):
Quote
CC: disable the sanity check when expanding the macro usage, this try to work around an issue reported here: http://forums.codeblocks.org/index.php/topic,19661.msg134291.html#msg134291. We still have a expansion limit to avoid the infinite loop.
See the bolded part in the above quote. Our commit removed the sanity check, so we depend on the expansion limit handling to avoid infinite loop. But we now know the expansion limit is not handled properly and needed to be hacked (by ollydbg's patch) when it's reached. Can someone confirm whether reverting commit 9933 (i.e., adding back the sanity check) solves the issue?
« Last Edit: March 06, 2015, 10:26:30 am by Huki »

Offline oBFusCATed

  • Developer
  • Lives here!
  • *****
  • Posts: 13438
    • Travis build status
Re: Macro expansion infinite loop.
« Reply #34 on: March 06, 2015, 12:30:52 pm »
You can try it yourself, I've posted a minimal project that reproduces the problem.
(most of the time I ignore long posts)
[strangers don't send me private messages, I'll ignore them; post a topic in the forum, but first read the rules!]

Offline ollydbg

  • Developer
  • Lives here!
  • *****
  • Posts: 5371
  • OpenCV and Robotics
    • Chinese OpenCV forum moderator
Re: Macro expansion infinite loop.
« Reply #35 on: March 07, 2015, 08:00:30 am »
...
Can someone confirm whether reverting commit 9933 (i.e., adding back the sanity check) solves the issue?
I did it in my pc, and it still hangs on the minimal project by OBF.

Same for me until the changes in trunk were no longer compatible with the branch.
A rebase and merge would help to continue testing.
I have rebased in my local git copy, do I need to release all the rebased patches?

Quote
However, what I noticed was that many methods of base classes were missing. That's what the wx replacements were for, for example.
I remember wx's macros are handled correctly with my patches. I will test it again.
If some piece of memory should be reused, turn them to variables (or const variables).
If some piece of operations should be reused, turn them to functions.
If they happened together, then turn them to classes.

Offline ollydbg

  • Developer
  • Lives here!
  • *****
  • Posts: 5371
  • OpenCV and Robotics
    • Chinese OpenCV forum moderator
Re: Macro expansion infinite loop.
« Reply #36 on: March 07, 2015, 08:25:26 am »
...
I remember wx's macros are handled correctly with my patches. I will test it again.
I don't see some thing wrong with a simple wx(wx3.0) project created by wizard. So, I what is exact problem you have?

I did test with my patches on our codeblocks.cbp. I notice one issue, in the file sdk\configmanager.cpp
In the function:
Code
TiXmlElement* ConfigManager::AssertPath(wxString& path)
{
    Collapse(path);

    wxString illegal(_T(" -:.\"\'$&()[]<>+#")); // our parser will skip to the end of this source file

It looks like the parser will skip to the end of the file, and comment out of the line don't expose the issue, so I will try to see what is the problem is. I think it is not hard to fix. (Maybe, this bug also exists on our trunk, I haven't tested it yet).
If some piece of memory should be reused, turn them to variables (or const variables).
If some piece of operations should be reused, turn them to functions.
If they happened together, then turn them to classes.

Offline ollydbg

  • Developer
  • Lives here!
  • *****
  • Posts: 5371
  • OpenCV and Robotics
    • Chinese OpenCV forum moderator
Re: Macro expansion infinite loop.
« Reply #37 on: March 08, 2015, 06:23:02 am »
...
I notice one issue, in the file sdk\configmanager.cpp
In the function:
Code
TiXmlElement* ConfigManager::AssertPath(wxString& path)
{
    Collapse(path);

    wxString illegal(_T(" -:.\"\'$&()[]<>+#")); // our parser will skip to the end of this source file

It looks like the parser will skip to the end of the file, and comment out of the line don't expose the issue, so I will try to see what is the problem is. I think it is not hard to fix. (Maybe, this bug also exists on our trunk, I haven't tested it yet).
Here is a test case for our cc which fails in my patch serials.
Code
#define wxCONCAT_HELPER(text, line) text ## line

#define wxT(x) wxCONCAT_HELPER(L, x)

#define _T(x) wxT(x)

wxString illegal(_T(" -:.\"\'$&()[]<>+#"));

int aaa;


//a //aaa

EDIT:
I found the reason, it caused by those code when expansion macros:
Code
    // 5. handling stringizing operator #
    for (int pos = expandedText.Find(_T("#"));
         pos != wxNOT_FOUND;
         pos = expandedText.Find(_T("#")))
    {
In-fact we should skip the cases that # is inside a literal
« Last Edit: March 08, 2015, 06:44:23 am by ollydbg »
If some piece of memory should be reused, turn them to variables (or const variables).
If some piece of operations should be reused, turn them to functions.
If they happened together, then turn them to classes.

Offline ollydbg

  • Developer
  • Lives here!
  • *****
  • Posts: 5371
  • OpenCV and Robotics
    • Chinese OpenCV forum moderator
Re: Macro expansion infinite loop.
« Reply #38 on: March 08, 2015, 07:43:36 am »
Git patches attached against(rebased on) current svn trunk head.
It also fixes the "#" issue in my previous post. :)


[attachment deleted by admin]
If some piece of memory should be reused, turn them to variables (or const variables).
If some piece of operations should be reused, turn them to functions.
If they happened together, then turn them to classes.

Offline Huki

  • Multiple posting newcomer
  • *
  • Posts: 95
Re: Macro expansion infinite loop.
« Reply #39 on: March 08, 2015, 08:48:18 am »
...
Can someone confirm whether reverting commit 9933 (i.e., adding back the sanity check) solves the issue?
I did it in my pc, and it still hangs on the minimal project by OBF.
You're right, commit 9933 is not related with this issue: it only deals with macros like "#define AAA x+y+AAA".
I tried the obf's minimal project and I can already suggest a fix:
In ReplaceBufferText() we have:
Code
        if (m_RepeatReplaceCount >= s_MaxRepeatReplaceCount)
        {
            m_TokenIndex = m_BufferLen - m_FirstRemainingLength;
            m_PeekAvailable = false;
            SkipToEOL(false);
            return false;
        }
Simply change the return to "return true;" and it will fix the problem. It works by skipping (i..e, not parsing) the problem variable "int min;" in the Test struct. If we return false, we stop the current repeat replacement, but later in ParserThread() we will start a new replacement on the same token. :( That's why we get the infinite loop.

It's also a good idea to reset the UndoToken like you did in your original patch. But I am not sure if SkipToEOL() is really required. I suggest a patch like this:
Code
@@ -1812,13 +1812,18 @@ bool Tokenizer::ReplaceBufferText(const wxString& target)
     if (m_RepeatReplaceCount > 0)
     {
         if (m_RepeatReplaceCount >= s_MaxRepeatReplaceCount)
         {
             m_TokenIndex = m_BufferLen - m_FirstRemainingLength;
+
+            // Reset undo token
+            m_SavedTokenIndex   = m_UndoTokenIndex = m_TokenIndex;
+            m_SavedLineNumber   = m_UndoLineNumber = m_LineNumber;
+            m_SavedNestingLevel = m_UndoNestLevel  = m_NestLevel;
+
             m_PeekAvailable = false;
-            SkipToEOL(false);
-            return false;
+            return true; // NOTE: we have to skip the problem token by returning true.
         }
         else
             ++m_RepeatReplaceCount;
     }
     else  // Set replace parsing state, and save first replace token index
« Last Edit: March 08, 2015, 08:49:53 am by Huki »

Offline blauzahn

  • Multiple posting newcomer
  • *
  • Posts: 112
Re: Macro expansion infinite loop.
« Reply #40 on: March 09, 2015, 08:36:48 pm »
Although my humble contribution is not directly related to the infinite loop, it may help the codecompletion a bit.

If I run cppcheck I get a bunch of remarks:

Code
$ cppcheck --enable=all *.cpp |grep -v Checking |grep -v 'files checked'
[cclogger.h:49]: (warning) Member variable 'CCLogger::m_Parent' is not assigned a value in 'CCLogger::operator='.
[cclogger.h:49]: (warning) Member variable 'CCLogger::m_LogId' is not assigned a value in 'CCLogger::operator='.
[cclogger.h:49]: (warning) Member variable 'CCLogger::m_DebugLogId' is not assigned a value in 'CCLogger::operator='.
[cclogger.h:49]: (warning) Member variable 'CCLogger::m_AddTokenId' is not assigned a value in 'CCLogger::operator='.
[tokenizer.h:249]: (performance) Prefer prefix ++/-- operators for non-primitive types.
[expression.cpp:125]: (style) Expression is always false because 'else if' condition matches previous condition at line 104.
[parser.cpp:782] -> [parser.cpp:786]: (performance) Variable 'isParsed' is reassigned a value before the old one has been used.
[parser_base.cpp:180]: (warning) Member variable 'ParserBase::m_BrowserOptions' is not initialized in the constructor.
[parserthread.cpp:1883] -> [parserthread.cpp:1884]: (performance) Variable 'current' is reassigned a value before the old one has been used.
[parserthread.cpp:2819]: (performance) Possible inefficient checking for 'components' emptiness.
[parserthread.cpp:2822]: (performance) Possible inefficient checking for 'components' emptiness.
[parserthread.cpp:3500]: (performance) Possible inefficient checking for 'results' emptiness.
[searchtree.cpp:598]: (style) The scope of the variable 'nchild' can be reduced.
[searchtree.cpp:670]: (style) The scope of the variable 'n' can be reduced.
[searchtree.cpp:759]: (style) The scope of the variable 'matches' can be reduced.
[searchtree.cpp:989]: (style) The scope of the variable 'u' can be reduced.
[searchtree.cpp:77]: (performance) Possible inefficient checking for 'm_Children' emptiness.
[searchtree.cpp:403]: (performance) Prefer prefix ++/-- operators for non-primitive types.
[searchtree.cpp:411]: (performance) Prefer prefix ++/-- operators for non-primitive types.
[searchtree.cpp:460]: (performance) Prefer prefix ++/-- operators for non-primitive types.
[searchtree.cpp:471]: (performance) Prefer prefix ++/-- operators for non-primitive types.
[searchtree.cpp:484]: (performance) Prefer prefix ++/-- operators for non-primitive types.
[searchtree.cpp:510]: (performance) Prefer prefix ++/-- operators for non-primitive types.
[token.cpp:238]: (performance) Possible inefficient checking for 'files' emptiness.
[token.cpp:295]: (performance) Prefer prefix ++/-- operators for non-primitive types.
[tokentree.cpp:150]: (style) The scope of the variable 'result' can be reduced.
[tokentree.cpp:178]: (style) The scope of the variable 'result' can be reduced.
[tokentree.cpp:210]: (style) The scope of the variable 'result' can be reduced.
[tokentree.cpp:243]: (style) The scope of the variable 'result' can be reduced.


[cctreectrl.cpp:86]: (style) C-style pointer casting
[cctreectrl.cpp:166]: (style) C-style pointer casting
[cctreectrl.cpp:167]: (style) C-style pointer casting
[classbrowser.cpp:286]: (style) C-style pointer casting
[classbrowser.cpp:477]: (style) C-style pointer casting
[classbrowser.cpp:492]: (style) C-style pointer casting
[classbrowser.cpp:539]: (style) C-style pointer casting
[classbrowser.cpp:544]: (style) C-style pointer casting
[classbrowser.cpp:807]: (style) C-style pointer casting
[classbrowserbuilderthread.cpp:66]: (warning) Member variable 'ClassBrowserBuilderThread::m_idThreadEvent' is not initialized in the constructor.
[codecompletion.cpp:1964]: (performance) Possible inefficient checking for 'result' emptiness.
[codecompletion.cpp:1749]: (performance) Prefer prefix ++/-- operators for non-primitive types.
[coderefactoring.cpp:438]: (style) The scope of the variable 'pos' can be reduced.
[doxygen_parser.cpp:845]: (performance) Possible inefficient checking for 'tokensIdx' emptiness.
[doxygen_parser.cpp:964]: (performance) Possible inefficient checking for 'result' emptiness.
[nativeparser_base.cpp:1217] -> [nativeparser_base.cpp:1219]: (warning) Possible null pointer dereference: token - otherwise it is redundant to check it against null.
[nativeparser_base.cpp:1247] -> [nativeparser_base.cpp:1249]: (warning) Possible null pointer dereference: token - otherwise it is redundant to check it against null.
[nativeparser_base.cpp:869]: (style) Unused variable: autualTypeResult
[nativeparser_base.cpp:141]: (performance) Prefer prefix ++/-- operators for non-primitive types.
[nativeparser_base.cpp:167]: (performance) Prefer prefix ++/-- operators for non-primitive types.

IMHO, most of them are little obvious things can be corrected with little effort. This might save ollydbg and huki time for the harder things.
Hope, I do not upset somebody in hijacking the thread to some extent. Thought it might fit here and was not worth opening a new one.

Thanks.

Offline ollydbg

  • Developer
  • Lives here!
  • *****
  • Posts: 5371
  • OpenCV and Robotics
    • Chinese OpenCV forum moderator
Re: Macro expansion infinite loop.
« Reply #41 on: March 10, 2015, 04:02:34 am »
Hello, blauzahn, thank you very much!
I fixed most of them in the trunk, see below:
Code
[cclogger.h:49]: (warning) Member variable 'CCLogger::m_Parent' is not assigned a value in 'CCLogger::operator='.
[cclogger.h:49]: (warning) Member variable 'CCLogger::m_LogId' is not assigned a value in 'CCLogger::operator='.
[cclogger.h:49]: (warning) Member variable 'CCLogger::m_DebugLogId' is not assigned a value in 'CCLogger::operator='.
[cclogger.h:49]: (warning) Member variable 'CCLogger::m_AddTokenId' is not assigned a value in 'CCLogger::operator='.
Not sure what is a correct fix.


[tokenizer.h:249]: (performance) Prefer prefix ++/-- operators for non-primitive types.
No need to fix

[expression.cpp:125]: (style) Expression is always false because 'else if' condition matches previous condition at line 104.
fixed


[parser.cpp:782] -> [parser.cpp:786]: (performance) Variable 'isParsed' is reassigned a value before the old one has been used.
no need to fix


[parser_base.cpp:180]: (warning) Member variable 'ParserBase::m_BrowserOptions' is not initialized in the constructor.
fixed


[parserthread.cpp:1883] -> [parserthread.cpp:1884]: (performance) Variable 'current' is reassigned a value before the old one has been used.
fixed.

[parserthread.cpp:2819]: (performance) Possible inefficient checking for 'components' emptiness.
[parserthread.cpp:2822]: (performance) Possible inefficient checking for 'components' emptiness.
no need to fix


[parserthread.cpp:3500]: (performance) Possible inefficient checking for 'results' emptiness.
no need to fix


[searchtree.cpp:598]: (style) The scope of the variable 'nchild' can be reduced.
fixed
[searchtree.cpp:670]: (style) The scope of the variable 'n' can be reduced.
fixed, but maybe n get reassigned?
[searchtree.cpp:759]: (style) The scope of the variable 'matches' can be reduced.
fixed
[searchtree.cpp:989]: (style) The scope of the variable 'u' can be reduced.
fixed


[searchtree.cpp:77]: (performance) Possible inefficient checking for 'm_Children' emptiness.
no need to fix

[searchtree.cpp:403]: (performance) Prefer prefix ++/-- operators for non-primitive types.
[searchtree.cpp:411]: (performance) Prefer prefix ++/-- operators for non-primitive types.
[searchtree.cpp:460]: (performance) Prefer prefix ++/-- operators for non-primitive types.
[searchtree.cpp:471]: (performance) Prefer prefix ++/-- operators for non-primitive types.
[searchtree.cpp:484]: (performance) Prefer prefix ++/-- operators for non-primitive types.
[searchtree.cpp:510]: (performance) Prefer prefix ++/-- operators for non-primitive types.
[token.cpp:238]: (performance) Possible inefficient checking for 'files' emptiness.
[token.cpp:295]: (performance) Prefer prefix ++/-- operators for non-primitive types.
all no need to fix.


[tokentree.cpp:150]: (style) The scope of the variable 'result' can be reduced.
fixed
[tokentree.cpp:178]: (style) The scope of the variable 'result' can be reduced.
fixed
[tokentree.cpp:210]: (style) The scope of the variable 'result' can be reduced.
fixed
[tokentree.cpp:243]: (style) The scope of the variable 'result' can be reduced.
fixed


[cctreectrl.cpp:86]: (style) C-style pointer casting
[cctreectrl.cpp:166]: (style) C-style pointer casting
[cctreectrl.cpp:167]: (style) C-style pointer casting
[classbrowser.cpp:286]: (style) C-style pointer casting
[classbrowser.cpp:477]: (style) C-style pointer casting
[classbrowser.cpp:492]: (style) C-style pointer casting
[classbrowser.cpp:539]: (style) C-style pointer casting
[classbrowser.cpp:544]: (style) C-style pointer casting
[classbrowser.cpp:807]: (style) C-style pointer casting
Not currently yet. (No need to fix?)


[classbrowserbuilderthread.cpp:66]: (warning) Member variable 'ClassBrowserBuilderThread::m_idThreadEvent' is not initialized in the constructor.
fixed


[codecompletion.cpp:1964]: (performance) Possible inefficient checking for 'result' emptiness.
[codecompletion.cpp:1749]: (performance) Prefer prefix ++/-- operators for non-primitive types.
no need to fix


[coderefactoring.cpp:438]: (style) The scope of the variable 'pos' can be reduced.
fixed


[doxygen_parser.cpp:845]: (performance) Possible inefficient checking for 'tokensIdx' emptiness.
fixed by using empty() function.


[doxygen_parser.cpp:964]: (performance) Possible inefficient checking for 'result' emptiness.
no need to fix


[nativeparser_base.cpp:1217] -> [nativeparser_base.cpp:1219]: (warning) Possible null pointer dereference: token - otherwise it is redundant to check it against null.
This is alrady checked in if (token && MatchType(token->m_TokenKind, kindMask))

[nativeparser_base.cpp:1247] -> [nativeparser_base.cpp:1249]: (warning) Possible null pointer dereference: token - otherwise it is redundant to check it against null.
This is alrady checked in if (token && MatchType(token->m_TokenKind, kindMask))


[nativeparser_base.cpp:869]: (style) Unused variable: autualTypeResult
fixed


[nativeparser_base.cpp:141]: (performance) Prefer prefix ++/-- operators for non-primitive types.
[nativeparser_base.cpp:167]: (performance) Prefer prefix ++/-- operators for non-primitive types.
no need to fix

The "no need to fix" means I don't think it is necessary to fix them. such as "it++" and "++it" should be the same thing.
The others are that I don't know a correct fix. :)
If some piece of memory should be reused, turn them to variables (or const variables).
If some piece of operations should be reused, turn them to functions.
If they happened together, then turn them to classes.

Offline blauzahn

  • Multiple posting newcomer
  • *
  • Posts: 112
Re: Macro expansion infinite loop.
« Reply #42 on: March 10, 2015, 07:43:08 am »
Thanks for applying this.

Quote
The "no need to fix" means I don't think it is necessary to fix them. such as "it++" and "++it" should be the same thing.
Even then I would change them. Prefer ++it. Make it a habit unless there is a reason to use it++. And it makes usage
of tools like cppcheck easier because it keeps the list short so that the chances to miss a real problem in a long list
are lower. I do not eliminate every warning of a tool slavishly but here it makes sense.

In cases where it++ would actually create a (often unwanted temporary) it may make the program slower.
Rationale: see Herb Sutter C++Coding Standards §28 "[...] prefer calling the prefix forms", §9 "Don't pessimize prematurely."

The same applies to C-style pointer casting. Rationale see Herb Sutter §95 "Don't use C-Style casts."

Quote
Code
[cclogger.h:49]: (warning) Member variable 'CCLogger::m_Parent' is not assigned a value in 'CCLogger::operator='.
[cclogger.h:49]: (warning) Member variable 'CCLogger::m_LogId' is not assigned a value in 'CCLogger::operator='.
[cclogger.h:49]: (warning) Member variable 'CCLogger::m_DebugLogId' is not assigned a value in 'CCLogger::operator='.
[cclogger.h:49]: (warning) Member variable 'CCLogger::m_AddTokenId' is not assigned a value in 'CCLogger::operator='.
Not sure what is a correct fix.
As for c++11 I would say for the cctor and op= =default to express the intent of really copying the non-owning pointer member m_Parent
and the ints. Even if the ugly static global variable is the only instance (is it?). In c++98 I would write the cctor explicitly and a canonical
op= and swap. A bit of boilerplate but makes life safer.

Why is there a virtual dtor. Apart from the dtor there is no other virtual method. So, the class is no polymorphic interface and probably
does not need a virtual dtor (including vtable etc.).

Having to remember to call the Init method (from cc ctor) after receiving a pointer to the global variable via Get() is a bit unfortunate since this is
rather bug prone. Since the data-member are private and no method changes any of them I'd make the methods const. If it never changes, may
be Get should return a (cheap and safe) copy instead of a pointer.



Offline oBFusCATed

  • Developer
  • Lives here!
  • *****
  • Posts: 13438
    • Travis build status
Re: Macro expansion infinite loop.
« Reply #43 on: March 10, 2015, 10:15:58 am »
Even then I would change them. Prefer ++it. ...
We're using optimizing compilers, so it won't make any difference! If you hate them so much post a patch. It is trivial to do.
And please do it in another topic. This one is related to something else and your post is quite off topic. Also this topic you've started is prone to flaming/religion and so on, so the original one will be swamped by off topic posts!

Fixing warnings is good but fixing real problems that have user visible effects is even better.

@admins: Please separate post related to cppcheck in a separate topic.
(most of the time I ignore long posts)
[strangers don't send me private messages, I'll ignore them; post a topic in the forum, but first read the rules!]

Offline ollydbg

  • Developer
  • Lives here!
  • *****
  • Posts: 5371
  • OpenCV and Robotics
    • Chinese OpenCV forum moderator
Re: Macro expansion infinite loop.
« Reply #44 on: March 13, 2015, 04:06:09 pm »
...
In ReplaceBufferText() we have:
Code
        if (m_RepeatReplaceCount >= s_MaxRepeatReplaceCount)
        {
            m_TokenIndex = m_BufferLen - m_FirstRemainingLength;
            m_PeekAvailable = false;
            SkipToEOL(false);
            return false;
        }
Simply change the return to "return true;" and it will fix the problem. It works by skipping (i..e, not parsing) the problem variable "int min;" in the Test struct. If we return false, we stop the current repeat replacement, but later in ParserThread() we will start a new replacement on the same token. :( That's why we get the infinite loop.
...
Does return true cause other issue? I don't know. Maybe, we could add some test like
change this
Code
void Tokenizer::GetReplacedToken(wxString& str)
{
    // this indicates we are already in macro replacement mode
    if (m_RepeatReplaceCount > 0)
    {
to
Code
void Tokenizer::GetReplacedToken(wxString& str)
{
    // this indicates we are already in macro replacement mode
    if (m_RepeatReplaceCount > 0 && m_RepeatReplaceCount < s_MaxRepeatReplaceCount )
    {
But I haven't tested yet.

If some piece of memory should be reused, turn them to variables (or const variables).
If some piece of operations should be reused, turn them to functions.
If they happened together, then turn them to classes.