diff mbox

Fix PR57451 (Incorrect debug ranges emitted for -freorder-blocks-and-partition -g)

Message ID CAAe5K+V2-qGQBbHmNXmzfDePk96xni5tgUMw2G0fRKa8EE01pA@mail.gmail.com
State New
Headers show

Commit Message

Teresa Johnson Aug. 20, 2013, 5:21 a.m. UTC
On Mon, Aug 19, 2013 at 5:01 PM, Teresa Johnson <tejohnson@google.com> wrote:
> On Mon, Aug 19, 2013 at 11:44 AM, Jeff Law <law@redhat.com> wrote:
>> On 08/19/2013 12:34 PM, Teresa Johnson wrote:
>>>
>>> Ping.
>>> Thanks,
>>> Teresa
>>>
>>> On Sun, Aug 11, 2013 at 9:35 PM, Teresa Johnson <tejohnson@google.com>
>>> wrote:
>>>>
>>>> This patch fixes PR rtl-optimizations/57451 by preventing scopes and
>>>> therefore lexical blocks from crossing split section boundaries.
>>>> This will prevent debug info generation from using DW_AT_low_pc/high_pc
>>>> pairs across the section boundary.
>>>>
>>>> Bootstrapped and tested on x86_64-unknown-linux-gnu. With this patch,
>>>> a profilebootstrap with -freorder-blocks-and-partition force-enabled
>>>> also passes. Ok for trunk?
>>>>
>>>> Thanks,
>>>> Teresa
>>>>
>>>> 2013-08-11  Teresa Johnson  <tejohnson@google.com>
>>>>
>>>>          PR rtl-optimizations/57451
>>>>          * final.c (reemit_insn_block_notes): Prevent lexical blocks
>>>>          from crossing split section boundaries.
>>
>> This is fine.  Is there any way you can turn the steps mentioned in 57451
>> into a dejagnu testcase?  Clearly it would only work in a native
>> configuration due to the need to run the instrumented program.
>
> Since this was a compile-time failure (link-time actually), I could
> just take the existing test case mentioned there and turn it into a
> "-g -freorder-blocks-and-partition" test (probably by cloning as a new
> test case the g++.dg/tree-prof testsuite directory). Does that seem
> reasonable?

Here is the new patch with that test added. Passes bootstrap and
regression tests on x86_64-unknown-linux-gnu. Ok for trunk?
Thanks,
Teresa

2013-08-19  Teresa Johnson  <tejohnson@google.com>

        PR rtl-optimizations/57451
        * final.c (reemit_insn_block_notes): Prevent lexical blocks
        from crossing split section boundaries.

        * testsuite/g++.dg/tree-prof/pr57451.C: New test.


>
> Thanks,
> Teresa
>
>>
>> jeff
>>
>
>
>
> --
> Teresa Johnson | Software Engineer | tejohnson@google.com | 408-460-2413

Comments

Jeff Law Aug. 20, 2013, 6:48 a.m. UTC | #1
On 08/19/2013 11:21 PM, Teresa Johnson wrote:
> On Mon, Aug 19, 2013 at 5:01 PM, Teresa Johnson <tejohnson@google.com> wrote:
>> On Mon, Aug 19, 2013 at 11:44 AM, Jeff Law <law@redhat.com> wrote:
>>> On 08/19/2013 12:34 PM, Teresa Johnson wrote:
>>>>
>>>> Ping.
>>>> Thanks,
>>>> Teresa
>>>>
>>>> On Sun, Aug 11, 2013 at 9:35 PM, Teresa Johnson <tejohnson@google.com>
>>>> wrote:
>>>>>
>>>>> This patch fixes PR rtl-optimizations/57451 by preventing scopes and
>>>>> therefore lexical blocks from crossing split section boundaries.
>>>>> This will prevent debug info generation from using DW_AT_low_pc/high_pc
>>>>> pairs across the section boundary.
>>>>>
>>>>> Bootstrapped and tested on x86_64-unknown-linux-gnu. With this patch,
>>>>> a profilebootstrap with -freorder-blocks-and-partition force-enabled
>>>>> also passes. Ok for trunk?
>>>>>
>>>>> Thanks,
>>>>> Teresa
>>>>>
>>>>> 2013-08-11  Teresa Johnson  <tejohnson@google.com>
>>>>>
>>>>>           PR rtl-optimizations/57451
>>>>>           * final.c (reemit_insn_block_notes): Prevent lexical blocks
>>>>>           from crossing split section boundaries.
>>>
>>> This is fine.  Is there any way you can turn the steps mentioned in 57451
>>> into a dejagnu testcase?  Clearly it would only work in a native
>>> configuration due to the need to run the instrumented program.
>>
>> Since this was a compile-time failure (link-time actually), I could
>> just take the existing test case mentioned there and turn it into a
>> "-g -freorder-blocks-and-partition" test (probably by cloning as a new
>> test case the g++.dg/tree-prof testsuite directory). Does that seem
>> reasonable?
>
> Here is the new patch with that test added. Passes bootstrap and
> regression tests on x86_64-unknown-linux-gnu. Ok for trunk?
> Thanks,
> Teresa
>
> 2013-08-19  Teresa Johnson  <tejohnson@google.com>
>
>          PR rtl-optimizations/57451
>          * final.c (reemit_insn_block_notes): Prevent lexical blocks
>          from crossing split section boundaries.
>
>          * testsuite/g++.dg/tree-prof/pr57451.C: New test.
Yea, that's great.  Thanks for pulling together the testcase.

