Better diagnostics for undefined symbols in assignments to dot

Better diagnostics for undefined symbols in assignments to dot

[Zack Weinberg]

Currently, if you write a linker script which assigns dot to an
expression involving a symbol, and that symbol is undefined, you get
the not-terribly-helpful diagnostic "invalid assignment to location
counter".  For instance:

$ cat test.ld
SECTIONS
{
  .text : { *(.text) }
  . = ALIGN(__data_align);
  .data : { *(.data) }
}
$ cat test.c
int x = 23;
int foo(void) { return x; }
$ gcc -c test.c

# intended use
$ ./ld-new --defsym __data_align=32 --script test.ld test.o -o test.x; echo $?
0

# mistaken use
$ ./ld-new --script test.ld test.o -o test.x; echo $?
test.ld:7: invalid assignment to location counter
1

The problem appears to be that fold_name only issues a diagnostic for
an undefined symbol if allocation_done is lang_final_phase_enum, but
the assignment to dot happens earlier than that.

Experimentally, I tried forcing allocation_done to
lang_final_phase_enum for the recursive call to exp_fold_tree to
process the right-hand side of the assignment expression.  This gets
me a nicer diagnostic:

$ ./ld-new --script test.ld test.o -o test.x; echo $?
test.ld:7: undefined symbol `__data_align' referenced in expression
1

but it also seems to break the i386 TLS tests.  Would anyone have any
ideas?  My patch is appended below for reference.

zw

        * ldexp.c (exp_fold_tree <case etree_assign>): For assignments
        to the location counter, force allocation_done to
        lang_final_phase_enum for better diagnostics.

===================================================================
Index: ld/ldexp.c
--- ld/ldexp.c	21 Jan 2005 04:15:58 -0000	1.42
+++ ld/ldexp.c	8 Feb 2005 20:53:14 -0000
@@ -754,9 +754,13 @@ exp_fold_tree (etree_type *tree,
 	      || (allocation_done == lang_final_phase_enum
 		  && current_section == abs_output_section))
 	    {
+	      /* Fold the source expression with allocation_done
+		 forced to lang_final_phase_enum in order to get a
+		 better diagnostic if the expression involves an
+		 undefined symbol.  */
 	      result = exp_fold_tree (tree->assign.src,
 				      current_section,
-				      allocation_done, dot,
+				      lang_final_phase_enum, dot,
 				      dotp);
 	      if (! result.valid_p)
 		einfo (_("%F%S invalid assignment to location counter\n"));

Re: Better diagnostics for undefined symbols in assignments to dot

[Zack Weinberg]

[-- Attachment #1: Type: text/plain, Size: 1565 bytes --]

On Tue, 2005-02-08 at 12:55 -0800, Zack Weinberg wrote:
> Currently, if you write a linker script which assigns dot to an
> expression involving a symbol, and that symbol is undefined, you get
> the not-terribly-helpful diagnostic "invalid assignment to location
> counter".  For instance:
[...]

Since no one seems interested in helping me with this, I have redone it
in a way which at least does not break anything.  I'm not hugely
enthused about adding a file-global state flag, but then neither am I
enthused about the alternative of adding another argument to just about
every function in ldexp.c (it wouldn't be used in most of them, but who
knows, my clients may come back next week with another place where the
diagnostics would be better if we knew we were processing an assignment
to the location counter).

I've also added a full set of tests for both the good diagnostic and the
construct that raised the issue in the first place.

OK to commit?

zw

	* ldexp.c (assigning_to_dot): New global flag.
	(fold_name): If assigning_to_dot is true, object immediately to
	an undefined symbol.
	(exp_fold_tree): Set and clear assigning_to_dot around the
	recursive call to exp_fold_tree to process the right-hand side
	of an assignment to the location counter.

testsuite:
	* ld-scripts/align.exp: Rename existing "ALIGN" test to "align1".
	Add dump tests "align2a", "align2b", "align2c".
	* ld-scripts/align2.t, ld-scripts/align2a.s, ld-scripts/align2a.d
	* ld-scripts/align2b.s, ld-scripts/align2b.d
	* ld-scripts/align2c.s, ld-scripts/align2c.d: New files.


[-- Attachment #2: Type: text/x-patch, Size: 4958 bytes --]

===================================================================
Index: ldexp.c
--- ldexp.c	21 Jan 2005 04:15:58 -0000	1.42
+++ ldexp.c	12 Feb 2005 00:11:51 -0000
@@ -50,6 +50,9 @@ struct exp_data_seg exp_data_seg;
 
 segment_type *segments;
 
+/* Principally used for diagnostics.  */
+static bfd_boolean assigning_to_dot = FALSE;
+
 /* Print the string representation of the given token.  Surround it
    with spaces if INFIX_P is TRUE.  */
 
@@ -595,7 +598,8 @@ fold_name (etree_type *tree,
 		    }
 		}
 	    }
-	  else if (allocation_done == lang_final_phase_enum)
+	  else if (allocation_done == lang_final_phase_enum
+		   || assigning_to_dot)
 	    einfo (_("%F%S: undefined symbol `%s' referenced in expression\n"),
 		   tree->name.name);
 	  else if (h->type == bfd_link_hash_new)
