[v4,11/14] xfail gdb.base/coredump-filter.exp test which now works without a binary

Message ID 20200705225807.2264705-12-kevinb@redhat.com
State New
Headers show
Series
  • Fix BZ 25631 - core file memory access problem
Related show

Commit Message

Simon Marchi via Gdb-patches July 5, 2020, 10:58 p.m.
See comment in patch for a description of what this is about.  In
a nutshell, a certain memory access was expected to not work in
order to PASS, but due to recent changes, the memory access now
works causing the test to FAIL.

gdb/testsuite/ChangeLog:

	* gdb.base/coredump-filter.exp (test_disasm):  Call
	setup_xfail for test "disassemble function with corefile and
	without a binary".
---
 gdb/testsuite/gdb.base/coredump-filter.exp | 21 +++++++++++++++++++++
 1 file changed, 21 insertions(+)

-- 
2.26.2

Comments

Pedro Alves July 10, 2020, 8:07 p.m. | #1
On 7/5/20 11:58 PM, Kevin Buettner via Gdb-patches wrote:
> See comment in patch for a description of what this is about.  In

> a nutshell, a certain memory access was expected to not work in

> order to PASS, but due to recent changes, the memory access now

> works causing the test to FAIL.

> 

> gdb/testsuite/ChangeLog:

> 

> 	* gdb.base/coredump-filter.exp (test_disasm):  Call

> 	setup_xfail for test "disassemble function with corefile and

> 	without a binary".

> ---

>  gdb/testsuite/gdb.base/coredump-filter.exp | 21 +++++++++++++++++++++

>  1 file changed, 21 insertions(+)

> 

> diff --git a/gdb/testsuite/gdb.base/coredump-filter.exp b/gdb/testsuite/gdb.base/coredump-filter.exp

> index ff398f2b85..fd4e0352d8 100644

> --- a/gdb/testsuite/gdb.base/coredump-filter.exp

> +++ b/gdb/testsuite/gdb.base/coredump-filter.exp

> @@ -96,6 +96,27 @@ proc test_disasm { core address should_fail } {

>  	}

>  

>  	if { $should_fail == 1 } {

> +	    # As originally conceived, an attempt here to disassemble

> +	    # addresses in main() with the core file loaded, but with

> +	    # the executable not loaded shouldn't work .  This was a

> +	    # good idea because it demonstrates that executable-backed

> +	    # memory was not dumped to the core file.  However, this

> +	    # test now fails (i.e. now successfully disassembles

> +	    # addresses in main()) due to the fact that GDB loads the

> +	    # file-based mappings in Linux's NT_FILE note.  So, even

> +	    # though the executable wasn't explicitly loaded, GDB will

> +	    # still be able to find the file-backed memory via the

> +	    # note (referencing the file) within the core file.  The

> +	    # data in question is not actually stored in the core

> +	    # file, but is instead found in one of the files mentioned

> +	    # in the core file note.

> +	    #

> +	    # It's tempting to simply remove this test because it's

> +	    # apparently no longer useful.  However, the idea behind

> +	    # the test is useful, so it's marked as xfail, along with

> +	    # this comment, as a reminder for someone to come up with

> +	    # a new approach.

> +	    setup_xfail "*-*-*"

>  	    gdb_test "x/i \$pc" "=> $hex:\tCannot access memory at address $hex" \

>  		"disassemble function with corefile and without a binary"

>  	} else {

> 


Could we instead make the test dependent on whether we have
the file mappings?  Like e.g., try if "info proc mappings" works
and store the result in a $have_mappings variable.  And then:

if !$have_mappings
  expect error
else
  expect some reasonable output and no error

Maybe the "should_fail" variable would be better renamed to
"have_binary" or something like that?

Also, not sure whether the test_disasm intro comment needs updating.

Thanks,
Pedro Alves
Simon Marchi via Gdb-patches July 13, 2020, 11:38 a.m. | #2
Idea: now that NT_FILE works, it probably makes sense to replace this test with two sequential ones:

1) First test that it still works if file is accessible in the filesystem
2) Temporarily move / rename the file and test that disassemble doesn't work anymore

Best regards,
Mihails

> -----Original Message-----

> From: Gdb-patches <gdb-patches-bounces@sourceware.org> On Behalf Of

> Pedro Alves

> Sent: Friday, July 10, 2020 10:08 PM

> To: Kevin Buettner <kevinb@redhat.com>; gdb-patches@sourceware.org

> Subject: Re: [PATCH v4 11/14] xfail gdb.base/coredump-filter.exp test which

> now works without a binary

> 

> On 7/5/20 11:58 PM, Kevin Buettner via Gdb-patches wrote:

> > See comment in patch for a description of what this is about.  In a

> > nutshell, a certain memory access was expected to not work in order to

> > PASS, but due to recent changes, the memory access now works causing

> > the test to FAIL.

> >

> > gdb/testsuite/ChangeLog:

> >

> > 	* gdb.base/coredump-filter.exp (test_disasm):  Call

> > 	setup_xfail for test "disassemble function with corefile and

> > 	without a binary".

> > ---

