public inbox for gcc-patches@gcc.gnu.org
 help / color / mirror / Atom feed
* RFA: PR 66655: Use COFF/PE weak symbols
@ 2015-12-22 10:17 Nick Clifton
  2016-01-02  5:29 ` Jeff Law
  0 siblings, 1 reply; 6+ messages in thread
From: Nick Clifton @ 2015-12-22 10:17 UTC (permalink / raw)
  To: gcc-patches

Hi Guys,

  The patch below is a proposed fix for PR 66655.  The issue I believe
  is not that the ming32 definition of bind_local_p is wrong, but rather
  that G++ thinks that it cannot make the decl weak even though
  bind_local_p says that it should.  The answer is to define
  MAKE_DECL_ONE_ONLY using the COFF/PE weak symbol support now available
  in gas and the linker.  Doing this allows the test to pass.

  OK to apply ?

Cheers
  Nick

gcc/ChangeLog
2015-12-22  Nick Clifton  <nickc@redhat.com>

	PR target/66655
	* config/i386/cygming.h (MAKE_DECL_ONE_ONLY): Use weak symbol
	support, if available.

Index: config/i386/cygming.h
===================================================================
--- config/i386/cygming.h       (revision 231898)
+++ config/i386/cygming.h       (working copy)
@@ -432,6 +432,10 @@
       fputc ('\n', (FILE));           \
     }                                 \
   while (0)
+
+/* Make use of the weak support for ONE_ONLY decls.  */
+#undef  MAKE_DECL_ONE_ONLY
+#define MAKE_DECL_ONE_ONLY(DECL) (DECL_WEAK (DECL) = 1)
 #endif /* HAVE_GAS_WEAK */
 
 /* FIXME: SUPPORTS_WEAK && TARGET_HAVE_NAMED_SECTIONS is true,

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: RFA: PR 66655: Use COFF/PE weak symbols
  2015-12-22 10:17 RFA: PR 66655: Use COFF/PE weak symbols Nick Clifton
@ 2016-01-02  5:29 ` Jeff Law
  2016-01-05 11:03   ` Nick Clifton
  0 siblings, 1 reply; 6+ messages in thread
From: Jeff Law @ 2016-01-02  5:29 UTC (permalink / raw)
  To: Nick Clifton, gcc-patches

On 12/22/2015 03:17 AM, Nick Clifton wrote:
> Hi Guys,
>
>    The patch below is a proposed fix for PR 66655.  The issue I believe
>    is not that the ming32 definition of bind_local_p is wrong, but rather
>    that G++ thinks that it cannot make the decl weak even though
>    bind_local_p says that it should.  The answer is to define
>    MAKE_DECL_ONE_ONLY using the COFF/PE weak symbol support now available
>    in gas and the linker.  Doing this allows the test to pass.
>
>    OK to apply ?
>
> Cheers
>    Nick
>
> gcc/ChangeLog
> 2015-12-22  Nick Clifton  <nickc@redhat.com>
>
> 	PR target/66655
> 	* config/i386/cygming.h (MAKE_DECL_ONE_ONLY): Use weak symbol
> 	support, if available.
You probably know the capabilities of COFF/PE better than I, so the 
patch itself is fine.

Is there any way that test can be shoved into our dejagnu testing 
harness?  I think we've got support somewhere for tests which require 
multiple input files.

Jeff

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: RFA: PR 66655: Use COFF/PE weak symbols
  2016-01-02  5:29 ` Jeff Law
@ 2016-01-05 11:03   ` Nick Clifton
  2016-01-05 19:08     ` Jeff Law
  0 siblings, 1 reply; 6+ messages in thread
From: Nick Clifton @ 2016-01-05 11:03 UTC (permalink / raw)
  To: Jeff Law, gcc-patches

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

Hi Jeff,

> You probably know the capabilities of COFF/PE better than I, so the
> patch itself is fine.

Thanks - committed.

> Is there any way that test can be shoved into our dejagnu testing
> harness?  I think we've got support somewhere for tests which require
> multiple input files.

dg-additional-sources is the command that you are thinking about.


The attached patch adds the test derived from the BZ.  There is one 
small problem - I could not find a way to stop the additional source 
file from being compiled as a test on its own.  When I investigated 
however it seems that is a common problem for all tests that use 
additional source files, so I do not think that it is a big issue.

Tested with no regressions on an x86_64-pc-linux-gnu toolchain.

OK to apply ?

Cheers
   Nick

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

	PR target/66655
	* g++.dg/pr66655.C: New test.
	* g++.dg/pr66655_1.C: Additional source file for the test.
	* g++.dg/pr66655.h: Header file for the test.

