public inbox for frysk@sourceware.org
 help / color / mirror / Atom feed
* Breakpoint stepping
@ 2007-07-04 18:20 Mark Wielaard
  2007-07-05  4:45 ` Phil Muldoon
                   ` (4 more replies)
  0 siblings, 5 replies; 22+ messages in thread
From: Mark Wielaard @ 2007-07-04 18:20 UTC (permalink / raw)
  To: frysk

[-- Attachment #1: Type: text/plain, Size: 9510 bytes --]

As you might have noticed over the last week I have dropped in the new
breakpoint stepping framework (or hopefully you didn't notice since I
did try to do it in chunks that all had no detectable functional
behavior changes). This is an update to my previous general stepping
overview http://sourceware.org/ml/frysk/2007-q2/msg00283.html
and a request for review of the next steps with regard to the
instruction parsers, testframework and what to use as ssol area.

The main difference with the previous stepping overview regarding the
ptrace state machine is that there is now an explicit Stepping state,
which is a sub-state of Running and that Running.sendContinue() now not
only determines how to instruct ptrace to continue the task, but also
returns a different Running state based on whether or not we asked for a
continue or step-continue. This makes interpreting the cause of the trap
event we subsequently expect to get easier. This was previously recorded
as a field in the Task itself.

With respect to breakpoints the ptrace task states have been adapted to
all call setupSteppingBreakpoint() after a breakpoint hit has been
detected, which makes sure the PC is setup correctly (which previously
could be off by one on some architectures) and to mark the Task as being
at that particular breakpoint (this is still kept as Task field for now)
and to not actually call Breakpoint.prepareStep(), which is now handled
in Running.sendContinue(). In the previous version when reset-breakpoint
stepping was used another task could miss the breakpoint if the Task
moved into a Blocked state before continuing.

All the logic of how to breakpoint step has been moved into the
Breakpoint class that checks the properties of a new Instruction class
object which is created by the Isa through an instruction parser before
the breakpoint is inserted at a given location (previously we
represented instructions just as a byte[]). An Instruction knows how
long the instruction is, which bytes it represents, whether it can be
single stepped out of line and how to set that up given the original pc
location and an alternative address, plus any fixups that are needed to
the Task afterwards (and it has a notion of whether or not the
Instruction can be simulated, but that isn't currently used, see below).
A Breakpoint ties an Instruction to a particular address and Proc (and
Tasks can have zero or more Breakpoints, they share the same Breakpoint
on the same address with other Tasks of a Proc and when no Tasks of a
Proc has an Breakpoint at a particular address anymore the Breakpoint is
removed).

For stepping the Breakpoint the Running.sendContinue() method first
calls Breakpoint.prepareStep(), then signals ptrace to do a single step
of the Task, putting the Task in Stepping state and then in
handleTrapEvent() calls Breakpoint.stepDone(). prepareStep() queries the
capabilities of the Instruction at the pc address and depending on that
either sets things up for doing a step out of line, simulate the
instruction (but none of the current Instructions have been setup to do
simulation yes, and look at the comment in prepareStep() to see what is
needed to fully enable this option) or reset the current Instruction
(removing the breakpoint instruction temporarily). Accordingly a
Breakpoint can be in the state NOT_STEPPING, OUT_OF_LINE_STEPPING,
SIMULATE_STEPPING or RESET_STEPPING.

In the case of RESET_STEPPING (which was the only option we supported
before) other Tasks might miss and just past the Breakpoint during the
brief period between the reset, step and reinstall. Breakpoint
prepareStep() just takes the Instruction bytes and puts them at the
current pc address, and doneStep() reinstates the breakpoint
instruction.

When the Instruction supports single step out of line then the
Breakpoint requests an address in the single step out of line area of
the Proc, instructs the Instruction to install itself there for the
current Task calling Instruction.setupExecuteOutOfLine(). The default
action of setupExecuteOutOfLine() is to set the pc to the given address
and place a copy the instruction bytes there (although this can be
overridden if an Instruction wants to do something more fancy). When the
task signals the Breakpoint that a step was taken by calling stepDone(),
the Breakpoint calls Instruction fixupExecuteOutOfLine() with the
original pc and replacement address so any adjustments can be done to
the Task registers. The default action is to just set the pc to the
original pc plus the length of the Instruction just stepped. But
Instructions can override that if more is needed. As an example the RET
instruction doesn't do any fixup (since the only action is setting the
pc to the right location in the first place) and the JMP instruction
sets the pc to original pc plus/minus the difference of the alternate
address and the pc after the single step. Afterward the Breakpoint
returns the used address to the Proc so it can be used by other Tasks
needing to do a single step out of line.

The Proc maintains a single step out of line area pool of addresses that
point to locations that are at least as big is the largest instruction
of the instruction set. The Proc gets this list from the Isa the first
time an address is requested through getOutOfLineAddress(). Currently
this is (for x86 and x86_64) just the address of the main function entry
point(see below). The address is taken out of the pool and the
Breakpoint is responsible for putting it back through doneOutOfLine (see
above). If no address is currently available the call blocks till one is
available (this was way easier than inventing yet another TaskState and
getting the communication between Proc and Task about this right, and
contention is very low and at the longest it takes for an address to
become available is one instruction single step).

All the above seems to work nicely for x86 and x86_64 (powerpc hasn't
been updated, but as long as an Isa doesn't return Instructions through
an instruction parser that returns single stepping out of line capable
Instructions the old reset-breakpoint stepping is used) with all the old
testcases and a couple of new ones but there are 3 areas of improvement
needed:

- Instruction Parser.  The framework is in place and works for the few
Instructions that are known to the instruction parse, but there are all
hand coded (see IA32InstructionParser which just handles NOP, INT3, RETQ
and one JMP variant, the X8664Instruction just delegates to the IA32 for
now). There don't seem to be libraries available to easily plugin that
would give us the fixup instructions needed. The best available is the
kprobes examples from the linux kernel which have as drawback that they
are coded to be intimately tied to the kernel/C way of doing things and
only handles instructions found in kernel space. For uprobes this should
have been extended to handle every instruction that can occur in user
space, but I haven't seen that work yet (and apparently is only
available for x86 and no other architecture at this time). Any
alternatives to look at would be appreciated. Otherwise I need to sit
down with the various instruction manuals and just code it up by hand.
(Bonus points for finding something that would not just give us ssol
fixups but also simulation of instructions when hooked to the registers
and memory of a Task).

- Testsuite.  The best I could come up with for now is TestInstructions
and funit-instructions.c which is a nice little framework for having a
simple assembly instructions labeled in such a way that a 'instruction
address path' can be communicated to the java side so it can do either a
full step through the program or install breakpoints on every
instruction and make sure that every step and/or breakpoint hit takes it
to the right next address. But the assembly part has to be written
completely by hand (and unsurprisingly the simple assembly program
currently used works identically on x86 and x86_64). Trying to reuse the
funit-asm.h generic assembly instructions to make it platform neutral
was really hard especially since the labels used on the C and inline
assembly side have to kept in sync and mixing C defines and asm
statements don't seem to mix very well. Also at the moment it is just
using nops and jmps to keep things simple. I guess that in the end, if
we have real instruction parsers we could try to generate them using
that. Ideas or experiences with setting something up to track individual
instructions and getting the right addresses listed appreciated.

- Single Step Out Of Line Address Area.  Currently the Isa (for x86 and
x86_64 at least) just provide one address. The address of the main()
function entry point taken by just doing:

        Elf elf = new Elf(proc.getExe(), ElfCommand.ELF_C_READ);
        Dwarf dwarf = new Dwarf(elf, DwarfCommand.READ, null);
        DwarfDie die = DwarfDie.getDecl(dwarf, "main");
        return die.getEntryBreakpoints();

This works surprisingly well for a simple first approach, and programs
generally don't reenter their own main() function. But it would be nice
to either find an area that is guaranteed to never be used (again) by
the process, or to map in an executable area in the inferior that is
just used by us (maybe just making the inferior load a dummy shared
library). Again any suggestions welcome.

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 189 bytes --]

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: Breakpoint stepping
  2007-07-04 18:20 Breakpoint stepping Mark Wielaard
@ 2007-07-05  4:45 ` Phil Muldoon
  2007-07-05 12:39   ` Mark Wielaard
  2007-07-05 18:37 ` Breakpoint stepping Andrew Cagney
                   ` (3 subsequent siblings)
  4 siblings, 1 reply; 22+ messages in thread
From: Phil Muldoon @ 2007-07-05  4:45 UTC (permalink / raw)
  To: Mark Wielaard; +Cc: frysk

Mark Wielaard wrote:
>
> - Single Step Out Of Line Address Area.  Currently the Isa (for x86 and
> x86_64 at least) just provide one address. The address of the main()
> function entry point taken by just doing:
>
>         Elf elf = new Elf(proc.getExe(), ElfCommand.ELF_C_READ);
>         Dwarf dwarf = new Dwarf(elf, DwarfCommand.READ, null);
>         DwarfDie die = DwarfDie.getDecl(dwarf, "main");
>         return die.getEntryBreakpoints();
>
> This works surprisingly well for a simple first approach, and programs
> generally don't reenter their own main() function. But it would be nice
> to either find an area that is guaranteed to never be used (again) by
> the process, or to map in an executable area in the inferior that is
> just used by us (maybe just making the inferior load a dummy shared
> library). Again any suggestions welcome.
>   

Mark,

I'm still reading the rest of your email (the state machine changes I'm 
still trying to understand).

Is the above entry point code similar too getting the Entry Point from 
the process auxiliary?

Something like:

Auxv[] auxv = proc.getAuxv ();
long entryPoint = 0;

if (auxv == null)
    return 0;

// Find the Auxv ENTRY data
for (int i = 0; i < auxv.length; i++)
if (auxv[i].type == inua.elf.AT.ENTRY)
{
    entryPoint = auxv[i].val;
    break;
}

Not sure if one is more expensive than the other, just trivia really. As 
a side point, be sure to close to call elf.close() which immediately 
dispenses of the fd associated with the Elf object. It will eventually 
be cleaned up "anyway" on  the finalize() call but that can happen quite 
a long time away.

I agree on the main() entry-point being a good first step to as a usable 
space, though I wonder how that would look in a corefile. Though I 
suspect if you are dumping core while stepping a process one is in 
deeper trouble than one suspects ;) One of the other ideas was creating 
a custom solib and using it's address space to store the stuff needed. 
All this sounds hacky though. I vaguely recall a discussion to how 
uprobes does something similar by mapping in a page from somewhere? Do 
you remember any of that stuff?

Regards

