Re: [PATCH] LVU: dump loclists with locviews

[Alan Modra <amodra at gmail dot com>]

On Wed, Sep 20, 2017 at 11:17:54PM -0300, Alexandre Oliva wrote:
> for  include/ChangeLog
> 
> 	* dwarf2.def (DW_AT_GNU_locviews): New.
> 	* dwarf2.h (enum dwarf_location_list_entry_type): Add
> 	DW_LLE_GNU_view_pair.
> 	(DW_LLE_view_pair): Define.
> 
> for  binutils/ChangeLog
> 
> 	* dwarf.h (debug_info): Add loc_views and num_loc_views.
> 	* dwarf.c (vm1): New constant.
> 	(print_dwarf_view): New function.
> 	(read_and_display_attr_value): Support DW_AT_GNU_locviews.
> 	(process_debug_info): Keep num_loc_offsets and num_loc_views
> 	in sync.
> 	(display_view_pair_list): New function.
> 	(display_loc_list_dwo): Take vstart_ptr; update it.  Dump
> 	location view pairs before the range they apply to, when a
> 	viewlist augments the loc list.
> 	(display_loc_list): Likewise.  Check view numbers in range
> 	tests.
> 	(display_loclists_list): Likewise.  Handle view pair entries,
> 	and warn on trailing ones.
> 	(loc_views): New variable.
> 	(loc_offsets_compar): Compare loc_views if loc_offsets are the
> 	same.
> 	(display_debug_loc): Check and sort loc_views too.  Accept
> 	loc_view as expected_start.  Skip if lists and views are the
> 	same.  Dump locview list separately in order, and pass the
> 	locview list base to each list dump function.  Warn and skip
> 	overlap and hole checking if we find loclists and locviews to
> 	not be adjacent.
> 	* testsuite/binutils-all/locview-1.s: New.
> 	* testsuite/binutils-all/readelf.locview-1: New.
> 	* testsuite/binutils-all/locview-2.s: New.
> 	* testsuite/binutils-all/readelf.locview-2: New.
> 	* testsuite/binutils-all/readelf.exp: Run new tests.  Fix
> 	option spelling in pr18374 fail message.  XFAIL dw5 test on
> 	nds32*-elf.

OK with a few tweaks..

