Re: [PATCH 2/6] Don't reset errno/bfd_error on 'throw_perror_with_name'

[Sergio Durigan Junior <sergiodj@redhat.com>]

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

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/