Hi Matt,

thanks for the patch.

> On 21 Sep 2021, at 23:29, Matt Jacobson via Gcc-patches 
> <gcc-patches@gcc.gnu.org> wrote:
> 
> Fix class_ro layout for non-LP64.  

> On LP64, the requisite padding is added at a lower level.  

However, the behaviour is changed - the existing implementation is explicit 
about the fields and
clears the reserved ones (and, ISTR, that was based on what the gcc-4.2.1 
compiler did).

(as an aside, in general, I have to test changes in the runtime across the 
range of supported
 Darwin versions, since there have been clang changes too).

> For non-LP64, this fixes binary compatibility with clang-built 
> classes/runtimes.

> Tested by examining the generated assembly for a class_ro in both cases (and 
> in 
> the case of clang), for both x86_64 (64-bit pointers) and AVR (16-bit 
> pointers).
> Tested by running a program on AVR with a GCC-built class using a clang-built 
> Objective-C runtime.  Tested by running a program on x86_64/Darwin with a GCC-
> built class and the clang-built system Objective-C runtime.
> 
> Patch also available at:
> <https://github.com/mhjacobson/gcc/commit/917dc8bb2f3265c2ca899ad750c5833b0161a11e>
> 
> I don't have commit access, so if this patch is suitable, I'd need someone 
> else 
> to commit it for me.  Thanks.

how about we keep the LP64 behaviour unchanged and amend the comments and 
ifdefs as below,
does this work for your case?
(typed into email and thus untested).

thanks
Iain

> gcc/objc/ChangeLog:
> 
> 2021-09-21  Matt Jacobson  <mhjacob...@me.com>
> 
>       * objc-next-runtime-abi-02.c (struct class_ro_t): Remove explicit 
> alignment 
>       padding.
>       (build_v2_class_templates): Remove explicit alignment padding.
>       (build_v2_class_ro_t_initializer): Adjust initializer.
> 
> 
> diff --git a/gcc/objc/objc-next-runtime-abi-02.c 
> b/gcc/objc/objc-next-runtime-abi-02.c
> index 42645e22316..c3af369ff0d 100644
> --- a/gcc/objc/objc-next-runtime-abi-02.c
> +++ b/gcc/objc/objc-next-runtime-abi-02.c
> @@ -632,9 +632,7 @@ struct class_ro_t
>     uint32_t const flags;
>     uint32_t const instanceStart;
>     uint32_t const instanceSize;\

> -#ifdef __LP64__
> -    uint32_t const reserved;
> -#endif
> +    // [32 bits of reserved space here on LP64 platforms]
^revert this

>     const uint8_t * const ivarLayout;
>     const char *const name;
>     const struct method_list_t * const baseMethods;
> @@ -677,11 +675,6 @@ build_v2_class_templates (void)
>   /* uint32_t const instanceSize; */
>   add_field_decl (integer_type_node, "instanceSize", &chain);
> 
#ifdef __LP64__
/* For compatibility with existing implementations of the 64 bit NeXT
   library, explicitly describe reserved fileds used for alignment
   padding.  */

>   /* uint32_t const reserved; */
>   add_field_decl (integer_type_node, "reserved", &chain);
#endif
> 
>   /* const uint8_t * const ivarLayout; */
>   cnst_strg_type = build_pointer_type (unsigned_char_type_node);
>   add_field_decl (cnst_strg_type, "ivarLayout", &chain);
> @@ -3225,12 +3218,6 @@ build_v2_class_ro_t_initializer (tree type, tree name,
>   CONSTRUCTOR_APPEND_ELT (initlist, NULL_TREE,
>                         build_int_cst (integer_type_node, instanceSize));
#ifdef __LP64__
/* For compatibility with existing implementations of the 64 bit NeXT
   library, ensure that reserved padding fields are 0-initialized.  */

>   CONSTRUCTOR_APPEND_ELT (initlist, NULL_TREE,
>                           build_int_cst (integer_type_node, 0));
#endif
> 
>   /* ivarLayout */
>   unsigned_char_star = build_pointer_type (unsigned_char_type_node);
>   if (ivarLayout)


Reply via email to