Re: [RFC] Adding new files, for new port to Interix (Services For Unix)

[Mark Kettenis <kettenis at chello dot nl>]

[ Sorry Joel for the duplicate, I forgot to CC gdb-patches. ]

Joel Brobecker <brobecker@gnat.com> writes:

> --uQr8t48UFsdbeI+V
> Content-Type: text/plain; charset=us-ascii
> Content-Disposition: inline
> 
> Hello,
> 
> ACT is working on the integration of some changes that would allow GDB
> to be built on interix (Services For Unix). The port has been done by
> Donn Terry, with a bit of light rework here and there from myself.

Thanks for the contribution.

> The port was initially made some time ago so it was not multi-arched.
> I started the conversion, and part of it is done. I haven't finished the
> conversion, because I need a bit of help for the remaining part
> to avoid spending too much time going in the wrong direction.

Ok, I reviewed the new files.  I hope my comments will be useful to you.

> I am attaching to this message some files that I'd like to add to the
> GDB trunk. These are only new files, and some other changes are still
> needed for GDB to first configure properly, and then build.
> 
> My current plan regarding the inclusion of this new port is the
> following:
>   - check-in the attached files, possibly (probably) with your comments
>     incorporated
>   - check-in the changes in the configury machinery in order for GDB to
>     configure
>   - check-in the minimal changes to allow GDB to build
>   - And then finally check-in the remaining changes that make GDB work.
>     For the curious, my regression testsuite results show about 6600
>     passes, and 1200 failures.

Looks like a good strategy to me.

> Any comments on the attached files, as well as the strategy above would
> be greatly appreciated. I'd like to commit these new files as soon as
> acceptable, as it makes it easier for me to discuss new changes and also
> facilitate the management of these files.

A general remark: It looks like the port was started a while back, and
not all changes in coding conventions for GDB that we had in the
meantime have been followed:

* Comments should always be full sentences in the sense that they
  start with a capital and end with a full stop (dot).  After a full
  stop we use two spaces.

* Don't use the keyword `register'.

* Use ISO C prototypes.

* Use spaces after every ',' and before and after '=', '+=' etc.

Success!

Mark

> --uQr8t48UFsdbeI+V
> Content-Type: text/plain; charset=us-ascii
> Content-Description: i386-interix-nat.c
> Content-Disposition: attachment; filename="i386-interix-nat.c"
> 
> /* Native-dependent code for Interix running on i386's, for GDB.
>    Copyright 2002 Free Software Foundation, Inc.
> 
> This file is part of GDB.
> 
> This program is free software; you can redistribute it and/or modify
> it under the terms of the GNU General Public License as published by
> the Free Software Foundation; either version 2 of the License, or
> (at your option) any later version.
> 
> This program is distributed in the hope that it will be useful,
> but WITHOUT ANY WARRANTY; without even the implied warranty of
> MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> GNU General Public License for more details.
> 
> You should have received a copy of the GNU General Public License
> along with this program; if not, write to the Free Software
> Foundation, Inc., 59 Temple Place - Suite 330, Boston, MA 02111-1307, USA.  */
> 
> #include "defs.h"
> 
> #include <sys/procfs.h>
> #include <ucontext.h>
> #include <frame.h>
> #include <inferior.h>
> #include <fcntl.h>
> 
> #include <i386-tdep.h>
> #include "symfile.h"
> #include "objfiles.h"
> #include "gdb-stabs.h"
> #include "gdbcore.h"
> #include "gregset.h"
> 
> 
> typedef unsigned long greg_t;  /* Where should this be defined? */

It should be in <sys/reg.h> if Interix is trying too look like SVR4.

> 
> /* See procfs.c and *interix*.h in config/[alpha,i386] */
> 
> /*  The /proc interface traditionally divides the target machine's register 
>     set up into two different sets, the general register set (gregset) 
>     and the floating point register set (fpregset).   This is reflected
>     in the data structures in <sys/regset.h>.
> 
>     However, the read/write /proc for interix treats them all as part
>     of the status subfile. 
> 
>     These routines provide the packing and unpacking of gregset_t and
>     fpregset_t formatted data.
> 
>  */
> 
> /* This is a duplicate of the table in i386-xdep.c. */

This statement is defenitely not true.

> static int regmap[] = 
> {
>   EAX, ECX, EDX, EBX,
>   UESP, EBP, ESI, EDI,
>   EIP, EFL, CS, SS,
>   DS, ES, FS, GS,
> };
> 
> /*  Given a pointer to a general register set in /proc format (gregset_t *),
>     unpack the register contents and supply them as gdb's idea of the current
>     register values. */
> 
> void
> supply_gregset (gregsetp)
>      gregset_t *gregsetp;
> {
>   register int regi;
>   register greg_t *regp = (greg_t *) &gregsetp->gregs;
> 
>   for (regi = 0; regi < I386_NUM_GREGS; regi++)
>     {
>       supply_register (regi, (char *) (regp + regmap[regi]));
>     }
> }
> 
> /* Fill in a gregset_t object with selected data from a gdb-format
>    register file.
>    - GREGSETP points to the gregset_t object to be filled.
>    - GDB_REGS points to the GDB-style register file providing the data.
>    - VALID is an array indicating which registers in GDB_REGS are
>      valid; the parts of *GREGSETP that would hold registers marked
>      invalid in GDB_REGS are left unchanged.  If VALID is zero, all
>      registers are assumed to be valid.  */
> void
> convert_to_gregset (gregset_t *gregsetp,
> 		    char *gdb_regs,
> 		    signed char *valid)
> {
>   int regi;
>   register greg_t *regp = (greg_t *) gregsetp->gregs;
> 
>   for (regi = 0; regi < I386_NUM_GREGS; regi++)
>     if (! valid || valid[regi])
>       *(regp + regmap[regi]) = * (int *) &registers[REGISTER_BYTE (regi)];
> }

