BookE "branch taken" behavior vis-a-vis updating the NIP register

BookE "branch taken" behavior vis-a-vis updating the NIP register

[pegasus]

Hello.

I was reading the source code for the debug exception under powerpc. I saw
that BookE processors stop before (actually) taking a branch. Hence in order
to force it to take that branch and then stop, the source code for it had to
be "hacked'  to (temporarily) enable single step until the branch
instruction has been taken, thereby mimicing the BookS behavior. 

By doing this, I believe we would want the exception to be triggered after
the branch has been successfully taken. Hence I put a printk to print the
value of the instruction that actually caused the exception. I was assuming
that initially a debug breakpoint would be hit on the branch instruction
(assuming the branch is supposed to be taken). Now since at this point in
time, the branch instruction has NOT finished, the kernel, after merely
disabling the BT bit in DBCR0 and enabling IC bit in DBCR0, returns. At this
point I was assuming we will see another exception at the very same
instruction in userspace. However, when printing the NIP it becomes clear
that when it gets to the debug exception handler after being (temporarily)
set to single step, NIP points to the instruction after the branch
instruction. To me, it appears that, after disabling BT (branch taken) debug
event monitoring (and enabling single stepping), it does not catch an
exception at that very same branch instruction, instead it catches an
exception for the subsequent instruction. Sorry for the repetition but I
wanted to clarify what I am seeing here.

May be this IS the way it is supposed to behave (which means my thinking
about it is flawed). I am a bit confused here. 

Please let me know more..Im keen to hear from you folks.



--
View this message in context: http://linuxppc.10917.n7.nabble.com/BookE-branch-taken-behavior-vis-a-vis-updating-the-NIP-register-tp77960.html
Sent from the linuxppc-dev mailing list archive at Nabble.com.

Re: BookE "branch taken" behavior vis-a-vis updating the NIP register

[James Yang]

On Fri, 8 Nov 2013, pegasus wrote:

> Hello.
> 
> I was reading the source code for the debug exception under powerpc. I saw
> that BookE processors stop before (actually) taking a branch. Hence in order
> to force it to take that branch and then stop, the source code for it had to
> be "hacked'  to (temporarily) enable single step until the branch
> instruction has been taken, thereby mimicing the BookS behavior. 

Have a look at this thread:

https://lists.ozlabs.org/pipermail/linuxppc-dev/2013-July/108618.html


> By doing this, I believe we would want the exception to be triggered after
> the branch has been successfully taken. Hence I put a printk to print the
> value of the instruction that actually caused the exception. I was assuming
> that initially a debug breakpoint would be hit on the branch instruction
> (assuming the branch is supposed to be taken). Now since at this point in
> time, the branch instruction has NOT finished, the kernel, after merely
> disabling the BT bit in DBCR0 and enabling IC bit in DBCR0, returns. At this
> point I was assuming we will see another exception at the very same
> instruction in userspace. However, when printing the NIP it becomes clear
> that when it gets to the debug exception handler after being (temporarily)
> set to single step, NIP points to the instruction after the branch
> instruction. To me, it appears that, after disabling BT (branch taken) debug
> event monitoring (and enabling single stepping), it does not catch an
> exception at that very same branch instruction, instead it catches an
> exception for the subsequent instruction. Sorry for the repetition but I
> wanted to clarify what I am seeing here.
>
> May be this IS the way it is supposed to behave (which means my thinking
> about it is flawed). I am a bit confused here. 

You have it correct, that is the behavior of what is there.  The 
Server branch execute debug exception occurs after the branch 
completes, and the SRR0 (the value in PT_NIP) points to the address of 
the instruction that is to be executed after the branch.  BookE branch 
exception occurs before the branch is executed, but only if the branch 
will be taken.  The hack tries to make BookE return an exception to 
the process with the same timing as Server: exception signal sent to 
process after the branch completes.

