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

[Viktor Kutuzov]

There are 2 different types of status codes we could get there:

1. The test status code, which bugpoint uses to narrow down the problem;
2. The remote client status code (for example if it cannot connect to a remote target and such).

As it was before, the remote client status code was returned, which made bugpoint handle remote client errors as test program 
errors - it assumes that something is wrong with the test program and tries to narrow down the problem.

As it is now (thanks Evan for fixing that warning for me), the test status code got appended to the output file and participates at 
the diffProgram step.
The remote client errors get reported and 0 always returned. The same behavior we have in the test suite.

Ideally, we need to have 2 separate ranges/namespaces for these 2 different types of status codes, in this case we can return test 
program status code from GCC::ExecuteProgram and escalate remote client errors with llvm_report_error.
Does anyone have a good idea how to do this?

Best regards,
Viktor

----- Original Message ----- 
From: "Dan Gohman" <gohman at apple.com>
To: "Commit Messages and Patches for LLVM" <llvm-commits at cs.uiuc.edu>
Sent: Tuesday, July 14, 2009 5:18 PM
Subject: Re: [llvm-commits] [llvm] r75665 -/llvm/trunk/tools/bugpoint/ToolRunner.cpp


>
> On Jul 14, 2009, at 12:11 PM, Viktor Kutuzov wrote:
>>     return RunProgramWithTimeout(OutputBinary, &ProgramArgs[0],
>>         sys::Path(InputFile), sys::Path(OutputFile), sys::Path
>> (OutputFile),
>>         Timeout, MemoryLimit);
>> -  else
>> -    return RunProgramWithTimeout(sys::Path(RemoteClientPath),
>> &ProgramArgs[0],
>> -        sys::Path(InputFile), sys::Path(OutputFile), sys::Path
>> (OutputFile),
>> -        Timeout, MemoryLimit);
>> +  } else {
>> +    std::cout << "<run remotely>" << std::flush;
>> +    int RemoteClientStatus = RunProgramWithTimeout(sys::Path
>> (RemoteClientPath),
>> +        &ProgramArgs[0], sys::Path(InputFile), sys::Path(OutputFile),
>> +        sys::Path(OutputFile), Timeout, MemoryLimit);
>> +    if (RemoteClientStatus != 0) {
>> +      std::cerr << "Remote Client failed with an error: " <<
>> +        RemoteClientStatus << ".\n" << std::flush;
>> +    }
>> +  }
>
> There's something very non-obvious happening here.
>
> The code above ignores the ReturnClientStatus value, and there's
> no return value. The immediate problem triggered a compiler warning,
> which has subsequently been silenced on TOT by making the function
> always return 0. That doesn't seem right, since ReturnClientStatus
> is still being ignored.
>
> Some things may happen to work, because
> utils/bugpoint/RemoteRunSafely.sh doesn't ever actually return a non-
> zero
> exit value. There are tests that rely on a non-zero exit value being
> noticed, and utils/bugpoint/RemoteRunSafely.sh satisfies this
> requirement
> by printing the exit value itself, which is something that bugpoint
> normally does.
>
> I don't know how bugpoint's --append-exit-code option is being used in
> all of this, but with RemoteRunSafely.sh unconditionally printing the
> exit code, it looks like something isn't right.
>
> Is there a reason why RemoteRunSafely.sh doesn't return the program's
> exit status, allowing bugpoint to handle it like a normal exit status?
>
> Dan
>
> _______________________________________________
> llvm-commits mailing list
> llvm-commits at cs.uiuc.edu
> http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits
>