Using `registers' is no longer allowed.

> 
> /* Store GDB's value for REGNO in *GREGSETP.  If REGNO is -1, do all
>    of them.  */
> void
> fill_gregset (gregset_t *gregsetp,
> 	      int regno)
> {
>   if (regno == -1)
>     convert_to_gregset (gregsetp, registers, 0);
>   else if (regno >= 0 && regno < I386_NUM_GREGS)
>     {
>       signed char valid[I386_NUM_GREGS];
>       memset (valid, 0, sizeof (valid));
>       valid[regno] = 1;
>       convert_to_gregset (gregsetp, registers, valid);
>     }
> }

Why this complicated stuff with two functions and an extra array necessary?

> /*  Given a pointer to a floating point register set in /proc format
>     (fpregset_t *), unpack the register contents and supply them as gdb's
>     idea of the current floating point register values. */
> 
> /* What is the address of st(N) within the fpregset_t structure F?  */
> #define FPREGSET_T_FPREG_ADDR(f, n) \
>   ((char *) &(f)->fpstate.fpregs + (n) * 10)
> 
> /* Fill GDB's register file with the floating-point register values in
>    *FPREGSETP.  */
> void 
> supply_fpregset (fpregset_t *fpregsetp)
> {
>   int i;
>   long l;
> 
>   /* Supply the floating-point registers.  */
>   for (i = 0; i < 8; i++)
>     supply_register (FP0_REGNUM + i, FPREGSET_T_FPREG_ADDR (fpregsetp, i));
> 
>   l = fpregsetp->fpstate.control & 0xffff;
>   supply_register (FCTRL_REGNUM, (char *) &l);
>   l = fpregsetp->fpstate.status & 0xffff;
>   supply_register (FSTAT_REGNUM, (char *) &l);
>   l = fpregsetp->fpstate.tag & 0xffff;
>   supply_register (FTAG_REGNUM,  (char *) &l);
>   supply_register (FCOFF_REGNUM, (char *) &fpregsetp->fpstate.erroroffset);
>   l = fpregsetp->fpstate.dataselector & 0xffff;
>   supply_register (FDS_REGNUM,   (char *) &l);
>   supply_register (FDOFF_REGNUM, (char *) &fpregsetp->fpstate.dataoffset);
>   
>   /* Extract the code segment and opcode from the  "fcs" member.  */
> 
>   l = fpregsetp->fpstate.errorselector & 0xffff;
>   supply_register (FCS_REGNUM, (char *) &l);
> 
>   l = (fpregsetp->fpstate.errorselector >> 16) & ((1 << 11) - 1);
>   supply_register (FOP_REGNUM, (char *) &l);
> }
> 
> 
> /* Fill in an fpregset_t structure with selected data from a
>    gdb-format register file.
>    - FPREGSETP points to the structure to be filled. 
>    - GDB_REGS points to the GDB-style register file providing the data.
>    - VALID is an array indicating which registers in GDB_REGS are
>      valid; the parts of *FPREGSETP that would hold registers marked
>      invalid in GDB_REGS are left unchanged.  If VALID is zero, all
>      registers are assumed to be valid.  */
> void
> convert_to_fpregset (fpregset_t *fpregsetp,
> 		     char *gdb_regs,
> 		     signed char *valid)
> {
>   int i;
> 
>   /* Fill in the floating-point registers.  */
>   for (i = 0; i < 8; i++)
>     if (!valid || valid[i])
>       memcpy (FPREGSET_T_FPREG_ADDR (fpregsetp, i),
> 	      &registers[REGISTER_BYTE (FP0_REGNUM + i)],
> 	      REGISTER_RAW_SIZE(FP0_REGNUM + i));
> 
> #define fill(MEMBER, REGNO)						      \
>   if (! valid || valid[(REGNO)])					      \
>     memcpy (&fpregsetp->fpstate.MEMBER, &registers[REGISTER_BYTE (REGNO)],    \
> 	    sizeof (fpregsetp->fpstate.MEMBER))
> 
>   fill (control, FCTRL_REGNUM);
>   fill (status, FSTAT_REGNUM);
>   fill (tag, FTAG_REGNUM);
>   fill (erroroffset, FCOFF_REGNUM);
>   fill (dataoffset, FDOFF_REGNUM);
>   fill (dataselector, FDS_REGNUM);
> 
> #undef fill
> 
>   if (! valid || valid[FCS_REGNUM])
>     fpregsetp->fpstate.errorselector
>       = ((fpregsetp->fpstate.errorselector & ~0xffff)
> 	 | (* (int *) &registers[REGISTER_BYTE (FCS_REGNUM)] & 0xffff));
> 
>   if (! valid || valid[FOP_REGNUM])
>     fpregsetp->fpstate.errorselector
>       = ((fpregsetp->fpstate.errorselector & 0xffff)
> 	 | ((*(int *) &registers[REGISTER_BYTE (FOP_REGNUM)] & ((1 << 11) - 1))
> 	    << 16));
> }
> 
> 
> /* Given a pointer to a floating point register set in (fpregset_t *)
>    format, update all of the registers from gdb's idea of the current
>    floating point register set.  */
> 
> void
> fill_fpregset (fpregset_t *fpregsetp,
> 	       int regno)
> {
>   convert_to_fpregset (fpregsetp, registers, 0);
> }

Is there any reason you can't use i387_supply_fsave and
i387_fill_fsave here?  I would be seriously surprised if you couldn't.

That said, please consider using i386v4-nat.c for Interix.  Apart from
the greg_t typedef issue that should work without problems, and I'm
certainly willing to add something like:

   #ifndef HAVE_GREG_T
   typedef unsigned long greg_t;
   #endif

to that file together with a suitable autoconf check.

> /*
> 
>    GLOBAL FUNCTION
> 
>    fetch_core_registers -- fetch current registers from core file
> 
>    SYNOPSIS
> 
>    void fetch_core_registers (char *core_reg_sect,
>    unsigned core_reg_size,
>    int which, CORE_ADDR reg_addr)
> 
>    DESCRIPTION
> 
>    Read the values of either the general register set (WHICH equals 0)
>    or the floating point register set (WHICH equals 2) from the core
>    file data (pointed to by CORE_REG_SECT), and update gdb's idea of
>    their current values.  The CORE_REG_SIZE parameter is compared to
>    the size of the gregset or fpgregset structures (as appropriate) to
>    validate the size of the structure from the core file.  The
>    REG_ADDR parameter is ignored.
> 
>  */

Please change this comment to only the description.

> 
> static void
> fetch_core_registers (char *core_reg_sect, unsigned core_reg_size, int which,
> 		      CORE_ADDR reg_addr)
> {
>   gdb_gregset_t gregset;
>   gdb_fpregset_t fpregset;
> 
>   if (which == 0)
>     {
>       if (core_reg_size != sizeof (gregset))
> 	{
> 	  warning ("wrong size gregset struct in core file");
> 	}
>       else
> 	{
> 	  memcpy ((char *) &gregset, core_reg_sect, sizeof (gregset));
> 	  supply_gregset (&gregset);
> 	}
>     }
>   else if (which == 2)
>     {
>       if (core_reg_size != sizeof (fpregset))
> 	{
> 	  warning ("wrong size fpregset struct in core file");
> 	}
>       else
> 	{
> 	  memcpy ((char *) &fpregset, core_reg_sect, sizeof (fpregset));
> 	  if (FP0_REGNUM >= 0)
> 	    supply_fpregset (&fpregset);
> 	}
>     }
> }

If that check for FP0_REGNUM is really necessary (it is if there are
Interix sub-targets that don't include the floating point registers),
please change it to:

   if (FP0_REGNUM > 0)

> 
> /* the name of the proc status struct depends on the implementation */
> #ifdef HAVE_PSTATUS_T
>   typedef pstatus_t gdb_prstatus_t;
> #else
>   typedef prstatus_t gdb_prstatus_t;
> #endif

What's this doing here?  These typedefs aren't used anywhere in this file.

> /* Figure out where the longjmp will land.
>    We expect the first arg to be a pointer to the jmp_buf structure from which
>    we extract the pc that we will land at.  The pc is copied into PC.
>    This routine returns true on success. */
> 
> #include <setjmp.h>
> 
> int
> get_longjmp_target (pc)
>      CORE_ADDR *pc;
> {
>   CORE_ADDR sp, jb_addr;
>   char raw_buffer[MAX_REGISTER_RAW_SIZE];
> 
>   sp = read_register (SP_REGNUM);
> 
>   if (target_read_memory (sp + SP_ARG0, /* Offset of first arg on stack */
> 			  raw_buffer,
> 			  TARGET_PTR_BIT / TARGET_CHAR_BIT))
>     return 0;
> 
>   jb_addr = extract_address (raw_buffer, TARGET_PTR_BIT / TARGET_CHAR_BIT);
> 
>   if (target_read_memory(jb_addr + offsetof(_JUMP_BUFFER,Eip), raw_buffer,
> 			 sizeof(CORE_ADDR)))
>     return 0;
> 
>   *pc = extract_address (raw_buffer, sizeof(CORE_ADDR));
>   return 1;
> }

This function does not beling in a *-nat.c file but in a *-tdep.c file.
Is there any reason why the new i386-tdep.c:i386_get_longjmp_target()
cannot be used?  Just setting tdep->jb_pc_offset to the numeric value
of offsetof (_JMP_BUFFER, Eip) should work fine AFAICT.

> static struct core_fns interix_core_fns =
> {
>   bfd_target_coff_flavour,		/* core_flavour  (more or less) */
>   default_check_format,			/* check_format */
>   default_core_sniffer,			/* core_sniffer */
>   fetch_core_registers,		        /* core_read_registers */
>   NULL					/* next */
> };
> 
> void
> _initialize_core_interix (void)
> {
>   add_core_fns (&interix_core_fns);
>   if (TARGET_LONG_BIT != 32)
>     printf_unfiltered ("ERROR: TARGET_LONG_BIT = %d /= 32!\n", TARGET_LONG_BIT);
>   if (TARGET_LONG_LONG_BIT != 64)
>     printf_unfiltered
>       ("ERROR: TARGET_LONG_LONG_BIT = %d /= 64!\n", TARGET_LONG_LONG_BIT);
> }

What is the purpose for the TARGET_LONG_BIT and TARGET_LONG_LONG_BIT
checks here?

> /* Adjust the section offsets in an objfile structure so that it's correct
>    for the type of symbols being read (or undo it with the _restore
>    arguments).  
> 
>    If main programs ever start showing up at other than the default Image
>    Base, this is where that would likely be applied. */
> void
> pei_adjust_objfile_offsets(struct objfile *objfile, enum objfile_adjusts type)
> {
>   int i;
>   CORE_ADDR symbols_offset;
> 
>   switch (type) {
>   case adjust_for_symtab:
>       symbols_offset = NONZERO_LINK_BASE(objfile->obfd);
>       break;
>   case adjust_for_symtab_restore:
>       symbols_offset = -NONZERO_LINK_BASE(objfile->obfd);
>       break;
>   case adjust_for_stabs:
>   case adjust_for_stabs_restore:
>   case adjust_for_dwarf:
>   case adjust_for_dwarf_restore:
>   default:
>       return;
>   }
> 
>   for (i = 0; i < SECT_OFF_MAX; i++)
>    {
>      (objfile->section_offsets)->offsets[i] += symbols_offset;
>    }
> }

Seems to me that this function doesn't belong in *-nat.c but in *-tdep.c.

> /* We don't have a /proc/pid/file or /proc/pid/exe to read a link from,
>    so read it from the same place ps gets the name.  */
> 
> char *
> child_pid_to_exec_file (int pid)
> {
>   char *path;
>   char *buf;
>   int fd, c;
>   char *p;
> 
>   asprintf (&path, "/proc/%d/stat", pid);
>   buf = xcalloc (MAXPATHLEN+1, sizeof (char));
>   make_cleanup (xfree, path);
>   make_cleanup (xfree, buf);
> 
>   fd = open (path, O_RDONLY);
> 
>   if (fd < 0) 
>       return NULL;
> 
>   /* Skip over "Argv0\t" */
>   lseek (fd, 6, SEEK_SET);
> 
>   c = read (fd, buf, MAXPATHLEN);
>   close (fd);
> 
>   if (c < 0) 
>       return NULL;
> 
>   buf[c] = '\0';  /* Ensure null termintion. */
>   p = strchr(buf, '\n');
>   if (p != NULL)
>      *p = '\0';
> 
>   return buf;
> }

> --uQr8t48UFsdbeI+V
> Content-Type: text/plain; charset=us-ascii
> Content-Description: i386-interix-tdep.c
> Content-Disposition: attachment; filename="i386-interix-tdep.c"
> 
> /* Native-dependent code for Interix running on i386's, for GDB.
>    Copyright 2002 Free Software Foundation, Inc.
> 
> This file is part of GDB.
> 
> This program is free software; you can redistribute it and/or modify
> it under the terms of the GNU General Public License as published by
> the Free Software Foundation; either version 2 of the License, or
> (at your option) any later version.
> 
> This program is distributed in the hope that it will be useful,
> but WITHOUT ANY WARRANTY; without even the implied warranty of
> MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> GNU General Public License for more details.
> 
> You should have received a copy of the GNU General Public License
> along with this program; if not, write to the Free Software
> Foundation, Inc., 59 Temple Place - Suite 330, Boston, MA 02111-1307, USA.  */
> 
> #include "defs.h"
> #include "arch-utils.h"
> 
> #include "frame.h"
> #include "gdb_string.h"
> #include "gdbcore.h"
> #include "gdbtypes.h"
> #include "inferior.h"
> #include "libbfd.h"
> #include "osabi.h"
> 
> /* FIXME: This is not right, we should not be including this file in
>    -tdep module. But this is needed by interix_back_one_frame. */
> #include <sys/procfs.h>

Yup that defenitely should be fixed before checking in this code.  It
seems that this is neccessary since you use `offset (mcontext_t,
...)'.  Can't you replace that by a constant, or does the value of
those expressions depend on the particular version of Interix or
Windhoos (Sorry that's the name we use for a particular OS we're not
too fond of at the company where I work; means something like
whirlwind).