I think some of the confusion is due to the existence of the hack.  
Its existence implies that BookE can perfectly emulate Server, but 
this is not possible because BookE would only take the exception if 
the branch's condition would cause the branch to be actually taken 
(that the branch condition, if present, is true). Therefore, even with 
this hack, untaken branches are not signaled at all on BookE while 
they would on Server.

Note that PTRACE_SINGLESTEP works the same on both Server and BookE. 
The exception occurs after an instruction completes, and the SRR0 / 
NIP points to the instruction that will be executed next.

Re: BookE "branch taken" behavior vis-a-vis updating the NIP register

[pegasus]

Thanks James.

So it means that, the NIP that I am seeing in the printks is indeed correct.
Right? And this is due to the pipelining feature that is inherent in all
processors. So it basically means that after having executed the branch, the
NIP has already progressed to the next instruction it is about to execute
(just after the branch or rather the branch target). The NIP hence basically
contains the address of the branch target. And this is how it is (and
perhaps I have been thinking about this in the wrong way).

But I still have a question about how one would then be able to signal to
the userspace who might be interpreting this information differently? I mean
if SRR0 contains, not the branch instruction address but the address of the
branch target, how would any debugger be able to catch function calls? May
be there is a trick involved here and hence gdb or for that matter the other
debuggers are still in the market. But then I would be immensely obliged if
you could shed some light on how is this accomplished. Lets say I am waiting
at the userspace in my own sigtrap, to watch out for branch instructions.
Lets say I want to profile my code to get to know how many branch
instructions it has generated. How could I ever do that using my own custom
SIGTRAP handler?

Coming on to PTRACE_SINGLESTEP, the sysroot that has been provided to us by
our vendor does not include a PTRACE_SINGLEBLOCK in sys/ptrace.h:

/* Type of the REQUEST argument to `ptrace.'  */
enum __ptrace_request
{
  /* Indicate that the process making this request should be traced.
     All signals received by this process can be intercepted by its
     parent, and its parent can use the other `ptrace' requests.  */
  PTRACE_TRACEME = 0,
#define PT_TRACE_ME PTRACE_TRACEME

  /* Return the word in the process's text space at address ADDR.  */
  PTRACE_PEEKTEXT = 1,
#define PT_READ_I PTRACE_PEEKTEXT

  /* Return the word in the process's data space at address ADDR.  */
  PTRACE_PEEKDATA = 2,
#define PT_READ_D PTRACE_PEEKDATA

  /* Return the word in the process's user area at offset ADDR.  */
  PTRACE_PEEKUSER = 3,
#define PT_READ_U PTRACE_PEEKUSER

  /* Write the word DATA into the process's text space at address ADDR.  */
  PTRACE_POKETEXT = 4,
#define PT_WRITE_I PTRACE_POKETEXT

  /* Write the word DATA into the process's data space at address ADDR.  */
  PTRACE_POKEDATA = 5,
#define PT_WRITE_D PTRACE_POKEDATA

  /* Write the word DATA into the process's user area at offset ADDR.  */
  PTRACE_POKEUSER = 6,
#define PT_WRITE_U PTRACE_POKEUSER

  /* Continue the process.  */
  PTRACE_CONT = 7,
#define PT_CONTINUE PTRACE_CONT

  /* Kill the process.  */
  PTRACE_KILL = 8,
#define PT_KILL PTRACE_KILL

  /* Single step the process.
     This is not supported on all machines.  */
  PTRACE_SINGLESTEP = 9,
#define PT_STEP PTRACE_SINGLESTEP

  /* Attach to a process that is already running. */
  PTRACE_ATTACH = 16,
#define PT_ATTACH PTRACE_ATTACH

  /* Detach from a process attached to with PTRACE_ATTACH.  */
  PTRACE_DETACH = 17,
#define PT_DETACH PTRACE_DETACH

  /* Continue and stop at the next (return from) syscall.  */
  PTRACE_SYSCALL = 24,
#define PT_SYSCALL PTRACE_SYSCALL

