[lttng-dev] current_thread_info() not respecting program order with gcc 4.8.x

[torvalds@linux-foundation.org (Linus Torvalds)]

On Tue, Nov 19, 2013 at 7:29 AM, Mathieu Desnoyers
<mathieu.desnoyers at efficios.com> wrote:
>
> Since each current_thread_info() is a different asm ("sp") without clobber nor volatile, AFAIU, the compiler is within its right to reorder them.

I don't understand why you say that.

The ordering of the access to the asm("sp") register is totally
irrelevant. You are "correct" in saying that the compiler is within
its right to re-order them, but that is the worst kind of correct:
it's totally immaterial. In fact, we *want* the compiler to not just
re-order the accesses to %sp, but to notice that it can combine them,
and do CSE on that whole expression when it is used multiple times
within the same function (like it often is used).

So the compiler can very much decide to re-read %sp all it wants, and
re-order those reads all it wants, and that's not the bug at all.
Putting a clobber or a volatile on it would disable the optimization
we *want* to happen.

So don't bark up the wrong tree.

The bug seems to be that gcc re-orders the *memory* *accesses* through
that point, which is not correct in any way, shape, or form. If we
have a write to a memory location followed by a read of the same
memory location, the compiler ABSOLUTELY MUST NOT RE-ORDER THEM. The
write obviously changes the value of the read.

It seems that some gcc alias analysis completely incorrectly thinks
that they are not the same memory location, and do not alias. My guess
would be that gcc sees that that they are based on the stack pointer
with "different" offsets, and decides that the memory locations must
be different - without noticing that the "& ~(THREAD_SIZE - 1)" will
end up generating the same address for both of them.

There may be some insane "two different objects on the stack cannot
alias" logic, which is true for *objects* on the stack, but it sure as
hell isn't true for random accesses through asm("sp").

If I read this thread correctly, you're all talking about something
else than the actual bug, and are trying to say that there is
something wrong with re-ordering the access to %sp itself. Missing the
_real_ bug entirely. See above.

                  Linus