@@ -754,10 +758,13 @@ exp_fold_tree (etree_type *tree,
 	      || (allocation_done == lang_final_phase_enum
 		  && current_section == abs_output_section))
 	    {
+	      /* Notify the folder that this is an assignment to dot.  */
+	      assigning_to_dot = TRUE;
 	      result = exp_fold_tree (tree->assign.src,
 				      current_section,
-				      allocation_done, dot,
-				      dotp);
+				      allocation_done, dot, dotp);
+	      assigning_to_dot = FALSE;
+
 	      if (! result.valid_p)
 		einfo (_("%F%S invalid assignment to location counter\n"));
 	      else
===================================================================
Index: testsuite/ld-scripts/align.exp
--- testsuite/ld-scripts/align.exp	8 Apr 2004 00:51:37 -0000	1.2
+++ testsuite/ld-scripts/align.exp	12 Feb 2005 00:11:51 -0000
@@ -22,7 +22,7 @@ if [istarget "rs6000-*-aix*"] {
     return
 }
 
-set testname "ALIGN"
+set testname "align1"
 
 if ![ld_assemble $as $srcdir/$subdir/align.s tmpdir/align.o] {
     unresolved $testname
@@ -34,3 +34,7 @@ if ![ld_simple_link $ld tmpdir/align "-T
 } else {
     pass $testname
 }
+
+run_dump_test align2a
+run_dump_test align2b
+run_dump_test align2c
===================================================================
Index: testsuite/ld-scripts/align2.t
--- testsuite/ld-scripts/align2.t	1 Jan 1970 00:00:00 -0000
+++ testsuite/ld-scripts/align2.t	12 Feb 2005 00:11:51 -0000
@@ -0,0 +1,6 @@
+SECTIONS
+{
+  .text : {*(.text)}
+  . = ALIGN(data_align);
+  .data : {*(.data)}
+}
===================================================================
Index: testsuite/ld-scripts/align2a.d
--- testsuite/ld-scripts/align2a.d	1 Jan 1970 00:00:00 -0000
+++ testsuite/ld-scripts/align2a.d	12 Feb 2005 00:11:51 -0000
@@ -0,0 +1,13 @@
+# ld: --defsym data_align=16 -T align2.t
+# objdump: --section-headers
+
+[^:]+: +file format.*
+
+Sections:
+Idx +Name +Size +VMA +LMA +File +off +Algn
+ +0 +\.text +00000004 +00000000 +00000000 +00001000 +2\*\*2
+ +CONTENTS, +ALLOC, +LOAD, +READONLY, +CODE
+ +1 +\.data +00000004 +00000010 +00000010 +00001010 +2\*\*2
+ +CONTENTS, +ALLOC, +LOAD, +DATA
+ +2 +\.bss +00000000 +00000014 +00000014 +00001014 +2\*\*2
+ +ALLOC
===================================================================
Index: testsuite/ld-scripts/align2a.s
--- testsuite/ld-scripts/align2a.s	1 Jan 1970 00:00:00 -0000
+++ testsuite/ld-scripts/align2a.s	12 Feb 2005 00:11:51 -0000
@@ -0,0 +1,4 @@
+	.text
+	.long 0
+	.data
+	.long 0x12345678
===================================================================
Index: testsuite/ld-scripts/align2b.d
--- testsuite/ld-scripts/align2b.d	1 Jan 1970 00:00:00 -0000
+++ testsuite/ld-scripts/align2b.d	12 Feb 2005 00:11:51 -0000
@@ -0,0 +1,13 @@
+# ld: --defsym data_align=32 -T align2.t
+# objdump: --section-headers
+
+[^:]+: +file +format.*
+
+Sections:
+Idx +Name +Size +VMA +LMA +File off +Algn
+ +0 +\.text +00000004 +00000000 +00000000 +00001000 +2\*\*2
+ +CONTENTS, +ALLOC, +LOAD, +READONLY, +CODE
+ +1 +\.data +00000004 +00000020 +00000020 +00001020 +2\*\*2
+ +CONTENTS, +ALLOC, +LOAD, +DATA
+ +2 +\.bss +00000000 +00000024 +00000024 +00001024 +2\*\*2
+ +ALLOC
===================================================================
Index: testsuite/ld-scripts/align2b.s
--- testsuite/ld-scripts/align2b.s	1 Jan 1970 00:00:00 -0000
+++ testsuite/ld-scripts/align2b.s	12 Feb 2005 00:11:51 -0000
@@ -0,0 +1,4 @@
+	.text
+	.long 0
+	.data
+	.long 0x12345678
===================================================================
Index: testsuite/ld-scripts/align2c.d
--- testsuite/ld-scripts/align2c.d	1 Jan 1970 00:00:00 -0000
+++ testsuite/ld-scripts/align2c.d	12 Feb 2005 00:11:51 -0000
@@ -0,0 +1,2 @@
+# ld: -T align2.t
+# error: undefined symbol.*in expression
===================================================================
Index: testsuite/ld-scripts/align2c.s
--- testsuite/ld-scripts/align2c.s	1 Jan 1970 00:00:00 -0000
+++ testsuite/ld-scripts/align2c.s	12 Feb 2005 00:11:51 -0000
@@ -0,0 +1,4 @@
+	.text
+	.long 0
+	.data
+	.long 0x12345678

Re: Better diagnostics for undefined symbols in assignments to dot

[Nick Clifton]

Hi Zack,

> Since no one seems interested in helping me with this, 

Sorry :-(


> I have redone it
> in a way which at least does not break anything.  I'm not hugely
> enthused about adding a file-global state flag,

I agree - but your way is certainly the cleanest and simplest way of 
solving the problem.

> 	* ldexp.c (assigning_to_dot): New global flag.
> 	(fold_name): If assigning_to_dot is true, object immediately to
> 	an undefined symbol.
> 	(exp_fold_tree): Set and clear assigning_to_dot around the
> 	recursive call to exp_fold_tree to process the right-hand side
> 	of an assignment to the location counter.
> 
> testsuite:
> 	* ld-scripts/align.exp: Rename existing "ALIGN" test to "align1".
> 	Add dump tests "align2a", "align2b", "align2c".
> 	* ld-scripts/align2.t, ld-scripts/align2a.s, ld-scripts/align2a.d
> 	* ld-scripts/align2b.s, ld-scripts/align2b.d
> 	* ld-scripts/align2c.s, ld-scripts/align2c.d: New files.

Approved - please apply.

Cheers
   Nick