Re: Runtiming hw watchpoints; Was: Split up Z packet enable/disable cmd

[Eli Zaretskii <eliz@delorie.com>]

> Date: Fri, 04 Aug 2000 13:51:05 +0000
> From: Fernando Nasser <fnasser@cygnus.com>
> 
> > >       i = hw_watchpoint_used_count (bp_type, &other_type_used);
> > >       target_resources_ok =
> > > !       TARGET_CAN_USE_HARDWARE_WATCHPOINT (bp_type, j, i + mem_cnt,
> > >                                             other_type_used);
> > 
> > The problem here is that the mere _count_ of the used up break- and
> > watch-points is not enough to return an accurate answer to the
> > question asked by TARGET_CAN_USE_HARDWARE_WATCHPOINT.  At least in the
> > DJGPP case (and I believe for every other x86 target, too), I need to
> > know the addresses and the size of watched regions of all watchpoints,
> > and also the addresses of all hardware-assisted breakpoints.  If
> > TARGET_CAN_USE_HARDWARE_WATCHPOINT will get that info, it could be
> > implemented so as to return a meaningful result.
> 
> That is taken care of in breakpoints.c also.  The function can_use_hardware_watchpoint()
> checks how many hardware registers will be necessary for implementing a watchpoint.
> It may need fix or parametrization, but it is in there.

can_use_hardware_watchpoint could do this if it gets the info about all
the watchpoints and hardware breakpoints defined previously.
Currently, it only gets information about one single watchpoint at a
time, and that is not enough, at least on x86.

Alternatively, we could request the target to remember all watchpoints
that GDB intends to insert when it resumes the debuggee, but that
calls for a change in the API, since currently
can_use_hardware_watchpoint does not guarantee the watchpoint will
actually be inserted.

> You see, nobody even know that these things are there because the macros/functions to 
> do the actual check for most targets did not get implemented.

I completely agree.  It took me quite a while, at the time, to
untangle the logic of the decisions made by
can_use_hardware_watchpoint and TARGET_CAN_USE_HARDWARE_WATCHPOINT,
and then get them right by introducing
TARGET_REGION_OK_FOR_HW_WATCHPOINT (before this, GDB only tested the
size of the region with TARGET_REGION_SIZE_OK_FOR_HW_WATCHPOINT).

One problem with that code is that, originally, it was written for
Sparc Lite (I think) where the hardware watchpoints are handled in a
very different way.
From eliz@delorie.com Fri Aug 04 11:11:00 2000
From: Eli Zaretskii <eliz@delorie.com>
To: robert.hoehne@gmx.net
Cc: gdb-patches@sourceware.cygnus.com
Subject: Re: Initialize the correct cwd for the child
Date: Fri, 04 Aug 2000 11:11:00 -0000
Message-id: <200008041811.OAA28514@indy.delorie.com>
References: <200008041341.PAA15581@robby.dittmannsdorf.de>
X-SW-Source: 2000-08/msg00101.html
Content-length: 693

> From: Robert Hoehne <robert.hoehne@gmx.net>
> Date: Fri, 4 Aug 2000 15:43:26 +0200
> 
> On DJGPP, the child's working directory have to be remembered.
> In the current implementation this is done once, when gdb is started.
> But this is wrong, since the user can change it with the gdb-command
> 
> cd <some-directory>
> 
> and when running the child, it is expected, that it get this directory as
> its starting directory. So I made a patch for this.

Thanks for the patch.  However, I wonder: what would be the cwd of an
inferior on Unix, after the following commands:

	 gdb foo
	 cd bar
	 run

I think the DJGPP port should, as much as possible, behave the same
way as GDB does on Unix.
From msnyder@redhat.com Fri Aug 04 11:13:00 2000
From: Michael Snyder <msnyder@redhat.com>
To: Eli Zaretskii <eliz@is.elta.co.il>
Cc: robert.hoehne@gmx.net, gdb-patches@sourceware.cygnus.com
Subject: Re: Initialize the correct cwd for the child
Date: Fri, 04 Aug 2000 11:13:00 -0000
Message-id: <398B07C5.10E3@redhat.com>
References: <200008041341.PAA15581@robby.dittmannsdorf.de> <200008041811.OAA28514@indy.delorie.com>
X-SW-Source: 2000-08/msg00102.html
Content-length: 874

