PATCH: Add -munwind-check=[none|warning|error]

PATCH: Add -munwind-check=[none|warning|error]

[H. J. Lu]

Hi Jim,

Here is the updated patch to add -munwind-check=[none|warning|error].
I documented this new option.


H.J.
---
gas/

2005-02-02  H.J. Lu  <hongjiu.lu@intel.com>

	* NEWS: Mention "-munwind-check=[none|warning|error]".

	* config/tc-ia64.c (md): Add unwind_check.
	(in_procedure): When a directive isn't in procedure, warn if
	md.unwind_check is warning and return -1 if md.unwind_check
	isn't error.
	(in_prologue): Likewise.
	(in_body): Likewise.
	(md_parse_option): Handle "-munwind-check=[none|warning|error]".
	(md_show_usage): Add "-munwind-check=[none|warning|error]".
	(dot_endp): Abort if in_procedure returns -1.

	* doc/as.texinfo: Add "-munwind-check=[none|warning|error]".
	* doc/c-ia64.texi: Likewise.

gas/testcase

2005-02-02  H.J. Lu  <hongjiu.lu@intel.com>

	* gas/ia64/ia64.exp: Pass -munwind-check=error for unwind-err.

--- gas/NEWS.check	2004-12-10 14:20:32.000000000 -0800
+++ gas/NEWS	2005-02-09 17:16:14.885016326 -0800
@@ -1,5 +1,8 @@
 -*- text -*-
 
+* New command line option -munwind-check=[none|warning|error] for IA64
+  targets.
+
 * Port to MAXQ processor contributed by HCL Tech.
 
 * Added support for generating unwind tables for ARM ELF targets.
--- gas/config/tc-ia64.c.check	2005-02-03 12:00:26.000000000 -0800
+++ gas/config/tc-ia64.c	2005-02-09 16:53:15.468350820 -0800
@@ -231,6 +231,9 @@ static struct
       auto_align : 1,
       keep_pending_output : 1;
 
+    /* What to do when something is wrong with unwind directives.  */
+    enum { none = -1, warning = 0, error = 1 } unwind_check;
+
     /* Each bundle consists of up to three instructions.  We keep
        track of four most recent instructions so we can correctly set
        the end_of_insn_group for the last instruction in a bundle.  */
@@ -3074,6 +3077,16 @@ in_procedure (const char *directive)
   if (unwind.proc_start
       && (!unwind.saved_text_seg || strcmp (directive, "endp") == 0))
     return 1;
+  switch (md.unwind_check)
+    {
+    case warning:
+      as_warn (".%s outside of procedure", directive);
+    case none:
+      return -1;
+      break;
+    case error:
+      break;
+    }
   as_bad (".%s outside of procedure", directive);
   ignore_rest_of_line ();
   return 0;
@@ -3082,12 +3095,31 @@ in_procedure (const char *directive)
 static int
 in_prologue (const char *directive)
 {
-  if (in_procedure (directive))
+  switch (in_procedure (directive))
     {
+    case 1:
+      /* We are really in procedure.  */
       if (unwind.prologue)
 	return 1;
-      as_bad (".%s outside of prologue", directive);
-      ignore_rest_of_line ();
+      switch (md.unwind_check)
+	{
+	case warning:
+	  as_warn (".%s outside of prologue", directive);
+	case none:
+	  return -1;
+	  break;
+	case error:
+	  as_bad (".%s outside of prologue", directive);
+	  ignore_rest_of_line ();
+	  break;
+	}
+    case -1:
+      /* We aren't in procedure.  */
+      if (md.unwind_check != error)
+	return -1;
+      break;
+    default:
+      break;
     }
   return 0;
 }
@@ -3095,12 +3127,31 @@ in_prologue (const char *directive)
 static int
 in_body (const char *directive)
 {
-  if (in_procedure (directive))
+  switch (in_procedure (directive))
     {
+    case 1:
+      /* We are really in procedure.  */
       if (unwind.body)
 	return 1;
-      as_bad (".%s outside of body region", directive);
-      ignore_rest_of_line ();
+      switch (md.unwind_check)
+	{
+	case warning:
+	  as_warn (".%s outside of body region", directive);
+	case none:
+	  return -1;
+	  break;
+	case error:
+	  as_bad (".%s outside of body region", directive);
+	  ignore_rest_of_line ();
+	  break;
+	}
+    case -1:
+      /* We aren't in procedure.  */
+      if (md.unwind_check != error)
+	return -1;
+      break;
+    default:
+      break;
     }
   return 0;
 }
@@ -4315,8 +4366,18 @@ dot_endp (dummy)
   char *name, *p, c;
   symbolS *sym;
 
-  if (!in_procedure ("endp"))
-    return;
+  switch (in_procedure ("endp"))
+    {
+    case 1:
+      break;
+    case -1:
+      /* We can't ignore this error.  */
+      as_fatal (".endp can't be outside of procedure");
+      break;
+    default:
+      return;
+      break;
+    }
 
   if (unwind.saved_text_seg)
     {
@@ -6794,6 +6855,18 @@ md_parse_option (c, arg)
 	  md.flags |= EF_IA_64_BE;
 	  default_big_endian = 1;
 	}
