diff mbox

RFA: Fix for cygwin/mingw PR 66655

Message ID 87lh7cv28r.fsf@redhat.com
State New
Headers show

Commit Message

Nick Clifton Jan. 26, 2016, 2:34 p.m. UTC
Hi Guys,

  The patch below is offered as a fix for PR 66655.  In testing it
  appears that the patch does work, and does not break building
  libstdc++-v3 for cygwin or mingw.  (Unlike the earlier version...)

  Due to my brain being so small, I have already checked the patch in,
  without receiving proper authorisation.  I apologise for this.  If the
  patch does prove suitable and is approved today, then I will leave it
  in.  Otherwise I will revert the change and wait for proper approval.

  The patch itself is also slightly dubious in that I am not sure if I
  have all the correct terms in the if-statement.  I was going for
  minimal impact on the current code, so I went for a set of selectors
  that matched the testcase for PR 66655, but nothing else.  In
  particular I did not check to see if a similar problem exists for
  methods or virtual functions.

  My theory was that if does turn out that these kinds of functions can
  also trigger this kind of bug, then the patch could be extended
  later.  Plus a new bug report is likely to include a new testcase that
  can be added to the testsuite.

  So ... OK to apply ?

Cheers
  Nick

gcc/ChangeLog
2016-01-26  Nick Clifton  <nickc@redhat.com>

	PR target/66655
	* config/i386/winnt.c (i386_pe_binds_local_p): If a function has
	been marked as DECL_ONE_ONLY but we do not the means to make it
	so, then do not allow it to bind locally.

Comments

Jeff Law Jan. 27, 2016, 6:43 a.m. UTC | #1
On 01/26/2016 07:34 AM, Nick Clifton wrote:
> Hi Guys,
>
>    The patch below is offered as a fix for PR 66655.  In testing it
>    appears that the patch does work, and does not break building
>    libstdc++-v3 for cygwin or mingw.  (Unlike the earlier version...)
>
>    Due to my brain being so small, I have already checked the patch in,
>    without receiving proper authorisation.  I apologise for this.  If the
>    patch does prove suitable and is approved today, then I will leave it
>    in.  Otherwise I will revert the change and wait for proper approval.
>
>    The patch itself is also slightly dubious in that I am not sure if I
>    have all the correct terms in the if-statement.  I was going for
>    minimal impact on the current code, so I went for a set of selectors
>    that matched the testcase for PR 66655, but nothing else.  In
>    particular I did not check to see if a similar problem exists for
>    methods or virtual functions.
>
>    My theory was that if does turn out that these kinds of functions can
>    also trigger this kind of bug, then the patch could be extended
>    later.  Plus a new bug report is likely to include a new testcase that
>    can be added to the testsuite.
>
>    So ... OK to apply ?
>
> Cheers
>    Nick
>
> gcc/ChangeLog
> 2016-01-26  Nick Clifton  <nickc@redhat.com>
>
> 	PR target/66655
> 	* config/i386/winnt.c (i386_pe_binds_local_p): If a function has
> 	been marked as DECL_ONE_ONLY but we do not the means to make it
> 	so, then do not allow it to bind locally.
This is OK.

Note that in general we're trying to get away from this kind of 
#if[n]def checking and instead use if (whatever).  The motivation is to 
avoid conditionally compiled code as much as possible.  That avoids lots 
of silly problems that we have to deal with far too often (like 
variables/arguments that are only used inside conditionally compiled 
code and similar stuff.

MAKE_DECL_ONE_ONLY is used elsewhere to create conditionally compiled 
code and we're in stage4, so I'm not going to insist on converting it 
right now.

And FWIW, with Kai leaving, we don't really have a cygwin/mingw 
maintainer.  So I really appreciate the effort you and others have put 
forth to addressing these issues -- if you didn't put forth that effort 
I suspect the windows based toolchains would probably be fairly broken 
for gcc-6.

jeff
diff mbox

Patch

Index: gcc/config/i386/winnt.c
===================================================================
--- gcc/config/i386/winnt.c	(revision 232784)
+++ gcc/config/i386/winnt.c	(working copy)
@@ -341,6 +341,20 @@ 
       && TREE_PUBLIC (exp)
       && DECL_EXTERNAL (exp))
     return true;
+
+#ifndef MAKE_DECL_ONE_ONLY
+  /* PR target/66655: If a function has been marked as DECL_ONE_ONLY
+     but we do not the means to make it so, then do not allow it to
+     bind locally.  */
+  if (DECL_P (exp)
+      && TREE_CODE (exp) == FUNCTION_DECL
+      && TREE_PUBLIC (exp)
+      && DECL_ONE_ONLY (exp)
+      && ! DECL_EXTERNAL (exp)
+      && DECL_DECLARED_INLINE_P (exp))
+    return false;
+#endif
+  
   return default_binds_local_p_1 (exp, 0);
 }