[Patch] Fix objc/48109

Iain Sandoe developer@sandoe-acoustics.co.uk
Thu Jun 30 11:36:00 GMT 2011


On 30 Jun 2011, at 11:27, Richard Guenther wrote:

> On Wed, Jun 29, 2011 at 7:47 PM, Iain Sandoe
> <developer@sandoe-acoustics.co.uk> wrote:
>> The bug arises because of the use, by the ObjC FE, of two old  
>> target macros
>> that emit efficient representations of class definitions and  
>> references.
>>
>> This 'works fine' (however wrong it might be conceptually), until  
>> LTO is
>> engaged, whereupon the definitions vanish without trace (since no
>> corresponding real variable is ever created).
>>
>> ---
>>
>> The patch creates appropriate variables in the FE and tags them as  
>> ObjC
>> meta-data (in the same manner as is done for other objc meta-data).
>>
>> We then intercept them in varasm.c with a hook that allows the  
>> target to
>> declare that it has completely handled the output of a variable -  
>> allowing
>> us to handle them in the required special manner.
>>
>> FAOD, It is necessary to preserve this mechanism for emitting the
>> definitions and references to permit linkage with existing system  
>> libraries.
>>
>> ----
>>
>> If the patch is acceptable, then I would expect to follow up with a  
>> patch to
>> remove ASM_DECLARE_CLASS_REFERENCE and  
>> ASM_DECLARE_UNRESOLVED_REFERENCE from
>> the tree - since this is their only use.
>>
>> ----
>>
>> bootstrapped on i686-linux, i686-darwin9, x86_64-darwin10,   
>> (checked to do
>> the Right Thing on darwin).
>>
>> Mike has already given this a 'seems reasonable' in the PR thread,  
>> however,
>> I need an approver for the varasm.c and target hook changes.
>>
>> OK for trunk & 4.6?
>
> Where does the target handle output of the variable?  If it is in  
> the hook
> then that hook is misnamed - it should then probably be called
> assemble_variable.

It is in the hook,
hence named "handled_assemble_variable_p"
.. allowing us to test if the target has completed the work and finish  
assemble_variable () early.

Does that explain it better? - would an expanded comment help?

(I am happy to use any name generally agreed, but IMO the name should  
convey the idea that the result should be tested to determine if the  
target did the work, or it will be confusing to maintain).

> Not sure if this is the most reasonable piece of
> assemble_variable to split out to a hook though.

It is only used by darwin at present - so nothing is split out for any  
other target (the default hook is simply 'false').

Initially, I thought about simply moving the  
ASM_DECLARE_CLASS_REFERENCE and ASM_DECLARE_UNRESOLVED_REFERENCE to  
varasm.c

However, there seems to be a general move to use hooks instead of  
macros, and this seemed like a good time to make the change (and pave  
the way to remove those two macros from the tree).

If the consensus is a preference to retain the macros - I could re- 
work the patch to do that.

> I'm not sure we want to backport this though (there isn't even a  
> bugreport
> about it?).

PR48109, reported initially against 4.6.0 (before it forked);
... I guess it's also wrong code under LTO.

thanks
Iain