+      else if (strncmp (arg, "unwind-check=", 13) == 0)
+	{
+	  arg += 13;
+	  if (strcmp (arg, "none") == 0)
+	    md.unwind_check = none;
+	  else if (strcmp (arg, "warning") == 0)
+	    md.unwind_check = warning;
+	  else if (strcmp (arg, "error") == 0)
+	    md.unwind_check = error;
+	  else
+	    return 0;
+	}
       else
 	return 0;
       break;
@@ -6897,6 +6970,8 @@ IA-64 options:\n\
 			  EF_IA_64_NOFUNCDESC_CONS_GP)\n\
   -milp32|-milp64|-mlp64|-mp64	select data model (default -mlp64)\n\
   -mle | -mbe		  select little- or big-endian byte order (default -mle)\n\
+  -munwind-check=[none|warning|error]\n\
+			  unwind directive check (default -munwind-check=warning)\n\
   -x | -xexplicit	  turn on dependency violation checking (default)\n\
   -xauto		  automagically remove dependency violations\n\
   -xdebug		  debug dependency violation checker\n"),
--- gas/doc/as.texinfo.check	2004-11-23 09:09:23.000000000 -0800
+++ gas/doc/as.texinfo	2005-02-09 17:17:26.952728318 -0800
@@ -315,6 +315,7 @@ gcc(1), ld(1), and the Info entries for 
    [@b{-mconstant-gp}|@b{-mauto-pic}]
    [@b{-milp32}|@b{-milp64}|@b{-mlp64}|@b{-mp64}]
    [@b{-mle}|@b{mbe}]
+   [@b{-munwind-check=none}|@b{-munwind-check=warning}|@b{-munwind-check=error}]
    [@b{-x}|@b{-xexplicit}] [@b{-xauto}] [@b{-xdebug}]
 @end ifset
 @ifset IP2K
--- gas/doc/c-ia64.texi.check	2003-10-27 11:00:37.000000000 -0800
+++ gas/doc/c-ia64.texi	2005-02-09 17:21:36.747535035 -0800
@@ -65,6 +65,16 @@ These options select the byte order.  Th
 byte order (default) and @code{-mbe} selects big-endian byte order.  Note that
 IA-64 machine code always uses little-endian byte order.
 
+@item -munwind-check=none
+@item -munwind-check=warning
+@item -munwind-check=error
+These options control if asssembler will do when a unwind directive
+check fails. @code{-munwind-check=none} will make asssembler not to do
+any check. @code{-munwind-check=warning} will make asssembler to issue
+a warning when a unwind directive check fails, which is the default.
+@code{-munwind-check=error} will make asssembler to treat a unwind
+directive check failure as an error.
+
 @item -x
 @item -xexplicit
 These options turn on dependency violation checking.  This checking is turned on by
--- gas/testsuite/gas/ia64/ia64.exp.check	2005-02-03 12:00:27.000000000 -0800
+++ gas/testsuite/gas/ia64/ia64.exp	2005-02-09 16:53:15.469350691 -0800
@@ -69,5 +69,5 @@ if [istarget "ia64-*"] then {
     run_list_test "last" ""
     run_list_test "proc" ""
     run_list_test "slot2" ""
-    run_list_test "unwind-err" ""
+    run_list_test "unwind-err" "-munwind-check=error"
 }

Re: PATCH: Add -munwind-check=[none|warning|error]

[James E Wilson]

On Thu, 2005-02-10 at 09:29, H. J. Lu wrote:
> Here is the updated patch to add -munwind-check=[none|warning|error].
> I documented this new option.

My feeling on this issue in general that we should be emitting a
diagnostic here always.  This new code is finding latent bugs in the
unwind info.  This is a correctness issue.  If we let the latent bugs
through, then they will only be found if an exception is generated and
the stack unwound, at which point it is far too late to recover from the
mistake.  This makes it very important for the assembler to give a
diagnostic when we find a problem.  I can live with this being a warning
for now, to avoid compatibility problems, but it really is important
that people fix their code if they want it to work correctly.

You seem to be going to a lot of unnecessary trouble here.  Why not just
change the as_bad calls to unwind_diagnostic, which then calls as_warn
or as_bad as appropriate?  That would seem to be a lot simpler, and
avoid a lot of duplicated strings.

As far as I can tell, most of this complexity is only to ensure that we
always get an error for a .endp outside a procedure.  That is easy
enough to continue doing just by forcing unwind_check to error before
the unwind_diagnostic call here and then restoring the original value
after the call.

If there is some reason why we need to do it this way, then in_prologue,
in_body, and in_procedure needs comments explaining the meaning of their
return values, as they are no longer obvious true/false return values. 
It would also be nice to document why we need to do things this way.

In two places you have

> +    case -1:
> +      /* We aren't in procedure.  */
> +      if (md.unwind_check != error)
> +	return -1;

But since in_procedure can return -1 only if unwind_check != error, this
appears to be a redundant test.

> +    case -1:
> +      /* We can't ignore this error.  */
> +      as_fatal (".endp can't be outside of procedure");