> /* ??? Why do we need an extern here??? */
> extern CORE_ADDR skip_trampoline_code (CORE_ADDR pc, char *name);

Better add the prototype to i386-tdep.h.

> /* Some constants describing the i386 interix target */
> static const int i386_interix_long_bit = 32;
> static const int i386_interix_long_long_bit = 64;
> static const int i386_interix_ptr_bit = 32;
> static const CORE_ADDR i386_interix_decr_pc_after_break = 0;

What's the purpose of these constants?  Apart from decr_pc_after_break
these are just the default values for the i386 target.

> /* See procfs.c and *interix*.h in config/[alpha,i386] */
> /* ??? These should be static, but this needs a bit of work before this
>    can be done.  */
> CORE_ADDR tramp_start;
> CORE_ADDR tramp_end;
> CORE_ADDR null_start;
> CORE_ADDR null_end;
> int winver; /* Windows NT version number */

The fact that it needs some more work, isn't a problem for me.  Can be
done after this code has been checked in.

> static int
> i386_interix_pc_in_sigtramp (CORE_ADDR pc, char *name)
> {
>   /* This is sufficient, where used, but is NOT a complete test; There
>      is more in INIT_EXTRA_FRAME_INFO (a.k.a. interix_back_one_frame) */
>   return (pc >= tramp_start && pc < tramp_end)
>          || (pc >= null_start && pc < null_end);
> }
> 
> static int
> i386_interix_in_solib_call_trampoline (CORE_ADDR pc, char *name)
> {
>   return skip_trampoline_code (pc, name);
> }
> 
> static CORE_ADDR
> i386_interix_skip_trampoline_code (CORE_ADDR pc)
> {
>   return skip_trampoline_code (pc, 0);
> }
> 
> static void
> i386_interix_init_frame_pc (int fromleaf, struct frame_info *prev)
> {
>   /* Nothing to do on Interix */
> }
> 
> static int
> i386_interix_frame_chain_valid (CORE_ADDR chain, struct frame_info *thisframe)
> {
>   /* In the context where this is used, we get the saved PC before we've
>      successfully unwound far enough to be sure what we've got (it may
>      be a signal handler caller).  If we're dealing with a signal
>      handler caller, this will return valid, which is fine.  If not,
>      it'll make the correct test. */
>   return thisframe->signal_handler_caller
>          || (chain != 0
>              && !inside_entry_file (read_memory_integer (thisframe->frame + 4,
>                                                          4)));
> }
> 
> /* We want to find the previous frame, which on Interix is tricky when signals
>    are involved; set frame->frame appropriately, and also get the pc
>    and tweak signal_handler_caller; this replaces a boatload of nested
>    macros, as well. */
> static void
> i386_interix_back_one_frame (fromleaf, frame)
>      int fromleaf;
>      struct frame_info *frame;
> {
>   CORE_ADDR ra;
>   CORE_ADDR fm;
>   CORE_ADDR context;
>   long t;
> 
>   if (frame == NULL)
>       abort();
>   
>   if (fromleaf)
>     {
>        frame->pc = SAVED_PC_AFTER_CALL (frame->next);
>        return;
>     }
> 
>   if (!frame->next)
>     {
>        frame->pc = read_pc();
> 
>        /* part of the signal stuff... see below */
>        if (stopped_by_random_signal)
> 	 {
> 	   /* we know we're in a system call mini-frame; was it
> 	      NullApi or something else? */
>            ra = SAVED_PC_AFTER_CALL (frame);
>            if (ra >= null_start && ra < null_end)
> 	      frame->signal_handler_caller = 1;
> 	   /* There might also be an indirect call to the mini-frame,
> 	      putting one more return address on the stack.  (XP only,
> 	      I think?)  This can't (reasonably) return the address of the 
> 	      signal handler caller unless it's that situation, so this
> 	      is safe. */
>            ra = read_memory_unsigned_integer (
> 	       (CORE_ADDR) read_register (SP_REGNUM)+4, 4);
>            if (ra >= null_start && ra < null_end)
> 	      frame->signal_handler_caller = 1;
> 	 }
>        return;
>     }
> 
>   if (!frame->next->signal_handler_caller)
>     {
>       frame->pc = (CORE_ADDR)read_memory_integer (frame->next->frame + 4, 4);
>       return;
>     }
> 
>   /* This is messy... (actually AWFUL) the "trampoline" might be 2, 3 
>      or all 5 entities on the frame. 
> 
>      Chunk 1 will be present when we're actually in a signal handler.
>      Chunk 2 will be present when an asynchronous signal (one that
> 	didn't come in with a system call) is present.
> 	We may not (yet) be in the handler, if we're just returning
> 	from the call.
>      When we're actually in a handler taken from an asynchronous
>      signal, both will be present.
> 
>      Chunk 1:
> 	PdxSignalDeliverer's frame 
>       + Context struct    -- not accounted for in any frame
> 
>      Chunk 2:
>       + PdxNullPosixApi's frame 
>       + PdxNullApiCaller's frame
>       + Context struct = 0x230  not accounted for in any frame
> 
>      The symbol names come from examining objdumps of psxdll.dll;
>      they don't appear in the runtime image.
> 
>      For gdb's purposes, we can pile all this into one frame.
>   */
> 
>   ra = frame->next->pc;
>   /* Are we already pointing at PdxNullPosixApi?  We are if
>      this is a signal frame, we're at next-to-top, and were stopped
>      by a random signal.  (If it wasn't the right address under
>      these circumstances, we wouldn't be here at all by tests above
>      on the prior frame.) */
>   if (frame->next->next == NULL && stopped_by_random_signal) 
>     {
>       /* We're pointing at the frame FOR PdxNullApi */
>       fm = frame->frame;
>     }
>   else
>     {
>       /* No... we must be pointing at the frame that
> 	 was called by PdxSignalDeliverer; back up across the
> 	 whole mess */
> 
>       /* extract the frame for PdxSignalDeliverer; note...FRAME_CHAIN
> 	 used the "old" frame pointer because we were a deliverer.
> 	 Get the address of the context record that's on here frameless */
>       context = read_memory_integer (frame->frame, 4);  /* an Arg */
> 
>       /* Now extract the frame pointer contained in the context */
>       fm = (CORE_ADDR)read_memory_integer 
> 	 (context + offsetof(mcontext_t, gregs.gregs[EBP]), 4);
> 
>       ra = (CORE_ADDR)read_memory_integer 
> 	 (context + offsetof(mcontext_t, gregs.gregs[EIP]), 4);
> 
>       /* We need to know if we're in a system call because we'll be
> 	 in a syscall mini-frame, if so, and the rules are different;
> 	 reserved[1] contains 0 if running free, 1 if blocked on a system
> 	 call, and 2 if blocked on an exception message (e.g. a trap);
> 	 we don't expect to get here with a 2. */
>       t = (long)read_memory_integer 
> 	 (context + offsetof(mcontext_t, gregs.reserved[1]), 4);
>       if (t != 1)
> 	{
> 	   /* not at a system call, therefore it can't be NullApi */
> 	   frame->pc = ra;
> 	   frame->frame = fm;
> 	   return;
> 	}
> 
>       /* It's a system call... mini frame, then look for NullApi */
>       /* Get the RA (on the stack) associated with this... it's
> 	 a system call mini-frame */
>       ra = (CORE_ADDR)read_memory_integer 
> 	 (context + offsetof(mcontext_t, gregs.gregs[UESP]), 4);
> 
>       if (winver>=51) {
> 	  /* Newer versions of Windows NT interpose another return
> 	     address (but no other "stack frame" stuff) that we need
> 	     to simply ignore here. */
> 	 ra+=4;
>       }
> 
>       ra = (CORE_ADDR)read_memory_integer(ra,4);
> 
>       /* We might be done (no Null API if so) */
>       if (!(ra >= null_start && ra < null_end))
> 	{
> 	   /* No Null API present; we're done */
> 	   frame->pc = ra;
> 	   frame->frame = fm;
> 	   return;
> 	}
>     }
>   /* At this point, we're looking at the frame for PdxNullPosixApi,
>      in either case.
> 
>      PdxNullPosixApi is called by PdxNullApiCaller (which in turn
>      is called by _PdxNullApiCaller (note the _).)
>      PdxNullPosixApiCaller (no _) is a frameless function.
> 
>      The saved frame pointer is as fm, but it's not of interest
>      to us because it skips us over the saved context, which is
>      the wrong thing to do, because it skips the interrrupted
>      routine!  PdxNullApiCaller takes as its only argument the
>      address of the context of the interrupded function (which
>      is really in no frame, but jammed on the stack by the system)
> 
>      So: fm+0: saved bp
>          fm+4: return address to _PdxNullApiCaller
>          fm+8: arg to PdxNullApiCaller pushed by _Pdx...  */
> 
>   fm = (CORE_ADDR)read_memory_integer (fm+0x8, 4); 
> 
>   /* Extract the second context record */
> 
>   ra = (CORE_ADDR)read_memory_integer 
>      (fm + offsetof(mcontext_t, gregs.gregs[EIP]), 4);
>   fm = (CORE_ADDR)read_memory_integer 
>      (fm + offsetof(mcontext_t, gregs.gregs[EBP]), 4);
> 
>   frame->frame = fm;
>   frame->pc = ra;
> 
>   return;
> }
> 
> static CORE_ADDR
> i386_interix_frame_saved_pc (struct frame_info *fi)
> {
>   /* Assume that we've already unwound enough to have the caller's address
>      if we're dealing with a signal handler caller.  (And if that fails,
>      return 0.)  */
>   if (fi->signal_handler_caller)
>     return fi->next ? fi->next->pc : 0;
>   else
>     return (CORE_ADDR) read_memory_integer (fi->frame + 4, 4);
> }
> 
> static int
> i386_interix_use_struct_convention (int gcc_p, struct type *type)
> {
>   /* Directly copied from nbsd where this function is no longer defined,
>      as a generic mechanism is used. Maybe we should take advantage of
>      this mechanism too? */
>   return !(TYPE_LENGTH (type) == 1
>            || TYPE_LENGTH (type) == 2
>            || TYPE_LENGTH (type) == 4
>            || TYPE_LENGTH (type) == 8);
> }


