On Thu, Aug 08, 2013 at 05:41:07PM +0200, Oleg Nesterov wrote: > On 08/07, Linus Torvalds wrote: > > > > Now, I do agree that the debug registers are *much* less likely to > > have those kinds of really subtle issues, so maybe relaxing some of > > the tests might be reasonable. I'd be a bit nervous about it, but if > > it's *only* the length/alignment, and Intel people can be convinced > > that it doesn't result in any nasty undefined behavior (as long as the > > address is in user space), maybe we could make that change just to > > make it easier for Wine. > > Oh, I do not know. And again, this way a user can't notice the problem > if the arguments are wrong. > > But personally I think it would be nice to cleanup the perf interface, > although probably it is too later. > > On x86 execute breakpoints are only a single byte, which has to be > the first byte of the instruction. IOW the hardware requires len = 1 > in dr7 or it doesn't work (iirc). > > But for some reason perf requires bp_len = sizeof(long), not 1. And > note that it sets info->len = X86_BREAKPOINT_LEN_X. The comment says: > > x86 inst breakpoints need to have a specific undefined len > > but despite its "special" name LEN_X is simply LEN_1, and other code > relies on this fact. > > Now, ptrace correctly requires DR_LEN_1. So arch_bp_generic_fields() > translates this into "gen_len = sizeof(long)" for validation. > > arch_build_bp_info() thinks that X86_BREAKPOINT_EXECUTE should have > ->bp_len == sizeof(long), so we translate it back into LEN_1 internally.
I did this interface and I'm sorry about it. This bp_len == sizeof(long) requirement comes from a very buggy conception I had at the time I wrote that. I thought it would be pretty intuitive to assume that instruction breakpoints should be the size of the instruction itself as a generic interface for all archs. But at least x86 instructions size aren't static. That sizeof(long) assumption just popped up from nowhere at 5 am two years ago I guess :-( And worse: I realized that mistake later but never moved it in the top of the TODO-list pile because I had the feeling that nobody was using the perf breakpoint interface anyway. I'm all for fixing this. May be we can start by backporting a patch that ignores the value of gen_len for instruction breakpoints in x86? I don't know how other archs use it. I need to check. But this bp_len should rather be used for range breakpoints on archs that support it. I hope we can still reuse it if the damage of my initial misconception isn't too widely expanded. What do you think? > > This looks confusing, imho. And imho X86_BREAKPOINT_LEN_X should die. Yep. Thanks. -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
