This is the mail archive of the
gdb-patches@sourceware.org
mailing list for the GDB project.
Re: [PATCH 2/6] Don't reset errno/bfd_error on 'throw_perror_with_name'
- From: Sergio Durigan Junior <sergiodj at redhat dot com>
- To: Pedro Alves <palves at redhat dot com>
- Cc: Tom Tromey <tom at tromey dot com>, GDB Patches <gdb-patches at sourceware dot org>, Eli Zaretskii <eliz at gnu dot org>, Ruslan Kabatsayev <b7 dot 10110111 at gmail dot com>
- Date: Mon, 02 Mar 2020 15:07:06 -0500
- Subject: Re: [PATCH 2/6] Don't reset errno/bfd_error on 'throw_perror_with_name'
- References: <20190926042155.31481-1-sergiodj@redhat.com> <20200226200542.746617-1-sergiodj@redhat.com> <20200226200542.746617-3-sergiodj@redhat.com> <87wo86ivbu.fsf@tromey.com> <a24ae841-2c59-d5d8-a5d7-7ad8ba72abe7@redhat.com> <87k146pi13.fsf@redhat.com> <89e47bf5-d5c8-849d-e345-9b79306418f8@redhat.com>
On Friday, February 28 2020, Pedro Alves wrote:
> On 2/28/20 8:35 PM, Sergio Durigan Junior wrote:
>> On Friday, February 28 2020, Pedro Alves wrote:
>>
>>> On 2/28/20 3:29 PM, Tom Tromey wrote:
>>>>>>>>> "Sergio" == Sergio Durigan Junior <sergiodj@redhat.com> writes:
>>>>
>>>> Sergio> Since this hunk may be a bit controversial, I decided to split it into
>>>> Sergio> a separate patch. This is going to be needed by the ptrace-error
>>>> Sergio> feature; GDB will need to be able to access the value of errno even
>>>> Sergio> after a call to our 'perror'-like functions.
>>>>
>>>> I'm in favor of this. The existing code seems pretty ugly.
>>>
>>> I'm not sure in favor of relying on errno being preserved from
>>> throw site to catch site, with potentially multiple try/catch hops
>>> in between. Sergio, can you point out exactly how you're
>>> intending to use that?
>>
>> Yeah. I caught this problem when I was testing to see if GDB would
>> print the ptrace fail reason when trying (unsuccessfully) to attach to a
>> process.
>>
>> The call stack looks like:
>>
>> linux_nat_target::attach
>> |
>> |--> inf_ptrace_target::attach # where ptrace fails
>> |
>> |--> throw_perror_with_name # where errno is set to 0
>>
>> When 'throw_perror_with_name' calls 'error', 'linux_nat_target::attach'
>> catches the exception and tries to print the reason:
>>
>> try
>> {
>> inf_ptrace_target::attach (args, from_tty);
>> }
>> catch (const gdb_exception_error &ex)
>> {
>> int saved_errno = errno;
>> pid_t pid = parse_pid_to_attach (args);
>> std::string reason = linux_ptrace_attach_fail_reason (pid, saved_errno);
>> ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
>>
>> if (!reason.empty ())
>> throw_error (ex.error, "warning: %s\n%s", reason.c_str (),
>> ex.what ());
>> else
>> throw_error (ex.error, "%s", ex.what ());
>> }
>>
>> However, at this point errno is already 0, so the function can't
>> determine the exact reason for the ptrace failure. In fact, because
>> errno = 0, 'linux_ptrace_attach_fail_reason' doesn't print anything,
>> because it thinks everything is OK!
>>
>> IMO, it doesn't make sense to have errno = 0 while you're handling an
>> exception (which, in this case, was caused exactly because a syscall
>> failed, and so we expect errno to be different than 0).
>
> This is bad design. Exception objects should be self contained
> and not rely on global state to transfer information.
OK. I implemented your idea, but I will wait until the other patches
are reviewed so I can submit a v2 of the whole series.
Thanks.
--
Sergio
GPG key ID: 237A 54B1 0287 28BF 00EF 31F4 D0EB 7628 65FC 5E36
Please send encrypted e-mail if possible
http://sergiodj.net/