<div dir="ltr"><div dir="ltr"><div dir="ltr"><br></div><br><div class="gmail_quote"><div dir="ltr" class="gmail_attr">On Thu, Mar 28, 2019 at 7:42 AM Alexander Potapenko <<a href="mailto:glider@google.com">glider@google.com</a>> wrote:<br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">On Thu, Mar 28, 2019 at 10:55 AM Dmitry Vyukov <<a href="mailto:dvyukov@google.com" target="_blank">dvyukov@google.com</a>> wrote:<br>
><br>
> On Wed, Mar 27, 2019 at 1:43 PM Dmitry Vyukov <<a href="mailto:dvyukov@google.com" target="_blank">dvyukov@google.com</a>> wrote:<br>
> ><br>
> > On Tue, Mar 26, 2019 at 11:30 PM James Y Knight <<a href="mailto:jyknight@google.com" target="_blank">jyknight@google.com</a>> wrote:<br>
> > ><br>
> > > The entirety of the named object is replaced. If you want to modify an object, instead of entirely replacing it, you use "+m".<br>
> > ><br>
> > > None of this is anything new or innovative -- GCC has had these semantics -- and been optimizing based on them -- for ages.<br>
> > ><br>
> > > E.g., here, all elements of the array are replaced, so the initialization goes away, and the return needs to explicitly add all 4 values written by the inline-asm.<br>
> > > int out[4] = {1,2,3,4};<br>
> > > asm("whatever" : "=m"(out));<br>
> > > return out[0] + out[1] + out[2] + out[3];<br>
> > ><br>
> > > Here, only out[1] is touched by the inline asm. The other values are not modified, so all of the initialization can disappear, and the generated code can simply return 8 + out[1].<br>
> > > int out[4] = {1,2,3,4};<br>
> > > asm("whatever" : "=m"(out[1]));<br>
> > > return out[0] + out[1] + out[2] + out[3];<br>
> ><br>
> > Thanks!<br>
> ><br>
> > What exactly do you mean by a named object? out[1] does not refer to<br>
> > any named object, right? Or *(a+i).<br>
> ><br>
> > How should be a memset-like function described that writes multiple<br>
> > (unknown) number of elements?<br>
> ><br>
> > What if we need to pass a pointer to beginning of an array, and the<br>
> > asm block writes to i-th (unknown) element of the array?<br>
> ><br>
> > What if we pass a pointer to int but actually write 6 or 32 bytes at<br>
> > that address?<br>
><br>
><br>
> I am asking because I am seeing all of these cases in the kernel code.<br>
> So I am trying to understand (1) if it has format semantics and what<br>
> are they (2) if it's precisely analyzable (3) if developers are aware<br>
> and respect these semantics, or we need to allocate another year for<br>
> fixing incorrect code if we go this route.<br>
<br>
James, what do you think about this particular case<br>
(<a href="https://godbolt.org/z/Vl2bst" rel="noreferrer" target="_blank">https://godbolt.org/z/Vl2bst</a>)?<br>
<br>
=======================<br>
void clear_bit(long nr, volatile unsigned long *addr) {<br>
asm volatile("lock; btr %1,%0"<br>
: "+m"(*(volatile long *)addr)<br>
: "Ir" (nr));<br>
}<br>
unsigned long foo() {<br>
unsigned long addr[2] = {1,2};<br>
clear_bit(65, addr);<br>
return addr[0] + addr[1];<br>
}<br>
=======================<br>
<br>
The declaration of clear_bit() is taken from the Linux kernel<br>
(<a href="https://elixir.bootlin.com/linux/latest/source/arch/x86/include/asm/bitops.h#L111" rel="noreferrer" target="_blank">https://elixir.bootlin.com/linux/latest/source/arch/x86/include/asm/bitops.h#L111</a>).<br>
It however appears to be incorrect: GCC assumes that only addr[0] can<br>
be overwritten by the inline assembly, whereas the call actually<br>
touches addr[1].<br>
Is this the expected behavior?<br></blockquote><div><br></div><div>Yes, this is the expected behavior of the above code. Which is to say: yes, this code is broken.</div><div> </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">
We can fix the situation by adding the "memory" clobber to the asm()<br>
directive, but maybe there's a more elegant way to tell the compiler<br>
we're potentially touching any byte of the array?</blockquote><div><br></div><div>There is no "any byte of the array" here, because you passed it a value of type "long". If you passed the asm a value of array type, it would treat it as touching any byte of the array. But, in this case, there's really no way of knowing the intended size of the data pointed to by the pointer, so that's not workable. Given that you're passing a pointer to unsized data, I would write this instead as simply taking the address and using a memory clobber.</div><div><br></div><div>That is:</div><div> asm volatile("lock; btr %1,%0"<br></div><div><div> :</div><div> : "r"(addr), "Ir" (nr)</div><div> : "memory");</div><div><br></div></div><div><br></div></div></div></div>