Re: ALIGN directive showing different behavior than documented

[Sandra Loosemore <sandra at codesourcery dot com>]

On 07/31/2015 03:28 PM, Sandra Loosemore wrote:
> Alan Modra wrote:
>
> > Right, lhs.value is evaluated from fold_name case NAME here:
> >       else if (tree->name.name[0] == '.' && tree->name.name[1] == 0)
> >     new_rel_from_abs (expld.dot);
> >
> > So the difference is that the unary operator is missing a conversion
> > from absolute to relative.  I think that is a bug since we say the
> > value of dot inside a section is relative to the start of the section.
> >
> > I wonder how many scripts rely on the old behaviour?  If I don't hear
> > any objections to this change, I'll install the following in a day or
> > two.
> >
> >     * ldexp.c (align_dot_val): New function.
> >     (fold_unary <ALIGN_K, NEXT>): Use it.
>
> It turns out that this patch is causing problems for some linker scripts
> Mentor distributes.  I don't know if it's the change to ALIGN that is
> doing it, or if it's exposing another bug somewhere else.  :-S

Looking at this some more, I'm becoming more convinced that the patch is 
incorrect.  The previous behavior of the unary ALIGN expression appears 
to have been both the original implementation and intent, and the 
discrepancy between that behavior and the documentation is a bug in the 
manual.

The oldest version of binutils I could find (2.3, from 1993) appears to 
implement the pre-patch behavior for unary ALIGN:

	 case ALIGN_K:
	  if (allocation_done != lang_first_phase_enum) {
	    result = new_rel_from_section(ALIGN_N(dot,
						result.value) ,
					  current_section);

	  }
	  else {
	    result.valid = false;
	  }
	  break;

and documents it thusly:

@item ALIGN(@var{exp})
@kindex ALIGN(@var{exp})
@cindex rounding up location counter
Return the result of the current location counter (@code{.}) aligned to
the next @var{exp} boundary.  @var{exp} must be an expression whose
value is a power of two.  This is equivalent to
@example
(. + @var{exp} - 1) & ~(@var{exp} - 1)
@end example

@code{ALIGN} doesn't change the value of the location counter---it just
does arithmetic on it.  As an example, to align the output @code{.data}
section to the next @code{0x2000} byte boundary after the preceding
section and to set a variable within the section to the next
@code{0x8000} boundary after the input sections:
@example
SECTIONS@{ @dots{}
        .data ALIGN(0x2000): @{
                *(.data)
                variable = ALIGN(0x8000);
        @}
@dots{} @}
@end example
[snip]

The two-argument form of ALIGN was added in 2004, by this patch:

https://sourceware.org/git/gitweb.cgi?p=binutils-gdb.git;a=commit;h=aa59a2adb9d7b0f3b63398d5572f1c50faeaf4a6

I think the bug is in the documentation change added by that commit, 
that added the statement

@code{ALIGN(@var{align})} is equivalent to @code{ALIGN(., @var{align})}

Either that statement was wrong and didn't accurately reflect what the 
one-argument form of ALIGN really did, or the implementation of the 
two-argument form didn't do what was intended.

Anyway.....  given that the previous behavior of unary ALIGN was in 
place for 20+ years, changing it now seems like a mistake and likely to 
cause compatibility problems for more users than just us.  Likewise, 
since the two-argument form of ALIGN has been there for 11+ years, it's 
probably a mistake to try to change that behavior now, too, unless there 
is some evidence that the breakage is very recent.  Can we revert the 
code change and fix the documentation instead?  Maybe instead of trying 
to give an equivalence between the one- and two-argument versions of 
ALIGN, it would be better to explain how "ALIGN(align)" differs from 
"ALIGN(., align)"?

-Sandra