[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

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



>>> 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?

>> 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?

>> 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.

Jan


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


 


Rackspace

Lists.xenproject.org is hosted with RackSpace, monitoring our
servers 24x7x365 and backed by RackSpace's Fanatical Support®.