Bug 25534 - [cc-with-dwz] FAIL: gdb.cp/m-static.exp: static const int initialized elsewhere
Summary: [cc-with-dwz] FAIL: gdb.cp/m-static.exp: static const int initialized elsewhere
Status: RESOLVED FIXED
Alias: None
Product: gdb
Classification: Unclassified
Component: gdb (show other bugs)
Version: HEAD
: P2 normal
Target Milestone: 10.1
Assignee: Not yet assigned to anyone
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2020-02-11 19:31 UTC by Tom de Vries
Modified: 2020-02-21 15:37 UTC (History)
2 users (show)

See Also:
Host:
Target:
Build:
Last reconfirmed:


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description Tom de Vries 2020-02-11 19:31:06 UTC
I see on master with board cc-with-dwz:
...
FAIL: gdb.cp/m-static.exp: static const int initialized elsewhere
FAIL: gdb.cp/m-static.exp: info variable everywhere
...

First FAIL in more detail:
...
(gdb) PASS: gdb.cp/m-static.exp: template object, static derived enum
print test4.elsewhere^M
$18 = <optimized out>^M
(gdb) FAIL: gdb.cp/m-static.exp: static const int initialized elsewhere
...
while normally we have instead:
...
(gdb) PASS: gdb.cp/m-static.exp: template object, static derived enum
print test4.elsewhere^M
$18 = 221^M
(gdb) PASS: gdb.cp/m-static.exp: static const int initialized elsewhere
...
Comment 1 Tom de Vries 2020-02-11 19:32:47 UTC
Regression by commit 0494dbecdf "Consolidate partial symtab dependency reading".

commit 0494dbecdfa0b0b9065393755a1ac64431997da1
Author: Tom Tromey <tom@tromey.com>
Date:   Wed Oct 23 09:46:25 2019 -0600

    Consolidate partial symtab dependency reading
    
    Most of the symbol readers have code to iterate over a partial symtabs
    dependencies, expanding each one and optionally printing a message.
    Now that the "second-stage" psymtab expansion is available as a
    method, these implementations can all be merged.
    
    This patch also changes a couple more warnings into assertions.
    
    gdb/ChangeLog
    2020-01-26  Tom Tromey  <tom@tromey.com>
    
            * xcoffread.c (xcoff_psymtab_to_symtab_1): Call
            read_dependencies.  Add assert.
            * psymtab.c (partial_symtab::read_dependencies): New method.
            * psympriv.h (struct partial_symtab) <read_dependencies>: New
            method.
            * mdebugread.c (psymtab_to_symtab_1): Call read_dependencies.
            * dwarf2read.c (dwarf2_psymtab::expand_psymtab): Call
            read_dependencies.
            * dbxread.c (dbx_psymtab_to_symtab_1): Call read_dependencies.
            Add assert.