Eli Zaretskii wrote:
> 
> > From: Robert Hoehne <robert.hoehne@gmx.net>
> > Date: Fri, 4 Aug 2000 15:43:26 +0200
> >
> > On DJGPP, the child's working directory have to be remembered.
> > In the current implementation this is done once, when gdb is started.
> > But this is wrong, since the user can change it with the gdb-command
> >
> > cd <some-directory>
> >
> > and when running the child, it is expected, that it get this directory as
> > its starting directory. So I made a patch for this.
> 
> Thanks for the patch.  However, I wonder: what would be the cwd of an
> inferior on Unix, after the following commands:
> 
>          gdb foo
>          cd bar
>          run
> 
> I think the DJGPP port should, as much as possible, behave the same
> way as GDB does on Unix.

I'm pretty sure the cwd would be bar, so the patch
would make djgpp behave more like unix does.
From kevinb@cygnus.com Fri Aug 04 11:43:00 2000
From: Kevin Buettner <kevinb@cygnus.com>
To: Elena Zannoni <ezannoni@cygnus.com>
Cc: gdb-patches@sourceware.cygnus.com
Subject: Re: [PATCH RFA]: Fix hash table bugs in minsyms.c
Date: Fri, 04 Aug 2000 11:43:00 -0000
Message-id: <1000804184305.ZM5237@ocotillo.lan>
References: <1000804003817.ZM3200@ocotillo.lan> <14730.64814.976208.581246@kwikemart.cygnus.com> <ezannoni@cygnus.com>
X-SW-Source: 2000-08/msg00103.html
Content-length: 166

On Aug 4,  1:28pm, Elena Zannoni wrote:

> Kevin Buettner writes:
>  > I request approval for committing the patch below.
> 
> Go ahead, it makes sense. 

Committed.
From fnasser@cygnus.com Fri Aug 04 12:12:00 2000
From: Fernando Nasser <fnasser@cygnus.com>
To: Eli Zaretskii <eliz@is.elta.co.il>
Cc: ac131313@cygnus.com, gdb-patches@sourceware.cygnus.com
Subject: Re: Runtiming hw watchpoints; Was: Split up Z packet enable/disable cmd
Date: Fri, 04 Aug 2000 12:12:00 -0000
Message-id: <398B1590.27E611B8@cygnus.com>
References: <3987DD9A.21F2B29A@cygnus.com> <39884DFA.12DD4043@cygnus.com> <3988E54B.39646794@cygnus.com> <200008030602.CAA26491@indy.delorie.com> <39898EA6.4683E9F2@cygnus.com> <200008031820.OAA27128@indy.delorie.com> <3989C2A4.C5322586@cygnus.com> <200008040922.FAA28046@indy.delorie.com> <398ACA49.8C933672@cygnus.com> <200008041810.OAA28510@indy.delorie.com>
X-SW-Source: 2000-08/msg00104.html
Content-length: 1578

Eli Zaretskii wrote:
> 
> t could do this if it gets the info about all
> the watchpoints and hardware breakpoints defined previously.
> Currently, it only gets information about one single watchpoint at a
> time, and that is not enough, at least on x86.
> 
You are absolutely right.  I missed that.  We can add a field in the breakpoint
structure and store the number of hw registers used by a hw watchpoint.
Then we can fix hw_watchpoint_used_count() to add them up.
Does it sound right?