  /* Set ptrace filter options.  */
  PTRACE_SETOPTIONS = 0x4200,
#define PT_SETOPTIONS PTRACE_SETOPTIONS

  /* Get last ptrace message.  */
  PTRACE_GETEVENTMSG = 0x4201,
#define PT_GETEVENTMSG PTRACE_GETEVENTMSG

  /* Get siginfo for process.  */
  PTRACE_GETSIGINFO = 0x4202,
#define PT_GETSIGINFO PTRACE_GETSIGINFO

  /* Set new siginfo for process.  */
  PTRACE_SETSIGINFO = 0x4203
#define PT_SETSIGINFO PTRACE_SETSIGINFO
};

Although I can clearly see that PTRACE_SINGLEBLOCK is supported in the
kernel. 

Hence I am not able to compile this simple program in userspace:

#include <stdio.h>
#include <sys/ptrace.h>
#include <sys/types.h>
#include <sys/wait.h>
#include <unistd.h>
#include <errno.h>
#include <stdlib.h>
#include <string.h>


int main() {   
	pid_t childpid;

    childpid = fork();

	if(childpid < 0) {
		perror("error forking:");
		return -1;
	}

    if(childpid == 0) {
		char cwd[512];
		char path[1024];
		int val;
		
        ptrace(PTRACE_TRACEME, 0, NULL, NULL);

		if (getcwd(cwd, sizeof(cwd)) != NULL)
			snprintf(path, sizeof(path), "%s/test", cwd);
		else {
			perror("getcwd() error. Exiting the program");
			exit(1);
		}

		puts("now execle'ing");
		val = execl(path, NULL); //this will not return
		if (val < 0) { //something went wrong
			perror("something went wrong");
			exit(1);
		}		
    }
    else {
		int status,rc;

		ptrace(PTRACE_SINGLEBLOCK, childpid, NULL, NULL);
		rc = waitpid(childpid, &status, 0);
		if (rc == -1) {
			perror("unable to waitpid:");
			exit(1);
		}
		
		if(WIFEXITED(status)) {
			printf("Child exited with status %d.\n", WEXITSTATUS(status));
		}
    }
    return 0;
}

Heres the error I get:
testptrace.c: In function 'int main()':
testptrace.c:47: error: invalid conversion from 'int' to '__ptrace_request'
testptrace.c:47: error:   initializing argument 1 of 'long int
ptrace(__ptrace_request, ...)'
make: *** [testptrace] Error 1

How should I go about using ptrace to test this? 

Keen to hear from you for the above two clarifications.



--
View this message in context: http://linuxppc.10917.n7.nabble.com/BookE-branch-taken-behavior-vis-a-vis-updating-the-NIP-register-tp77960p78004.html
Sent from the linuxppc-dev mailing list archive at Nabble.com.

Re: BookE "branch taken" behavior vis-a-vis updating the NIP register

[James Yang]

On Mon, 11 Nov 2013, pegasus wrote:

> And this is due to the pipelining feature that is inherent in all 
> processors.

No, it has nothing to do with pipelining at all.  It is just the 
convention that IBM defined as to how the branch taken exception works 
in BookE Power ISA.  The pipeline behavior is not visible from the 
architected state.

 
> But I still have a question about how one would then be able to 
> signal to the userspace who might be interpreting this information 
> differently? I mean if SRR0 contains, not the branch instruction 
> address but the address of the branch target, how would any debugger 
> be able to catch function calls?

On Server and the BookE-mimicking-Server, PT_NIP would point to the 
start of the function you are calling.  However, you do not get the 
callsite address unless you know you stopped after a function-calling 
branch-and-link executed, single step again and read the LR and 
subtract 4.  If you don't know if the branch that caused the exception 
was actually a branch-and-link, you don't know if you are at the 
beginning of a function or not, unless you keep a table of the address 
of all possible functions and look up each PT_NIP to see if you are.  
But that wouldn't be 100% accurate either, since a non-linking branch 
could just redirect to the beginning of the function.  In the other 
thread, Ben said they have the CFAR register, but from the way it is 
described in the ISA document, you will not always get the address of 
the callsite, e.g. if your target is in the same cache black, the CFAR 
might not change.  So, I don't think it can work in a simple way.

