public inbox for gcc-patches@gcc.gnu.org
 help / color / mirror / Atom feed
* PATCH: PR target/63815: [5 Regression] g++.dg/other/pr53811.C fails with -mcmodel=large -fpic
@ 2014-11-12 20:47 H.J. Lu
  2014-11-12 20:59 ` Uros Bizjak
  2014-11-12 21:11 ` Jakub Jelinek
  0 siblings, 2 replies; 9+ messages in thread
From: H.J. Lu @ 2014-11-12 20:47 UTC (permalink / raw)
  To: gcc-patches; +Cc: Uros Bizjak

Hi,

r216154 exposed an x86 backend bug.  For large PIC mode thunk, there
are

      if (ix86_cmodel == CM_LARGE_PIC && SYMBOLIC_CONST (fnaddr))
	fnaddr = legitimize_pic_address (fnaddr,
					 gen_rtx_REG (Pmode, tmp_regno));

and legitimize_pic_address does:

      if (reg != 0)
        {
          new_rtx = expand_simple_binop (Pmode, PLUS, reg, pic_offset_table_rtx,
                                         tmpreg, 1, OPTAB_DIRECT);
          new_rtx = reg;
        }

However, pic_offset_table_rtx was never initialized.  Befor r216154,
we generated thunk with random value in hardcoded PIC register.  After
r216154, compiler crashes.  This patch sets PIC register to %r11 and
initialize it.  Tested on Linux/x86-64.  OK for trunk and backport 
to 4.8/4.9 branches?

Thanks.


H.J.
---
gcc/

2014-11-12  H.J. Lu  <hongjiu.lu@intel.com>

	PR target/63815
	* config/i386/i386.c (ix86_init_large_pic_reg): New.  Extracted
	from ...
	(ix86_init_pic_reg): Here.  Use ix86_init_large_pic_reg.
	(x86_output_mi_thunk): Set PIC register to %r11.  Call
	ix86_init_large_pic_reg to initialize PIC register.

gcc/testsuite/

2014-11-12  H.J. Lu  <hongjiu.lu@intel.com>

	PR target/63815
	* g++.dg/other/pr63815.C: New test.

diff --git a/gcc/config/i386/i386.c b/gcc/config/i386/i386.c
index 2866900..0bd3ff3 100644
--- a/gcc/config/i386/i386.c
+++ b/gcc/config/i386/i386.c
@@ -6195,6 +6195,27 @@ ix86_use_pseudo_pic_reg (void)
   return true;
 }
 
+/* Initialize large model PIC register.  */
+
+static void
+ix86_init_large_pic_reg (unsigned int tmp_regno)
+{
+  rtx_code_label *label;
+  rtx tmp_reg;
+
+  gcc_assert (Pmode == DImode);
+  label = gen_label_rtx ();
+  emit_label (label);
+  LABEL_PRESERVE_P (label) = 1;
+  tmp_reg = gen_rtx_REG (Pmode, tmp_regno);
+  gcc_assert (REGNO (pic_offset_table_rtx) != tmp_regno);
+  emit_insn (gen_set_rip_rex64 (pic_offset_table_rtx,
+				label));
+  emit_insn (gen_set_got_offset_rex64 (tmp_reg, label));
+  emit_insn (ix86_gen_add3 (pic_offset_table_rtx,
+			    pic_offset_table_rtx, tmp_reg));
+}
+
 /* Create and initialize PIC register if required.  */
 static void
 ix86_init_pic_reg (void)