Comment 2 Simon Marchi 2020-02-12 05:02:53 UTC
If I understand correctly, this commit in not present in GDB 9, so it means the bug is not in GDB 9?  I will therefore put the target milestone to 10.1, we'll want to fix that before GDB 10.
Comment 3 Tom de Vries 2020-02-12 07:14:20 UTC
(In reply to Simon Marchi from comment #2)
> If I understand correctly, this commit in not present in GDB 9, so it means
> the bug is not in GDB 9?

Yes.

> I will therefore put the target milestone to 10.1,
> we'll want to fix that before GDB 10.

Ack.
Comment 4 Tom de Vries 2020-02-12 08:58:49 UTC
Difference before and after offending commit using cc-with-dwz board:
...
$ diff -U 0 ref-dwz/gdb.sum gdb.sum | grep -v @@
--- ref-dwz/gdb.sum     2020-02-12 08:43:21.261947714 +0100
+++ gdb.sum     2020-02-12 09:55:25.113550063 +0100
-PASS: gdb.ada/arr_acc_idx_w_gap.exp: print indexed_by_enum(lit2..lit4)
+FAIL: gdb.ada/arr_acc_idx_w_gap.exp: print indexed_by_enum(lit2..lit4)
-PASS: gdb.ada/array_char_idx.exp: Display my_table
+FAIL: gdb.ada/array_char_idx.exp: Display my_table
-PASS: gdb.ada/array_return.exp: value printed by finish of Create_Small
+FAIL: gdb.ada/array_return.exp: value printed by finish of Create_Small
-PASS: gdb.ada/array_return.exp: value printed by finish of Create_Large
+FAIL: gdb.ada/array_return.exp: value printed by finish of Create_Large
-PASS: gdb.ada/array_return.exp: value printed by finish of Create_Small_Float_Vector
+FAIL: gdb.ada/array_return.exp: value printed by finish of Create_Small_Float_Vector
-PASS: gdb.ada/funcall_char.exp: print f('A')
+FAIL: gdb.ada/funcall_char.exp: print f('A')
-PASS: gdb.ada/iwide.exp: print s_access.all
-PASS: gdb.ada/iwide.exp: print sp_access.all
+FAIL: gdb.ada/iwide.exp: print s_access.all
+FAIL: gdb.ada/iwide.exp: print sp_access.all
-PASS: gdb.ada/ptype_field.exp: ptype circle
-PASS: gdb.ada/ptype_field.exp: ptype circle.pos
-PASS: gdb.ada/ptype_field.exp: ptype circle.pos.x
+FAIL: gdb.ada/ptype_field.exp: ptype circle
+FAIL: gdb.ada/ptype_field.exp: ptype circle.pos
+FAIL: gdb.ada/ptype_field.exp: ptype circle.pos.x
-PASS: gdb.ada/rec_return.exp: print bar
+FAIL: gdb.ada/rec_return.exp: print bar
-PASS: gdb.ada/tagged_access.exp: ptype c.all
-PASS: gdb.ada/tagged_access.exp: ptype c.menu_name
+FAIL: gdb.ada/tagged_access.exp: ptype c.all
+FAIL: gdb.ada/tagged_access.exp: ptype c.menu_name
-PASS: gdb.ada/var_arr_typedef.exp: print va
-PASS: gdb.ada/var_arr_typedef.exp: print vb
-PASS: gdb.ada/var_arr_typedef.exp: print a
+FAIL: gdb.ada/var_arr_typedef.exp: print va
+FAIL: gdb.ada/var_arr_typedef.exp: print vb
+FAIL: gdb.ada/var_arr_typedef.exp: print a
-PASS: gdb.ada/var_rec_arr.exp: print a1
+FAIL: gdb.ada/var_rec_arr.exp: print a1
-PASS: gdb.ada/var_rec_arr.exp: print a1(2)
-PASS: gdb.ada/var_rec_arr.exp: print a1(3)
-PASS: gdb.ada/var_rec_arr.exp: print a2
-PASS: gdb.ada/var_rec_arr.exp: print a2(1)
-PASS: gdb.ada/var_rec_arr.exp: print a2(2)
+FAIL: gdb.ada/var_rec_arr.exp: print a1(2)
+FAIL: gdb.ada/var_rec_arr.exp: print a1(3)
+FAIL: gdb.ada/var_rec_arr.exp: print a2
+FAIL: gdb.ada/var_rec_arr.exp: print a2(1)
+FAIL: gdb.ada/var_rec_arr.exp: print a2(2)
-PASS: gdb.cp/m-static.exp: static const int initialized elsewhere
+FAIL: gdb.cp/m-static.exp: static const int initialized elsewhere
-PASS: gdb.cp/m-static.exp: info variable everywhere
+FAIL: gdb.cp/m-static.exp: info variable everywhere
-PASS: gdb.cp/ovsrch.exp: break A::outer::foo  (char*)  const if (A::outer::func ())
-PASS: gdb.cp/ovsrch.exp: break 'A::outer::foo  (char*)  const' if (A::outer::func ())
+FAIL: gdb.cp/ovsrch.exp: break A::outer::foo  (char*)  const if (A::outer::func ())
+FAIL: gdb.cp/ovsrch.exp: break 'A::outer::foo  (char*)  const' if (A::outer::func ())
-PASS: gdb.cp/ovsrch.exp: break A::outer::foo  (int)  const if (A::outer::func ())
-PASS: gdb.cp/ovsrch.exp: break 'A::outer::foo  (int)  const' if (A::outer::func ())
+FAIL: gdb.cp/ovsrch.exp: break A::outer::foo  (int)  const if (A::outer::func ())
+FAIL: gdb.cp/ovsrch.exp: break 'A::outer::foo  (int)  const' if (A::outer::func ())
-PASS: gdb.cp/ovsrch.exp: break A::B::inner::foo  (char*)  const if (A::outer::func ())
-PASS: gdb.cp/ovsrch.exp: break 'A::B::inner::foo  (char*)  const' if (A::outer::func ())
+FAIL: gdb.cp/ovsrch.exp: break A::B::inner::foo  (char*)  const if (A::outer::func ())
+FAIL: gdb.cp/ovsrch.exp: break 'A::B::inner::foo  (char*)  const' if (A::outer::func ())
-PASS: gdb.cp/ovsrch.exp: break A::B::inner::foo  (int)  const if (A::outer::func ())
-PASS: gdb.cp/ovsrch.exp: break 'A::B::inner::foo  (int)  const' if (A::outer::func ())
+FAIL: gdb.cp/ovsrch.exp: break A::B::inner::foo  (int)  const if (A::outer::func ())
+FAIL: gdb.cp/ovsrch.exp: break 'A::B::inner::foo  (int)  const' if (A::outer::func ())
-# of expected passes           77894
-# of unexpected failures       14
+# of expected passes           77861
+# of unexpected failures       47
...
Comment 5 Tom de Vries 2020-02-12 09:28:25 UTC
(In reply to Tom de Vries from comment #4)
> Difference before and after offending commit using cc-with-dwz board:
> ...
> $ diff -U 0 ref-dwz/gdb.sum gdb.sum | grep -v @@
> -# of expected passes           77894
> -# of unexpected failures       14
> +# of expected passes           77861
> +# of unexpected failures       47
> ...

Unexpected failures 14 -> 47.

Likewise, for cc-with-dwz-m board:
...
$ diff -U 0 ref-dwz-m/gdb.sum gdb.sum | grep -v @@
-# of expected passes           77389
-# of unexpected failures       31
-# of unexpected successes      3
-# of expected failures         128
-# of unknown successes         3
-# of known failures            71
-# of unresolved testcases      1
+# of expected passes           73779
+# of unexpected failures       2960
+# of unexpected successes      4
+# of expected failures         119
+# of known failures            74
+# of unresolved testcases      5
...

Unexpected failures 31 -> 2960.
Comment 6 Tom de Vries 2020-02-12 13:55:45 UTC
The difference in behaviour seems to come from dropping the dependencies[i]->user == NULL test.

Using this patch, we reintroduce it and get the test passing again:
...
diff --git a/gdb/dwarf2read.c b/gdb/dwarf2read.c
index dafe01d94a..c90af761e7 100644
--- a/gdb/dwarf2read.c
+++ b/gdb/dwarf2read.c
@@ -9657,6 +9657,31 @@ process_queue (struct dwarf2_per_objfile *dwarf2_per_objfile)
     }
 }
 
