On Wed, May 30, 2018 at 11:39:00PM +0800, Leo Yan wrote:
Hi Mike,
On Wed, May 30, 2018 at 04:04:34PM +0100, Mike Leach wrote:
[...]
/* Generate sample for exception packet */if (etmq->prev_packet->exc == true)generate_sample = true;Please don't do that. Exception packets have a type of their own and can be added to the decoder packet queue the same way INST_RANGE and TRACE_ON packets are. Moreover exception packet containt an address that, if I'm reading the documenation properly, can be used to keep track of instructions that were executed between the last address of the previous range packet and the address executed just before the exception occurred. Mike and Rob will have to confirm this as the decoder may be doing all that hard work for us.
clarification on the exception packets....
The Opencsd output exception packet gives you the exception number, and optionally the preferred return address. If this address is present does depend a lot on the underlying protocol - will normally be there with ETMv4. Exceptions are marked differently in the underlying protocol - the OCSD packets abstract away these differences.
consider the code:
0x1000: <some instructions> 0x1100: BR 0x2000 .... 0x2000: <some instructions> 0x2020 BZ r4
Without an exception this would result in the packets
OCSD_RANGE(0x1000,0x1104, Last instr type=Br, taken) // recall that range packets have start addr inclusive, end addr exclusive. OCSD_RANGE(0x2000,0x2024, Last instr type=Br, <taken / not taken - depends on condition>
Now consider an exception occurring before the BR 0x2000
this will result in:- OCSD_RANGE(0x1000, 0x1100, Last instr type=Other) OCSD_EXECEPTION(IRQ, ret-addr 0x1100) OCSD_RANGE(IRQ_START, IRQ_END+4, Last instr type = BR, taken) // this is more likely to have multiple ranges / branches before any return, but simplified here. OCSD_EXCEPTION_RETURN() // present if exception returns are explicitly marked in underlying trace - may not always be depending on circumstances. OCSD_RANGE(0x1100,0x1104, Last=BR, taken) // continue on with short range - just the branch OCSD_RANGE(0x2000,0x2024, Last instr type=Br, <taken / not taken - depends on condition>
Now consider the exception occurring after the BR, but before any other instructions are executed.
OCSD_RANGE(0x1000,0x1104, Last instr type=Br, taken) // recall that range packets have start addr inclusive, end addr exclusive. OCSD_EXECEPTION(IRQ, ret-addr 0x2000) // here the preferred return address is actually the target of the branch. OCSD_RANGE(IRQ_START, IRQ_END+4, Last instr type = BR, taken) // this is more likely to have multiple ranges / branches before any return, but simplified here. OCSD_RANGE(0x2000,0x2024, Last instr type=Br, <taken / not taken - depends on condition>
So in general it is possible to arrive in the IRQ_START range with the previous packet having been either a taken branch, a not taken branch, or not a branch. Care must be taken - whether AutoFDO or normal trace disassembly not to assume that having the last range packet as a taken branch means that the next range packet is the target, if there is an intervening exception packet.
Thanks a lot for detailed explaination.
IIUC, AutoFDO will not have such issue due every range packet will be handled for it. On the other hand, as you remind, the branch samples (and its consumer trace disassembler) is very dependent on the flag 'last_instr_taken_branch'.
According to your explaination, I think we consider the branch is taken for below situations:
- The new coming packet is exception packet (both for exception entry and exit packets);
- The previous packet is expcetion packet;
- The previous packet is normal range packet with 'last_instr_taken_branch' = true;
So I'd like to use below function to demonstrate my understanding for exception packets handling. I also will send out one new patch for support exception packet for reviewing.
If you have concern or I miss anything, please let me know.
static bool cs_etm__is_taken_branch(struct cs_etm_packet *prev_packet, struct cs_etm_packet *packet,) { /* The branch is taken for normal range packet with taken branch flag */ if (prev_packet->sample_type == CS_ETM_RANGE && prev_packet->last_instr_taken_branch) return true;
/* The branch is taken if previous packet is exception packet */ if (prev_packet->sample_type == CS_ETM_EXCEPTION || prev_packet->sample_type == CS_ETM_EXCEPTION_RET) return true; /* The branch is taken for an intervening exception packet */ if (packet->sample_type == CS_ETM_EXCEPTION || packet->sample_type == CS_ETM_EXCEPTION_RET) return true; return false;}
Just clarify, I missed to mention I introduce two extra sample types: CS_ETM_EXCEPTION and CS_ETM_EXCEPTION_RET, one is for exception entry packet and another is for exception exit packet. If this is hard for understanding, you could hold on for reveiwing new patch.
Thanks, Leo Yan