[PATCH] DWARF: remove pessimistic DWARF version checks for imported entities

[PATCH] DWARF: remove pessimistic DWARF version checks for imported entities

[Pierre-Marie de Rodat]

Hello,

A check in dwarf2out_imported_module_or_decl prevents valid strict
DWARF2 constructs such as DW_TAG_imported_declaration from being emitted
in dwarf2out_imported_module_or_decl_1.

The latter already protects the emission of newer DWARF tags with
appropriate checks, so the one in the former is redundant and
pessimistic.  This function is already called from places like
process_scope_var, which are not protected anyway.

This patch removes the check in dwarf2out_imported_module_or_decl so
that tags like DW_TAG_imported_declaration are emitted even in strict
DWARF2 mode.

Bootstrapped and regtested on x86_64-linux, no regression.  I also
checked that the new testcase fails on mainline.  Ok to commit?
Thank you in advance!

gcc/

	* dwarf2out.c (dwarf2out_imported_module_or_decl): Remove
	pessimistic DWARF version check.

gcc/testsuite/

	* gnat.dg/debug7.adb, gnat.dg/debug7.ads: New testcase.
---
 gcc/dwarf2out.c                  |  3 ---
 gcc/testsuite/gnat.dg/debug7.adb | 10 ++++++++++
 gcc/testsuite/gnat.dg/debug7.ads |  4 ++++
 3 files changed, 14 insertions(+), 3 deletions(-)
 create mode 100644 gcc/testsuite/gnat.dg/debug7.adb
 create mode 100644 gcc/testsuite/gnat.dg/debug7.ads

diff --git a/gcc/dwarf2out.c b/gcc/dwarf2out.c
index 0fdab9a..7bc0378 100644
--- a/gcc/dwarf2out.c
+++ b/gcc/dwarf2out.c
@@ -24034,9 +24034,6 @@ dwarf2out_imported_module_or_decl (tree decl, tree name, tree context,
       && !should_emit_struct_debug (context, DINFO_USAGE_DIR_USE))
     return;
 
-  if (!(dwarf_version >= 3 || !dwarf_strict))
-    return;
-
   scope_die = get_context_die (context);
 
   if (child)
diff --git a/gcc/testsuite/gnat.dg/debug7.adb b/gcc/testsuite/gnat.dg/debug7.adb
new file mode 100644
index 0000000..98230ba
--- /dev/null
+++ b/gcc/testsuite/gnat.dg/debug7.adb
@@ -0,0 +1,10 @@
+-- { dg-do compile }
+-- { dg-options "-cargs -g -gdwarf-2 -gstrict-dwarf -dA" }
+-- { dg-final { scan-assembler "DW_TAG_imported_decl" } }
+
+package body Debug7 is
+   function Next (I : Integer) return Integer is
+   begin
+      return I + 1;
+   end Next;
+end Debug7;
diff --git a/gcc/testsuite/gnat.dg/debug7.ads b/gcc/testsuite/gnat.dg/debug7.ads
new file mode 100644
index 0000000..047d4a6
--- /dev/null
+++ b/gcc/testsuite/gnat.dg/debug7.ads
@@ -0,0 +1,4 @@
+package Debug7 is
+   function Next (I : Integer) return Integer;
+   function Renamed_Next (I : Integer) return Integer renames Next;
+end Debug7;
-- 
2.9.3

Re: [PATCH] DWARF: remove pessimistic DWARF version checks for imported entities

[Jeff Law]

On 08/18/2016 03:33 AM, Pierre-Marie de Rodat wrote:
> Hello,
>
> A check in dwarf2out_imported_module_or_decl prevents valid strict
> DWARF2 constructs such as DW_TAG_imported_declaration from being emitted
> in dwarf2out_imported_module_or_decl_1.
>
> The latter already protects the emission of newer DWARF tags with
> appropriate checks, so the one in the former is redundant and
> pessimistic.  This function is already called from places like
> process_scope_var, which are not protected anyway.
>
> This patch removes the check in dwarf2out_imported_module_or_decl so
> that tags like DW_TAG_imported_declaration are emitted even in strict
> DWARF2 mode.
>
> Bootstrapped and regtested on x86_64-linux, no regression.  I also
> checked that the new testcase fails on mainline.  Ok to commit?
> Thank you in advance!
>
> gcc/
>
> 	* dwarf2out.c (dwarf2out_imported_module_or_decl): Remove
> 	pessimistic DWARF version check.
>
> gcc/testsuite/
>
> 	* gnat.dg/debug7.adb, gnat.dg/debug7.ads: New testcase.

