[PATCH][gdb/testsuite] Dump /proc/cpuinfo into gdb.log

[PATCH][gdb/testsuite] Dump /proc/cpuinfo into gdb.log

[Tom de Vries via Gdb-patches]

Hi,

When interpreting the testsuite results, it's often relevant what kind of
machine the testsuite ran on.  On a local machine one can just do
/proc/cpuinfo, but in case of running tests using a remote system
that distributes test runs to other remote systems that are not directly
accessible, that's not possible.

Fix this by dumping /proc/cpuinfo into the gdb.log.

We could do this at the start of each test run, by putting it into unix.exp
or some such.  However, this might be too verbose, so we choose to put it into
its own test-case, such that it get triggered in a full testrun, but not when
running one or a subset of tests.

We put the test-case into the gdb.testsuite directory, which is currently the
only place in the testsuite where we do not test gdb.   Though perhaps this
should be put into a new gdb.info directory, since the test-case doesn't
actually test the testsuite.

Tested on x86_64-linux.

Any comments?

Thanks,
- Tom

[gdb/testsuite] Dump /proc/cpuinfo into gdb.log

---
 gdb/testsuite/gdb.testsuite/dump-cpuinfo.exp | 27 +++++++++++++++++++++++++++
 1 file changed, 27 insertions(+)

diff --git a/gdb/testsuite/gdb.testsuite/dump-cpuinfo.exp b/gdb/testsuite/gdb.testsuite/dump-cpuinfo.exp
new file mode 100644
index 00000000000..ee94aa1ae32
--- /dev/null
+++ b/gdb/testsuite/gdb.testsuite/dump-cpuinfo.exp
@@ -0,0 +1,27 @@
+# Copyright 2021 Free Software Foundation, Inc.
+# This program is free software; you can redistribute it and/or modify
+# it under the terms of the GNU General Public License as published by
+# the Free Software Foundation; either version 3 of the License, or
+# (at your option) any later version.
+#
+# This program is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+# GNU General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with this program.  If not, see <http://www.gnu.org/licenses/>.
+
+# The purpose of this test-case is to dump /proc/cpuinfo into gdb.log.
+
+# Check if /proc/cpuinfo is available.
+set res [remote_exec target "test -r /proc/cpuinfo"]
+set status [lindex $res 0]
+set output [lindex $res 1]
+
+if { $status == 0 && $output == "" } {
+    verbose -log "Cpuinfo available, dumping:"
+    remote_exec target "cat /proc/cpuinfo"
+} else {
+    verbose -log "Cpuinfo not available"
+}

Re: [PATCH][gdb/testsuite] Dump /proc/cpuinfo into gdb.log

[Simon Marchi via Gdb-patches]

On 2021-09-21 4:01 a.m., Tom de Vries via Gdb-patches wrote:
> Hi,
> 
> When interpreting the testsuite results, it's often relevant what kind of
> machine the testsuite ran on.  On a local machine one can just do
> /proc/cpuinfo, but in case of running tests using a remote system
> that distributes test runs to other remote systems that are not directly
> accessible, that's not possible.
> 
> Fix this by dumping /proc/cpuinfo into the gdb.log.
> 
> We could do this at the start of each test run, by putting it into unix.exp
> or some such.  However, this might be too verbose, so we choose to put it into
> its own test-case, such that it get triggered in a full testrun, but not when
> running one or a subset of tests.
> 
> We put the test-case into the gdb.testsuite directory, which is currently the
> only place in the testsuite where we do not test gdb.   Though perhaps this
> should be put into a new gdb.info directory, since the test-case doesn't
> actually test the testsuite.

I think in the gdb.testsuite directory is fine.

I like the idea.  I even think it would be useful to dump more things
about the system, like:

 - "--version" output of compilers used for testing
 - "lsb_release -a" output (if lsb_release is available)
 - "uname -a" output (if uname is available)

Can you think of more?

Simon

Re: [PATCH][gdb/testsuite] Dump /proc/cpuinfo into gdb.log

[Tom de Vries via Gdb-patches]

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

