[RFA] Re: x86 linux GDB and SIGALRM

[RFA] Re: x86 linux GDB and SIGALRM

[Mark Kettenis]

[ There is a comment right before this code by JimB, so maybe you want
  to comment on this Jim. ]

I think I've found the bug that's causing the problem Jonathan was
seeing.  This patch restores the behaviour of GDB 4.18.  GDB 4.18 was
released before handle_inferior_event() was introduced.  At the point
of the check_sigtramp2() call there was a "goto check_sigtramp2".
Since in the old situation after the check_sigtramp2 code the
keep_going label would be reached, I think that the we should call
keep_going() here and return.

If we don't this we'll land in the breakpoint handling code.  The
breakpoint handling code has the side-effect of cancelling any single
stepping and stopping right away.  Now if we single step, but the
OS arranges to actually pass us a pending signal before actually
stepping (which is what happens on Linux/i386), we'll never step and
we see the behaviour that Jonathan observed.

[ Perhaps I should modify the comment a bit, now that the code
  actually does keep_going. ]

Mark


2000-05-01  Mark Kettenis  <kettenis@gnu.org>

	* infrun.c (handle_inferior_event): Add missing call to keep_going
	and missing return when handling an ordinary signal from the
	inferior.


Index: infrun.c
===================================================================
RCS file: /cvs/src/src/gdb/infrun.c,v
retrieving revision 1.8
diff -u -p -r1.8 infrun.c
--- infrun.c	2000/04/20 11:00:34	1.8
+++ infrun.c	2000/05/01 18:43:13
@@ -2274,6 +2274,8 @@ handle_inferior_event (struct execution_
            the HP-UX maintainer to furnish a fix that doesn't break other
            platforms.  --JimB, 20 May 1999 */
 	check_sigtramp2 (ecs);
+	keep_going (ecs);
+	return;
       }
 
     /* Handle cases caused by hitting a breakpoint.  */
From jimb@zwingli.cygnus.com Mon May 01 14:05:00 2000
From: Jim Blandy <jimb@zwingli.cygnus.com>
To: Mark Kettenis <kettenis@wins.uva.nl>
Cc: jlarmour@redhat.co.uk, gdb@sourceware.cygnus.com, gdb-patches@sourceware.cygnus.com, ezannoni@cygnus.com
Subject: Re: [RFA] Re: x86 linux GDB and SIGALRM
Date: Mon, 01 May 2000 14:05:00 -0000
Message-id: <npd7n6gewq.fsf@zwingli.cygnus.com>
References: <38DBF46A.8EABE9BF@redhat.co.uk> <38DF3159.DF837D7D@cygnus.com> <200003271020.MAA25618@landau.wins.uva.nl> <38F245CE.A2B7B5D0@redhat.co.uk> <200005011853.e41Ir7N06091@delius.kettenis.local>
X-SW-Source: 2000-05/msg00002.html
Content-length: 2133

I last changed this code to allow GDB to step over calls to
pthread_create properly.  If you can still do a "next" over a call to
pthread_create after your change, I have no further comments.  The
test gdb.threads/linux-dp.exp ought to run on Linux and verify that
this works.