+void
+dwarf2_psymtab::read_dependencies (struct objfile *objfile)
+{
+  int i;
+
+  for (i = 0; i < number_of_dependencies; i++)
+    if (!dependencies[i]->readin
+       && dependencies[i]->user == NULL)
+      {
+        /* Inform about additional files that need to be read in.  */
+        if (info_verbose)
+          {
+           /* FIXME: i18n: Need to make this a single string.  */
+            fputs_filtered (" ", gdb_stdout);
+            wrap_here ("");
+            fputs_filtered ("and ", gdb_stdout);
+            wrap_here ("");
+            printf_filtered ("%s...", dependencies[i]->filename);
+            wrap_here ("");     /* Flush output.  */
+            gdb_flush (gdb_stdout);
+          }
+       dependencies[i]->expand_psymtab (objfile);
+      }
+}
+
 /* Read in full symbols for PST, and anything it depends on.  */
 
 void
diff --git a/gdb/dwarf2read.h b/gdb/dwarf2read.h
index c5b69020d5..f06faf744c 100644
--- a/gdb/dwarf2read.h
+++ b/gdb/dwarf2read.h
@@ -286,6 +286,7 @@ struct dwarf2_psymtab : public partial_symtab
 
   void read_symtab (struct objfile *) override;
   void expand_psymtab (struct objfile *) override;