Please use the generic mechanism.  Just set tdep->struct_return to
reg_struct_return.

> void
> i386_interix_cache_trampoline_addresses_from_bfd (void)
> {
>   /* This is painful, but there doesn't seem to be a better way.
>      See interix tm.h, IN_SIGTRAMP, and procfs.c for more details. */
>   asection *section;
>   pstatus_t *p;
>   
>   section = bfd_get_section_by_name (core_bfd, ".pstatus");
>   if (section == NULL)
>     {
>       /* This should never happen, we are just being extra cautious.  */
>       warning (".pstatus section not found.");
>       return;
>     }
> 
>   p = (pstatus_t *)section->contents;
>   if (p == NULL)
>     {
>       p = (pstatus_t *)bfd_alloc(core_bfd, section->_raw_size);
>       bfd_get_section_contents (core_bfd, section, p, 0, section->_raw_size);
>     }
>   
>   tramp_start = (CORE_ADDR)p->pr_signaldeliverer;
>   tramp_end = (CORE_ADDR)p->pr_endsignaldeliverer;
>   null_start = (CORE_ADDR)p->pr_nullapi;
>   null_end = (CORE_ADDR)p->pr_endnullapi;
> }
> 
> void
> i386_interix_cache_trampoline_addresses (CORE_ADDR trampoline_start,
>                                          CORE_ADDR trampoline_end,
>                                          CORE_ADDR nullapi_start,
>                                          CORE_ADDR nullapi_end)
> {
>   /* On Interix, the location of the trampoline code can
>      float, and it's not part of the symbol table (because
>      it's internal to a DLL).  However, the system knows where
>      it is, and tells us, so we capture that info here. */
>   /* While we're doing such ugly things, find out which
>      version of NT this is, so we can change our unwind rules
>      a bit.
>      FIXME: If we move this, move the include of utsname.h, too.*/
>   
>   tramp_start = trampoline_start;
>   tramp_end = trampoline_end;
>   null_start = nullapi_start;
>   null_end = nullapi_end;
> }
> 
> static void
> i386_interix_init_abi (struct gdbarch_info info, struct gdbarch *gdbarch)
> {
>   /* ??? Externs, temporarily defined  */
>   extern int get_longjmp_target (CORE_ADDR *pc);
> 
>   struct gdbarch_tdep *tdep = gdbarch_tdep (gdbarch);
> 
>   set_gdbarch_long_bit (gdbarch, i386_interix_long_bit);
>   set_gdbarch_long_long_bit (gdbarch, i386_interix_long_long_bit);
>   set_gdbarch_use_struct_convention (gdbarch,
>                                      i386_interix_use_struct_convention);
>   set_gdbarch_ptr_bit (gdbarch, i386_interix_ptr_bit);

See earlier comments.

>   set_gdbarch_decr_pc_after_break (gdbarch, i386_interix_decr_pc_after_break);

Just using 0 here is fine by me.  It's obvious where to look for it.

>   set_gdbarch_pc_in_sigtramp (gdbarch, i386_interix_pc_in_sigtramp);
>   /* ??? The following may already be the default on i386 architecture  */
>   set_gdbarch_in_solib_call_trampoline (gdbarch,
>                                         i386_interix_in_solib_call_trampoline);

It isn't the default on the i386 target, so removing this comment is fine.

>   set_gdbarch_skip_trampoline_code (gdbarch, i386_interix_skip_trampoline_code);
>   set_gdbarch_init_extra_frame_info (gdbarch, i386_interix_back_one_frame);
>   set_gdbarch_init_frame_pc (gdbarch, i386_interix_init_frame_pc);
>   set_gdbarch_frame_chain_valid (gdbarch, i386_interix_frame_chain_valid);
>   set_gdbarch_frame_saved_pc (gdbarch, i386_interix_frame_saved_pc);
>   /* ??? the following may already be the default for i386 architecture  */
>   set_gdbarch_get_longjmp_target (gdbarch, get_longjmp_target);

Yes, please use the generic mechanism as indicated by the comments in
the Interix *-nat.c file.

> }
> 
> static enum gdb_osabi
> i386_interix_osabi_sniffer (bfd *abfd)
> {
>   char *target_name = bfd_get_target (abfd);
> 
>   if (strcmp (target_name, "pei-i386") == 0)
>     return GDB_OSABI_INTERIX;
> 
>   return GDB_OSABI_UNKNOWN;
> }
> 
> void
> _initialize_i386_interix_tdep (void)
> {
>   gdbarch_register_osabi_sniffer (bfd_arch_i386, bfd_target_coff_flavour,
> 				  i386_interix_osabi_sniffer);
> 
>   gdbarch_register_osabi (bfd_arch_i386, GDB_OSABI_INTERIX,
> 			  i386_interix_init_abi);
> }
> 
> 
> --uQr8t48UFsdbeI+V
> Content-Type: text/plain; charset=us-ascii
> Content-Description: config/i386/tm-i386interix.h
> Content-Disposition: attachment; filename="tm-i386interix.h"
> 
> /* Macro definitions for i386 running under Interix
>    Copyright 2002 Free Software Foundation, Inc.
> 
> This file is part of GDB.
> 
> This program is free software; you can redistribute it and/or modify
> it under the terms of the GNU General Public License as published by
> the Free Software Foundation; either version 2 of the License, or
> (at your option) any later version.
> 
> This program is distributed in the hope that it will be useful,
> but WITHOUT ANY WARRANTY; without even the implied warranty of
> MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> GNU General Public License for more details.
> 
> You should have received a copy of the GNU General Public License
> along with this program; if not, write to the Free Software
> Foundation, Inc., 59 Temple Place - Suite 330, Boston, MA 02111-1307, USA.  */
> 
> #ifndef TM_INTERIX_H
> #define TM_INTERIX_H
> 
> #include "i386/tm-i386.h"
> 
> /* configure can't be expected to noodle out MS's alternate spelling for
>    64-bit integers, so we'll tell it. */
> #define PRINTF_HAS_LONG_LONG 1
> #ifdef __GNUC__
> #define BFD_HOST_64_BIT long long
> #define BFD_HOST_U_64_BIT unsigned long long
> #elif defined(_MSC_VER)
> #define BFD_HOST_64_BIT __int64
> #define BFD_HOST_U_64_BIT unsigned __int64
> #else
> #error... OK what compiler is this?
> #endif
> 
> #undef LONGEST
> #define LONGEST BFD_HOST_64_BIT
> 
> #undef ULONGEST
> #define ULONGEST BFD_HOST_U_64_BIT