I believe there is a duplicate diagnostic emitted in this case, when
unwind_check == warning.  First we will get a warning that endp is
outside a procedure, then we get this error.  If we can't ignore this
error, then we shouldn't have emitted the warning in the first place.

As I mentioned before, I think a better way to handle this case is to
force unwind_check = error to ensure we get an error.  With this change,
I think all of the -1 return values are no longer needed which
simplifies a lot of code.

You aren't initializing md.unwind_check.  It appears that you are
relying on a trick, that enum warning = 0, and hence the fact that we
zero this somewhere forces the default to be a warning.  However,
nowhere have you documented the fact that enum warning must be zero in
order for this patch to work.  Instead of relying on an undocumented
trick here, I would just suggesting adding a line to initialize it, e.g.
putting
  md.unwind_check = warning;
in ia64_init.

It would be nice if there was a comment saying that the default can be
changed from warning to error at some point in the future as a
reminder.  Presumably this would be in 2.17 or later.

The phrasing in the c-ia64.texi file is a bit awkward.  I would suggest
something like:

These options control what the assembler will do when performing
consistency checks on unwind directives.  @code{-munwind-check=none}
will disable the checks.  @code{-munwind-check=warning} will make the
assembler issue a warning when an unwind directive check fails.  This is
the default.  @code{-munwind-check=error} will make the assembler issue
an error when an unwind directive check fails.
-- 
Jim Wilson, GNU Tools Support, http://www.SpecifixInc.com

Re: PATCH: Add -munwind-check=[none|warning|error]

[H. J. Lu]

On Thu, Feb 10, 2005 at 06:30:17PM -0800, James E Wilson wrote:
> On Thu, 2005-02-10 at 09:29, H. J. Lu wrote:
> > Here is the updated patch to add -munwind-check=[none|warning|error].
> > I documented this new option.
> 
> My feeling on this issue in general that we should be emitting a
> diagnostic here always.  This new code is finding latent bugs in the
> unwind info.  This is a correctness issue.  If we let the latent bugs
> through, then they will only be found if an exception is generated and
> the stack unwound, at which point it is far too late to recover from the
> mistake.  This makes it very important for the assembler to give a
> diagnostic when we find a problem.  I can live with this being a warning
> for now, to avoid compatibility problems, but it really is important
> that people fix their code if they want it to work correctly.
> 

Here is the new patch. I removed the -munwind-check=none option. I also
included the patch for the .endp check. IAS does ignore the name
after .endp. But checking it is a good idea. People will get a
warning unless they fix the code.


H.J.
----
gas/

2005-02-11  H.J. Lu  <hongjiu.lu@intel.com>

	* NEWS: Mention "-munwind-check=[warning|error]".

	* config/tc-ia64.c (md): Add unwind_check.
	(unwind_diagnostic): New.
	(in_procedure): Call unwind_diagnostic when a directive isn't
	in procedure.
	(in_prologue): Call unwind_diagnostic when a directive isn't in
	prologue.
	(in_body): Call unwind_diagnostic when a directive isn't in
	body region.
	(dot_endp): Set md.unwind_check to error before calling
	in_procedure and restore it after. When the name is missing or
	couldn't be found, use the one from the last .proc if
	md.unwind_check isn't error. Warn if md.unwind_check is
	warning.
	(md_parse_option): Handle "-munwind-check=[warning|error]".
	(md_show_usage): Add "-munwind-check=[warning|error]".
	(ia64_init): Set md.unwind_check to warning.

	* doc/as.texinfo: Add "-munwind-check=[none|warning|error]".
	* doc/c-ia64.texi: Likewise.

gas/testcase

2005-02-11  H.J. Lu  <hongjiu.lu@intel.com>

	* gas/ia64/ia64.exp: Pass -munwind-check=error for unwind-err
	and proc.

--- gas/NEWS.check	2004-12-10 14:20:32.000000000 -0800
+++ gas/NEWS	2005-02-11 09:39:54.396270244 -0800
@@ -1,5 +1,8 @@
 -*- text -*-
 
+* New command line option -munwind-check=[warning|error] for IA64
+  targets.
+
 * Port to MAXQ processor contributed by HCL Tech.
 
 * Added support for generating unwind tables for ARM ELF targets.
--- gas/config/tc-ia64.c.check	2005-02-11 09:20:08.000000000 -0800
+++ gas/config/tc-ia64.c	2005-02-11 10:40:59.035659535 -0800
@@ -231,6 +231,13 @@ static struct
       auto_align : 1,
       keep_pending_output : 1;
 
+    /* What to do when something is wrong with unwind directives.  */
+    enum
+      {
+	unwind_check_warning,
+	unwind_check_error
+      } unwind_check;
+
     /* Each bundle consists of up to three instructions.  We keep
        track of four most recent instructions so we can correctly set
        the end_of_insn_group for the last instruction in a bundle.  */
@@ -3048,14 +3055,25 @@ dot_special_section (which)
   set_section ((char *) special_section_name[which]);
 }
 