> [ There is a comment right before this code by JimB, so maybe you want
>   to comment on this Jim. ]
> 
> I think I've found the bug that's causing the problem Jonathan was
> seeing.  This patch restores the behaviour of GDB 4.18.  GDB 4.18 was
> released before handle_inferior_event() was introduced.  At the point
> of the check_sigtramp2() call there was a "goto check_sigtramp2".
> Since in the old situation after the check_sigtramp2 code the
> keep_going label would be reached, I think that the we should call
> keep_going() here and return.
> 
> If we don't this we'll land in the breakpoint handling code.  The
> breakpoint handling code has the side-effect of cancelling any single
> stepping and stopping right away.  Now if we single step, but the
> OS arranges to actually pass us a pending signal before actually
> stepping (which is what happens on Linux/i386), we'll never step and
> we see the behaviour that Jonathan observed.
> 
> [ Perhaps I should modify the comment a bit, now that the code
>   actually does keep_going. ]
> 
> Mark
> 
> 
> 2000-05-01  Mark Kettenis  <kettenis@gnu.org>
> 
> 	* infrun.c (handle_inferior_event): Add missing call to keep_going
> 	and missing return when handling an ordinary signal from the
> 	inferior.
> 
> 
> Index: infrun.c
> ===================================================================
> RCS file: /cvs/src/src/gdb/infrun.c,v
> retrieving revision 1.8
> diff -u -p -r1.8 infrun.c
> --- infrun.c	2000/04/20 11:00:34	1.8
> +++ infrun.c	2000/05/01 18:43:13
> @@ -2274,6 +2274,8 @@ handle_inferior_event (struct execution_
>             the HP-UX maintainer to furnish a fix that doesn't break other
>             platforms.  --JimB, 20 May 1999 */
>  	check_sigtramp2 (ecs);
> +	keep_going (ecs);
> +	return;
>        }
>  
>      /* Handle cases caused by hitting a breakpoint.  */
> 
From Peter.Schauer@regent.e-technik.tu-muenchen.de Tue May 02 04:30:00 2000
From: "Peter.Schauer" <Peter.Schauer@regent.e-technik.tu-muenchen.de>
To: gdb@sourceware.cygnus.com
Cc: ezannoni@cygnus.com
Subject: Trunk GDB dumps core in solib.c:symbol_add_stub
Date: Tue, 02 May 2000 04:30:00 -0000
Message-id: <200005021130.NAA13067@reisser.regent.e-technik.tu-muenchen.de>
X-SW-Source: 2000-05/msg00003.html
Content-length: 1993

This change:

2000-04-17  Elena Zannoni  <ezannoni@kwikemart.cygnus.com>

	* solib.c (symbol_add_stub): The special field text_addr is not
 	available anymore. Search for the .text field the hard way.

causes a GDB core dump on Solaris x86 as soon as a simple main is linked
against -lintl (which contains no .text section) and GDB is told to run to
to main:

pes@eno_2439$ cat main.c
int
main()
{
  return 0;
}
pes@eno_2440$ gcc -g main.c -lintl
pes@eno_2441$ gdb a.out
GNU gdb 20000204
Copyright 2000 Free Software Foundation, Inc.
GDB is free software, covered by the GNU General Public License, and you are
welcome to change it and/or distribute copies of it under certain conditions.
Type "show copying" to see the conditions.
There is absolutely no warranty for GDB.  Type "show warranty" for details.
This GDB was configured as "i386-pc-solaris2.6"...
(gdb) b main
Breakpoint 1 at 0x8048a4b: file main.c, line 4.
(gdb) r
Starting program: /users/pes/tst/a.out 
Segmentation Fault(coredump)