Jakub -- you want to take a looksie at this one?  Jason is on PTO for 
the next couple weeks.  I'm not well versed enough to verify that 
d_i_m_o_d_1 has all the right checks or not.


Jeff

[PATCH, PING] DWARF: remove pessimistic DWARF version checks for imported entities

[Pierre-Marie de Rodat]

Hello,

On 08/18/2016 10:12 PM, Jeff Law wrote:
> Jakub -- you want to take a looksie at this one?  Jason is on PTO for
> the next couple weeks.  I'm not well versed enough to verify that
> d_i_m_o_d_1 has all the right checks or not.

Ping for the patch submitted at 
<https://gcc.gnu.org/ml/gcc-patches/2016-08/msg01315.html>. Thank you in 
advance!

-- 
Pierre-Marie de Rodat

Re: [PATCH] DWARF: remove pessimistic DWARF version checks for imported entities

[Jason Merrill]

On Thu, Aug 18, 2016 at 5:33 AM, Pierre-Marie de Rodat
<derodat@adacore.com> wrote:
> A check in dwarf2out_imported_module_or_decl prevents valid strict
> DWARF2 constructs such as DW_TAG_imported_declaration from being emitted
> in dwarf2out_imported_module_or_decl_1.
>
> The latter already protects the emission of newer DWARF tags with
> appropriate checks, so the one in the former is redundant and
> pessimistic.  This function is already called from places like
> process_scope_var, which are not protected anyway.
>
> This patch removes the check in dwarf2out_imported_module_or_decl so
> that tags like DW_TAG_imported_declaration are emitted even in strict
> DWARF2 mode.
>
> Bootstrapped and regtested on x86_64-linux, no regression.

This check was added for bug 41405, a Darwin bootstrap problem.
Dominique, can you verify that the compiler still bootstraps with this
change?  OK if so.

Jason

Re: [PATCH] DWARF: remove pessimistic DWARF version checks for imported entities

