Re: gcore support for AIX

["Ulrich Weigand" <uweigand at de dot ibm dot com>]

Sangamesh Mallaya wrote:

> I have came across a way to add gencore support for the AIX.
> Need to your input and suggestions on current implementation or is their
> any other way this can be done ?
> 
> Some background on the gencore command/API on AIX.
> 
> AIX provides a gencore command and gencore api for generating a
> core of a running program. If debugging a process through gdb,
> gdb uses ptrace to control the prcoess execution.
> 
> In this case all threads of process must have stopped by
> gdb and gencore will hang if gencore command or API is used on the
> process which gdb is debugging.
> 
> In order to keep it from hanging, gdb would need to continue one thread
> in the process for as little as one instruction.
> If a thread is continued, then when it runs it will be resumed such that
> the core signal will be handled by that thread of the process and the
> core file will be written.

I'd prefer if you could find another way to just do whatever it is
that "gencore" is doing directly in GDB.  Doing this extra "step"
would make the semantics of the generate-core-file command different
on AIX than it is on any other platform, which seems confusing to
end users.

> In this implementation gencore api will be called through a separate gdb
> thread
> which will be scheduled and gencore will be executed after a call to
> "step_1_ext (0, 1, "1")" which will call step_1 to execute single
> instruction.

I really don't like this either :-)  GDB currently is not multi-threaded,
and making it so just on one platform for this one special case seems odd.
I'd rather just fork and exec the gencore command instead, if we do have
to use it ...


Some comments on the patch itself:

> +#if (GDB_OSABI_DEFAULT == GDB_OSABI_AIX)

We really try to avoid this type of #ifdef conditional code.  Platform-
specific code should be encapsulated into target callback routines.
There are already some of those related to gcore.  If those do not
suffice to implement this feature on AIX, they should be extended
or new ones added.  The actual implementation can then be provided in
rs6000-nat.c

> +#include <core.h>
> +#include <pthread.h>
> +#include <sys/stat.h>
> +#include <libgen.h>
> +struct aix_gencore_args {
> +    char *name;
> +    pid_t pid; 
> +};

This shouldn't be in the header file.

> +extern void step_1_ext (int, int, char *);

This should be in an appropriate header if we need it, not here.

> +           /* Their can be a slight delay in scheduling gencore call after gdb's
> +              single step for an multi-threaded programs, and above checks might
> +              fail. But eventually we will be having a core file generated from
> +              the gencore. So display informative message that core will be 
> +              created. */
> +           fprintf_filtered (gdb_stdout,
> +                          "corefile might be saved as '%s' \n", aix_core); 

This isn't nice :-)  Shouldn't it be possible to wait on completion of the
gencore process, simply by waiting on termination of the thread (or process)
running it?

> +void step_1_ext (int skip, int inst, char *string)
> +{
> +    step_1 (skip, inst, string);
> +}

What is the point of creating a new routine here?

> --- ./include/libiberty.h_orig	2017-10-10 06:15:51 -0500
> +++ ./include/libiberty.h	2017-10-10 06:49:24 -0500
> @@ -105,6 +105,7 @@
>     to find the declaration so provide a fully prototyped one.  If it
>     is 1, we found it so don't provide any declaration at all.  */
>  #if !HAVE_DECL_BASENAME
> +#ifndef _AIX
>  #if defined (__GNU_LIBRARY__ ) || defined (__linux__) \
>   || defined (__FreeBSD__) || defined (__OpenBSD__) || defined (__NetBSD__) \
>   || defined (__CYGWIN__) || defined (__CYGWIN32__) || defined (__MINGW32__) \
> @@ -117,6 +118,7 @@
>  #define basename basename_cannot_be_used_without_a_prototype
>  #endif
>  #endif
> +#endif

libiberty patches need to be handled separately since libiberty is
shared between multiple projects, in particular including GCC.

What does this have to do with the gencore patch anyway?  It doesn't
appear to be using basename in the first place?

Bye,
Ulrich 

-- 
  Dr. Ulrich Weigand
  GNU/Linux compilers and toolchain
  Ulrich.Weigand@de.ibm.com