On BookE with my patch to NOT mimic server, you will stop before the 
branch executes, so PT_NIP points to the branch itself, so that there 
is your callsite information.  If you want the function target, you 
decode the instruction to see if it is a branch-and-link, and then 
compute the target address or manually singlestep and grab the new 
PT_NIP.


> May be there is a trick involved here and hence gdb or for that 
> matter the other debuggers are still in the market. 

Per the other thread, and my inquiry through my company's gdb experts 
and upstream on the gdb mailing list, gdb does NOT use hardware branch 
tracing, at least not using PTRACE_SINGLEBLOCK.  I don't personally 
know of any debuggers that use it in the form that it exists in Linux.


> But then I would be immensely obliged if you could shed some light 
> on how is this accomplished. Lets say I am waiting at the userspace 
> in my own sigtrap, to watch out for branch instructions. Lets say I 
> want to profile my code to get to know how many branch instructions 
> it has generated. How could I ever do that using my own custom 
> SIGTRAP handler?

Lots of ways to skin this cat, but I don't think any of them are easy 
or simple.

- Using valgrind is probably the easiest way to get this information 
on an unmodified executable.

- You may want to recompile your program with call-arch profiling if 
you have source code.  gcc -fprofile-arcs but read the docs.

- Another method is to set up performance monitor counter to interrupt 
after a branch has executed, but callsite information may be lost as 
well and you still have the issue of discerning whether you just 
called a function or not.  

- You can use my patch for BookE, but it's just an RFC.  Also, using 
PTRACE_SINGLEBLOCK is slow.  You could also stop on every 
PTRACE_SINGLESTEP, but this is even slower.



> Coming on to PTRACE_SINGLESTEP, the sysroot that has been provided 
> to us by our vendor does not include a PTRACE_SINGLEBLOCK in 
> sys/ptrace.h:

> Although I can clearly see that PTRACE_SINGLEBLOCK is supported in the
> kernel. 
> 
> Hence I am not able to compile this simple program in userspace:

> Heres the error I get:
> testptrace.c: In function 'int main()':
> testptrace.c:47: error: invalid conversion from 'int' to '__ptrace_request'
> testptrace.c:47: error:   initializing argument 1 of 'long int
> ptrace(__ptrace_request, ...)'
> make: *** [testptrace] Error 1
> 
> How should I go about using ptrace to test this? 

I don't know. You'll probably have to request support from your vendor 
or whoever provided you with those headers.  If you are using a glibc 
based toolchain, your glibc is probably out-of-date. You can try 
adding PTRACE_SINGLEBLOCK into the enum definition just to get past 
the compiler syntax error, but I don't know if that will break things 
or not in your libc ptrace().  You may have to write your own function 
that calls the __ptrace syscall directly if the libc ptrace() does 
something to the request or the response.

Re: BookE "branch taken" behavior vis-a-vis updating the NIP register

[pegasus]

I re-read the link you posted earlier and this time it made more sense to me.
The kind of questions which are coming into my mind were being discussed.

So, off I went and downloaded the latest version of
arch/powerpc/kernel/traps.c hoping to see those very changes in them.
However it didn't match one on one with what was written in that thread.
Ditto for the other files in your patch. Looks like your patch didn't make
it to upstream but it looks exactly like what I need here. So allow me to
discuss certain finer points of it, to make sure I understand what it does
correctly.

In that thread you say 
James Yang wrote
> BookE ISA's branch taken exception triggers before a branch that will be
> taken executes.  This allows software to examine the branch and the
> conditions under which it will be taken.  It also means software can tell
> where basic blocks end (at least the ones which are terminated by taken
> branches).  
*
> There are no architected registers that report the address of the branch
> instruction after it has executed.
*
My thoughts exactly! 