Well, then configure should be taught about it.  We might tolerate
some hacks like this but not in the tm-*.h files.  Put it a xm-*.h
file, but bether yet avoid it if you can: Add an appropriate configure
check for PRINTF_HAS_LONG_LONG and fix the BFD_HOST_* handling in BFD.
See the *-*-windows*) case in bfd/configure.host.

> #define I386_FLOATS_RETURN_IN_ST0

What's the purpose of this?  The standard i386 target already stores
floating point return values in $st(0), and I don't see any use of
this macro in these files.

> /* Interix has a minimal sbrk() (but not what's wanted for this usage),
>    and it's relationship to environ[] is not what's usually expected
>    (as in, there is no specific relationship at all).  Just pretend we don't
>    have an sbrk(). */
> #undef HAVE_SBRK

That defenitely belongs in an xm-*.h file.

> #define ADJUST_OBJFILE_OFFSETS(objfile, type) \
>     pei_adjust_objfile_offsets(objfile, type)

What's the purpose of this.  This macro is used nowhere in the entire
GDB tree.

> extern CORE_ADDR bfd_getImageBase(bfd *abfd);
> #define NONZERO_LINK_BASE(abfd) bfd_getImageBase(abfd)

Eventually you'll have to find another location for these defines.
Why not put them into *-tdep.c right from the start?