[Dominique d'Humières]

> Le 28 sept. 2016 à 17:44, Jason Merrill <jason@redhat.com> a écrit :
> 
> On Thu, Aug 18, 2016 at 5:33 AM, Pierre-Marie de Rodat
> <derodat@adacore.com> wrote:
>> A check in dwarf2out_imported_module_or_decl prevents valid strict
>> DWARF2 constructs such as DW_TAG_imported_declaration from being emitted
>> in dwarf2out_imported_module_or_decl_1.
>> 
>> The latter already protects the emission of newer DWARF tags with
>> appropriate checks, so the one in the former is redundant and
>> pessimistic.  This function is already called from places like
>> process_scope_var, which are not protected anyway.
>> 
>> This patch removes the check in dwarf2out_imported_module_or_decl so
>> that tags like DW_TAG_imported_declaration are emitted even in strict
>> DWARF2 mode.
>> 
>> Bootstrapped and regtested on x86_64-linux, no regression.
> 
> This check was added for bug 41405, a Darwin bootstrap problem.
> Dominique, can you verify that the compiler still bootstraps with this
> change?  OK if so.

OK, but not before tomorrow afternoon.

Dominique

> Jason

Re: [PATCH] DWARF: remove pessimistic DWARF version checks for imported entities

[Dominique d'Humières]

I have bootstrapped revision r240599 with the patch https://gcc.gnu.org/ml/gcc-patches/2016-08/msg01315.html without any problem. However compiling the following Fortran tests

FAIL: gfortran.dg/binding_label_tests_16.f03  -g  (internal compiler error)
FAIL: gfortran.dg/module_commons_3.f90        -g  (internal compiler error)
FAIL: gfortran.dg/module_equivalence_1.f90    -g  (internal compiler error)
FAIL: gfortran.dg/use_11.f90                  -g  (internal compiler error)
FAIL: gfortran.dg/use_only_1.f90              -g  (internal compiler error)
FAIL: gfortran.dg/widechar_5.f90              -g  (internal compiler error)

FAIL: libgomp.fortran/udr15.f90               -g  (internal compiler error)

are giving an ICE with -g of the kind

internal compiler error: in dwarf2out_imported_module_or_decl, at dwarf2out.c:24070

corresponding to

      gcc_assert (scope_die->die_child);

The Ada test gnat.dg/debug7.adb is also failing with

FAIL: gnat.dg/debug7.adb (test for excess errors)
Excess errors:
gnat1: incorrect object file extension

Dominique

> Le 28 sept. 2016 à 17:44, Jason Merrill <jason@redhat.com> a écrit :
> 
> On Thu, Aug 18, 2016 at 5:33 AM, Pierre-Marie de Rodat
> <derodat@adacore.com> wrote:
>> A check in dwarf2out_imported_module_or_decl prevents valid strict
>> DWARF2 constructs such as DW_TAG_imported_declaration from being emitted
>> in dwarf2out_imported_module_or_decl_1.
>> 
>> The latter already protects the emission of newer DWARF tags with
>> appropriate checks, so the one in the former is redundant and
>> pessimistic.  This function is already called from places like
>> process_scope_var, which are not protected anyway.
>> 
>> This patch removes the check in dwarf2out_imported_module_or_decl so
>> that tags like DW_TAG_imported_declaration are emitted even in strict
>> DWARF2 mode.
>> 
>> Bootstrapped and regtested on x86_64-linux, no regression.
> 
> This check was added for bug 41405, a Darwin bootstrap problem.
> Dominique, can you verify that the compiler still bootstraps with this
> change?  OK if so.
> 
> Jason

Re: [PATCH] DWARF: remove pessimistic DWARF version checks for imported entities

[Pierre-Marie de Rodat]

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

Hello,

Thank you very much for you help, Dominique!

On 09/29/2016 03:16 PM, Dominique d'Humières wrote:
> FAIL: gfortran.dg/binding_label_tests_16.f03  -g  (internal compiler error)
> FAIL: gfortran.dg/module_commons_3.f90        -g  (internal compiler error)
> FAIL: gfortran.dg/module_equivalence_1.f90    -g  (internal compiler error)
> FAIL: gfortran.dg/use_11.f90                  -g  (internal compiler error)
> FAIL: gfortran.dg/use_only_1.f90              -g  (internal compiler error)
> FAIL: gfortran.dg/widechar_5.f90              -g  (internal compiler error)
>
> FAIL: libgomp.fortran/udr15.f90               -g  (internal compiler error)
>
> are giving an ICE with -g of the kind
>
> internal compiler error: in dwarf2out_imported_module_or_decl, at dwarf2out.c:24070
>
> corresponding to
>
>       gcc_assert (scope_die->die_child);

So this is an oversight I did: the check I removed was actually useful 
for one thing: not emitting DW_TAG_imported_module DIEs in strict 
DWARFv2. DW_TAG_imported_declaration ones are always fine, though, so 
what I should do is to move the check, not remove it.

> The Ada test gnat.dg/debug7.adb is also failing with
>
> FAIL: gnat.dg/debug7.adb (test for excess errors)
> Excess errors:
> gnat1: incorrect object file extension

It seems it’s a bad interaction between dg-options "-cargs […]" and the 
testsuite framework. Can be fixed adding “-margs” at the end.

Here is an updated patch, fixing all the issues Dominique reported. 
Bootstrapped and regtested on x86_64-linux. I also tested on 
x86_64-apple-darwin14.5.0 that the above errors are gone.

-- 
Pierre-Marie de Rodat

[-- Attachment #2: 0001-DWARF-move-pessimistic-DWARF-version-checks-for-impo.patch --]
[-- Type: text/x-diff, Size: 3106 bytes --]

From fbcac99dc769c2a2280bc816e67506b67124660c Mon Sep 17 00:00:00 2001
From: Pierre-Marie de Rodat <derodat at adacore dot com>
Date: Thu, 18 Aug 2016 11:33:23 +0200
Subject: [PATCH] DWARF: move pessimistic DWARF version checks for imported
 entities

A check in dwarf2out_imported_module_or_decl prevents
DW_TAG_imported_module from being emitted as it was introduced in the
DWARFv3 standard. However, this also prevents valid strict DWARFv2
constructs such as DW_TAG_imported_declaration from being emitted in
dwarf2out_imported_module_or_decl_1.

The latter already protects the emission of newer DWARF tags with
appropriate checks, so the one in the former is redundant and
pessimistic.  This function is already called from places like
process_scope_var, which are not protected anyway.

This patch moves the check in dwarf2out_imported_module_or_decl so that
in strict DWARFv2 mode, tags like DW_TAG_imported_declaration are
emitted while DW_TAG_imported_module are not.

gcc/

	* dwarf2out.c (dwarf2out_imported_module_or_decl): Move DWARF
	version check to protect only DW_TAG_imported_module generation.

gcc/testsuite/

	* gnat.dg/debug7.adb, gnat.dg/debug7.ads: New testcase.
---
 gcc/dwarf2out.c                  |  8 +++++---
 gcc/testsuite/gnat.dg/debug7.adb | 10 ++++++++++
 gcc/testsuite/gnat.dg/debug7.ads |  4 ++++
 3 files changed, 19 insertions(+), 3 deletions(-)
 create mode 100644 gcc/testsuite/gnat.dg/debug7.adb
 create mode 100644 gcc/testsuite/gnat.dg/debug7.ads

diff --git a/gcc/dwarf2out.c b/gcc/dwarf2out.c
index 972da16..c1a3ae8 100644
--- a/gcc/dwarf2out.c
+++ b/gcc/dwarf2out.c
@@ -24041,13 +24041,15 @@ dwarf2out_imported_module_or_decl (tree decl, tree name, tree context,
       && !should_emit_struct_debug (context, DINFO_USAGE_DIR_USE))
     return;
 
-  if (!(dwarf_version >= 3 || !dwarf_strict))
-    return;
-
   scope_die = get_context_die (context);
 
   if (child)
     {
+      /* DW_TAG_imported_module was introduced in the DWARFv3 specification, so
+	 there is nothing we can do, here.  */
+      if (dwarf_version < 3 && dwarf_strict)
+	return;
+
       gcc_assert (scope_die->die_child);
       gcc_assert (scope_die->die_child->die_tag == DW_TAG_imported_module);
       gcc_assert (TREE_CODE (decl) != NAMESPACE_DECL);
diff --git a/gcc/testsuite/gnat.dg/debug7.adb b/gcc/testsuite/gnat.dg/debug7.adb
new file mode 100644
index 0000000..cdaf089
--- /dev/null
+++ b/gcc/testsuite/gnat.dg/debug7.adb
@@ -0,0 +1,10 @@
+-- { dg-do compile }
+-- { dg-options "-cargs -g -gdwarf-2 -gstrict-dwarf -dA -margs" }
+-- { dg-final { scan-assembler "DW_TAG_imported_decl" } }
+
+package body Debug7 is
+   function Next (I : Integer) return Integer is
+   begin
+      return I + 1;
+   end Next;
+end Debug7;
diff --git a/gcc/testsuite/gnat.dg/debug7.ads b/gcc/testsuite/gnat.dg/debug7.ads
new file mode 100644
index 0000000..047d4a6
--- /dev/null
+++ b/gcc/testsuite/gnat.dg/debug7.ads
@@ -0,0 +1,4 @@
+package Debug7 is
+   function Next (I : Integer) return Integer;
+   function Renamed_Next (I : Integer) return Integer renames Next;
+end Debug7;
-- 
2.10.0

Re: [PATCH] DWARF: remove pessimistic DWARF version checks for imported entities

[Jason Merrill]

OK.

On Tue, Oct 4, 2016 at 9:58 AM, Pierre-Marie de Rodat
<derodat@adacore.com> wrote:
> Hello,
>
> Thank you very much for you help, Dominique!
>
> On 09/29/2016 03:16 PM, Dominique d'Humières wrote:
>>
>> FAIL: gfortran.dg/binding_label_tests_16.f03  -g  (internal compiler
>> error)
>> FAIL: gfortran.dg/module_commons_3.f90        -g  (internal compiler
>> error)
>> FAIL: gfortran.dg/module_equivalence_1.f90    -g  (internal compiler
>> error)
>> FAIL: gfortran.dg/use_11.f90                  -g  (internal compiler
>> error)
>> FAIL: gfortran.dg/use_only_1.f90              -g  (internal compiler
>> error)
>> FAIL: gfortran.dg/widechar_5.f90              -g  (internal compiler
>> error)
>>
>> FAIL: libgomp.fortran/udr15.f90               -g  (internal compiler
>> error)
>>
>> are giving an ICE with -g of the kind
>>
>> internal compiler error: in dwarf2out_imported_module_or_decl, at
>> dwarf2out.c:24070
>>
>> corresponding to
>>
>>       gcc_assert (scope_die->die_child);
>
>
> So this is an oversight I did: the check I removed was actually useful for
> one thing: not emitting DW_TAG_imported_module DIEs in strict DWARFv2.
> DW_TAG_imported_declaration ones are always fine, though, so what I should
> do is to move the check, not remove it.
>
>> The Ada test gnat.dg/debug7.adb is also failing with
>>
>> FAIL: gnat.dg/debug7.adb (test for excess errors)
>> Excess errors:
>> gnat1: incorrect object file extension
>
>
> It seems it’s a bad interaction between dg-options "-cargs […]" and the
> testsuite framework. Can be fixed adding “-margs” at the end.
>
> Here is an updated patch, fixing all the issues Dominique reported.
> Bootstrapped and regtested on x86_64-linux. I also tested on
> x86_64-apple-darwin14.5.0 that the above errors are gone.
>
> --
> Pierre-Marie de Rodat

Re: [PATCH] DWARF: remove pessimistic DWARF version checks for imported entities

[Dominique d'Humières]

The new patch works on darwin without new regression.

Thanks,

Dominique

> Le 4 oct. 2016 à 15:58, Pierre-Marie de Rodat <derodat@adacore.com> a écrit :
> 
> Hello,
> 
> Thank you very much for you help, Dominique!
> 
> On 09/29/2016 03:16 PM, Dominique d'Humières wrote:
>> FAIL: gfortran.dg/binding_label_tests_16.f03  -g  (internal compiler error)
>> FAIL: gfortran.dg/module_commons_3.f90        -g  (internal compiler error)
>> FAIL: gfortran.dg/module_equivalence_1.f90    -g  (internal compiler error)
>> FAIL: gfortran.dg/use_11.f90                  -g  (internal compiler error)
>> FAIL: gfortran.dg/use_only_1.f90              -g  (internal compiler error)
>> FAIL: gfortran.dg/widechar_5.f90              -g  (internal compiler error)
>> 
>> FAIL: libgomp.fortran/udr15.f90               -g  (internal compiler error)
>> 
>> are giving an ICE with -g of the kind
>> 
>> internal compiler error: in dwarf2out_imported_module_or_decl, at dwarf2out.c:24070
>> 
>> corresponding to
>> 
>>      gcc_assert (scope_die->die_child);
> 
> So this is an oversight I did: the check I removed was actually useful for one thing: not emitting DW_TAG_imported_module DIEs in strict DWARFv2. DW_TAG_imported_declaration ones are always fine, though, so what I should do is to move the check, not remove it.
> 
>> The Ada test gnat.dg/debug7.adb is also failing with
>> 
>> FAIL: gnat.dg/debug7.adb (test for excess errors)
>> Excess errors:
>> gnat1: incorrect object file extension
> 
> It seems it’s a bad interaction between dg-options "-cargs […]" and the testsuite framework. Can be fixed adding “-margs” at the end.
> 
> Here is an updated patch, fixing all the issues Dominique reported. Bootstrapped and regtested on x86_64-linux. I also tested on x86_64-apple-darwin14.5.0 that the above errors are gone.
> 
> -- 
> Pierre-Marie de Rodat
> <0001-DWARF-move-pessimistic-DWARF-version-checks-for-impo.patch>

Re: [PATCH] DWARF: remove pessimistic DWARF version checks for imported entities

[Pierre-Marie de Rodat]

On 10/04/2016 09:50 PM, Jason Merrill wrote:
> OK.

On 10/05/2016 06:53 AM, Dominique d'Humières wrote:
> The new patch works on darwin without new regression.

Thank you both for your help! This is now comitted.

-- 
Pierre-Marie de Rodat