> >  gdb/testsuite/gdb.base/coredump-filter.exp | 21

> +++++++++++++++++++++

> >  1 file changed, 21 insertions(+)

> >

> > diff --git a/gdb/testsuite/gdb.base/coredump-filter.exp

> > b/gdb/testsuite/gdb.base/coredump-filter.exp

> > index ff398f2b85..fd4e0352d8 100644

> > --- a/gdb/testsuite/gdb.base/coredump-filter.exp

> > +++ b/gdb/testsuite/gdb.base/coredump-filter.exp

> > @@ -96,6 +96,27 @@ proc test_disasm { core address should_fail } {

> >  	}

> >

> >  	if { $should_fail == 1 } {

> > +	    # As originally conceived, an attempt here to disassemble

> > +	    # addresses in main() with the core file loaded, but with

> > +	    # the executable not loaded shouldn't work .  This was a

> > +	    # good idea because it demonstrates that executable-backed

> > +	    # memory was not dumped to the core file.  However, this

> > +	    # test now fails (i.e. now successfully disassembles

> > +	    # addresses in main()) due to the fact that GDB loads the

> > +	    # file-based mappings in Linux's NT_FILE note.  So, even

> > +	    # though the executable wasn't explicitly loaded, GDB will

> > +	    # still be able to find the file-backed memory via the

> > +	    # note (referencing the file) within the core file.  The

> > +	    # data in question is not actually stored in the core

> > +	    # file, but is instead found in one of the files mentioned

> > +	    # in the core file note.

> > +	    #

> > +	    # It's tempting to simply remove this test because it's

> > +	    # apparently no longer useful.  However, the idea behind

> > +	    # the test is useful, so it's marked as xfail, along with

> > +	    # this comment, as a reminder for someone to come up with

> > +	    # a new approach.

> > +	    setup_xfail "*-*-*"

> >  	    gdb_test "x/i \$pc" "=> $hex:\tCannot access memory at address

> $hex" \

> >  		"disassemble function with corefile and without a binary"

> >  	} else {

> >

> 

> Could we instead make the test dependent on whether we have the file

> mappings?  Like e.g., try if "info proc mappings" works and store the result in

> a $have_mappings variable.  And then:

> 

> if !$have_mappings

>   expect error

> else

>   expect some reasonable output and no error

> 

> Maybe the "should_fail" variable would be better renamed to "have_binary"

> or something like that?

> 

> Also, not sure whether the test_disasm intro comment needs updating.

> 

> Thanks,

> Pedro Alves

Intel Deutschland GmbH
Registered Address: Am Campeon 10-12, 85579 Neubiberg, Germany
Tel: +49 89 99 8853-0, www.intel.de
Managing Directors: Christin Eisenschmid, Gary Kershaw
Chairperson of the Supervisory Board: Nicole Lau
Registered Office: Munich
Commercial Register: Amtsgericht Muenchen HRB 186928
Simon Marchi via Gdb-patches July 13, 2020, 5:04 p.m. | #3
On Mon, 13 Jul 2020 11:38:49 +0000
"Strasuns, Mihails via Gdb-patches" <gdb-patches@sourceware.org> wrote:

> Idea: now that NT_FILE works, it probably makes sense to replace this test with two sequential ones:

> 

> 1) First test that it still works if file is accessible in the filesystem

> 2) Temporarily move / rename the file and test that disassemble doesn't work anymore


That's a good idea.  I'll give it a try...

Thanks,

Kevin

Patch

diff --git a/gdb/testsuite/gdb.base/coredump-filter.exp b/gdb/testsuite/gdb.base/coredump-filter.exp
index ff398f2b85..fd4e0352d8 100644
--- a/gdb/testsuite/gdb.base/coredump-filter.exp
+++ b/gdb/testsuite/gdb.base/coredump-filter.exp
@@ -96,6 +96,27 @@  proc test_disasm { core address should_fail } {
 	}
 
 	if { $should_fail == 1 } {
+	    # As originally conceived, an attempt here to disassemble
+	    # addresses in main() with the core file loaded, but with
+	    # the executable not loaded shouldn't work .  This was a
+	    # good idea because it demonstrates that executable-backed
+	    # memory was not dumped to the core file.  However, this
+	    # test now fails (i.e. now successfully disassembles
+	    # addresses in main()) due to the fact that GDB loads the
+	    # file-based mappings in Linux's NT_FILE note.  So, even
+	    # though the executable wasn't explicitly loaded, GDB will
+	    # still be able to find the file-backed memory via the
+	    # note (referencing the file) within the core file.  The
+	    # data in question is not actually stored in the core
+	    # file, but is instead found in one of the files mentioned
+	    # in the core file note.
+	    #
+	    # It's tempting to simply remove this test because it's
+	    # apparently no longer useful.  However, the idea behind
+	    # the test is useful, so it's marked as xfail, along with
+	    # this comment, as a reminder for someone to come up with
+	    # a new approach.
+	    setup_xfail "*-*-*"
 	    gdb_test "x/i \$pc" "=> $hex:\tCannot access memory at address $hex" \
 		"disassemble function with corefile and without a binary"
 	} else {