public inbox for binutils@sourceware.org
 help / color / mirror / Atom feed
* [PATCH 1/3] ld: Add additional checking for warnings/errors in testsuite
  2017-02-13 11:30 [PATCH 0/3] Improve information in linker errors Andrew Burgess
@ 2017-02-13 11:30 ` Andrew Burgess
  2017-02-14 12:32   ` Nick Clifton
  2017-02-13 11:30 ` [PATCH 2/3] bfd/dwarf: Improve use of previously loaded dwarf information Andrew Burgess
  2017-02-13 11:30 ` [PATCH 3/3] bfd: Improve lookup of file / line information for errors Andrew Burgess
  2 siblings, 1 reply; 14+ messages in thread
From: Andrew Burgess @ 2017-02-13 11:30 UTC (permalink / raw)
  To: binutils; +Cc: Andrew Burgess

This commit adds new actions to the run_cc_link_tests mechanism in the
linker testsuite.

Previously this procedure could take a parameter containing a regular
expression that would be matched against warnings from the linker.

After this commit the warnings parameter is removed, instead, the
actions list can contain the actions 'warning', 'error',
'warning_output', or 'error_output'.  The action names are chosen to
match the actions already present in the run_dump_test procedure.

These new actions allow for the current warning checking, but also allow
for checking of errors using a regular expression.  More interestingly,
the *_output actions allow for the warning/error patterns to be placed
in a separate file.

The small number of tests that make use of the warning parameter have
been updated to the new mechanism.  Later commits will make use of the
new features added in this commit.

ld/ChangeLog:

	* testsuite/lib/ld-lib.exp (run_cc_link_tests): Add warning,
	error, warning_output, and error_output actions.  Remove separate
	warnings parameter.
	* testsuite/ld-elf/shared.exp (build_tests): Updated to use
	'warning' action.
	* testsuite/ld-plugin/lto.exp (lto_link_tests): Likewise.
---
 ld/ChangeLog                   |  9 +++++
 ld/testsuite/ld-elf/shared.exp |  6 ++--
 ld/testsuite/ld-plugin/lto.exp |  6 ++--
 ld/testsuite/lib/ld-lib.exp    | 79 +++++++++++++++++++++++++++++++++++-------
 4 files changed, 84 insertions(+), 16 deletions(-)

diff --git a/ld/testsuite/ld-elf/shared.exp b/ld/testsuite/ld-elf/shared.exp
index 2946262..9d5a9d9 100644
--- a/ld/testsuite/ld-elf/shared.exp
+++ b/ld/testsuite/ld-elf/shared.exp
@@ -103,8 +103,10 @@ set build_tests {
    {begin.c end.c} {} "libbar.so"}
   {"Build warn libbar.so"
    "-shared" "-fPIC"
-   {beginwarn.c end.c} {{readelf {-S --wide} libbarw.rd}} "libbarw.so"
-   "c" {^.*\): warning: function foo is deprecated$} }
+  {beginwarn.c end.c}
+  {{readelf {-S --wide} libbarw.rd}
+   {warning "^.*\\): warning: function foo is deprecated$"}}
+  "libbarw.so" "c"}
   {"Build hidden libbar.so"
    "-shared" "-fPIC"
    {begin.c endhidden.c} {} "libbarh.so"}
diff --git a/ld/testsuite/ld-plugin/lto.exp b/ld/testsuite/ld-plugin/lto.exp
index c738895..80c084b 100644
--- a/ld/testsuite/ld-plugin/lto.exp
+++ b/ld/testsuite/ld-plugin/lto.exp
@@ -154,7 +154,8 @@ set lto_link_tests [list \
    {pr12760b.c} {} "libpr12760.a"] \
   [list "PR ld/12760" \
    "-O2 -Wl,-e,foo -nostdlib -flto -fuse-linker-plugin tmpdir/pr12760a.o -Wl,--start-group tmpdir/libpr12760.a -Wl,--end-group" "" \
-   {dummy.c} {} "pr12760.exe" "c" "pr12760a.c:6: warning: Bad \\.?bar"] \
+   {dummy.c} {{warning "pr12760a.c:6: warning: Bad \\.?bar"}} \
+   "pr12760.exe" "c"] \
   [list "Build libpr13183.a" \
    "-T" "-flto -O2 $lto_fat" \
    {pr13183a.c} {} "libpr13183.a"] \