The patch below gets rid of the core dump, but I am not sure if it
does the right thing (don't we have to consider the lowest section ?).

--- gdb/solib.c.orig	Thu Apr 20 17:36:56 2000
+++ gdb/solib.c	Sun Apr 30 12:14:52 2000
@@ -1188,11 +1188,15 @@ symbol_add_stub (arg)
 
   /* Look for the index for the .text section in the sap structure. */
   text_section = bfd_get_section_by_name (so->abfd, ".text");
-  for (i = 0; i < MAX_SECTIONS && sap->other[i].name; i++)
-    if (sap->other[i].sectindex == text_section->index)
-      break;
-  
-  sap->other[i].addr = text_addr;
+  if (text_section != NULL)
+    {
+      for (i = 0; i < MAX_SECTIONS && sap->other[i].name; i++)
+	if (sap->other[i].sectindex == text_section->index)
+	  break;
+
+      if (i < MAX_SECTIONS)
+	sap->other[i].addr = text_addr;
+    }
   so->objfile = symbol_file_add (so->so_name, so->from_tty,
 				 sap, 0, OBJF_SHARED);
   free_section_addr_info (sap);

-- 
Peter Schauer			pes@regent.e-technik.tu-muenchen.de
From shebs@shebs.cnchost.com Tue May 02 06:04:00 2000
From: Stan Shebs <shebs@shebs.cnchost.com>
To: gdb@sourceware.cygnus.com
Subject: Re: [RFA] Re: x86 linux GDB and SIGALRM
Date: Tue, 02 May 2000 06:04:00 -0000
Message-id: <390ED3FE.B08587F9@shebs.cnchost.com>
X-SW-Source: 2000-05/msg00004.html
Content-length: 998

Mark Kettenis sez:
>
> I think I've found the bug that's causing the problem Jonathan was
> seeing.  This patch restores the behaviour of GDB 4.18.  GDB 4.18 was
> released before handle_inferior_event() was introduced.  At the point
> of the check_sigtramp2() call there was a "goto check_sigtramp2".
> Since in the old situation after the check_sigtramp2 code the
> keep_going label would be reached, I think that the we should call
> keep_going() here and return.

I agree.  I remember looking at that case when converting from wfi,
and convincing myself that the keep_going() wasn't necessary; but
there were a lot of control flow cases, and it's quite likely
that a previous mistake in the conversion process misled me.
(Are the giant printouts still on the wall in the Cygnus Sunnyvale
office?)

Note that since my mistake didn't cause any regressions in the GDB
testsuite, we really really need to add Jonathan's nice example
to the testsuite, so that we exercise this path regularly.

Stan
From kettenis@wins.uva.nl Tue May 02 07:40:00 2000
From: Mark Kettenis <kettenis@wins.uva.nl>
To: shebs@shebs.cnchost.com
Cc: gdb@sourceware.cygnus.com
Subject: Re: [RFA] Re: x86 linux GDB and SIGALRM
Date: Tue, 02 May 2000 07:40:00 -0000
Message-id: <200005021440.QAA17176@landau.wins.uva.nl>
References: <390ED3FE.B08587F9@shebs.cnchost.com>
X-SW-Source: 2000-05/msg00005.html
Content-length: 352

   Date: Tue, 02 May 2000 06:11:27 -0700
   From: Stan Shebs <shebs@shebs.cnchost.com>

   Note that since my mistake didn't cause any regressions in the GDB
   testsuite, we really really need to add Jonathan's nice example
   to the testsuite, so that we exercise this path regularly.

I'll try to convert his test program into a dejagnu test.

Mark
From ezannoni@cygnus.com Tue May 02 09:13:00 2000
From: Elena Zannoni <ezannoni@cygnus.com>
To: "Peter.Schauer" <Peter.Schauer@Regent.E-Technik.TU-Muenchen.DE>
Cc: gdb@sourceware.cygnus.com, ezannoni@cygnus.com
Subject: Re: Trunk GDB dumps core in solib.c:symbol_add_stub
Date: Tue, 02 May 2000 09:13:00 -0000
Message-id: <14606.65183.752921.400435@kwikemart.cygnus.com>
References: <200005021130.NAA13067@reisser.regent.e-technik.tu-muenchen.de>
X-SW-Source: 2000-05/msg00006.html
Content-length: 4838

Ah, sorry about that. Thanks for catching it.
I am afraid then that other solib type files may have the same problem.
I'll take a look.


 > 
 > The patch below gets rid of the core dump, but I am not sure if it
 > does the right thing (don't we have to consider the lowest section ?).
 > 
 > --- gdb/solib.c.orig	Thu Apr 20 17:36:56 2000
 > +++ gdb/solib.c	Sun Apr 30 12:14:52 2000
 > @@ -1188,11 +1188,15 @@ symbol_add_stub (arg)
 >  
 >    /* Look for the index for the .text section in the sap structure. */
 >    text_section = bfd_get_section_by_name (so->abfd, ".text");
 > -  for (i = 0; i < MAX_SECTIONS && sap->other[i].name; i++)
 > -    if (sap->other[i].sectindex == text_section->index)
 > -      break;
 > -  
 > -  sap->other[i].addr = text_addr;
 > +  if (text_section != NULL)
 > +    {
 > +      for (i = 0; i < MAX_SECTIONS && sap->other[i].name; i++)
 > +	if (sap->other[i].sectindex == text_section->index)
 > +	  break;
 > +
 > +      if (i < MAX_SECTIONS)
 > +	sap->other[i].addr = text_addr;
 > +    }
 >    so->objfile = symbol_file_add (so->so_name, so->from_tty,
 >  				 sap, 0, OBJF_SHARED);
 >    free_section_addr_info (sap);
 > 

You are right. If lowest_sect is not .text that won't work.  How about
this? All we really need to do is keep track of the index of the
lowest section, and change the offset address for that section only,
right?  The real problem is that I don't quite understand what
syms_from_objfile does with lower_offset. The comments in there are
confusing.

Elena

Index: solib.c
===================================================================
RCS file: /cvs/src/src/gdb/solib.c,v
retrieving revision 1.11
diff -c -r1.11 solib.c
*** solib.c	2000/04/17 16:09:04	1.11
--- solib.c	2000/05/02 15:55:07
***************
*** 1153,1162 ****
       PTR arg;
  {
    register struct so_list *so = (struct so_list *) arg;  /* catch_errs bogon */
-   CORE_ADDR text_addr = 0;
    struct section_addr_info *sap;
!   int i;
!   asection *text_section;
  
    /* Have we already loaded this shared object?  */
    ALL_OBJFILES (so->objfile)
--- 1153,1162 ----
       PTR arg;
  {
    register struct so_list *so = (struct so_list *) arg;  /* catch_errs bogon */
    struct section_addr_info *sap;
!   CORE_ADDR lowest_addr = 0;
!   int lowest_index;
!   asection *lowest_sect = NULL;
  
    /* Have we already loaded this shared object?  */
    ALL_OBJFILES (so->objfile)
***************
*** 1167,1198 ****
  
    /* Find the shared object's text segment.  */
    if (so->textsection)
!     text_addr = so->textsection->addr;
    else if (so->abfd != NULL)
      {
!       asection *lowest_sect;
! 
!       /* If we didn't find a mapped non zero sized .text section, set up
!          text_addr so that the relocation in symbol_file_add does no harm.  */
        lowest_sect = bfd_get_section_by_name (so->abfd, ".text");
        if (lowest_sect == NULL)
  	bfd_map_over_sections (so->abfd, find_lowest_section,
  			       (PTR) &lowest_sect);
        if (lowest_sect)
! 	text_addr = bfd_section_vma (so->abfd, lowest_sect)
! 	  + LM_ADDR (so);
      }
  
    sap = build_section_addr_info_from_section_table (so->sections,
                                                      so->sections_end);
  
-   /* Look for the index for the .text section in the sap structure. */
-   text_section = bfd_get_section_by_name (so->abfd, ".text");
-   for (i = 0; i < MAX_SECTIONS && sap->other[i].name; i++)
-     if (sap->other[i].sectindex == text_section->index)
-       break;
-   
-   sap->other[i].addr = text_addr;
    so->objfile = symbol_file_add (so->so_name, so->from_tty,
  				 sap, 0, OBJF_SHARED);
    free_section_addr_info (sap);
--- 1167,1199 ----
  
    /* Find the shared object's text segment.  */
    if (so->textsection)
!     {
!       lowest_addr = so->textsection->addr;
!       lowest_sect = bfd_get_section_by_name (so->abfd, ".text");
!       lowest_index = lowest_sect->index;
!     }
    else if (so->abfd != NULL)
      {
!       /* If we didn't find a mapped non zero sized .text section, set
!          up lowest_addr so that the relocation in symbol_file_add does
!          no harm.  */
        lowest_sect = bfd_get_section_by_name (so->abfd, ".text");
        if (lowest_sect == NULL)
  	bfd_map_over_sections (so->abfd, find_lowest_section,
  			       (PTR) &lowest_sect);
        if (lowest_sect)
! 	{
! 	  lowest_addr = bfd_section_vma (so->abfd, lowest_sect)
! 	    + LM_ADDR (so);
! 	  lowest_index = lowest_sect->index;
! 	}
      }
  
    sap = build_section_addr_info_from_section_table (so->sections,
                                                      so->sections_end);
+ 
+   sap->other[lowest_index].addr = lowest_addr;
  
    so->objfile = symbol_file_add (so->so_name, so->from_tty,
  				 sap, 0, OBJF_SHARED);
    free_section_addr_info (sap);