Re: [igt-dev] [PATCH i-g-t] lib/igt_core: Detect gdb harder

["Ville Syrjälä" <ville.syrjala@linux.intel.com>]

On Wed, Jan 22, 2020 at 05:09:19PM +0000, Chris Wilson wrote:
> Quoting Ville Syrjala (2020-01-22 16:43:32)
> > From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > 
> > The current gdb detection only works for the parent igt process,
> > but none of its children will see the gdb and so won't trap properly
> > in igt_fail_assert(). Also we will not detect gdb if it was attached
> > after the fact. Fix all of that by looking for the "TracerPid"
> > information in /proc/<pid>/status. We'll leave the current "assume
> > parent may be gdb" approach as a fallback.
> > 
> > Also annoyingly by default gdb will only track a single process.
> > To make it track all of them, and let them all run simultanously
> > one needs the following incantations:
> >  set detach-on-fork off
> >  set schedule-multiple on
> > 
> > Maybe that will save someone from having to trawl as many
> > docs/gogole hits as I did.
> > 
> > Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > ---
> >  lib/igt_core.c | 38 ++++++++++++++++++++++++++++++++++++--
> >  1 file changed, 36 insertions(+), 2 deletions(-)
> > 
> > diff --git a/lib/igt_core.c b/lib/igt_core.c
> > index 0a0068946a6a..109b5926eac4 100644
> > --- a/lib/igt_core.c
> > +++ b/lib/igt_core.c
> > @@ -1609,12 +1609,12 @@ void igt_describe_f(const char *fmt, ...)
> >         assert(ret < sizeof(__current_description));
> >  }
> >  
> > -static bool running_under_gdb(void)
> > +static bool is_gdb(pid_t pid)
> >  {
> >         char pathname[30], buf[1024];
> >         ssize_t len;
> >  
> > -       sprintf(pathname, "/proc/%d/exe", getppid());
> > +       sprintf(pathname, "/proc/%d/exe", pid);
> >         len = readlink(pathname, buf, sizeof(buf) - 1);
> >         if (len < 0)
> >                 return false;
> > @@ -1624,6 +1624,40 @@ static bool running_under_gdb(void)
> >         return strncmp(basename(buf), "gdb", 3) == 0;
> >  }
> >  
> > +static pid_t tracer_pid(void)
> > +{
> > +       char pathname[30];
> > +       pid_t pid = 0;
> > +       FILE *f;
> > +
> > +       sprintf(pathname, "/proc/%d/status", getpid());
> > +
> > +       f = fopen(pathname, "r");
> > +       if (!f)
> > +               return getppid();
> > +
> > +       for (;;) {
> > +               char buf[1024];
> > +               char *s;
> > +
> > +               s = fgets(buf, sizeof(buf), f);
> 
> I'd prefer fgetline, /proc/<pid>status is >1024 bytes so there's always
> that chance we cut the TracerPid line.

fgets() reads at most one line (which means my 1024 copypasta is
probably a bit much). Not sure what fgetline() is.

> 
> > +               if (!s)
> > +                       break;
> > +
> > +               if (sscanf(s, "TracerPid: %d", &pid) == 1)
> > +                       break;
> > +       }
> > +
> > +       fclose(f);
> > +
> > +       return pid ?: getppid();
> 
> Makes sense.
> 
> > +}
> > +
> > +static bool running_under_gdb(void)
> > +{
> 
> /*
>  * Annoyingly by default gdb will only track a single process.
>  * To make it track all of them, and let them all run simultaneously
>  * one needs the following incantations:
>  *   set detach-on-fork off
>  *   set schedule-multiple on
>  */

Yeah, a comment is probably a good idea for this.

> 
> 
> TIL two things, so
> Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk>
> -Chris

-- 
Ville Syrjälä
Intel
_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev