Thiemo Seufer wrote:
> I don't like this patch. I wrote the code to
> a) print the handler before the (potentially fatal) memcpy. Touching
> EBASE for the first time is a place where things like to go wrong.
Sorry I don't see what you mean...
Do you mean ebase (BTW this name sounds very local...) can be set to
an incorrect value and therefore the memcpy can get mad ?
If so I would say that with this patch applied you can easily guess
that an issue happen in this place because the log would be:
Synthesized TLB refill handler (x instructions)
<nothing>
and there would be no dump of the handler.
Whereas currently we would have:
Synthesized TLB refill handler (x instructions)
<handler dump>
<nothing>
and now you don't know if the crash happen in the memcpy or later...
And it seems better to dump the code which is going to be _executed_
instead of a temporary buffer.
> b) avoid printing leading nops which are never executed. The trailing
> nops have less potential for confusion.
Well, I disagree for 2 reasons:
1/ By printing the leading nops you can detect a bug which would
wrongly write in this unused part of the handler. That's one of
the reason I added the handler instruction addresses BTW: to
skip easily this part when reading the dump.
2/ This is just debug code and it should not make the real code
harder to read. Take a look at the end of
build_r4000_tlb_refill_handler() function. It's not really
obvious that this part is only for debug except the memcpy.
But you're the maintainer of this file so if you still think I should
drop this patch then I will.
Thanks.
Franck
