CC makes C::B hang in ExpandBackticks

[Alpha]

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"))

[MortenMacFly]

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?

[Jenna]

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.

[oBFusCATed]

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.

[Jenna]

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  .

[oBFusCATed]

Yes, this message is printed by wxexecute...

[Alpha]

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.

[oBFusCATed]

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...

[MortenMacFly]

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.

[Jenna]

The failure message does no longer occur.
I wonder why it was there before.
The bull-logger should have eaten it also I think.

[oBFusCATed]

Which distro do you use (in case its Windows)?

Gentoo linux in this case, I've not booted windows for quite a long time...

[ollydbg]

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:

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.

[Alpha]

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.

[oBFusCATed]

... 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

[Alpha]

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"))