Phil

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: Breakpoint stepping
  2007-07-05  4:45 ` Phil Muldoon
@ 2007-07-05 12:39   ` Mark Wielaard
  2007-07-10  9:59     ` Leaving visible breakpoints in memory/core (Was: Breakpoint stepping) Mark Wielaard
  0 siblings, 1 reply; 22+ messages in thread
From: Mark Wielaard @ 2007-07-05 12:39 UTC (permalink / raw)
  To: Phil Muldoon; +Cc: frysk

[-- Attachment #1: Type: text/plain, Size: 2815 bytes --]

Hi Phil,

On Wed, 2007-07-04 at 23:45 -0500, Phil Muldoon wrote:
> I'm still reading the rest of your email (the state machine changes I'm 
> still trying to understand).

Please ask, I might not explain them right, or maybe they are really not
that clear/well done in the first place.

> Is the above entry point code similar too getting the Entry Point from 
> the process auxiliary?

Cool! That is so much easier than what I was doing. Thanks, I didn't
even know the auxiliary vector of a proc contained the entry point.
Tested on x86 and x86_64 (and also added to powerpc now, even though I
cannot test it and powerpc would need some other fixes to fully support
ssol) and it works like a charm. Much nicer than mucking through the Elf
image by hand.

2007-07-05  Mark Wielaard  <mwielaard@redhat.com>

        IsaIA32.java (getOutOfLineAddresses): Use Auxv entry point.
        IsaPowerPC.java (getOutOfLineAddresses): Likewise.
        IsaX8664.java (getOutOfLineAddresses): Likewise.

I have to post more code to the list I see. You triggered on actual code
instead of all the explanation of what it is all supposed to do. It is
probably time to introduce a frysk-patches list to discuss actual
patches a bit more (clicking through on the commit list URLs and trying
to figure out what/why a change was made is pretty hard).

> I agree on the main() entry-point being a good first step to as a usable 
> space, though I wonder how that would look in a corefile.

Isn't that what ElfPrAuxv represents? But it might be wrong to have this
in the Isa in the first place. It is probably a property of the Proc,
not of the Isa. When I cleanup the outOfLineAddresses storage that you
pointed out in the previous review I'll try to move this at the same
time.

>  Though I 
> suspect if you are dumping core while stepping a process one is in 
> deeper trouble than one suspects ;)

I admit to not have thought of this scenario. That is indeed troublesome
since some breakpoints might actually still be embedded in the Proc code
memory while the kernel writes out the core file. Have to think about
that. What scenarios are there for a process to dump core? And is there
any way for us to intercept and quickly remove any changes we done to
the code segments before that?

> One of the other ideas was creating 
> a custom solib and using it's address space to store the stuff needed. 
> All this sounds hacky though. I vaguely recall a discussion to how 
> uprobes does something similar by mapping in a page from somewhere? Do 
> you remember any of that stuff?

uprobes has the kernel to help. So they just allocate a whole new VM
area. We would have to somehow trigger a dummy shared library load
inside the inferior (and hope it doesn't interfere with anything the
process is doing at the time).

Cheers,

Mark

[-- Attachment #2: auxv.patch --]
[-- Type: text/x-patch, Size: 4960 bytes --]

Index: frysk-core/frysk/proc/IsaIA32.java
===================================================================
RCS file: /cvs/frysk/frysk-core/frysk/proc/IsaIA32.java,v
retrieving revision 1.25
diff -u -r1.25 IsaIA32.java
--- frysk-core/frysk/proc/IsaIA32.java	3 Jul 2007 18:16:04 -0000	1.25
+++ frysk-core/frysk/proc/IsaIA32.java	5 Jul 2007 12:26:03 -0000
@@ -42,6 +42,7 @@
 import java.util.HashMap;
 import java.util.Iterator;
 import java.util.LinkedHashMap;
+import java.util.LinkedList;
 import java.util.List;
 import inua.eio.ByteOrder;
 import lib.unwind.RegisterX86;
@@ -51,15 +52,8 @@
 import frysk.proc.live.RegisterSetByteBuffer;
 import frysk.proc.live.AddressSpaceByteBuffer;
 
-import lib.elf.Elf;
-import lib.elf.ElfCommand;
-import lib.elf.ElfException;
 import lib.elf.ElfEMachine;
 
-import lib.dw.Dwarf;
-import lib.dw.DwarfCommand;
-import lib.dw.DwarfDie;
-
 public class IsaIA32 implements Isa
 {
   /**
@@ -293,7 +287,7 @@
    */
   public long getBreakpointAddress(Task task)
   {
-    long pcValue = 0;
+    long pcValue;
     
     pcValue = this.pc(task);
     pcValue = pcValue - 1;
@@ -308,21 +302,15 @@
    */
   public List getOutOfLineAddresses(Proc proc)
   {
-    String func = "main";
-    try
-      {
-	Elf elf = new Elf(proc.getExe(), ElfCommand.ELF_C_READ);
-        Dwarf dwarf = new Dwarf(elf, DwarfCommand.READ, null);
-        DwarfDie die = DwarfDie.getDecl(dwarf, func);
-        return die.getEntryBreakpoints();
-      }
-    catch (ElfException ee)
+    LinkedList addrs = new LinkedList();
+    Auxv[] auxv = proc.getAuxv ();
+    // Find the Auxv ENTRY data
+    for (int i = 0; i < auxv.length; i++)
       {
-	IllegalStateException ise;
-	ise = new IllegalStateException("Unable to get at " + func);
-	ise.initCause(ee);
-	throw ise;
+	if (auxv[i].type == inua.elf.AT.ENTRY)
+	addrs.add(Long.valueOf(auxv[i].val));
       }
+    return addrs;
   }
 
   /**
Index: frysk-core/frysk/proc/IsaPowerPC.java
===================================================================
RCS file: /cvs/frysk/frysk-core/frysk/proc/IsaPowerPC.java,v
retrieving revision 1.9
diff -u -r1.9 IsaPowerPC.java
--- frysk-core/frysk/proc/IsaPowerPC.java	2 Jul 2007 14:40:17 -0000	1.9
+++ frysk-core/frysk/proc/IsaPowerPC.java	5 Jul 2007 12:26:03 -0000
@@ -1,6 +1,7 @@
 // This file is part of the program FRYSK.
 //
 // Copyright 2006 IBM Corp.
+// Copyright 2007 Red Hat Inc.
 //
 // FRYSK is free software; you can redistribute it and/or modify it
 // under the terms of the GNU General Public License as published by
@@ -41,6 +42,7 @@
 import java.util.HashMap;
 import java.util.Iterator;
 import java.util.LinkedHashMap;
+import java.util.LinkedList;
 import java.util.List;
 import inua.eio.ByteBuffer;
 import frysk.proc.live.AddressSpaceByteBuffer;
@@ -102,7 +104,15 @@
 
   public List getOutOfLineAddresses(Proc proc)
   {
-    throw new IllegalStateException("getOutOfLineAddresses not implemented");
+    LinkedList addrs = new LinkedList();
+    Auxv[] auxv = proc.getAuxv ();
+    // Find the Auxv ENTRY data
+    for (int i = 0; i < auxv.length; i++)
+      {
+	if (auxv[i].type == inua.elf.AT.ENTRY)
+	addrs.add(Long.valueOf(auxv[i].val));
+      }
+    return addrs;
   }
 
   /**
Index: frysk-core/frysk/proc/IsaX8664.java
===================================================================
RCS file: /cvs/frysk/frysk-core/frysk/proc/IsaX8664.java,v
retrieving revision 1.17
diff -u -r1.17 IsaX8664.java
--- frysk-core/frysk/proc/IsaX8664.java	3 Jul 2007 18:16:04 -0000	1.17
+++ frysk-core/frysk/proc/IsaX8664.java	5 Jul 2007 12:26:03 -0000
@@ -42,6 +42,7 @@
 import java.util.HashMap;
 import java.util.Iterator;
 import java.util.LinkedHashMap;
+import java.util.LinkedList;
 import java.util.List;
 import inua.eio.ByteOrder;
 import inua.eio.ByteBuffer;
@@ -50,16 +51,9 @@
 import frysk.proc.live.RegisterSetByteBuffer;
 import frysk.proc.live.AddressSpaceByteBuffer;
 
-import lib.elf.Elf;
-import lib.elf.ElfCommand;
-import lib.elf.ElfException;
 import lib.elf.ElfEMachine;
 import lib.unwind.RegisterAMD64;
 
-import lib.dw.Dwarf;
-import lib.dw.DwarfCommand;
-import lib.dw.DwarfDie;
-
 
 public class IsaX8664 implements Isa
 {
@@ -305,21 +299,15 @@
    */
   public List getOutOfLineAddresses(Proc proc)
   {
-    String func = "main";
-    try
-      {
-	Elf elf = new Elf(proc.getExe(), ElfCommand.ELF_C_READ);
-	Dwarf dwarf = new Dwarf(elf, DwarfCommand.READ, null);
-	DwarfDie die = DwarfDie.getDecl(dwarf, func);
-	return die.getEntryBreakpoints();
-      }
-    catch (ElfException ee)
+    LinkedList addrs = new LinkedList();
+    Auxv[] auxv = proc.getAuxv ();
+    // Find the Auxv ENTRY data
+    for (int i = 0; i < auxv.length; i++)
       {
-	IllegalStateException ise;
-	ise = new IllegalStateException("Unable to get at " + func);
-	ise.initCause(ee);
-	throw ise;
+	if (auxv[i].type == inua.elf.AT.ENTRY)
+	addrs.add(Long.valueOf(auxv[i].val));
       }
+    return addrs;
   }
 
   /**

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: Breakpoint stepping
  2007-07-04 18:20 Breakpoint stepping Mark Wielaard
  2007-07-05  4:45 ` Phil Muldoon
@ 2007-07-05 18:37 ` Andrew Cagney
  2007-07-23 12:19   ` Mark Wielaard
  2007-07-10 10:39 ` Instruction parser (Was: Breakpoint stepping) Mark Wielaard
                   ` (2 subsequent siblings)
  4 siblings, 1 reply; 22+ messages in thread
From: Andrew Cagney @ 2007-07-05 18:37 UTC (permalink / raw)
  To: Mark Wielaard; +Cc: frysk

Mark,

Can I suggest adding this, or something based on it, to 
frysk.proc.live.package.html?  It's this useful! information we should 
be accumulating in the source tree.

Andrew