+static void
+unwind_diagnostic (const char * region, const char *directive)
+{
+  if (md.unwind_check == unwind_check_warning)
+    as_warn (".%s outside of %s", directive, region);
+  else
+    {
+      as_bad (".%s outside of %s", directive, region);
+      ignore_rest_of_line ();
+    }
+}
+
 static int
 in_procedure (const char *directive)
 {
   if (unwind.proc_start
       && (!unwind.saved_text_seg || strcmp (directive, "endp") == 0))
     return 1;
-  as_bad (".%s outside of procedure", directive);
-  ignore_rest_of_line ();
+  unwind_diagnostic ("procedure", directive);
   return 0;
 }
 
@@ -3064,10 +3082,10 @@ in_prologue (const char *directive)
 {
   if (in_procedure (directive))
     {
+      /* We are in a procedure. Check if we are in a prologue.  */
       if (unwind.prologue)
 	return 1;
-      as_bad (".%s outside of prologue", directive);
-      ignore_rest_of_line ();
+      unwind_diagnostic ("prologue", directive);
     }
   return 0;
 }
@@ -3077,10 +3095,10 @@ in_body (const char *directive)
 {
   if (in_procedure (directive))
     {
+      /* We are in a procedure. Check if we are in a body.  */
       if (unwind.body)
 	return 1;
-      as_bad (".%s outside of body region", directive);
-      ignore_rest_of_line ();
+      unwind_diagnostic ("body region", directive);
     }
   return 0;
 }
@@ -4292,11 +4310,14 @@ dot_endp (dummy)
   long where;
   segT saved_seg;
   subsegT saved_subseg;
-  char *name, *p, c;
+  char *name, *default_name, *p, c;
   symbolS *sym;
+  int unwind_check = md.unwind_check;
 
+  md.unwind_check = unwind_check_error;
   if (!in_procedure ("endp"))
     return;