> #define NAME_OF_MALLOC "_malloc"

What's this for?
> 
> #endif /* TM_INTERIX_H */


The bottom line seems to be that we don't need a special tm-*.h for
Interix.  Please use tm-i386.h for this new target.

> --uQr8t48UFsdbeI+V
> Content-Type: text/plain; charset=us-ascii
> Content-Description: config/i386/nm-interix.h
> Content-Disposition: attachment; filename="nm-interix.h"
> 
> /* Native-dependent definitions for Intel 386 running Interix, for GDB.
>    Copyright 2002 Free Software Foundation, Inc.
> 
> This file is part of GDB.
> 
> This program is free software; you can redistribute it and/or modify
> it under the terms of the GNU General Public License as published by
> the Free Software Foundation; either version 2 of the License, or
> (at your option) any later version.
> 
> This program is distributed in the hope that it will be useful,
> but WITHOUT ANY WARRANTY; without even the implied warranty of
> MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> GNU General Public License for more details.
> 
> You should have received a copy of the GNU General Public License
> along with this program; if not, write to the Free Software
> Foundation, Inc., 59 Temple Place - Suite 330, Boston, MA 02111-1307, USA.  */
> 
> #ifndef NM_INTERIX_H
> #define NM_INTERIX_H
> 
> #define USE_PROC_FS
> 
> /* submodes of USE_PROC_FS */
> #define PROCFS_USE_READ_WRITE
> #define HAVE_MULTIPLE_PROC_FDS
> #define UNIXWARE
> #define HAVE_STRUCT_LWP
> 
> #define ATTACH_DETACH
> 
> /* Be shared lib aware */
> #include "solib.h"
> 
> /* This is the amount to subtract from u.u_ar0
>    to get the offset in the core file of the register values.  */
> 
> #if 0
> #include <machine/vmparam.h>
> #define KERNEL_U_ADDR USRSTACK
> #endif
> 
> #define REGISTER_U_ADDR(addr, blockend, regno) \
> 	(addr) = i386_register_u_addr ((blockend),(regno));
> 
> extern int
> i386_register_u_addr PARAMS ((int, int));
> 
> #define PTRACE_ARG3_TYPE char*
> 
> /* Return sizeof user struct to callers in less machine dependent routines */
> 
> #define KERNEL_U_SIZE kernel_u_size()
> extern int kernel_u_size PARAMS ((void));