Mark Wielaard wrote:
> As you might have noticed over the last week I have dropped in the new
> breakpoint stepping framework (or hopefully you didn't notice since I
> did try to do it in chunks that all had no detectable functional
> behavior changes). This is an update to my previous general stepping
> overview http://sourceware.org/ml/frysk/2007-q2/msg00283.html
> and a request for review of the next steps with regard to the
> instruction parsers, testframework and what to use as ssol area.
>
> The main difference with the previous stepping overview regarding the
> ptrace state machine is that there is now an explicit Stepping state,
> which is a sub-state of Running and that Running.sendContinue() now not
> only determines how to instruct ptrace to continue the task, but also
> returns a different Running state based on whether or not we asked for a
> continue or step-continue. This makes interpreting the cause of the trap
> event we subsequently expect to get easier. This was previously recorded
> as a field in the Task itself.
>
> With respect to breakpoints the ptrace task states have been adapted to
> all call setupSteppingBreakpoint() after a breakpoint hit has been
> detected, which makes sure the PC is setup correctly (which previously
> could be off by one on some architectures) and to mark the Task as being
> at that particular breakpoint (this is still kept as Task field for now)
> and to not actually call Breakpoint.prepareStep(), which is now handled
> in Running.sendContinue(). In the previous version when reset-breakpoint
> stepping was used another task could miss the breakpoint if the Task
> moved into a Blocked state before continuing.
>
> All the logic of how to breakpoint step has been moved into the
> Breakpoint class that checks the properties of a new Instruction class
> object which is created by the Isa through an instruction parser before
> the breakpoint is inserted at a given location (previously we
> represented instructions just as a byte[]). An Instruction knows how
> long the instruction is, which bytes it represents, whether it can be
> single stepped out of line and how to set that up given the original pc
> location and an alternative address, plus any fixups that are needed to
> the Task afterwards (and it has a notion of whether or not the
> Instruction can be simulated, but that isn't currently used, see below).
> A Breakpoint ties an Instruction to a particular address and Proc (and
> Tasks can have zero or more Breakpoints, they share the same Breakpoint
> on the same address with other Tasks of a Proc and when no Tasks of a
> Proc has an Breakpoint at a particular address anymore the Breakpoint is
> removed).
>
> For stepping the Breakpoint the Running.sendContinue() method first
> calls Breakpoint.prepareStep(), then signals ptrace to do a single step
> of the Task, putting the Task in Stepping state and then in
> handleTrapEvent() calls Breakpoint.stepDone(). prepareStep() queries the
> capabilities of the Instruction at the pc address and depending on that
> either sets things up for doing a step out of line, simulate the
> instruction (but none of the current Instructions have been setup to do
> simulation yes, and look at the comment in prepareStep() to see what is
> needed to fully enable this option) or reset the current Instruction
> (removing the breakpoint instruction temporarily). Accordingly a
> Breakpoint can be in the state NOT_STEPPING, OUT_OF_LINE_STEPPING,
> SIMULATE_STEPPING or RESET_STEPPING.
>
> In the case of RESET_STEPPING (which was the only option we supported
> before) other Tasks might miss and just past the Breakpoint during the
> brief period between the reset, step and reinstall. Breakpoint
> prepareStep() just takes the Instruction bytes and puts them at the
> current pc address, and doneStep() reinstates the breakpoint
> instruction.
>
> When the Instruction supports single step out of line then the
> Breakpoint requests an address in the single step out of line area of
> the Proc, instructs the Instruction to install itself there for the
> current Task calling Instruction.setupExecuteOutOfLine(). The default
> action of setupExecuteOutOfLine() is to set the pc to the given address
> and place a copy the instruction bytes there (although this can be
> overridden if an Instruction wants to do something more fancy). When the
> task signals the Breakpoint that a step was taken by calling stepDone(),
> the Breakpoint calls Instruction fixupExecuteOutOfLine() with the
> original pc and replacement address so any adjustments can be done to
> the Task registers. The default action is to just set the pc to the
> original pc plus the length of the Instruction just stepped. But
> Instructions can override that if more is needed. As an example the RET
> instruction doesn't do any fixup (since the only action is setting the
> pc to the right location in the first place) and the JMP instruction
> sets the pc to original pc plus/minus the difference of the alternate
> address and the pc after the single step. Afterward the Breakpoint
> returns the used address to the Proc so it can be used by other Tasks
> needing to do a single step out of line.
>
> The Proc maintains a single step out of line area pool of addresses that
> point to locations that are at least as big is the largest instruction
> of the instruction set. The Proc gets this list from the Isa the first
> time an address is requested through getOutOfLineAddress(). Currently
> this is (for x86 and x86_64) just the address of the main function entry
> point(see below). The address is taken out of the pool and the
> Breakpoint is responsible for putting it back through doneOutOfLine (see
> above). If no address is currently available the call blocks till one is
> available (this was way easier than inventing yet another TaskState and
> getting the communication between Proc and Task about this right, and
> contention is very low and at the longest it takes for an address to
> become available is one instruction single step).
>
> All the above seems to work nicely for x86 and x86_64 (powerpc hasn't
> been updated, but as long as an Isa doesn't return Instructions through
> an instruction parser that returns single stepping out of line capable
> Instructions the old reset-breakpoint stepping is used) with all the old
> testcases and a couple of new ones but there are 3 areas of improvement
> needed:
>
> - Instruction Parser.  The framework is in place and works for the few
> Instructions that are known to the instruction parse, but there are all
> hand coded (see IA32InstructionParser which just handles NOP, INT3, RETQ
> and one JMP variant, the X8664Instruction just delegates to the IA32 for
> now). There don't seem to be libraries available to easily plugin that
> would give us the fixup instructions needed. The best available is the
> kprobes examples from the linux kernel which have as drawback that they
> are coded to be intimately tied to the kernel/C way of doing things and
> only handles instructions found in kernel space. For uprobes this should
> have been extended to handle every instruction that can occur in user
> space, but I haven't seen that work yet (and apparently is only
> available for x86 and no other architecture at this time). Any
> alternatives to look at would be appreciated. Otherwise I need to sit
> down with the various instruction manuals and just code it up by hand.
> (Bonus points for finding something that would not just give us ssol
> fixups but also simulation of instructions when hooked to the registers
> and memory of a Task).
>
> - Testsuite.  The best I could come up with for now is TestInstructions
> and funit-instructions.c which is a nice little framework for having a
> simple assembly instructions labeled in such a way that a 'instruction
> address path' can be communicated to the java side so it can do either a
> full step through the program or install breakpoints on every
> instruction and make sure that every step and/or breakpoint hit takes it
> to the right next address. But the assembly part has to be written
> completely by hand (and unsurprisingly the simple assembly program
> currently used works identically on x86 and x86_64). Trying to reuse the
> funit-asm.h generic assembly instructions to make it platform neutral
> was really hard especially since the labels used on the C and inline
> assembly side have to kept in sync and mixing C defines and asm
> statements don't seem to mix very well. Also at the moment it is just
> using nops and jmps to keep things simple. I guess that in the end, if
> we have real instruction parsers we could try to generate them using
> that. Ideas or experiences with setting something up to track individual
> instructions and getting the right addresses listed appreciated.
>
> - Single Step Out Of Line Address Area.  Currently the Isa (for x86 and
> x86_64 at least) just provide one address. The address of the main()
> function entry point taken by just doing:
>
>         Elf elf = new Elf(proc.getExe(), ElfCommand.ELF_C_READ);
>         Dwarf dwarf = new Dwarf(elf, DwarfCommand.READ, null);
>         DwarfDie die = DwarfDie.getDecl(dwarf, "main");
>         return die.getEntryBreakpoints();
>
> This works surprisingly well for a simple first approach, and programs
> generally don't reenter their own main() function. But it would be nice
> to either find an area that is guaranteed to never be used (again) by
> the process, or to map in an executable area in the inferior that is
> just used by us (maybe just making the inferior load a dummy shared
> library). Again any suggestions welcome.
>   

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Leaving visible breakpoints in memory/core (Was: Breakpoint  stepping)
  2007-07-05 12:39   ` Mark Wielaard
@ 2007-07-10  9:59     ` Mark Wielaard
  2007-07-10 13:52       ` Andrew Cagney
  2007-07-10 18:06       ` Phil Muldoon
  0 siblings, 2 replies; 22+ messages in thread
From: Mark Wielaard @ 2007-07-10  9:59 UTC (permalink / raw)
  To: Phil Muldoon; +Cc: frysk

[-- Attachment #1: Type: text/plain, Size: 1710 bytes --]

Hi Phil,

On Thu, 2007-07-05 at 14:39 +0200, Mark Wielaard wrote:
> >  Though I 
> > suspect if you are dumping core while stepping a process one is in 
> > deeper trouble than one suspects ;)
> 
> I admit to not have thought of this scenario. That is indeed troublesome
> since some breakpoints might actually still be embedded in the Proc code
> memory while the kernel writes out the core file. Have to think about
> that. What scenarios are there for a process to dump core? And is there
> any way for us to intercept and quickly remove any changes we done to
> the code segments before that?

After thinking about it a bit more and some off-list chatter I think
there are 2 scenarios here. 1) Having our inserted breakpoints show up
in memory views as used in frysk. 2) Inserted breakpoints show up in
core dumps of programs we are analyzing. It is probably not worth it to
worry about 2) since as you say the user has deeper troubles then. And
if they want to analyse the actual core file later on it is probably
even more fair to make sure the breakpoints are still in the core file
so they have a real picture of what went wrong (it could even have been
frysk's fault!)

But 1) is a problem since it would distort the view of the user while
using frysk. So there is now bug #4761 and it is on my TODO list to
create a memory view that the "non-breakpoint aware" parts of Frysk will
use for memory inspection. The use case is to have the fhpd or frysk-gui
insert a breakpoint, stop at it, and let the user inspect the code
instructions around the breakpoint. This should not show traces of the
breakpoint even though frysk might still have it inserted.

Cheers,

Mark

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 189 bytes --]

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Instruction parser (Was: Breakpoint stepping)
  2007-07-04 18:20 Breakpoint stepping Mark Wielaard
  2007-07-05  4:45 ` Phil Muldoon
  2007-07-05 18:37 ` Breakpoint stepping Andrew Cagney
