Re: [RFC VTV] Fix VTV for targets that have section anchors.

[Ramana Radhakrishnan <ramana.radhakrishnan@foss.arm.com>]

[-- Attachment #1: Type: text/plain, Size: 1887 bytes --]




On 12/10/15 21:44, Jeff Law wrote:
> On 10/09/2015 03:17 AM, Ramana Radhakrishnan wrote:
>> This started as a Friday afternoon project ...
>>
>> It turned out enabling VTV for AArch64 and ARM was a matter of fixing
>> PR67868 which essentially comes from building libvtv with section
>> anchors turned on. The problem was that the flow of control from
>> output_object_block through to switch_section did not have the same
>> special casing for the vtable section that exists in
>> assemble_variable.
> That's some ugly code.  You might consider factoring that code into a function and just calling it from both places.  Your version doesn't seem to handle PECOFF, so I'd probably refactor from assemble_variable.
> 

I was a bit lazy as I couldn't immediately think of a target that would want PECOFF, section anchors and VTV. That combination seems to be quite rare, anyway point taken on the refactor.

Ok if no regressions ?

>>
>> However both these failures also occur on x86_64 - so I'm content to
>> declare victory on AArch64 as far as basic enablement goes.
> Cool.
> 
>>
>> 1. Are the generic changes to varasm.c ok ? 2. Can we take the
>> AArch64 support in now, given this amount of testing ? Marcus /
>> Caroline ? 3. Any suggestions / helpful debug hints for VTV debugging
>> (other than turning VTV_DEBUG on and inspecting trace) ?
> I think that with refactoring they'd be good to go.  No opinions on the AArch64 specific question -- call for the AArch64 maintainers.
> 
> Good to see someone hacking on vtv.  It's in my queue to look at as well.

Yeah figuring out more about vtv is also in my background queue.

regards
Ramana

PR other/67868

* varasm.c (assemble_variable): Move special vtv handling to..
(handle_vtv_comdat_sections): .. here. New function.
(output_object_block): Handle vtv sections.

libvtv/Changelog

* configure.tgt: Support aarch64 and arm.

[-- Attachment #2: vtv-rework.txt --]
[-- Type: text/plain, Size: 5882 bytes --]

diff --git a/gcc/varasm.c b/gcc/varasm.c
index f1564bc..62ad863 100644
--- a/gcc/varasm.c
+++ b/gcc/varasm.c
@@ -127,6 +127,7 @@ static void asm_output_aligned_bss (FILE *, tree, const char *,
 #endif /* BSS_SECTION_ASM_OP */
 static void mark_weak (tree);
 static void output_constant_pool (const char *, tree);
+static void handle_vtv_comdat_section (section *, const_tree);
 \f
 /* Well-known sections, each one associated with some sort of *_ASM_OP.  */
 section *text_section;
@@ -2230,56 +2231,10 @@ assemble_variable (tree decl, int top_level ATTRIBUTE_UNUSED,
     assemble_noswitch_variable (decl, name, sect, align);
   else
     {
-      /* The following bit of code ensures that vtable_map 
-         variables are not only in the comdat section, but that
-         each variable has its own unique comdat name.  If this
-         code is removed, the variables end up in the same section
-         with a single comdat name.
-
-         FIXME:  resolve_unique_section needs to deal better with
-         decls with both DECL_SECTION_NAME and DECL_ONE_ONLY.  Once
-         that is fixed, this if-else statement can be replaced with
-         a single call to "switch_to_section (sect)".  */
+      /* Special-case handling of vtv comdat sections.  */
       if (sect->named.name
 	  && (strcmp (sect->named.name, ".vtable_map_vars") == 0))
-	{
-#if defined (OBJECT_FORMAT_ELF)
-          targetm.asm_out.named_section (sect->named.name,
-					 sect->named.common.flags
-				         | SECTION_LINKONCE,
-			    	         DECL_NAME (decl));
-          in_section = sect;
-#elif defined (TARGET_PECOFF)
-          /* Neither OBJECT_FORMAT_PE, nor OBJECT_FORMAT_COFF is set here.
-             Therefore the following check is used.
-             In case a the target is PE or COFF a comdat group section
-             is created, e.g. .vtable_map_vars$foo. The linker places
-             everything in .vtable_map_vars at the end.
-
-             A fix could be made in
-             gcc/config/i386/winnt.c: i386_pe_unique_section. */
-          if (TARGET_PECOFF)
-          {
-            char *name;
-            
-            if (TREE_CODE (DECL_NAME (decl)) == IDENTIFIER_NODE)
-              name = ACONCAT ((sect->named.name, "$",
-                               IDENTIFIER_POINTER (DECL_NAME (decl)), NULL));
-            else
-              name = ACONCAT ((sect->named.name, "$",
-                    IDENTIFIER_POINTER (DECL_COMDAT_GROUP (DECL_NAME (decl))),
-                    NULL));
-
-            targetm.asm_out.named_section (name,
-                                           sect->named.common.flags
-                                           | SECTION_LINKONCE,
-                                           DECL_NAME (decl));
-            in_section = sect;
-        }
-#else
-          switch_to_section (sect);
-#endif
-        }
+	handle_vtv_comdat_section (sect, decl);
       else
 	switch_to_section (sect);
       if (align > BITS_PER_UNIT)
@@ -7329,7 +7284,14 @@ output_object_block (struct object_block *block)
 
   /* Switch to the section and make sure that the first byte is
      suitably aligned.  */
-  switch_to_section (block->sect);
+  /* Special case VTV comdat sections similar to assemble_variable.  */
+  if (SECTION_STYLE (block->sect) == SECTION_NAMED
+      && block->sect->named.name
+      && (strcmp (block->sect->named.name, ".vtable_map_vars") == 0))
+    handle_vtv_comdat_section (block->sect, block->sect->named.decl);
+  else
+    switch_to_section (block->sect);
+
   assemble_align (block->alignment);
 
   /* Define the values of all anchors relative to the current section
@@ -7772,4 +7734,56 @@ default_asm_output_ident_directive (const char *ident_str)
     fprintf (asm_out_file, "%s\"%s\"\n", ident_asm_op, ident_str);
 }
 
+
+/* This function ensures that vtable_map variables are not only
+   in the comdat section, but that each variable has its own unique
+   comdat name.  Without this the variables end up in the same section
+   with a single comdat name.
+
+   FIXME:  resolve_unique_section needs to deal better with
+   decls with both DECL_SECTION_NAME and DECL_ONE_ONLY.  Once
+   that is fixed, this if-else statement can be replaced with
+   a single call to "switch_to_section (sect)".  */
+
+static void
+handle_vtv_comdat_section (section *sect, const_tree decl)
+{
+#if defined (OBJECT_FORMAT_ELF)
+  targetm.asm_out.named_section (sect->named.name,
+				 sect->named.common.flags
+				 | SECTION_LINKONCE,
+				 DECL_NAME (decl));
+  in_section = sect;
+#elif defined (TARGET_PECOFF)
+  /* Neither OBJECT_FORMAT_PE, nor OBJECT_FORMAT_COFF is set here.
+     Therefore the following check is used.
+     In case a the target is PE or COFF a comdat group section
+     is created, e.g. .vtable_map_vars$foo. The linker places
+     everything in .vtable_map_vars at the end.
+
+     A fix could be made in
+     gcc/config/i386/winnt.c: i386_pe_unique_section.  */
+  if (TARGET_PECOFF)
+    {
+      char *name;
+
+      if (TREE_CODE (DECL_NAME (decl)) == IDENTIFIER_NODE)
+	name = ACONCAT ((sect->named.name, "$",
+			 IDENTIFIER_POINTER (DECL_NAME (decl)), NULL));
+      else
+	name = ACONCAT ((sect->named.name, "$",
+			 IDENTIFIER_POINTER (DECL_COMDAT_GROUP (DECL_NAME (decl))),
+			 NULL));
+
+      targetm.asm_out.named_section (name,
+				     sect->named.common.flags
+				     | SECTION_LINKONCE,
+				     DECL_NAME (decl));
+      in_section = sect;
+    }
+#else
+  switch_to_section (sect);
+#endif
+}
+
 #include "gt-varasm.h"
diff --git a/libvtv/configure.tgt b/libvtv/configure.tgt
index e543371..adcff5c 100644
--- a/libvtv/configure.tgt
+++ b/libvtv/configure.tgt
@@ -37,6 +37,10 @@ case "${target}" in
   sparc*-*-linux*)
 	;;
   arm*-*-linux*)
+	VTV_SUPPORTED=yes
+	;;
+  aarch64*-*-linux*)
+	VTV_SUPPORTED=yes
 	;;
   x86_64-*-darwin[1]* | i?86-*-darwin[1]*)
 	;;