On 9/23/21 4:32 PM, Simon Marchi wrote:
> 
> 
> On 2021-09-21 4:01 a.m., Tom de Vries via Gdb-patches wrote:
>> Hi,
>>
>> When interpreting the testsuite results, it's often relevant what kind of
>> machine the testsuite ran on.  On a local machine one can just do
>> /proc/cpuinfo, but in case of running tests using a remote system
>> that distributes test runs to other remote systems that are not directly
>> accessible, that's not possible.
>>
>> Fix this by dumping /proc/cpuinfo into the gdb.log.
>>
>> We could do this at the start of each test run, by putting it into unix.exp
>> or some such.  However, this might be too verbose, so we choose to put it into
>> its own test-case, such that it get triggered in a full testrun, but not when
>> running one or a subset of tests.
>>
>> We put the test-case into the gdb.testsuite directory, which is currently the
>> only place in the testsuite where we do not test gdb.   Though perhaps this
>> should be put into a new gdb.info directory, since the test-case doesn't
>> actually test the testsuite.
> 
> I think in the gdb.testsuite directory is fine.
> 

Ack (still leaving the comment in the log message though).

> I like the idea.  I even think it would be useful to dump more things
> about the system, like:
> 
>  - "--version" output of compilers used for testing
>  - "lsb_release -a" output (if lsb_release is available)
>  - "uname -a" output (if uname is available)

I've added the latter two (and renamed the test-case to
dump-system-info.exp).

I'm not sure about compiler version, ISTM we already have that
information in the log (though you may have to grep for it).

> Can you think of more?
> 

Atm not, no.  I guess we can add if and when we think of something else.
 At least this gives us a place to add it to.

I'll commit tomorrow unless there are further comments.

Thanks,
- Tom

> Simon
> 

