This is the mail archive of the binutils@sources.redhat.com mailing list for the binutils project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

Re: [PATCH] ia64 unwind directive semantics


>>> James E Wilson <wilson@specifixinc.com> 11.02.05 00:40:35 >>>
>On Wed, 2005-02-02 at 06:52, Jan Beulich wrote:
>>        hash_delete (md.dynreg_hash, dr->name);
>> +      obstack_free (&notes, (void *) dr->name);
>
>I don't believe this is safe.  obstacks are stack based.  When you
call
>obstack_free, not only does it free dr->name, but also everything
newer
>than it.  There is no guarantee that everything newer than dr->name
is
>unused.  This is very unlikely to be the case.  There could have been
an
>obstack call used for something else (non hash table) in the
meantime.

Indeed. What lead me to think these aren't, despite their name,
stack-like was the fact that dot_alias, in its error path, has two
invocations of obstack_free; according what you say (the later) one
should be sufficient there...

>There is also an efficiency issue here, as the right way to free
these
>string if we could would be with a single obstack_free call, for the
>oldest string.  Calling obstack_free on each string means we are
freeing
>them multiple times, which is probably not good.
>
>This is different than the demand_copy_C_string case, as in that case
we
>know that there is no other obstack allocation between the
>demand_copy_C_string call and the obstack_free.
>
>If we want to be able to free these strings here, then we can't use
>obstack to allocate them.

Perhaps we should.

>>  	  as_bad ("Attempt to redefine register set `%s'", name);
>> +	  obstack_free (&notes, name);
>
>This one is safe, because we know that there are no other allocations
on
>the notes obstack since this string was allocated.

After you stated the above, it would seem to me that this isn't safe
either, because there is another (conditional allocation inbetween).
Thus the order of these two needs to be changed.

Below/attached an updated patch.

Jan

gas/
2005-02-11  Jan Beulich  <jbeulich@novell.com>

	* config/tc-ia64.c (dot_rot): Add comment that name strings
should
	be freed when wiping out previous state. Canonicalize names
before
	use. Free name string when detecting redefinition.
	(dot_pred_rel): Call generic expression parser to process
arguments.
	Handle O_register case for individual predicates and O_subtract
for
	ranges.
	(ia64_parse_name): Canonicalize name before looking it up in
dynamic
	register hash.
	(ia64_canonicalize_symbol_name): Strip off all trailing #
characters.
	Warn if multiple found, issue error if resulting symbol name has
zero
	length.
	(dot_alias): Canonicalize name before use.

gas/testsuite/
2005-02-11  Jan Beulich  <jbeulich@novell.com>

	* gas/ia64/pound.[ls]: New.
	* gas/ia64/ia64.exp: Run new test.

---
/home/jbeulich/src/binutils/mainline/2005-02-11/gas/config/tc-ia64.c	2005-02-11
11:09:20.000000000 +0100
+++ 2005-02-11/gas/config/tc-ia64.c	2005-02-11 12:33:39.408990008
+0100
@@ -4479,6 +4479,7 @@ dot_rot (type)
   for (dr = md.dynreg[type]; dr && dr->num_regs; dr = dr->next)
     {
       hash_delete (md.dynreg_hash, dr->name);
+      /* FIXME: Free dr->name.  */
       dr->num_regs = 0;
     }
 
@@ -4487,8 +4488,8 @@ dot_rot (type)
     {
       start = input_line_pointer;
       ch = get_symbol_end ();
+      len = strlen (ia64_canonicalize_symbol_name (start));
       *input_line_pointer = ch;
-      len = (input_line_pointer - start);
 
       SKIP_WHITESPACE ();
       if (*input_line_pointer != '[')
@@ -4537,16 +4538,16 @@ dot_rot (type)
 	  break;
 	}
 
-      name = obstack_alloc (&notes, len + 1);
-      memcpy (name, start, len);
-      name[len] = '\0';
-
       if (!*drpp)
 	{
 	  *drpp = obstack_alloc (&notes, sizeof (*dr));
 	  memset (*drpp, 0, sizeof (*dr));
 	}
 
+      name = obstack_alloc (&notes, len + 1);
+      memcpy (name, start, len);
+      name[len] = '\0';
+
       dr = *drpp;
       dr->name = name;
       dr->num_regs = num_regs;
