Author Topic: CC makes C::B hang in ExpandBackticks  (Read 72172 times)

Offline Alpha

  • Developer
  • Lives here!
  • *****
  • Posts: 1513
Re: CC makes C::B hang in ExpandBackticks
« Reply #30 on: January 05, 2013, 04:10:30 am »
Congratulations, you have discovered not one bug, but two!  Compiler::GetExecName() is supposed to return the passed argument, if it is not a macro to be expanded.  Also, it appears that asynchronous use of wxExecute() acts differently on Linux and Windows.
Code
Index: src/sdk/compiler.cpp
===================================================================
--- src/sdk/compiler.cpp (revision 8756)
+++ src/sdk/compiler.cpp (working copy)
@@ -1138,20 +1138,20 @@
         }
         wxSetEnv(wxT("PATH"), path);
         cmd[0] = GetExecName(cmd[0]);
-        if (node->GetAttribute(wxT("regex"), &test))
+
+        long ret;
         {
-            long ret;
-            {
-                wxLogNull logNo;
-                ret = wxExecute(GetStringFromArray(cmd, wxT(" "), false), cmd);
-            }
+            wxLogNull logNo; // do not warn if execution fails
+            ret = wxExecute(GetStringFromArray(cmd, wxT(" "), false), cmd);
+        }
+
+        if (ret != 0) // execution failed
+            val = (node->GetAttribute(wxT("default"), wxEmptyString) == wxT("true"));
+        else if (node->GetAttribute(wxT("regex"), &test))
+        {
             wxRegEx re;
-            if (ret != 0)
+            if (re.Compile(test))
             {
-                val = (node->GetAttribute(wxT("default"), wxEmptyString) == wxT("true"));
-            }
-            else if (re.Compile(test))
-            {
                 for (size_t i = 0; i < cmd.GetCount(); ++i)
                 {
                     if (re.Matches(cmd[i]))
@@ -1162,20 +1162,17 @@
                 }
             }
         }
