obj_coff_loc Internal Error

[Tracy Kuhrt]

Nick,


> Hi Tracy,
>
> > Following is a patch that I think solves the problems.
>
> First of all - do you have a binutils copyright assignment on file
> with the FSF ?  Without such an assignment we cannot accept your
> patch.  (I know that the patch is small, but I consider it to be
> non-trivial and not-obvious, so we do need the assignment).

I am working on this.  I have yet to receive the form from FSF.

>
>
> > +  /* Skip the comma if it exists; otherwise, give warning.  */
> > +  if (*input_line_pointer == ',')
> > +    input_line_pointer++;
> > +  else
> > +    as_warn (_("missing comma assumed."));
>
> Secondly I am not sure that it is correct to issue a warning if the
> comma is missing.  The code before your patch parsed the expression
> without requiring an line number and I feel that we should continue to
> support that behaviour unless there is a good reason not too.

We can remove the warning, but I thought it strange for the directive to
accept two arguments that did not require a comma.

>
>
> > NOTE:  I have moved the add_lineno before the listing code.  I am
> > not sure if this is correct.
>
> This does look wrong, or at least different.  The listing code does
> change the variable 'lineno' which is used by the call to add_lineno()
> so the behaviour will change.  Why did you move the call ?  Were you
> detecting incorrect line numbers in your output ?

I could not find any documentation on this directive except for the
comment block before the directive.

/* .loc is essentially the same as .ln; parse it for assembler
   compatibility.  */

I made an assumption that if it were like .ln, then we would want it to
perform like .ln.  Maybe "essentially" means that it actually acts
differently (?)

>
>
> Finally, since you are changing the behaviour of the .loc directive, it
> would be great if you could add some documentation about it to the
> assembler doc file (gas/doc/as.texinfo).  This is optional, but nice.

I do not see any documentation for this directive in gas/doc/as.texinfo.
I wanted to make sure I understood the directive (and was suggesting the
correct patch) before adding any documentation.

Tracy