public inbox for binutils@sourceware.org
 help / color / mirror / Atom feed
* PATCH: Mention symbol name in non-constant .size expression
@ 2011-03-05 15:32 H.J. Lu
  2011-03-06  8:33 ` Alan Modra
  2011-03-07  2:22 ` Hans-Peter Nilsson
  0 siblings, 2 replies; 7+ messages in thread
From: H.J. Lu @ 2011-03-05 15:32 UTC (permalink / raw)
  To: binutils; +Cc: Alan Modra

Hi,

The error message in non-constant .size expression doesn't show
the correct line number nor symbol names.  It is hard to get the
line number right.  This patch displays symbol name in non-constant
.size expression.  It will help users identify the assembly code bugs.
OK to install?

Thanks.


H.J.
---
diff --git a/gas/ChangeLog.intel b/gas/ChangeLog.intel
index 8eb236d..38ee6a2 100644
--- a/gas/ChangeLog.intel
+++ b/gas/ChangeLog.intel
@@ -1,3 +1,8 @@
+2011-03-05  H.J. Lu  <hongjiu.lu@intel.com>
+
+	* config/obj-elf.c (elf_frob_symbol): Mention symbol name in
+	non-constant .size expression.
+
 2010-03-08  Alan Modra  <amodra@gmail.com>
 
 	PR gas/11356