-        else
-        {
-            wxLogNull logNo;
-            long ret = wxExecute(GetStringFromArray(cmd, wxT(" "), false));
-            val = (ret != 0);
-        }
-        wxSetEnv(wxT("PATH"), origPath);
+        else // execution succeeded (and no regex test given)
+            val = true;
+
+        wxSetEnv(wxT("PATH"), origPath); // restore path
     }
     return val;
 }
 
 wxString Compiler::GetExecName(const wxString& name)
 {
-    wxString ret;
+    wxString ret = name;
     if (name == wxT("C"))
         ret = m_Programs.C;
     else if (name == wxT("CPP"))
« Last Edit: January 05, 2013, 04:21:20 am by Alpha »

Offline MortenMacFly

  • Administrator
  • Lives here!
  • *****
  • Posts: 9702
Re: CC makes C::B hang in ExpandBackticks
« Reply #31 on: January 05, 2013, 07:30:25 am »
Does this one:
Congratulations, you have discovered not one bug, but two! [...]
Code
Index: src/sdk/compiler.cpp
...make this one:
Yes, they are caused by the xml compiler code, Alpha can you look at them?
[...]
I suggest catching the error output and either give a more clear error message or just suppress it.
Code
Index: src/plugins/compilergcc/resources/compilers/options_clang.xml
...obsolete?
Compiler logging: Settings->Compiler & Debugger->tab "Other"->Compiler logging="Full command line"
C::B Manual: https://www.codeblocks.org/docs/main_codeblocks_en.html
C::B FAQ: https://wiki.codeblocks.org/index.php?title=FAQ

Offline Jenna

  • Administrator
  • Lives here!
  • *****
  • Posts: 7256
Re: CC makes C::B hang in ExpandBackticks
« Reply #32 on: January 05, 2013, 10:16:32 am »
Does this one:
Congratulations, you have discovered not one bug, but two! [...]
Code
Index: src/sdk/compiler.cpp
...make this one:
Yes, they are caused by the xml compiler code, Alpha can you look at them?
[...]
I suggest catching the error output and either give a more clear error message or just suppress it.
Code
Index: src/plugins/compilergcc/resources/compilers/options_clang.xml
...obsolete?
The error-message ("execvp(som ething, someargument) failed with error 2!" will still occur, because wxExecute is called for a not existent exe.
It will only be visible from a console, but as it is not meant for the user it should be caught (with a wxArrayString) and dismissed.

Offline oBFusCATed

  • Developer
  • Lives here!
  • *****
  • Posts: 13413
    • Travis build status
Re: CC makes C::B hang in ExpandBackticks
« Reply #33 on: January 05, 2013, 12:29:37 pm »
Also, this is a real problem because the linking with clang is broken. Probably logging some errors here in the normal log will be needed.
(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 Jenna

  • Administrator
  • Lives here!
  • *****
  • Posts: 7256
Re: CC makes C::B hang in ExpandBackticks
« Reply #34 on: January 05, 2013, 12:39:13 pm »
Also, this is a real problem because the linking with clang is broken. Probably logging some errors here in the normal log will be needed.
There should probably a predefined error-message in the xml-file, which can be shown if the execution fails.
The actual message is not very helpful, at least not for many users, who seem to have problems with more clear messages sometimes  :( .

Offline oBFusCATed

  • Developer
  • Lives here!
  • *****
  • Posts: 13413
    • Travis build status
Re: CC makes C::B hang in ExpandBackticks
« Reply #35 on: January 05, 2013, 01:03:38 pm »
Yes, this message is printed by wxexecute...
(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 Alpha

  • Developer
  • Lives here!
  • *****
  • Posts: 1513
Re: CC makes C::B hang in ExpandBackticks
« Reply #36 on: January 05, 2013, 02:10:00 pm »
Does this one:
Congratulations, you have discovered not one bug, but two! [...]
Code
Index: src/sdk/compiler.cpp
...make this one:
Yes, they are caused by the xml compiler code, Alpha can you look at them?
[...]
I suggest catching the error output and either give a more clear error message or just suppress it.
Code
Index: src/plugins/compilergcc/resources/compilers/options_clang.xml
...obsolete?
Yes.

It will only be visible from a console, but as it is not meant for the user it should be caught (with a wxArrayString) and dismissed.
With this patch, it should no longer report it to the console; is it still doing that for you?

My original intention behind
Code
<if exec="XXX">
was to allow a conditional to test if a program existed, so in my opinion, there should not be any error message if execution fails.
Also, this is a real problem because the linking with clang is broken.
Clang's linker is not broken (to my knowledge); I had discovered (by accident) that it was possible to install a working Clang without llvm.  The conditional is there so that hopefully Clang will "just work" regardless of user configuration.

Offline oBFusCATed

  • Developer
  • Lives here!
  • *****
  • Posts: 13413
    • Travis build status
Re: CC makes C::B hang in ExpandBackticks
« Reply #37 on: January 05, 2013, 02:25:24 pm »
Clang's linker is not broken (to my knowledge); I had discovered (by accident) that it was possible to install a working Clang without llvm.  The conditional is there so that hopefully Clang will "just work" regardless of user configuration.
I've just upgraded to llvm/clang 3.2 and linking .a files doesn't work anymore. With 3.1 all was fine...

Also the problem is that you never check if the cmd[0]=="". As far as I can see your patch doesn't add such checks. Executing ""+" "+"-version" will never be a good idea...
(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: 9702
Re: CC makes C::B hang in ExpandBackticks
« Reply #38 on: January 05, 2013, 02:41:07 pm »
I've just upgraded to llvm/clang 3.2 and linking .a files doesn't work anymore. With 3.1 all was fine...
Which distro do you use (in case its Windows)? I am looking for a working up-to-date distro of Clang for Windows 32.

I used to use this one:
http://www.ishani.org/web/articles/code/clang-win32/
...but it doesn't work anymore on WinXP/32.
Compiler logging: Settings->Compiler & Debugger->tab "Other"->Compiler logging="Full command line"
C::B Manual: https://www.codeblocks.org/docs/main_codeblocks_en.html
C::B FAQ: https://wiki.codeblocks.org/index.php?title=FAQ

Offline Jenna

  • Administrator
  • Lives here!
  • *****
  • Posts: 7256
Re: CC makes C::B hang in ExpandBackticks
« Reply #39 on: January 05, 2013, 02:42:54 pm »
The failure message does no longer occur.
I wonder why it was there before.
The bull-logger should have eaten it also I think.

Offline oBFusCATed

  • Developer
  • Lives here!
  • *****
  • Posts: 13413
    • Travis build status
Re: CC makes C::B hang in ExpandBackticks
« Reply #40 on: January 05, 2013, 03:09:33 pm »
Which distro do you use (in case its Windows)?
Gentoo linux in this case, I've not booted windows for quite a long time...
(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: 6036
  • OpenCV and Robotics
    • Chinese OpenCV forum moderator
Re: CC makes C::B hang in ExpandBackticks
« Reply #41 on: January 05, 2013, 03:30:43 pm »
I've just upgraded to llvm/clang 3.2 and linking .a files doesn't work anymore. With 3.1 all was fine...
Which distro do you use (in case its Windows)? I am looking for a working up-to-date distro of Clang for Windows 32.

I used to use this one:
http://www.ishani.org/web/articles/code/clang-win32/
...but it doesn't work anymore on WinXP/32.

OT
From mingw64 maillist:
Quote
Hi,

A new LLVM/Clang has been released, with even better C++11 support. Sadly, no important Windows specific things were fixed or added since the previous release (that I know of), so you're still limited to GCC 4.6.3's libstdc++.

The previous 4.6.3 GCC linux64 build was also incompatible with older Linux installs, like Debian stable. This one is buillt on Debian stable, so should work on any non-ancient Linux install. If you need older or 32-bit linux, download the source package and build it yourself :)

Cheers,

Ruben

PS: Download links:
http://sourceforge.net/projects/mingw-w64/files/Toolchain%20sources/Personal%20Builds/rubenvb/release/
https://sourceforge.net/projects/mingw-w64/files/Toolchains%20targetting%20Win32/Personal%20Builds/rubenvb/
https://sourceforge.net/projects/mingw-w64/files/Toolchains%20targetting%20Win64/Personal%20Builds/rubenvb/


I haven't tried it.
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 Alpha

  • Developer
  • Lives here!
  • *****
  • Posts: 1513
Re: CC makes C::B hang in ExpandBackticks
« Reply #42 on: January 05, 2013, 04:54:37 pm »
I've just upgraded to llvm/clang 3.2 and linking .a files doesn't work anymore. With 3.1 all was fine...
Oh; I was using 3.0 and 3.1 of Clang.

Also the problem is that you never check if the cmd[0]=="". As far as I can see your patch doesn't add such checks. Executing ""+" "+"-version" will never be a good idea...
cmd[0] only became an empty string because of the incorrect behavior in GetExecName() (which the patch fixes).
... but I guess I should add a check in case there is some other situation that has not been considered.

Offline oBFusCATed

  • Developer
  • Lives here!
  • *****
  • Posts: 13413
    • Travis build status
Re: CC makes C::B hang in ExpandBackticks
« Reply #43 on: January 05, 2013, 05:07:04 pm »
... but I guess I should add a check in case there is some other situation that has not been considered.
Yes, this part of the code should be more bulletproofed than the others :)
(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 Alpha

  • Developer
  • Lives here!
  • *****
  • Posts: 1513
Re: CC makes C::B hang in ExpandBackticks
« Reply #44 on: January 05, 2013, 05:31:55 pm »
Yes, this part of the code should be more bulletproofed than the others :)
Code
Index: src/sdk/compiler.cpp
===================================================================
--- src/sdk/compiler.cpp (revision 8758)
+++ src/sdk/compiler.cpp (working copy)
@@ -1151,6 +1151,8 @@
     else if (node->GetAttribute(wxT("exec"), &test))
     {
         wxArrayString cmd = GetArrayFromString(test, wxT(" "));
+        if (cmd.IsEmpty())
+            return false;
         wxString path;
         wxGetEnv(wxT("PATH"), &path);
         const wxString origPath = path;
@@ -1171,20 +1173,21 @@
         }
         wxSetEnv(wxT("PATH"), path);
         cmd[0] = GetExecName(cmd[0]);
-        if (node->GetAttribute(wxT("regex"), &test))
+
+        long ret = -1;
+        if ( !cmd[0].IsEmpty() ) // should never be empty
         {
-            long ret;
-            {
-                wxLogNull logNo;
-                ret = wxExecute(GetStringFromArray(cmd, wxT(" "), false), cmd);
-            }
+            wxLogNull logNo; // do not warn if execution fails
+            ret = wxExecute(GetStringFromArray(cmd, wxT(" "), false), cmd);
+        }
+
+        if (ret != 0) // execution failed
+            val = (node->GetAttribute(wxT("default"), wxEmptyString) == wxT("true"));
+        else if (node->GetAttribute(wxT("regex"), &test))
+        {
             wxRegEx re;
-            if (ret != 0)
+            if (re.Compile(test))
             {
-                val = (node->GetAttribute(wxT("default"), wxEmptyString) == wxT("true"));
-            }
-            else if (re.Compile(test))
-            {
                 for (size_t i = 0; i < cmd.GetCount(); ++i)
                 {
                     if (re.Matches(cmd[i]))
@@ -1195,20 +1198,17 @@
                 }
             }
         }
-        else
-        {
-            wxLogNull logNo;
-            long ret = wxExecute(GetStringFromArray(cmd, wxT(" "), false));
-            val = (ret != 0);
-        }
-        wxSetEnv(wxT("PATH"), origPath);
+        else // execution succeeded (and no regex test given)
+            val = true;
+
+        wxSetEnv(wxT("PATH"), origPath); // restore path
     }
     return val;
 }
 
 wxString Compiler::GetExecName(const wxString& name)
 {
-    wxString ret;
+    wxString ret = name;
     if (name == wxT("C"))
         ret = m_Programs.C;
     else if (name == wxT("CPP"))