@@ -4557,6 +4558,7 @@ dot_rot (type)
       if (hash_insert (md.dynreg_hash, name, dr))
 	{
 	  as_bad ("Attempt to redefine register set `%s'", name);
+	  obstack_free (&notes, name);
 	  goto err;
 	}
 
@@ -4984,59 +4986,57 @@ dot_pred_rel (type)
   SKIP_WHITESPACE ();
   while (1)
     {
-      valueT bit = 1;
+      valueT bits = 1;
       int regno;
+      expressionS pr, *pr1, *pr2;
 
-      if (TOUPPER (*input_line_pointer) != 'P'
-	  || (regno = atoi (++input_line_pointer)) < 0
-	  || regno > 63)
-	{
-	  as_bad (_("Predicate register expected"));
-	  ignore_rest_of_line ();
-	  return;
-	}
-      while (ISDIGIT (*input_line_pointer))
-	++input_line_pointer;
-      if (p1 == -1)
-	p1 = regno;
-      else if (p2 == -1)
-	p2 = regno;
-      bit <<= regno;
-      if (mask & bit)
-	as_warn (_("Duplicate predicate register ignored"));
-      mask |= bit;
-      count++;
-      /* See if it's a range.  */
-      if (*input_line_pointer == '-')
-	{
-	  valueT stop = 1;
-	  ++input_line_pointer;
-
-	  if (TOUPPER (*input_line_pointer) != 'P'
-	      || (regno = atoi (++input_line_pointer)) < 0
-	      || regno > 63)
-	    {
-	      as_bad (_("Predicate register expected"));
-	      ignore_rest_of_line ();
-	      return;
-	    }
-	  while (ISDIGIT (*input_line_pointer))
-	    ++input_line_pointer;
-	  stop <<= regno;
-	  if (bit >= stop)
+      expression (&pr);
+      if (pr.X_op == O_register
+	  && pr.X_add_number >= REG_P
+	  && pr.X_add_number <= REG_P + 63)
+	{
+	  regno = pr.X_add_number - REG_P;
+	  bits <<= regno;
+	  count++;
+	  if (p1 == -1)
+	    p1 = regno;
+	  else if (p2 == -1)
+	    p2 = regno;
+	}
+      else if (type != 'i'
+	  && pr.X_op == O_subtract
+	  && (pr1 = symbol_get_value_expression (pr.X_add_symbol))
+	  && pr1->X_op == O_register
+	  && pr1->X_add_number >= REG_P
+	  && pr1->X_add_number <= REG_P + 63
+	  && (pr2 = symbol_get_value_expression (pr.X_op_symbol))
+	  && pr2->X_op == O_register
+	  && pr2->X_add_number >= REG_P
+	  && pr2->X_add_number <= REG_P + 63)
+	{
+	  /* It's a range.  */
+	  int stop;
+
+	  regno = pr1->X_add_number - REG_P;
+	  stop = pr2->X_add_number - REG_P;
+	  if (regno >= stop)
 	    {
 	      as_bad (_("Bad register range"));
 	      ignore_rest_of_line ();
 	      return;
 	    }
-	  while (bit < stop)
-	    {
-	      bit <<= 1;
-	      mask |= bit;
-	      count++;
-	    }
-	  SKIP_WHITESPACE ();
+	  bits = ((bits << stop) << 1) - (bits << regno);
+	  count += stop - regno + 1;
+	}
+      else
+	{
+	  as_bad (_("Predicate register expected"));
+	  ignore_rest_of_line ();
+	  return;
 	}
+      if (mask & bits)
+	as_warn (_("Duplicate predicate register ignored"));
+      mask |= bits;
       if (*input_line_pointer != ',')
 	break;
       ++input_line_pointer;
@@ -7639,6 +7639,9 @@ ia64_parse_name (name, e, nextcharP)
 	}
     }
 
