patch to add breakpoint commands

patch to add breakpoint commands

[Tom Tromey]

Here's a patch that adds breakpoint commands.  I recommend not
committing it, because it is ugly, but I thought I'd post it so folks
can see what needs to be done.

Problems with this:

* I really wanted to give commands a "-silent" option (I never liked
  gdb's "'silent' is a magical first command" thing) -- but doing this
  would mean hooking into BreakpointCommand.

* We need a new command to show all the commands.  I guess plain
  "commands" could do it.  Really, though, I think "actions" should do
  it (a la "info b" in gdb)...

* Also notice the WeakReference stuff.  I guess other choices for
  doing the memory management would have worked ok.  Personally I
  would probably just refactor SourceBreakpoint, or make a new
  subclass used by all standard fhpd commands, and put a lot of stuff
  into that -- silence -vs- announcing the breakpoint, commands,
  conditions.

* CLI is strangely factored... it holds the prompt but only fhpd.java
  uses it.  I don't really follow the design here.  Ideally (for
  "commands") CLI should be re-entrant...

* Related to that, "commands" doesn't know about itself, so the first
  "end" will terminate the commands, even if you try to nest them.

* There is some cut-and-paste code in CommandsCommand, mostly taken
  from ActionPointCommands, which has its own share of cut-and-paste
  and weird stuff.

Really this is 4th or 5th on my must-have list.  More important would
be breakpoint conditions.  Or annotations -- but I wanted to get my
feet wet with something simpler first.

Tom

