Re: [PATCH 2/2] jump labels/x86: Use etiher 5 byte or 2 byte jumps

[Steven Rostedt <rostedt@goodmis.org>]

On Thu, 2012-01-19 at 09:58 -0500, Steven Rostedt wrote:
> On Thu, 2012-01-19 at 06:46 -0800, H. Peter Anvin wrote:

> > 
> > >> +
> > >> +	/* Use probe_kernel_read()? */
> > >> +	op = *(unsigned char *)ip;
> > >> +	nop = ideal_nops[NOP_ATOMIC5][0];
> > >>  
> > >>  	if (type == JUMP_LABEL_ENABLE) {
> > >> -		code.jump = 0xe9;
> > >> -		code.offset = entry->target -
> > >> -				(entry->code + JUMP_LABEL_NOP_SIZE);
> > >> -	} else
> > >> -		memcpy(&code, ideal_nops[NOP_ATOMIC5], JUMP_LABEL_NOP_SIZE);
> > >> +		if (op == 0xe9 || op == 0xeb)
> > >> +			/* Already enabled. Warn? */
> > > 
> > > This could be caused by failure to run the link-time script, or running
> > > the transform twice. A warning would indeed be welcome, as this should
> > > never happen.
> > > 
> > 
> > Warning?  No.  ERROR.  Something very bad could be happening here.  We
> > have covered this before.
> 
> Heh, not this exactly. We covered run time errors, this is build time
> errors.

Oops, this is patch 2 not 1. It is run time error.

> 
> But I agree, it should error. The mcount code errors on problems, this
> should too.

I'll have to look at this in more detail, to make sure jump labels are
consistent in their updates. It may legitimately enable them twice. If
that's the case, what should be done is to make sure the entire
instruction is what we expect it to be, otherwise BUG().

-- Steve