| Message ID | 1485870927-12623-1-git-send-email-lgustavo@codesourcery.com |
|---|---|
| State | New |
| Headers | show |
On 01/31/2017 01:55 PM, Luis Machado wrote: > This test attempts to load a x86 core file no matter what the target > architecture is. If the architecture is not x86, GDB will not recognize > the core file and therefore won't have any memory to inspect. All we will > have is a memory read error, resulting in a FAIL. > > The following patch addresses this by checking if we successfully loaded > the core file. If not, just return. Otherwise it keeps testing. > > gdb/testsuite/ChangeLog: > > 2017-01-31 Luis Machado <lgustavo@codesourcery.com> > > * gdb.arch/i386-biarch-core.exp: Return if core file was not > recognized. This seems to contradict a bit the point of the test, which says: > # Test if at least the core file segments memory has been loaded. when: > # Wrongly built GDB complains by: > # "..." is not a core dump: File format not recognized I.e., would we be potentially losing coverage? Assurance that we're not I think should be part of the rationale for this change. Does the test pass for you if your non --target=x86 build ends up including x86 support, with e.g., --enable-targets=all? If it does, then maybe we should check whether the GDB build supports x86, by e.g., doing: gdb_test_multiple "complete set architecture i386" ... and seeing if something comes out. then only do "unsupported" if x86 is not supported, otherwise, the memory read failure is really a fail. Thanks, Pedro Alves
On 02/06/2017 11:28 AM, Pedro Alves wrote: > On 01/31/2017 01:55 PM, Luis Machado wrote: >> This test attempts to load a x86 core file no matter what the target >> architecture is. If the architecture is not x86, GDB will not recognize >> the core file and therefore won't have any memory to inspect. All we will >> have is a memory read error, resulting in a FAIL. >> >> The following patch addresses this by checking if we successfully loaded >> the core file. If not, just return. Otherwise it keeps testing. >> >> gdb/testsuite/ChangeLog: >> >> 2017-01-31 Luis Machado <lgustavo@codesourcery.com> >> >> * gdb.arch/i386-biarch-core.exp: Return if core file was not >> recognized. > > This seems to contradict a bit the point of the test, which says: > >> # Test if at least the core file segments memory has been loaded. > > when: > >> # Wrongly built GDB complains by: >> # "..." is not a core dump: File format not recognized You're right. The same message would've come out in a broken gdb trying to load the test core file. > > I.e., would we be potentially losing coverage? Assurance > that we're not I think should be part of the rationale for > this change. > > Does the test pass for you if your non --target=x86 build > ends up including x86 support, with e.g., --enable-targets=all? > It should, because then gdb will recognize it. > If it does, then maybe we should check whether the GDB > build supports x86, by e.g., doing: > > gdb_test_multiple "complete set architecture i386" ... > Yes, indeed. This test should never try to force a gdb that doesn't understand i386/x86-64 core files to load one. > and seeing if something comes out. > > then only do "unsupported" if x86 is not supported, otherwise, > the memory read failure is really a fail. Right. In this case it was a fail because gdb didn't even load the core file. As opposed to failing to read from a core file memory segment. I'll adjust this.
On 02/06/2017 05:58 PM, Luis Machado wrote: > Yes, indeed. This test should never try to force a gdb that doesn't > understand i386/x86-64 core files to load one. TBC, I think it should, to make sure gdb doesn't crash. Thanks, Pedro Alves
diff --git a/gdb/testsuite/gdb.arch/i386-biarch-core.exp b/gdb/testsuite/gdb.arch/i386-biarch-core.exp index 4c5f450..2776ac3 100644 --- a/gdb/testsuite/gdb.arch/i386-biarch-core.exp +++ b/gdb/testsuite/gdb.arch/i386-biarch-core.exp @@ -62,7 +62,16 @@ if {$corestat(size) != 102400} { # objcopy as it corrupts the core file beyond all recognition. # The output therefore does not matter much, just we should not get GDB # internal error. -gdb_test "core-file ${corefile}" ".*" "core-file" +set test "load core file" +gdb_test_multiple "core-file ${corefile}" $test { + -re ".* no core file handler recognizes format(.*\r\n)?$gdb_prompt $" { + untested $test + return + } + -re "\r\n$gdb_prompt $" { + pass $test + } +} # Test if at least the core file segments memory has been loaded. # https://bugzilla.redhat.com/show_bug.cgi?id=457187