diff --git a/frysk-core/frysk/hpd/CLI.java b/frysk-core/frysk/hpd/CLI.java
index f8f16c8..dbe3243 100644
--- a/frysk-core/frysk/hpd/CLI.java
+++ b/frysk-core/frysk/hpd/CLI.java
@@ -192,7 +192,7 @@ public class CLI {
     final PrintWriter outWriter;
     private Preprocessor prepro;
     private String prompt; // string to represent prompt, will be moved
-    private final Command topLevelCommand = new TopLevelCommand();
+    final Command topLevelCommand = new TopLevelCommand();
     final DbgVariables dbgvars = new DbgVariables();
 
     // PT set related stuff
@@ -209,6 +209,11 @@ public class CLI {
     // alias
     final HashMap aliases;
 
+    // When evaluating a command, this holds an iterator of the
+    // following commands on the line.  Usually this should only be
+    // used by commands which themselves read and store commands.
+    Iterator inputIterator;
+
     /*
      * Public methods
      */
@@ -277,8 +282,9 @@ public class CLI {
 	    // NULL when EOF.
 	    try {
 		// preprocess and iterate
-		for (Iterator iter = prepro.preprocess(cmd); iter.hasNext();) {
-		    String pcmd = (String)iter.next();
+		inputIterator = prepro.preprocess(cmd);
+		while (inputIterator.hasNext()) {
+		    String pcmd = (String)inputIterator.next();
 		    Input command = new Input(pcmd);
 		    // Ignore empty commands
 		    if (command.size() > 0)
@@ -287,6 +293,7 @@ public class CLI {
 	    } catch (RuntimeException e) {
 		printError(e);
 	    }
+	    inputIterator = null;
 	}
 	flushMessages();
 	return null;
diff --git a/frysk-core/frysk/hpd/CommandsCommand.java b/frysk-core/frysk/hpd/CommandsCommand.java
new file mode 100644
index 0000000..a7bcba6
--- /dev/null
+++ b/frysk-core/frysk/hpd/CommandsCommand.java
@@ -0,0 +1,209 @@
+// This file is part of the program FRYSK.
+//
+// Copyright 2008, 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.
+
+package frysk.hpd;
+
+import frysk.rt.SourceBreakpointObserver;
+import frysk.rt.SourceBreakpoint;
+import frysk.rt.BreakpointManager;
+import frysk.proc.Task;
+import frysk.util.ObservingTerminal;
+import frysk.sys.FileDescriptor;
+
+import jline.ConsoleReader;
+
+import java.io.FileInputStream;
+import java.io.IOException;
+import java.io.PrintWriter;
+import java.lang.ref.WeakReference;
+import java.util.List;
+import java.util.ArrayList;
+import java.util.Arrays;
+import java.util.Iterator;
+
+public class CommandsCommand extends ParameterizedCommand {
+    private static class Options {
+	boolean delete;
+    }
+
+    Object options() {
+	return new Options();
+    }
+
+    private static class CommandsObserver implements SourceBreakpointObserver {
+	private CLI cli;
+	private List commands;
+
+	public CommandsObserver(CLI cli, List commands) {
+	    this.cli = cli;
+	    this.commands = commands;
+	}
+
+	public void updateHit(SourceBreakpoint breakpoint, Task task,
+			      long address) {
+	    Iterator i = commands.iterator();
+	    while (i.hasNext()) {
+		Input cmd = (Input) i.next();
+		cli.topLevelCommand.interpret(cli, cmd);
+	    }
+	}
+
+	public void addedTo(Object o) { }
+	public void addFailed(Object o, Throwable t) { }
+	public void deletedFrom(Object o) { }
+    }
+
+    // FIXME: super lame.
+    private WeakReference[] allCommands;
+
+    CommandsCommand() {
+	// FIXME: we'd like to support '-silent', like gdb's "silent", but
+	// that actually requires changes in BreakpointCommand.
+	super("Set or modify commands for an action point",
+	      "commands ACTIONPOINT-ID",
+	      "Set or delete the commands attached to an action point.  "
+	      + "With -delete, the command list is deleted.  "
+	      + "Otherwise, commands are read until \"end\".");
+	add(new CommandOption("delete", "Delete commands for action point") {
+		void parse(String arg, Object options) {
+		    ((Options) options).delete = true;
+		}
+	    });
+    }
+
+    // Add strings from the iterator to the list.
+    // If we see "end", return true.
+    // Otherwise add all the strings and return false.
+    private boolean addFromIterator(Iterator iter, List result) {
+	while (iter.hasNext()) {
+	    String cmd = (String) iter.next();
+	    if (cmd.equals("end"))
+		return true;
+	    result.add(new Input(cmd));
+	}
+	return false;
+    }
+
+    private List readCommands(CLI cli) {
+	List result = new ArrayList();
+	// Perhaps there were other commands on the same command line.
+	if (cli.inputIterator != null) {
+	    if (addFromIterator(cli.inputIterator, result))
+		return result;
+	}
+	// Read more commands from the user.
+	// FIXME: should be able to use the same reader as the
+	// top-level, but this is not available anywhere.
+	try {
+	    ConsoleReader reader = new ConsoleReader
+		(new FileInputStream(java.io.FileDescriptor.in),
+		 new PrintWriter(System.out),
+		 null,
+		 new ObservingTerminal(FileDescriptor.in));
+	    Preprocessor prep = new Preprocessor();
+
+	    while (true) {
+		String line = reader.readLine("> ");
+		if (line != null) {
+		    if (addFromIterator(prep.preprocess(line), result)) {
+			return result;
+		    }
+		}
+	    }
+	} catch (IOException e) {
+	    throw new RuntimeException(e);
+	}
+    }
+
+    void interpret(CLI cli, Input input, Object opts) {
+	if (input.size() == 0) {
+	    throw new InvalidCommandException("missing actionpoint argument");
+	} else if (input.size() != 1) {
+	    throw new InvalidCommandException("too many arguments");
+	}
+
+	// Cut and paste from ActionPointCommands.
+	String[] points = input.stringValue().split(",");
+	int[] ids = new int[points.length];
+	for (int i = 0; i < points.length; i++)
+	    try {
+		ids[i] = Integer.parseInt(points[i]);
+	    } catch (NumberFormatException e) {
+		throw new InvalidCommandException
+		    ("Invalid actionpoint id " + points[i]);
+	    }
+
+	Options options = (Options) opts;
+	List commands = null;
+	if (!options.delete)
+	    commands = readCommands(cli);
+
+	BreakpointManager bpManager = cli.getSteppingEngine()
+	    .getBreakpointManager();
+	Iterator iterator = bpManager.getBreakpointTableIterator();
+	while (iterator.hasNext()) {
+	    SourceBreakpoint bpt = (SourceBreakpoint) iterator.next();
+	    int id = bpt.getId();
+	    if (Arrays.binarySearch(ids, id) < 0)
+		continue;
+	    if (options.delete) {
+		if (allCommands == null || id >= allCommands.length
+		    || allCommands[id].get() == null)
+		    continue;
+		bpt.deleteObserver((CommandsObserver) allCommands[id].get());
+	    } else {
+		if (allCommands == null) {
+		    allCommands = new WeakReference[20];
+		} else if (allCommands.length <= id) {
+		    WeakReference[] newCommands = new WeakReference[id];
+		    System.arraycopy(newCommands, 0, allCommands, 0,
+				     allCommands.length);
+		    allCommands = newCommands;
+		}
+		CommandsObserver ob = new CommandsObserver(cli, commands);
+		bpt.addObserver(ob);
+		allCommands[id] = new WeakReference(ob);
+	    }
+	}
+    }
+
+    int completer(CLI cli, Input input, int cursor, List candidates) {
+	return -1;
+    }
+}
diff --git a/frysk-core/frysk/hpd/TopLevelCommand.java b/frysk-core/frysk/hpd/TopLevelCommand.java
index d6d14bc..81e1d81 100644
--- a/frysk-core/frysk/hpd/TopLevelCommand.java
+++ b/frysk-core/frysk/hpd/TopLevelCommand.java
@@ -1,6 +1,6 @@
 // This file is part of the program FRYSK.
 //
-// Copyright 2007, Red Hat Inc.
+// Copyright 2007, 2008, 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
@@ -119,5 +119,6 @@ public class TopLevelCommand extends MultiLevelCommand {
         add(new ViewsetCommand(), "viewset");
         add(new DefsetCommand(), "defset");
         add(new UndefsetCommand(), "undefset");
+	add(new CommandsCommand(), "commands");
     }
 }

Re: patch to add breakpoint commands

[Andrew Cagney]

Tom Tromey wrote:
> Here's a patch that adds breakpoint commands.  I recommend not
> committing it, because it is ugly, but I thought I'd post it so folks
> can see what needs to be done.
>
> Problems with this:
>
> * I really wanted to give commands a "-silent" option (I never liked
>   gdb's "'silent' is a magical first command" thing) -- but doing this
>   would mean hooking into BreakpointCommand.
>   

I have to agree; hpd describes this entire group as "actionpoints" and 
lists:

- breakpoints
- watchpoints
- barrier

and we've suggestions for additions such as:

- catch syscall
- catch dlopen (?)
- catch signal
- catch fork
- catch clone
- catch exec

for all of these they have several things:

- the commands to run when the event is triggered
- should the hit be verbose or silent
- should the end result be to stop or continue (attribute or not?)
- ?

that can be characterised as attributes; as, dare I suggest, you're 
reading into the intent of the HPD, the way to do this is would be with  
-silent option to the commands that create and edit (see actions) 
actionpoints.

Andrew

Re: patch to add breakpoint commands

[Tom Tromey]

Andrew> for all of these they have several things:
[...]
Andrew> - should the end result be to stop or continue (attribute or not?)

For this I don't see much wrong with the gdb model -- stop by default,
let the command specify "go" if it wants.  This has never caused me
much trouble (while OTOH I frequently forget 'silent').

Andrew> that can be characterised as attributes; as, dare I suggest,
Andrew> you're reading into the intent of the HPD, the way to do this
Andrew> is would be with -silent option to the commands that create
Andrew> and edit (see actions) actionpoints.

Yeah, that would be fine by me.

Tom