[Bug lto/97586] New: "make check" failures in binutils with -flto=jobserver

[Bug lto/97586] New: "make check" failures in binutils with -flto=jobserver

[hjl.tools at gmail dot com]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=97586

            Bug ID: 97586
           Summary: "make check" failures in binutils with -flto=jobserver
           Product: gcc
           Version: 11.0
            Status: UNCONFIRMED
          Severity: normal
          Priority: P3
         Component: lto
          Assignee: unassigned at gcc dot gnu.org
          Reporter: hjl.tools at gmail dot com
                CC: marxin at gcc dot gnu.org
  Target Milestone: ---

On Linux/x86-64, with GCC 11

commit 4f8cfb42883cc247f11096a3703e379d1f24ab3f
Author: Jan Hubicka <jh@suse.cz>
Date:   Mon Oct 26 20:19:33 2020 +0100

    Extend builtin fnspecs

"make check" on binutils users/hjl/lto branch:

https://gitlab.com/x86-binutils/binutils-gdb/-/commits/users/hjl/lto

gave

FAIL: nm --line-numbers on DWARF-4 debug info (grep for externd global
file/line)
FAIL: build-id-debuglink (grepping for source file name in disassembly output)
FAIL: objdump -S
FAIL: objdump --source-comment
FAIL: DWARF2 debugging information 2
FAIL: DWARF2 debugging information 2 with SHF_COMPRESSED
FAIL: 64bit DWARF2 debugging information 2
FAIL: 64bit DWARF2 debugging information 2 with SHF_COMPRESSED
FAIL: DWARF parse during linker error
FAIL: Build warn libbar.so
FAIL: Dump pr21978.so
FAIL: Run warn with versioned libfoo.so
FAIL: undefined symbol with compressed debug sections
FAIL: PR ld/12760
FAIL: plugin claimfile lost symbol
FAIL: plugin claimfile replace symbol
FAIL: plugin claimfile resolve symbol
FAIL: plugin claimfile lost symbol with source
FAIL: plugin claimfile replace symbol with source
FAIL: plugin claimfile resolve symbol with source
FAIL: plugin 2 with source lib
FAIL: load plugin 2 with source
FAIL: plugin 3 with source lib
FAIL: load plugin 3 with source
FAIL: undefined line
FAIL: undefined line
FAIL: undefined symbol with compressed debug sections

when compiled with -O2 -g -flto=jobserver.

[Bug lto/97586] [11 Regression] "make check" failures in binutils with -flto=jobserver