+  md.unwind_check = unwind_check;
 
   if (unwind.saved_text_seg)
     {
@@ -4368,6 +4389,11 @@ dot_endp (dummy)
 
   subseg_set (saved_seg, saved_subseg);
 
+  if (unwind.proc_start)
+    default_name = (char *) S_GET_NAME (unwind.proc_start);
+  else
+    default_name = NULL;
+
   /* Parse names of main and alternate entry points and set symbol sizes.  */
   while (1)
     {
@@ -4376,10 +4402,35 @@ dot_endp (dummy)
       c = get_symbol_end ();
       p = input_line_pointer;
       if (!*name)
-	as_bad ("Empty argument of .endp");
-      else
+	{
+	  if (md.unwind_check == unwind_check_warning)
+	    {
+	      if (default_name)
+		{
+		  as_warn ("Empty argument of .endp. Use the default name `%s'",
+			   default_name);
+		  name = default_name;
+		}
+	      else
+		as_warn ("Empty argument of .endp");
+	    }
+	  else
+	    as_bad ("Empty argument of .endp");
+	}
+      if (*name)
 	{
 	  sym = symbol_find (name);
+	  if (!sym
+	      && md.unwind_check == unwind_check_warning
+	      && default_name
+	      && default_name != name)
+	    {
+	      /* We have a bad name. Try the default one if needed.  */
+	      as_warn ("`%s' was not defined within procedure. Use the default name `%s'",
+		       name, default_name);
+	      name = default_name;
+	      sym = symbol_find (name);
+	    }
 	  if (!sym || !S_IS_DEFINED (sym))
 	    as_bad ("`%s' was not defined within procedure", name);
 	  else if (unwind.proc_start
@@ -6689,6 +6740,16 @@ md_parse_option (c, arg)
 	  md.flags |= EF_IA_64_BE;
 	  default_big_endian = 1;
 	}
+      else if (strncmp (arg, "unwind-check=", 13) == 0)
+	{
+	  arg += 13;
+	  if (strcmp (arg, "warning") == 0)
+	    md.unwind_check = unwind_check_warning;
+	  else if (strcmp (arg, "error") == 0)
+	    md.unwind_check = unwind_check_error;
+	  else
+	    return 0;
+	}
       else
 	return 0;
       break;
@@ -6792,6 +6853,8 @@ IA-64 options:\n\
 			  EF_IA_64_NOFUNCDESC_CONS_GP)\n\
   -milp32|-milp64|-mlp64|-mp64	select data model (default -mlp64)\n\
   -mle | -mbe		  select little- or big-endian byte order (default -mle)\n\
+  -munwind-check=[warning|error]\n\
+			  unwind directive check (default -munwind-check=warning)\n\
   -x | -xexplicit	  turn on dependency violation checking (default)\n\
   -xauto		  automagically remove dependency violations\n\
   -xdebug		  debug dependency violation checker\n"),
@@ -7126,9 +7189,10 @@ md_begin ()
   md.entry_labels = NULL;
 }
 
-/* Set the elf type to 64 bit ABI by default.  Cannot do this in md_begin
-   because that is called after md_parse_option which is where we do the
-   dynamic changing of md.flags based on -mlp64 or -milp32.  Also, set the
+/* Set the elf type to 64 bit ABI and unwind directive checking to
+   warning by default.  Cannot do this in md_begin because that is
+   called after md_parse_option which is where we do the dynamic
+   changing of md.flags based on -mlp64 or -milp32.  Also, set the
    default endianness.  */
 
 void
@@ -7137,6 +7201,8 @@ ia64_init (argc, argv)
      char **argv ATTRIBUTE_UNUSED;
 {
   md.flags = MD_FLAGS_DEFAULT;
+  /* FIXME: We should change it to unwind_check_error someday.  */
+  md.unwind_check = unwind_check_warning;
 }
 
 /* Return a string for the target object file format.  */
--- gas/doc/as.texinfo.check	2005-02-11 09:20:16.000000000 -0800
+++ gas/doc/as.texinfo	2005-02-11 09:40:10.764149990 -0800
@@ -315,6 +315,7 @@ gcc(1), ld(1), and the Info entries for 
    [@b{-mconstant-gp}|@b{-mauto-pic}]
    [@b{-milp32}|@b{-milp64}|@b{-mlp64}|@b{-mp64}]
    [@b{-mle}|@b{mbe}]
+   [@b{-munwind-check=warning}|@b{-munwind-check=error}]
    [@b{-x}|@b{-xexplicit}] [@b{-xauto}] [@b{-xdebug}]
 @end ifset
 @ifset IP2K
--- gas/doc/c-ia64.texi.check	2005-02-11 09:20:16.000000000 -0800
+++ gas/doc/c-ia64.texi	2005-02-11 09:41:16.774599159 -0800
@@ -65,6 +65,14 @@ These options select the byte order.  Th
 byte order (default) and @code{-mbe} selects big-endian byte order.  Note that
 IA-64 machine code always uses little-endian byte order.
 
+@item -munwind-check=warning
+@item -munwind-check=error
+These options control what the assembler will do when performing
+consistency checks on unwind directives.  @code{-munwind-check=warning}
+will make the assembler issue a warning when an unwind directive check
+fails.  This is the default.  @code{-munwind-check=error} will make the
+assembler issue an error when an unwind directive check fails.
+
 @item -x
 @item -xexplicit
 These options turn on dependency violation checking.  This checking is turned on by
--- gas/testsuite/gas/ia64/ia64.exp.check	2005-02-03 12:00:27.000000000 -0800
+++ gas/testsuite/gas/ia64/ia64.exp	2005-02-11 10:02:18.138205233 -0800
@@ -67,7 +67,7 @@ if [istarget "ia64-*"] then {
     run_dump_test "bundling"
     run_list_test "label" ""
     run_list_test "last" ""
-    run_list_test "proc" ""
+    run_list_test "proc" "-munwind-check=error"
     run_list_test "slot2" ""
-    run_list_test "unwind-err" ""
+    run_list_test "unwind-err" "-munwind-check=error"
 }

Re: PATCH: Add -munwind-check=[none|warning|error]

[H. J. Lu]

On Fri, Feb 11, 2005 at 10:45:11AM -0800, H. J. Lu wrote:
> On Thu, Feb 10, 2005 at 06:30:17PM -0800, James E Wilson wrote:
> > On Thu, 2005-02-10 at 09:29, H. J. Lu wrote:
> > > Here is the updated patch to add -munwind-check=[none|warning|error].
> > > I documented this new option.
> > 
> > My feeling on this issue in general that we should be emitting a
> > diagnostic here always.  This new code is finding latent bugs in the
> > unwind info.  This is a correctness issue.  If we let the latent bugs
> > through, then they will only be found if an exception is generated and
> > the stack unwound, at which point it is far too late to recover from the
> > mistake.  This makes it very important for the assembler to give a
> > diagnostic when we find a problem.  I can live with this being a warning
> > for now, to avoid compatibility problems, but it really is important
> > that people fix their code if they want it to work correctly.
> > 
> 
> Here is the new patch. I removed the -munwind-check=none option. I also
> included the patch for the .endp check. IAS does ignore the name
> after .endp. But checking it is a good idea. People will get a
> warning unless they fix the code.
> 
> 

Here is a small update. I modified the comments for ia64_init. I am
working on a patch which needs to set a new default option in md.


H.J.
----
gas/

2005-02-11  H.J. Lu  <hongjiu.lu@intel.com>

	* NEWS: Mention "-munwind-check=[warning|error]".

	* config/tc-ia64.c (md): Add unwind_check.
	(unwind_diagnostic): New.
	(in_procedure): Call unwind_diagnostic when a directive isn't
	in procedure.
	(in_prologue): Call unwind_diagnostic when a directive isn't in
	prologue.
	(in_body): Call unwind_diagnostic when a directive isn't in
	body region.
	(dot_endp): Set md.unwind_check to error before calling
	in_procedure and restore it after. When the name is missing or
	couldn't be found, use the one from the last .proc if
	md.unwind_check isn't error. Warn if md.unwind_check is
	warning.
	(md_parse_option): Handle "-munwind-check=[warning|error]".
	(md_show_usage): Add "-munwind-check=[warning|error]".
	(ia64_init): Set md.unwind_check to warning.

	* doc/as.texinfo: Add "-munwind-check=[none|warning|error]".
	* doc/c-ia64.texi: Likewise.

gas/testcase

2005-02-11  H.J. Lu  <hongjiu.lu@intel.com>

	* gas/ia64/ia64.exp: Pass -munwind-check=error for unwind-err
	and proc.

--- gas/NEWS.check	2004-12-10 14:20:32.000000000 -0800
+++ gas/NEWS	2005-02-11 10:51:56.872522083 -0800
@@ -1,5 +1,8 @@
 -*- text -*-
 
+* New command line option -munwind-check=[warning|error] for IA64
+  targets.
+
 * Port to MAXQ processor contributed by HCL Tech.
 
 * Added support for generating unwind tables for ARM ELF targets.
--- gas/config/tc-ia64.c.check	2005-02-11 09:20:08.000000000 -0800
+++ gas/config/tc-ia64.c	2005-02-11 10:54:33.968190713 -0800
@@ -231,6 +231,13 @@ static struct
       auto_align : 1,
       keep_pending_output : 1;
 
+    /* What to do when something is wrong with unwind directives.  */
+    enum
+      {
+	unwind_check_warning,
+	unwind_check_error
+      } unwind_check;
+
     /* Each bundle consists of up to three instructions.  We keep
        track of four most recent instructions so we can correctly set
        the end_of_insn_group for the last instruction in a bundle.  */
@@ -3048,14 +3055,25 @@ dot_special_section (which)
   set_section ((char *) special_section_name[which]);
 }
 
