This is the mail archive of the
frysk@sources.redhat.com
mailing list for the frysk project.
Re: cvs head has lots of test failures
-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1
Wu Zhou wrote:
> Quoting Tim Moore <timoore@redhat.com>:
>> The problem is that the file descriptor passed to libelf is *never*
>> closed, whether or not the Elf Java object is garbage collected. I've
>> added an explicit close() method for the Elf object so that getIsa() can
>> release the file descriptor right away, and of course added the code to
>> close the file descriptor. frysk-core TestRunner tests run now, but I'm
>> going to sleep in it before checking in my code.
>>
>
> That is just what I thought of. Can you post your code here?
Attached, to be checked in soon.
>
> BTW. This make me think of another checkin policy: before checking in
> code, we can post the diff on the mail-list, if nobody reject in a
> specified time (one day, two days, or longer), it can get in. Another
> option is for some people to review the code, but that will take some
> more time.
A code review before radical patches is good, but it's really more
important to get the tests in shape so that lazy developers, like
myself, will run them.
>> Mea culpa for not running the tests more carefully, though in my defense
>> the tests were broken at the time on x8664 :)
>
> I guess Mea culpa is French's saying for saying sorry. If it is, we
> need say sorry here too. :-) These getIsa() code is from us anyway.
>
Actually, it's Latin :) This wasn't the fault of getIsa(); the code was
doing something quite reasonable and exposed a bug elsewhere.
Tim
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.4 (GNU/Linux)
Comment: Using GnuPG with Fedora - http://enigmail.mozdev.org
iD8DBQFE3CG2eDhWHdXrDRURAm9uAJ9+YfrwX+so8llOqfACwsKFZw5H6wCfUYNh
wP6LI8BoVV4EOnhFLzoTRdc=
=RpmW
-----END PGP SIGNATURE-----
Index: frysk-core/frysk/proc/IsaFactory.java
===================================================================
RCS file: /cvs/frysk/frysk-core/frysk/proc/IsaFactory.java,v
retrieving revision 1.2
diff -d -u -r1.2 IsaFactory.java
--- frysk-core/frysk/proc/IsaFactory.java 7 Aug 2006 20:22:15 -0000 1.2
+++ frysk-core/frysk/proc/IsaFactory.java 11 Aug 2006 06:10:39 -0000
@@ -83,21 +83,28 @@
{
throw new TaskException("getting task's executable", e);
}
+ try
+ {
+
+ ElfEHeader header = elfFile.getEHeader();
- ElfEHeader header = elfFile.getEHeader();
-
- switch (header.machine)
+ switch (header.machine)
+ {
+ case ElfEMachine.EM_386:
+ return LinuxIa32.isaSingleton ();
+ case ElfEMachine.EM_PPC:
+ return LinuxPPC.isaSingleton ();
+ case ElfEMachine.EM_PPC64:
+ return LinuxPPC64.isaSingleton ();
+ case ElfEMachine.EM_X86_64:
+ return LinuxEMT64.isaSingleton ();
+ default:
+ throw new TaskException("Unknown machine type " + header.machine);
+ }
+ }
+ finally
{
- case ElfEMachine.EM_386:
- return LinuxIa32.isaSingleton ();
- case ElfEMachine.EM_PPC:
- return LinuxPPC.isaSingleton ();
- case ElfEMachine.EM_PPC64:
- return LinuxPPC64.isaSingleton ();
- case ElfEMachine.EM_X86_64:
- return LinuxEMT64.isaSingleton ();
- default:
- throw new TaskException("Unknown machine type " + header.machine);
+ elfFile.close();
}
}
Index: frysk-core/frysk/proc/LinuxTask.java
===================================================================
RCS file: /cvs/frysk/frysk-core/frysk/proc/LinuxTask.java,v
retrieving revision 1.34
diff -d -u -r1.34 LinuxTask.java
--- frysk-core/frysk/proc/LinuxTask.java 10 Aug 2006 19:27:21 -0000 1.34
+++ frysk-core/frysk/proc/LinuxTask.java 11 Aug 2006 06:10:40 -0000
@@ -57,24 +57,25 @@
{
private long ptraceOptions = 0;
- // XXX: For moment wire in standard 32-bit little-endian memory
+ // XXX: For moment wire in standard 32-bit memory
// map. This will be replaced by a memory map created using
// information from /proc/PID/maps.
- private void setupMapsXXX ()
+ private void setupMapsXXX() throws TaskException
{
+ ByteOrder byteOrder = getIsa().getByteOrder();
// XXX: For writing at least, PTRACE must be used as /proc/mem
// cannot be written to.
- memory = new PtraceByteBuffer (id.id, PtraceByteBuffer.Area.DATA,
- 0xffffffffl);
- memory.order (ByteOrder.LITTLE_ENDIAN);
+ memory = new PtraceByteBuffer(id.id, PtraceByteBuffer.Area.DATA,
+ 0xffffffffl);
+ memory.order (byteOrder);
// XXX: For moment wire in a standard 32-bit little-endian
// register set.
registerBank = new ByteBuffer[]
{
- new PtraceByteBuffer (id.id, PtraceByteBuffer.Area.USR)
+ new PtraceByteBuffer(id.id, PtraceByteBuffer.Area.USR)
};
- registerBank[0].order (ByteOrder.LITTLE_ENDIAN);
+ registerBank[0].order(byteOrder);
}
/**
Index: frysk-imports/lib/elf/Elf.java
===================================================================
RCS file: /cvs/frysk/frysk-imports/lib/elf/Elf.java,v
retrieving revision 1.16
diff -d -u -r1.16 Elf.java
--- frysk-imports/lib/elf/Elf.java 7 Aug 2006 20:22:15 -0000 1.16
+++ frysk-imports/lib/elf/Elf.java 11 Aug 2006 06:10:40 -0000
@@ -50,10 +50,12 @@
{
private long pointer;
+ protected int fd; // ecj thinks this isn't used...
public Elf (long ptr)
{
this.pointer = ptr;
+ this.fd = -1;
}
/**
@@ -117,6 +119,15 @@
}
}
+ /**
+ * Destroy the external elf file object associated with this object.
+ */
+ public void close()
+ {
+ elf_end();
+ pointer = 0;
+ }
+
public Elf clone (ElfCommand command)
{
return new Elf(elf_clone(command.getValue()));
Index: frysk-imports/lib/elf/cni/Elf.cxx
===================================================================
RCS file: /cvs/frysk/frysk-imports/lib/elf/cni/Elf.cxx,v
retrieving revision 1.17
diff -d -u -r1.17 Elf.cxx
--- frysk-imports/lib/elf/cni/Elf.cxx 7 Aug 2006 20:22:15 -0000 1.17
+++ frysk-imports/lib/elf/cni/Elf.cxx 11 Aug 2006 06:10:40 -0000
@@ -37,6 +37,7 @@
// version and license this file solely under the GPL without
// exception.
#include <stdlib.h>
+#include <unistd.h>
#include <gelf.h>
#include <gcj/cni.h>
#include <string.h>
@@ -66,7 +67,7 @@
fileName[fileNameLen]='\0';
errno = 0;
- int fd = open (fileName, O_RDONLY);
+ fd = open (fileName, O_RDONLY);
if(errno != 0){
char* message = "Could not open %s for reading";
char error[strlen(fileName) + strlen(message) - 2];
@@ -75,14 +76,17 @@
file);
}
- if(::elf_version(EV_CURRENT) == EV_NONE)
+ if(::elf_version(EV_CURRENT) == EV_NONE) {
+ ::close(fd);
throw new lib::elf::ElfException(JvNewStringUTF("Elf library version out of date"));
-
+ }
errno = 0;
::Elf* new_elf = ::elf_begin (fd, (Elf_Cmd) command, (::Elf*) 0);
- if(errno != 0 || !new_elf)
+ if(errno != 0 || !new_elf) {
+ ::close(fd);
throw new lib::elf::ElfException(JvNewStringUTF("Could not open Elf file"));
+ }
this->pointer = (jlong) new_elf;
}
@@ -110,7 +114,10 @@
jint
lib::elf::Elf::elf_end(){
- return ::elf_end((::Elf*) this->pointer);
+ jint val = ::elf_end((::Elf*) this->pointer);
+ if (fd >= 0)
+ ::close(fd);
+ return val;
}
jlong