diff --git a/gas/config/obj-elf.c b/gas/config/obj-elf.c
index 969a509..d43409a 100644
--- a/gas/config/obj-elf.c
+++ b/gas/config/obj-elf.c
@@ -1879,6 +1879,7 @@ void
 elf_frob_symbol (symbolS *symp, int *puntp)
 {
   struct elf_obj_sy *sy_obj;
+  expressionS *size;
 
 #ifdef NEED_ECOFF_DEBUG
   if (ECOFF_DEBUGGING)
@@ -1887,13 +1888,50 @@ elf_frob_symbol (symbolS *symp, int *puntp)
 
   sy_obj = symbol_get_obj (symp);
 
-  if (sy_obj->size != NULL)
+  size = sy_obj->size;
+  if (size != NULL)
     {
-      if (resolve_expression (sy_obj->size)
-	  && sy_obj->size->X_op == O_constant)
-	S_SET_SIZE (symp, sy_obj->size->X_add_number);
+      if (resolve_expression (size)
+	  && size->X_op == O_constant)
+	S_SET_SIZE (symp, size->X_add_number);
       else
-	as_bad (_(".size expression does not evaluate to a constant"));
+	{
+	  const char *op_name = NULL;
+	  const char *add_name = NULL;
+
+	  if (size->X_op == O_subtract)
+	    {
+	      op_name = S_GET_NAME (size->X_op_symbol);
+	      add_name = S_GET_NAME (size->X_add_symbol);
+	      if (strcmp (op_name, FAKE_LABEL_NAME) == 0)
+		op_name = NULL;
+	      if (strcmp (add_name, FAKE_LABEL_NAME) == 0)
+		add_name = NULL;
+
+	      if (op_name && add_name)
+		as_bad (_(".size expression with symbols `%s' and `%s' "
+			  "does not evaluate to a constant"),
+			op_name, add_name);
+	      else
+		{
+		  const char *name;
+
+		  if (op_name)
+		    name = op_name;
+		  else if (add_name)
+		    name = add_name;
+		  else
+		    name = NULL;
+
+		  if (name)
+		    as_bad (_(".size expression with symbol `%s' "
+			      "does not evaluate to a constant"), name);
+		}
+	    }
+	  
+	  if (!op_name && !add_name)
+	    as_bad (_(".size expression does not evaluate to a constant"));
+	}
       free (sy_obj->size);
       sy_obj->size = NULL;
     }
diff --git a/gas/testsuite/ChangeLog.intel b/gas/testsuite/ChangeLog.intel
index 47d7048..602730c 100644
--- a/gas/testsuite/ChangeLog.intel
+++ b/gas/testsuite/ChangeLog.intel
@@ -1,3 +1,7 @@
+2011-03-05  H.J. Lu  <hongjiu.lu@intel.com>
+
+	* gas/elf/bad-size.err: Updated.
+
 2010-03-08  H.J. Lu  <hongjiu.lu@intel.com>
 
 	PR gas/9966
diff --git a/gas/testsuite/gas/elf/bad-size.err b/gas/testsuite/gas/elf/bad-size.err
index 5e01ef2..a5bfc31 100644
--- a/gas/testsuite/gas/elf/bad-size.err
+++ b/gas/testsuite/gas/elf/bad-size.err
@@ -1,2 +1,2 @@
 .*bad-size\.s: Assembler messages:
-.*bad-size\.s:6: Error: .*
+.*bad-size\.s:6: Error:.*`_test_nop'.*

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

* Re: PATCH: Mention symbol name in non-constant .size expression
  2011-03-05 15:32 PATCH: Mention symbol name in non-constant .size expression H.J. Lu
@ 2011-03-06  8:33 ` Alan Modra
  2011-03-07  2:22 ` Hans-Peter Nilsson
  1 sibling, 0 replies; 7+ messages in thread
From: Alan Modra @ 2011-03-06  8:33 UTC (permalink / raw)
  To: H.J. Lu; +Cc: binutils

On Sat, Mar 05, 2011 at 07:32:49AM -0800, H.J. Lu wrote:
> +	* config/obj-elf.c (elf_frob_symbol): Mention symbol name in
> +	non-constant .size expression.

OK.  I may revert this soon when/if I code up an idea I had of
creating a hash table to store size expressions and their original
file/line number.  It seems silly to tack an extra field on all
symbols just to handle .size on a few.

-- 
Alan Modra
Australia Development Lab, IBM

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

* Re: PATCH: Mention symbol name in non-constant .size expression
  2011-03-05 15:32 PATCH: Mention symbol name in non-constant .size expression H.J. Lu
  2011-03-06  8:33 ` Alan Modra
@ 2011-03-07  2:22 ` Hans-Peter Nilsson
  2011-03-07  2:51   ` H.J. Lu
  1 sibling, 1 reply; 7+ messages in thread
From: Hans-Peter Nilsson @ 2011-03-07  2:22 UTC (permalink / raw)
  To: hjl.tools; +Cc: binutils, amodra

> Date: Sat, 5 Mar 2011 07:32:49 -0800
> From: "H.J. Lu" <hongjiu.lu@intel.com>

> @@ -1,3 +1,7 @@
> +2011-03-05  H.J. Lu  <hongjiu.lu@intel.com>
> +
> +	* gas/elf/bad-size.err: Updated.
> +
>  2010-03-08  H.J. Lu  <hongjiu.lu@intel.com>
>  
>  	PR gas/9966

> diff --git a/gas/testsuite/gas/elf/bad-size.err b/gas/testsuite/gas/elf/bad-size.err
> index 5e01ef2..a5bfc31 100644
> --- a/gas/testsuite/gas/elf/bad-size.err
> +++ b/gas/testsuite/gas/elf/bad-size.err
> @@ -1,2 +1,2 @@
>  .*bad-size\.s: Assembler messages:
> -.*bad-size\.s:6: Error: .*
> +.*bad-size\.s:6: Error:.*`_test_nop'.*
> 


That fails for cris-elf and cris-axis-linux-gnu.  I still get:

../as-new   -o dump.o /tmp/hpautotest-binutils/bsrc/src/gas/testsuite/gas/elf/bad-size.s
Executing on host: sh -c {../as-new   -o dump.o /tmp/hpautotest-binutils/bsrc/src/gas/testsuite/gas/elf/bad-size.s 2>&1}
  /dev/null gas.out (timeout = 300)
/tmp/hpautotest-binutils/bsrc/src/gas/testsuite/gas/elf/bad-size.s: Assembler messages:
/tmp/hpautotest-binutils/bsrc/src/gas/testsuite/gas/elf/bad-size.s:6: Error: operation combines symbols in different seg
ments
/tmp/hpautotest-binutils/bsrc/src/gas/testsuite/gas/elf/bad-size.s: Assembler messages:
/tmp/hpautotest-binutils/bsrc/src/gas/testsuite/gas/elf/bad-size.s:6: Error: operation combines symbols in different seg
ments
regexp_diff match failure
regexp "^.*bad-size\.s:6: Error:.*`_test_nop'.*$"
line   "/tmp/hpautotest-binutils/bsrc/src/gas/testsuite/gas/elf/bad-size.s:6: Error: operation combines symbols in different segments"
FAIL: Check bad size directive

brgds, H-P

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

* Re: PATCH: Mention symbol name in non-constant .size expression
  2011-03-07  2:22 ` Hans-Peter Nilsson
@ 2011-03-07  2:51   ` H.J. Lu
  2011-03-07  3:28     ` Hans-Peter Nilsson
  0 siblings, 1 reply; 7+ messages in thread
From: H.J. Lu @ 2011-03-07  2:51 UTC (permalink / raw)
  To: Hans-Peter Nilsson; +Cc: binutils, amodra

On Sun, Mar 6, 2011 at 6:22 PM, Hans-Peter Nilsson
<hans-peter.nilsson@axis.com> wrote:
>> Date: Sat, 5 Mar 2011 07:32:49 -0800
>> From: "H.J. Lu" <hongjiu.lu@intel.com>
>
>> @@ -1,3 +1,7 @@
>> +2011-03-05  H.J. Lu  <hongjiu.lu@intel.com>
>> +
>> +     * gas/elf/bad-size.err: Updated.
>> +
>>  2010-03-08  H.J. Lu  <hongjiu.lu@intel.com>
>>
>>       PR gas/9966
>
>> diff --git a/gas/testsuite/gas/elf/bad-size.err b/gas/testsuite/gas/elf/bad-size.err
>> index 5e01ef2..a5bfc31 100644
>> --- a/gas/testsuite/gas/elf/bad-size.err
>> +++ b/gas/testsuite/gas/elf/bad-size.err
>> @@ -1,2 +1,2 @@
>>  .*bad-size\.s: Assembler messages:
>> -.*bad-size\.s:6: Error: .*
>> +.*bad-size\.s:6: Error:.*`_test_nop'.*
>>
>
>
> That fails for cris-elf and cris-axis-linux-gnu.  I still get:
>
> ../as-new   -o dump.o /tmp/hpautotest-binutils/bsrc/src/gas/testsuite/gas/elf/bad-size.s
> Executing on host: sh -c {../as-new   -o dump.o /tmp/hpautotest-binutils/bsrc/src/gas/testsuite/gas/elf/bad-size.s 2>&1}
>  /dev/null gas.out (timeout = 300)
> /tmp/hpautotest-binutils/bsrc/src/gas/testsuite/gas/elf/bad-size.s: Assembler messages:
> /tmp/hpautotest-binutils/bsrc/src/gas/testsuite/gas/elf/bad-size.s:6: Error: operation combines symbols in different seg
> ments
> /tmp/hpautotest-binutils/bsrc/src/gas/testsuite/gas/elf/bad-size.s: Assembler messages:
> /tmp/hpautotest-binutils/bsrc/src/gas/testsuite/gas/elf/bad-size.s:6: Error: operation combines symbols in different seg
> ments
> regexp_diff match failure
> regexp "^.*bad-size\.s:6: Error:.*`_test_nop'.*$"
> line   "/tmp/hpautotest-binutils/bsrc/src/gas/testsuite/gas/elf/bad-size.s:6: Error: operation combines symbols in different segments"
> FAIL: Check bad size directive
>

I can  change it to skip cris.


-- 
H.J.

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

* Re: PATCH: Mention symbol name in non-constant .size expression
  2011-03-07  2:51   ` H.J. Lu
@ 2011-03-07  3:28     ` Hans-Peter Nilsson
  2011-03-07  3:55       ` Alan Modra
  0 siblings, 1 reply; 7+ messages in thread
From: Hans-Peter Nilsson @ 2011-03-07  3:28 UTC (permalink / raw)
  To: hjl.tools; +Cc: hp, binutils, amodra

> Date: Sun, 6 Mar 2011 18:51:43 -0800
> From: "H.J. Lu" <hjl.tools@gmail.com>

> On Sun, Mar 6, 2011 at 6:22 PM, Hans-Peter Nilsson
> <hans-peter.nilsson@axis.com> wrote:
> > FAIL: Check bad size directive
> 
> I can  change it to skip cris.

If there's no intention to actually fix it, I think xfail is
more appropriate; no particular reason it should fail, IMHO.

brgds, H-P

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

* Re: PATCH: Mention symbol name in non-constant .size expression
  2011-03-07  3:28     ` Hans-Peter Nilsson
@ 2011-03-07  3:55       ` Alan Modra
  2011-03-07  5:43         ` H.J. Lu
  0 siblings, 1 reply; 7+ messages in thread
From: Alan Modra @ 2011-03-07  3:55 UTC (permalink / raw)
  To: Hans-Peter Nilsson; +Cc: hjl.tools, hp, binutils

On Mon, Mar 07, 2011 at 04:28:02AM +0100, Hans-Peter Nilsson wrote:
> If there's no intention to actually fix it, I think xfail is
> more appropriate; no particular reason it should fail, IMHO.

The new test will fail for any target without DIFF_EXPR_OK defined.
HJ, I don't think it is worth going to the bother of firstly finding
out exactly what set of targets that is, then xfailing them all.
Please just revert the testsuite change that checks for the symbol
name.

-- 
Alan Modra
Australia Development Lab, IBM

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

* Re: PATCH: Mention symbol name in non-constant .size expression
  2011-03-07  3:55       ` Alan Modra
@ 2011-03-07  5:43         ` H.J. Lu
  0 siblings, 0 replies; 7+ messages in thread
From: H.J. Lu @ 2011-03-07  5:43 UTC (permalink / raw)
  To: Hans-Peter Nilsson, hp, binutils; +Cc: Alan Modra

On Sun, Mar 6, 2011 at 7:54 PM, Alan Modra <amodra@gmail.com> wrote:
> On Mon, Mar 07, 2011 at 04:28:02AM +0100, Hans-Peter Nilsson wrote:
>> If there's no intention to actually fix it, I think xfail is
>> more appropriate; no particular reason it should fail, IMHO.
>
> The new test will fail for any target without DIFF_EXPR_OK defined.
> HJ, I don't think it is worth going to the bother of firstly finding
> out exactly what set of targets that is, then xfailing them all.
> Please just revert the testsuite change that checks for the symbol
> name.
>

Done.


-- 
H.J.

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

end of thread, other threads:[~2011-03-07  5:43 UTC | newest]

Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2011-03-05 15:32 PATCH: Mention symbol name in non-constant .size expression H.J. Lu
2011-03-06  8:33 ` Alan Modra
2011-03-07  2:22 ` Hans-Peter Nilsson
2011-03-07  2:51   ` H.J. Lu
2011-03-07  3:28     ` Hans-Peter Nilsson
2011-03-07  3:55       ` Alan Modra
2011-03-07  5:43         ` H.J. Lu

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