[llvm-commits] [llvm] r76432 -/llvm/trunk/tools/bugpoint/ToolRunner.cpp

[Viktor Kutuzov]

Hello David,

This patch has broken the bugpoint.
The problems I see are:

1. The definition of the std::string Exec is local in the "if" block and the string got destroyed before it gets actually used 
(notice that ProgramArgs is a vector of pointers and it doesn't change the string scope).

2. Having that "cd" in the remote client arguments requres the remote client to support the cd comand, I guess. Which it not a good 
idea.

3. In the reality, the calling command now looks like this:

utils/bugpoint/RemoteRunSafely.sh 192.168.235.179 -l vkutuzov -p 22 -o option cd 
/home/vkutuzov/Development/llvm-project/obj.llvm/Debug/bin; ./bugpoint-test-program.bc-GDdGsd.llc.s.gcc.exe

Depending on a shell, the list of arguments could be terminated by that ";".
Did you want to change a directory before launching the remote client?

Anyway, what's the point of changing the working directory in the first place?
What's wrong with the using whatever the working directory is?

Best regards,
Viktor

----- Original Message ----- 
From: "David Goodwin" <david_goodwin at apple.com>
To: <llvm-commits at cs.uiuc.edu>
Sent: Monday, July 20, 2009 10:15 AM
Subject: [llvm-commits] [llvm] r76432 -/llvm/trunk/tools/bugpoint/ToolRunner.cpp


> Author: david_goodwin
> Date: Mon Jul 20 12:15:03 2009
> New Revision: 76432
>
> URL: http://llvm.org/viewvc/llvm-project?rev=76432&view=rev
> Log:
> For remote execution, must cd to the executable directory since the exe expects to find a dylib in the CWD ('.').
>
> Modified:
>    llvm/trunk/tools/bugpoint/ToolRunner.cpp
>
> Modified: llvm/trunk/tools/bugpoint/ToolRunner.cpp
> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/tools/bugpoint/ToolRunner.cpp?rev=76432&r1=76431&r2=76432&view=diff
>
> ==============================================================================
> --- llvm/trunk/tools/bugpoint/ToolRunner.cpp (original)
> +++ llvm/trunk/tools/bugpoint/ToolRunner.cpp Mon Jul 20 12:15:03 2009
> @@ -688,7 +688,6 @@
>
>   std::vector<const char*> ProgramArgs;
>
> -  std::string Exec;
>   if (RemoteClientPath.isEmpty())
>     ProgramArgs.push_back(OutputBinary.c_str());
>   else {
> @@ -704,10 +703,12 @@
>       ProgramArgs.push_back(RemoteExtra.c_str());
>     }
>
> -    // Full path to the binary
> +    // Full path to the binary. We need to cd to the exec directory because
> +    // there is a dylib there that the exec expects to find in the CWD
>     char* env_pwd = getenv("PWD");
> +    std::string Exec = "cd ";
>     Exec += env_pwd;
> -    Exec += "/";
> +    Exec += "; ./";
>     Exec += OutputBinary.c_str();
>     ProgramArgs.push_back(Exec.c_str());
>   }
>
>
> _______________________________________________
> llvm-commits mailing list
> llvm-commits at cs.uiuc.edu
> http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits
>