+static void
+unwind_diagnostic (const char * region, const char *directive)
+{
+  if (md.unwind_check == unwind_check_warning)
+    as_warn (".%s outside of %s", directive, region);
+  else
+    {
+      as_bad (".%s outside of %s", directive, region);
+      ignore_rest_of_line ();
+    }
+}
+
 static int
 in_procedure (const char *directive)
 {
   if (unwind.proc_start
       && (!unwind.saved_text_seg || strcmp (directive, "endp") == 0))
     return 1;
-  as_bad (".%s outside of procedure", directive);
-  ignore_rest_of_line ();
+  unwind_diagnostic ("procedure", directive);
   return 0;
 }
 
@@ -3064,10 +3082,10 @@ in_prologue (const char *directive)
 {
   if (in_procedure (directive))
     {
+      /* We are in a procedure. Check if we are in a prologue.  */
       if (unwind.prologue)
 	return 1;
-      as_bad (".%s outside of prologue", directive);
-      ignore_rest_of_line ();
+      unwind_diagnostic ("prologue", directive);
     }
   return 0;
 }
@@ -3077,10 +3095,10 @@ in_body (const char *directive)
 {
   if (in_procedure (directive))
     {
+      /* We are in a procedure. Check if we are in a body.  */
       if (unwind.body)
 	return 1;
-      as_bad (".%s outside of body region", directive);
-      ignore_rest_of_line ();
+      unwind_diagnostic ("body region", directive);
     }
   return 0;
 }
@@ -4292,11 +4310,14 @@ dot_endp (dummy)
   long where;
   segT saved_seg;
   subsegT saved_subseg;
-  char *name, *p, c;
+  char *name, *default_name, *p, c;
   symbolS *sym;
+  int unwind_check = md.unwind_check;
 
+  md.unwind_check = unwind_check_error;
   if (!in_procedure ("endp"))
     return;