@@ -205,7 +206,8 @@ set lto_link_tests [list \
    {pr20267b.c} {} "libpr20267b.a"] \
   [list "Build pr20321" \
    "-flto -Wl,-plugin,$plug_so" "-flto" \
-   {pr20321.c} {} "pr20321" "c" ".*: duplicated plugin"] \
+   {pr20321.c} {{warning ".*: duplicated plugin"}} \
+   "pr20321" "c"] \
 ]
 
 if { [at_least_gcc_version 4 7] } {
diff --git a/ld/testsuite/lib/ld-lib.exp b/ld/testsuite/lib/ld-lib.exp
index fece871..42cfe1c 100644
--- a/ld/testsuite/lib/ld-lib.exp
+++ b/ld/testsuite/lib/ld-lib.exp
@@ -1508,12 +1508,15 @@ proc run_ld_link_exec_tests { ldtests args } {
 #  4:action and options.
 #  5:name of output file
 #  6:language (optional)
-#  7:linker warnings (optional)
 #
 # Actions:
 # objdump: Apply objdump options on result.  Compare with regex (last arg).
 # nm: Apply nm options on result.  Compare with regex (last arg).
 # readelf: Apply readelf options on result.  Compare with regex (last arg).
+# warning: Check linker output against regex (last arg).
+# error: Like 'warning' but checking output in error case.
+# warning_output: Check linker output against regex in a file (last arg).
+# error_output: Like 'warning_output' but checking output in error case.
 #
 proc run_cc_link_tests { ldtests } {
     global nm
@@ -1544,10 +1547,11 @@ proc run_cc_link_tests { ldtests } {
 	set actions [lindex $testitem 4]
 	set binfile tmpdir/[lindex $testitem 5]
 	set lang [lindex $testitem 6]
-	set warnings [lindex $testitem 7]
 	set objfiles {}
 	set is_unresolved 0
 	set failed 0
+	set check_ld(terminal) 0
+	set check_ld(source) ""
 
 	#verbose -log "testname  is $testname"
 	#verbose -log "ldflags   is $ldflags"
@@ -1556,7 +1560,37 @@ proc run_cc_link_tests { ldtests } {
 	#verbose -log "actions   is $actions"
 	#verbose -log "binfile   is $binfile"
 	#verbose -log "lang      is $lang"
-	#verbose -log "warnings  is $warnings"
+
+	foreach actionlist $actions {
+	    set action [lindex $actionlist 0]
+	    set progopts [lindex $actionlist 1]
+
+	    # Find actions related to error/warning processing.
+	    switch -- $action {
+	        error
+	        {
+	            set check_ld(source) "regexp"
+	            set check_ld(regexp) $progopts
+	            set check_ld(terminal) 1
+	        }
+	        warning
+	        {
+	            set check_ld(source) "regexp"
+	            set check_ld(regexp) $progopts
+	        }
+	        error_output
+	        {
+	            set check_ld(source) "file"
+	            set check_ld(file) $progopts
+	            set check_ld(terminal) 1
+	        }
+	        warning_output
+	        {
+	            set check_ld(source) "file"
+	            set check_ld(file) $progopts
+	        }
+	    }
+	}
 
 	# Compile each file in the test.
 	foreach src_file $src_files {
@@ -1598,18 +1632,35 @@ proc run_cc_link_tests { ldtests } {
 		set failed 1
 	    }
 	} else {
-	    if { ![ld_link $cc_cmd $binfile "$board_cflags -L$srcdir/$subdir $ldflags $objfiles"] } {
-		set failed 1
-	    }
+	    ld_link $cc_cmd $binfile "$board_cflags -L$srcdir/$subdir $ldflags $objfiles"
+	    set ld_output "$exec_output"
 
-	    # Check if exec_output is expected.
-	    if { $warnings != "" } then {
-		verbose -log "returned with: <$exec_output>, expected: <$warnings>"
-		if { [regexp $warnings $exec_output] } then {
-		    set failed 0
-		} else {
+	    if { $check_ld(source) == "regexp" } then {
+		# Match output against regexp argument.
+		verbose -log "returned with: <$ld_output>, expected: <$check_ld(regexp)>"
+		if { ![regexp $check_ld(regexp) $ld_output] } then {
 		    set failed 1
 		}
+	    } elseif { $check_ld(source) == "file" } then {
+		# Match output against patterns in a file.
+		set_file_contents "tmpdir/ld.messages" "$ld_output"
+		verbose "ld.messages has '[file_contents tmpdir/ld.messages]'"
+		if { [regexp_diff "tmpdir/ld.messages" "$srcdir/$subdir/$check_ld(file)"] } then {
+		    verbose "output is $ld_output" 2
+		    set failed 1
+		}
+	    }
+
+	    if { $check_ld(source) != "" } then {
+                if { $ld_output == "" } then {
+                    verbose -log "Linker was expected to give error or warning"
+                    set failed 1
+                }
+	    } else {
+                if { $ld_output != "" } then {
+                    verbose -log "Unexpected linker warning or error"
+                    set failed 1
+                }
 	    }
 	}
 
@@ -1629,6 +1680,10 @@ proc run_cc_link_tests { ldtests } {
 		        { set dump_prog $nm }
 		    readelf
 		        { set dump_prog $READELF }
+		    error {}
+		    warning {}
+		    error_output {}
+		    warning_output {}
 		    default
 			{
 			    perror "Unrecognized action $action"
-- 
2.5.1

^ permalink raw reply	[flat|nested] 14+ messages in thread

* [PATCH 2/3] bfd/dwarf: Improve use of previously loaded dwarf information
  2017-02-13 11:30 [PATCH 0/3] Improve information in linker errors Andrew Burgess
  2017-02-13 11:30 ` [PATCH 1/3] ld: Add additional checking for warnings/errors in testsuite Andrew Burgess
@ 2017-02-13 11:30 ` Andrew Burgess
  2017-02-14  9:33   ` Nick Clifton
  2017-02-17  7:10   ` Alan Modra
  2017-02-13 11:30 ` [PATCH 3/3] bfd: Improve lookup of file / line information for errors Andrew Burgess
  2 siblings, 2 replies; 14+ messages in thread
From: Andrew Burgess @ 2017-02-13 11:30 UTC (permalink / raw)
  To: binutils; +Cc: Andrew Burgess

When parsing DWARF data in order to report file/line type error messages
we perform section placement to make section addresses unique within
relocatable object files.

Currently, if we reuse previously loaded (and cached) dwarf data then we
neglect to perform section placement, the result is that the section
addresses will not be unique, and we might, incorrectly associate an
address with the wrong debug information, and so report an incorrect
file and line number.

Further we neglect to check that that bfd for which we are looking up
debug information is actually the bfd for which the previous debug
information was loaded, it is possible that we will reuse previously
loaded debug information for a different bfd.

And finally, due to following of gnu_debuglink links in one bfd to
another, the process of checking that the cached debug information is
valid requires us to track the original bfd in the cached debug
information.  The original debug information here is either the bfd that
we're interested in, not the bfd we finally load the debug information
from.

bfd/ChangeLog:

	* dwarf2.c (struct dwarf2_debug): Add orig_bfd member.
	(_bfd_dwarf2_slurp_debug_info): If stashed debug information does
	not match current bfd, then reload debug information.  Record bfd
	we're loading debug info for in the stash.  If we have debug
	informatin in the cache then perform section placement before
	returning.

ld/ChangeLog:

	* testsuite/ld-elf/dwarf.exp (build_tests): Add new tests.
	* testsuite/ld-elf/dwarf2.err: New file.
	* testsuite/ld-elf/dwarf2a.c: New file.
	* testsuite/ld-elf/dwarf2b.c: New file.
	* testsuite/ld-elf/dwarf3.c: New file.
	* testsuite/ld-elf/dwarf3.err: New file.
---
 bfd/ChangeLog                  |  9 +++++++++
 bfd/dwarf2.c                   | 23 +++++++++++++++++++++--
 ld/ChangeLog                   |  9 +++++++++
 ld/testsuite/ld-elf/dwarf.exp  |  6 ++++++
 ld/testsuite/ld-elf/dwarf2.err |  4 ++++
 ld/testsuite/ld-elf/dwarf2a.c  |  8 ++++++++
 ld/testsuite/ld-elf/dwarf2b.c  | 10 ++++++++++
 ld/testsuite/ld-elf/dwarf3.c   | 13 +++++++++++++
 ld/testsuite/ld-elf/dwarf3.err |  4 ++++
 9 files changed, 84 insertions(+), 2 deletions(-)
 create mode 100644 ld/testsuite/ld-elf/dwarf2.err
 create mode 100644 ld/testsuite/ld-elf/dwarf2a.c
 create mode 100644 ld/testsuite/ld-elf/dwarf2b.c
 create mode 100644 ld/testsuite/ld-elf/dwarf3.c
 create mode 100644 ld/testsuite/ld-elf/dwarf3.err

diff --git a/bfd/dwarf2.c b/bfd/dwarf2.c
index 3699587..591c2b1 100644
--- a/bfd/dwarf2.c
+++ b/bfd/dwarf2.c
@@ -100,6 +100,12 @@ struct dwarf2_debug
   /* Pointer to the end of the .debug_info section memory buffer.  */
   bfd_byte *info_ptr_end;
 
+  /* Pointer to the original bfd for which debug was loaded.  This is what
+     we use to compare and so check that the cached debug data is still
+     valid - it saves having to possibly dereference the gnu_debuglink each
+     time.  */
+  bfd *orig_bfd;
+
   /* Pointer to the bfd, section and address of the beginning of the
      section.  The bfd might be different than expected because of
      gnu_debuglink sections.  */
@@ -3897,8 +3903,20 @@ _bfd_dwarf2_slurp_debug_info (bfd *abfd, bfd *debug_bfd,
 
   if (stash != NULL)
     {
-      if (section_vma_same (abfd, stash))
-	return TRUE;
+      if (stash->orig_bfd == abfd
+          && section_vma_same (abfd, stash))
+        {
+          /* Check that we did previously find some debug information
+             before attempting to make use of it.  */
+          if (stash->bfd_ptr != NULL)
+            {
+              if (do_place && !place_sections (abfd, stash))
+                return FALSE;
+              return TRUE;
+            }
+
+          return FALSE;
+        }
       _bfd_dwarf2_cleanup_debug_info (abfd, pinfo);
       memset (stash, 0, amt);
     }
@@ -3908,6 +3926,7 @@ _bfd_dwarf2_slurp_debug_info (bfd *abfd, bfd *debug_bfd,
       if (! stash)
 	return FALSE;
     }
+  stash->orig_bfd = abfd;
   stash->debug_sections = debug_sections;
   stash->syms = symbols;
   if (!save_section_vma (abfd, stash))
diff --git a/ld/testsuite/ld-elf/dwarf.exp b/ld/testsuite/ld-elf/dwarf.exp
index 529ebcc..25b0c9d 100644
--- a/ld/testsuite/ld-elf/dwarf.exp
+++ b/ld/testsuite/ld-elf/dwarf.exp
@@ -49,6 +49,12 @@ set build_tests {
   {"Build libdwarf1.so"
    "-s -shared" "-fPIC -g -feliminate-dwarf2-dups"
    {dwarf1.c} {} "libdwarf1.so"}
+  {"DWARF parse during linker error"
+   "" "-fno-toplevel-reorder"
+   {dwarf2a.c dwarf2b.c} {{error_output "dwarf2.err"}} "dwarf2.x"}
+  {"Handle no DWARF information"
+   "" "-g0"
+   {dwarf3.c} {{error_output "dwarf3.err"}} "dwarf3.x"}
 }
 
 set run_tests {
diff --git a/ld/testsuite/ld-elf/dwarf2.err b/ld/testsuite/ld-elf/dwarf2.err
new file mode 100644
index 0000000..872d282
--- /dev/null
+++ b/ld/testsuite/ld-elf/dwarf2.err
@@ -0,0 +1,4 @@
+tmpdir/dwarf2b\.o:\(\.data\+0x0\): multiple definition of `global_var'
+tmpdir/dwarf2a\.o:\(\.data\+0x0\): first defined here
+tmpdir/dwarf2b\.o:\(\.data\+0x4\): multiple definition of `other_var'
+tmpdir/dwarf2a\.o:\(\.data\+0x4\): first defined here
diff --git a/ld/testsuite/ld-elf/dwarf2a.c b/ld/testsuite/ld-elf/dwarf2a.c
new file mode 100644
index 0000000..1de5794
--- /dev/null
+++ b/ld/testsuite/ld-elf/dwarf2a.c
@@ -0,0 +1,8 @@
+int global_var = 3;
+int other_var = 4;
+
+int
+function (void)
+{
+  return 0;
+}
diff --git a/ld/testsuite/ld-elf/dwarf2b.c b/ld/testsuite/ld-elf/dwarf2b.c
new file mode 100644
index 0000000..c557978
--- /dev/null
+++ b/ld/testsuite/ld-elf/dwarf2b.c
@@ -0,0 +1,10 @@
+int global_var = 4;
+int other_var = 2;
+
+extern int function (void);
+
+int
+main ()
+{
+  return function ();
+}
diff --git a/ld/testsuite/ld-elf/dwarf3.c b/ld/testsuite/ld-elf/dwarf3.c
new file mode 100644
index 0000000..054eb9b
--- /dev/null
+++ b/ld/testsuite/ld-elf/dwarf3.c
@@ -0,0 +1,13 @@
+/* This test is actually used to test for a segfault that came from the bfd
+   dwarf parsing code in the case when there is _no_ dwarf info.  */
+
+extern void bar (int a);
+
+int
+main ()
+{
+  bar (1);
+  bar (2);
+
+  return 0;
+}
diff --git a/ld/testsuite/ld-elf/dwarf3.err b/ld/testsuite/ld-elf/dwarf3.err
new file mode 100644
index 0000000..6027431
--- /dev/null
+++ b/ld/testsuite/ld-elf/dwarf3.err
@@ -0,0 +1,4 @@
+.*/dwarf3\.o: In function `main':
+.*dwarf3.c:\(\.text.*\+0x[0-9a-f]+\): undefined reference to `bar'
+.*dwarf3.c:\(\.text.*\+0x[0-9a-f]+\): undefined reference to `bar'
+#...
\ No newline at end of file
-- 
2.5.1

^ permalink raw reply	[flat|nested] 14+ messages in thread

* [PATCH 0/3] Improve information in linker errors
@ 2017-02-13 11:30 Andrew Burgess
  2017-02-13 11:30 ` [PATCH 1/3] ld: Add additional checking for warnings/errors in testsuite Andrew Burgess
                   ` (2 more replies)
  0 siblings, 3 replies; 14+ messages in thread
From: Andrew Burgess @ 2017-02-13 11:30 UTC (permalink / raw)
  To: binutils; +Cc: Andrew Burgess

This series contains two significant patches that try to improve the
error messages, specifically, the file and line information, given in
linker errors.

The first patch is setup work, extending the checks we can do as part
of the run_cc_link_tests mechanism in the linker testsuite.  I use
this to add come C based linker tests in the later patches.

The second patch fixes some issues with how we reuse stashed DWARF
data in _bfd_dwarf2_slurp_debug_info.  We have problems with not fully
validating a bfd match before reusing cached data, further, we neglect
to call place_sections, which can cause issues if we do reuse cached
DWARF data.

The final patch improves on the case where we are looking up file and
line information for data symbols.  In this case, we currently, alway
fallback to using section + OFFSET style messages, with this patch we
now try to give meaningful file:line type messages instead.

There's tests included for the improved functionality, and I've
updated the expected results in cases where things have changed.  I've
tested against a wide range of targets and don't believe I've
introduced any regressions.

The only testing issue I did see was on an x86-64 target where I added
"-m32" to the CFLAGS at configure time, in this case the new C test
does not compile, however, NON of the run_cc_link_tests compile in
this case, so I think it's reasonable for me not to worry about this
at this time.

All feedback welcome.

Thanks,
Andrew


Andrew Burgess (3):
  ld: Add additional checking for warnings/errors in testsuite
  bfd/dwarf: Improve use of previously loaded dwarf information
  bfd: Improve lookup of file / line information for errors

 bfd/ChangeLog                  | 14 ++++++++
 bfd/dwarf2.c                   | 55 +++++++++++++++++++++++++++--
 ld/ChangeLog                   | 23 ++++++++++++
 ld/testsuite/ld-elf/dwarf.exp  |  6 ++++
 ld/testsuite/ld-elf/dwarf2.err |  5 +++
 ld/testsuite/ld-elf/dwarf2a.c  |  8 +++++
 ld/testsuite/ld-elf/dwarf2b.c  | 10 ++++++
 ld/testsuite/ld-elf/dwarf3.c   | 13 +++++++
 ld/testsuite/ld-elf/dwarf3.err |  4 +++
 ld/testsuite/ld-elf/shared.exp |  8 +++--
 ld/testsuite/ld-plugin/lto.exp |  6 ++--
 ld/testsuite/lib/ld-lib.exp    | 79 +++++++++++++++++++++++++++++++++++-------
 12 files changed, 212 insertions(+), 19 deletions(-)
 create mode 100644 ld/testsuite/ld-elf/dwarf2.err
 create mode 100644 ld/testsuite/ld-elf/dwarf2a.c
 create mode 100644 ld/testsuite/ld-elf/dwarf2b.c
 create mode 100644 ld/testsuite/ld-elf/dwarf3.c
 create mode 100644 ld/testsuite/ld-elf/dwarf3.err

-- 
2.5.1

^ permalink raw reply	[flat|nested] 14+ messages in thread

* [PATCH 3/3] bfd: Improve lookup of file / line information for errors
  2017-02-13 11:30 [PATCH 0/3] Improve information in linker errors Andrew Burgess
  2017-02-13 11:30 ` [PATCH 1/3] ld: Add additional checking for warnings/errors in testsuite Andrew Burgess
  2017-02-13 11:30 ` [PATCH 2/3] bfd/dwarf: Improve use of previously loaded dwarf information Andrew Burgess
@ 2017-02-13 11:30 ` Andrew Burgess
  2017-02-14  9:10   ` Nick Clifton
  2 siblings, 1 reply; 14+ messages in thread
From: Andrew Burgess @ 2017-02-13 11:30 UTC (permalink / raw)
  To: binutils; +Cc: Andrew Burgess

When looking up file and line information (used from the linker to
report error messages) if no symbol is passed in, then use the symbol
list to look for a matching symbol.

If a matching symbol is found then use this to look up the file / line
information.

This should improve errors when looking up file / line information for
data sections.  Hopefully we should find a matching data symbol, which
should, in turn (we hope) match a DW_TAG_variable in the DWARF, this
should allow us to give accurate file / line errors for data symbols.

As the hope is to find a matching DW_TAG_variable in the DWARF then we
ignore section symbols, and prefer global symbols to locals.

bfd/ChangeLog:

	* dwarf2.c (_bfd_dwarf2_find_nearest_line): Perform symbol lookup
	before trying to fine matching file and line information.

ld/ChangeLog:

	* testsuite/ld-elf/shared.exp: Update expected results.
	* testsuite/ld-elf/dwarf2.err: Likewise
---
 bfd/ChangeLog                  |  5 +++++
 bfd/dwarf2.c                   | 32 ++++++++++++++++++++++++++++++++
 ld/ChangeLog                   |  5 +++++
 ld/testsuite/ld-elf/dwarf2.err |  9 +++++----
 ld/testsuite/ld-elf/shared.exp |  4 ++--
 5 files changed, 49 insertions(+), 6 deletions(-)

diff --git a/bfd/dwarf2.c b/bfd/dwarf2.c
index 591c2b1..6e2f5ad 100644
--- a/bfd/dwarf2.c
+++ b/bfd/dwarf2.c
@@ -4155,6 +4155,38 @@ _bfd_dwarf2_find_nearest_line (bfd *abfd,
     {
       BFD_ASSERT (section != NULL && functionname_ptr != NULL);
       addr = offset;
+
+      /* If we have no SYMBOL but the section we're looking at is not a
+         code section, then take a look through the list of symbols to see
+         if we have a symbol at the address we're looking for.  If we do
+         then use this to look up line information.  This will allow us to
+         give file and line results for data symbols.  We exclude code
+         symbols here, if we look up a function symbol and then look up the
+         line information we'll actually return the line number for the
+         opening '{' rather than the function definition line.  This is
+         because looking up by symbol uses the line table, in which the
+         first line for a function is usually the opening '{', while
+         looking up the function by section + offset uses the
+         DW_AT_decl_line from the function DW_TAG_subprogram for the line,
+         which will be the line of the function name.  */
+      if ((section->flags & SEC_CODE) == 0)
+	{
+	  asymbol **tmp;
+
+	  for (tmp = symbols; (*tmp) != NULL; ++tmp)
+	    if ((*tmp)->the_bfd == abfd
+		&& (*tmp)->section == section
+		&& (*tmp)->value == offset
+		&& ((*tmp)->flags & BSF_SECTION_SYM) == 0)
+	      {
+		symbol = *tmp;
+		do_line = TRUE;
+                /* For local symbols, keep going in the hope we find a
+                   global.  */
+                if ((symbol->flags & BSF_GLOBAL) != 0)
+                  break;
+	      }
+	}
     }
 
   if (section->output_section)
diff --git a/ld/testsuite/ld-elf/dwarf2.err b/ld/testsuite/ld-elf/dwarf2.err
index 872d282..b4ea67f 100644
--- a/ld/testsuite/ld-elf/dwarf2.err
+++ b/ld/testsuite/ld-elf/dwarf2.err
@@ -1,4 +1,5 @@
-tmpdir/dwarf2b\.o:\(\.data\+0x0\): multiple definition of `global_var'
-tmpdir/dwarf2a\.o:\(\.data\+0x0\): first defined here
-tmpdir/dwarf2b\.o:\(\.data\+0x4\): multiple definition of `other_var'
-tmpdir/dwarf2a\.o:\(\.data\+0x4\): first defined here
+tmpdir/dwarf2b\.o:.*dwarf2b\.c:1: multiple definition of `global_var'
+tmpdir/dwarf2a\.o:.*dwarf2a\.c:1: first defined here
+tmpdir/dwarf2b\.o:.*dwarf2b\.c:2: multiple definition of `other_var'
+tmpdir/dwarf2a\.o:.*dwarf2a\.c:2: first defined here
+#...
\ No newline at end of file
diff --git a/ld/testsuite/ld-elf/shared.exp b/ld/testsuite/ld-elf/shared.exp
index 9d5a9d9..3b8ab18 100644
--- a/ld/testsuite/ld-elf/shared.exp
+++ b/ld/testsuite/ld-elf/shared.exp
@@ -105,7 +105,7 @@ set build_tests {
    "-shared" "-fPIC"
   {beginwarn.c end.c}
   {{readelf {-S --wide} libbarw.rd}
-   {warning "^.*\\): warning: function foo is deprecated$"}}
+   {warning "^.*beginwarn.c:7: warning: function foo is deprecated$"}}
   "libbarw.so" "c"}
   {"Build hidden libbar.so"
    "-shared" "-fPIC"
@@ -350,7 +350,7 @@ set run_tests [list \
     [list "Run warn with versioned libfoo.so" \
      "-Wl,--no-as-needed tmpdir/beginwarn.o tmpdir/libfoov.so" "" \
      {main.c} "warn" "warn.out" \
-     "" "c" {^.*\): warning: function foo is deprecated$} ] \
+     "" "c" {^.*beginwarn.c:7: warning: function foo is deprecated$} ] \
     [list "Run protected with versioned libfoo.so" \
      "-Wl,--no-as-needed tmpdir/begin.o tmpdir/libfoov.so tmpdir/endprotected.o" "" \
      {main.c} "protected" "normal.out" ] \
-- 
2.5.1

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH 3/3] bfd: Improve lookup of file / line information for errors
  2017-02-13 11:30 ` [PATCH 3/3] bfd: Improve lookup of file / line information for errors Andrew Burgess
@ 2017-02-14  9:10   ` Nick Clifton
  2017-02-15 23:42     ` Andrew Burgess
  0 siblings, 1 reply; 14+ messages in thread
From: Nick Clifton @ 2017-02-14  9:10 UTC (permalink / raw)
  To: Andrew Burgess, binutils

Hi Andrew,

> @@ -4155,6 +4155,38 @@ _bfd_dwarf2_find_nearest_line (bfd *abfd,

The find_nearest_line functions are used not only when emitting error/warning
messages, but also when displaying disassembly mixed with source code.  Hence
it is helpful if these functions can be relatively efficient.  (I have fixed
several PRs over the last year about "objdump -dS" being very slow...)  With
that in mind then:


> +	  for (tmp = symbols; (*tmp) != NULL; ++tmp)
> +	    if ((*tmp)->the_bfd == abfd
> +		&& (*tmp)->section == section
> +		&& (*tmp)->value == offset
> +		&& ((*tmp)->flags & BSF_SECTION_SYM) == 0)

Wouldn't a binary search be more efficient here ?  Also it might be worthwhile
caching the result in case the next call is for the same offset, or a very similar
one.

Cheers
  Nick


^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH 2/3] bfd/dwarf: Improve use of previously loaded dwarf information
  2017-02-13 11:30 ` [PATCH 2/3] bfd/dwarf: Improve use of previously loaded dwarf information Andrew Burgess
@ 2017-02-14  9:33   ` Nick Clifton
  2017-02-17  7:10   ` Alan Modra
  1 sibling, 0 replies; 14+ messages in thread
From: Nick Clifton @ 2017-02-14  9:33 UTC (permalink / raw)
  To: Andrew Burgess, binutils

Hi Andrew,

> bfd/ChangeLog:
> 
> 	* dwarf2.c (struct dwarf2_debug): Add orig_bfd member.
> 	(_bfd_dwarf2_slurp_debug_info): If stashed debug information does
> 	not match current bfd, then reload debug information.  Record bfd
> 	we're loading debug info for in the stash.  If we have debug
> 	informatin in the cache then perform section placement before
> 	returning.
> 
> ld/ChangeLog:
> 
> 	* testsuite/ld-elf/dwarf.exp (build_tests): Add new tests.
> 	* testsuite/ld-elf/dwarf2.err: New file.
> 	* testsuite/ld-elf/dwarf2a.c: New file.
> 	* testsuite/ld-elf/dwarf2b.c: New file.
> 	* testsuite/ld-elf/dwarf3.c: New file.
> 	* testsuite/ld-elf/dwarf3.err: New file.

Approved - please apply.

(Yes, I am reviewing these patches in reverse order.  No, there isn't a
good reason for this. :-)

Cheers
  Nick

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH 1/3] ld: Add additional checking for warnings/errors in testsuite
  2017-02-13 11:30 ` [PATCH 1/3] ld: Add additional checking for warnings/errors in testsuite Andrew Burgess
@ 2017-02-14 12:32   ` Nick Clifton
  2017-02-15 21:15     ` Andrew Burgess
  0 siblings, 1 reply; 14+ messages in thread
From: Nick Clifton @ 2017-02-14 12:32 UTC (permalink / raw)
  To: Andrew Burgess, binutils

Hi Andrew,

> The small number of tests that make use of the warning parameter have
> been updated to the new mechanism.  Later commits will make use of the
> new features added in this commit.

How did you test this change ?  IE: Have you run regression tests and if
so for which target(s).  With a change like this, with possible global
repercussions, it is important to let us know that you did test it.  (I
ran regressions tests locally, and I know that the patch is OK, but I
just wanted to make the point).

> ld/ChangeLog:
> 
> 	* testsuite/lib/ld-lib.exp (run_cc_link_tests): Add warning,
> 	error, warning_output, and error_output actions.  Remove separate
> 	warnings parameter.
> 	* testsuite/ld-elf/shared.exp (build_tests): Updated to use
> 	'warning' action.
> 	* testsuite/ld-plugin/lto.exp (lto_link_tests): Likewise.
 
Approved - please apply.

Cheers
  Nick


^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH 1/3] ld: Add additional checking for warnings/errors in testsuite
  2017-02-14 12:32   ` Nick Clifton
@ 2017-02-15 21:15     ` Andrew Burgess
  2017-02-16  9:42       ` Nick Clifton
  0 siblings, 1 reply; 14+ messages in thread
From: Andrew Burgess @ 2017-02-15 21:15 UTC (permalink / raw)
  To: Nick Clifton; +Cc: binutils

* Nick Clifton <nickc@redhat.com> [2017-02-14 12:32:13 +0000]:

> Hi Andrew,
> 
> > The small number of tests that make use of the warning parameter have
> > been updated to the new mechanism.  Later commits will make use of the
> > new features added in this commit.
> 
> How did you test this change ?  IE: Have you run regression tests and if
> so for which target(s).  With a change like this, with possible global
> repercussions, it is important to let us know that you did test it.  (I
> ran regressions tests locally, and I know that the patch is OK, but I
> just wanted to make the point).

Nick,

That's fine.  What's a good way to report the set of tests I've run
against?  I am currently using this Makefile for testing:

   https://raw.githubusercontent.com/T-J-Teru/binutils-gdb-testing/master/Makefile

which runs against 245 targets I've found from various sources.  I
expect there's probably some duplication in there, one day I'll try to
reduce the list...

Should I include the list of targets with the patch, or just include
a link to the test script?

Thanks,
Andrew

> 
> > ld/ChangeLog:
> > 
> > 	* testsuite/lib/ld-lib.exp (run_cc_link_tests): Add warning,
> > 	error, warning_output, and error_output actions.  Remove separate
> > 	warnings parameter.
> > 	* testsuite/ld-elf/shared.exp (build_tests): Updated to use
> > 	'warning' action.
> > 	* testsuite/ld-plugin/lto.exp (lto_link_tests): Likewise.
>  
> Approved - please apply.
> 
> Cheers
>   Nick
> 
> 

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH 3/3] bfd: Improve lookup of file / line information for errors
  2017-02-14  9:10   ` Nick Clifton
@ 2017-02-15 23:42     ` Andrew Burgess
  2017-02-16 10:22       ` Nick Clifton
  0 siblings, 1 reply; 14+ messages in thread
From: Andrew Burgess @ 2017-02-15 23:42 UTC (permalink / raw)
  To: Nick Clifton; +Cc: binutils

* Nick Clifton <nickc@redhat.com> [2017-02-14 09:10:08 +0000]:

> Hi Andrew,
> 
> > @@ -4155,6 +4155,38 @@ _bfd_dwarf2_find_nearest_line (bfd *abfd,
> 
> The find_nearest_line functions are used not only when emitting error/warning
> messages, but also when displaying disassembly mixed with source code.  Hence
> it is helpful if these functions can be relatively efficient.  (I have fixed
> several PRs over the last year about "objdump -dS" being very slow...)  With
> that in mind then:
> 
> 
> > +	  for (tmp = symbols; (*tmp) != NULL; ++tmp)
> > +	    if ((*tmp)->the_bfd == abfd
> > +		&& (*tmp)->section == section
> > +		&& (*tmp)->value == offset
> > +		&& ((*tmp)->flags & BSF_SECTION_SYM) == 0)
> 
> Wouldn't a binary search be more efficient here ?  Also it might be worthwhile
> caching the result in case the next call is for the same offset, or a very similar
> one.

The line immediately before the loop:

    if ((section->flags & SEC_CODE) == 0)

means that 'objdump -dS' will not be slowed down, however, I see your
point, 'objdump -DS' would be slowed down.

If I wanted to add a binary search we'd need the symbols sorted.  I
don't think that we can rely on the symbols being sorted when we enter
_bfd_dwarf2_find_nearest_line can we?  In the objdump/disassemble case
the symbols come (for elf) from elf_slurp_symbol_table, which, I don't
think guarantees ordering.

Caching the last match might help I guess, in the disassembly case
where the offset is constantly changing we could resume searching from
the last match, wrapping round at the end of the symbol list and
stopping when we get back to our start point.  This would speed things
up if we assume that symbols are usually sorted (sounds likely).

My question then would be, do you agree that the incoming symbols are
unsorted, and do you think I should create a sorted copy in the cached
data, and so allow for a binary search?

I appreciate your advice,

Thanks,
Andrew

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH 1/3] ld: Add additional checking for warnings/errors in testsuite
  2017-02-15 21:15     ` Andrew Burgess
@ 2017-02-16  9:42       ` Nick Clifton
  0 siblings, 0 replies; 14+ messages in thread
From: Nick Clifton @ 2017-02-16  9:42 UTC (permalink / raw)
  To: Andrew Burgess; +Cc: binutils

Hi Andrew,

> That's fine.  What's a good way to report the set of tests I've run
> against?

If you test against lots of targets then just saying so should be enough.
It is more important to know if only a few targets have been tested as then
there might be gaps in the coverage.

>    https://raw.githubusercontent.com/T-J-Teru/binutils-gdb-testing/master/Makefile
> 
> which runs against 245 targets I've found from various sources.  I
> expect there's probably some duplication in there, one day I'll try to
> reduce the list...

What a great list of targets.  There were several that I had missed, so I have added
them to my local build scripts.

Cheers
  Nick


^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH 3/3] bfd: Improve lookup of file / line information for errors
  2017-02-15 23:42     ` Andrew Burgess
@ 2017-02-16 10:22       ` Nick Clifton
  0 siblings, 0 replies; 14+ messages in thread
From: Nick Clifton @ 2017-02-16 10:22 UTC (permalink / raw)
  To: Andrew Burgess; +Cc: binutils

Hi Andrew,

> The line immediately before the loop:
> 
>     if ((section->flags & SEC_CODE) == 0)
> 
> means that 'objdump -dS' will not be slowed down, however, I see your
> point, 'objdump -DS' would be slowed down.

Good point.

> Caching the last match might help I guess, in the disassembly case
> where the offset is constantly changing we could resume searching from
> the last match, wrapping round at the end of the symbol list and
> stopping when we get back to our start point.  This would speed things
> up if we assume that symbols are usually sorted (sounds likely).

Let's apply the KISS principle here and not make the code more complex
than it needs to be at this point.

> My question then would be, do you agree that the incoming symbols are
> unsorted,

Yes.  Or at least we cannot rely upon them being sorted.

> and do you think I should create a sorted copy in the cached
> data, and so allow for a binary search?

Hmm tough call.  Given that this is for data sections, not code, I think
that the overhead would probably be too much.  So lets not do this now.
If later on we get a user report complaining about poor performance of 
this code (for a reasonable use case) then we can consider adding sorting.

So, just to be clear, your original 3/3 patch is now approved - please apply.

Cheers
  Nick

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH 2/3] bfd/dwarf: Improve use of previously loaded dwarf information
  2017-02-13 11:30 ` [PATCH 2/3] bfd/dwarf: Improve use of previously loaded dwarf information Andrew Burgess
  2017-02-14  9:33   ` Nick Clifton
@ 2017-02-17  7:10   ` Alan Modra
  2017-02-17  7:32     ` Alan Modra
  1 sibling, 1 reply; 14+ messages in thread
From: Alan Modra @ 2017-02-17  7:10 UTC (permalink / raw)
  To: Andrew Burgess; +Cc: binutils

On Mon, Feb 13, 2017 at 11:30:08AM +0000, Andrew Burgess wrote:
> --- /dev/null
> +++ b/ld/testsuite/ld-elf/dwarf3.c
> @@ -0,0 +1,13 @@
> +/* This test is actually used to test for a segfault that came from the bfd
> +   dwarf parsing code in the case when there is _no_ dwarf info.  */
[snip]

If the above is indeed the aim of the test, then I think we should
relax the test a little.  Some targets (eg. alpha-linux and
hppa-linux) don't emit STT_FILE symbols in objects, in which case
_bfd_elf_find_function cannot return a filename.  The result is
something like:

tmpdir/dwarf3.o: In function `main':
(.text.startup+0x8): undefined reference to `bar'
(.text.startup+0x10): undefined reference to `bar'

diff --git a/ld/ChangeLog b/ld/ChangeLog
index 7d47397..12c93c4 100644
--- a/ld/ChangeLog
+++ b/ld/ChangeLog
@@ -1,3 +1,8 @@
+2017-02-17  Alan Modra  <amodra@gmail.com>
+
+	* testsuite/ld-elf/dwarf2.err: Add missing newline at end.
+	* testsuite/ld-elf/dwarf3.err: Likewise.  Allow match without filename.
+
 2017-02-16  Andrew Burgess  <andrew.burgess@embecosm.com>
 
 	* testsuite/ld-elf/shared.exp: Update expected results.
diff --git a/ld/testsuite/ld-elf/dwarf2.err b/ld/testsuite/ld-elf/dwarf2.err
index b4ea67f..6b97ee6 100644
--- a/ld/testsuite/ld-elf/dwarf2.err
+++ b/ld/testsuite/ld-elf/dwarf2.err
@@ -2,4 +2,4 @@ tmpdir/dwarf2b\.o:.*dwarf2b\.c:1: multiple definition of `global_var'
 tmpdir/dwarf2a\.o:.*dwarf2a\.c:1: first defined here
 tmpdir/dwarf2b\.o:.*dwarf2b\.c:2: multiple definition of `other_var'
 tmpdir/dwarf2a\.o:.*dwarf2a\.c:2: first defined here
-#...
\ No newline at end of file
+#...
diff --git a/ld/testsuite/ld-elf/dwarf3.err b/ld/testsuite/ld-elf/dwarf3.err
index 6027431..6f5a8cc 100644
--- a/ld/testsuite/ld-elf/dwarf3.err
+++ b/ld/testsuite/ld-elf/dwarf3.err
@@ -1,4 +1,4 @@
 .*/dwarf3\.o: In function `main':
-.*dwarf3.c:\(\.text.*\+0x[0-9a-f]+\): undefined reference to `bar'
-.*dwarf3.c:\(\.text.*\+0x[0-9a-f]+\): undefined reference to `bar'
-#...
\ No newline at end of file
+.*\(\.text.*\+0x[0-9a-f]+\): undefined reference to `bar'
+.*\(\.text.*\+0x[0-9a-f]+\): undefined reference to `bar'
+#...

-- 
Alan Modra
Australia Development Lab, IBM

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH 2/3] bfd/dwarf: Improve use of previously loaded dwarf information
  2017-02-17  7:10   ` Alan Modra
@ 2017-02-17  7:32     ` Alan Modra
  2017-02-17 23:10       ` Andrew Burgess
  0 siblings, 1 reply; 14+ messages in thread
From: Alan Modra @ 2017-02-17  7:32 UTC (permalink / raw)
  To: Andrew Burgess; +Cc: binutils

I should have waited for my test run to finish before committing the
last patch.  This change is for sparc64, which helpfully spits out
"Disabling relaxation: it will not work with multiple definitions"
after the first error.

diff --git a/ld/ChangeLog b/ld/ChangeLog
index 1c17c17..59a3b0d 100644
--- a/ld/ChangeLog
+++ b/ld/ChangeLog
@@ -1,5 +1,10 @@
 2017-02-17  Alan Modra  <amodra@gmail.com>
 
+	* testsuite/ld-elf/dwarf2.err: Accept other errors between the
+	multiple definition errors.
+
+2017-02-17  Alan Modra  <amodra@gmail.com>
+
 	* testsuite/ld-elf/dwarf2.err: Add missing newline at end.
 	* testsuite/ld-elf/dwarf3.err: Likewise.  Allow match without filename.
 
diff --git a/ld/testsuite/ld-elf/dwarf2.err b/ld/testsuite/ld-elf/dwarf2.err
index 6b97ee6..b3d301f 100644
--- a/ld/testsuite/ld-elf/dwarf2.err
+++ b/ld/testsuite/ld-elf/dwarf2.err
@@ -1,5 +1,6 @@
 tmpdir/dwarf2b\.o:.*dwarf2b\.c:1: multiple definition of `global_var'
 tmpdir/dwarf2a\.o:.*dwarf2a\.c:1: first defined here
+#...
 tmpdir/dwarf2b\.o:.*dwarf2b\.c:2: multiple definition of `other_var'
 tmpdir/dwarf2a\.o:.*dwarf2a\.c:2: first defined here
 #...

-- 
Alan Modra
Australia Development Lab, IBM

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH 2/3] bfd/dwarf: Improve use of previously loaded dwarf information
  2017-02-17  7:32     ` Alan Modra
@ 2017-02-17 23:10       ` Andrew Burgess
  0 siblings, 0 replies; 14+ messages in thread
From: Andrew Burgess @ 2017-02-17 23:10 UTC (permalink / raw)
  To: Alan Modra; +Cc: binutils

* Alan Modra <amodra@gmail.com> [2017-02-17 18:02:23 +1030]:

> I should have waited for my test run to finish before committing the
> last patch.  This change is for sparc64, which helpfully spits out
> "Disabling relaxation: it will not work with multiple definitions"
> after the first error.

Alan,

Thanks for finding and fixing these problems for me.  I guess I really
should have built myself a set of cross GCCs to really test this
change :-/

Thanks again,
Andrew



> 
> diff --git a/ld/ChangeLog b/ld/ChangeLog
> index 1c17c17..59a3b0d 100644
> --- a/ld/ChangeLog
> +++ b/ld/ChangeLog
> @@ -1,5 +1,10 @@
>  2017-02-17  Alan Modra  <amodra@gmail.com>
>  
> +	* testsuite/ld-elf/dwarf2.err: Accept other errors between the
> +	multiple definition errors.
> +
> +2017-02-17  Alan Modra  <amodra@gmail.com>
> +
>  	* testsuite/ld-elf/dwarf2.err: Add missing newline at end.
>  	* testsuite/ld-elf/dwarf3.err: Likewise.  Allow match without filename.
>  
> diff --git a/ld/testsuite/ld-elf/dwarf2.err b/ld/testsuite/ld-elf/dwarf2.err
> index 6b97ee6..b3d301f 100644
> --- a/ld/testsuite/ld-elf/dwarf2.err
> +++ b/ld/testsuite/ld-elf/dwarf2.err
> @@ -1,5 +1,6 @@
>  tmpdir/dwarf2b\.o:.*dwarf2b\.c:1: multiple definition of `global_var'
>  tmpdir/dwarf2a\.o:.*dwarf2a\.c:1: first defined here
> +#...
>  tmpdir/dwarf2b\.o:.*dwarf2b\.c:2: multiple definition of `other_var'
>  tmpdir/dwarf2a\.o:.*dwarf2a\.c:2: first defined here
>  #...
> 
> -- 
> Alan Modra
> Australia Development Lab, IBM

^ permalink raw reply	[flat|nested] 14+ messages in thread

end of thread, other threads:[~2017-02-17 23:10 UTC | newest]

Thread overview: 14+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-02-13 11:30 [PATCH 0/3] Improve information in linker errors Andrew Burgess
2017-02-13 11:30 ` [PATCH 1/3] ld: Add additional checking for warnings/errors in testsuite Andrew Burgess
2017-02-14 12:32   ` Nick Clifton
2017-02-15 21:15     ` Andrew Burgess
2017-02-16  9:42       ` Nick Clifton
2017-02-13 11:30 ` [PATCH 2/3] bfd/dwarf: Improve use of previously loaded dwarf information Andrew Burgess
2017-02-14  9:33   ` Nick Clifton
2017-02-17  7:10   ` Alan Modra
2017-02-17  7:32     ` Alan Modra
2017-02-17 23:10       ` Andrew Burgess
2017-02-13 11:30 ` [PATCH 3/3] bfd: Improve lookup of file / line information for errors Andrew Burgess
2017-02-14  9:10   ` Nick Clifton
2017-02-15 23:42     ` Andrew Burgess
2017-02-16 10:22       ` Nick Clifton

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).