Hi Iain, Thanks for reviewing. I’m happy to make the suggested changes. One comment inline.
> On Sep 22, 2021, at 2:49 PM, Iain Sandoe <i...@sandoe.co.uk> wrote: > > 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). My original change does in fact clear the reserved bytes on LP64 platforms. The padding space compiles down to a `.space` assembler directive, and GNU as is documented to fill that space with zeros. So the reserved bits are indeed cleared. However, I understand the argument that this is too implicit, in that the C standard makes no guarantee about the contents of padding bytes. So future standard-conforming changes to GCC *could* cause that space to be filled with other values (however unlikely that may actually be). (Of course, clang -- which also does not explicitly declare this field -- essentially faces this same theoretical peril...) One problem with the proposed diff: `__LP64__` there refers to the host, not the target. What's the right way to refer to the LP64-ness of the target? I see `TARGET_LP64`, but it's only defined for Intel. I'm using it below (and backstopping it to zero), but I'm not sure if that's correct. Note that it's a run-time-of-compiler (not build-time-of-compiler) check. === Here's v2. <https://github.com/mhjacobson/gcc/commit/8193903a1d5a1569a6799174e13cb22925f1f428> gcc/objc/ChangeLog: 2021-09-26 Matt Jacobson <mhjacob...@me.com> * objc-next-runtime-abi-02.c (build_v2_class_templates): Remove explicit padding on non-LP64. (build_v2_class_ro_t_initializer): Remove initialization of explicit padding on non-LP64. diff --git a/gcc/objc/objc-next-runtime-abi-02.c b/gcc/objc/objc-next-runtime-abi-02.c index 42645e22316..22d5232614d 100644 --- a/gcc/objc/objc-next-runtime-abi-02.c +++ b/gcc/objc/objc-next-runtime-abi-02.c @@ -85,6 +85,10 @@ along with GCC; see the file COPYING3. If not see #define OBJC2_CLS_HAS_CXX_STRUCTORS 0x0004L +#ifndef TARGET_LP64 +#define TARGET_LP64 0 +#endif + enum objc_v2_tree_index { /* Templates. */ @@ -677,10 +681,12 @@ build_v2_class_templates (void) /* uint32_t const instanceSize; */ add_field_decl (integer_type_node, "instanceSize", &chain); - /* This ABI is currently only used on m64 NeXT. We always - explicitly declare the alignment padding. */ + /* For compatibility with existing implementations of the 64-bit NeXT v2 + ABI, explicitly declare reserved fields that otherwise would be filled + with alignment padding. */ /* uint32_t const reserved; */ - add_field_decl (integer_type_node, "reserved", &chain); + if (TARGET_LP64) + add_field_decl (integer_type_node, "reserved", &chain); /* const uint8_t * const ivarLayout; */ cnst_strg_type = build_pointer_type (unsigned_char_type_node); @@ -3225,10 +3231,12 @@ build_v2_class_ro_t_initializer (tree type, tree name, CONSTRUCTOR_APPEND_ELT (initlist, NULL_TREE, build_int_cst (integer_type_node, instanceSize)); - /* This ABI is currently only used on m64 NeXT. We always - explicitly declare the alignment padding. */ - /* reserved, pads alignment. */ - CONSTRUCTOR_APPEND_ELT (initlist, NULL_TREE, + /* For compatibility with existing implementations of the 64-bit NeXT v2 + ABI, explicitly zero-fill reserved fields that otherwise would be filled + with alignment padding. */ + /* reserved */ + if (TARGET_LP64) + CONSTRUCTOR_APPEND_ELT (initlist, NULL_TREE, build_int_cst (integer_type_node, 0)); /* ivarLayout */