+  void read_dependencies (struct objfile *objfile) override;
 
   struct dwarf2_per_cu_data *per_cu_data;
 };
diff --git a/gdb/psympriv.h b/gdb/psympriv.h
index e4b23301e9..e9ed0d9df8 100644
--- a/gdb/psympriv.h
+++ b/gdb/psympriv.h
@@ -135,7 +135,7 @@ struct partial_symtab
   virtual void expand_psymtab (struct objfile *) = 0;
 
   /* Ensure that all the dependencies are read in.  */
-  void read_dependencies (struct objfile *);
+  virtual void read_dependencies (struct objfile *);
 
   /* Return the raw low text address of this partial_symtab.  */
   CORE_ADDR raw_text_low () const
...

Tom, was the dropping of the dependencies[i]->user == NULL test intentional?

Is this patch the right way to fix this?
Comment 7 Tom de Vries 2020-02-20 14:26:33 UTC
(In reply to Tom de Vries from comment #6)
> The difference in behaviour seems to come from dropping the
> dependencies[i]->user == NULL test.
> 
> Using this patch, we reintroduce it and get the test passing again:


Submitted as https://sourceware.org/ml/gdb-patches/2020-02/msg00820.html .
Comment 8 Sourceware Commits 2020-02-21 15:37:28 UTC
The master branch has been updated by Tom de Vries <vries@sourceware.org>:

https://sourceware.org/git/gitweb.cgi?p=binutils-gdb.git;h=1eb731795357d77ac1f6200edd04f86d210bec79

commit 1eb731795357d77ac1f6200edd04f86d210bec79
Author: Tom de Vries <tdevries@suse.de>
Date:   Fri Feb 21 16:36:48 2020 +0100

    [gdb] Fix cc-with-dwz regression
    
    I noticed a regression with board cc-with-dwz:
    ...
    FAIL: gdb.cp/m-static.exp: static const int initialized elsewhere
    FAIL: gdb.cp/m-static.exp: info variable everywhere
    ...
    
    The problem started with commit 0494dbecdf "Consolidate partial symtab
    dependency reading".
    
    The commit replaces the dwarf2_psymtab::expand_psymtab specific reading of
    dependencies, which contains a "dependencies[i]->user == NULL" test, with a
    generic partial_symtab::read_dependencies call, which does not test the user
    field.
    
    This patch fixes the regression by adding back the test, in the generic
    partial_symtab::read_dependencies.
    
    Build and reg-tested on x86_64-linux.
    
    Tested natively, as well as with boards cc-with-dwz and cc-with-dwz-m.
    
    The patch fixes all 33 regressions with cc-with-dwz, and all 2929 regression
    with cc-with-dwz-m.
    
    gdb/ChangeLog:
    
    2020-02-21  Tom de Vries  <tdevries@suse.de>
    
    	PR gdb/25534
    	* psymtab.c (partial_symtab::read_dependencies): Don't read dependency
    	if dependencies[i]->user != NULL.
Comment 9 Tom de Vries 2020-02-21 15:37:57 UTC
Patch committed, marking resolved-fixed.