Re: [Ping] Port of VTV for Cygwin and MinGW

[Kai Tietz <ktietz70@googlemail.com>]

2014-10-09 15:52 GMT+02:00 Patrick Wollgast <patrick.wollgast@rub.de>:
> On 27.09.2014 12:50, Kai Tietz wrote:
>> Hi Patrick,
>>
>> the mingw/cygwin part your patch looks fine to me.  Nevertheless I
>> have one question regarding to you.  Do you have FSF papers for gcc
>> already?  As I asked an overseer and he didn't found you on the list.
>>
>> Regards,
>> Kai
>>
>
> The papers FSF have been taken care of, and the signed papers have been
> exchanged.
>
>
> A short recap:
>
> Mail with the latest patch and changelog:
> https://gcc.gnu.org/ml/gcc-patches/2014-09/msg02200.html
>
> Approved:
> * gcc/config/i386/*
> * libstdc++-v3/*
> * libvtv/*
>
> Not approved:
> * gcc/cp/vtable-class-hierarchy.c

Index: gcc/cp/vtable-class-hierarchy.c
===================================================================
--- gcc/cp/vtable-class-hierarchy.c    (Revision 214408)
+++ gcc/cp/vtable-class-hierarchy.c    (Arbeitskopie)
@@ -1182,7 +1182,7 @@ vtv_generate_init_routine (void)
       TREE_STATIC (vtv_fndecl) = 1;
       TREE_USED (vtv_fndecl) = 1;
       DECL_PRESERVE_P (vtv_fndecl) = 1;
-      if (flag_vtable_verify == VTV_PREINIT_PRIORITY)
+      if (flag_vtable_verify == VTV_PREINIT_PRIORITY && !TARGET_PECOFF)

You need to check that TARGET_PECOFF is defined.  Otherwise you break
compilation for none i386 targets.

         DECL_STATIC_CONSTRUCTOR (vtv_fndecl) = 0;

       gimplify_function_tree (vtv_fndecl);
@@ -1190,7 +1190,7 @@ vtv_generate_init_routine (void)

       cgraph_process_new_functions ();

-      if (flag_vtable_verify == VTV_PREINIT_PRIORITY)
+      if (flag_vtable_verify == VTV_PREINIT_PRIORITY && !TARGET_PECOFF)

See above. Likewise
         assemble_vtv_preinit_initializer (vtv_fndecl);

     }


> * gcc/varasm.c

Index: gcc/varasm.c
===================================================================
--- gcc/varasm.c    (Revision 214408)
+++ gcc/varasm.c    (Arbeitskopie)
@@ -2165,6 +2165,33 @@ assemble_variable (tree decl, int top_le
                              DECL_NAME (decl));
           in_section = sect;
 #else
+          /* 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)

You need to test, if TARGET_PECOFF is defined!

+          {
+            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,

Here it seems to me that you have some whitespace issues,

+                       DECL_NAME (decl));
+            in_section = sect;
+        }
+        else
           switch_to_section (sect);
 #endif


> * libgcc/Makefile.in

Looks ok to me.

> * libgcc/config.host

Looks fine to me, too.

> * libiberty/obstack.c

Why you use instead of C-runtime exit/abort-functions the
platform-functions to terminate the process.  This looks to me like
useless change.  For cygwin this might be even wrong in some aspects.
What is the reasoning for this change?

Another note I have about re-implementation of mprotect in ---
libvtv/vtv_malloc.cc.  Why you need that?  it is already part of
libgcc for mingw.  And for cygwin this function is part of cygwin's
library itself.  So why re-implementing it here?


> Regards,
> Patrick

Regards,
Kai