Re: [Xen-devel] [PATCH 4/4] xen/arm: correctly handle an empty array of platform descs.

[Ian Campbell <ian.campbell@xxxxxxxxxx>]

On Fri, 2013-05-17 at 11:39 +0100, Jan Beulich wrote:
> >>> On 17.05.13 at 12:07, Ian Campbell <ian.campbell@xxxxxxxxxx> wrote:
> > On Thu, 2013-05-16 at 11:17 +0100, Jan Beulich wrote:
> >> >>> On 15.05.13 at 15:47, Ian Campbell <ian.campbell@xxxxxxxxxx> wrote:
> >> > On Wed, 2013-05-15 at 13:19 +0100, Jan Beulich wrote:
> >> >> >>> On 15.05.13 at 11:47, Ian Campbell <ian.campbell@xxxxxxxxxx> wrote:
> >> >> > "<" fails because it does process the (non-existent) first entry in 
> >> >> > the
> >> >> > array. This happened to be "safe" in the case I saw but it wouldn't be
> >> >> > in general.
> >> >> 
> >> >> Okay, I misread one of your earlier responses then. Did you do
> >> >> the necessary auditing already, or should I put this on my todo
> >> >> list?
> >> > 
> >> > I haven't done an audit. I put a very quicly grepped list in a previous
> >> > mail but it is surely incomplete.
> >> 
> >> So I went through all of them - the only other ones that can be
> >> potentially empty are .ctors and .xsm_initcalls.init (I didn't check
> >> whether ARM has any guaranteed .ex_table.pre uses though).
> > 
> > On a random arm64 binary which I have here both ex_table and
> > ex_table.pre are empty...
> 
> Indeed. Yet arch/arm/ also has no reference to
> __{start,stop}__{,_pre}_ex_table, so this is no problem. Out of
> curiosity - there's nothing you ever do in ARM on behalf of the
> user than can fault?

That's right, we access guest memory with domain_map_page and never by
using a guest virtual address directly (since we don't share page tables
with the guest). The mechanism for translating guest virtual addresses
into MFNs has an explicit error return and doesn't fault.

> >> Both use "<", and the compiler translates this safely on x86. My
> >> ARM assembly skills are still lacking, but afaict the early exit being
> >> done with "popcs" / "b.cs" should be safe too, as they cover the
> >> "==" case (APSR.C being set for x <= y). Thus I wonder what
> >> code you saw being generated for the "<" case...
> > 
> > 00000000 <test>:
> >    0:       e92d4038        push    {r3, r4, r5, lr}
> >    4:       e59f4020        ldr     r4, [pc, #32]   ; 2c <test+0x2c>
> >    8:       e59f5020        ldr     r5, [pc, #32]   ; 30 <test+0x30>
> >    c:       e1540005        cmp     r4, r5
> >   10:       28bd8038        popcs   {r3, r4, r5, pc}
> >   14:       e1a00004        mov     r0, r4
> >   18:       e2844004        add     r4, r4, #4
> >   1c:       ebfffffe        bl      0 <u>
> >   20:       e1540005        cmp     r4, r5
> >   24:       3afffffa        bcc     14 <test+0x14>
> >   28:       e8bd8038        pop     {r3, r4, r5, pc}
> > 
> > So indeed I think you are correct that the popcs will do the right
> > thing, I obviously missed the update of PC via that instruction when I
> > looked at this before.
> 
> But you still said that in practice you observed one unwanted
> iteration of such loops - how does that fit together?

You're right, I was sure I did observe that, but trying again now I
don't. I think I may just have been confused earlier -- probably
confusing my debug print after the loop with the one from inside. Sorry
for the noise.

> >> And btw., for both 32- and 64-bit ARM, other than for x86, I see
> >> empty structure instances occupy zero bytes (and hence distinct
> >> symbols end up at the same address), so the compiler is conflicting
> >> with itself here.
> > 
> > I imagine this is as much to do with the architecture ABI as the
> > compiler.
> 
> So do I, but this doesn't make this any less of a compiler bug.

Is there a requirement (e.g. from the C standard) that an empty struct
take at least one byte, or more likely I suppose something about
different instances having different addresses which sort of implies it?

Ian


_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
http://lists.xen.org/xen-devel