> I completely agree.  It took me quite a while, at the time, to
> untangle the logic of the decisions made by
> can_use_hardware_watchpoint and TARGET_CAN_USE_HARDWARE_WATCHPOINT,
> and then get them right by introducing
> TARGET_REGION_OK_FOR_HW_WATCHPOINT (before this, GDB only tested the
> size of the region with TARGET_REGION_SIZE_OK_FOR_HW_WATCHPOINT).
> 
> One problem with that code is that, originally, it was written for
> Sparc Lite (I think) where the hardware watchpoints are handled in a
> very different way.

Right again.  We will probably have to fix things a bit to make them more
generic.  But I guess this will only happen if we try to use it for good.
No pain, no gain :-)

I am merging my Pentium III changes to the current sourceware code.
I will retest things and post some patches as soon as I get them to
work again.  We can start from that.

-- 
Fernando Nasser
Red Hat - Toronto                       E-Mail:  fnasser@cygnus.com
2323 Yonge Street, Suite #300           Tel:  416-482-2661 ext. 311
Toronto, Ontario   M4P 2C9              Fax:  416-482-6299
From kevinb@cygnus.com Fri Aug 04 14:16:00 2000
From: Kevin Buettner <kevinb@cygnus.com>
To: gdb-patches@sourceware.cygnus.com
Subject: [PATCH RFA] Fixup SYMBOL_SECTION for objfiles_relocate()
Date: Fri, 04 Aug 2000 14:16:00 -0000
Message-id: <1000804211627.ZM5495@ocotillo.lan>
X-SW-Source: 2000-08/msg00105.html
Content-length: 5756

I request permission to commit the patch below.

The OS for the project that I'm working on relocates the read/write
and the read-only sections by different amounts.  Thus it is critical
that objfile_relocate() actually have correct section indices during
the relocation process.

 I also discovered that for some symbol sym, SYMBOL_SECTION(sym) was
not providing the correct section index.  In fact, it was almost
always 0.  The same was true for partial symbols (upon which the same
macro magically works) as well.

The reason is that (at least for DWARF2) the SYMBOL_SECTION field
(section) was simply getting initialized to 0.  Furthermore, no
attempt was made to set things right later on.

The interesting thing is that SYMBOL_BFD_SECTION has exactly same
problem, but we have some code which attempts to set things right at
various points along the way.  The relevant functions are
fixup_symbol_section() and fixup_psymbol_section().  It seems to me
that we ought to be setting SYMBOL_SECTION at the same time that we're
setting BFD_SECTION and that is what the second hunk in the diff for
symtab.c is doing below.  [Thanks to Elena for bringing this to my
attention.]

Even once this is done, there is no guarantee that the symbols
and/or partial symbols will have been fixed up (with respect to
SYMBOL_SECTION) by the time that objfile_relocate() has been called.
So... now objfile_relocate() calls fixup_symbol_section() and
fixup_psymbol_section() as appropriate.

