Do not pass NULL for the string in catch_errors
Commit Message
On 10/23/2015 02:48 PM, Luis Machado wrote:
> On 10/22/2015 11:43 AM, Pedro Alves wrote:
>> On 10/22/2015 01:36 PM, Luis Machado wrote:
>>> On 10/22/2015 09:50 AM, Pedro Alves wrote:
>>>> On 10/22/2015 12:23 PM, Luis Machado wrote:
>>
>>> That would be fine by me. I was just experimenting with
>>> TRY/CATCH/END_CATCH after my unsuccessful replacement of catch_errors
>>> with catch_exceptions. See below.
>>>>>
>>
>>>>> With catch_exceptions, instead of catching the error and letting the
>>>>> inferior continue, it will just cause the inferior to terminate.
>>>>
>>>> I don't understand. Why do you say this will happen?
>>>>
>>>
>>> I replaced catch_errors with catch_exceptions in record-full.c. I saw a
>>> bunch of failures in gdb.reverse/sigall-reverse.exp, starting at this
>>> point:
>>>
>>> Breakpoint 142, handle_TERM (sig=15) at
>>> ../../../gdb-head-ro/gdb/testsuite/gdb.reverse/sigall-reverse.c:378^M
>>> 378 }^M
>>> (gdb) PASS: gdb.reverse/sigall-reverse.exp: send signal TERM
>>> continue^M
>>> Continuing.^M
>>> The next instruction is syscall exit_group. It will make the program
>>> exit. Do you want to stop the program?([y] or n) yes^M
>>> Process record: inferior program stopped.^M
>>> ^M
>>> [process 21188] #1 stopped.^M
>>>
>>> The above is a normal run. If i replace catch_errors with
>>> catch_exceptions, instead of stopping the inferior, it will terminate.
>>> Maybe there is a bug somewhere, or something is being mishandled.
>>
>> It just sounds to me that you didn't take into account
>> that the return values of catch_errors and catch_exceptions
>> differ.
>>
>> while one does:
>>
>> if (exception.reason < 0)
>> {
>> ...
>> return exception.reason;
>> }
>>
>> the other does:
>>
>> if (exception.reason != 0)
>> return 0;
>>
>> This matters because the result is returned by
>> record_full_message_wrapper_safe, and checked here:
>>
>> if (!record_full_message_wrapper_safe (regcache,
>> GDB_SIGNAL_0))
>> {
>> status->kind = TARGET_WAITKIND_STOPPED;
>> status->value.sig = GDB_SIGNAL_0;
>> break;
>> }
>>
>
> Indeed this is the case. I think i'll keep catch_errors and only fix the
> NULL parameter then. Having to adjust return values from unrelated
> functions sounds error-prone and maybe not worth it if we're moving away
> from these types of constructs in the future.
>
>
I've pushed the following now as 7cc53fba0a4e5c316a6e86fdae28f8cc9d0f9a68.
2015-10-26 Luis Machado <lgustavo@codesourcery.com>
* record-full.c (record_full_message_wrapper_safe): Pass empty string to
catch_errors call instead of NULL.
@@ -667,7 +667,7 @@ record_full_message_wrapper_safe (struct regcache *regcache,
args.regcache = regcache;
args.signal = signal;
- return catch_errors (record_full_message_wrapper, &args, NULL,
+ return catch_errors (record_full_message_wrapper, &args, "",
RETURN_MASK_ALL);
}