* [PATCH][gdb/testsuite] Use prototype to call libc functions
@ 2022-09-05 12:27 Tom de Vries
2022-09-07 8:01 ` Tom de Vries
2022-09-07 17:41 ` Kevin Buettner
0 siblings, 2 replies; 5+ messages in thread
From: Tom de Vries @ 2022-09-05 12:27 UTC (permalink / raw)
To: gdb-patches
Hi,
On openSUSE Tumbleweed (using glibc 2.36), I run into:
...
(gdb) print /d (int) munmap (4198400, 4096)^M
Invalid cast.^M
(gdb) FAIL: gdb.base/break-main-file-remove-fail.exp: cmdline: \
get integer valueof "(int) munmap (4198400, 4096)"
...
The problem is that after starting the executable, the symbol has type
"void (*) (void)":
...
(gdb) p munmap
$1 = {<text variable, no debug info>} 0x401030 <munmap@plt>
(gdb) start
...
(gdb) p munmap
$2 = {void (void)} 0x7ffff7feb9a0 <__GI_munmap>
...
which causes the "Invalid cast" error.
Looking at the debug info for glibc for symbol __GI_munmap:
...
<0><189683>: Abbrev Number: 1 (DW_TAG_compile_unit)
<189691> DW_AT_name : ../sysdeps/unix/syscall-template.S
<189699> DW_AT_producer : GNU AS 2.39.0
<1><1896ae>: Abbrev Number: 2 (DW_TAG_subprogram)
<1896af> DW_AT_name : __GI___munmap
<1896b3> DW_AT_external : 1
<1896b4> DW_AT_low_pc : 0x10cad0
<1896bc> DW_AT_high_pc : 37
...
that's probably caused by this bit (or similar bits for other munmap aliases).
This is fixed in gas on trunk by commit 5578fbf672e ("GAS: Add a return type
tag to DWARF DIEs generated for function symbols").
Work around this (for say gas 2.39) by explicitly specifying the prototype for
munmap.
Likewise for getpid in a couple of other test-cases.
Tested on x86_64-linux.
Any comments?
Thanks,
- Tom
[gdb/testsuite] Use prototype to call libc functions
---
gdb/testsuite/gdb.base/break-main-file-remove-fail.exp | 4 +++-
gdb/testsuite/gdb.base/dprintf-detach.exp | 2 +-
gdb/testsuite/gdb.base/info-os.exp | 2 +-
gdb/testsuite/gdb.threads/siginfo-threads.exp | 2 +-
4 files changed, 6 insertions(+), 4 deletions(-)
diff --git a/gdb/testsuite/gdb.base/break-main-file-remove-fail.exp b/gdb/testsuite/gdb.base/break-main-file-remove-fail.exp
index 41c3114aa05..444b1d33ace 100644
--- a/gdb/testsuite/gdb.base/break-main-file-remove-fail.exp
+++ b/gdb/testsuite/gdb.base/break-main-file-remove-fail.exp
@@ -87,7 +87,9 @@ proc test_remove_bp { initial_load } {
# should warn the user about it.
set pagesize [get_integer_valueof "pg_size" 0]
set align_addr [expr $bp_addr - $bp_addr % $pagesize]
- set munmap [get_integer_valueof "(int) munmap ($align_addr, $pagesize)" -1]
+ set munmap_prototype "int (*) (void *, size_t)"
+ set munmap_expr "(($munmap_prototype) munmap) ($align_addr, $pagesize)"
+ set munmap [get_integer_valueof $munmap_expr -1]
if {$munmap != 0} {
unsupported "can't munmap foo's page"
diff --git a/gdb/testsuite/gdb.base/dprintf-detach.exp b/gdb/testsuite/gdb.base/dprintf-detach.exp
index 692048f82a3..b6da01a2845 100644
--- a/gdb/testsuite/gdb.base/dprintf-detach.exp
+++ b/gdb/testsuite/gdb.base/dprintf-detach.exp
@@ -52,7 +52,7 @@ proc dprintf_detach_test { breakpoint_always_inserted dprintf_style disconnected
# Get PID of test program.
set inferior_pid -1
set test "get inferior process ID"
- gdb_test_multiple "call (int) getpid ()" $test {
+ gdb_test_multiple "call ((int (*) (void)) getpid) ()" $test {
-re ".* = ($decimal).*$gdb_prompt $" {
set inferior_pid $expect_out(1,string)
pass $test
diff --git a/gdb/testsuite/gdb.base/info-os.exp b/gdb/testsuite/gdb.base/info-os.exp
index 7967e2e43df..d12f8750a9c 100644
--- a/gdb/testsuite/gdb.base/info-os.exp
+++ b/gdb/testsuite/gdb.base/info-os.exp
@@ -39,7 +39,7 @@ if ![runto_main] then {
# Get PID of test program.
set inferior_pid ""
set test "get inferior process ID"
-gdb_test_multiple "call (int) getpid()" $test {
+gdb_test_multiple "call ((int (*) (void)) getpid) ()" $test {
-re ".* = ($decimal).*$gdb_prompt $" {
set inferior_pid $expect_out(1,string)
pass $test
diff --git a/gdb/testsuite/gdb.threads/siginfo-threads.exp b/gdb/testsuite/gdb.threads/siginfo-threads.exp
index f10c7fdcd25..8dd34b753a2 100644
--- a/gdb/testsuite/gdb.threads/siginfo-threads.exp
+++ b/gdb/testsuite/gdb.threads/siginfo-threads.exp
@@ -41,7 +41,7 @@ gdb_breakpoint [gdb_get_line_number "break-at-exit"]
set test "get pid"
set pid ""
-gdb_test_multiple "p (int) getpid ()" $test {
+gdb_test_multiple "p ((int (*) (void))getpid) ()" $test {
-re " = (\[0-9\]+)\r\n$gdb_prompt $" {
set pid $expect_out(1,string)
pass $test
^ permalink raw reply [flat|nested] 5+ messages in thread* Re: [PATCH][gdb/testsuite] Use prototype to call libc functions
2022-09-05 12:27 [PATCH][gdb/testsuite] Use prototype to call libc functions Tom de Vries
@ 2022-09-07 8:01 ` Tom de Vries
2022-09-07 17:41 ` Kevin Buettner
1 sibling, 0 replies; 5+ messages in thread
From: Tom de Vries @ 2022-09-07 8:01 UTC (permalink / raw)
To: gdb-patches; +Cc: Mark Wielaard, Kevin Buettner
On 9/5/22 14:27, Tom de Vries via Gdb-patches wrote:
> Hi,
>
> On openSUSE Tumbleweed (using glibc 2.36), I run into:
> ...
> (gdb) print /d (int) munmap (4198400, 4096)^M
> Invalid cast.^M
> (gdb) FAIL: gdb.base/break-main-file-remove-fail.exp: cmdline: \
> get integer valueof "(int) munmap (4198400, 4096)"
> ...
>
> The problem is that after starting the executable, the symbol has type
> "void (*) (void)":
> ...
> (gdb) p munmap
> $1 = {<text variable, no debug info>} 0x401030 <munmap@plt>
> (gdb) start
> ...
> (gdb) p munmap
> $2 = {void (void)} 0x7ffff7feb9a0 <__GI_munmap>
> ...
> which causes the "Invalid cast" error.
>
> Looking at the debug info for glibc for symbol __GI_munmap:
> ...
> <0><189683>: Abbrev Number: 1 (DW_TAG_compile_unit)
> <189691> DW_AT_name : ../sysdeps/unix/syscall-template.S
> <189699> DW_AT_producer : GNU AS 2.39.0
> <1><1896ae>: Abbrev Number: 2 (DW_TAG_subprogram)
> <1896af> DW_AT_name : __GI___munmap
> <1896b3> DW_AT_external : 1
> <1896b4> DW_AT_low_pc : 0x10cad0
> <1896bc> DW_AT_high_pc : 37
> ...
> that's probably caused by this bit (or similar bits for other munmap aliases).
>
> This is fixed in gas on trunk by commit 5578fbf672e ("GAS: Add a return type
> tag to DWARF DIEs generated for function symbols").
>
> Work around this (for say gas 2.39) by explicitly specifying the prototype for
> munmap.
>
> Likewise for getpid in a couple of other test-cases.
>
> Tested on x86_64-linux.
>
> Any comments?
>
I've pushed this, given that it fixes gdb-rawhide-x86_64 buildbot.
Thanks,
- Tom
> [gdb/testsuite] Use prototype to call libc functions
>
> ---
> gdb/testsuite/gdb.base/break-main-file-remove-fail.exp | 4 +++-
> gdb/testsuite/gdb.base/dprintf-detach.exp | 2 +-
> gdb/testsuite/gdb.base/info-os.exp | 2 +-
> gdb/testsuite/gdb.threads/siginfo-threads.exp | 2 +-
> 4 files changed, 6 insertions(+), 4 deletions(-)
>
> diff --git a/gdb/testsuite/gdb.base/break-main-file-remove-fail.exp b/gdb/testsuite/gdb.base/break-main-file-remove-fail.exp
> index 41c3114aa05..444b1d33ace 100644
> --- a/gdb/testsuite/gdb.base/break-main-file-remove-fail.exp
> +++ b/gdb/testsuite/gdb.base/break-main-file-remove-fail.exp
> @@ -87,7 +87,9 @@ proc test_remove_bp { initial_load } {
> # should warn the user about it.
> set pagesize [get_integer_valueof "pg_size" 0]
> set align_addr [expr $bp_addr - $bp_addr % $pagesize]
> - set munmap [get_integer_valueof "(int) munmap ($align_addr, $pagesize)" -1]
> + set munmap_prototype "int (*) (void *, size_t)"
> + set munmap_expr "(($munmap_prototype) munmap) ($align_addr, $pagesize)"
> + set munmap [get_integer_valueof $munmap_expr -1]
>
> if {$munmap != 0} {
> unsupported "can't munmap foo's page"
> diff --git a/gdb/testsuite/gdb.base/dprintf-detach.exp b/gdb/testsuite/gdb.base/dprintf-detach.exp
> index 692048f82a3..b6da01a2845 100644
> --- a/gdb/testsuite/gdb.base/dprintf-detach.exp
> +++ b/gdb/testsuite/gdb.base/dprintf-detach.exp
> @@ -52,7 +52,7 @@ proc dprintf_detach_test { breakpoint_always_inserted dprintf_style disconnected
> # Get PID of test program.
> set inferior_pid -1
> set test "get inferior process ID"
> - gdb_test_multiple "call (int) getpid ()" $test {
> + gdb_test_multiple "call ((int (*) (void)) getpid) ()" $test {
> -re ".* = ($decimal).*$gdb_prompt $" {
> set inferior_pid $expect_out(1,string)
> pass $test
> diff --git a/gdb/testsuite/gdb.base/info-os.exp b/gdb/testsuite/gdb.base/info-os.exp
> index 7967e2e43df..d12f8750a9c 100644
> --- a/gdb/testsuite/gdb.base/info-os.exp
> +++ b/gdb/testsuite/gdb.base/info-os.exp
> @@ -39,7 +39,7 @@ if ![runto_main] then {
> # Get PID of test program.
> set inferior_pid ""
> set test "get inferior process ID"
> -gdb_test_multiple "call (int) getpid()" $test {
> +gdb_test_multiple "call ((int (*) (void)) getpid) ()" $test {
> -re ".* = ($decimal).*$gdb_prompt $" {
> set inferior_pid $expect_out(1,string)
> pass $test
> diff --git a/gdb/testsuite/gdb.threads/siginfo-threads.exp b/gdb/testsuite/gdb.threads/siginfo-threads.exp
> index f10c7fdcd25..8dd34b753a2 100644
> --- a/gdb/testsuite/gdb.threads/siginfo-threads.exp
> +++ b/gdb/testsuite/gdb.threads/siginfo-threads.exp
> @@ -41,7 +41,7 @@ gdb_breakpoint [gdb_get_line_number "break-at-exit"]
>
> set test "get pid"
> set pid ""
> -gdb_test_multiple "p (int) getpid ()" $test {
> +gdb_test_multiple "p ((int (*) (void))getpid) ()" $test {
> -re " = (\[0-9\]+)\r\n$gdb_prompt $" {
> set pid $expect_out(1,string)
> pass $test
^ permalink raw reply [flat|nested] 5+ messages in thread* Re: [PATCH][gdb/testsuite] Use prototype to call libc functions
2022-09-05 12:27 [PATCH][gdb/testsuite] Use prototype to call libc functions Tom de Vries
2022-09-07 8:01 ` Tom de Vries
@ 2022-09-07 17:41 ` Kevin Buettner
2022-09-08 9:26 ` Tom de Vries
2022-09-08 10:15 ` Luis Machado
1 sibling, 2 replies; 5+ messages in thread
From: Kevin Buettner @ 2022-09-07 17:41 UTC (permalink / raw)
To: Tom de Vries via Gdb-patches; +Cc: Tom de Vries
On Mon, 5 Sep 2022 14:27:07 +0200
Tom de Vries via Gdb-patches <gdb-patches@sourceware.org> wrote:
> Hi,
>
> On openSUSE Tumbleweed (using glibc 2.36), I run into:
> ...
> (gdb) print /d (int) munmap (4198400, 4096)^M
> Invalid cast.^M
> (gdb) FAIL: gdb.base/break-main-file-remove-fail.exp: cmdline: \
> get integer valueof "(int) munmap (4198400, 4096)"
> ...
>
> The problem is that after starting the executable, the symbol has type
> "void (*) (void)":
> ...
> (gdb) p munmap
> $1 = {<text variable, no debug info>} 0x401030 <munmap@plt>
> (gdb) start
> ...
> (gdb) p munmap
> $2 = {void (void)} 0x7ffff7feb9a0 <__GI_munmap>
> ...
> which causes the "Invalid cast" error.
>
> Looking at the debug info for glibc for symbol __GI_munmap:
> ...
> <0><189683>: Abbrev Number: 1 (DW_TAG_compile_unit)
> <189691> DW_AT_name : ../sysdeps/unix/syscall-template.S
> <189699> DW_AT_producer : GNU AS 2.39.0
> <1><1896ae>: Abbrev Number: 2 (DW_TAG_subprogram)
> <1896af> DW_AT_name : __GI___munmap
> <1896b3> DW_AT_external : 1
> <1896b4> DW_AT_low_pc : 0x10cad0
> <1896bc> DW_AT_high_pc : 37
> ...
> that's probably caused by this bit (or similar bits for other munmap aliases).
>
> This is fixed in gas on trunk by commit 5578fbf672e ("GAS: Add a return type
> tag to DWARF DIEs generated for function symbols").
>
> Work around this (for say gas 2.39) by explicitly specifying the prototype for
> munmap.
>
> Likewise for getpid in a couple of other test-cases.
>
> Tested on x86_64-linux.
>
> Any comments?
I have mixed feelings about patches like this.
One the one hand, it's nice to have the test "fixed" so that it doesn't
fail.
But, on the other hand, this failure found a problem with the debug info
in glibc, so fixing it as you did here will remove that test.
Perhaps you could add a new test which will still fail when the debug info
incorrectly specifies that munmap has a void return type?
Kevin
^ permalink raw reply [flat|nested] 5+ messages in thread* Re: [PATCH][gdb/testsuite] Use prototype to call libc functions
2022-09-07 17:41 ` Kevin Buettner
@ 2022-09-08 9:26 ` Tom de Vries
2022-09-08 10:15 ` Luis Machado
1 sibling, 0 replies; 5+ messages in thread
From: Tom de Vries @ 2022-09-08 9:26 UTC (permalink / raw)
To: Kevin Buettner, Tom de Vries via Gdb-patches
On 9/7/22 19:41, Kevin Buettner wrote:
> On Mon, 5 Sep 2022 14:27:07 +0200
> Tom de Vries via Gdb-patches <gdb-patches@sourceware.org> wrote:
>
>> Hi,
>>
>> On openSUSE Tumbleweed (using glibc 2.36), I run into:
>> ...
>> (gdb) print /d (int) munmap (4198400, 4096)^M
>> Invalid cast.^M
>> (gdb) FAIL: gdb.base/break-main-file-remove-fail.exp: cmdline: \
>> get integer valueof "(int) munmap (4198400, 4096)"
>> ...
>>
>> The problem is that after starting the executable, the symbol has type
>> "void (*) (void)":
>> ...
>> (gdb) p munmap
>> $1 = {<text variable, no debug info>} 0x401030 <munmap@plt>
>> (gdb) start
>> ...
>> (gdb) p munmap
>> $2 = {void (void)} 0x7ffff7feb9a0 <__GI_munmap>
>> ...
>> which causes the "Invalid cast" error.
>>
>> Looking at the debug info for glibc for symbol __GI_munmap:
>> ...
>> <0><189683>: Abbrev Number: 1 (DW_TAG_compile_unit)
>> <189691> DW_AT_name : ../sysdeps/unix/syscall-template.S
>> <189699> DW_AT_producer : GNU AS 2.39.0
>> <1><1896ae>: Abbrev Number: 2 (DW_TAG_subprogram)
>> <1896af> DW_AT_name : __GI___munmap
>> <1896b3> DW_AT_external : 1
>> <1896b4> DW_AT_low_pc : 0x10cad0
>> <1896bc> DW_AT_high_pc : 37
>> ...
>> that's probably caused by this bit (or similar bits for other munmap aliases).
>>
>> This is fixed in gas on trunk by commit 5578fbf672e ("GAS: Add a return type
>> tag to DWARF DIEs generated for function symbols").
>>
>> Work around this (for say gas 2.39) by explicitly specifying the prototype for
>> munmap.
>>
>> Likewise for getpid in a couple of other test-cases.
>>
>> Tested on x86_64-linux.
>>
>> Any comments?
>
> I have mixed feelings about patches like this.
>
> One the one hand, it's nice to have the test "fixed" so that it doesn't
> fail.
>
> But, on the other hand, this failure found a problem with the debug info
> in glibc, so fixing it as you did here will remove that test.
>
> Perhaps you could add a new test which will still fail when the debug info
> incorrectly specifies that munmap has a void return type?
Hi Kevin,
thanks for the review.
Alternatively, I could have marked these xfails, because they represent
a failure in the environment, not in gdb. But I think that if we can
avoid xfails by working around them, we should, because it simplifies
testsuite maintenance.
The fact that we no longer use that libc debuginfo is fine from the
point of view of the gdb testsuite being a regression test suite for gdb.
That does beg the question, where else would libc debuginfo be tested,
and I don't know where, so perhaps we could have some dedicated gdb.libc
testsuite subdir for that. I would put the test you propose in there.
What is useful about the proposed test is that it eventually (once the
new binutils release makes it into the various distros) will test the
munmap debuginfo as generated by the fixed gas, that is, using
DW_TAG_unspecified_type.
We don't seem to have a dedicated test for this tag. There's some
ada-specific bit in the code in gdb/dwarf/read.c handling this, which is
supposed to be tested by gdb.ada/taft_type.exp, but how it will behave
with the fixed gas, I have no idea.
So, can we already test this now? Let's try:
https://sourceware.org/bugzilla/show_bug.cgi?id=29558 . AFAIU, the
generated dwarf is incorrect, so I've filed a gas PR for that.
We can rewrite this into a gdb test-case, which would test gas debuginfo
(so gdb.gas subdir?), but I guess we'd probably also want a dwarf
assembly test-case.
Thanks,
- Tom
^ permalink raw reply [flat|nested] 5+ messages in thread* Re: [PATCH][gdb/testsuite] Use prototype to call libc functions
2022-09-07 17:41 ` Kevin Buettner
2022-09-08 9:26 ` Tom de Vries
@ 2022-09-08 10:15 ` Luis Machado
1 sibling, 0 replies; 5+ messages in thread
From: Luis Machado @ 2022-09-08 10:15 UTC (permalink / raw)
To: Kevin Buettner, Tom de Vries via Gdb-patches; +Cc: Tom de Vries
On 9/7/22 18:41, Kevin Buettner via Gdb-patches wrote:
> On Mon, 5 Sep 2022 14:27:07 +0200
> Tom de Vries via Gdb-patches <gdb-patches@sourceware.org> wrote:
>
>> Hi,
>>
>> On openSUSE Tumbleweed (using glibc 2.36), I run into:
>> ...
>> (gdb) print /d (int) munmap (4198400, 4096)^M
>> Invalid cast.^M
>> (gdb) FAIL: gdb.base/break-main-file-remove-fail.exp: cmdline: \
>> get integer valueof "(int) munmap (4198400, 4096)"
>> ...
>>
>> The problem is that after starting the executable, the symbol has type
>> "void (*) (void)":
>> ...
>> (gdb) p munmap
>> $1 = {<text variable, no debug info>} 0x401030 <munmap@plt>
>> (gdb) start
>> ...
>> (gdb) p munmap
>> $2 = {void (void)} 0x7ffff7feb9a0 <__GI_munmap>
>> ...
>> which causes the "Invalid cast" error.
>>
>> Looking at the debug info for glibc for symbol __GI_munmap:
>> ...
>> <0><189683>: Abbrev Number: 1 (DW_TAG_compile_unit)
>> <189691> DW_AT_name : ../sysdeps/unix/syscall-template.S
>> <189699> DW_AT_producer : GNU AS 2.39.0
>> <1><1896ae>: Abbrev Number: 2 (DW_TAG_subprogram)
>> <1896af> DW_AT_name : __GI___munmap
>> <1896b3> DW_AT_external : 1
>> <1896b4> DW_AT_low_pc : 0x10cad0
>> <1896bc> DW_AT_high_pc : 37
>> ...
>> that's probably caused by this bit (or similar bits for other munmap aliases).
>>
>> This is fixed in gas on trunk by commit 5578fbf672e ("GAS: Add a return type
>> tag to DWARF DIEs generated for function symbols").
>>
>> Work around this (for say gas 2.39) by explicitly specifying the prototype for
>> munmap.
>>
>> Likewise for getpid in a couple of other test-cases.
>>
>> Tested on x86_64-linux.
>>
>> Any comments?
>
> I have mixed feelings about patches like this.
>
> One the one hand, it's nice to have the test "fixed" so that it doesn't
> fail.
>
> But, on the other hand, this failure found a problem with the debug info
> in glibc, so fixing it as you did here will remove that test.
>
> Perhaps you could add a new test which will still fail when the debug info
> incorrectly specifies that munmap has a void return type?
>
> Kevin
>
My $0.02 is that the GDB testsuite is already very often affected by bugs in other components (compiler, debug info, linker, gas etc), which make it
even harder to have a somewhat clean GDB testsuite run.
I'd say it is the responsibility of whatever component contains the bug to have a test for it and fix it. In this case, gas.
The XFAIL idea is also OK in my opinion.
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2022-09-08 10:16 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-09-05 12:27 [PATCH][gdb/testsuite] Use prototype to call libc functions Tom de Vries
2022-09-07 8:01 ` Tom de Vries
2022-09-07 17:41 ` Kevin Buettner
2022-09-08 9:26 ` Tom de Vries
2022-09-08 10:15 ` Luis Machado
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).