@ 2007-07-10 10:39 ` Mark Wielaard
  2007-07-10 10:50 ` Instruction breakpoint-stepping testsuite " Mark Wielaard
  2007-07-10 10:57 ` SSOL Area " Mark Wielaard
  4 siblings, 0 replies; 22+ messages in thread
From: Mark Wielaard @ 2007-07-10 10:39 UTC (permalink / raw)
  To: frysk

[-- Attachment #1: Type: text/plain, Size: 2494 bytes --]

Hi,

Thanks to Andrew for suggesting to put the ptrace ssol framework text in
some form to the package source file, which I will do. And to Phil for
pointing out a better way to get at the start address of a process,
which I have already implemented. But I was hoping for some more public
feedback on the other questions I tagged on the end of that long email
on suggestions for moving forward with the implementation. So I am now
repeating them in separate messages with some explanation of my thinking
so far. Comments more than welcome.

On Wed, 2007-07-04 at 20:20 +0200, Mark Wielaard wrote:
> - Instruction Parser.  The framework is in place and works for the few
> Instructions that are known to the instruction parse, but there are all
> hand coded (see IA32InstructionParser which just handles NOP, INT3, RETQ
> and one JMP variant, the X8664Instruction just delegates to the IA32 for
> now). There don't seem to be libraries available to easily plugin that
> would give us the fixup instructions needed. The best available is the
> kprobes examples from the linux kernel which have as drawback that they
> are coded to be intimately tied to the kernel/C way of doing things and
> only handles instructions found in kernel space. For uprobes this should
> have been extended to handle every instruction that can occur in user
> space, but I haven't seen that work yet (and apparently is only
> available for x86 and no other architecture at this time). Any
> alternatives to look at would be appreciated. Otherwise I need to sit
> down with the various instruction manuals and just code it up by hand.
> (Bonus points for finding something that would not just give us ssol
> fixups but also simulation of instructions when hooked to the registers
> and memory of a Task).

I haven't found a library yet that is suitable for providing fixup
information and determining instruction validity that is usable by
Frysk. Without it the ssol framework is kind of fake and we fall back to
reset-stepping breakpoints, but without stop-the-world, so it keeps
being unreliable. The problem with the current kprobes (and by extension
uprobes) approach at the moment is that it isn't robust in the face of
arbitrary user space instructions. So I will probably end up writing it
myself for x86/x86_64 as suggested by Roland in this systemtap/uprobes
message: http://sourceware.org/ml/systemtap/2007-q1/msg00571.html

This is bug #4762 now.

Cheers,

Mark

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 189 bytes --]

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Instruction breakpoint-stepping testsuite (Was: Breakpoint  stepping)
  2007-07-04 18:20 Breakpoint stepping Mark Wielaard
                   ` (2 preceding siblings ...)
  2007-07-10 10:39 ` Instruction parser (Was: Breakpoint stepping) Mark Wielaard
@ 2007-07-10 10:50 ` Mark Wielaard
  2007-07-16  9:19   ` [patch] " Mark Wielaard
  2007-07-10 10:57 ` SSOL Area " Mark Wielaard
  4 siblings, 1 reply; 22+ messages in thread
From: Mark Wielaard @ 2007-07-10 10:50 UTC (permalink / raw)
  To: frysk

[-- Attachment #1: Type: text/plain, Size: 2691 bytes --]

On Wed, 2007-07-04 at 20:20 +0200, Mark Wielaard wrote:
> - Testsuite.  The best I could come up with for now is TestInstructions
> and funit-instructions.c which is a nice little framework for having a
> simple assembly instructions labeled in such a way that a 'instruction
> address path' can be communicated to the java side so it can do either a
> full step through the program or install breakpoints on every
> instruction and make sure that every step and/or breakpoint hit takes it
> to the right next address. But the assembly part has to be written
> completely by hand (and unsurprisingly the simple assembly program
> currently used works identically on x86 and x86_64). Trying to reuse the
> funit-asm.h generic assembly instructions to make it platform neutral
> was really hard especially since the labels used on the C and inline
> assembly side have to kept in sync and mixing C defines and asm
> statements don't seem to mix very well. Also at the moment it is just
> using nops and jmps to keep things simple. I guess that in the end, if
> we have real instruction parsers we could try to generate them using
> that. Ideas or experiences with setting something up to track individual
> instructions and getting the right addresses listed appreciated.

I think I know how this can be made way easier. The current setup
inserts labels at every instruction. This makes it really hard to
combine with the generic funit-asm.h assembly instructions since it
introduces 2 layers of macros to write a simple test. And the test has
to be marked up fully with the labels and a path through the program
that can be checked. But the current setup already shows an easier way
to do this. In TestInstructions we already do a full step through the
assembly and double check that all the labels are hit in the right
order. But this can be turned around. We can first do this full step
through the assembly instructions and record all addresses, then we put
breakpoints at every instruction that we just past and do another run
with the breakpoints inserted that should then run exactly the same as
before (except for hitting a breakpoint at every step of course).

If we do it that way then we just need the start and end address of the
instruction stream and can hopefully completely reuse funit-asm.h as
long as the test writer makes sure the program always ends at the same
address.

I'll try and implement it as above using as much instructions from
funit-asm as possible. The only tricky thing is then just coming up with
a good/long enough instruction sequence to proof we really handle all
those instructions out there.

Cheers,

Mark

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 189 bytes --]

^ permalink raw reply	[flat|nested] 22+ messages in thread

* SSOL Area (Was: Breakpoint stepping)
  2007-07-04 18:20 Breakpoint stepping Mark Wielaard
                   ` (3 preceding siblings ...)
  2007-07-10 10:50 ` Instruction breakpoint-stepping testsuite " Mark Wielaard
@ 2007-07-10 10:57 ` Mark Wielaard
  4 siblings, 0 replies; 22+ messages in thread
From: Mark Wielaard @ 2007-07-10 10:57 UTC (permalink / raw)
  To: frysk

[-- Attachment #1: Type: text/plain, Size: 1525 bytes --]

On Wed, 2007-07-04 at 20:20 +0200, Mark Wielaard wrote:
> - Single Step Out Of Line Address Area.  Currently the Isa (for x86 and
> x86_64 at least) just provide one address. The address of the main()
> function entry point taken by just doing:
> 
>         Elf elf = new Elf(proc.getExe(), ElfCommand.ELF_C_READ);
>         Dwarf dwarf = new Dwarf(elf, DwarfCommand.READ, null);
>         DwarfDie die = DwarfDie.getDecl(dwarf, "main");
>         return die.getEntryBreakpoints();
> 
> This works surprisingly well for a simple first approach, and programs
> generally don't reenter their own main() function. But it would be nice
> to either find an area that is guaranteed to never be used (again) by
> the process, or to map in an executable area in the inferior that is
> just used by us (maybe just making the inferior load a dummy shared
> library). Again any suggestions welcome.

Phil already suggested that I use the auxiliary vector of a proc to more
easily get at the entry point which I have implemented now. It seems to
work great. Thanks Phil.

Off-list I did talk a bit about this with Andrew. And it isn't clear
this is a major roadblock for now. So I am not really going to
experiment for now till it is more clear that this doesn't scale. And
this all depends on getting the instruction parser fully in place so we
actually use the ssol implementation for most instructions.

What would be a good real world testcase so see if it scales or not in
practise? 

Cheers,

Mark

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 189 bytes --]

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: Leaving visible breakpoints in memory/core (Was: Breakpoint   stepping)
  2007-07-10  9:59     ` Leaving visible breakpoints in memory/core (Was: Breakpoint stepping) Mark Wielaard
@ 2007-07-10 13:52       ` Andrew Cagney
  2007-07-10 18:06       ` Phil Muldoon
  1 sibling, 0 replies; 22+ messages in thread
From: Andrew Cagney @ 2007-07-10 13:52 UTC (permalink / raw)
  To: Mark Wielaard; +Cc: Phil Muldoon, frysk

Mark,

Expanding on our discussion:

-> user examining memory: breakpoints invisible
-> process dumps core via kernel: breakpoints are visible (no choice 
with current kernel infrastructure)
-> user requests fcore: breakpoints should not be visible

I.e., breakpoints and their implementation are not visible to the user 
(or beyond frysk.proc)

However, for our own debugging sanity, we will also need a way to 
examine the raw/modified memory to see what the program is really 
executing :-)

Andrew

Mark Wielaard wrote:
> Hi Phil,
>
> On Thu, 2007-07-05 at 14:39 +0200, Mark Wielaard wrote:
>   
>>>  Though I 
>>> suspect if you are dumping core while stepping a process one is in 
>>> deeper trouble than one suspects ;)
>>>       
>> I admit to not have thought of this scenario. That is indeed troublesome
>> since some breakpoints might actually still be embedded in the Proc code
>> memory while the kernel writes out the core file. Have to think about
>> that. What scenarios are there for a process to dump core? And is there
>> any way for us to intercept and quickly remove any changes we done to
>> the code segments before that?
>>     
>
> After thinking about it a bit more and some off-list chatter I think
> there are 2 scenarios here. 1) Having our inserted breakpoints show up
> in memory views as used in frysk. 2) Inserted breakpoints show up in
> core dumps of programs we are analyzing. It is probably not worth it to
> worry about 2) since as you say the user has deeper troubles then. And
> if they want to analyse the actual core file later on it is probably
> even more fair to make sure the breakpoints are still in the core file
> so they have a real picture of what went wrong (it could even have been
> frysk's fault!)
>
> But 1) is a problem since it would distort the view of the user while
> using frysk. So there is now bug #4761 and it is on my TODO list to
> create a memory view that the "non-breakpoint aware" parts of Frysk will
> use for memory inspection. The use case is to have the fhpd or frysk-gui
> insert a breakpoint, stop at it, and let the user inspect the code
> instructions around the breakpoint. This should not show traces of the
> breakpoint even though frysk might still have it inserted.
>
> Cheers,
>
> Mark
>   

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: Leaving visible breakpoints in memory/core (Was: Breakpoint stepping)
  2007-07-10  9:59     ` Leaving visible breakpoints in memory/core (Was: Breakpoint stepping) Mark Wielaard
  2007-07-10 13:52       ` Andrew Cagney
@ 2007-07-10 18:06       ` Phil Muldoon
  2007-07-11  9:47         ` Mark Wielaard
  1 sibling, 1 reply; 22+ messages in thread
From: Phil Muldoon @ 2007-07-10 18:06 UTC (permalink / raw)
  To: Mark Wielaard; +Cc: frysk

Mark Wielaard wrote:
> Hi Phil,
>   
> After thinking about it a bit more and some off-list chatter I think
> there are 2 scenarios here. 1) Having our inserted breakpoints show up
> in memory views as used in frysk. 2) Inserted breakpoints show up in
> core dumps of programs we are analyzing. It is probably not worth it to
> worry about 2) since as you say the user has deeper troubles then. And
> if they want to analyse the actual core file later on it is probably
> even more fair to make sure the breakpoints are still in the core file
> so they have a real picture of what went wrong (it could even have been
> frysk's fault!)
>   

Well I think in the kernel dumping core there is nothing to be done with 
2) other than just let it happen. I just wonder if the entry-point of 
the program being rewritten by Frysk in this scenario will cause 
confusion. In the case of fcore, we can always strip all this stuff 
beforehand, dump the core, and then put it all back. But in the case of 
gcore and google's coredumper that won't be the case. A process has no 
idea when a userland program like gcore/fcore is creating a coredump of it.