I'm not convinced that fixing these fields up at various random places
in the code where we need to access these fields is the right approach
to solving this problem.  It seems to me that it would be better to
attempt to set them correctly at the time that (or perhaps slightly
after) the symbol (or partial symbol) is created.  I made an attempt
at doing this, but I could not get it to work.  (Perhaps the minimal
symbols weren't available yet?) In any event, this is probably an
issue that the symbol table maintainers should consider in the future.
(grep for fixup_symbol_section() and look at all the places that
it's used.)

	* symtab.h (fixup_psymbol_section): Declare.
	* symtab.c (fixup_psymbol_section): Make extern.
	(fixup_section): Fix up section as well as bfd_section.
	* objfiles.c (objfile_relocate): Call fixup_symbol_section
	or fixup_psymbol_section before attempting to access
	the SYMBOL_SECTION component of a symbol or partial symbol.

Index: objfiles.c
===================================================================
RCS file: /cvs/src/src/gdb/objfiles.c,v
retrieving revision 1.7
diff -u -p -r1.7 objfiles.c
--- objfiles.c	2000/07/30 01:48:26	1.7
+++ objfiles.c	2000/08/04 20:46:02
@@ -564,6 +564,9 @@ objfile_relocate (struct objfile *objfil
 	  for (j = 0; j < BLOCK_NSYMS (b); ++j)
 	    {
 	      struct symbol *sym = BLOCK_SYM (b, j);
+
+	      fixup_symbol_section (sym, objfile);
+
 	      /* The RS6000 code from which this was taken skipped
 	         any symbols in STRUCT_NAMESPACE or UNDEF_NAMESPACE.
 	         But I'm leaving out that test, on the theory that
@@ -606,15 +609,21 @@ objfile_relocate (struct objfile *objfil
     for (psym = objfile->global_psymbols.list;
 	 psym < objfile->global_psymbols.next;
 	 psym++)
-      if (SYMBOL_SECTION (*psym) >= 0)
-	SYMBOL_VALUE_ADDRESS (*psym) += ANOFFSET (delta,
-						  SYMBOL_SECTION (*psym));
+      {
+	fixup_psymbol_section (*psym, objfile);
+	if (SYMBOL_SECTION (*psym) >= 0)
+	  SYMBOL_VALUE_ADDRESS (*psym) += ANOFFSET (delta,
+						    SYMBOL_SECTION (*psym));
+      }
     for (psym = objfile->static_psymbols.list;
 	 psym < objfile->static_psymbols.next;
 	 psym++)
-      if (SYMBOL_SECTION (*psym) >= 0)
-	SYMBOL_VALUE_ADDRESS (*psym) += ANOFFSET (delta,
-						  SYMBOL_SECTION (*psym));
+      {
+	fixup_psymbol_section (*psym, objfile);
+	if (SYMBOL_SECTION (*psym) >= 0)
+	  SYMBOL_VALUE_ADDRESS (*psym) += ANOFFSET (delta,
+						    SYMBOL_SECTION (*psym));
+      }
   }
 
   {
Index: symtab.c
===================================================================
RCS file: /cvs/src/src/gdb/symtab.c,v
retrieving revision 1.10
diff -u -p -r1.10 symtab.c
--- symtab.c	2000/07/30 01:48:27	1.10
+++ symtab.c	2000/08/04 20:46:07
@@ -81,10 +81,6 @@ static struct partial_symbol *lookup_par
 						     const char *, int,
 						     namespace_enum);
 
-static struct partial_symbol *fixup_psymbol_section (struct
-						     partial_symbol *,
-						     struct objfile *);
-
 static struct symtab *lookup_symtab_1 (char *);
 
 static void cplusplus_hint (char *);
@@ -520,7 +516,10 @@ fixup_section (struct general_symbol_inf
   msym = lookup_minimal_symbol (ginfo->name, NULL, objfile);
 
   if (msym)
-    ginfo->bfd_section = SYMBOL_BFD_SECTION (msym);
+    {
+      ginfo->bfd_section = SYMBOL_BFD_SECTION (msym);
+      ginfo->section = SYMBOL_SECTION (msym);
+    }
 }
 
 struct symbol *
@@ -537,7 +536,7 @@ fixup_symbol_section (struct symbol *sym
   return sym;
 }
 
-static struct partial_symbol *
+struct partial_symbol *
 fixup_psymbol_section (struct partial_symbol *psym, struct objfile *objfile)
 {
   if (!psym)
Index: symtab.h
===================================================================
RCS file: /cvs/src/src/gdb/symtab.h,v
retrieving revision 1.11
diff -u -p -r1.11 symtab.h
--- symtab.h	2000/06/05 20:49:53	1.11
+++ symtab.h	2000/08/04 20:46:09
@@ -1414,6 +1414,10 @@ extern int in_prologue (CORE_ADDR pc, CO
 extern struct symbol *fixup_symbol_section (struct symbol *,
 					    struct objfile *);
 
+extern struct partial_symbol *fixup_psymbol_section (struct partial_symbol
+						     *psym,
+						     struct objfile *objfile);
+
 /* Symbol searching */
 
 /* When using search_symbols, a list of the following structs is returned.