>
> Thanks,
> Richard.
>
>> Iain
>>
>> ===
>>
>> gcc/
>>
>>        * target.def (handled_assemble_variable_p): New hook.
>>        * varasm.c (assemble_variable): Allow target to handle  
>> variable
>> output
>>        in some special manner.
>>        * doc/tm.texi: Regenerate.
>>        * config/darwin.c (darwin_objc1_section): Handle class defs/ 
>> refs.
>>        (darwin_handled_assemble_variable_p): New.
>>        * config/darwin-protos.h  
>> (darwin_handled_assemble_variable_p): New.
>>        * config/darwin.h (TARGET_ASM_HANDLED_ASSEMBLE_VARIABLE_P):  
>> New.
>>
>> gcc/objc/
>>
>>        *objc-next-runtime-abi-01.c (handle_next_class_ref): Don't  
>> emit lazy
>> refs. for
>>        cases where the class is local.  Declare a real, meta-data  
>> item
>> tagged as
>>        a class reference.
>>        (handle_next_impent): Declare a real, meta-data item tagged  
>> as a
>> class def.
>>
>> ===
>>
>> Index: gcc/target.def
>> ===================================================================
>> --- gcc/target.def      (revision 175628)
>> +++ gcc/target.def      (working copy)
>> @@ -449,6 +449,13 @@ DEFHOOK
>>  bool, (FILE *file, rtx x),
>>  default_asm_output_addr_const_extra)
>>
>> +DEFHOOK
>> +(handled_assemble_variable_p,
>> + "Returns @code{true} iff the target has handled the assembly of  
>> the\
>> +  variable @var{var_decl}",
>> + bool, (tree var_decl),
>> + hook_bool_tree_false)
>> +
>>  /* ??? The TARGET_PRINT_OPERAND* hooks are part of the asm_out  
>> struct,
>>    even though that is not reflected in the macro name to override  
>> their
>>    initializers.  */
>> Index: gcc/objc/objc-next-runtime-abi-01.c
>> ===================================================================
>> --- gcc/objc/objc-next-runtime-abi-01.c (revision 175628)
>> +++ gcc/objc/objc-next-runtime-abi-01.c (working copy)
>> @@ -2267,27 +2267,51 @@ generate_objc_symtab_decl (void)
>>                   init_objc_symtab (TREE_TYPE (UOBJC_SYMBOLS_decl)));
>>  }
>>
>> -
>>  static void
>>  handle_next_class_ref (tree chain)
>>  {
>> +  tree decl, exp;
>> +  struct imp_entry *impent;
>>   const char *name = IDENTIFIER_POINTER (TREE_VALUE (chain));
>>   char *string = (char *) alloca (strlen (name) + 30);
>>
>> +  for (impent = imp_list; impent; impent = impent->next)
>> +    if (TREE_CODE (impent->imp_context) == CLASS_IMPLEMENTATION_TYPE
>> +        && IDENTIFIER_POINTER (CLASS_NAME (impent->imp_context))
>> +           == IDENTIFIER_POINTER (TREE_VALUE (chain)))
>> +      return; /* we declare this, no need for a lazy ref.  */
>> +
>>   sprintf (string, ".objc_class_name_%s", name);
>>
>> -#ifdef ASM_DECLARE_UNRESOLVED_REFERENCE
>> -  ASM_DECLARE_UNRESOLVED_REFERENCE (asm_out_file, string);
>> -#else
>> -  return ; /* NULL build for targets other than Darwin.  */
>> -#endif
>> +  decl = build_decl (UNKNOWN_LOCATION,
>> +                    VAR_DECL, get_identifier (string),  
>> char_type_node);
>> +  TREE_PUBLIC (decl) = 1;
>> +  DECL_EXTERNAL (decl) = 1;
>> +  DECL_CONTEXT (decl) = NULL_TREE;
>> +  finish_var_decl (decl, NULL);
>> +
>> +  /* We build a variable to signal the reference.  This will be  
>> intercepted
>> +     and output as a lazy reference.  */
>> +  sprintf (string, "_OBJC_class_ref_%s", name);
>> +  exp = build1 (ADDR_EXPR, string_type_node, decl);
>> +  decl = build_decl (input_location,
>> +                    VAR_DECL, get_identifier (string),  
>> string_type_node);
>> +  TREE_STATIC (decl) = 1;
>> +  DECL_ARTIFICIAL (decl) = 1;
>> +  DECL_INITIAL (decl) = error_mark_node;
>> +
>> +  /* We must force the reference.  */
>> +  DECL_PRESERVE_P (decl) = 1;
>> +  OBJCMETA (decl, objc_meta, get_identifier ("V1_CREF"));
>> +  DECL_CONTEXT (decl) = NULL_TREE;
>> +  finish_var_decl (decl, exp);
>>  }
>>
>>  static void
>>  handle_next_impent (struct imp_entry *impent)
>>  {
>>   char buf[BUFSIZE];
>> -
>> +  tree decl;
>>   switch (TREE_CODE (impent->imp_context))
>>     {
>>     case CLASS_IMPLEMENTATION_TYPE:
>> @@ -2303,11 +2327,16 @@ handle_next_impent (struct imp_entry *impent)
>>       return;
>>     }
>>
>> -#ifdef ASM_DECLARE_CLASS_REFERENCE
>> -  ASM_DECLARE_CLASS_REFERENCE (asm_out_file, buf);
>> -#else
>> -  return ; /* NULL build for targets other than Darwin.  */
>> -#endif
>> +  /* We build a variable to signal that this TU contains this class
>> metadata.  */
>> +  decl = build_decl (UNKNOWN_LOCATION,
>> +                    VAR_DECL, get_identifier (buf), char_type_node);
>> +  TREE_PUBLIC (decl) = 1;
>> +  DECL_CONTEXT (decl) = NULL_TREE;
>> +  OBJCMETA (decl, objc_meta, get_identifier ("V1_CDEF"));
>> +  TREE_STATIC (decl) = 1;
>> +  DECL_ARTIFICIAL (decl) = 1;
>> +  DECL_INITIAL (decl) = error_mark_node;
>> +  finish_var_decl (decl,  NULL);
>>  }
>>
>>  static void
>> Index: gcc/varasm.c
>> ===================================================================
>> --- gcc/varasm.c        (revision 175628)
>> +++ gcc/varasm.c        (working copy)
>> @@ -2009,6 +2009,10 @@ assemble_variable (tree decl, int top_level  
>> ATTRIB
>>   align_variable (decl, dont_output_data);
>>   set_mem_align (decl_rtl, DECL_ALIGN (decl));
>>
>> +  /* Allow the target to handle the variable output in some  
>> special manner.
>>  */
>> +  if (targetm.asm_out.handled_assemble_variable_p (decl))
>> +    return;
>> +
>>   if (TREE_PUBLIC (decl))
>>     maybe_assemble_visibility (decl);
>>
>> Index: gcc/config/darwin-protos.h
>> ===================================================================
>> --- gcc/config/darwin-protos.h  (revision 175628)
>> +++ gcc/config/darwin-protos.h  (working copy)
>> @@ -106,6 +106,7 @@ extern void  
>> darwin_asm_output_aligned_decl_local (
>>  extern void darwin_asm_output_aligned_decl_common (FILE *, tree,  
>> const char
>> *,
>>                                                   unsigned  
>> HOST_WIDE_INT,
>>                                                   unsigned int);
>> +extern bool darwin_handled_assemble_variable_p (tree);
>>
>>  extern bool darwin_binds_local_p (const_tree);
>>  extern void darwin_cpp_builtins (struct cpp_reader *);
>> Index: gcc/config/darwin.c
>> ===================================================================
>> --- gcc/config/darwin.c (revision 175628)
>> +++ gcc/config/darwin.c (working copy)
>> @@ -1439,6 +1439,11 @@ darwin_objc1_section (tree decl  
>> ATTRIBUTE_UNUSED,
>>   else if (!strncmp (p, "V2_CSTR", 7))
>>     return darwin_sections[objc_constant_string_object_section];
>>
>> +  else if (!strncmp (p, "V1_CREF", 7))
>> +    return darwin_sections[objc_cls_refs_section];
>> +  else if (!strncmp (p, "V1_CDEF", 7))
>> +    return data_section;
>> +
>>   return base;
>>  }
>>
>> @@ -2594,6 +2599,53 @@ darwin_assemble_visibility (tree decl, int  
>> vis)
>>             "not supported in this configuration; ignored");
>>  }
>>
>> +/* For certain Objective-C metadata we handle the assembly of the  
>> variables
>> +   here (it must be here, rather than in darwin-c.c, to cater for  
>> LTO).
>>  When
>> +   we reference a class we emit a lazy ref, when we have class  
>> metadata
>> +   (local to a specific object), we emit a tag so that linkage  
>> will be
>> +   satisfied for the class.  */
>> +
>> +bool
>> +darwin_handled_assemble_variable_p (tree decl)
>> +{
>> +  tree meta;
>> +  if (DECL_ATTRIBUTES (decl)
>> +      && (meta = lookup_attribute ("OBJC1META", DECL_ATTRIBUTES  
>> (decl))))
>> +    {
>> +      const char *name;
>> +      rtx decl_rtl, symbol;
>> +
>> +      if (TREE_VALUE (meta) == get_identifier ("V1_CREF"))
>> +       {
>> +         tree exp = DECL_INITIAL (decl);
>> +         gcc_assert (exp
>> +                     && exp != error_mark_node
>> +                     && TREE_CODE (exp) == ADDR_EXPR);
>> +         exp = TREE_OPERAND (exp, 0);
>> +         decl_rtl = DECL_RTL (exp);
>> +         symbol = XEXP (decl_rtl, 0);
>> +         name = XSTR (symbol, 0);
>> +         targetm.asm_out.globalize_decl_name (asm_out_file, exp);
>> +         fputs ("\t.lazy_reference\t",asm_out_file);
>> +         assemble_name (asm_out_file, name);
>> +         fputs ("\n",asm_out_file);
>> +         return true;
>> +       }
>> +      else if (TREE_VALUE (meta) == get_identifier ("V1_CDEF"))
>> +       {
>> +         decl_rtl = DECL_RTL (decl);
>> +         symbol = XEXP (decl_rtl, 0);
>> +         name = XSTR (symbol, 0);
>> +         targetm.asm_out.globalize_decl_name (asm_out_file, decl);
>> +         fputs ("\t",asm_out_file);
>> +         assemble_name (asm_out_file, name);
>> +         fputs (" = 0\n",asm_out_file);
>> +         return true;
>> +       }
>> +    }
>> +  return false;
>> +}
>> +
>>  /* VEC Used by darwin_asm_dwarf_section.
>>    Maybe a hash tab would be better here - but the intention is  
>> that this is
>>    a very short list (fewer than 16 items) and each entry should  
>> (ideally,
>> Index: gcc/config/darwin.h
>> ===================================================================
>> --- gcc/config/darwin.h (revision 175628)
>> +++ gcc/config/darwin.h (working copy)
>> @@ -678,6 +678,9 @@ extern GTY(()) section *  
>> darwin_sections[NUM_DARWI
>>  #undef TARGET_ASM_SELECT_SECTION
>>  #define TARGET_ASM_SELECT_SECTION machopic_select_section
>>
>> +#undef TARGET_ASM_HANDLED_ASSEMBLE_VARIABLE_P
>> +#define TARGET_ASM_HANDLED_ASSEMBLE_VARIABLE_P
>> darwin_handled_assemble_variable_p
>> +
>>  #undef TARGET_ASM_FUNCTION_SECTION
>>  #define TARGET_ASM_FUNCTION_SECTION darwin_function_section
>>
>>
>>
>>



More information about the Gcc-patches mailing list