Provide pid_to_exec_file on Solaris (PR tdep/17903)

  While looking through gdb.log, I found that two tests FAIL like this:

warning: No executable has been specified and target does not support
determining executable automatically.  Try using the "file" command.
0x00400dc4 in ?? ()
(gdb) FAIL: gdb.base/attach.exp: attach2, with no file

The other is gdb.base/quit-live.exp.  I've implemented the following
patch that fixes both failures, only then detecting that I'd previously
reported the issue as PR tdep/17903.

Tested on amd64-pc-solaris2.10 and amd64-pc-solaris2.11, ok for master?

	Rainer
  

Hi Rainer,

On Wed, Sep 19, 2018 at 04:17:57PM +0200, Rainer Orth wrote:
> While looking through gdb.log, I found that two tests FAIL like this:
> 
> warning: No executable has been specified and target does not support
> determining executable automatically.  Try using the "file" command.
> 0x00400dc4 in ?? ()
> (gdb) FAIL: gdb.base/attach.exp: attach2, with no file
> 
> The other is gdb.base/quit-live.exp.  I've implemented the following
> patch that fixes both failures, only then detecting that I'd previously
> reported the issue as PR tdep/17903.
> 
> Tested on amd64-pc-solaris2.10 and amd64-pc-solaris2.11, ok for master?
> 
> 	Rainer
> 
> -- 
> -----------------------------------------------------------------------------
> Rainer Orth, Center for Biotechnology, Bielefeld University
> 
> 
> 2018-06-13  Rainer Orth  <ro@CeBiTec.Uni-Bielefeld.DE>
> 
> 	PR tdep/17903
> 	* procfs.c (procfs_target): Declare pid_to_exec_file.
> 	(procfs_target::pid_to_exec_file): New.

Nice :). This is OK for me as is; one question: Have you considered
the use of an std::string for the variable "name"? I thought about it,
and I'm not sure it would make the code all that better, but thought
I'd mention it again, in case you or someone else sees something
I don't see.

> diff --git a/gdb/procfs.c b/gdb/procfs.c
> --- a/gdb/procfs.c
> +++ b/gdb/procfs.c
> @@ -128,6 +128,8 @@ public:
>  
>    const char *pid_to_str (ptid_t) override;
>  
> +  char *pid_to_exec_file (int pid) override;
> +
>    thread_control_capabilities get_thread_control_capabilities () override
>    { return tc_schedlock; }
>  
> @@ -3135,6 +3137,35 @@ procfs_target::pid_to_str (ptid_t ptid)
>    return buf;
>  }
>  
> +/* Accepts an integer PID; Returns a string representing a file that
> +   can be opened to get the symbols for the child process.  */
> +
> +char *
> +procfs_target::pid_to_exec_file (int pid)
> +{
> +  static char buf[PATH_MAX];
> +  char name[PATH_MAX];
> +
> +  /* Solaris 11 introduced /proc/<proc-id>/execname.  */
> +  xsnprintf (name, PATH_MAX, "/proc/%d/execname", pid);
> +  scoped_fd fd (open (name, O_RDONLY));
> +  if (fd.get () < 0 || read (fd.get (), buf, PATH_MAX - 1) < 0)
> +    {
> +      /* If that fails, fall back to /proc/<proc-id>/path/a.out introduced in
> +	 Solaris 10.  */
> +      ssize_t len;
> +
> +      xsnprintf (name, PATH_MAX, "/proc/%d/path/a.out", pid);
> +      len = readlink (name, buf, PATH_MAX - 1);
> +      if (len <= 0)
> +	strcpy (buf, name);
> +      else
> +	buf[len] = '\0';
> +    }
> +
> +  return buf;
> +}
> +
>  /* Insert a watchpoint.  */
>  
>  static int
  

>>>>> "Rainer" == Rainer Orth <ro@CeBiTec.Uni-Bielefeld.DE> writes:

Rainer> Tested on amd64-pc-solaris2.10 and amd64-pc-solaris2.11, ok for master?

Thank you for doing this.

Rainer> +  char *pid_to_exec_file (int pid) override;

Boy this is a bad API!
Not your problem but I'll make a note on my to-do list to change it to
return a std::string or something like that.

Rainer> +  scoped_fd fd (open (name, O_RDONLY));

You may wish to use gdb_open_cloexec.

procfs.c is one of the cloexec holdouts, because I could never compile it.
But I guess once the buildbot is up I can do it :)

Tom
  

On 2018-09-19 10:48, Tom Tromey wrote:
> procfs.c is one of the cloexec holdouts, because I could never compile 
> it.
> But I guess once the buildbot is up I can do it :)

I don't know if you are registered to the gcc compile farm announce 
mailing list, so just in case, there are now some Solaris machines 
(perhaps the same as Rainer is using for the buildbot?):

https://lists.tetaneutral.net/pipermail/cfarm-announces/2018-September/000002.html

Simon
  

Hi Joel,

>> 2018-06-13  Rainer Orth  <ro@CeBiTec.Uni-Bielefeld.DE>
>> 
>> 	PR tdep/17903
>> 	* procfs.c (procfs_target): Declare pid_to_exec_file.
>> 	(procfs_target::pid_to_exec_file): New.
>
> Nice :). This is OK for me as is; one question: Have you considered
> the use of an std::string for the variable "name"? I thought about it,
> and I'm not sure it would make the code all that better, but thought
> I'd mention it again, in case you or someone else sees something
> I don't see.

TBH, I know close to nothing about C++, so rely on others for
suggestions here.

	Rainer
  

Hi Tom,

> Rainer> +  scoped_fd fd (open (name, O_RDONLY));
>
> You may wish to use gdb_open_cloexec.

I'll give it a whirl and change the patch accordingly before committing
if it works out.

> procfs.c is one of the cloexec holdouts, because I could never compile it.
> But I guess once the buildbot is up I can do it :)

Certainly: my proposed buildbot patch adds them to the try_ssh scheduler
to allow just that.  In the meantime, you may well send procfs patches
my way if you like ;-)

	Rainer
  

Hi Simon,

> On 2018-09-19 10:48, Tom Tromey wrote:
>> procfs.c is one of the cloexec holdouts, because I could never compile
>> it.
>> But I guess once the buildbot is up I can do it :)
>
> I don't know if you are registered to the gcc compile farm announce mailing
> list, so just in case, there are now some Solaris machines (perhaps the
> same as Rainer is using for the buildbot?):
>
> https://lists.tetaneutral.net/pipermail/cfarm-announces/2018-September/000002.html

they aren't: I've got two Solaris 11.4 kernel zones (sparc and x86)
intended to run both the buildbots and in principle fit for addition to
the compile farm.  However, a transparent integration probably won't be
possible because University policy requires prior acceptance (though
online) of the Acceptable Use Policy.

	Rainer
  

> >> 	PR tdep/17903
> >> 	* procfs.c (procfs_target): Declare pid_to_exec_file.
> >> 	(procfs_target::pid_to_exec_file): New.
> >
> > Nice :). This is OK for me as is; one question: Have you considered
> > the use of an std::string for the variable "name"? I thought about it,
> > and I'm not sure it would make the code all that better, but thought
> > I'd mention it again, in case you or someone else sees something
> > I don't see.
> 
> TBH, I know close to nothing about C++, so rely on others for
> suggestions here.

In that case, I would go with what you have now considering the current
interface for this method, and possibly enhance it later as a followup.
It's not obvious to me that switching that part to C++ is a win.