On 12/14/15 19:19, Richard Henderson wrote:
On 12/12/2015 12:02 PM, Sergey Fedorov wrote:
On 12/12/15 02:39, Richard Henderson wrote:
On 12/10/2015 10:47 AM, Sergey Fedorov wrote:
The "PC advancement" trick was used just after recognizing that a
breakpoint exception was going to be generated. This trick has had two
points:
1. Guarantee that tb->size isn't zero: there are many places
where it's
expected to be non-zero. In fact, that is even stated in the
comment
for this field.
2. Try to satisfy disassembler's check for instruction length. To
this
end, PC advancement was done for estimated instruction length,
but
actually, didn't work properly in variable-instruction-length
cases.
Substitute this trick with checking for TB size at the end of
translation. If we get an empty TB then just set tb->size to 1 and
skip
disassembling. Setting tb->size to 1 is enough to get correct
behaviour,
whereas an empty TB doesn't obviously need to be disassembled.
This doesn't help when the TB already has instructions, the TB would
ordinarily cross a page boundary, and the breakpoint is at the page
boundary.
I see your point. But I am wondering why most architectures stop
translating on
a page boundary whereas i386 and m86k don't. There are some comments
which say
that's to ensure instruction fetch aborts occur at the right place.
Isn't it
necessary for all architectures?
In order to support targets with variable-sized instructions, like
i386, where a single instruction may straddle the page boundary,
generic code can handle a TB taking code from two (and only two) pages.
It's true that some of the way we treat i386 TBs plays a bit fast and
loose, in that it's possible to design a code sequence that will
report a page access error at the beginning of the TB, rather than
another runtime exception which might have been visible if we'd
executed the TB.
I believe that someone fixed that for arm thumb, but no one has
bothered about this for i386, m68k, or s390. Because it just doesn't
happen in practice; one has to go out of one's way to design the
errant code sequence.
As I can see, Peter M. fixed the translation of page-straddling ARM
Thumb32 insns in 541ebcd (27 Oct 2015). The fixed code ensures that we
stop translating if we encounter a page-straddling insn in addition to
stopping at an insn from a next page. Thus we put a page-straddling insn
into a separate TB so that the insn fetch exception would report the
actual PC value for both pages. Looks like any other arch should do the
same, right?
But, if we forget about page-straddling instructions for a moment,
shouldn't every arch stop translating when crossing a page boundary? If
yes then a breakpoint at a page boundary could only translate to an
empty TB.
At least for those architectures which do stop translating on a page
boundary,
I think this patch is applicable. Certainly, it would be better to
have a
single solution for all architectures.
For that, I think it might be interesting to arrange for non-empty TBs
to exit prior to recognizing a breakpoint. So that a breakpoint TB is
always just the one operation. Except for the fact that "generate an
exception" has traditionally been a target-specific helper, we could
almost make the entire breakpoint generation be done in common code.
I'd think something like a generic "must we end the TB now" predicate
would be the proper hook. It would contain all of the usual stuff:
tcg_op_buf_full and checks for singlestep, but then add "is there a
breakpoint at the next pc".
This could be a next step :)
Thanks,
Sergey