[-- Attachment #2: 0002-gdb-testsuite-Add-gdb.testsuite-dump-system-info.exp.patch --]
[-- Type: text/x-patch, Size: 3014 bytes --]

[gdb/testsuite] Add gdb.testsuite/dump-system-info.exp

When interpreting the testsuite results, it's often relevant what kind of
machine the testsuite ran on.  On a local machine one can just do
/proc/cpuinfo, but in case of running tests using a remote system
that distributes test runs to other remote systems that are not directly
accessible, that's not possible.

Fix this by dumping /proc/cpuinfo into the gdb.log, as well as lsb_release -a
and uname -a.

We could do this at the start of each test run, by putting it into unix.exp
or some such.  However, this might be too verbose, so we choose to put it into
its own test-case, such that it get triggered in a full testrun, but not when
running one or a subset of tests.

We put the test-case into the gdb.testsuite directory, which is currently the
only place in the testsuite where we do not test gdb.   [ Though perhaps this
could be put into a new gdb.info directory, since the test-case doesn't
actually test the testsuite. ]

Tested on x86_64-linux.

---
 gdb/testsuite/gdb.testsuite/dump-system-info.exp | 48 ++++++++++++++++++++++++
 1 file changed, 48 insertions(+)

diff --git a/gdb/testsuite/gdb.testsuite/dump-system-info.exp b/gdb/testsuite/gdb.testsuite/dump-system-info.exp
new file mode 100644
index 00000000000..bf181469bd5
--- /dev/null
+++ b/gdb/testsuite/gdb.testsuite/dump-system-info.exp
@@ -0,0 +1,48 @@
+# Copyright 2021 Free Software Foundation, Inc.
+# This program is free software; you can redistribute it and/or modify
+# it under the terms of the GNU General Public License as published by
+# the Free Software Foundation; either version 3 of the License, or
+# (at your option) any later version.
+#
+# This program is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+# GNU General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with this program.  If not, see <http://www.gnu.org/licenses/>.
+
+# The purpose of this test-case is to dump /proc/cpuinfo and similar system
+# info into gdb.log.
+
+# Check if /proc/cpuinfo is available.
+set res [remote_exec target "test -r /proc/cpuinfo"]
+set status [lindex $res 0]
+set output [lindex $res 1]
+
+if { $status == 0 && $output == "" } {
+    verbose -log "Cpuinfo available, dumping:"
+    remote_exec target "cat /proc/cpuinfo"
+} else {
+    verbose -log "Cpuinfo not available"
+}
+
+set res [remote_exec target "lsb_release -a"]
+set status [lindex $res 0]
+set output [lindex $res 1]
+
+if { $status == 0 } {
+    verbose -log "lsb_release -a availabe, dumping:\n$output"
+} else {
+    verbose -log "lsb_release -a not available"
+}
+
+set res [remote_exec target "uname -a"]
+set status [lindex $res 0]
+set output [lindex $res 1]
+
+if { $status == 0 } {
+    verbose -log "uname -a availabe, dumping:\n$output"
+} else {
+    verbose -log "uname -a not available"
+}

Re: [PATCH][gdb/testsuite] Dump /proc/cpuinfo into gdb.log

[Pedro Alves]

On 2021-09-23 11:06 p.m., Tom de Vries via Gdb-patches wrote:

>  gdb/testsuite/gdb.testsuite/dump-system-info.exp | 48 ++++++++++++++++++++++++
>  1 file changed, 48 insertions(+)
> 
> diff --git a/gdb/testsuite/gdb.testsuite/dump-system-info.exp b/gdb/testsuite/gdb.testsuite/dump-system-info.exp
> new file mode 100644
> index 00000000000..bf181469bd5
> --- /dev/null
> +++ b/gdb/testsuite/gdb.testsuite/dump-system-info.exp
> @@ -0,0 +1,48 @@
> +# Copyright 2021 Free Software Foundation, Inc.
> +# This program is free software; you can redistribute it and/or modify
> +# it under the terms of the GNU General Public License as published by
> +# the Free Software Foundation; either version 3 of the License, or
> +# (at your option) any later version.
> +#
> +# This program is distributed in the hope that it will be useful,
> +# but WITHOUT ANY WARRANTY; without even the implied warranty of
> +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> +# GNU General Public License for more details.
> +#
> +# You should have received a copy of the GNU General Public License
> +# along with this program.  If not, see <http://www.gnu.org/licenses/>.
> +
> +# The purpose of this test-case is to dump /proc/cpuinfo and similar system
> +# info into gdb.log.
> +
> +# Check if /proc/cpuinfo is available.
> +set res [remote_exec target "test -r /proc/cpuinfo"]
> +set status [lindex $res 0]
> +set output [lindex $res 1]

OOC, why "test -r" -> "cat" instead of "cat" straight away, which
is basically what is done for the other dumps?

Consider factoring out a proc, like (untested, written in email):

proc dump_info {cmd {what ""}} {

  if {$what == ""} {
    set what $cmd
  }

  set res [remote_exec target $cmd]
  set status [lindex $res 0]
  set output [lindex $res 1]

  if { $status == 0 } {
    verbose -log "$what available, dumping:\n$output"
  } else {
    verbose -log "$what not available"
  }
}

dump_info "cat /proc/cpuinfo" "Cpuinfo"
dump_info "uname -a"
dump_info "lsb_release -a"

> +
> +if { $status == 0 && $output == "" } {
> +    verbose -log "Cpuinfo available, dumping:"
> +    remote_exec target "cat /proc/cpuinfo"
> +} else {
> +    verbose -log "Cpuinfo not available"
> +}
> +
> +set res [remote_exec target "lsb_release -a"]
> +set status [lindex $res 0]
> +set output [lindex $res 1]
> +
> +if { $status == 0 } {
> +    verbose -log "lsb_release -a availabe, dumping:\n$output"

Typo: "availabe".

> +} else {
> +    verbose -log "lsb_release -a not available"
> +}
> +
> +set res [remote_exec target "uname -a"]
> +set status [lindex $res 0]
> +set output [lindex $res 1]
> +
> +if { $status == 0 } {
> +    verbose -log "uname -a availabe, dumping:\n$output"
> +} else {
> +    verbose -log "uname -a not available"
> +}
> 

Re: [PATCH][gdb/testsuite] Dump /proc/cpuinfo into gdb.log

[Tom de Vries via Gdb-patches]

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

On 9/24/21 2:21 PM, Pedro Alves wrote:
> On 2021-09-23 11:06 p.m., Tom de Vries via Gdb-patches wrote:
> 
>>  gdb/testsuite/gdb.testsuite/dump-system-info.exp | 48 ++++++++++++++++++++++++
>>  1 file changed, 48 insertions(+)
>>
>> diff --git a/gdb/testsuite/gdb.testsuite/dump-system-info.exp b/gdb/testsuite/gdb.testsuite/dump-system-info.exp
>> new file mode 100644
>> index 00000000000..bf181469bd5
>> --- /dev/null
>> +++ b/gdb/testsuite/gdb.testsuite/dump-system-info.exp
>> @@ -0,0 +1,48 @@
>> +# Copyright 2021 Free Software Foundation, Inc.
>> +# This program is free software; you can redistribute it and/or modify
>> +# it under the terms of the GNU General Public License as published by
>> +# the Free Software Foundation; either version 3 of the License, or
>> +# (at your option) any later version.
>> +#
>> +# This program is distributed in the hope that it will be useful,
>> +# but WITHOUT ANY WARRANTY; without even the implied warranty of
>> +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
>> +# GNU General Public License for more details.
>> +#
>> +# You should have received a copy of the GNU General Public License
>> +# along with this program.  If not, see <http://www.gnu.org/licenses/>.
>> +
>> +# The purpose of this test-case is to dump /proc/cpuinfo and similar system
>> +# info into gdb.log.
>> +
>> +# Check if /proc/cpuinfo is available.
>> +set res [remote_exec target "test -r /proc/cpuinfo"]
>> +set status [lindex $res 0]
>> +set output [lindex $res 1]
> 

Hi,

thanks for the review.

> OOC, why "test -r" -> "cat" instead of "cat" straight away, which
> is basically what is done for the other dumps?
> 

The other cases are commands without file argument, this is a command
with file argument.  So, I was trying to not cause errors due to missing
file.

But you're right, it's not really necesssary.

> Consider factoring out a proc, like (untested, written in email):
> 

Copied from mail, and done ... which also fixes the typo you reported.

I'll commit this unless there are further comments.

Thanks,
- Tom



[-- Attachment #2: 0002-gdb-testsuite-Factor-out-dump_info-in-gdb.testsuite-dump-system-info.exp.patch --]
[-- Type: text/x-patch, Size: 2020 bytes --]

[gdb/testsuite] Factor out dump_info in gdb.testsuite/dump-system-info.exp

Factor out new proc dump_info, and in the process:
- fix a few typos
- remove unnecessary "test -r /proc/cpuinfo"

Tested on x86_64-linux.

Co-Authored-By: Pedro Alves <pedro@palves.net>

---
 gdb/testsuite/gdb.testsuite/dump-system-info.exp | 42 +++++++++---------------
 1 file changed, 16 insertions(+), 26 deletions(-)

diff --git a/gdb/testsuite/gdb.testsuite/dump-system-info.exp b/gdb/testsuite/gdb.testsuite/dump-system-info.exp
index bf181469bd5..1831479265c 100644
--- a/gdb/testsuite/gdb.testsuite/dump-system-info.exp
+++ b/gdb/testsuite/gdb.testsuite/dump-system-info.exp
@@ -15,34 +15,24 @@
 # The purpose of this test-case is to dump /proc/cpuinfo and similar system
 # info into gdb.log.
 
-# Check if /proc/cpuinfo is available.
-set res [remote_exec target "test -r /proc/cpuinfo"]
-set status [lindex $res 0]
-set output [lindex $res 1]
 
-if { $status == 0 && $output == "" } {
-    verbose -log "Cpuinfo available, dumping:"
-    remote_exec target "cat /proc/cpuinfo"
-} else {
-    verbose -log "Cpuinfo not available"
-}
-
-set res [remote_exec target "lsb_release -a"]
-set status [lindex $res 0]
-set output [lindex $res 1]
+proc dump_info {cmd {what ""}} {
 
-if { $status == 0 } {
-    verbose -log "lsb_release -a availabe, dumping:\n$output"
-} else {
-    verbose -log "lsb_release -a not available"
-}
+  if {$what == ""} {
+    set what $cmd
+  }
 
-set res [remote_exec target "uname -a"]
-set status [lindex $res 0]
-set output [lindex $res 1]
+  set res [remote_exec target $cmd]
+  set status [lindex $res 0]
+  set output [lindex $res 1]
 
-if { $status == 0 } {
-    verbose -log "uname -a availabe, dumping:\n$output"
-} else {
-    verbose -log "uname -a not available"
+  if { $status == 0 } {
+    verbose -log "$what available, dumping:\n$output"
+  } else {
+    verbose -log "$what not available"
+  }
 }
+
+dump_info "cat /proc/cpuinfo" "Cpuinfo"
+dump_info "uname -a"
+dump_info "lsb_release -a"

Re: [PATCH][gdb/testsuite] Dump /proc/cpuinfo into gdb.log

[Pedro Alves]

On 2021-09-24 1:48 p.m., Tom de Vries wrote:

> Copied from mail, and done ... which also fixes the typo you reported.
> 
> I'll commit this unless there are further comments.
> 

No further comments from me, thanks.