However until there is a better place to store this stuff other than at 
the entry-point address, then I guess the question it sort of moot.

Regards

Phil
> But 1) is a problem since it would distort the view of the user while
> using frysk. So there is now bug #4761 and it is on my TODO list to
> create a memory view that the "non-breakpoint aware" parts of Frysk will
> use for memory inspection. The use case is to have the fhpd or frysk-gui
> insert a breakpoint, stop at it, and let the user inspect the code
> instructions around the breakpoint. This should not show traces of the
> breakpoint even though frysk might still have it inserted.
>
> Cheers,
>
> Mark
>   

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: Leaving visible breakpoints in memory/core (Was:  Breakpoint stepping)
  2007-07-10 18:06       ` Phil Muldoon
@ 2007-07-11  9:47         ` Mark Wielaard
  2007-07-12  2:49           ` Roland McGrath
  0 siblings, 1 reply; 22+ messages in thread
From: Mark Wielaard @ 2007-07-11  9:47 UTC (permalink / raw)
  To: Phil Muldoon; +Cc: frysk

[-- Attachment #1: Type: text/plain, Size: 1190 bytes --]

Hi Phil,

On Tue, 2007-07-10 at 13:06 -0500, Phil Muldoon wrote:
> Well I think in the kernel dumping core there is nothing to be done with 
> 2) other than just let it happen. I just wonder if the entry-point of 
> the program being rewritten by Frysk in this scenario will cause 
> confusion. In the case of fcore, we can always strip all this stuff 
> beforehand, dump the core, and then put it all back. But in the case of 
> gcore and google's coredumper that won't be the case. A process has no 
> idea when a userland program like gcore/fcore is creating a coredump of it.
> 
> However until there is a better place to store this stuff other than at 
> the entry-point address, then I guess the question it sort of moot.

Yes, but the breakpoints instructions themselves are stored in the
image, only the original instruction is (if we use ssol, which is almost
never at this point) (re)stored at the entry-point address. And it is
the breakpoint instructions that we don't want to have in a fcore
result. Since both you and Andrew mentioned it I added it as second
scenario to the bug report, so I won't forget to create a testcase for
that.

Cheers,

Mark

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 189 bytes --]

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: Leaving visible breakpoints in memory/core (Was:  Breakpoint stepping)
  2007-07-11  9:47         ` Mark Wielaard
@ 2007-07-12  2:49           ` Roland McGrath
  2007-07-12 14:24             ` Phil Muldoon
  0 siblings, 1 reply; 22+ messages in thread
From: Roland McGrath @ 2007-07-12  2:49 UTC (permalink / raw)
  To: Mark Wielaard; +Cc: Phil Muldoon, frysk

This is something that you get for free from the "fancy VM tricks"
facilities that are still pie in the sky, but is more or less hopeless
short of that.  So I just wouldn't worry about it any time soon.  The best
I think we can do for now is just remove the breakpoints when you know it's
about to dump core.  This is pretty easy (at least in utrace-based
interfaces) since you just catch the "signal that is about to try to dump
core" event, which you were probably catching already, and it clearly
distinguishes "definitely about to die" from "might run a signal handler
and survive".


Thanks,
Roland

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: Leaving visible breakpoints in memory/core (Was:  Breakpoint  stepping)
  2007-07-12  2:49           ` Roland McGrath
@ 2007-07-12 14:24             ` Phil Muldoon
  2007-07-12 20:24               ` Roland McGrath
  2007-07-16 15:53               ` Mark Wielaard
  0 siblings, 2 replies; 22+ messages in thread
From: Phil Muldoon @ 2007-07-12 14:24 UTC (permalink / raw)
  To: Roland McGrath; +Cc: Mark Wielaard, frysk

Roland McGrath wrote:
> This is something that you get for free from the "fancy VM tricks"
> facilities that are still pie in the sky, but is more or less hopeless
> short of that.  So I just wouldn't worry about it any time soon.  The best
> I think we can do for now is just remove the breakpoints when you know it's
> about to dump core.  This is pretty easy (at least in utrace-based
> interfaces) since you just catch the "signal that is about to try to dump
> core" event, which you were probably catching already, and it clearly
> distinguishes "definitely about to die" from "might run a signal handler
> and survive".
>
>   
I'm torn between whether tools like fcore (well only fcore in this 
instance) should just leave things as they are and construct core of the 
process "as is". Depends on the definition of a coredump and whether 
prettying up the process by removing bps, and other blockers is a "good 
thing" before a userland coredump. Or whether we should just dump it, 
warts and all.

The second question speaks to a Frysk mechanics question, if we do 
remove all the bits, then dump core, can we replace them exactly as they 
were before? I'd like to say yes, but I get the nagging feeling this 
isn't so? What do others think?

Phil

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: Leaving visible breakpoints in memory/core (Was:  Breakpoint  stepping)
  2007-07-12 14:24             ` Phil Muldoon
@ 2007-07-12 20:24               ` Roland McGrath
  2007-07-16 15:57                 ` Mark Wielaard
  2007-07-16 15:53               ` Mark Wielaard
  1 sibling, 1 reply; 22+ messages in thread
From: Roland McGrath @ 2007-07-12 20:24 UTC (permalink / raw)
  To: Phil Muldoon; +Cc: Mark Wielaard, frysk

For a nondestructive dump, you also have the option of editting the memory
as you copy it.

^ permalink raw reply	[flat|nested] 22+ messages in thread

* [patch] Re: Instruction breakpoint-stepping testsuite (Was:  Breakpoint stepping)
  2007-07-10 10:50 ` Instruction breakpoint-stepping testsuite " Mark Wielaard
@ 2007-07-16  9:19   ` Mark Wielaard
  0 siblings, 0 replies; 22+ messages in thread
From: Mark Wielaard @ 2007-07-16  9:19 UTC (permalink / raw)
  To: frysk

