[llvm-commits] [PATCH] improve ARM halt encoding

[JF Bastien]

Hi Jim,

I updated this and I now don't think it's so ugly anymore, thanks to some
guidance from Eli. The one dodgy thing is in ARM_MC::ParseARMTriple: I
think it should be using Triple, but that's better done in a separate
change.

Please take a look.

Thanks,

JF


On Tue, Jan 22, 2013 at 4:28 PM, JF Bastien <jfb at google.com> wrote:

> Hi Jim,
>
> Here's an updated patch.
>
> I'm not sure it's quite right yet and would appreciate any suggestions you
> may have, especially when it comes to reconciling ISel and MC: they seem to
> get information slightly differently, the former from triple and the latter
> from CPU features (mostly mattr). I may be mistaken, but I believe that I
> need a SubTargetFeature for MC. The oddness I refer to comes out in the
> test files that I changed.
>
> One suggestion from Derek was to create a separate trap_nacl mnemonic at
> the assembly level. The backend up to ISel would still do lowering
> decisions for @llvm.trap based on the presence of the NaCl triple, but MC
> would be oblivious to this since the mnemonic stands alone. Thoughts on
> this?
>
> Thanks,
>
> JF
>
>
> On Mon, Jan 21, 2013 at 2:42 PM, JF Bastien <jfb at google.com> wrote:
>
>> I see, that's what I was trying to avoid without realizing that it was
>> the only way to go :-)
>>
>> I'll send an updated patch tomorrow, at least so I can pretend to take
>> MLK off.
>>
>>
>> On Mon, Jan 21, 2013 at 10:43 AM, Jim Grosbach <grosbach at apple.com>wrote:
>>
>>> Hi JF,
>>>
>>> That's the ugly part. You need two totally separate definitions,
>>> including a different instruction name.
>>>
>>> // The new definition.
>>> def TRAPnacl: …. Requires<[IsARM, IsPNaCL]>;
>>> // The old definition. Updated "Requires" clause.
>>> def TRAP: …. Requires<[IsARM, IsNotPNaCL]>;
>>>
>>> Any .cpp code which references ARM::TRAP will also need to be updated.
>>>
>>> -Jim
>>>
>>> On Jan 18, 2013, at 11:27 AM, JF Bastien <jfb at google.com> wrote:
>>>
>>> I've tried a few different things and tablegen eludes me. How do I do
>>> something like this:
>>>   let isBarrier = 1, isTerminator = 1 in
>>>   def TRAP : AXI<(outs), (ins), MiscFrm, NoItinerary,
>>>                  "trap", [(trap)]>,
>>>              Requires<[IsARM]> {
>>>     bits<16> imm16 = [{
>>>       if (IsNaCl)
>>>         return 0b1110110111100000;
>>>       return 0b1111110111101110;
>>>     }];
>>>     let Inst{31-20} = 0b111001111111;
>>>     let Inst{19-8} = imm16{15-4};
>>>     let Inst{7-4} = 0b1111;
>>>     let Inst{3-0} = imm16{3-0};
>>>   }
>>> ?
>>>
>>> Multiclasses+Requires doesn't quite work because it would create
>>> different names for the NaCl/non-NaCl versions.
>>>
>>>
>>> On Wed, Jan 16, 2013 at 5:25 PM, Jim Grosbach <grosbach at apple.com>wrote:
>>>
>>>>
>>>> On Jan 16, 2013, at 4:58 PM, JF Bastien <jfb at google.com> wrote:
>>>>
>>>> The sneaky part is indeed our original intent: we want to mitigate
>>>> mode-switching attacks so we sketched out a mitigation that's kind of like
>>>> constant blinding, it makes the final code somewhat unpredictable to the
>>>> attacker. I do think that it's possible to bias instruction selection
>>>> without incurring an undue performance hit while still messing with
>>>> known-code creation.
>>>>
>>>>
>>>> With clever help from register allocation, that's quite possible. The
>>>> low bits are often enough mostly register and immediate values that they
>>>> can be twiddled creatively. I'm curious what you come up with there. That
>>>> could be quite interesting!
>>>>
>>>> The one issue we had was that we needed to freeze our ARM ABI, and that
>>>> implied freezing all the trap/halt/debug instructions that the NaCl "OS"
>>>> recognizes, hence this change.
>>>>
>>>> The mitigation is something we'll implement later, once PNaCl reaches
>>>> its performance goals. We've already branched NaCl ARM with a frozen ABI
>>>> for Chrome M25, and although the current backend is GCC we'll keep the same
>>>> ABI once we migrate to PNaCl.
>>>>
>>>> In the meantime we'd like to reduce PNaCl's localmods, so while this
>>>> change on its own is quite silly, we think it's critical as part of a whole
>>>> system.
>>>>
>>>> Given this, would it make sense to commit this change, once I
>>>> specialize it for the NaCl ARM target?
>>>>
>>>>
>>>> Yeah, that's fine. I'll probably grumble a bit about the added
>>>> complexity, but it's the right thing to do. :)
>>>>
>>>> -Jim
>>>>
>>>>
>>>> On Wed, Jan 16, 2013 at 4:33 PM, Jim Grosbach <grosbach at apple.com>wrote:
>>>>
>>>>> Hi JF,
>>>>>
>>>>> Assuming there's real security benefits to be had, yes. However, I'm
>>>>> reticent to add complexity to the code on a purely theoretical benefit. Can
>>>>> you elaborate a bit more on why this is worth it?
>>>>>
>>>>> In particular, I'm skeptical of benefits to an overlapping ARM/Thumb
>>>>> TRAP instruction. Now, I can definitely see benefit if you could find a way
>>>>> to get ARM ISel to more frequently have Thumb2 undefined bitpatterns in the
>>>>> bitstream (as the low-order bits of normal ARM instructions, that is). That
>>>>> would be quite clever and downright sneaky. Also likely pretty hard to do
>>>>> it w/o completely crushing performance…
>>>>>
>>>>> -Jim
>>>>>
>>>>> On Jan 16, 2013, at 4:27 PM, JF Bastien <jfb at google.com> wrote:
>>>>>
>>>>> As discussed over IRC: it then makes sense to only change the encoding
>>>>> for the NaCl triple (which is effectively its own OS).
>>>>>
>>>>>
>>>>> On Wed, Jan 16, 2013 at 4:17 PM, Jim Grosbach <grosbach at apple.com>wrote:
>>>>>
>>>>>>
>>>>>> On Jan 16, 2013, at 3:45 PM, Renato Golin Linaro <
>>>>>> renato.golin at linaro.org> wrote:
>>>>>>
>>>>>> On 16 January 2013 22:39, Jim Grosbach <grosbach at apple.com> wrote:
>>>>>>
>>>>>>> The entire encoding is semantically significant on Darwin. I suspect
>>>>>>> that's true on other platforms, too, but I don't know for sure.
>>>>>>>
>>>>>>
>>>>>> I'm not sure either. I agree with Bastien that it *should* trap on
>>>>>> both ARM and Thumb, but it also depends on what catch routine is installed
>>>>>> and other hard-to-know problems.
>>>>>>
>>>>>>
>>>>>> Both instructions will trap; however, how they trap is also
>>>>>> important. In this case, it's the difference between the user program
>>>>>> terminating with SIGILL vs. SIGTRAP.
>>>>>>
>>>>>> -Jim
>>>>>>
>>>>>>
>>>>>> Bastien, Have you tested in which platforms?
>>>>>>
>>>>>> cheers,
>>>>>> --renato
>>>>>>
>>>>>>
>>>>>>
>>>>>>
>>>>>
>>>>>
>>>>
>>>>
>>>
>>>
>>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20130123/9ab1849f/attachment.html>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: arm-halt.diff
Type: application/octet-stream
Size: 10000 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20130123/9ab1849f/attachment.obj>