@@ -6210,22 +6231,7 @@ ix86_init_pic_reg (void)
   if (TARGET_64BIT)
     {
       if (ix86_cmodel == CM_LARGE_PIC)
-	{
-	  rtx_code_label *label;
-	  rtx tmp_reg;
-
-	  gcc_assert (Pmode == DImode);
-	  label = gen_label_rtx ();
-	  emit_label (label);
-	  LABEL_PRESERVE_P (label) = 1;
-	  tmp_reg = gen_rtx_REG (Pmode, R11_REG);
-	  gcc_assert (REGNO (pic_offset_table_rtx) != REGNO (tmp_reg));
-	  emit_insn (gen_set_rip_rex64 (pic_offset_table_rtx,
-					label));
-	  emit_insn (gen_set_got_offset_rex64 (tmp_reg, label));
-	  emit_insn (ix86_gen_add3 (pic_offset_table_rtx,
-				    pic_offset_table_rtx, tmp_reg));
-	}
+	ix86_init_large_pic_reg (R11_REG);
       else
 	emit_insn (gen_set_got_rex64 (pic_offset_table_rtx));
     }
@@ -42686,8 +42692,12 @@ x86_output_mi_thunk (FILE *file, tree, HOST_WIDE_INT delta,
   else
     {
       if (ix86_cmodel == CM_LARGE_PIC && SYMBOLIC_CONST (fnaddr))
-	fnaddr = legitimize_pic_address (fnaddr,
-					 gen_rtx_REG (Pmode, tmp_regno));
+	{
+	  SET_REGNO (pic_offset_table_rtx, R11_REG);
+	  ix86_init_large_pic_reg (tmp_regno);
+	  fnaddr = legitimize_pic_address (fnaddr,
+					   gen_rtx_REG (Pmode, tmp_regno));
+	}
 
       if (!sibcall_insn_operand (fnaddr, word_mode))
 	{
diff --git a/gcc/testsuite/g++.dg/other/pr63815.C b/gcc/testsuite/g++.dg/other/pr63815.C
new file mode 100644
index 0000000..fce6226
--- /dev/null
+++ b/gcc/testsuite/g++.dg/other/pr63815.C
@@ -0,0 +1,50 @@
+// PR target/63815
+// { dg-do run { target { { i?86-*-linux* x86_64-*-linux* } && lp64 } } }
+// { dg-options "-mcmodel=large" }
+// { dg-additional-options "-fpic" { target fpic } }
+
+struct ICCStringClass
+{
+  virtual int CreateString (int) = 0;
+};
+
+struct AGSCCDynamicObject
+{
+  virtual void Unserialize () = 0;
+};
+
+struct ScriptString:AGSCCDynamicObject, ICCStringClass
+{
+  virtual int CreateString (int);
+  virtual void Unserialize ();
+};
+
+int
+__attribute__ ((noinline))
+CreateNewScriptString (int fromText, bool reAllocate = true)
+{
+  return fromText;
+}
+
+int
+__attribute__ ((noinline))
+ScriptString::CreateString (int fromText)
+{
+  return CreateNewScriptString (fromText);
+}
+
+void
+__attribute__ ((noinline))
+ScriptString::Unserialize ()
+{
+}
+
+int
+main ()
+{
+  ICCStringClass *x = new ScriptString;
+
+  if (x->CreateString (1) != 1)
+    __builtin_abort ();
+  return 0;
+}

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

* Re: PATCH: PR target/63815: [5 Regression] g++.dg/other/pr53811.C fails with -mcmodel=large -fpic
  2014-11-12 20:47 PATCH: PR target/63815: [5 Regression] g++.dg/other/pr53811.C fails with -mcmodel=large -fpic H.J. Lu
@ 2014-11-12 20:59 ` Uros Bizjak
  2014-11-12 21:11 ` Jakub Jelinek
  1 sibling, 0 replies; 9+ messages in thread
From: Uros Bizjak @ 2014-11-12 20:59 UTC (permalink / raw)
  To: H.J. Lu; +Cc: gcc-patches, Jakub Jelinek

On Wed, Nov 12, 2014 at 9:43 PM, H.J. Lu <hongjiu.lu@intel.com> wrote:
> Hi,
>
> r216154 exposed an x86 backend bug.  For large PIC mode thunk, there
> are
>
>       if (ix86_cmodel == CM_LARGE_PIC && SYMBOLIC_CONST (fnaddr))
>         fnaddr = legitimize_pic_address (fnaddr,
>                                          gen_rtx_REG (Pmode, tmp_regno));
>
> and legitimize_pic_address does:
>
>       if (reg != 0)
>         {
>           new_rtx = expand_simple_binop (Pmode, PLUS, reg, pic_offset_table_rtx,
>                                          tmpreg, 1, OPTAB_DIRECT);
>           new_rtx = reg;
>         }
>
> However, pic_offset_table_rtx was never initialized.  Befor r216154,
> we generated thunk with random value in hardcoded PIC register.  After
> r216154, compiler crashes.  This patch sets PIC register to %r11 and
> initialize it.  Tested on Linux/x86-64.  OK for trunk and backport
> to 4.8/4.9 branches?
>
> 2014-11-12  H.J. Lu  <hongjiu.lu@intel.com>
>
>         PR target/63815
>         * config/i386/i386.c (ix86_init_large_pic_reg): New.  Extracted
>         from ...
>         (ix86_init_pic_reg): Here.  Use ix86_init_large_pic_reg.
>         (x86_output_mi_thunk): Set PIC register to %r11.  Call
>         ix86_init_large_pic_reg to initialize PIC register.
>
> gcc/testsuite/
>
> 2014-11-12  H.J. Lu  <hongjiu.lu@intel.com>
>
>         PR target/63815
>         * g++.dg/other/pr63815.C: New test.

OK for mainline with a short comment addition.

Backports to release branches need RM's approval.

Thanks,
Uros.

>        if (ix86_cmodel == CM_LARGE_PIC && SYMBOLIC_CONST (fnaddr))
> -       fnaddr = legitimize_pic_address (fnaddr,
> -                                        gen_rtx_REG (Pmode, tmp_regno));
> +       {

Please put a short comment why we need hard reg here.

> +         SET_REGNO (pic_offset_table_rtx, R11_REG);

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

* Re: PATCH: PR target/63815: [5 Regression] g++.dg/other/pr53811.C fails with -mcmodel=large -fpic
  2014-11-12 20:47 PATCH: PR target/63815: [5 Regression] g++.dg/other/pr53811.C fails with -mcmodel=large -fpic H.J. Lu
  2014-11-12 20:59 ` Uros Bizjak
@ 2014-11-12 21:11 ` Jakub Jelinek
  2014-11-12 21:18   ` H.J. Lu
  1 sibling, 1 reply; 9+ messages in thread
From: Jakub Jelinek @ 2014-11-12 21:11 UTC (permalink / raw)
  To: H.J. Lu; +Cc: gcc-patches, Uros Bizjak

On Wed, Nov 12, 2014 at 12:43:17PM -0800, H.J. Lu wrote:
> @@ -42686,8 +42692,12 @@ x86_output_mi_thunk (FILE *file, tree, HOST_WIDE_INT delta,
>    else
>      {
>        if (ix86_cmodel == CM_LARGE_PIC && SYMBOLIC_CONST (fnaddr))
> -	fnaddr = legitimize_pic_address (fnaddr,
> -					 gen_rtx_REG (Pmode, tmp_regno));
> +	{
> +	  SET_REGNO (pic_offset_table_rtx, R11_REG);

If pic_offset_table_rtx has never been initialized, how you can use
SET_REGNO on it?  Shouldn't that be pic_offset_table_rtx = gen_raw_REG (Pmode, R11_REG);
or similar?  Or is it initialized from some earlier function emitted, just
with a different reg?

	Jakub

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

* Re: PATCH: PR target/63815: [5 Regression] g++.dg/other/pr53811.C fails with -mcmodel=large -fpic
  2014-11-12 21:11 ` Jakub Jelinek
@ 2014-11-12 21:18   ` H.J. Lu
  2014-11-12 21:40     ` Jakub Jelinek
  0 siblings, 1 reply; 9+ messages in thread
From: H.J. Lu @ 2014-11-12 21:18 UTC (permalink / raw)
  To: Jakub Jelinek; +Cc: GCC Patches, Uros Bizjak

On Wed, Nov 12, 2014 at 1:02 PM, Jakub Jelinek <jakub@redhat.com> wrote:
> On Wed, Nov 12, 2014 at 12:43:17PM -0800, H.J. Lu wrote:
>> @@ -42686,8 +42692,12 @@ x86_output_mi_thunk (FILE *file, tree, HOST_WIDE_INT delta,
>>    else
>>      {
>>        if (ix86_cmodel == CM_LARGE_PIC && SYMBOLIC_CONST (fnaddr))
>> -     fnaddr = legitimize_pic_address (fnaddr,
>> -                                      gen_rtx_REG (Pmode, tmp_regno));
>> +     {
>> +       SET_REGNO (pic_offset_table_rtx, R11_REG);
>
> If pic_offset_table_rtx has never been initialized, how you can use

It is a pseudo PIC register which is uninitialized:

(gdb) call debug_rtx (this_target_rtl->x_pic_offset_table_rtx)
(reg:DI 89)
(gdb)

> SET_REGNO on it?  Shouldn't that be pic_offset_table_rtx = gen_raw_REG (Pmode, R11_REG);

I added the following comments and am checking it into trunk:

          // CM_LARGE_PIC always uses pseudo PIC register which is
          // uninitialized.  Since FUNCTION is local and calling it
          // doesn't go through PLT, we use scratch register %r11 as
          // PIC register and initialize it here.


-- 
H.J.
-----

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

* Re: PATCH: PR target/63815: [5 Regression] g++.dg/other/pr53811.C fails with -mcmodel=large -fpic
  2014-11-12 21:18   ` H.J. Lu
@ 2014-11-12 21:40     ` Jakub Jelinek
  2014-11-12 22:00       ` H.J. Lu
  0 siblings, 1 reply; 9+ messages in thread
From: Jakub Jelinek @ 2014-11-12 21:40 UTC (permalink / raw)
  To: H.J. Lu; +Cc: GCC Patches, Uros Bizjak

On Wed, Nov 12, 2014 at 01:11:40PM -0800, H.J. Lu wrote:
> On Wed, Nov 12, 2014 at 1:02 PM, Jakub Jelinek <jakub@redhat.com> wrote:
> > On Wed, Nov 12, 2014 at 12:43:17PM -0800, H.J. Lu wrote:
> >> @@ -42686,8 +42692,12 @@ x86_output_mi_thunk (FILE *file, tree, HOST_WIDE_INT delta,
> >>    else
> >>      {
> >>        if (ix86_cmodel == CM_LARGE_PIC && SYMBOLIC_CONST (fnaddr))
> >> -     fnaddr = legitimize_pic_address (fnaddr,
> >> -                                      gen_rtx_REG (Pmode, tmp_regno));
> >> +     {
> >> +       SET_REGNO (pic_offset_table_rtx, R11_REG);
> >
> > If pic_offset_table_rtx has never been initialized, how you can use
> 
> It is a pseudo PIC register which is uninitialized:
> 
> (gdb) call debug_rtx (this_target_rtl->x_pic_offset_table_rtx)
> (reg:DI 89)
> (gdb)
> 
> > SET_REGNO on it?  Shouldn't that be pic_offset_table_rtx = gen_raw_REG (Pmode, R11_REG);
> 
> I added the following comments and am checking it into trunk:
> 
>           // CM_LARGE_PIC always uses pseudo PIC register which is
>           // uninitialized.  Since FUNCTION is local and calling it
>           // doesn't go through PLT, we use scratch register %r11 as
>           // PIC register and initialize it here.

From what I can see, it probably in this case should be always non-NULL,
as it is initialized in init_emit_regs:
5847	  if ((unsigned) PIC_OFFSET_TABLE_REGNUM != INVALID_REGNUM)
5848	    pic_offset_table_rtx = gen_raw_REG (Pmode, PIC_OFFSET_TABLE_REGNUM);
5849	  else
5850	    pic_offset_table_rtx = NULL_RTX;
and as pic_offset_table_rtx is NULL there, PIC_OFFSET_TABLE_REGNUM is
BX_REG.
Later on, assign_params will change that to:
3684	  if (targetm.use_pseudo_pic_reg ())
3685	    pic_offset_table_rtx = gen_reg_rtx (Pmode);
and do_reload, after reload temporarily changing the REGNO, will overwrite
it again to the chosen pseudo:
5470	  if (pic_offset_table_regno != INVALID_REGNUM)
5471	    pic_offset_table_rtx = gen_rtx_REG (Pmode, pic_offset_table_regno);

So, all in all, SET_REGNO sounds inappropriate to me, everywhere else
gen_raw_REG or gen_rtx_REG is used instead, so IMNSHO you should do the
same.

	Jakub

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

* Re: PATCH: PR target/63815: [5 Regression] g++.dg/other/pr53811.C fails with -mcmodel=large -fpic
  2014-11-12 21:40     ` Jakub Jelinek
@ 2014-11-12 22:00       ` H.J. Lu
  2014-11-12 22:05         ` Jakub Jelinek
  0 siblings, 1 reply; 9+ messages in thread
From: H.J. Lu @ 2014-11-12 22:00 UTC (permalink / raw)
  To: Jakub Jelinek; +Cc: GCC Patches, Uros Bizjak

On Wed, Nov 12, 2014 at 1:39 PM, Jakub Jelinek <jakub@redhat.com> wrote:
> On Wed, Nov 12, 2014 at 01:11:40PM -0800, H.J. Lu wrote:
>> On Wed, Nov 12, 2014 at 1:02 PM, Jakub Jelinek <jakub@redhat.com> wrote:
>> > On Wed, Nov 12, 2014 at 12:43:17PM -0800, H.J. Lu wrote:
>> >> @@ -42686,8 +42692,12 @@ x86_output_mi_thunk (FILE *file, tree, HOST_WIDE_INT delta,
>> >>    else
>> >>      {
>> >>        if (ix86_cmodel == CM_LARGE_PIC && SYMBOLIC_CONST (fnaddr))
>> >> -     fnaddr = legitimize_pic_address (fnaddr,
>> >> -                                      gen_rtx_REG (Pmode, tmp_regno));
>> >> +     {
>> >> +       SET_REGNO (pic_offset_table_rtx, R11_REG);
>> >
>> > If pic_offset_table_rtx has never been initialized, how you can use
>>
>> It is a pseudo PIC register which is uninitialized:
>>
>> (gdb) call debug_rtx (this_target_rtl->x_pic_offset_table_rtx)
>> (reg:DI 89)
>> (gdb)
>>
>> > SET_REGNO on it?  Shouldn't that be pic_offset_table_rtx = gen_raw_REG (Pmode, R11_REG);
>>
>> I added the following comments and am checking it into trunk:
>>
>>           // CM_LARGE_PIC always uses pseudo PIC register which is
>>           // uninitialized.  Since FUNCTION is local and calling it
>>           // doesn't go through PLT, we use scratch register %r11 as
>>           // PIC register and initialize it here.
>
> From what I can see, it probably in this case should be always non-NULL,
> as it is initialized in init_emit_regs:
> 5847      if ((unsigned) PIC_OFFSET_TABLE_REGNUM != INVALID_REGNUM)
> 5848        pic_offset_table_rtx = gen_raw_REG (Pmode, PIC_OFFSET_TABLE_REGNUM);
> 5849      else
> 5850        pic_offset_table_rtx = NULL_RTX;
> and as pic_offset_table_rtx is NULL there, PIC_OFFSET_TABLE_REGNUM is
> BX_REG.
> Later on, assign_params will change that to:
> 3684      if (targetm.use_pseudo_pic_reg ())
> 3685        pic_offset_table_rtx = gen_reg_rtx (Pmode);
> and do_reload, after reload temporarily changing the REGNO, will overwrite
> it again to the chosen pseudo:
> 5470      if (pic_offset_table_regno != INVALID_REGNUM)
> 5471        pic_offset_table_rtx = gen_rtx_REG (Pmode, pic_offset_table_regno);
>
> So, all in all, SET_REGNO sounds inappropriate to me, everywhere else
> gen_raw_REG or gen_rtx_REG is used instead, so IMNSHO you should do the
> same.
>
>         Jakub

It makes sense.  I checked in the following patch.

Thanks.

-- 
H.J.
---
Index: ChangeLog
===================================================================
--- ChangeLog (revision 217445)
+++ ChangeLog (working copy)
@@ -1,5 +1,10 @@
 2014-11-12  H.J. Lu  <hongjiu.lu@intel.com>

+ * config/i386/i386.c (x86_output_mi_thunk): Set gen_rtx_REG to
+ set pic_offset_table_rtx.
+
+2014-11-12  H.J. Lu  <hongjiu.lu@intel.com>
+
  PR target/63815
  * config/i386/i386.c (ix86_init_large_pic_reg): New.  Extracted
  from ...
Index: config/i386/i386.c
===================================================================
--- config/i386/i386.c (revision 217445)
+++ config/i386/i386.c (working copy)
@@ -42697,7 +42697,7 @@ x86_output_mi_thunk (FILE *file, tree, H
   // uninitialized.  Since FUNCTION is local and calling it
   // doesn't go through PLT, we use scratch register %r11 as
   // PIC register and initialize it here.
-  SET_REGNO (pic_offset_table_rtx, R11_REG);
+  pic_offset_table_rtx = gen_rtx_REG (Pmode, R11_REG);
   ix86_init_large_pic_reg (tmp_regno);
   fnaddr = legitimize_pic_address (fnaddr,
    gen_rtx_REG (Pmode, tmp_regno));

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

* Re: PATCH: PR target/63815: [5 Regression] g++.dg/other/pr53811.C fails with -mcmodel=large -fpic
  2014-11-12 22:00       ` H.J. Lu
@ 2014-11-12 22:05         ` Jakub Jelinek
  2014-11-12 22:06           ` Uros Bizjak
  2014-11-12 22:09           ` H.J. Lu
  0 siblings, 2 replies; 9+ messages in thread
From: Jakub Jelinek @ 2014-11-12 22:05 UTC (permalink / raw)
  To: H.J. Lu; +Cc: GCC Patches, Uros Bizjak

On Wed, Nov 12, 2014 at 01:51:01PM -0800, H.J. Lu wrote:
> > So, all in all, SET_REGNO sounds inappropriate to me, everywhere else
> > gen_raw_REG or gen_rtx_REG is used instead, so IMNSHO you should do the
> > same.
> >
> >         Jakub
> 
> It makes sense.  I checked in the following patch.

gen_raw_REG would be better, the difference is that gen_rtx_REG might
in theory return you pic_offset_table_rtx again.

Anyway, I don't think anything is needed for release branches,
pic_offset_table_rtx should be initialized there just once and not changed
afterwards.

	Jakub

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

* Re: PATCH: PR target/63815: [5 Regression] g++.dg/other/pr53811.C fails with -mcmodel=large -fpic
  2014-11-12 22:05         ` Jakub Jelinek
@ 2014-11-12 22:06           ` Uros Bizjak
  2014-11-12 22:09           ` H.J. Lu
  1 sibling, 0 replies; 9+ messages in thread
From: Uros Bizjak @ 2014-11-12 22:06 UTC (permalink / raw)
  To: Jakub Jelinek; +Cc: H.J. Lu, GCC Patches

On Wed, Nov 12, 2014 at 11:00 PM, Jakub Jelinek <jakub@redhat.com> wrote:
> On Wed, Nov 12, 2014 at 01:51:01PM -0800, H.J. Lu wrote:
>> > So, all in all, SET_REGNO sounds inappropriate to me, everywhere else
>> > gen_raw_REG or gen_rtx_REG is used instead, so IMNSHO you should do the
>> > same.
>> >
>> >         Jakub
>>
>> It makes sense.  I checked in the following patch.
>
> gen_raw_REG would be better, the difference is that gen_rtx_REG might
> in theory return you pic_offset_table_rtx again.
>
> Anyway, I don't think anything is needed for release branches,
> pic_offset_table_rtx should be initialized there just once and not changed
> afterwards.

Please see the test(s) from [1]. The executable segfaults when
compiled with gcc-4.9.

[1] https://gcc.gnu.org/bugzilla/show_bug.cgi?id=63815#c10

Uros.

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

* Re: PATCH: PR target/63815: [5 Regression] g++.dg/other/pr53811.C fails with -mcmodel=large -fpic
  2014-11-12 22:05         ` Jakub Jelinek
  2014-11-12 22:06           ` Uros Bizjak
@ 2014-11-12 22:09           ` H.J. Lu
  1 sibling, 0 replies; 9+ messages in thread
From: H.J. Lu @ 2014-11-12 22:09 UTC (permalink / raw)
  To: Jakub Jelinek; +Cc: GCC Patches, Uros Bizjak

On Wed, Nov 12, 2014 at 2:00 PM, Jakub Jelinek <jakub@redhat.com> wrote:
> On Wed, Nov 12, 2014 at 01:51:01PM -0800, H.J. Lu wrote:
>> > So, all in all, SET_REGNO sounds inappropriate to me, everywhere else
>> > gen_raw_REG or gen_rtx_REG is used instead, so IMNSHO you should do the
>> > same.
>> >
>> >         Jakub
>>
>> It makes sense.  I checked in the following patch.
>
> gen_raw_REG would be better, the difference is that gen_rtx_REG might
> in theory return you pic_offset_table_rtx again.

The whole  x86_output_mi_thunk function uses gen_rtx_REG.  Using
gen_raw_REG is out of space.

> Anyway, I don't think anything is needed for release branches,
> pic_offset_table_rtx should be initialized there just once and not changed
> afterwards.

Try this with released GCC:

[hjl@gnu-6 tmp]$ cat foo.h
struct ICCStringClass
{
  virtual int CreateString (int) = 0;
};

struct AGSCCDynamicObject
{
  virtual void Unserialize () = 0;
};


struct ScriptString:AGSCCDynamicObject, ICCStringClass
{
  virtual int CreateString (int);
  virtual void Unserialize ();
};
[hjl@gnu-6 tmp]$ cat foo1.cc
#include "foo.h"

int
__attribute__ ((noinline))
CreateNewScriptString (int fromText, bool reAllocate = true)
{
  return fromText;
}

int
__attribute__ ((noinline))
ScriptString::CreateString (int fromText)
{
  return CreateNewScriptString (fromText);
}

void
__attribute__ ((noinline))
ScriptString::Unserialize ()
{
}
[hjl@gnu-6 tmp]$ cat foo2.cc
#include "foo.h"

int
main ()
{
  ICCStringClass *x = new ScriptString;
  if (x->CreateString (1) != 1)
    __builtin_abort ();
  return 0;
}
[hjl@gnu-6 tmp]$ g++ -shared -fpic -O2 -mcmodel=large -o libfoo.so foo1.cc
[hjl@gnu-6 tmp]$ g++ foo2.cc ./libfoo.so
[hjl@gnu-6 tmp]$ ./a.out
Segmentation fault
[hjl@gnu-6 tmp]$




-- 
H.J.

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

end of thread, other threads:[~2014-11-12 22:06 UTC | newest]

Thread overview: 9+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2014-11-12 20:47 PATCH: PR target/63815: [5 Regression] g++.dg/other/pr53811.C fails with -mcmodel=large -fpic H.J. Lu
2014-11-12 20:59 ` Uros Bizjak
2014-11-12 21:11 ` Jakub Jelinek
2014-11-12 21:18   ` H.J. Lu
2014-11-12 21:40     ` Jakub Jelinek
2014-11-12 22:00       ` H.J. Lu
2014-11-12 22:05         ` Jakub Jelinek
2014-11-12 22:06           ` Uros Bizjak
2014-11-12 22:09           ` H.J. Lu

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).