[-- Attachment #1: Type: text/plain, Size: 3581 bytes --]

Hi,

On Tue, 2007-07-10 at 12:50 +0200, Mark Wielaard wrote:
> I think I know how this can be made way easier. The current setup
> inserts labels at every instruction. This makes it really hard to
> combine with the generic funit-asm.h assembly instructions since it
> introduces 2 layers of macros to write a simple test. And the test has
> to be marked up fully with the labels and a path through the program
> that can be checked. But the current setup already shows an easier way
> to do this. In TestInstructions we already do a full step through the
> assembly and double check that all the labels are hit in the right
> order. But this can be turned around. We can first do this full step
> through the assembly instructions and record all addresses, then we put
> breakpoints at every instruction that we just past and do another run
> with the breakpoints inserted that should then run exactly the same as
> before (except for hitting a breakpoint at every step of course).
> 
> If we do it that way then we just need the start and end address of the
> instruction stream and can hopefully completely reuse funit-asm.h as
> long as the test writer makes sure the program always ends at the same
> address.

The above is what the following patch implements. The test is replaced
by a full .S assembly file that only uses instructions as defined in
frysk-asm.h. It creates 2 global labels istart and iend inside the test
function (instructions) that are used by the rewitten TestInstructions
harness which now assumes setting a breakpoint, stopping and removing it
(to detect istart) and instruction stepping (till iend is detected) work
(which is a good assumption to make since we have other unit tests that
explicitly test this) in setup(). Then each different test then sets
breakpoints on the addresses collected between istart and iend and the
test function is called again by funit-instructions and each test checks
that the same instruction sequence is hit independent from which Code
breakpoint (and/or other) observers are inserted.

frysk-core/frysk/proc/ChangeLog
2007-07-13  Mark Wielaard  <mwielaard@redhat.com>

   * TestInstructions.java (pid, proc, in, out, labels): Removed fields.
   (addresses, io, start_address, end_address): New fields.
   (getGlobalLabelAddress): New method.
   (Symbol): New static class.
   (setup): Rewritten to no use ForkLib and to use Code and Instruction
   observer to find code instruction path.
   (tearDown): Clear addresses.
   (testBreakAndStepInstructions): Renamed to...
   (testBreakOnStartThenStepAllInstructions): Rewriten to use addresses,
   breakpoint on first, delete, then step all.
   (testAllBreakpoints): Rewritten to use addresses.
   (testInsertAllBreakpointsAndStepAll): New test.
   (InstructionObserver.block): New field.
   (InstructionObserver.deletedFrom): Don't stop event loop.
   (InstructionObserver.updateExecuted): Check block field.
   (InstructionObserver.setBlock): New method.

frysk-core/frysk/pkglibdir/ChangeLog
2007-07-13  Mark Wielaard  <mwielaard@redhat.com>

   * funit-instructions.c: Removed.
   * funit-instructions.S: New file.

Tested on x86_64 and x86.

The instruction sequences have been extended a bit to exercise many of
the generic instructions found in frysk-asm.h. The next thing would be
extending these sequences with full architecture specific instructions.
Which has to be added when we have a full ssol instruction parser.

When cross 32-64 bit tests are enabled funit-instructions should now
just compile and run as is in either 32 or 64 bit mode.

Cheers,

Mark

[-- Attachment #2: instructions-test.patch --]
[-- Type: text/x-patch, Size: 20225 bytes --]

Index: frysk-core/frysk/pkglibdir/funit-instructions.S
===================================================================
RCS file: frysk-core/frysk/pkglibdir/funit-instructions.S
diff -N frysk-core/frysk/pkglibdir/funit-instructions.S
--- /dev/null	1 Jan 1970 00:00:00 -0000
+++ frysk-core/frysk/pkglibdir/funit-instructions.S	16 Jul 2007 09:15:37 -0000
@@ -0,0 +1,100 @@
+// This file is part of the program FRYSK.
+//
+// Copyright 2007 Red Hat Inc.
+//
+// FRYSK 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; version 2 of the License.
+//
+// FRYSK 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 FRYSK; if not, write to the Free Software Foundation,
+// Inc., 59 Temple Place, Suite 330, Boston, MA 02111-1307 USA.
+// 
+// In addition, as a special exception, Red Hat, Inc. gives You the
+// additional right to link the code of FRYSK with code not covered
+// under the GNU General Public License ("Non-GPL Code") and to
+// distribute linked combinations including the two, subject to the
+// limitations in this paragraph. Non-GPL Code permitted under this
+// exception must only link to the code of FRYSK through those well
+// defined interfaces identified in the file named EXCEPTION found in
+// the source code files (the "Approved Interfaces"). The files of
+// Non-GPL Code may instantiate templates or use macros or inline
+// functions from the Approved Interfaces without causing the
+// resulting work to be covered by the GNU General Public
+// License. Only Red Hat, Inc. may make changes or additions to the
+// list of Approved Interfaces. You must obey the GNU General Public
+// License in all respects for all of the FRYSK code and other code
+// used in conjunction with FRYSK except the Non-GPL Code covered by
+// this exception. If you modify this file, you may extend this
+// exception to your version of the file, but you are not obligated to
+// do so. If you do not wish to provide this exception without
+// modification, you must delete this exception statement from your
+// version and license this file solely under the GPL without
+// exception.
+
+#include "frysk-asm.h"
+
+.data	// Start of data segment
+	
+.text	// Start of code segment
+
+	// Calls the function instruction twice so the test
+	// harness (TestInstructions) can step through it once
+	// recording all releavant information and then can run
+	// it another time to check that it runs exactly the same
+	// in the presence of Code breakpoint (and/or other) observers.	
+	.global main
+	.type main,@function
+main:
+	ENTER_MAIN
+	CALL(instructions)
+	CALL(instructions)	
+	EXIT
+
+	// Make this function global so the test harness can find
+	// the begin and end of it while stepping and setting
+	// breakpoints. The test harness will use the istart and iend
+	// markers and assume that those two addresses are used only
+	// once per run of this function and that when this function is
+	// called multiple times (see main) the exact same instruction
+	// sequence is executed.
+	.globl instructions,istart,iend
+	.type instructions,@function
+instructions:
+	ENTER
+istart:	NO_OP
+	NO_OP
+	JUMP(label1)
+label2:	JUMP(label3)
+label1: NO_OP
+	JUMP(label2)
+label3:	NO_OP
+	CALL(subfunc)
+	CALL(subfunc)
+iend:	NO_OP
+	EXIT
+
+	// Non-global helper function called from instructions to
+	// check enter/exit of functions.
+	.type subfunc,@function
+subfunc:
+	ENTER
+	LOAD_IMMED(REG2,1)
+sublabel:	
+	COMPARE_IMMED(REG2,1)
+	JUMP_NE(sublabel)
+	LOAD_IMMED(REG2,2)
+	COMPARE_IMMED(REG2,1)
+	JUMP_NE(subexit)
+	JUMP(sublabel)
+subexit:	
+	EXIT
+	
+// make the stack non-executable
+.section        .note.GNU-stack,"",@progbits
+	
Index: frysk-core/frysk/pkglibdir/funit-instructions.c
===================================================================
RCS file: frysk-core/frysk/pkglibdir/funit-instructions.c
diff -N frysk-core/frysk/pkglibdir/funit-instructions.c
--- frysk-core/frysk/pkglibdir/funit-instructions.c	3 Jul 2007 16:35:19 -0000	1.1
+++ /dev/null	1 Jan 1970 00:00:00 -0000
@@ -1,101 +0,0 @@
-// This file is part of the program FRYSK.
-//
-// Copyright 2007 Red Hat Inc.
-//
-// FRYSK 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; version 2 of the License.
-//
-// FRYSK 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 FRYSK; if not, write to the Free Software Foundation,
-// Inc., 59 Temple Place, Suite 330, Boston, MA 02111-1307 USA.
-// 
-// In addition, as a special exception, Red Hat, Inc. gives You the
-// additional right to link the code of FRYSK with code not covered
-// under the GNU General Public License ("Non-GPL Code") and to
-// distribute linked combinations including the two, subject to the
-// limitations in this paragraph. Non-GPL Code permitted under this
-// exception must only link to the code of FRYSK through those well
-// defined interfaces identified in the file named EXCEPTION found in
-// the source code files (the "Approved Interfaces"). The files of
-// Non-GPL Code may instantiate templates or use macros or inline
-// functions from the Approved Interfaces without causing the
-// resulting work to be covered by the GNU General Public
-// License. Only Red Hat, Inc. may make changes or additions to the
-// list of Approved Interfaces. You must obey the GNU General Public
-// License in all respects for all of the FRYSK code and other code
-// used in conjunction with FRYSK except the Non-GPL Code covered by
-// this exception. If you modify this file, you may extend this
-// exception to your version of the file, but you are not obligated to
-// do so. If you do not wish to provide this exception without
-// modification, you must delete this exception statement from your
-// version and license this file solely under the GPL without
-// exception.
-
-#include <stdio.h>
-#include <stdlib.h>
-
-int
-main (int argc, char *argv[], char *envp[])
-{
-  __label__ nop1_instruction;
-  __label__ nop2_instruction;
-  __label__ nop3_instruction;
-
-  __label__ jump1_instruction;
-  __label__ jump2_instruction;
-  __label__ jump3_instruction;
-
-  __label__ done;
-
-  // Tell the tester the addresses of the assembly instruction labels
-  // in order that they should appear.
-  printf("%p\n", &&nop1_instruction);
-  printf("%p\n", &&nop2_instruction);
-  printf("%p\n", &&jump1_instruction);
-  printf("%p\n", &&nop3_instruction);
-  printf("%p\n", &&jump3_instruction);
-  printf("%p\n", &&jump2_instruction);
-  printf("%p\n", &&nop4_instruction);
-  printf("%p\n", &&done);
-  printf("%p\n", (void *) 0);
-  fflush(stdout);
-
-  // Wait till we are OK to go!
-  char c = getchar();
-  if (c == EOF)
-    exit (-1);
-
-// This works for both x86 and x86_64
-#if defined __i386__  || defined __x86_64__
- nop1_instruction:
-  asm volatile ("nop1_instruction: nop");
-  
- nop2_instruction:
-  asm volatile ("nop2_instruction: nop");
-
- jump1_instruction:
-  asm volatile ("jump1_instruction: jmp nop3_instruction");
-
- jump2_instruction:
-  asm volatile ("jump2_instruction: jmp nop4_instruction");
-
- nop3_instruction:
-  asm volatile ("nop3_instruction: nop");
-
- jump3_instruction:
-  asm volatile ("jump3_instruction: jmp jump2_instruction");
-
- nop4_instruction:
-  asm volatile ("nop4_instruction: nop");
-#else
-#error Not ported for this arch.
-#endif
- done:
-  return 0;
-}
Index: frysk-core/frysk/proc/TestInstructions.java
===================================================================
RCS file: /cvs/frysk/frysk-core/frysk/proc/TestInstructions.java,v
retrieving revision 1.1
diff -u -u -r1.1 TestInstructions.java
--- frysk-core/frysk/proc/TestInstructions.java	3 Jul 2007 16:35:19 -0000	1.1
+++ frysk-core/frysk/proc/TestInstructions.java	16 Jul 2007 09:15:37 -0000
@@ -39,29 +39,87 @@
 
 package frysk.proc;
 
-import frysk.testbed.ForkTestLib;
-
-import java.io.BufferedReader;
-import java.io.DataOutputStream;
-import java.io.InputStreamReader;
-import java.io.IOException;
-
 import java.util.ArrayList;
+import java.util.HashMap;
 import java.util.Iterator;
 
+import lib.dw.*;
+import frysk.dwfl.*;
+
 public class TestInstructions
   extends TestLib
 {
-  // Process id, Proc, and Task representation of our test program.
-  private int pid;
-  private Proc proc;
   private Task task;
 
-  // How we communicate with the test program.
-  private BufferedReader in;
-  private DataOutputStream out;
+  private long start_address;
+  private long end_address;
+
+  private ArrayList addresses;
+
+  // Created and added by setup() in blocked state.
+  // Tests will need to delete (or unblock) it as soon as they are ready
+  // setting up their own observer.
+  private InstructionObserver io;
+
+  /**
+   * Returns the address of a global label by quering the the Proc
+   * main Task's Dwlf.
+   */
+  long getGlobalLabelAddress(String label)
+  {
+    Dwfl dwfl = DwflCache.getDwfl(task);
+    Symbol sym = Symbol.get(dwfl, label);
+    return sym.getAddress();
+  }
+  
+  // Helper class since there there isn't a get symbol method in Dwfl,
+  // so we need to wrap it all in a builder pattern.
+  static class Symbol implements SymbolBuilder
+  {
+    private String name;
+    private long address;
+
+    private boolean found;
+
+    private Symbol()
+    {
+      // properties get set in public static get() method.
+    }
+
+    static Symbol get(Dwfl dwfl, String name)
+    {
+      Symbol sym = new Symbol();
+      sym.name = name;
+      DwflModule[] modules = dwfl.getModules();
+      for (int i = 0; i < modules.length && ! sym.found; i++)
+	modules[i].getSymbolByName(name, sym);
+      
+      if (sym.found)
+	return sym;
+      else
+	return null;
+    }
+
+    String getName()
+    {
+      return name;
+    }
+
+    long getAddress()
+    {
+      return address;
+    }
 
-  private ArrayList labels;
+    public void symbol(String name, long value, long size,
+		       int type, int bind, int visibility)
+    {
+      if (name.equals(this.name))
+	{
+	  this.address = value;
+	  this.found = true;
+	}
+    }
+  }
 
   /**
    * Launch our test program and setup clean environment with the test
@@ -74,47 +132,50 @@
     // and destroyed in tearDown().
     super.setUp();
 
-    // Create a process that we will communicate with through stdin/out.
-    String command = getExecPath ("funit-instructions");
-    ForkTestLib.ForkedProcess process;
-    process = ForkTestLib.fork(new String[] { command });
-    pid = process.pid;
-    in = new BufferedReader(new InputStreamReader(process.in));
-    out = new DataOutputStream(process.out);
-    
-    // Make sure the core knows about it.
-    Manager.host.requestFindProc(new ProcId(pid), new Host.FindProc()
-      {
-	public void procFound (ProcId procId)
-	{
-	  proc = Manager.host.getProc(procId);
-	  Manager.eventLoop.requestStop();
-	}
-	public void procNotFound (ProcId procId, Exception e)
-	{
-	  fail(procId + " not found: " + e);
-	}
-      });
-    assertRunUntilStop("finding proc");
-    
-    task = proc.getMainTask();
+    // Create a process that we want to observe.
+    AttachedObserver ao = new AttachedObserver();
+    String[] cmd = new String[] { getExecPath("funit-instructions") };
+    Manager.host.requestCreateAttachedProc("/dev/null",
+                                           "/dev/null",
+                                           "/dev/null", cmd, ao);
+    assertRunUntilStop("attach then block");
+    assertTrue("AttachedObserver got Task", ao.task != null);
+
+    task = ao.task;
+
+    start_address = getGlobalLabelAddress("istart");
+    end_address = getGlobalLabelAddress("iend");
+
+    CodeObserver code = new CodeObserver(start_address);
+    task.requestAddCodeObserver(code, start_address);
+    assertRunUntilStop("inserting setup code observer");
+    task.requestDeleteAttachedObserver(ao);
+    assertRunUntilStop("setup start address code observer");
+
+    // Read the addresses of all the instructions by stepping from
+    // istart to iend.
+    addresses = new ArrayList();
+    addresses.add(Long.valueOf(start_address));
+    io = new InstructionObserver(task);
+    task.requestAddInstructionObserver(io);
+    assertRunUntilStop("setup instruction observer");
+    task.requestDeleteCodeObserver(code, start_address);
+    assertRunUntilStop("setup remove start code observer");
 
-    // Read the addresses of the labels
-    labels = new ArrayList();
-    try
+    boolean iend_seen = false;
+    while (! iend_seen)
       {
-	String line = in.readLine();
-	while (! line.equals("(nil)"))
-	  {
-	    Long label = Long.decode(line);
-	    labels.add(label);
-	    line = in.readLine();
-	  }
-      }
-    catch (IOException e)
-      {
-	fail("reading breakpoint addresses: " + e);
+	task.requestUnblock(io);
+	assertRunUntilStop("Step till iend");
+	long addr = io.getAddr();
+	Long value = Long.valueOf(addr);
+	addresses.add(value);
+	iend_seen = addr == end_address;
       }
+
+    // And make one final step out of the istart-iend range
+    task.requestUnblock(io);
+    assertRunUntilStop("Step out of range");
   }
 
   /**
@@ -124,12 +185,8 @@
    */
   public void tearDown()
   {
-    pid = -1;
-    proc = null;
     task = null;
-    in = null;
-    out = null;
-    labels = null;
+    addresses = null;
 
     Manager.eventLoop.requestStop();
 
@@ -137,65 +194,140 @@
     super.tearDown();
   }
 
-  public void testBreakAndStepInstructions() throws IOException
+  public void testBreakOnStartThenStepAllInstructions()
   {
-    long first = ((Long) labels.remove(0)).longValue();
+    Long value = (Long) addresses.remove(0);
+    long first = value.longValue();
     CodeObserver code = new CodeObserver(first);
     task.requestAddCodeObserver(code, first);
     assertRunUntilStop("first code observer added");
 
-    out.writeByte(1);
-    assertRunUntilStop("go!");
+    // Go!
+    task.requestDeleteInstructionObserver(io);
+    assertRunUntilStop("Remove setup instruction observer");
 
     assertEquals("stopped at first breakpoint",
 		 task.getIsa().pc(task), first);
 
-    InstructionObserver io = new InstructionObserver(task);
+    // Reinsert instruction observer now that we are at the start.
     task.requestAddInstructionObserver(io);
     assertRunUntilStop("add instruction observer");
 
     task.requestUnblock(code);
-    Iterator it = labels.iterator();
+    Iterator it = addresses.iterator();
     while (it.hasNext())
       {
-	long nextLabel = ((Long) it.next()).longValue();
+	long addr = ((Long) it.next()).longValue();
 	task.requestUnblock(io);
-	assertRunUntilStop("unblock for " + nextLabel);
-	assertEquals("step observer hit: " + nextLabel,
-		     io.getAddr(), nextLabel);
+	assertRunUntilStop("unblock for " + addr);
+	assertEquals("step observer hit: " + addr, io.getAddr(), addr);
       }
 
       task.requestUnblock(io);
   }
 
-  public void testAllBreakpoints() throws IOException
+  public void testAllBreakpoints()
   {
+    // Map to make sure that even if we loop only one code observer is
+    // inserted at each address.
+    HashMap codeObserver = new HashMap();
     ArrayList codeObservers = new ArrayList();
-    Iterator it = labels.iterator();
+    Iterator it = addresses.iterator();
     while (it.hasNext())
       {
-	long label = ((Long) it.next()).longValue();
-	CodeObserver code = new CodeObserver(label);
-	task.requestAddCodeObserver(code, label);
+	Long value = (Long) it.next();
+	CodeObserver code = (CodeObserver) codeObserver.get(value);
+	if (code == null)
+	  {
+	    long addr = value.longValue();
+	    code = new CodeObserver(addr);
+	    codeObserver.put(value, code);
+	    task.requestAddCodeObserver(code, addr);
+	    assertRunUntilStop("add code observer" + addr);
+	  }
 	codeObservers.add(code);
-	assertRunUntilStop("add code observer for " + label);
       }
 
-    out.writeByte(1);
-    assertRunUntilStop("go!");
+    // Go!
+    task.requestDeleteInstructionObserver(io);
+    assertRunUntilStop("Remove setup instruction observer");
 
-    it = labels.iterator();
+    it = addresses.iterator();
     while (it.hasNext())
       {
-	long label = ((Long) it.next()).longValue();
+	long addr = ((Long) it.next()).longValue();
 	CodeObserver code = (CodeObserver) codeObservers.remove(0);
-	assertEquals("code observer hit: " + label,
-		     task.getIsa().pc(task), label);
+	assertEquals("code observer hit: " + addr,
+		     task.getIsa().pc(task), addr);
 	task.requestUnblock(code);
 	if (it.hasNext())
 	  assertRunUntilStop("wait for next code observer hit after "
-			     + Long.toHexString(label));
+			     + Long.toHexString(addr));
+      }
+  }
+
+  public void testInsertAllBreakpointsAndStepAll()
+  {
+    // Map to make sure that even if we loop only one code observer is
+    // inserted at each address.
+    HashMap codeObserver = new HashMap();
+    ArrayList codeObservers = new ArrayList();
+    Iterator it = addresses.iterator();
+    while (it.hasNext())
+      {
+	Long value = (Long) it.next();
+	CodeObserver code = (CodeObserver) codeObserver.get(value);
+	if (code == null)
+	  {
+	    long addr = value.longValue();
+	    code = new CodeObserver(addr);
+	    codeObserver.put(value, code);
+	    task.requestAddCodeObserver(code, addr);
+	    assertRunUntilStop("add code observer" + addr);
+	  }
+	codeObservers.add(code);
       }
+
+    // Go!
+    io.setBlock(false);
+    task.requestUnblock(io);
+    assertRunUntilStop("Unblock setup instruction observer");
+    
+    it = addresses.iterator();
+    while (it.hasNext())
+      {
+	long addr = ((Long) it.next()).longValue();
+	CodeObserver code = (CodeObserver) codeObservers.remove(0);
+	assertEquals("code observer hit: " + addr,
+		     task.getIsa().pc(task), addr);
+	assertEquals("step observer hit: " + addr, io.getAddr(), addr);
+	task.requestUnblock(io);
+	task.requestUnblock(code);
+	if (it.hasNext())
+	  assertRunUntilStop("wait for next code observer hit after "
+			     + Long.toHexString(addr));
+      }
+  }
+  
+  class AttachedObserver implements TaskObserver.Attached
+  {
+    Task task;
+    
+    public void addedTo (Object o){ }
+    
+    public Action updateAttached (Task task)
+    {
+      this.task = task;
+      Manager.eventLoop.requestStop();
+      return Action.BLOCK;
+    }
+    
+    public void addFailed  (Object observable, Throwable w)
+    {
+      System.err.println("addFailed: " + observable + " cause: " + w);
+    }
+    
+    public void deletedFrom (Object o) { }
   }
 
   private class CodeObserver
@@ -216,7 +348,7 @@
       Manager.eventLoop.requestStop();
       return Action.BLOCK;
     }
-
+    
     public void addFailed(Object o, Throwable w)
     {
       fail("add to " + o + " failed, because " + w);
@@ -239,6 +371,8 @@
     private final Task task; 
     private long addr;
 
+    private boolean block = true;
+
     public void addedTo(Object o)
     {
       Manager.eventLoop.requestStop();
@@ -246,7 +380,6 @@
 
     public void deletedFrom(Object o)
     {
-      Manager.eventLoop.requestStop();
     }
 
     public void addFailed (Object o, Throwable w)
@@ -267,8 +400,17 @@
                                         + this.task);
       
       addr = task.getIsa().pc(task);
-      Manager.eventLoop.requestStop();
-      return Action.BLOCK;
+      if (block)
+	{
+	  Manager.eventLoop.requestStop();
+	  return Action.BLOCK;
+	}
+      return Action.CONTINUE;
+    }
+
+    public void setBlock(boolean block)
+    {
+      this.block = block;
     }
 
     public long getAddr()

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: Leaving visible breakpoints in memory/core (Was: Breakpoint  stepping)
  2007-07-12 14:24             ` Phil Muldoon
  2007-07-12 20:24               ` Roland McGrath
@ 2007-07-16 15:53               ` Mark Wielaard
  2007-07-17 15:47                 ` Phil Muldoon
  1 sibling, 1 reply; 22+ messages in thread
From: Mark Wielaard @ 2007-07-16 15:53 UTC (permalink / raw)
  To: Phil Muldoon; +Cc: Roland McGrath, frysk

[-- Attachment #1: Type: text/plain, Size: 1327 bytes --]

Hi Phil,

On Thu, 2007-07-12 at 09:24 -0500, Phil Muldoon wrote:
> The second question speaks to a Frysk mechanics question, if we do 
> remove all the bits, then dump core, can we replace them exactly as they 
> were before? I'd like to say yes, but I get the nagging feeling this 
> isn't so? What do others think?

Unless you record them somewhere you won't be able to reinsert them. I
personally don't think you should record them and try to reinsert them
when creating a core file through fcore because I think that the user
requesting the core file to be created from frysk is interested in the
original code.

But if you really wanted to then you could at least get the breakpoints
instructions that frysk-core inserted from BreakpointAddresses (which I
will soon extend with a handy getAllAdresses() method for fixing bug
#4761), and then you can get the actual breakpoint instruction bytes
used from the relevant Isa getBreakpointInstruction(). This is probably
too low level though. And since you cannot guarantee that the created
core file is read back through frysk (someone could be using gdb!) I
don't know where you would actually put this information or how you
would make all consumers know about both the original bytes and the
breakpoint instructions in the image.

Cheers,

Mark

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 189 bytes --]

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: Leaving visible breakpoints in memory/core (Was: Breakpoint  stepping)
  2007-07-12 20:24               ` Roland McGrath
@ 2007-07-16 15:57                 ` Mark Wielaard
  2007-07-17 15:43                   ` Phil Muldoon
  0 siblings, 1 reply; 22+ messages in thread
From: Mark Wielaard @ 2007-07-16 15:57 UTC (permalink / raw)
  To: Roland McGrath; +Cc: Phil Muldoon, frysk

[-- Attachment #1: Type: text/plain, Size: 530 bytes --]

On Thu, 2007-07-12 at 13:23 -0700, Roland McGrath wrote:
> For a nondestructive dump, you also have the option of editting the memory
> as you copy it.

Yes, that is the plan for bug #4761 (Task Memory view without inserted
breakpoints showing). There will be a getMemory() view that shows the
original bytes as found in the process (with anything frysk-core might
have added to it for things like breakpointing filtered out) and a
getRawMemory() that gives the raw bytes as manipulated by frysk-core.

Cheers,

Mark

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 189 bytes --]

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: Leaving visible breakpoints in memory/core (Was: Breakpoint stepping)
  2007-07-16 15:57                 ` Mark Wielaard
@ 2007-07-17 15:43                   ` Phil Muldoon
  2007-07-17 17:06                     ` Mark Wielaard
  0 siblings, 1 reply; 22+ messages in thread
From: Phil Muldoon @ 2007-07-17 15:43 UTC (permalink / raw)
  To: Mark Wielaard; +Cc: Roland McGrath, frysk

Mark Wielaard wrote:
> On Thu, 2007-07-12 at 13:23 -0700, Roland McGrath wrote:
>   
>> For a nondestructive dump, you also have the option of editting the memory
>> as you copy it.
>>     
>
> Yes, that is the plan for bug #4761 (Task Memory view without inserted
> breakpoints showing). There will be a getMemory() view that shows the
> original bytes as found in the process (with anything frysk-core might
> have added to it for things like breakpointing filtered out) and a
> getRawMemory() that gives the raw bytes as manipulated by frysk-core.
>   
Mark

Some questions thoughts:

How is this going to be implemented in the task? Are you planning on 
extending the abstract class Task with a getRawMemory()?

If so, what does that mean for an implementing core file task? In the 
corefile task (dead/LinuxTask.java) are getMemory() and getRawMemory() 
returning the same ByteBuffer (CorefileByteBuffer) instance?

Is this too live process specific and should be implemented some other way?

Regards

Phil

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: Leaving visible breakpoints in memory/core (Was: Breakpoint stepping)
  2007-07-16 15:53               ` Mark Wielaard
@ 2007-07-17 15:47                 ` Phil Muldoon
  2007-07-17 17:08                   ` Mark Wielaard
  0 siblings, 1 reply; 22+ messages in thread
From: Phil Muldoon @ 2007-07-17 15:47 UTC (permalink / raw)
  To: Mark Wielaard; +Cc: Roland McGrath, frysk

Mark Wielaard wrote:
> But if you really wanted to then you could at least get the breakpoints
> instructions that frysk-core inserted from BreakpointAddresses (which I
> will soon extend with a handy getAllAdresses() method for fixing bug
> #4761), and then you can get the actual breakpoint instruction bytes
> used from the relevant Isa getBreakpointInstruction(). This is probably
> too low level though. And since you cannot guarantee that the created
> core file is read back through frysk (someone could be using gdb!) I
> don't know where you would actually put this information or how you
> would make all consumers know about both the original bytes and the
> breakpoint instructions in the image.
>   

What's the argument about just dumping the task's memory as is, with all 
breakpoints there? A corefile is a representation of that process and 
it's thread as it is at that time ...

And besides, this is only addressing a tiny corner case of someone 
running fcore on a process that is being debugged with Frysk at that 
time. I can't think of a scenario why I would do that. Can you? If they 
were debugging with GDB and fcore was run on that process, the included 
core image would have all the break point information.

It seems like a lot of work for a very small edge case?

Regards

Phil

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: Leaving visible breakpoints in memory/core (Was: Breakpoint  stepping)
  2007-07-17 15:43                   ` Phil Muldoon
@ 2007-07-17 17:06                     ` Mark Wielaard
  0 siblings, 0 replies; 22+ messages in thread
From: Mark Wielaard @ 2007-07-17 17:06 UTC (permalink / raw)
  To: Phil Muldoon; +Cc: Roland McGrath, frysk

[-- Attachment #1: Type: text/plain, Size: 1508 bytes --]

Hi Phil,

On Tue, 2007-07-17 at 10:42 -0500, Phil Muldoon wrote:
> How is this going to be implemented in the task? Are you planning on 
> extending the abstract class Task with a getRawMemory()?

Yes. With an default implementation:

  /**                                                                           
   * Returns the memory as seen by frysk-core. That includes things like
   * inserted breakpoint instructions bytes which are filtered out by
   * <code>getMemory()</code> (which is what you normally want unless           
   * you are interested in frysk-core specifics).
   * <p>                               
   * Default implementation calls <code>getMemory()</code>, need to be
   * overriden by subclasses for which the raw memory view and the
   * logical memory view are different.
   */
  public ByteBuffer getRawMemory()
  {
    return getMemory();
  }

> If so, what does that mean for an implementing core file task? In the 
> corefile task (dead/LinuxTask.java) are getMemory() and getRawMemory() 
> returning the same ByteBuffer (CorefileByteBuffer) instance?

Yes, you get the support for free.

> Is this too live process specific and should be implemented some other way?

No, I think the generic implementation is the way to go so the user
doesn't have to worry about what specific implementation instance of
Task they are using. But please do comment when I post the patch and you
feel it should be done differently.

Cheers,

Mark

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 189 bytes --]

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: Leaving visible breakpoints in memory/core (Was: Breakpoint  stepping)
  2007-07-17 15:47                 ` Phil Muldoon
@ 2007-07-17 17:08                   ` Mark Wielaard
  0 siblings, 0 replies; 22+ messages in thread
From: Mark Wielaard @ 2007-07-17 17:08 UTC (permalink / raw)
  To: Phil Muldoon; +Cc: Roland McGrath, frysk

[-- Attachment #1: Type: text/plain, Size: 2815 bytes --]

Hi Phil,

On Tue, 2007-07-17 at 10:46 -0500, Phil Muldoon wrote:
> Mark Wielaard wrote:
> What's the argument about just dumping the task's memory as is, with all 
> breakpoints there? A corefile is a representation of that process and 
> it's thread as it is at that time ...

Because you don't know why/what those breakpoint instructions are doing
there. Since none of the observer logic is saved together with the core
file you wouldn't know why and what that breakpoint instruction is doing
there. Worse, you don't even know what is underneath it.

> And besides, this is only addressing a tiny corner case of someone 
> running fcore on a process that is being debugged with Frysk at that 
> time. I can't think of a scenario why I would do that. Can you?

Yes. The scenario using fcore I have in mind is one where a user has
reported some behavior that cannot easily be explained and that only
happens in some production environment. You might have an idea why it
might happen but need proof and would need to inspect the actual state
of the process more closely when it happens. But the user doesn't want
to hand over the whole running process to you so you can stop it
completely. So you write a little frysk-core based program that triggers
under certain conditions (possibly involving breakpoints) and that
generates a core file of the full process state but that you then want
to inspect offline (maybe even by a third person with something like
gdb). You want the fcore generated file to represent the state of the
process as if it was pristine, not with lingering low level breakpoint
or other frysk artifacts in it so you know what the actual code was
doing.

>  If they 
> were debugging with GDB and fcore was run on that process, the included 
> core image would have all the break point information.

Yes, although in practise this currently doesn't work since our ptrace
based implementation won't let two different processes attach to the
same process.

> It seems like a lot of work for a very small edge case?

But it isn't just for fcore. And as you showed above maybe fcore should
have an option to select whether or not to dump logical memory or raw
memory. It is mainly used for other observers inspecting the task. We
don't want to retract all low level breakpoints when one task stops (so
other tasks can keep running and possible hit that or another low level
breakpoint). But when looking at the actual memory dump we want it to
show what would actually be there. The user is most likely interested in
the actual instruction at that point, not in the frysk inserted
breakpoint instruction. This makes the higher level tasks not need to
worry how the actual implementation of some lower level task actually
works.

Cheers,

Mark

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 189 bytes --]

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: Breakpoint stepping
  2007-07-05 18:37 ` Breakpoint stepping Andrew Cagney
@ 2007-07-23 12:19   ` Mark Wielaard
  0 siblings, 0 replies; 22+ messages in thread
From: Mark Wielaard @ 2007-07-23 12:19 UTC (permalink / raw)
  To: frysk

[-- Attachment #1: Type: text/plain, Size: 589 bytes --]

Hi Andrew,

On Thu, 2007-07-05 at 14:37 -0400, Andrew Cagney wrote:
> Can I suggest adding this, or something based on it, to 
> frysk.proc.live.package.html?  It's this useful! information we should 
> be accumulating in the source tree.

Merged with the old stepping story I send to the list last month and
cleanup up to only mention the current implementation details and
current deficiencies:

2007-07-23  Mark Wielaard  <mwielaard@redhat.com>

        * package.html: New file describing implementation details of
        Instruction and Code observers.

See attached.

Cheers,

Mark

[-- Attachment #2: package.html.gz --]
[-- Type: application/x-gzip, Size: 3753 bytes --]

^ permalink raw reply	[flat|nested] 22+ messages in thread

end of thread, other threads:[~2007-07-23 12:19 UTC | newest]

Thread overview: 22+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2007-07-04 18:20 Breakpoint stepping Mark Wielaard
2007-07-05  4:45 ` Phil Muldoon
2007-07-05 12:39   ` Mark Wielaard
2007-07-10  9:59     ` Leaving visible breakpoints in memory/core (Was: Breakpoint stepping) Mark Wielaard
2007-07-10 13:52       ` Andrew Cagney
2007-07-10 18:06       ` Phil Muldoon
2007-07-11  9:47         ` Mark Wielaard
2007-07-12  2:49           ` Roland McGrath
2007-07-12 14:24             ` Phil Muldoon
2007-07-12 20:24               ` Roland McGrath
2007-07-16 15:57                 ` Mark Wielaard
2007-07-17 15:43                   ` Phil Muldoon
2007-07-17 17:06                     ` Mark Wielaard
2007-07-16 15:53               ` Mark Wielaard
2007-07-17 15:47                 ` Phil Muldoon
2007-07-17 17:08                   ` Mark Wielaard
2007-07-05 18:37 ` Breakpoint stepping Andrew Cagney
2007-07-23 12:19   ` Mark Wielaard
2007-07-10 10:39 ` Instruction parser (Was: Breakpoint stepping) Mark Wielaard
2007-07-10 10:50 ` Instruction breakpoint-stepping testsuite " Mark Wielaard
2007-07-16  9:19   ` [patch] " Mark Wielaard
2007-07-10 10:57 ` SSOL Area " Mark Wielaard

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).