From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 30673 invoked by alias); 3 Apr 2008 13:07:11 -0000 Received: (qmail 30031 invoked by uid 22791); 3 Apr 2008 13:07:07 -0000 X-Spam-Status: No, hits=-0.2 required=5.0 tests=AWL,BAYES_00,J_CHICKENPOX_44,J_CHICKENPOX_45,J_CHICKENPOX_47,J_CHICKENPOX_48,J_CHICKENPOX_64,J_CHICKENPOX_66,J_CHICKENPOX_74,SPF_HELO_PASS,SPF_PASS X-Spam-Check-By: sourceware.org Received: from mx1.redhat.com (HELO mx1.redhat.com) (66.187.233.31) by sourceware.org (qpsmtpd/0.31) with ESMTP; Thu, 03 Apr 2008 13:06:50 +0000 Received: from int-mx1.corp.redhat.com (int-mx1.corp.redhat.com [172.16.52.254]) by mx1.redhat.com (8.13.8/8.13.8) with ESMTP id m33D6m9s031782 for ; Thu, 3 Apr 2008 09:06:48 -0400 Received: from pobox-2.corp.redhat.com (pobox-2.corp.redhat.com [10.11.255.15]) by int-mx1.corp.redhat.com (8.13.1/8.13.1) with ESMTP id m33D6mhj008740 for ; Thu, 3 Apr 2008 09:06:48 -0400 Received: from localhost.localdomain (vpn-14-103.rdu.redhat.com [10.11.14.103]) by pobox-2.corp.redhat.com (8.13.1/8.13.1) with ESMTP id m33D6kmQ006972 for ; Thu, 3 Apr 2008 09:06:46 -0400 Message-ID: <47F4D665.7040101@redhat.com> Date: Thu, 03 Apr 2008 13:07:00 -0000 From: Phil Muldoon User-Agent: Thunderbird 2.0.0.12 (X11/20080226) MIME-Version: 1.0 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. References: <20080403130002.26637.qmail@sourceware.org> In-Reply-To: <20080403130002.26637.qmail@sourceware.org> Content-Type: text/plain; charset=ISO-8859-1; format=flowed Content-Transfer-Encoding: 7bit X-Scanned-By: MIMEDefang 2.58 on 172.16.52.254 X-IsSubscribed: yes Mailing-List: contact frysk-help@sourceware.org; run by ezmlm Precedence: bulk List-Id: List-Subscribe: List-Post: List-Help: , Sender: frysk-owner@sourceware.org X-SW-Source: 2008-q2/txt/msg00014.txt.bz2 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 > 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 > > * 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 > + > + * 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 > > * 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.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 >