Re: gcc 4.5.1 / as 2.20.51.0.11 miscompiling drivers/char/i8k.c ?

From: Jim Bos
Date: Mon Nov 15 2010 - 12:42:52 EST


On 11/15/2010 05:04 PM, Linus Torvalds wrote:
> On Mon, Nov 15, 2010 at 3:16 AM, Jakub Jelinek <jakub@xxxxxxxxxx> wrote:
>>
>> I don't see any problems on the assembly level. i8k_smm is
>> not inlined in this case and checks all 3 conditions.
>
> If it really is related to gcc not understanding that "*regs" has
> changed due to the memory being an automatic variable, and passing in
> "regs" itself as a pointer to that automatic variable together with
> the "memory" clobber not being sufficient, than I think it's the lack
> of inlining that will automatically hide the bug.
>
> (Side note: and I think this does show how much of a gcc bug it is not
> to consider "memory" together with passing in a pointer to an asm to
> always be a clobber).
>
> Because if it isn't inlined, then "regs" will be seen a a real pointer
> to some external memory (the caller) rather than being optimized to
> just be the auto structure on the stack. Because *mem is auto only
> within the context of the caller.
>
> Which actually points to a possible simpler:
> - remove the "+m" since it adds too much register pressure
> - mark the i8k_smm() as "noinline" instead.
>
> Quite frankly, I'd hate to add even more crud to that inline asm (to
> save/restore the registers manually). It's already not the prettiest
> thing around.
>
> So does the attached patch work for everybody?
>
> Linus

Hmm, that doesn't work.

[ Not sure if you read to whole thread but initial workaround was to
change the asm(..) to asm volatile(..) which did work. ]

Jim.
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/