[PATCH] Use ARM-style representation for C++ method pointers under PNaCl

[John McCall]

On Jul 8, 2013, at 7:16 PM, Mark Seaborn <mseaborn at chromium.org> wrote:
> On 8 July 2013 15:42, John McCall <rjmccall at apple.com> wrote:
> On Jul 8, 2013, at 3:38 PM, Mark Seaborn <mseaborn at chromium.org> wrote:
>> On 8 July 2013 13:51, John McCall <rjmccall at apple.com> wrote:
>> On Jul 8, 2013, at 12:56 PM, Mark Seaborn <mseaborn at chromium.org> wrote:
>>> On 19 June 2013 22:43, Mark Seaborn <mseaborn at chromium.org> wrote:
>>> On 19 June 2013 15:20, John McCall <rjmccall at apple.com> wrote:
>>> On Jun 19, 2013, at 3:17 PM, Mark Seaborn <mseaborn at chromium.org> wrote:
>>>> On 19 June 2013 13:17, John McCall <rjmccall at apple.com> wrote:
>>>> On Jun 19, 2013, at 1:05 PM, Mark Seaborn <mseaborn at chromium.org> wrote:
>>>>> On 19 June 2013 13:01, Mark Seaborn <mseaborn at chromium.org> wrote:
>>>>> Use ARM-style representation for C++ method pointers under PNaCl
>>>>> 
>>>>> Before this change, Clang uses the x86 representation for C++ method
>>>>> pointers when generating code for PNaCl.  However, the resulting code
>>>>> will assume that function pointers are 0 mod 2.  This assumption is
>>>>> not safe for PNaCl, where function pointers could have any value
>>>>> (especially in future sandboxing models).
>>>>> 
>>>>> So, switch to using the ARM representation for PNaCl code, which makes
>>>>> no assumptions about the alignment of function pointers.
>>>>> 
>>>>> See: https://code.google.com/p/nativeclient/issues/detail?id=3450
>>>>> 
>>>>> Oops, I meant to send this to cfe-commits rather than llvm-commits.
>>>> 
>>>> I do not think you should just unconditionally opt in to random ARM
>>>> behavior.  In particular, ARM uses 32-bit guard variables because that's
>>>> the size of a pointer on ARM;  PNaCl needs to be able to efficiently
>>>> support 64-bit platforms as well.
>>>> 
>>>> The code does always use 64-bit guard variables on 64-bit systems.  It does this:
>>>> 
>>>>     // Guard variables are 64 bits in the generic ABI and size width on ARM
>>>>     // (i.e. 32-bit on AArch32, 64-bit on AArch64).
>>>>     guardTy = (IsARM ? CGF.SizeTy : CGF.Int64Ty);
>>>> 
>>>> Having said that, PNaCl is 32-bit-only:  PNaCl programs assume a 32-bit address space.  We don't support 64-bit pointers in PNaCl.  In Clang, targeting PNaCl is identified by "le32" being in the triple, and I assume there's no way to get 64-bit pointers with "le32". :-)
>>> 
>>> Interesting, okay.
>>> 
>>> I still do not want PNaCl to claim to be ARM.  Abstract the code so that
>>> you can opt into the specific behaviors you want without pretending to
>>> be ARM.
>>> 
>>> OK, I have changed the patch to split IsARM into two separate fields.  I called the fields UseARMMethodPtrABI and UseARMGuardVarABI out of a lack of imagination.
>>> 
>>> I've changed the patch to use the non-ARM guard variable ABI for PNaCl.  Having looked at the code more carefully, I see it's inlining a different guard variable check on ARM, which we don't necessarily want to use for PNaCl; it's not just a different guard var size.
>>> 
>>> I've updated the patch to also add a test for C++ guard variable usage under PNaCl.  Is this OK to commit?
>> 
>> +  bool UseARMMethodPtrABI;
>> +  bool UseARMGuardVarABI;
>>  
>> Good enough.
>> 
>> +    if (CGM.getContext().getTargetInfo().getTriple().getArch()
>> +        == llvm::Triple::le32) {
>> 
>> I think testing the OS would be more reasonable here.
>> 
>> Actually, we do want to test the architecture field here.  If we use the triple "i686-unknown-nacl", we want to get the x86 ABI for C++ method pointers, so that the generated code is interoperable with the NaCl GCC toolchain.  If Clang tested the OS field here, that would break this use case.
> 
> So, PNaCl cannot be used to generate interoperable i386 code; you always have to emit a separate i386 slice?
> 
> That's right.  At least, interoperability with native ABIs is limited unless you pass an architecture-specific target triple to Clang when compiling.
> 
> To elaborate, there are two ways of using NaCl's LLVM-based toolchain:
> 
> 1) Compile C/C++ code to a "pexe" (architecture-neutral) offline; ship one pexe file; translate to arch-specific machine code in the web browser.
>  * This is PNaCl proper.
>  * This uses the triple "le32-nacl" when compiling.
>  * There's no option for linking with non-portable, arch-specific native code in this case.  So there's no need to interoperate with complex arch-specific native ABIs (such as C++ method pointers, by-value struct passing, and va_list).  The pexe can use whatever representation it chooses for these features internally.
>  * There are a handful of browser-provided functions that the pexe can call, but these interfaces are all simple enough to avoid interoperability problems (no passing structs by value, no passing of va_list, etc.).
> 
> 2) Compile C/C++ code to arch-specific "nexes" offline; ship multiple nexe files.
>  * In this case, you can link together code generated by LLVM and GCC.
>  * You can compile with either "le32-nacl" or arch-specific triples like "i386-nacl" or "arm-nacl", depending on whether you want ABI compatibility with GCC-generated code.

Okay, then I agree that you need to at least check the arch.

I'm not familiar with the LLVM history, but the comment in Triple.h suggests that there are multiple platforms on le32.  So either (1) check specifically for your platform, which is the combination of le32 and nacl, or (2) convince me (shouldn't take much) that all the platforms either want this or explicitly don't care.

John.
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20130708/3a7c9145/attachment.html>