+  md.unwind_check = unwind_check;
 
   if (unwind.saved_text_seg)
     {
@@ -4368,6 +4389,11 @@ dot_endp (dummy)
 
   subseg_set (saved_seg, saved_subseg);
 
+  if (unwind.proc_start)
+    default_name = (char *) S_GET_NAME (unwind.proc_start);
+  else
+    default_name = NULL;
+
   /* Parse names of main and alternate entry points and set symbol sizes.  */
   while (1)
     {
@@ -4376,10 +4402,35 @@ dot_endp (dummy)
       c = get_symbol_end ();
       p = input_line_pointer;
       if (!*name)
-	as_bad ("Empty argument of .endp");
-      else
+	{
+	  if (md.unwind_check == unwind_check_warning)
+	    {
+	      if (default_name)
+		{
+		  as_warn ("Empty argument of .endp. Use the default name `%s'",
+			   default_name);
+		  name = default_name;
+		}
+	      else
+		as_warn ("Empty argument of .endp");
+	    }
+	  else
+	    as_bad ("Empty argument of .endp");
+	}
+      if (*name)
 	{
 	  sym = symbol_find (name);
+	  if (!sym
+	      && md.unwind_check == unwind_check_warning
+	      && default_name
+	      && default_name != name)
+	    {
+	      /* We have a bad name. Try the default one if needed.  */
+	      as_warn ("`%s' was not defined within procedure. Use the default name `%s'",
+		       name, default_name);
+	      name = default_name;
+	      sym = symbol_find (name);
+	    }
 	  if (!sym || !S_IS_DEFINED (sym))
 	    as_bad ("`%s' was not defined within procedure", name);
 	  else if (unwind.proc_start
@@ -6689,6 +6740,16 @@ md_parse_option (c, arg)
 	  md.flags |= EF_IA_64_BE;
 	  default_big_endian = 1;
 	}
+      else if (strncmp (arg, "unwind-check=", 13) == 0)
+	{
+	  arg += 13;
+	  if (strcmp (arg, "warning") == 0)
+	    md.unwind_check = unwind_check_warning;
+	  else if (strcmp (arg, "error") == 0)
+	    md.unwind_check = unwind_check_error;
+	  else
+	    return 0;
+	}
       else
 	return 0;
       break;
@@ -6792,6 +6853,8 @@ IA-64 options:\n\
 			  EF_IA_64_NOFUNCDESC_CONS_GP)\n\
   -milp32|-milp64|-mlp64|-mp64	select data model (default -mlp64)\n\
   -mle | -mbe		  select little- or big-endian byte order (default -mle)\n\
+  -munwind-check=[warning|error]\n\
+			  unwind directive check (default -munwind-check=warning)\n\
   -x | -xexplicit	  turn on dependency violation checking (default)\n\
   -xauto		  automagically remove dependency violations\n\
   -xdebug		  debug dependency violation checker\n"),
@@ -7126,10 +7189,9 @@ md_begin ()
   md.entry_labels = NULL;
 }
 
-/* Set the elf type to 64 bit ABI by default.  Cannot do this in md_begin
-   because that is called after md_parse_option which is where we do the
-   dynamic changing of md.flags based on -mlp64 or -milp32.  Also, set the
-   default endianness.  */
+/* Set the default options in md.  Cannot do this in md_begin because
+   that is called after md_parse_option which is where we set the
+   options in md based on command line options.  */
 
 void
 ia64_init (argc, argv)
@@ -7137,6 +7199,8 @@ ia64_init (argc, argv)
      char **argv ATTRIBUTE_UNUSED;
 {
   md.flags = MD_FLAGS_DEFAULT;
+  /* FIXME: We should change it to unwind_check_error someday.  */
+  md.unwind_check = unwind_check_warning;
 }
 
 /* Return a string for the target object file format.  */
--- gas/doc/as.texinfo.check	2005-02-11 09:20:16.000000000 -0800
+++ gas/doc/as.texinfo	2005-02-11 10:51:56.883520660 -0800
@@ -315,6 +315,7 @@ gcc(1), ld(1), and the Info entries for 
    [@b{-mconstant-gp}|@b{-mauto-pic}]
    [@b{-milp32}|@b{-milp64}|@b{-mlp64}|@b{-mp64}]
    [@b{-mle}|@b{mbe}]
+   [@b{-munwind-check=warning}|@b{-munwind-check=error}]
    [@b{-x}|@b{-xexplicit}] [@b{-xauto}] [@b{-xdebug}]
 @end ifset
 @ifset IP2K
--- gas/doc/c-ia64.texi.check	2005-02-11 09:20:16.000000000 -0800
+++ gas/doc/c-ia64.texi	2005-02-11 10:51:56.883520660 -0800
@@ -65,6 +65,14 @@ These options select the byte order.  Th
 byte order (default) and @code{-mbe} selects big-endian byte order.  Note that
 IA-64 machine code always uses little-endian byte order.
 
+@item -munwind-check=warning
+@item -munwind-check=error
+These options control what the assembler will do when performing
+consistency checks on unwind directives.  @code{-munwind-check=warning}
+will make the assembler issue a warning when an unwind directive check
+fails.  This is the default.  @code{-munwind-check=error} will make the
+assembler issue an error when an unwind directive check fails.
+
 @item -x
 @item -xexplicit
 These options turn on dependency violation checking.  This checking is turned on by
--- gas/testsuite/gas/ia64/ia64.exp.check	2005-02-03 12:00:27.000000000 -0800
+++ gas/testsuite/gas/ia64/ia64.exp	2005-02-11 10:51:56.884520530 -0800
@@ -67,7 +67,7 @@ if [istarget "ia64-*"] then {
     run_dump_test "bundling"
     run_list_test "label" ""
     run_list_test "last" ""
-    run_list_test "proc" ""
+    run_list_test "proc" "-munwind-check=error"
     run_list_test "slot2" ""
-    run_list_test "unwind-err" ""
+    run_list_test "unwind-err" "-munwind-check=error"
 }

Re: PATCH: Add -munwind-check=[none|warning|error]

[James E Wilson]

On Fri, 2005-02-11 at 11:00, H. J. Lu wrote:
> > Here is the new patch. I removed the -munwind-check=none option. I also
> > included the patch for the .endp check. IAS does ignore the name
> > after .endp. But checking it is a good idea. People will get a
> > warning unless they fix the code.

In case I wasn't clear, I am not opposed to having the
-munwind-check=none option, just to the amount of ugly code that was
used to implement it in the previous patch.  I think it would be foolish
to disable these checks now that we have them, but I can see that there
might be unusual cases where this might be useful.

About the .endp patch, I needed to take a closer look at that one as it
confused me.  Jan's comment said to change default name from NULL to ""
to avoid a segfault, you thanked him for the bug report, but you didn't
change this.  And since I apparently didn't receive either your original
patch or Jan's reply to it, it wasn't clear to me whether the problem
was still broken or had been fixed a different way.

I now suspect it was fixed a different way by adding the "&&
default_name" line, as I don't see a problem.

> 	* NEWS: Mention "-munwind-check=[warning|error]".
> 	* config/tc-ia64.c (md): Add unwind_check.
> 	(unwind_diagnostic): New.
>	...

OK.
-- 
Jim Wilson, GNU Tools Support, http://www.SpecifixInc.com

Re: PATCH: Add -munwind-check=[none|warning|error]

[H. J. Lu]

On Fri, Feb 11, 2005 at 10:45:11AM -0800, H. J. Lu wrote:
> On Thu, Feb 10, 2005 at 06:30:17PM -0800, James E Wilson wrote:
> > On Thu, 2005-02-10 at 09:29, H. J. Lu wrote:
> > > Here is the updated patch to add -munwind-check=[none|warning|error].
> > > I documented this new option.
> > 
> > My feeling on this issue in general that we should be emitting a
> > diagnostic here always.  This new code is finding latent bugs in the
> > unwind info.  This is a correctness issue.  If we let the latent bugs
> > through, then they will only be found if an exception is generated and
> > the stack unwound, at which point it is far too late to recover from the
> > mistake.  This makes it very important for the assembler to give a
> > diagnostic when we find a problem.  I can live with this being a warning
> > for now, to avoid compatibility problems, but it really is important
> > that people fix their code if they want it to work correctly.
> > 
> 
> Here is the new patch. I removed the -munwind-check=none option. I also
> included the patch for the .endp check. IAS does ignore the name
> after .endp. But checking it is a good idea. People will get a
> warning unless they fix the code.
> 
> 

It turned out that I need to return non-0 for warnings to work. Also
I need to return -1 to indicate that a warning has been issued so
that a second one isn't needed.


H.J.
------
2005-02-11  H.J. Lu  <hongjiu.lu@intel.com>

	* config/tc-ia64.c (unwind_diagnostic): Return -1 for warning
	and 0 for error.
	(in_procedure): Return -1 for warning.
	(in_prologue): Likewise.
	(in_body): Likewise.

--- gas/config/tc-ia64.c.warn	2005-02-11 13:21:20.056751130 -0800
+++ gas/config/tc-ia64.c	2005-02-11 15:08:03.424884229 -0800
@@ -3063,50 +3063,80 @@ dot_special_section (which)
   set_section ((char *) special_section_name[which]);
 }
 