Please install.

Thanks,
Jeff
Steve Ellcey Aug. 22, 2013, 11:49 p.m. UTC | #2
On Mon, 2013-08-19 at 22:21 -0700, Teresa Johnson wrote:

> 2013-08-19  Teresa Johnson  <tejohnson@google.com>
> 
>         PR rtl-optimizations/57451
>         * final.c (reemit_insn_block_notes): Prevent lexical blocks
>         from crossing split section boundaries.
> 
>         * testsuite/g++.dg/tree-prof/pr57451.C: New test.

Teresa,

This patch is causing a problem in my mips compiler.  I am creating a
cross-toolchain running on x86 targeting mips-mti-linux-gnu and if I
compile glibc using a GCC that has this patch all of the programs I
compile go into an infinite loop when run under the qemu simulator.  I
think it is the dynamic linker that is getting miscompiled, but I
haven't tracked down the exact nature of the miscompilation yet.  Has
anyone else reported any problems with it?  I am adding Richard
Sandiford to the email list since he is a mips expert and also might
have some insight to using next_active_insn vs. next_real_insn vs.
next_insn, though it doesn't look like you have changed what is used
here.

Steve Ellcey
sellcey@mips.com
Kaz Kojima Aug. 22, 2013, 11:59 p.m. UTC | #3
Steve Ellcey <sellcey@mips.com> wrote:
> On Mon, 2013-08-19 at 22:21 -0700, Teresa Johnson wrote:
> 
>> 2013-08-19  Teresa Johnson  <tejohnson@google.com>
>> 
>>         PR rtl-optimizations/57451
>>         * final.c (reemit_insn_block_notes): Prevent lexical blocks
>>         from crossing split section boundaries.
>> 
>>         * testsuite/g++.dg/tree-prof/pr57451.C: New test.
> 
> Teresa,
> 
> This patch is causing a problem in my mips compiler.  I am creating a
> cross-toolchain running on x86 targeting mips-mti-linux-gnu and if I
> compile glibc using a GCC that has this patch all of the programs I
> compile go into an infinite loop when run under the qemu simulator.  I
> think it is the dynamic linker that is getting miscompiled, but I
> haven't tracked down the exact nature of the miscompilation yet.  Has
> anyone else reported any problems with it?  I am adding Richard
> Sandiford to the email list since he is a mips expert and also might
> have some insight to using next_active_insn vs. next_real_insn vs.
> next_insn, though it doesn't look like you have changed what is used
> here.

Similar on SH.  I've just filed rtl-optimization/58220.

Regards,
	kaz
diff mbox

Patch

Index: final.c
===================================================================
--- final.c     (revision 201644)
+++ final.c     (working copy)
@@ -1650,12 +1650,26 @@  reemit_insn_block_notes (void)
   rtx insn, note;

   insn = get_insns ();
-  if (!active_insn_p (insn))
-    insn = next_active_insn (insn);
-  for (; insn; insn = next_active_insn (insn))
+  for (; insn; insn = next_insn (insn))
     {
       tree this_block;

+      /* Prevent lexical blocks from straddling section boundaries.  */
+      if (NOTE_P (insn) && NOTE_KIND (insn) == NOTE_INSN_SWITCH_TEXT_SECTIONS)
+        {
+          for (tree s = cur_block; s != DECL_INITIAL (cfun->decl);
+               s = BLOCK_SUPERCONTEXT (s))
+            {
+              rtx note = emit_note_before (NOTE_INSN_BLOCK_END, insn);
+              NOTE_BLOCK (note) = s;
+              note = emit_note_after (NOTE_INSN_BLOCK_BEG, insn);
+              NOTE_BLOCK (note) = s;
+            }
+        }
+
+      if (!active_insn_p (insn))
+        continue;
+
       /* Avoid putting scope notes between jump table and its label.  */
       if (JUMP_TABLE_DATA_P (insn))
        continue;
Index: testsuite/g++.dg/tree-prof/pr57451.C
===================================================================
--- testsuite/g++.dg/tree-prof/pr57451.C        (revision 0)
+++ testsuite/g++.dg/tree-prof/pr57451.C        (revision 0)
@@ -0,0 +1,27 @@ 
+// { dg-do run }
+// { dg-require-effective-target freorder }
+// { dg-options "-O2 -freorder-blocks-and-partition -g" }
+
+extern "C" void abort (void);
+struct MyException {};
+struct Data {
+    int nr;
+    Data() : nr(66) {}
+};
+Data __attribute__((noinline,noclone)) getData(int i)
+{
+  if (i) throw MyException();
+  Data data;
+  data.nr = i;
+  return data;
+}
+int main(int, char **)
+{
+  Data data;
+  try {
+      data = getData(1);
+  } catch (MyException& e) {
+      if (data.nr != 66)
+       abort ();
+  }
+}