> +static void
> +print_dwarf_view (dwarf_vma value, unsigned num_bytes, int force)
> +{
> +  int len;
> +  if (!num_bytes)
> +    len = 4;
> +  else
> +    len = num_bytes * 2;
> +
> +  assert (value == (unsigned long)value);
> +  if (value || force)
> +    printf ("v%0*lx ", len - 1, (unsigned long)value);

Space after cast.

> --- a/binutils/testsuite/binutils-all/readelf.exp
> +++ b/binutils/testsuite/binutils-all/readelf.exp
> @@ -366,7 +366,7 @@ if {![binutils_assemble $srcdir/$subdir/version.s tmpdir/version.o]} then {
>  if {![binutils_assemble $srcdir/$subdir/pr18374.s tmpdir/pr18374.o]} then {
>      perror "could not assemble PR18374 test file"
>      unresolved "readelf - failed to assemble"
> -    fail "readelf --debug-loc"
> +    fail "readelf --debug-dump=loc"
>  } else {
>  
>      if ![is_remote host] {
> @@ -379,6 +379,37 @@ if {![binutils_assemble $srcdir/$subdir/pr18374.s tmpdir/pr18374.o]} then {
>  }
>  
>  
> +# locview - Check dumping of location lists with location views.
> +if {![binutils_assemble $srcdir/$subdir/locview-1.s tmpdir/locview-1.o]} then {
> +    perror "could not assemble locview-1 test file"
> +    unresolved "readelf - failed to assemble"
> +    fail "readelf --debug-dump=loc"

I know you're copying existing tests, but the "unresolved" line here
is really weird.  The argument to "unresolved" ought to be the test
name, but of course a test shouldn't be both unresolved and fail.
I think both the "perror" and "unresolved" lines should disappear, and
if you want to convey more information than just that the test failed,
then
    fail "readelf --debug-dump=loc locview-1 (failed to assemble)"
would be reasonable.

Also, you now have a number of "readelf --debug-dump=loc" tests.

Oh blah, I guess I may as well fix the existing tests myself.  Please
adjust your new tests to suit.  I'll commit the following after
running the testsuite over my usual set of targets, probably in about
an hour.

diff --git a/binutils/ChangeLog b/binutils/ChangeLog
index 53ac9fe..d6bfa63 100644
--- a/binutils/ChangeLog
+++ b/binutils/ChangeLog
@@ -1,3 +1,12 @@
+2017-09-22  Alan Modra  <amodra@gmail.com>
+
+	* testsuite/binutils-all/readelf.exp: Don't perror and exit on
+	bintest.s assembly failure.  Report tests unresolved instead.
+	Likewise for version note test, pr18374, decompress, and dw5
+	tests.
+	(readelf_test): Set testname to include both option and binary
+	file name.  Use for pass/fail.
+
 2017-09-05  Nick Clifton  <nickc@redhat.com>
 
 	PR 21995
diff --git a/binutils/testsuite/binutils-all/readelf.exp b/binutils/testsuite/binutils-all/readelf.exp
index d9e437d..6a7b562 100644
--- a/binutils/testsuite/binutils-all/readelf.exp
+++ b/binutils/testsuite/binutils-all/readelf.exp
@@ -84,6 +84,8 @@ proc readelf_test { options binary_file regexp_file xfails } {
     global srcdir
     global subdir
 
+    set testname "readelf $options [file rootname [file tail $binary_file]]"
+
     send_log "exec $READELF $READELFFLAGS $options $binary_file > readelf.out\n"
     set got [remote_exec host "$READELF $READELFFLAGS $options $binary_file" "" "/dev/null" "readelf.out"]
 
@@ -92,7 +94,7 @@ proc readelf_test { options binary_file regexp_file xfails } {
     }
 
     if { [lindex $got 0] != 0 || ![string match "" [lindex $got 1]] } then {
-	fail "readelf $options (reason: unexpected output)"
+	fail "$testname (reason: unexpected output)"
 	send_log $got
 	send_log "\n"
 	return
@@ -116,12 +118,12 @@ proc readelf_test { options binary_file regexp_file xfails } {
     }
 
     if { [regexp_diff readelf.out $srcdir/$subdir/$regexp_file] } then {
-	fail "readelf $options"
+	fail $testname
 	verbose "output is \n[file_contents readelf.out]" 2
 	return
     }
 
-    pass "readelf $options"
+    pass $testname
 }
 
 # Simple proc to skip certain expected warning messages.
@@ -318,26 +320,28 @@ send_user "Version [binutil_version $READELF]"
 
 # Assemble the test file.
 if {![binutils_assemble $srcdir/$subdir/bintest.s tmpdir/bintest.o]} then {
-    perror "could not assemble test file"
-    unresolved "readelf - failed to assemble"
-    return
-}
-
-if ![is_remote host] {
-    set tempfile tmpdir/bintest.o
+    unresolved "readelf -h bintest (failed to assemble)"
+    unresolved "readelf -S bintest (failed to assemble)"
+    unresolved "readelf -s bintest (failed to assemble)"
+    unresolved "readelf -r bintest (failed to assemble)"
 } else {
-    set tempfile [remote_download host tmpdir/bintest.o]
-}
 
-# First, determine the size, so specific output matchers can be used.
-readelf_find_size $tempfile
+    if ![is_remote host] {
+	set tempfile tmpdir/bintest.o
+    } else {
+	set tempfile [remote_download host tmpdir/bintest.o]
+    }
 
-# Run the tests.
-readelf_test -h $tempfile readelf.h  {}
-readelf_test -S $tempfile readelf.s  {}
-setup_xfail "mips-*-*irix*"
-readelf_test -s $tempfile readelf.ss {}
-readelf_test -r $tempfile readelf.r  {}
+    # First, determine the size, so specific output matchers can be used.
+    readelf_find_size $tempfile
+
+    # Run the tests.
+    readelf_test -h $tempfile readelf.h  {}
+    readelf_test -S $tempfile readelf.s  {}
+    setup_xfail "mips-*-*irix*"
+    readelf_test -s $tempfile readelf.ss {}
+    readelf_test -r $tempfile readelf.r  {}
+}
 
 readelf_wi_test
 readelf_compressed_wa_test
@@ -346,9 +350,7 @@ readelf_dump_test
 
 # PR 13482 - Check for off-by-one errors when dumping .note sections.
 if {![binutils_assemble $srcdir/$subdir/version.s tmpdir/version.o]} then {
-    perror "could not assemble version note test file"
-    unresolved "readelf - failed to assemble"
-    fail "readelf -n"
+    unresolved "readelf -n version (failed to assemble)"
 } else {
 
     if ![is_remote host] {
@@ -364,9 +366,7 @@ if {![binutils_assemble $srcdir/$subdir/version.s tmpdir/version.o]} then {
 # PR 18374 - Check that relocations against the .debug_loc section
 # do not prevent readelf from displaying all the location lists.
 if {![binutils_assemble $srcdir/$subdir/pr18374.s tmpdir/pr18374.o]} then {
-    perror "could not assemble PR18374 test file"
-    unresolved "readelf - failed to assemble"
-    fail "readelf --debug-loc"
+    unresolved "readelf --debug-dump=loc pr18374 (failed to assemble)"
 } else {
 
     if ![is_remote host] {
@@ -381,9 +381,7 @@ if {![binutils_assemble $srcdir/$subdir/pr18374.s tmpdir/pr18374.o]} then {
 
 # Check that decompressed dumps work.
 if {![binutils_assemble $srcdir/$subdir/z.s tmpdir/z.o]} then {
-    perror "could not assemble decompress dump test file"
-    unresolved "readelf - failed to assemble"
-    fail "readelf -z"
+    unresolved "readelf --decompress --hex-dump .debug_loc z (failed to assemble)"
 } else {
 
     if ![is_remote host] {
@@ -405,21 +403,20 @@ if ![istarget "riscv*-*-*"] then {
     }
     # Assemble the DWARF-5 test file.
     if {![binutils_assemble_flags $srcdir/$subdir/dw5.S tmpdir/dw5.o $hpux]} then {
-	perror "could not assemble dw5 test file"
-	unresolved "readelf - failed to assemble dw5"
-	return
-    }
-
-    # Download it.
-    if ![is_remote host] {
-	set tempfile tmpdir/dw5.o
+	unresolved "readelf -wiaoRlL dw5 (failed to assemble)"
     } else {
-	set tempfile [remote_download host tmpdir/dw5.o]
-    }
 
-    # First, determine the size, so specific output matchers can be used.
-    readelf_find_size $tempfile
+	# Download it.
+	if ![is_remote host] {
+	    set tempfile tmpdir/dw5.o
+	} else {
+	    set tempfile [remote_download host tmpdir/dw5.o]
+	}
+
+	# First, determine the size, so specific output matchers can be used.
+	readelf_find_size $tempfile
 
-    # Make sure that readelf can decode the contents.
-    readelf_test -wiaoRlL $tempfile dw5.W {}
+	# Make sure that readelf can decode the contents.
+	readelf_test -wiaoRlL $tempfile dw5.W {}
+    }
 }

-- 
Alan Modra
Australia Development Lab, IBM