In the first patch's description, you say 
James Yang wrote
> This patch makes available the unmodified BookE branch taken debug
> exception through PTRACE_SINGLEBLOCK if the ptrace() addr parameter is set
> to 2.  (The existing behavior of PTRACE_SINGLEBLOCK is retained for any
> other addr parameter value, e.g., 0.)  
*
> SIGTRAP will be signaled with the NIP pointing to the branch instruction
> before it has executed.  The ptrace-calling program can then examine the
> program state.
*
>   
/
> It should then request a PTRACE_SINGLESTEP in order to advance the program
> to the next instruction or a PTRACE_CONT to resume normal program
> execution.
/
>  The si_code now also reports TRAP_BRANCH.

 So requesting PTRACE_CONT has to happen inside the SIGTRAP signal handler
right? So as to advance the branch instruction (and since we are talking
BookE here, we are dead sure this branch will be taken). Now as for the
second patch, as far as I can see, implements the same functionality.
However it makes the change permanent and any tool which is used to the NIP
pointing to the branch target will be broken. 

Anyways, for me either of them will work. But I think the first patch makes
everyone happy by using the 'addr' field of ptrace. This also means I will
have to make my (broken) ptrace working which, it seems is not as easy
adding an enum field as you suggested. May be theres a check somewhere in
the actual ptrace code which checks for illegal values and hence even after
adding an enum, it is being reported as illegal in my case. However getting
that to work is another story.

Please confirm my understanding of your patches and since these patches have
not made their way to the upstream kernel, will have to use them myself
directly. By the way, I'm using 2.6.32.10 (you know..the long-term kernel)
and I couldn't find any of your changes in them but then again I couldn't
find it in the latest 3.12 version either.




--
View this message in context: http://linuxppc.10917.n7.nabble.com/BookE-branch-taken-behavior-vis-a-vis-updating-the-NIP-register-tp77960p78036.html
Sent from the linuxppc-dev mailing list archive at Nabble.com.

Re: BookE "branch taken" behavior vis-a-vis updating the NIP register

[James Yang]

On Tue, 12 Nov 2013, pegasus wrote:

> So, off I went and downloaded the latest version of 
> arch/powerpc/kernel/traps.c hoping to see those very changes in 
> them. However it didn't match one on one with what was written in 
> that thread. Ditto for the other files in your patch. Looks like 
> your patch didn't make it to upstream but it looks exactly like what 
> I need here. 

The patches were posted RFC -- request for comment.  Thank you for 
posting your comments.


>  So requesting PTRACE_CONT has to happen inside the SIGTRAP signal 
> handler right? 

After you get the SIGTRAP from the branch, yeah.


> Now as for the second patch, as far as I can see, implements the 
> same functionality. However it makes the change permanent and any 
> tool which is used to the NIP pointing to the branch target will be 
> broken.

The second patch places the burden of determining if you are BookE or 
server on the tool.  I feel this is important because the Server and 
Book-E branch exceptions are fundamentally incompatible, and the 
behavior of PTRACE_SINGLEBLOCK can not be made to be identical by the 
kernel for both subarch, so a tool will have to adjust accordingly.


> Anyways, for me either of them will work. But I think the first patch makes
> everyone happy by using the 'addr' field of ptrace. This also means I will
> have to make my (broken) ptrace working which, it seems is not as easy
> adding an enum field as you suggested. May be theres a check somewhere in
> the actual ptrace code which checks for illegal values and hence even after
> adding an enum, it is being reported as illegal in my case. However getting
> that to work is another story.

