Re: ping: [testsuite patch] for: [PATCH] [PR corefiles/17808] i386: Fix internal error when prstatus in core file is too big

[Jan Kratochvil <jan dot kratochvil at redhat dot com>]

On Wed, 15 Jul 2015 18:14:01 +0200, Yao Qi wrote:
> Jan Kratochvil <jan.kratochvil@redhat.com> writes:
> 
> > (1) The testcase did not really test if elf64-i386 is supported by GDB (BFD).
> > That was OK for a Fedora testcase but I forgot about it when submitting it
> > upstream.
> >
> > I haven't really verified if the GNU target is elf64-little but it seems so,
> > no other one seems suitable from:
> > 	elf32-x86-64
> > 	elf64-big
> > 	elf64-k1om
> > 	elf64-l1om
> > 	elf64-little
> > 	elf64-x86-64
> > 	pei-x86-64
> 
> Hi Jan,
> Why can't we use istarget here?

I do not know much dejagnu but I expect 'istarget' tests against the site.exp
'target_triplet' content which is set to the primary GDB target
(--target=...).

GDB is normally never configured for primary target elf64-i386, I think BFD
does not know such explicit target, it gets recognized as elf64-little.

In fact many testfiles of the GDB testsuite are wrong as they require
'istarget' (therefore primary GDB target) even for just loading arch specific
files which would be sufficient with secondary target (--enable-targets=...)
support.


> I thought we still check istarget "x86_64-*-*", no?

This my new patch removes this 'istarget' check as it is IMO unrelated to what
we need to test.  Although you are right we do 'x/i' and test for 'hlt' so
I think we should test also for available 'set architecture i386'.
We could also test by 'x/bx' instead of 'x/i' to avoid such additional
test/requirement.


> > (2) The output of the "core-file" command itself can be arbitrary as the
> > elf64-i386 file with x86_64 registers is really broken; but that does not
> > matter much, important is the following test whether core file memory is
> > readable.
> 
> "that does not matter much" mean if internal error isn't triggered, any
> output is acceptable, right?

Yes.

> and the purpose of following test "x/i $address"
> is to verify this (internal error not triggered)?

I did not think specifically about internal error but I agree.  After all the
core file should be loaded which is tested by readability of a core file's
segment.


> >  # Wrongly built GDB complains by:
> >  # "..." is not a core dump: File format not recognized
> >  # As the provided test core has 64bit PRSTATUS i386 built GDB cannot parse it.
> >  # This is just a problem of the test case, real-world elf64-i386 file will have
> >  # 32bit PRSTATUS.  One cannot prepare elf64-i386 core file from elf32-i386 by
> >  # objcopy as it corrupts the core file beyond all recognition.
> 
> As you said, the output of command "core-file" doesn't matter much, we
> need to update the comments here.

I think the comments above are useful to understand why it does not behave as
sanely as one would expect (=the real world case for loading kdump i386 kernel
core files).

So to add another part of the comment?
	# The output therefore does not matter much, just we should not get
	# GDB internal error.

Although this whole feature is becoming marginal as i386 kernels in enterprise
usage (=kdump) have AFAIK mostly disappeared.


> > -gdb_test "core-file ${corefile}" "\r\nwarning: Unexpected size of section `\\.reg/6901' in core file\\.\r\n.*Core was generated by `\[^\r\n\]*'\\.\r\nProgram terminated with signal SIGSEGV, Segmentation fault\\.\r\n.*" "core-file"
> > +gdb_test "core-file ${corefile}" ".*" "core-file"
> 
> >  
> >  gdb_test "x/i $address" "\r\n\[ \t\]*$address:\[ \t\]*hlt\[ \t\]*" ".text is readable"
> 
> We also need comment here to explain the purpose this "x/i $address" test.

Such a comment?
	# Test readability of a core file segment memory.


Jan