What's the purpose of all these Unix-specific defines on Interix?

> /* It's ALMOST coff; bfd does the same thing */
> #define COFF_WITH_PE
> #define COFF_IMAGE_WITH_PE

These are referenced nowhere in the entire GDB source tree.

> /* Turn on our own child_pid_to_exec_file. */
> #define CHILD_PID_TO_EXEC_FILE
> 
> #endif /* NM_INTERIX_H */
> 
> --uQr8t48UFsdbeI+V
> Content-Type: text/plain; charset=us-ascii
> Content-Description: config/i386/interix.mh
> Content-Disposition: attachment; filename="interix.mh"
> 
> # Host: Intel 386 running Interix
> XDEPFILES= 
> NATDEPFILES= corelow.o core-regset.o fork-child.o i386-interix-nat.o \
> 	procfs.o proc-api.o proc-events.o proc-flags.o proc-why.o
> NAT_FILE= nm-interix.h
> # The below may be temporary; mmalloc relies on sbrk() at the moment
> MMALLOC=
> MMALLOC_CFLAGS=-DNO_MMALLOC
> 
> --uQr8t48UFsdbeI+V
> Content-Type: text/plain; charset=us-ascii
> Content-Description: config/i386/interix.mt
> Content-Disposition: attachment; filename="interix.mt"
> 
> # Target: Intel 386 running Interix
> TDEPFILES= i386-tdep.o i387-tdep.o i386-interix-tdep.o \
>         solib.o solib-pei.o i386nbsd-tdep.o

i386nbsd-tdep.o ???

> TM_FILE= tm-i386interix.h
> 
> --uQr8t48UFsdbeI+V--