-static void
+/* Return -1 for warning and 0 for error.  */
+
+static int
 unwind_diagnostic (const char * region, const char *directive)
 {
   if (md.unwind_check == unwind_check_warning)
-    as_warn (".%s outside of %s", directive, region);
+    {
+      as_warn (".%s outside of %s", directive, region);
+      return -1;
+    }
   else
     {
       as_bad (".%s outside of %s", directive, region);
       ignore_rest_of_line ();
+      return 0;
     }
 }
 
+/* Return 1 if a directive is in a procedure, -1 if a diretive isn't in
+   a procedure but the unwind diretive check is set to warning, 0 if
+   a diretive isn't in a procedure and the unwind diretive check is set
+   to error.  */
+
 static int
 in_procedure (const char *directive)
 {
   if (unwind.proc_start
       && (!unwind.saved_text_seg || strcmp (directive, "endp") == 0))
     return 1;
-  unwind_diagnostic ("procedure", directive);
-  return 0;
+  return unwind_diagnostic ("procedure", directive);
 }
 
+/* Return 1 if a directive is in a prologue, -1 if a diretive isn't in
+   a prologue but the unwind diretive check is set to warning, 0 if
+   a diretive isn't in a prologue and the unwind diretive check is set
+   to error.  */
+
 static int
 in_prologue (const char *directive)
 {
-  if (in_procedure (directive))
+  int in = in_procedure (directive);
+  if (in)
     {
       /* We are in a procedure. Check if we are in a prologue.  */
       if (unwind.prologue)
 	return 1;
-      unwind_diagnostic ("prologue", directive);
+      /* We only want to issue one message.  */
+      if (in == 1)
+	return unwind_diagnostic ("prologue", directive);
+      else
+	return -1;
     }
   return 0;
 }
 
+/* Return 1 if a directive is in a body, -1 if a diretive isn't in
+   a body but the unwind diretive check is set to warning, 0 if
+   a diretive isn't in a body and the unwind diretive check is set
+   to error.  */
+
 static int
 in_body (const char *directive)
 {
-  if (in_procedure (directive))
+  int in = in_procedure (directive);
+  if (in)
     {
       /* We are in a procedure. Check if we are in a body.  */
       if (unwind.body)
 	return 1;
-      unwind_diagnostic ("body region", directive);
+      /* We only want to issue one message.  */
+      if (in == 1)
+	return unwind_diagnostic ("body region", directive);
+      else
+	return -1;
     }
   return 0;
 }

Re: PATCH: Add -munwind-check=[none|warning|error]

[James E Wilson]

On Fri, 2005-02-11 at 15:14, H. J. Lu wrote:
> 	* config/tc-ia64.c (unwind_diagnostic): Return -1 for warning
> 	and 0 for error.
> 	(in_procedure): Return -1 for warning.
> 	(in_prologue): Likewise.
> 	(in_body): Likewise.

OK.

There are about 12 places where you need to replace "diretive" with
"directive".
-- 
Jim Wilson, GNU Tools Support, http://www.SpecifixInc.com