Actually, I provided the first patch to show why it probably should 
not be done even though technically possible, since it requires ptrace 
API to be extended to now recognize addr field, needs a TIF flag, etc.  
The second patch seems to me more reasonable to support, since nobody 
has come forth to say that there are actually any tools that rely upon 
the existing hack for BookE or even PTRACE_SINGLEBLOCK.  And the 
existing hack's behavior makes things somewhat useless on BookE, as 
you and I have confirmed.

 
> Please confirm my understanding of your patches and since these 
> patches have not made their way to the upstream kernel, will have to 
> use them myself directly. By the way, I'm using 2.6.32.10 (you 
> know..the long-term kernel) and I couldn't find any of your changes 
> in them but then again I couldn't find it in the latest 3.12 version 
> either.


You will have to backport the patches to your kernel if you want to 
use them.  Both patches change the behavior of existing API, and I 
guess we are still waiting to hear if anyone cares which way it should 
be fixed.

Re: BookE "branch taken" behavior vis-a-vis updating the NIP register

[Sakthi]

pegasus,

Were you able to achieve the intended behavior with any of the suggested RFC
patch?

Thanks!


pegasus wrote
> I re-read the link you posted earlier and this time it made more sense to
> me. The kind of questions which are coming into my mind were being
> discussed.
> 
> So, off I went and downloaded the latest version of
> arch/powerpc/kernel/traps.c hoping to see those very changes in them.
> However it didn't match one on one with what was written in that thread.
> Ditto for the other files in your patch. Looks like your patch didn't make
> it to upstream but it looks exactly like what I need here. So allow me to
> discuss certain finer points of it, to make sure I understand what it does
> correctly.
> 
> In that thread you say 
> James Yang wrote
>> BookE ISA's branch taken exception triggers before a branch that will be
>> taken executes.  This allows software to examine the branch and the
>> conditions under which it will be taken.  It also means software can tell
>> where basic blocks end (at least the ones which are terminated by taken
>> branches).  
*
>> There are no architected registers that report the address of the branch
>> instruction after it has executed.
*
> 
> My thoughts exactly! 
> 
> In the first patch's description, you say 
> James Yang wrote
>> This patch makes available the unmodified BookE branch taken debug
>> exception through PTRACE_SINGLEBLOCK if the ptrace() addr parameter is
>> set to 2.  (The existing behavior of PTRACE_SINGLEBLOCK is retained for
>> any other addr parameter value, e.g., 0.)  
*
>> SIGTRAP will be signaled with the NIP pointing to the branch instruction
>> before it has executed.  The ptrace-calling program can then examine the
>> program state.
*
>>   
/
>> It should then request a PTRACE_SINGLESTEP in order to advance the
>> program to the next instruction or a PTRACE_CONT to resume normal program
>> execution.
/
>>  The si_code now also reports TRAP_BRANCH.
>  So requesting PTRACE_CONT has to happen inside the SIGTRAP signal handler
> right? So as to advance the branch instruction (and since we are talking
> BookE here, we are dead sure this branch will be taken). Now as for the
> second patch, as far as I can see, implements the same functionality.
> However it makes the change permanent and any tool which is used to the
> NIP pointing to the branch target will be broken. 
> 
> Anyways, for me either of them will work. But I think the first patch
> makes everyone happy by using the 'addr' field of ptrace. This also means
> I will have to make my (broken) ptrace working which, it seems is not as
> easy adding an enum field as you suggested. May be theres a check
> somewhere in the actual ptrace code which checks for illegal values and
> hence even after adding an enum, it is being reported as illegal in my
> case. However getting that to work is another story.
> 
> Please confirm my understanding of your patches and since these patches
> have not made their way to the upstream kernel, will have to use them
> myself directly. By the way, I'm using 2.6.32.10 (you know..the long-term
> kernel) and I couldn't find any of your changes in them but then again I
> couldn't find it in the latest 3.12 version either.





--
View this message in context: http://linuxppc.10917.n7.nabble.com/BookE-branch-taken-behavior-vis-a-vis-updating-the-NIP-register-tp77960p83658.html
Sent from the linuxppc-dev mailing list archive at Nabble.com.