+  end = alloca (strlen (name) + 1);
+  strcpy (end, name);
+  name = ia64_canonicalize_symbol_name (end);
   if ((dr = hash_find (md.dynreg_hash, name)))
     {
       /* We've got ourselves the name of a rotating register set.
@@ -7658,9 +7661,20 @@ char *
 ia64_canonicalize_symbol_name (name)
      char *name;
 {
-  size_t len = strlen (name);
-  if (len > 1 && name[len - 1] == '#')
-    name[len - 1] = '\0';
+  size_t len = strlen (name), full = len;
+
+  while (len > 0 && name[len - 1] == '#')
+    --len;
+  if (len <= 0)
+    {
+      if (full > 0)
+	as_bad ("Standalone `#' is illegal");
+      else
+	as_bad ("Zero-length symbol is illegal");
+    }
+  else if (len < full - 1)
+    as_warn ("Redundant `#' suffix operators");
+  name[len] = '\0';
   return name;
 }
 
@@ -11229,6 +11243,7 @@ dot_alias (int section)
 
   input_line_pointer++;
   *end_name = 0;
+  ia64_canonicalize_symbol_name (name);
 
   /* We call demand_copy_C_string to check if alias string is valid.
      There should be a closing `"' and no `\0' in the string.  */
---
/home/jbeulich/src/binutils/mainline/2005-02-11/gas/testsuite/gas/ia64/ia64.exp	2005-02-02
08:33:18.000000000 +0100
+++ 2005-02-11/gas/testsuite/gas/ia64/ia64.exp	2005-02-11
12:29:05.257667344 +0100
@@ -67,6 +67,7 @@ if [istarget "ia64-*"] then {
     run_dump_test "bundling"
     run_list_test "label" ""
     run_list_test "last" ""
+    run_list_test "pound" "-al"
     run_list_test "proc" ""
     run_list_test "slot2" ""
     run_list_test "unwind-err" ""
---
/home/jbeulich/src/binutils/mainline/2005-02-11/gas/testsuite/gas/ia64/pound.l	1970-01-01
01:00:00.000000000 +0100
+++ 2005-02-11/gas/testsuite/gas/ia64/pound.l	2005-02-02
14:20:57.000000000 +0100
@@ -0,0 +1,58 @@
+.*: Assembler messages:
+.*:35: Warning: .* WAW .*
+#...
+.*:41: Error: symbol .esym. .* .efunction.
+.*:43: Error: section .\.extra. .* .esection.
+GAS LISTING .*
+#...
+[[:space:]]*[[:digit:]]+[[:space:]]+\.explicit
+[[:space:]]*[[:digit:]]+[[:space:]]+
+[[:space:]]*[[:digit:]]+[[:space:]]+\.global esym#
+[[:space:]]*[[:digit:]]+[[:space:]]+
+[[:space:]]*[[:digit:]]+[[:space:]]+\.section \.extra#, "a",
@progbits
+[[:space:]]*[[:digit:]]+[[:space:]]+
+[[:space:]]*[[:digit:]]+[[:space:]]+\.text
+[[:space:]]*[[:digit:]]+[[:space:]]+
+[[:space:]]*[[:digit:]]+[[:space:]]+	break		0
+[[:space:]]*[[:digit:]]+[[:space:]]+
+[[:space:]]*[[:digit:]]+[[:space:]]+\?*[[:space:]]+[[:xdigit:]]+[[:space:]]+\.proc
psym
+#...
+[[:space:]]*[[:digit:]]+[[:space:]]+psym:
+[[:space:]]*[[:digit:]]+[[:space:]]+	mov\.ret\.sptk	b7 = r0, tag#
+[[:space:]]*[[:digit:]]+[[:space:]]+	mov		r8 = 0
+[[:space:]]*[[:digit:]]+[[:space:]]+\[tag:\]	br\.ret\.sptk	rp
+[[:space:]]*[[:digit:]]+[[:space:]]+\?*[[:space:]]+[[:xdigit:]]+[[:space:]]+\.endp
psym
+#...
+[[:space:]]*[[:digit:]]+[[:space:]]+
+[[:space:]]*[[:digit:]]+[[:space:]]+\.proc esym#
+[[:space:]]*[[:digit:]]+[[:space:]]+\.entry entry#
+[[:space:]]*[[:digit:]]+[[:space:]]+esym:
+[[:space:]]*[[:digit:]]+[[:space:]]+\.unwentry
+[[:space:]]*[[:digit:]]+[[:space:]]+\.personality psym#
+[[:space:]]*[[:digit:]]+[[:space:]]+\.regstk 0, 8, 0, 8
+[[:space:]]*[[:digit:]]+[[:space:]]+\.rotp p#\[2\], p1#\[4\]
+[[:space:]]*[[:digit:]]+[[:space:]]+\.rotr r#\[2\], r1#\[4\]
+[[:space:]]*[[:digit:]]+[[:space:]]+\.reg\.val r#\[1\], 0
+[[:space:]]*[[:digit:]]+[[:space:]]+\.reg\.val r1#\[3\], 0
+[[:space:]]*[[:digit:]]+[[:space:]]+\(p1#\[1\]\) cmp\.eq	p\[0\] =
r\[1\], r1#\[1\]
+[[:space:]]*[[:digit:]]+[[:space:]]+\(p1#\[3\]\) cmp\.eq	p#\[1\]
= r#\[1\], r1#\[3\]
+[[:space:]]*[[:digit:]]+[[:space:]]+\.pred\.rel "mutex", p#\[0\],
p\[1\]
+[[:space:]]*[[:digit:]]+[[:space:]]+	nop		0
+[[:space:]]*[[:digit:]]+[[:space:]]+	;;
+[[:space:]]*[[:digit:]]+[[:space:]]+entry:
+[[:space:]]*[[:digit:]]+[[:space:]]+\?*[[:space:]]+61828446[[:space:]]+\(p\[0\]\)	mov		r8
= 1
+[[:space:]]*[[:digit:]]+[[:space:]]+00781509[[:space:]]*
+[[:space:]]*[[:digit:]]+[[:space:]]+95007000[[:space:]]*
+[[:space:]]*[[:digit:]]+[[:space:]]+00000400[[:space:]]*
+[[:space:]]*[[:digit:]]+[[:space:]]+\(p#\[1\]\)	mov		r8
= 0
+[[:space:]]*[[:digit:]]+[[:space:]]+	br\.ret\.sptk	rp
+[[:space:]]*[[:digit:]]+[[:space:]]+\.xdata4 \.extra#, -1
+[[:space:]]*[[:digit:]]+[[:space:]]+\?*[[:space:]]+11420400+[[:space:]]+\.endp
esym#
+[[:space:]]*[[:digit:]]+[[:space:]]+00648400[[:space:]]*
+[[:space:]]*[[:digit:]]+[[:space:]]+00004880[[:space:]]*
+[[:space:]]*[[:digit:]]+[[:space:]]+00008400[[:space:]]*
+#...
+[[:space:]]*[[:digit:]]+[[:space:]]+\.alias esym#, "efunction"
+[[:space:]]*[[:digit:]]+[[:space:]]+\.alias esym, "efunc"
+[[:space:]]*[[:digit:]]+[[:space:]]+\.secalias \.extra#, "esection"
+[[:space:]]*[[:digit:]]+[[:space:]]+\.secalias \.extra, "esec"
---
/home/jbeulich/src/binutils/mainline/2005-02-11/gas/testsuite/gas/ia64/pound.s	1970-01-01
01:00:00.000000000 +0100
+++ 2005-02-11/gas/testsuite/gas/ia64/pound.s	2005-02-02
13:23:59.000000000 +0100
@@ -0,0 +1,43 @@
+.explicit
+
+.global esym#
+
+.section .extra#, "a", @progbits
+
+.text
+
+	break		0
+
+.proc psym
+psym:
+	mov.ret.sptk	b7 = r0, tag#
+	mov		r8 = 0
+[tag:]	br.ret.sptk	rp
+.endp psym
+
+.proc esym#
+.entry entry#
+esym:
+.unwentry
+.personality psym#
+.regstk 0, 8, 0, 8
+.rotp p#[2], p1#[4]
+.rotr r#[2], r1#[4]
+.reg.val r#[1], 0
+.reg.val r1#[3], 0
+(p1#[1]) cmp.eq	p[0] = r[1], r1#[1]
+(p1#[3]) cmp.eq	p#[1] = r#[1], r1#[3]
+.pred.rel "mutex", p#[0], p[1]
+	nop		0
+	;;
+entry:
+(p[0])	mov		r8 = 1
+(p#[1])	mov		r8 = 0
+	br.ret.sptk	rp
+.xdata4 .extra#, -1
+.endp esym#
+
+.alias esym#, "efunction"
+.alias esym, "efunc"
+.secalias .extra#, "esection"
+.secalias .extra, "esec"

Attachment: binutils-mainline-ia64-pound.patch
Description: Text document


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]