Messages in this thread | | | Date | Tue, 19 Nov 2013 16:41:17 -0800 | Subject | Re: current_thread_info() not respecting program order with gcc 4.8.x | From | Linus Torvalds <> |
| |
On Tue, Nov 19, 2013 at 7:29 AM, Mathieu Desnoyers <mathieu.desnoyers@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
| |