Quick pc hack for exe loading

Quick pc hack for exe loading

[Mark Wielaard]

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

Hi Rick,

I wanted to test some libunwind lookup changes I made locally and the
exe target is nice for that since it is so simple. The attached patch
sets up the PC value and makes it possible to see where the exe would
start:

(fhpd) load /bin/bash
Loaded executable file: /bin/bash
(fhpd) where
[0.0]
DebugInfoStackFactory.printStackTrace() numberOfFrames 0
#0 0x0000000000419150 in _start () from /bin/bash

It still cannot actually do the other way around though:

(fhpd) print _start
[0.0]
Error: Object _start was not found

Haven't looked into why that was since I was only interested in the
stack thing.

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

        * LinuxExeTask.java (bankBuffers): New final field.
        (LinuxExeTask): Setup bankBuffers and explicitly set pc.
        (sendrecRegisterBanks): Use cached bankBuffers.

It is a bit of a hack (as the comment explains), but it seems to work
great and all tests still pass with this applied. Feel free to rip it
out and completely replace with a real, less hacky, solution though.

Cheers,

Mark

[-- Attachment #2: exe-pc.patch --]
[-- Type: text/x-patch, Size: 1541 bytes --]

diff --git a/frysk-core/frysk/proc/dead/LinuxExeTask.java b/frysk-core/frysk/proc/dead/LinuxExeTask.java
index 3eb584c..1302be5 100644
--- a/frysk-core/frysk/proc/dead/LinuxExeTask.java
+++ b/frysk-core/frysk/proc/dead/LinuxExeTask.java
@@ -47,17 +47,44 @@ import frysk.proc.TaskId;
 import frysk.proc.TaskState;
 import frysk.isa.ISA;
 
+import lib.dwfl.*;
 
 public class LinuxExeTask extends DeadTask
 {
 
   LinuxExeProc proc = null;
   TaskId id = null;
+
+  // Holds all the register values, setup once in the constructor.
+  private final ByteBuffer[] bankBuffers;
   
   protected LinuxExeTask(LinuxExeProc proc, TaskId id, TaskState state) {
       super(proc, id, state);
       this.proc = proc;
       this.id = id;
+      this.bankBuffers = sendrecRegisterBuffersFIXME();
+
+      // Fake PC.  XXX should be done in Proc instead of creating Elf
+      // object in the Task itself.
+      long pc;
+      Elf e = null;
+      try
+	{
+	  e = new Elf(getProc().getExe(), ElfCommand.ELF_C_READ);
+	  ElfEHeader h = e.getEHeader();
+	  pc = h.entry;
+	}
+      catch (ElfException ee)
+	{
+	  // Nice try, just give up.
+	  pc = 0;
+	}
+      finally
+	{
+	  if (e != null)
+	    e.close();
+	}
+      getIsa().setPC(this, pc);
   }
   
   protected ISA sendrecISA() {
@@ -93,6 +120,6 @@ public class LinuxExeTask extends DeadTask
 
   protected RegisterBanks sendrecRegisterBanks() {
       return CorefileRegisterBanksFactory.create
-      	  (getISA(), sendrecRegisterBuffersFIXME());
+      	  (getISA(), bankBuffers);
   }
 }

Re: Quick pc hack for exe loading

[Rick Moseley]

Hi Mark,

Cool. That ought to be fine.

Rick

Mark Wielaard wrote:
> Hi Rick,
>
> I wanted to test some libunwind lookup changes I made locally and the
> exe target is nice for that since it is so simple. The attached patch
> sets up the PC value and makes it possible to see where the exe would
> start:
>
> (fhpd) load /bin/bash
> Loaded executable file: /bin/bash
> (fhpd) where
> [0.0]
> DebugInfoStackFactory.printStackTrace() numberOfFrames 0
> #0 0x0000000000419150 in _start () from /bin/bash
>
> It still cannot actually do the other way around though:
>
> (fhpd) print _start
> [0.0]
> Error: Object _start was not found
>
> Haven't looked into why that was since I was only interested in the
> stack thing.
>
> 2007-11-23  Mark Wielaard  <mwielaard@redhat.com>
>
>         * LinuxExeTask.java (bankBuffers): New final field.
>         (LinuxExeTask): Setup bankBuffers and explicitly set pc.
>         (sendrecRegisterBanks): Use cached bankBuffers.
>
> It is a bit of a hack (as the comment explains), but it seems to work
> great and all tests still pass with this applied. Feel free to rip it
> out and completely replace with a real, less hacky, solution though.
>
> Cheers,
>
> Mark
>   

Re: Quick pc hack for exe loading

[Andrew Cagney]

Rick Moseley wrote:
> Hi Mark,
>
> Cool. That ought to be fine.
>

Yea, except this bit:

+      this.bankBuffers = sendrecRegisterBuffersFIXME();

the word "FIXME" should be a hint.

Andrew



> Rick
>
> Mark Wielaard wrote:
>> Hi Rick,
>>
>> I wanted to test some libunwind lookup changes I made locally and the
>> exe target is nice for that since it is so simple. The attached patch
>> sets up the PC value and makes it possible to see where the exe would
>> start:
>>
>> (fhpd) load /bin/bash
>> Loaded executable file: /bin/bash
>> (fhpd) where
>> [0.0]
>> DebugInfoStackFactory.printStackTrace() numberOfFrames 0
>> #0 0x0000000000419150 in _start () from /bin/bash
>>
>> It still cannot actually do the other way around though:
>>
>> (fhpd) print _start
>> [0.0]
>> Error: Object _start was not found
>>
>> Haven't looked into why that was since I was only interested in the
>> stack thing.
>>
>> 2007-11-23  Mark Wielaard  <mwielaard@redhat.com>
>>
>>         * LinuxExeTask.java (bankBuffers): New final field.
>>         (LinuxExeTask): Setup bankBuffers and explicitly set pc.
>>         (sendrecRegisterBanks): Use cached bankBuffers.
>>
>> It is a bit of a hack (as the comment explains), but it seems to work
>> great and all tests still pass with this applied. Feel free to rip it
>> out and completely replace with a real, less hacky, solution though.
>>
>> Cheers,
>>
>> Mark
>>   
>

Re: Quick pc hack for exe loading

[Mark Wielaard]

Hi Andrew,

On Fri, 2007-11-23 at 12:30 -0500, Andrew Cagney wrote:
> Rick Moseley wrote:
> > Hi Mark,
> >
> > Cool. That ought to be fine.
> >
> 
> Yea, except this bit:
> 
> +      this.bankBuffers = sendrecRegisterBuffersFIXME();
> 
> the word "FIXME" should be a hint.

Indeed, that was moved from the pre-cached function
sendrecRegisterBanks() to the cache in the constructor and also why I
added the XXX and comment. I assume this is because you are in the
middle of moving Isa and register related methods to their own package?
Or is that work finished now and these remaining FIXME() methods just
need to be cleaned up now?

Cheers,

Mark

Re: Quick pc hack for exe loading

[Andrew Cagney]

Mark Wielaard wrote:
> Hi Andrew,
>
> On Fri, 2007-11-23 at 12:30 -0500, Andrew Cagney wrote:
>   
>> Rick Moseley wrote:
>>     
>>> Hi Mark,
>>>
>>> Cool. That ought to be fine.
>>>
>>>       
>> Yea, except this bit:
>>
>> +      this.bankBuffers = sendrecRegisterBuffersFIXME();
>>
>> the word "FIXME" should be a hint.
>>     
>
> Indeed, that was moved from the pre-cached function
> sendrecRegisterBanks() to the cache in the constructor and also why I
> added the XXX and comment. I assume this is because you are in the
> middle of moving Isa and register related methods to their own package?
> Or is that work finished now and these remaining FIXME() methods just
> need to be cleaned up now?
>
>   
The XXX comment:

+      // Fake PC.  XXX should be done in Proc instead of creating Elf
+      // object in the Task itself.

seems unrelated.

The comment for getRegisterBuffers notes:

  protected abstract ByteBuffer[] sendrecRegisterBuffersFIXME();
  /**
   * Return the machine's register banks as an array.
   *
   * XXX: This is being replaced by "getRegisterBanks()" that returns
   * a class that abstracts the ByteArray[] + BankRegister
   * combination.
   */
  public ByteBuffer[] getRegisterBuffersFIXME ()

what prevents the use of getRegisterBanks()?

Andrew

> Cheers,
>
> Mark
>
>   

Re: Quick pc hack for exe loading

[Mark Wielaard]

Hi Andrew,

On Mon, 2007-11-26 at 11:52 -0500, Andrew Cagney wrote:
> The comment for getRegisterBuffers notes:
> 
>   protected abstract ByteBuffer[] sendrecRegisterBuffersFIXME();
>   /**
>    * Return the machine's register banks as an array.
>    *
>    * XXX: This is being replaced by "getRegisterBanks()" that returns
>    * a class that abstracts the ByteArray[] + BankRegister
>    * combination.
>    */
>   public ByteBuffer[] getRegisterBuffersFIXME ()
> 
> what prevents the use of getRegisterBanks()?

Right that is the question. Since you introduced the FIXMEs and are
working on this refactoring I assumed that you would fix it when you
were ready and nothing prevented it anymore. Which is the reason I
didn't change it myself.

Cheers,

Mark

Re: Quick pc hack for exe loading

[Andrew Cagney]

Mark Wielaard wrote:
>
>
> Right that is the question. Since you introduced the FIXMEs and are
> working on this refactoring I assumed that you would fix it when you
> were ready and nothing prevented it anymore. Which is the reason I
> didn't change it myself.
>   

I'm more than happy to fix the existing; however, this is new.

Andrew

Re: Quick pc hack for exe loading

[Mark Wielaard]

Hi Andrew,

On Tue, 2007-11-27 at 09:40 -0500, Andrew Cagney wrote:
> Mark Wielaard wrote:
> > Right that is the question. Since you introduced the FIXMEs and are
> > working on this refactoring I assumed that you would fix it when you
> > were ready and nothing prevented it anymore. Which is the reason I
> > didn't change it myself.
>
> I'm more than happy to fix the existing; however, this is new.

OK, I am not completely sure what you mean by that. But if you feel that
your fixes are hampered by my patch please feel free to revert it and
let me or Rick know when you are finished so the functionality can be
added correctly later.

Cheers,

Mark