PR debug/46102 Disable -feliminate-dwarf2-dups when reading a PCH
diff mbox

Message ID 54E62C70.8020208@redhat.com
State New
Headers show

Commit Message

Aldy Hernandez Feb. 19, 2015, 6:33 p.m. UTC
On 02/19/2015 08:50 AM, Jakub Jelinek wrote:
> On Thu, Feb 19, 2015 at 08:45:08AM -0800, Aldy Hernandez wrote:
>> [And this time, actually CCing the list :)].
>>
>> Gentlemen!
>>
>> Reading in the compiler state for pch (gt_pch_restore) obliterates the
>> DIE table, and consequently the DW_TAG_GNU_[BE]INCL* DIEs that may have
>> been in it.  This causes inconsistencies when reading in _any_
>> pre-compiled header into a source file that uses
>> -feliminate-dwarf2-dups, and consequently already has some
>> DW_TAG_GNU_[BE]INCL* DIEs.
>>
>> Normally the DIE table should be empty this early on, especially since
>> mainline generates dwarf at the end of the compilation unit, but the DIE
>> table may have DW_TAG_GNU_[BE]INCL* DIEs that were created early by
>> dwarf2out_start_source_file/etc or it may have the DW_TAG_compile_unit.
>>
>> I suppose we could merge incoming DIEs with existing DIEs and complicate
>> our lives, but considering we will probably have to tackle this in the
>> debug-early work, I propose we disable this combination for now (and
>> possibly permanently, unless we really care about it).
>>
>> OK for mainline pending tests?
>
> Wouldn't it be better to disable PCH reading if -feliminate-dwarf2-dups
> is used?  I mean, fail to read the PCH silently (or with warning for -Wpch
> or what the warning is about why PCH couldn't be read or was ignored),

Sure, that sounds reasonable.  Patch attached.

> perhaps error out if you try to generate PCH with -feliminate-dwarf2-dups?

Well, any PCH file we generate will have some sort of early DIE in it 
(at the very least the compilation unit DIE) and we will read these in 
at PCH read-in time, obliterating whatever was already there.  But most 
importantly, with the attached patch we will not use these 
DW_TAG_GNU_[BE]INCL* DIEs, since the reader will avoid reading the pch 
file.  So, I don't think erroring out at output time is necessary.

How does this look?
commit d90a408ad21aa0868cc13de24ea38e210ef78a68
Author: Aldy Hernandez <aldyh@redhat.com>
Date:   Thu Feb 19 07:35:59 2015 -0800

    	PR debug/46102
    	* c-pch.c (c_common_valid_pch): Mark PCH file with
    	-feliminate-dwarf2-dups as invalid.

Comments

Jakub Jelinek Feb. 19, 2015, 6:41 p.m. UTC | #1
On Thu, Feb 19, 2015 at 10:33:20AM -0800, Aldy Hernandez wrote:
> Well, any PCH file we generate will have some sort of early DIE in it (at
> the very least the compilation unit DIE) and we will read these in at PCH
> read-in time, obliterating whatever was already there.  But most
> importantly, with the attached patch we will not use these
> DW_TAG_GNU_[BE]INCL* DIEs, since the reader will avoid reading the pch file.
> So, I don't think erroring out at output time is necessary.
> 
> How does this look?

Looks reasonable to me, but I'd prefer to defer this to Jason as debug
maintainer.

> commit d90a408ad21aa0868cc13de24ea38e210ef78a68
> Author: Aldy Hernandez <aldyh@redhat.com>
> Date:   Thu Feb 19 07:35:59 2015 -0800
> 
>     	PR debug/46102
>     	* c-pch.c (c_common_valid_pch): Mark PCH file with
>     	-feliminate-dwarf2-dups as invalid.
> 
> diff --git a/gcc/c-family/c-pch.c b/gcc/c-family/c-pch.c
> index 0ede92a..55163c9 100644
> --- a/gcc/c-family/c-pch.c
> +++ b/gcc/c-family/c-pch.c
> @@ -224,6 +224,19 @@ c_common_valid_pch (cpp_reader *pfile, const char *name, int fd)
>    const char *pch_ident;
>    struct c_pch_validity v;
>  
> +  /* We may have outputted a few DIEs corresponding to
> +     DW_TAG_GNU_[BE]INCL.  Reading the compiler state later will read
> +     in these DIEs, and obliterate any DW_TAG_GNU_[BE]INCL the reader
> +     may have generated itself.  Do not read the PCH if this may
> +     happen.  */
> +  if (flag_eliminate_dwarf2_dups)
> +    {
> +      if (cpp_get_options (pfile)->warn_invalid_pch)
> +	cpp_error (pfile, CPP_DL_WARNING,
> +		   "%s: cannot be used with -feliminate-dwarf2-dups", name);
> +      return 2;
> +    }
> +
>    /* Perform a quick test of whether this is a valid
>       precompiled header for the current language.  */
>  


	Jakub
Aldy Hernandez Feb. 25, 2015, 3:50 p.m. UTC | #2
On 02/19/2015 10:41 AM, Jakub Jelinek wrote:
> On Thu, Feb 19, 2015 at 10:33:20AM -0800, Aldy Hernandez wrote:
>> Well, any PCH file we generate will have some sort of early DIE in it (at
>> the very least the compilation unit DIE) and we will read these in at PCH
>> read-in time, obliterating whatever was already there.  But most
>> importantly, with the attached patch we will not use these
>> DW_TAG_GNU_[BE]INCL* DIEs, since the reader will avoid reading the pch file.
>> So, I don't think erroring out at output time is necessary.
>>
>> How does this look?
>
> Looks reasonable to me, but I'd prefer to defer this to Jason as debug
> maintainer.
>
>> commit d90a408ad21aa0868cc13de24ea38e210ef78a68
>> Author: Aldy Hernandez <aldyh@redhat.com>
>> Date:   Thu Feb 19 07:35:59 2015 -0800
>>
>>      	PR debug/46102
>>      	* c-pch.c (c_common_valid_pch): Mark PCH file with
>>      	-feliminate-dwarf2-dups as invalid.
>>
>> diff --git a/gcc/c-family/c-pch.c b/gcc/c-family/c-pch.c
>> index 0ede92a..55163c9 100644
>> --- a/gcc/c-family/c-pch.c
>> +++ b/gcc/c-family/c-pch.c
>> @@ -224,6 +224,19 @@ c_common_valid_pch (cpp_reader *pfile, const char *name, int fd)
>>     const char *pch_ident;
>>     struct c_pch_validity v;
>>
>> +  /* We may have outputted a few DIEs corresponding to
>> +     DW_TAG_GNU_[BE]INCL.  Reading the compiler state later will read
>> +     in these DIEs, and obliterate any DW_TAG_GNU_[BE]INCL the reader
>> +     may have generated itself.  Do not read the PCH if this may
>> +     happen.  */
>> +  if (flag_eliminate_dwarf2_dups)
>> +    {
>> +      if (cpp_get_options (pfile)->warn_invalid_pch)
>> +	cpp_error (pfile, CPP_DL_WARNING,
>> +		   "%s: cannot be used with -feliminate-dwarf2-dups", name);
>> +      return 2;
>> +    }
>> +
>>     /* Perform a quick test of whether this is a valid
>>        precompiled header for the current language.  */
>>
>
>
> 	Jakub
>

Patch
diff mbox

diff --git a/gcc/c-family/c-pch.c b/gcc/c-family/c-pch.c
index 0ede92a..55163c9 100644
--- a/gcc/c-family/c-pch.c
+++ b/gcc/c-family/c-pch.c
@@ -224,6 +224,19 @@  c_common_valid_pch (cpp_reader *pfile, const char *name, int fd)
   const char *pch_ident;
   struct c_pch_validity v;
 
+  /* We may have outputted a few DIEs corresponding to
+     DW_TAG_GNU_[BE]INCL.  Reading the compiler state later will read
+     in these DIEs, and obliterate any DW_TAG_GNU_[BE]INCL the reader
+     may have generated itself.  Do not read the PCH if this may
+     happen.  */
+  if (flag_eliminate_dwarf2_dups)
+    {
+      if (cpp_get_options (pfile)->warn_invalid_pch)
+	cpp_error (pfile, CPP_DL_WARNING,
+		   "%s: cannot be used with -feliminate-dwarf2-dups", name);
+      return 2;
+    }
+
   /* Perform a quick test of whether this is a valid
      precompiled header for the current language.  */