This is the mail archive of the gdb-patches@sources.redhat.com mailing list for the GDB project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

S390 GDB patch for Dignus Linux/390 compiler support


At the end of this email is a patch to gdb-5.1.1/gdb/s390-tdep.c.
This patch adds another condition in s390_get_frame_info() to
understand the function prologues generated by the Dignus
Systems/C and Systems/C++ 390 compilers (which can run on Linux/390)

The change is to support using S (subtract) instead of AHI for
decrementing the stack pointer on entry to a function.  We do not
use AHI because our compiler is made to work on older chips where
AHI is not available.  There is code in s390-tdep.c to handle a
similar prologue GCC will generate for extremely large stack frames,
but it is not quite identical because we store the offset in our
general constant pool (rather than generating a BRAS for a single
constant).  I also made it no longer use const_pool_state to
determine the value of good_prologue at the end of the function
because with gcc it can be 0 or 2 and with Dignus it is always 1
(we do not use an AHI to adjust it, because our constant pool works
so differently).  That covers all of the values it can have, so it's
always good so far as I'm concerned.

I'm also attaching a version of this patch built against
5.1.90_20020315 (almost the same).  The "current" tree appears to
have an identical s390-tdep.c, so it should work on that too.

Here's the ChangeLog for that patch:

2002-03-15  Greg Alexander  <greg@dignus.com>

	* s390-tdep.c: support Dignus compiler prologues



There is another issue which concerns me, though, when it comes to
S390 support.  The 5.1.1 sources did not compile for S390
straight out of the box.  There were four similar cases where I
had to add an ampersand:
proc-service.c:236 (in ps_lgetregs)
proc-service.c:253 (in ps_lsetregs)
thread-db.c:804 (in thread_db_fetch_registers)
thread-db.c:837 (in thread_db_store_registers)

These are all calls to fill_gregset() or supply_gregset():
  fill_gregset ((gdb_gregset_t *) gregset, -1);
  supply_gregset ((gdb_gregset_t *) gregset);

It turns out, tracing back all the typedefs, that this cast is wrong.
gregset is of type gdb_gregset_t, so casting it to gdb_gregset_t*
doesn't make much sense.  On the platforms I checked (except S390)
the first thing these functions do is take their argument and cast
it back to gdb_gregset_t.  So overall it has no effect on the code,
because gdb_gregset_t is, on most platforms, already a pointer type.
However, on S390, gdb_gregset_t is a structure type, and the
fill_gregset() and supply_gregset() (s390-nat.c) functions expect
a pointer to the structure.  The really easy/hacky fix is to add
something like #ifdef S390 around the affected calls, but that
seems unacceptable.  Alternatively, though it's a bit of effort to
determine which typedefs exactly need to change, we could make the
s390 gdb_gregset_t structure actually be a pointer to the structure.
But that seems wrong, too, because we still have all these calls
that are passing around the wrong type.  Ideally, someone would
decide how these functions really ought to be prototyped, and
adjust all implementations and calls to use the proper arguments.
Do you want me to do this?  To adjust them all to actually use the
types as they are defined now (i.e., add the ampersands in the
callers and derefs in all of the implementations) shouldn't take
too much time.

We can't just leave the common code alone and change the s390-nat.c
definition of the fill_gregset/supply_gregset functions because
that cast only succeeds at compile-time in the specific circumstance
where gdb_gregset_t is already a pointer (or scalar) type.

It seems that as of now, anybody compiling the S390 gdb simply
by hand adds the ampersands (this was present in one of the patches
from D.J. Barrow).  Is this really the current build methodology
for gdb, or am I missing something?

Thank you everyone for GDB!  I use it every day.
Especially thanks to D.J. Barrow at IBM for all the work on the 
Linux/390 port.

- Greg

--------patch against 5.1.1 release-------------
--- s390-tdep.c	2002/02/26 16:34:06	1.1
+++ s390-tdep.c	2002/03/15 16:47:42
@@ -204,6 +204,7 @@
   bfd_byte instr[S390_MAX_INSTR_SIZE];
   CORE_ADDR test_pc = pc, test_pc2;
   CORE_ADDR orig_sp = 0, save_reg_addr = 0, *saved_regs = NULL;
+  CORE_ADDR constant_pool_addr = 0;
   int valid_prologue, good_prologue = 0;
   int gprs_saved[S390_NUM_GPRS];
   int fprs_saved[S390_NUM_FPRS];
@@ -398,11 +399,13 @@
 	  else
 	    {
 	      /* Check for BASR gpr13,gpr0 used to load constant pool pointer to r13 in old compiler */
+	      /* Also used by Dignus compilers */
 	      if (instr[0] == 0xd && (instr[1] & 0xf) == 0
 		  && ((instr[1] >> 4) == CONST_POOL_REGIDX))
 		{
 		  const_pool_state = 1;
 		  valid_prologue = 1;
+		  constant_pool_addr = test_pc + instrlen;
 		  continue;
 		}
 	    }
@@ -500,6 +503,28 @@
 	  valid_prologue = 1;
 	  continue;
 	}
