Re: memcmp-sse4.S EqualHappy bug

[Andrea Arcangeli <aarcange at redhat dot com>]

On Thu, Jun 18, 2015 at 11:50:40AM +0200, Torvald Riegel wrote:
> Nonetheless, at least the C++ committee is working on specifying a means
> to access non-atomic-typed data atomically (but requiring all concurrent
> accesses to also be atomic then (e.g., by using the same means)).

On a side note in the kernel we already do this to access not-atomic
regions "safely" (or as safe as feasible today) in C:

static __always_inline void __read_once_size(const volatile void *p, void *res, int size)
{
	switch (size) {
	case 1: *(__u8 *)res = *(volatile __u8 *)p; break;
	case 2: *(__u16 *)res = *(volatile __u16 *)p; break;
	case 4: *(__u32 *)res = *(volatile __u32 *)p; break;
	case 8: *(__u64 *)res = *(volatile __u64 *)p; break;
	default:
		barrier();
		__builtin_memcpy((void *)res, (const void *)p, size);
		barrier();
	}
}

static __always_inline void __write_once_size(volatile void *p, void *res, int size)
{
	switch (size) {
	case 1: *(volatile __u8 *)p = *(__u8 *)res; break;
	case 2: *(volatile __u16 *)p = *(__u16 *)res; break;
	case 4: *(volatile __u32 *)p = *(__u32 *)res; break;
	case 8: *(volatile __u64 *)p = *(__u64 *)res; break;
	default:
		barrier();
		__builtin_memcpy((void *)p, (const void *)res, size);
		barrier();
	}
}

#define READ_ONCE(x) \
	({ union { typeof(x) __val; char __c[1]; } __u; __read_once_size(&(x), __u.__c, sizeof(x)); __u.__val; })

#define WRITE_ONCE(x, val) \
	({ typeof(x) __val = (val); __write_once_size(&(x), &__val, sizeof(__val)); __val; })

As you can see if __builtin_memcpy starts to leverage the pure C
semantics (like not copying everything if the data changes at the
start of the memory buffer so triggering undefined beahvior), things
would get "interesting" as far as the kernel is concerned. I mean we
depend on memcpy to explicitly not behave like memcmp is behaving now.

For non-atomic regions, we can read/write it fine with
READ_ONCE/WRITE_ONCE, we just lost a way to do memcmp in userland on
non-atomic regions.

Thanks,
Andrea