[-- Attachment #2: pr66655.test.patch --]
[-- Type: text/x-patch, Size: 990 bytes --]

--- /dev/null	2016-01-05 08:31:00.212966083 +0000
+++ gcc/testsuite/g++.dg/pr66655.C	2016-01-05 10:42:01.769654305 +0000
@@ -0,0 +1,25 @@
+/* { dg-do run } */
+/* { dg-additional-sources "pr66655_1.C" } */
+
+#include "pr66655.h"
+
+extern "C" void abort (void);
+
+#define COOKIE 0xabcd0123
+
+int
+g (void)
+{
+  return COOKIE;
+}
+
+extern int f (void);
+
+int
+main (void)
+{
+  S::set(0);
+  if (f () != COOKIE)
+    abort ();
+  return 0;
+}
--- /dev/null	2016-01-05 08:31:00.212966083 +0000
+++ gcc/testsuite/g++.dg/pr66655_1.C	2016-01-05 10:41:23.266433205 +0000
@@ -0,0 +1,17 @@
+/* { dg-do compile } */
+/* { dg-options "-O2" } */
+
+#include "pr66655.h"
+
+extern int g (void);
+
+int S::i;
+
+int
+f (void)
+{
+  int ret = g ();
+
+  S::set (ret);
+  return ret;
+}
--- /dev/null	2016-01-05 08:31:00.212966083 +0000
+++ gcc/testsuite/g++.dg/pr66655.h	2016-01-05 10:41:45.629561622 +0000
@@ -0,0 +1,5 @@
+struct S
+{
+  static int i;
+  static void set (int ii) { i = -ii; }
+};

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: RFA: PR 66655: Use COFF/PE weak symbols
  2016-01-05 11:03   ` Nick Clifton
@ 2016-01-05 19:08     ` Jeff Law
  2016-01-07 12:15       ` Nick Clifton
  0 siblings, 1 reply; 6+ messages in thread
From: Jeff Law @ 2016-01-05 19:08 UTC (permalink / raw)
  To: Nick Clifton, gcc-patches

On 01/05/2016 04:03 AM, Nick Clifton wrote:
> Hi Jeff,
>
>> You probably know the capabilities of COFF/PE better than I, so the
>> patch itself is fine.
>
> Thanks - committed.
>
>> Is there any way that test can be shoved into our dejagnu testing
>> harness?  I think we've got support somewhere for tests which require
>> multiple input files.
>
> dg-additional-sources is the command that you are thinking about.
That's it!

>
>
> The attached patch adds the test derived from the BZ.  There is one
> small problem - I could not find a way to stop the additional source
> file from being compiled as a test on its own.  When I investigated
> however it seems that is a common problem for all tests that use
> additional source files, so I do not think that it is a big issue.
I think for C++ code it's usually worked around by naming the 
additional-source file with ".cc" rather than .C.  Can you give that a 
quick whirl and see if that avoids having the additional file used as a 
a test on its own?

Thanks,
jeff

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: RFA: PR 66655: Use COFF/PE weak symbols
  2016-01-05 19:08     ` Jeff Law
@ 2016-01-07 12:15       ` Nick Clifton
  2016-01-07 16:44         ` Jeff Law
  0 siblings, 1 reply; 6+ messages in thread
From: Nick Clifton @ 2016-01-07 12:15 UTC (permalink / raw)
  To: Jeff Law, gcc-patches

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

Hi Jeff,


>> The attached patch adds the test derived from the BZ.  There is one
>> small problem - I could not find a way to stop the additional source
>> file from being compiled as a test on its own.

> I think for C++ code it's usually worked around by naming the
> additional-source file with ".cc" rather than .C.  Can you give that a
> quick whirl and see if that avoids having the additional file used as a
> a test on its own?

Thanks - that worked. :-)

Revised test attached.  OK to apply ?

Cheers
   Nick


[-- Attachment #2: pr66655.test.patch --]
[-- Type: text/x-patch, Size: 937 bytes --]

--- /dev/null	2016-01-07 09:30:14.144966933 +0000
+++ gcc/testsuite/g++.dg/pr66655.C	2016-01-07 12:13:15.757187619 +0000
@@ -0,0 +1,25 @@
+/* { dg-do run } */
+/* { dg-additional-sources "pr66655_1.cc" } */
+
+#include "pr66655.h"
+
+extern "C" void abort (void);
+
+#define COOKIE 0xabcd0123
+
+int
+g (void)
+{
+  return COOKIE;
+}
+
+extern int f (void);
+
+int
+main (void)
+{
+  S::set(0);
+  if (f () != COOKIE)
+    abort ();
+  return 0;
+}
--- /dev/null	2016-01-07 09:30:14.144966933 +0000
+++ gcc/testsuite/g++.dg/pr66655_1.cc	2016-01-07 12:04:30.586623022 +0000
@@ -0,0 +1,14 @@
+#include "pr66655.h"
+
+extern int g (void);
+
+int S::i;
+
+int
+f (void)
+{
+  int ret = g ();
+
+  S::set (ret);
+  return ret;
+}
--- /dev/null	2016-01-07 09:30:14.144966933 +0000
+++ gcc/testsuite/g++.dg/pr66655.h	2016-01-05 10:41:45.629561622 +0000
@@ -0,0 +1,5 @@
+struct S
+{
+  static int i;
+  static void set (int ii) { i = -ii; }
+};

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: RFA: PR 66655: Use COFF/PE weak symbols
  2016-01-07 12:15       ` Nick Clifton
@ 2016-01-07 16:44         ` Jeff Law
  0 siblings, 0 replies; 6+ messages in thread
From: Jeff Law @ 2016-01-07 16:44 UTC (permalink / raw)
  To: Nick Clifton, gcc-patches

On 01/07/2016 05:15 AM, Nick Clifton wrote:
> Hi Jeff,
>
>
>>> The attached patch adds the test derived from the BZ.  There is one
>>> small problem - I could not find a way to stop the additional source
>>> file from being compiled as a test on its own.
>
>> I think for C++ code it's usually worked around by naming the
>> additional-source file with ".cc" rather than .C.  Can you give that a
>> quick whirl and see if that avoids having the additional file used as a
>> a test on its own?
>
> Thanks - that worked. :-)
>
> Revised test attached.  OK to apply ?
Yes.

jeff

^ permalink raw reply	[flat|nested] 6+ messages in thread

end of thread, other threads:[~2016-01-07 16:44 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2015-12-22 10:17 RFA: PR 66655: Use COFF/PE weak symbols Nick Clifton
2016-01-02  5:29 ` Jeff Law
2016-01-05 11:03   ` Nick Clifton
2016-01-05 19:08     ` Jeff Law
2016-01-07 12:15       ` Nick Clifton
2016-01-07 16:44         ` Jeff Law

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).