[rguenth at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=97586

Richard Biener <rguenth at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
     Ever confirmed|0                           |1
           Keywords|                            |wrong-code
                 CC|                            |hubicka at gcc dot gnu.org
            Summary|"make check" failures in    |[11 Regression] "make
                   |binutils with               |check" failures in binutils
                   |-flto=jobserver             |with -flto=jobserver
   Last reconfirmed|                            |2020-10-27
   Target Milestone|---                         |11.0
             Status|UNCONFIRMED                 |WAITING

--- Comment #1 from Richard Biener <rguenth at gcc dot gnu.org> ---
Just to be clear - the FAILs do not reproduce with just -flto rather than
-flto=jobserver?  Do the FAILs reproduce when building binutils itself with
-flto but not the tests or are the FAILs appearing when building binutils
without -flto but the tests with -flto?

[Bug lto/97586] [11 Regression] "make check" failures in binutils with -flto=jobserver

[marxin at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=97586

--- Comment #2 from Martin Liška <marxin at gcc dot gnu.org> ---
I'm going to analyze it.

[Bug lto/97586] [11 Regression] "make check" failures in binutils with -flto=jobserver since r11-3641-gc34db4b6f8a5d803

[marxin at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=97586

Martin Liška <marxin at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
           Priority|P3                          |P1
            Summary|[11 Regression] "make       |[11 Regression] "make
                   |check" failures in binutils |check" failures in binutils
                   |with -flto=jobserver        |with -flto=jobserver since
                   |                            |r11-3641-gc34db4b6f8a5d803
             Status|WAITING                     |NEW

--- Comment #3 from Martin Liška <marxin at gcc dot gnu.org> ---
Confirmed, started with r11-3641-gc34db4b6f8a5d803.
It's not related to -flto=jobserver, -flto is enough.

[Bug lto/97586] [11 Regression] "make check" failures in binutils with -flto since r11-3641-gc34db4b6f8a5d803

[marxin at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=97586

--- Comment #4 from Martin Liška <marxin at gcc dot gnu.org> ---
So e.g. addr2line is miscompiled:

$ as  --32 --compress-debug-sections  -o compress.o
/tmp/binutils-gdb/gas/testsuite/gas/i386/dw2-compress-2.s

The .s file is part of the test-suite.

$ ./addr2line  0x0 0x10 -e /tmp/compress.o
dw2-compress-2.c:?
dw2-compress-2.c:?

While good output is:

$ ./addr2line  0x0 0x10 -e /tmp/compress.o
./dw2-compress-2.c:12
./dw2-compress-2.c:5

[Bug lto/97586] [11 Regression] "make check" failures in binutils with -flto since r11-3641-gc34db4b6f8a5d803

[marxin at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=97586

--- Comment #5 from Martin Liška <marxin at gcc dot gnu.org> ---
One can trigger it only with one debug counter value:
-fdbg-cnt=ipa_mod_ref:869-869 (and multiple LTRANS partitions).

The only changed function is in:
comp_unit_maybe_decode_line_info (struct comp_unit * unit)
...

--- good/addr2line.ltrans7.ltrans.122t.dom2     2020-10-27 10:06:56.935982898
+0100
+++ bad/addr2line.ltrans7.ltrans.122t.dom2      2020-10-27 10:06:29.248177802
+0100
@@ -25035,6 +25035,8 @@
 threading bb 269 up to 16 stmts
 threading bb 269 kills 1 stmts
 threading bb 269 up to 16 stmts
+***dbgcnt: lower limit 869 reached for ipa_mod_ref.***
+***dbgcnt: upper limit 869 reached for ipa_mod_ref.***
 threading bb 141 kills 9 stmts
 threading bb 141 up to 24 stmts
 threading bb 139 kills 2 stmts
@@ -27429,9 +27431,9 @@
   # DEBUG dir => dir_142
   # DEBUG BEGIN_STMT
   _143 = line_ptr;
-  _144 = bytes_read;
-  _145 = (sizetype) _144;
-  _146 = _143 + _145;
+  _144 = _481;
+  _145 = _138;
+  _146 = _143 + _138;
   line_ptr = _146;
   # DEBUG BEGIN_STMT
   _147 = _bfd_safe_read_leb128.constprop (_146, &bytes_read, 0, line_end_55);

The change is:

***dbgcnt: lower limit 869 reached for ipa_mod_ref.***
***dbgcnt: upper limit 869 reached for ipa_mod_ref.***
ipa-modref: call stmt _141 = _bfd_safe_read_leb128.constprop (_140,
&bytes_read, 0, line_end_55);
ipa-modref: call to _bfd_safe_read_leb128.constprop/17919 does not clobber ref:
bytes_read alias sets: 7->7
FIND: _481
  Replaced redundant expr '[../bfd/../../bfd/dwarf2.c:2321:13] bytes_read' with
'_481'
==== ASGN _144 = _481

[Bug lto/97586] [11 Regression] "make check" failures in binutils with -flto since r11-3641-gc34db4b6f8a5d803

[marxin at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=97586

--- Comment #6 from Martin Liška <marxin at gcc dot gnu.org> ---
Points to the following statements:
../bfd/../../bfd/dwarf2.c:2321:13

https://sourceware.org/git/?p=binutils-gdb.git;a=blob;f=bfd/dwarf2.c;h=977bf43a6a181d97fa3ae71329478f9f263b090f;hb=HEAD#l2321

[Bug lto/97586] [11 Regression] "make check" failures in binutils with -flto since r11-3641-gc34db4b6f8a5d803

[marxin at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=97586

--- Comment #7 from Martin Liška <marxin at gcc dot gnu.org> ---
So the _bfd_safe_read_leb128.constprop removes the first unused argument:

bfd_vma
_bfd_safe_read_leb128 (bfd *abfd ATTRIBUTE_UNUSED,
                       bfd_byte *data,
                       unsigned int *length_return,
                       bfd_boolean sign,
                       const bfd_byte * const end)
{
  bfd_vma result = 0;
  unsigned int num_read = 0;
  unsigned int shift = 0;
  unsigned char byte = 0;

  while (data < end)
    {
      byte = bfd_get_8 (abfd, data);
      data++;
      num_read++;

      result |= ((bfd_vma) (byte & 0x7f)) << shift;

      shift += 7;
      if ((byte & 0x80) == 0)
        break;
    }

  if (length_return != NULL)
    *length_return = num_read;

  if (sign && (shift < 8 * sizeof (result)) && (byte & 0x40))
    result |= -((bfd_vma) 1 << shift);

  return result;
}

But the analysis is bogus:

ipa-modref: call to _bfd_safe_read_leb128.constprop/17919 does not clobber ref:
bytes_read alias sets: 7->7

The &bytes_read is always modified in the function (if it's non-null).

Re: [Bug lto/97586] [11 Regression] "make check" failures in binutils with -flto since r11-3641-gc34db4b6f8a5d803

[Jan Hubicka]

> So the _bfd_safe_read_leb128.constprop removes the first unused argument:
> 
...
> 
> But the analysis is bogus:
> 
> ipa-modref: call to _bfd_safe_read_leb128.constprop/17919 does not clobber ref:
> bytes_read alias sets: 7->7
> 
> The &bytes_read is always modified in the function (if it's non-null).

There is code in modref-transform that is supposed to update the
summary.  It produces debug output about it, but to be honest I am not
sure where it will land since we now materialize lazily. Can you do
-fdump-tree-all-details and then grep for "Updating summary for"
and see if the resulting summary makes sense and we do not miss the
update?

Maybe problem is that if we call across partitioining boundary, the
summary update happens in one ltrans while the call happens in different
ltrans?  When partitioning, we may need to update summaries at
stream-out time....

Honza

[Bug ipa/97586] [11 Regression] "make check" failures in binutils with -flto since r11-3641-gc34db4b6f8a5d803

[hubicka at ucw dot cz]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=97586

--- Comment #8 from Jan Hubicka <hubicka at ucw dot cz> ---
> So the _bfd_safe_read_leb128.constprop removes the first unused argument:
> 
...
> 
> But the analysis is bogus:
> 
> ipa-modref: call to _bfd_safe_read_leb128.constprop/17919 does not clobber ref:
> bytes_read alias sets: 7->7
> 
> The &bytes_read is always modified in the function (if it's non-null).

There is code in modref-transform that is supposed to update the
summary.  It produces debug output about it, but to be honest I am not
sure where it will land since we now materialize lazily. Can you do
-fdump-tree-all-details and then grep for "Updating summary for"
and see if the resulting summary makes sense and we do not miss the
update?

Maybe problem is that if we call across partitioining boundary, the
summary update happens in one ltrans while the call happens in different
ltrans?  When partitioning, we may need to update summaries at
stream-out time....

Honza

[Bug ipa/97586] [11 Regression] "make check" failures in binutils with -flto since r11-3641-gc34db4b6f8a5d803

[marxin at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=97586

--- Comment #9 from Martin Liška <marxin at gcc dot gnu.org> ---
> There is code in modref-transform that is supposed to update the
> summary.  It produces debug output about it, but to be honest I am not
> sure where it will land since we now materialize lazily. Can you do
> -fdump-tree-all-details and then grep for "Updating summary for"
> and see if the resulting summary makes sense and we do not miss the
> update?

Nice, I can see it here:

grep 'Updating summary' addr2line*
addr2line.ltrans15.ltrans.083i.modref:Updating summary for
coff_set_custom_section_alignment.constprop/17873 from:
addr2line.ltrans15.ltrans.083i.modref:Updating summary for
_bfd_elf_assign_file_position_for_section.constprop/17888 from:
addr2line.ltrans15.ltrans.083i.modref:Updating summary for
_bfd_safe_read_leb128.constprop/17919 from:
addr2line.ltrans15.ltrans.083i.modref:Updating summary for
elf_i386_tpoff.isra/17989 from:
addr2line.ltrans15.ltrans.083i.modref:Updating summary for
elf_x86_64_tpoff.isra/18043 from:

So yes, _bfd_safe_read_leb128.constprop/17919 is updated in ltrans15, while the
use is in ltrans7.

Updating summary for _bfd_safe_read_leb128.constprop/17919 from:
  loads:
    Limits: 32 bases, 16 refs
    Every base
  stores:
    Limits: 32 bases, 16 refs
      Base 0: alias set 2
        Ref 0: alias set 2
          access: Parm 2 param offset:0 offset:0 size:32 max_size:32
to:
  loads:
    Limits: 32 bases, 16 refs
    Every base
  stores:
    Limits: 32 bases, 16 refs
      Base 0: alias set 2
        Ref 0: alias set 2
          access: Parm 1 param offset:0 offset:0 size:32 max_size:32

Re: [Bug ipa/97586] [11 Regression] "make check" failures in binutils with -flto since r11-3641-gc34db4b6f8a5d803

[Jan Hubicka]

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

Hi,
this is patch that moves updates to WPA time.  Does it work for you?
Honza

[-- Attachment #2: fix --]
[-- Type: text/plain, Size: 2871 bytes --]



2020-10-27  Jan Hubicka  <hubicka@ucw.cz>

	* ipa-modref.c (modref_summaries_lto::duplicate): Check that no clones
	happens after modref.
	(modref_transform): Rename to ...
	(update_signature): ... this one.
	(pass_ipa_modref::execute): Update all summaries once done.

diff --git a/gcc/ipa-modref.c b/gcc/ipa-modref.c
index 3a70965d156..235d712a986 100644
--- a/gcc/ipa-modref.c
+++ b/gcc/ipa-modref.c
@@ -1080,6 +1080,9 @@ modref_summaries_lto::duplicate (cgraph_node *, cgraph_node *,
 				 modref_summary_lto *src_data,
 				 modref_summary_lto *dst_data)
 {
+  /* Be sure that no furhter cloning happens after ipa-modref.  If it does
+     we will need to update signatures for possible param changes.  */
+  gcc_checking_assert (!((modref_summaries_lto *)summaries_lto)->propagated);
   dst_data->stores = modref_records_lto::create_ggc
 			(src_data->stores->max_bases,
 			 src_data->stores->max_refs,
@@ -1503,14 +1506,14 @@ remap_arguments (vec <int> *map, modref_records *tt)
 
 /* If signature changed, update the summary.  */
 
-static unsigned int
-modref_transform (struct cgraph_node *node)
+static void
+update_signature (struct cgraph_node *node)
 {
   if (!node->clone.param_adjustments || !optimization_summaries)
-    return 0;
+    return;
   modref_summary *r = optimization_summaries->get (node);
   if (!r)
-    return 0;
+    return;
   if (dump_file)
     {
       fprintf (dump_file, "Updating summary for %s from:\n",
@@ -1546,7 +1549,7 @@ modref_transform (struct cgraph_node *node)
       fprintf (dump_file, "to:\n");
       r->dump (dump_file);
     }
-  return 0;
+  return;
 }
 
 /* Definition of the modref IPA pass.  */
@@ -1575,7 +1578,7 @@ public:
 		      modref_read,     /* read_optimization_summary */
 		      NULL,            /* stmt_fixup */
 		      0,               /* function_transform_todo_flags_start */
-		      modref_transform,/* function_transform */
+		      NULL,	       /* function_transform */
 		      NULL)            /* variable_transform */
   {}
 
@@ -2137,6 +2140,9 @@ pass_ipa_modref::execute (function *)
 
       modref_propagate_in_scc (component_node);
     }
+  cgraph_node *node;
+  FOR_EACH_FUNCTION (node)
+    update_signature (node);
   if (summaries_lto)
     ((modref_summaries_lto *)summaries_lto)->propagated = true;
   ipa_free_postorder_info ();
diff --git a/gcc/testsuite/g++.dg/ipa/devirt-24.C b/gcc/testsuite/g++.dg/ipa/devirt-24.C
index eaef1f5b3f8..7b5b806dd05 100644
--- a/gcc/testsuite/g++.dg/ipa/devirt-24.C
+++ b/gcc/testsuite/g++.dg/ipa/devirt-24.C
@@ -37,4 +37,4 @@ C *b = new (C);
   }
 }
 /* { dg-final { scan-ipa-dump-times "Discovered a virtual call to a known target" 1 "inline" { xfail *-*-* } } } */
-/* { dg-final { scan-ipa-dump-times "Aggregate passed by reference" 1 "cp"  } } */
+/* { dg-final { scan-ipa-dump-times "Aggregate passed by reference" 2 "cp"  } } */

[Bug ipa/97586] [11 Regression] "make check" failures in binutils with -flto since r11-3641-gc34db4b6f8a5d803

[hubicka at ucw dot cz]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=97586

--- Comment #10 from Jan Hubicka <hubicka at ucw dot cz> ---
Hi,
this is patch that moves updates to WPA time.  Does it work for you?
Honza

Re: [Bug ipa/97586] [11 Regression] "make check" failures in binutils with -flto since r11-3641-gc34db4b6f8a5d803

[Jan Hubicka]

> Hi,
> this is patch that moves updates to WPA time.  Does it work for you?
Actually it won't help, since it updates only non-lto summary.  I am
testing better patch, sorry for that.

Honza

[Bug ipa/97586] [11 Regression] "make check" failures in binutils with -flto since r11-3641-gc34db4b6f8a5d803

[hubicka at ucw dot cz]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=97586

--- Comment #11 from Jan Hubicka <hubicka at ucw dot cz> ---
> Hi,
> this is patch that moves updates to WPA time.  Does it work for you?
Actually it won't help, since it updates only non-lto summary.  I am
testing better patch, sorry for that.

Honza

[Bug ipa/97586] [11 Regression] "make check" failures in binutils with -flto since r11-3641-gc34db4b6f8a5d803

[marxin at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=97586

--- Comment #12 from Martin Liška <marxin at gcc dot gnu.org> ---
(In reply to Jan Hubicka from comment #11)
> > Hi,
> > this is patch that moves updates to WPA time.  Does it work for you?
> Actually it won't help, since it updates only non-lto summary.  I am
> testing better patch, sorry for that.
> 
> Honza

Yes, I noticed that right now :) Please attach me the patch here.

[Bug ipa/97586] [11 Regression] "make check" failures in binutils with -flto since r11-3641-gc34db4b6f8a5d803

[hubicka at ucw dot cz]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=97586

--- Comment #13 from Jan Hubicka <hubicka at ucw dot cz> ---
> Yes, I noticed that right now :) Please attach me the patch here.
Sorry for bogus patch.  This one has chance to work.
Honza

[Bug ipa/97586] [11 Regression] "make check" failures in binutils with -flto since r11-3641-gc34db4b6f8a5d803

[marxin at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=97586

--- Comment #14 from Martin Liška <marxin at gcc dot gnu.org> ---
(In reply to Jan Hubicka from comment #13)
> Created attachment 49450 [details]
> fix
> 
> > Yes, I noticed that right now :) Please attach me the patch here.
> Sorry for bogus patch.  This one has chance to work.
> Honza

Nice, this one fixes that! Thanks.

[Bug ipa/97586] [11 Regression] "make check" failures in binutils with -flto since r11-3641-gc34db4b6f8a5d803

[hubicka at ucw dot cz]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=97586

--- Comment #15 from Jan Hubicka <hubicka at ucw dot cz> ---
> https://gcc.gnu.org/bugzilla/show_bug.cgi?id=97586
> 
> --- Comment #14 from Martin Liška <marxin at gcc dot gnu.org> ---
> (In reply to Jan Hubicka from comment #13)
> > Created attachment 49450 [details]
> > fix
> > 
> > > Yes, I noticed that right now :) Please attach me the patch here.
> > Sorry for bogus patch.  This one has chance to work.
> > Honza
> 
> Nice, this one fixes that! Thanks.
Great, thanks for confirmation!
I will commit it once ltoboottrap concludes.

Honza

[Bug ipa/97586] [11 Regression] "make check" failures in binutils with -flto since r11-3641-gc34db4b6f8a5d803

[cvs-commit at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=97586

--- Comment #16 from CVS Commits <cvs-commit at gcc dot gnu.org> ---
The master branch has been updated by Jan Hubicka <hubicka@gcc.gnu.org>:

https://gcc.gnu.org/g:fe90c504416e6423c6a56f37a9265deabdb03de9

commit r11-4445-gfe90c504416e6423c6a56f37a9265deabdb03de9
Author: Jan Hubicka <jh@suse.cz>
Date:   Tue Oct 27 16:25:12 2020 +0100

    Fix ipa-modref signature updates

            PR ipa/97586
            * ipa-modref-tree.h (modref_tree::remap_params): New member
function.
            * ipa-modref.c (modref_summaries_lto::duplicate): Check that
            optimization summaries are not duplicated.
            (remap_arguments): Remove.
            (modref_transform): Rename to ...
            (update_signature): ... this one; handle also lto summary.
            (pass_ipa_modref::execute): Update signatures here rather
            than in transform hook.

[Bug ipa/97586] [11 Regression] "make check" failures in binutils with -flto since r11-3641-gc34db4b6f8a5d803

[hjl.tools at gmail dot com]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=97586

H.J. Lu <hjl.tools at gmail dot com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|NEW                         |RESOLVED
         Resolution|---                         |FIXED

--- Comment #17 from H.J. Lu <hjl.tools at gmail dot com> ---
Fixed for GCC 11.