+      /* Alternatively check for a S or SG R15,offs(CONST_POOL_REGIDX)
+	 emitted by Dignus compilers.
+	 constant_pool_addr must be set upon encountering a
+	 BASR to set CONST_POOL_REGIDX. */
+      if ((save_link_state == 1) && (const_pool_state == 1)
+	  && (constant_pool_addr != 0)
+	  && ((GDB_TARGET_IS_ESAME
+	       && (instr[0] == 0xe3) && (instr[1] == 0xf0)
+	       && ((instr[2]>>4) == CONST_POOL_REGIDX)
+	       && (instr[5] == 0x09))
+	      || ((instr[0] == 0x5b) && (instr[1] == 0xf0)
+	          && ((instr[2]>>4) == CONST_POOL_REGIDX))))
+	{
+	  unsigned int offs = ((instr[2] & 0x0f) << 8) + instr[3];
+	  if (fextra_info)
+	    target_read_memory (constant_pool_addr + offs,
+				(char *) &fextra_info->stack_bought,
+				sizeof (fextra_info->stack_bought));
+	  save_link_state = 3;
+	  valid_prologue = 1;
+	  continue;
+	}
       /* check for LA gprx,offset(15) used for varargs */
       if ((instr[0] == 0x41) && ((instr[2] >> 4) == 0xf) &&
 	  ((instr[1] & 0xf) == 0))
@@ -557,7 +582,6 @@
   if (good_prologue)
     {
       good_prologue = (((got_state == 0) || (got_state == 2)) &&
-		       ((const_pool_state == 0) || (const_pool_state == 2)) &&
 		       ((save_link_state == 0) || (save_link_state == 4)) &&
 		       ((varargs_state == 0) || (varargs_state == 2)));
     }
--------END OF PATCH-------------
--------patch against 5.1.90_20020315 "branch"-------------

--- s390-tdep.c.orig	Sun Feb 24 17:31:19 2002
+++ s390-tdep.c	Fri Mar 15 16:18:35 2002
@@ -219,6 +219,7 @@
   bfd_byte instr[S390_MAX_INSTR_SIZE];
   CORE_ADDR test_pc = pc, test_pc2;
   CORE_ADDR orig_sp = 0, save_reg_addr = 0, *saved_regs = NULL;
+  CORE_ADDR constant_pool_addr = 0;
   int valid_prologue, good_prologue = 0;
   int gprs_saved[S390_NUM_GPRS];
   int fprs_saved[S390_NUM_FPRS];
@@ -482,11 +483,13 @@
 	  else
 	    {
 	      /* Check for BASR gpr13,gpr0 used to load constant pool pointer to r13 in old compiler */
+	      /* Also used by Dignus compilers */
 	      if (instr[0] == 0xd && (instr[1] & 0xf) == 0
 		  && ((instr[1] >> 4) == CONST_POOL_REGIDX))
 		{
 		  const_pool_state = 1;
 		  valid_prologue = 1;
+		  constant_pool_addr = test_pc + instrlen;
 		  continue;
 		}
 	    }
@@ -585,6 +588,28 @@
 	  valid_prologue = 1;
 	  continue;
 	}
+      /* Alternatively check for a S or SG R15,offs(CONST_POOL_REGIDX)
+	 emitted by Dignus compilers.
+	 constant_pool_addr must be set upon encountering a
+	 BASR to set CONST_POOL_REGIDX. */
+      if ((save_link_state == 1) && (const_pool_state == 1)
+	  && (constant_pool_addr != 0)
+	  && ((GDB_TARGET_IS_ESAME
+	       && (instr[0] == 0xe3) && (instr[1] == 0xf0)
+	       && ((instr[2]>>4) == CONST_POOL_REGIDX)
+	       && (instr[5] == 0x09))
+	      || ((instr[0] == 0x5b) && (instr[1] == 0xf0)
+	          && ((instr[2]>>4) == CONST_POOL_REGIDX))))
+	{
+	  unsigned int offs = ((instr[2] & 0x0f) << 8) + instr[3];
+	  if (fextra_info)
+	    target_read_memory (constant_pool_addr + offs,
+				(char *) &fextra_info->stack_bought,
+				sizeof (fextra_info->stack_bought));
+	  save_link_state = 3;
+	  valid_prologue = 1;
+	  continue;
+	}
       /* check for LA gprx,offset(15) used for varargs */
       if ((instr[0] == 0x41) && ((instr[2] >> 4) == 0xf) &&
 	  ((instr[1] & 0xf) == 0))
@@ -656,8 +681,7 @@
           instrlen = got_load_len;
         }
         
-      good_prologue = (((const_pool_state == 0) || (const_pool_state == 2)) &&
-		       ((save_link_state == 0) || (save_link_state == 4)) &&
+      good_prologue = (((save_link_state == 0) || (save_link_state == 4)) &&
 		       ((varargs_state == 0) || (varargs_state == 2)));
     }
   if (fextra_info)
--------END OF PATCH-------------


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]