From: Phil Muldoon <pmuldoon@redhat.com>
To: frysk@sourceware.org
Subject: Re: [SCM] master: Create Watchpoint data class. Return Watchpoint class instead of long. Do not allow user to set global/local flags. Add new test,testing new getAllWatchPoints() funtion.
Date: Thu, 03 Apr 2008 13:07:00 -0000 [thread overview]
Message-ID: <47F4D665.7040101@redhat.com> (raw)
In-Reply-To: <20080403130002.26637.qmail@sourceware.org>
pmuldoon@sourceware.org wrote:
This commit covers several areas:
- New watchpoint data class to hold the specifics of a watchpoint. It
leaks implementations details right now as it is not package private and
contains register information. When the "how to optimize watchpoints"
questions has been answered, I'll close these gaps
- readWatchpoint now returns a watch point class over just a long
- getAllWatchpoints() is a new function that returns all the watchpoints
that are held in the debug control register. Added a test that tests
this, and by default the modified readWatchpoint class.
- The tests now use a real process and locate an elf symbol to base
their watchpoint tests on, instead of an arbitrary address. Because of
this I've add a temporary elf symbol locater private class in the code
until location of elf symbols is better managed in Frysk.
Regards
Phil
> The branch, master has been updated
> via 72db660d1ea985caa145e3cf93880c6e705d8b51 (commit)
> from 21953327b97883d2adb9423c3dad56fc319f2f12 (commit)
>
> Those revisions listed above that are new to this repository have
> not appeared on any other notification email.
>
> - Log -----------------------------------------------------------------
> commit 72db660d1ea985caa145e3cf93880c6e705d8b51
> Author: Phil Muldoon <pmuldoon@redhat.com>
> Date: Thu Apr 3 13:58:27 2008 +0100
>
> Create Watchpoint data class. Return Watchpoint class instead of long. Do not allow user to set global/local flags. Add new test,testing new getAllWatchPoints() funtion.
>
> 2008-04-03 Phil Muldoon <pmuldoon@redhat.com>
>
> * TestWatchpoint.java (Symbol.Symbol): New temporary class to resolve
> Elf symbols.
> (getGlobalSymbolAddress): New function.
> (testWatchFourBytesBitPattern): Adjust to use funit-watchpoints.S.
> GetAddress from Watchpoint.java
> (testWatchTwoBytesBitPattern): Ditto.
> (testWatchOneByteBitPattern): Ditto.
> (testGetAllWatchpoints): New.
> * IA32WatchpointFunctions.java (setWatchpoint): Delete localOnly flag.
> Remove local/global breakpoint bit logic.
> (readWatchpoint): Rewrite. Return Watchpoint.
> * X8664WatchpointFunctions.java (setWatchpoint): Delete localOnly flag.
> Remove local/global breakpoint bit logic.
> (readWatchpoint): Rewrite. Return Watchpoint.
> * WatchpointFunctions.java (getAllWatchpoints): New. Implement.
> (setWatchpoint): Remove localOnly flag.
> (readWatchpoint): return a Watchpoint class over a long.
>
> -----------------------------------------------------------------------
>
> Summary of changes:
> frysk-core/frysk/isa/watchpoints/ChangeLog | 20 +++
> .../isa/watchpoints/IA32WatchpointFunctions.java | 67 +++++++---
> .../frysk/isa/watchpoints/TestWatchpoint.java | 135 +++++++++++++++++---
> .../watchpoints/Watchpoint.java} | 98 +++++++--------
> .../frysk/isa/watchpoints/WatchpointFunctions.java | 25 +++-
> .../isa/watchpoints/X8664WatchpointFunctions.java | 65 +++++++---
> 6 files changed, 299 insertions(+), 111 deletions(-)
> copy frysk-core/frysk/{symtab/Symbol.java => isa/watchpoints/Watchpoint.java} (53%)
>
> First 500 lines of diff:
> diff --git a/frysk-core/frysk/isa/watchpoints/ChangeLog b/frysk-core/frysk/isa/watchpoints/ChangeLog
> index e3f419a..93952e1 100644
> --- a/frysk-core/frysk/isa/watchpoints/ChangeLog
> +++ b/frysk-core/frysk/isa/watchpoints/ChangeLog
> @@ -1,3 +1,23 @@
> +2008-04-03 Phil Muldoon <pmuldoon@redhat.com>
> +
> + * TestWatchpoint.java (Symbol.Symbol): New temporary class to resolve
> + Elf symbols.
> + (getGlobalSymbolAddress): New function.
> + (testWatchFourBytesBitPattern): Adjust to use funit-watchpoints.S.
> + GetAddress from Watchpoint.java
> + (testWatchTwoBytesBitPattern): Ditto.
> + (testWatchOneByteBitPattern): Ditto.
> + (testGetAllWatchpoints): New.
> + * IA32WatchpointFunctions.java (setWatchpoint): Delete localOnly flag.
> + Remove local/global breakpoint bit logic.
> + (readWatchpoint): Rewrite. Return Watchpoint.
> + * X8664WatchpointFunctions.java (setWatchpoint): Delete localOnly flag.
> + Remove local/global breakpoint bit logic.
> + (readWatchpoint): Rewrite. Return Watchpoint.
> + * WatchpointFunctions.java (getAllWatchpoints): New. Implement.
> + (setWatchpoint): Remove localOnly flag.
> + (readWatchpoint): return a Watchpoint class over a long.
> +
> 2008-04-02 Phil Muldoon <pmuldoon@redhat.com>
>
> * Watchpoint.java: Refactor to WatchpointFunctions.java
> diff --git a/frysk-core/frysk/isa/watchpoints/IA32WatchpointFunctions.java b/frysk-core/frysk/isa/watchpoints/IA32WatchpointFunctions.java
> index 15117c5..e2883c7 100644
> --- a/frysk-core/frysk/isa/watchpoints/IA32WatchpointFunctions.java
> +++ b/frysk-core/frysk/isa/watchpoints/IA32WatchpointFunctions.java
> @@ -65,8 +65,7 @@ class IA32WatchpointFunctions extends WatchpointFunctions {
> */
> public void setWatchpoint(Task task, int index,
> long addr, int range,
> - boolean writeOnly,
> - boolean localOnly) {
> + boolean writeOnly) {
>
>
> // turn bit off (b = bit no): l &= ~(1L << b)
> @@ -86,17 +85,10 @@ class IA32WatchpointFunctions extends WatchpointFunctions {
> // Calculate "Global Exact Breakpoint #index Enabled" bit to set
> int bitToSet = index * 2;
>
> - if (localOnly) {
> - // Set "Local Exact Breakpoint #index Enabled" to 0
> - debugControl |= (1L << bitToSet);
> - // Set Global Exact Breakpoint to 1
> - debugControl &= ~(1L << bitToSet+1);
> - } else {
> - // Set "Local Exact Breakpoint #index Enabled" to 0
> - debugControl &= ~(1L << bitToSet);
> - // Set Global Exact Breakpoint to 1
> - debugControl |= (1L << bitToSet+1);
> - }
> + // Set "Local Exact Breakpoint #index Enabled" to 0
> + debugControl &= ~(1L << bitToSet);
> + // Set Global Exact Breakpoint to 1
> + debugControl |= (1L << bitToSet+1);
>
> // Dending on the WP register to set, the next
> // 4 bits are offset 4 * WP Count. On x8664
> @@ -148,18 +140,53 @@ class IA32WatchpointFunctions extends WatchpointFunctions {
> "range. Has to be 1, 2, 4 or 8");
> }
>
> +
> /**
> * Reads a watchpoint. Takes a task, and an index.
> *
> * @param task - task to read a watchpoint from.
> * @param index - watchpoint number to read.
> - *
> - * @return long - value of register for watchpoint.
> +
> + * @return Watchpoint - value of Watchpoint at
> + * register.
> */
> - public long readWatchpoint(Task task, int index) {
> - return task.getRegister(IA32Registers.DEBUG_REGS_GROUP.getRegisters()[index]);
> - }
> + public Watchpoint readWatchpoint(Task task, int index) {
> + // Get Address from watchpoint register
> + long address = task.getRegister(
> + IA32Registers.DEBUG_REGS_GROUP.getRegisters()[index]);
> +
> + // Get debug status register for all other values
> + long debugStatus = task.getRegister(IA32Registers.DEBUG_CONTROL);
>
> + boolean writeOnly = false;
> +
> + // To find write/read, or read only the bit setting is 0 + no of
> + // register to check * 4 bits (each register has four bits assigned
> + // for r/w and global/local bits.
> + int typeOfWpTrap = 16 + (index *4);
> +
> + // if 1 and 0 set, must be writeOnly.
> + if ( (testBit(debugStatus,typeOfWpTrap)) && (!testBit(debugStatus,typeOfWpTrap+1)))
> + writeOnly = true;
> +
> + // Move over +2 bits for length
> + int lengthOfWP = typeOfWpTrap + 2;
> + int length = 0;
> +
> + // Test length on combination of bits. 00 = 1, 01 = 2
> + // 11 = 4.
> + if (!testBit(debugStatus,lengthOfWP))
> + if (!testBit(debugStatus,lengthOfWP+1))
> + length = 1;
> + else
> + length = 2;
> + else
> + if (testBit(debugStatus, lengthOfWP))
> + length = 4;
> + return Watchpoint.create(address, length, index, writeOnly);
> + }
> +
> +
> /**
> * Deletes a watchpoint. Takes a task, and an index.
> *
> @@ -219,6 +246,10 @@ class IA32WatchpointFunctions extends WatchpointFunctions {
> public long readControlRegister(Task task) {
> return task.getRegister(IA32Registers.DEBUG_CONTROL);
> }
> +
> + private boolean testBit(long register, int bitToTest) {
> + return (register & (1L << bitToTest)) != 0;
> + }
>
> }
>
> diff --git a/frysk-core/frysk/isa/watchpoints/TestWatchpoint.java b/frysk-core/frysk/isa/watchpoints/TestWatchpoint.java
> index 49e5ceb..db52a10 100644
> --- a/frysk-core/frysk/isa/watchpoints/TestWatchpoint.java
> +++ b/frysk-core/frysk/isa/watchpoints/TestWatchpoint.java
> @@ -40,6 +40,17 @@
>
> package frysk.isa.watchpoints;
>
> +import java.util.ArrayList;
> +import java.util.Iterator;
> +
> +import lib.dwfl.Dwfl;
> +import lib.dwfl.DwflModule;
> +import lib.dwfl.ElfSymbolBinding;
> +import lib.dwfl.ElfSymbolType;
> +import lib.dwfl.ElfSymbolVisibility;
> +import lib.dwfl.SymbolBuilder;
> +import frysk.config.Config;
> +import frysk.dwfl.DwflCache;
> import frysk.proc.Proc;
> import frysk.proc.Task;
> import frysk.testbed.DaemonBlockedAtEntry;
> @@ -56,17 +67,17 @@ public class TestWatchpoint extends TestLib {
> return;
> Proc proc = giveMeABlockedProc();
> Task task = proc.getMainTask();
> - long address = 0x1000;
> + long address = getGlobalSymbolAddress(task,"source");
> long debugControlRegister;
> WatchpointFunctions wp = WatchpointFunctionFactory.getWatchpoint(task.getISA());
> long savedDebugControlRegister = wp.readControlRegister(task);
> for (int i=0; i<4; i++) {
>
>
> - wp.setWatchpoint(task, i, address, 4, false, false);
> + wp.setWatchpoint(task, i, address, 4, false);
> // simple first test. Does register contain address?
> assertEquals("Saved watchpoint and address are similar",
> - address, wp.readWatchpoint(task, i));
> + address, wp.readWatchpoint(task, i).getAddress());
>
> debugControlRegister = wp.readControlRegister(task);
>
> @@ -92,7 +103,7 @@ public class TestWatchpoint extends TestLib {
> for (int j=0; j < 4; j++) {
> wp.deleteWatchpoint(task,j);
> assertEquals("Deleted Watchpoint is 0",
> - wp.readWatchpoint(task, j),0);
> + wp.readWatchpoint(task, j).getAddress(),0);
> }
>
> assertEquals("Debug control register is left as we originally found it",
> @@ -109,17 +120,17 @@ public class TestWatchpoint extends TestLib {
> return;
> Proc proc = giveMeABlockedProc();
> Task task = proc.getMainTask();
> - long address = 0x1000;
> + long address = getGlobalSymbolAddress(task,"source");
> long debugControlRegister;
> WatchpointFunctions wp = WatchpointFunctionFactory.getWatchpoint(task.getISA());
> long savedDebugControlRegister = wp.readControlRegister(task);
> for (int i=0; i<4; i++) {
>
>
> - wp.setWatchpoint(task, i, address, 1, true, false);
> + wp.setWatchpoint(task, i, address, 1, true);
> // simple first test. Does register contain address?
> assertEquals("Saved watchpoint and address are similar",
> - address, wp.readWatchpoint(task, i));
> + address, wp.readWatchpoint(task, i).getAddress());
>
> debugControlRegister = wp.readControlRegister(task);
>
> @@ -145,7 +156,7 @@ public class TestWatchpoint extends TestLib {
> for (int j=0; j < 4; j++) {
> wp.deleteWatchpoint(task,j);
> assertEquals("Deleted Watchpoint is 0",
> - wp.readWatchpoint(task, j),0);
> + wp.readWatchpoint(task, j).getAddress(),0);
> }
>
> assertEquals("Debug control register is left as we originally found it",
> @@ -164,25 +175,25 @@ public class TestWatchpoint extends TestLib {
> return;
> Proc proc = giveMeABlockedProc();
> Task task = proc.getMainTask();
> - long address = 0x0;
> + long address = getGlobalSymbolAddress(task,"source");
> long debugControlRegister;
> WatchpointFunctions wp = WatchpointFunctionFactory.getWatchpoint(task.getISA());
>
> long savedDebugControlRegister = wp.readControlRegister(task);
>
> for (int i=0; i<4; i++) {
> - wp.setWatchpoint(task, i, address, 1, false, true);
> + wp.setWatchpoint(task, i, address, 1, false);
>
> // simple first test. Does register contain address?
> assertEquals("Saved watchpoint and address are similar",
> - address, wp.readWatchpoint(task, i));
> + address, wp.readWatchpoint(task, i).getAddress());
>
> debugControlRegister = wp.readControlRegister(task);
>
> - // Test Debug Control Register Bit Pattern. Local Exact.
> - assertEquals(i + " wp local exact bit", true,
> + // Test Debug Control Register Bit Pattern. Global Exact.
> + assertEquals(i + " wp local exact bit", false,
> testBit(debugControlRegister, 0 + (i*2)));
> - assertEquals(i + " wp global exact bit", false,
> + assertEquals(i + " wp global exact bit", true,
> testBit(debugControlRegister, 1 + (i*2)));
>
> // Test Debug Control Register. Fire on Read and Write
> @@ -201,7 +212,7 @@ public class TestWatchpoint extends TestLib {
> for (int j=0; j < 4; j++) {
> wp.deleteWatchpoint(task,j);
> assertEquals("Deleted Watchpoint is 0",
> - wp.readWatchpoint(task, j),0);
> + wp.readWatchpoint(task, j).getAddress(),0);
> }
>
> assertEquals("Debug control register is left as we originally found it",
> @@ -209,17 +220,103 @@ public class TestWatchpoint extends TestLib {
>
> }
>
> + public void testGetAllWatchpoints () {
> + // Set maximum number of watchpoints, then test them
> + // via getAll.
> + if (unresolvedOnPPC(5991))
> + return;
> + int lengthSet[] = {1,1,2,4};
> + int count = 0;
> + Proc proc = giveMeABlockedProc();
> + Task task = proc.getMainTask();
> + long address = getGlobalSymbolAddress(task,"source");
> + WatchpointFunctions wp = WatchpointFunctionFactory.getWatchpoint(task.getISA());
> + for (int i=0; i<wp.getWatchpointCount(); i++)
> + wp.setWatchpoint(task, i, address, lengthSet[i], true);
> +
> + ArrayList watchpointSet = (ArrayList) wp.getAllWatchpoints(task);
> + Iterator i = watchpointSet.iterator();
> + while (i.hasNext()) {
> + Watchpoint watchpoint = ((Watchpoint) i.next());
> + assertNotNull("Check retrieved watchpoint is not null", watchpoint);
> + assertEquals("address = source var address for Watchpoint " + i,
> + address,watchpoint.getAddress());
> + assertEquals("length = " + lengthSet[count], lengthSet[count],
> + watchpoint.getRange());
> + assertEquals("register allocation = " + count, count,
> + watchpoint.getRegister());
> + assertEquals("writeOnly ", true,
> + watchpoint.isWriteOnly());
> + count++;
> + }
> +
> + assertEquals("Returned count is correct", wp.getWatchpointCount(),count);
> + }
> +
> +
> private boolean testBit(long register, int bitToTest) {
> return (register & (1L << bitToTest)) != 0;
> }
>
> - private Proc giveMeABlockedProc ()
> - {
> - String[] nocmds = {};
> - DaemonBlockedAtEntry ackProc = new DaemonBlockedAtEntry(nocmds);
> + private Proc giveMeABlockedProc () {
> + DaemonBlockedAtEntry ackProc = new DaemonBlockedAtEntry(
> + Config.getPkgLibFile("funit-watchpoint"));
> assertNotNull(ackProc);
> return ackProc.getMainTask().getProc();
> }
>
> + /**
> + * Returns the address of a global label by quering the the Proc
> + * main Task's Dwlf.
> + */
> + long getGlobalSymbolAddress(Task task, 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;
> + }
> +
> + public void symbol(String name, long value, long size, ElfSymbolType type,
> + ElfSymbolBinding bind, ElfSymbolVisibility visibility) {
> + if (name.equals(this.name)) {
> + this.address = value;
> + this.found = true;
> + }
> +
> + }
> + }
> }
>
> diff --git a/frysk-core/frysk/symtab/Symbol.java b/frysk-core/frysk/isa/watchpoints/Watchpoint.java
> similarity index 53%
> copy from frysk-core/frysk/symtab/Symbol.java
> copy to frysk-core/frysk/isa/watchpoints/Watchpoint.java
> index fae9612..ef88924 100644
> --- a/frysk-core/frysk/symtab/Symbol.java
> +++ b/frysk-core/frysk/isa/watchpoints/Watchpoint.java
> @@ -1,6 +1,9 @@
> -// This file is part of the program FRYSK.
> +
> +
> +package frysk.isa.watchpoints;
> +//This file is part of the program FRYSK.
> //
> -// Copyright 2007, 2008, Red Hat Inc.
> +// 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
> @@ -36,71 +39,58 @@
> // modification, you must delete this exception statement from your
> // version and license this file solely under the GPL without
> // exception.
> -
> -package frysk.symtab;
> -
> -import lib.stdcpp.Demangler;
> -
> -/**
> - * The object-file symbol. Typically obtained by reading ELF
> - * information.
> - *
> - * Do not confuse this with higher-level symbolic information, such as
> - * function names, obtained from debug information such as DWARF.
> - */
> -
> -public class Symbol {
> +public class Watchpoint {
>
> private final long address;
> - private final long size;
> - private final String name;
> - protected String demangledName = null;
> + private final int range;
> + private final int register;
> + private final boolean writeOnly;
>
> - // package private constructor.
> - Symbol(long address, long size, String name) {
> + private Watchpoint(long address, int range, int register, boolean writeOnly) {
> this.address = address;
> - this.size = size;
> - this.name = name;
> + this.range = range;
> + this.register = register;
> + this.writeOnly = writeOnly;
> }
>
> /**
> - * Return the address of the symbol.
> + * Create
> + *
> + * Watchpoint. This is an immutable class that carries only information.
> + * It is not connected with the underlying hardware, and there is no
> + * guarantee that the information contained in this class is current,
> + * or even exists at any given time.
> + *
> + * The watchpoint manager can, and will, optimize watchpoint allocation
> + * to maximize use; and it can, and will, sometimes combine or split
> + * hardware watchpoints. Thus this class is immutable, and cannot be
> + * changed after instantiation. If you want to alter a watchpoint, you should
> + * apply it via the WatchpointFunctionFactory, WatchpointFunction classes and
> + * their subclasses, and a new watchpoint object will be generated.
> + *
> + * Clients should not instantiate this class directly.
> + *
> + * @param address - address of watchpoint.
> + * @param range - range of the watchpoint.
> + * @param register - register watchpoint was allocated.
> + * @param writeOnly - true if the watchpoint will only trigger on a write.
> */
> - public long getAddress () {
> - return address;
> + public static Watchpoint create(long address, int range, int register, boolean writeOnly) {
> + return new Watchpoint(address, range, register, writeOnly);
> }
> -
> - /**
> - * Return the size of the symbol (possibly zero).
> - */
> - public long getSize () {
> - return size;
> + public int getRegister() {
> + return register;
> }
>
> - /**
> - * Return the mangled name (the raw string found in the symbol
> - * table). Or NULL, of the name is unknown.
> - */
> - public String getName () {
> - return name;
> + public long getAddress() {
> + return address;
> }
>
> - /**
> - * Return the demangled name, or "" of the name isn't known.
> - *
> - * XXX: Is returning "" better than null? Sounds like a cheat for
> - * code that should be conditional on the symbol being known.
> - */
> - public String getDemangledName () {
> - if (demangledName == null)
> - demangledName = Demangler.demangle (getName());
> - return demangledName;
> + public int getRange() {
> + return range;
> }
>
> - /**
> - * Dump the symbol's contents.
> - */
> - public String toString () {
> - return name + "@" + Long.toHexString (address) + ":" + size;
> - }
> + public boolean isWriteOnly() {
> + return writeOnly;
> + }
> }
> diff --git a/frysk-core/frysk/isa/watchpoints/WatchpointFunctions.java b/frysk-core/frysk/isa/watchpoints/WatchpointFunctions.java
> index 30e23fc..a5a30a9 100644
> --- a/frysk-core/frysk/isa/watchpoints/WatchpointFunctions.java
> +++ b/frysk-core/frysk/isa/watchpoints/WatchpointFunctions.java
> @@ -40,6 +40,9 @@
>
>
> hooks/post-receive
> --
> frysk system monitor/debugger
>
parent reply other threads:[~2008-04-03 13:07 UTC|newest]
Thread overview: expand[flat|nested] mbox.gz Atom feed
[parent not found: <20080403130002.26637.qmail@sourceware.org>]
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=47F4D665.7040101@redhat.com \
--to=pmuldoon@redhat.com \
--cc=frysk@sourceware.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
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).