Discussion:
Error: non-constant expression in ".if" statement
Jérôme Forissier
2016-06-14 12:07:55 UTC
Permalink
Hi,

I've stumbled across an assembler error message that I don't understand.

bl1/aarch64/bl1_exceptions.S: Assembler messages:
bl1/aarch64/bl1_exceptions.S:53: Error: non-constant expression in
".if" statement

It occurs when building ARM Trusted Firmware with aarch64-linux-gnu-gcc
that ships with Ubuntu 16.04. It does _not_ occur with an older Linaro
toolchain. More details in [1].

The .align directives in the vector_base and vector_entry macros _do_
make a difference. Are they the cause of the problem? Would you
recommend writing the code differently? Or is it a compiler bug?

[1] https://github.com/ARM-software/tf-issues/issues/401

Thanks,
--
Jerome
Christophe Lyon
2016-06-14 18:37:38 UTC
Permalink
Post by Jérôme Forissier
Hi,
I've stumbled across an assembler error message that I don't understand.
bl1/aarch64/bl1_exceptions.S:53: Error: non-constant expression in
".if" statement
It occurs when building ARM Trusted Firmware with aarch64-linux-gnu-gcc
that ships with Ubuntu 16.04. It does _not_ occur with an older Linaro
toolchain. More details in [1].
The .align directives in the vector_base and vector_entry macros _do_
make a difference. Are they the cause of the problem? Would you
recommend writing the code differently? Or is it a compiler bug?
[1] https://github.com/ARM-software/tf-issues/issues/401
Hi Jerome,

Can you try with more recent Linaro releases (5.x)?

I've tried with our 5.3-2016.02, and compilation worked.

I tried 2 older revisions on our development branch and it worked too.

Christophe
Post by Jérôme Forissier
Thanks,
--
Jerome
_______________________________________________
linaro-toolchain mailing list
https://lists.linaro.org/mailman/listinfo/linaro-toolchain
Jim Wilson
2016-06-14 19:43:48 UTC
Permalink
On Tue, Jun 14, 2016 at 11:37 AM, Christophe Lyon
Post by Jérôme Forissier
Hi,
I've stumbled across an assembler error message that I don't understand.
bl1/aarch64/bl1_exceptions.S:53: Error: non-constant expression in
".if" statement
I can reproduce with an FSF binutils. Not obvious why it is failing though.

The assembler error comes because you are subtracting two addresses in
a .if, and one of more of those addresses is not a constant, because
the assembler could not calculate a fixed address on the first pass
through. However, it isn't obvious why the assembler is failing to
calculate a fixed address, or why the aligns matter. If the aligns
added a variable amount of in section padding, then that would prevent
the addresses from being constant. But in this case, the aligns
should not be adding a variable amount of padding. I'd have to spend
time debugging the assembler to figure out what is wrong here.

Jim
Jim Wilson
2016-06-14 21:50:16 UTC
Permalink
Post by Jim Wilson
I can reproduce with an FSF binutils. Not obvious why it is failing though.
A git bisect tracks the problem down to this commit.

palantir:2177$ git bisect bad
c1baaddf8861aea666b84baeb4746caff51a579d is the first bad commit
commit c1baaddf8861aea666b84baeb4746caff51a579d
Author: Renlin Li <***@arm.com>
Date: Thu Apr 2 14:59:45 2015 +0100

[AArch64] Emit DATA_MAP in order within text section

2015-03-27 Renlin Li <***@arm.com>

gas/
* config/tc-aarch64.c (mapping_state): Emit MAP_DATA within text
section in order.
(mapping_state_2): Don't emit MAP_DATA here.
(s_aarch64_inst): Align frag during state transition.
(md_assemble): Likewise.

:040000 040000 513292511d3717bb5d8fa7f3227e5eb352025a02
06d2298d935ea4d0698c7dbf2b713e2dc290f498 M gas
palantir:2178$

The problem here is with the md_assemble change, which is calling
frag_align_code inside a loop, which means it can emit multiple
alignment frags with a single call. We are not in the text section,
which means we always get this alignment frag. These alignment frags
have variable size, and hence the .if test on dot no longer works.

This patch isn't present in the binutils-2.25 that tcwg is using. The
patch is present in binutils-2.26.

I've never looked at this MAP_DATA support, so the solution is not
immediately obvious to me.

Testcase attached.

Jim
Jim Wilson
2016-06-15 03:34:26 UTC
Permalink
A few more comments...

The MAP_DATA support is emitting mapping symbols documented in the
AARCH64 ELF ABI. Mapping symbols are emitted when switching from data
to instructions, or vice versa, inside a section. However, mapping
symbols are just addresses. There should be no need to emit implicit
alignment directives before padding symbols. When switching between
data and instructions, it should be the responsibility of the
programmer or compiler to emit any necessary alignment directives.

The testcase is using e.g. ".align 11, 0". This specifies that the
padding will be data bytes of 0, instead of perhaps a nop instruction.
Since we are explicitly emitting data into a text section, this forces
a switch when we emit the first instruction, which then forces an
implicit alignment directive. if the code uses ".align 11" instead,
then the assembler will pad with nop instructions (or maybe zeros),
and there is no following switch from data mode to instruction mode.

The code to emit the implicit alignment directives is checking for a
.text section. It should be checking for a section of any name that
contains code instead. That eliminates unnecessary implicit alignment
directives emitted in non-text text sections, like in the example
which has a text section called .vectors.

I didn't try the first solution, as it isn't obvious why the
apparently unnecessary implicit alignment directives are being
emitted. If this padding is necessary for some obscure reason, then
removing it would break something.

I did try solutions 2+3, and used together they allow the testcase to
work. Patches attached.

Jim
Jérôme Forissier
2016-06-16 13:58:10 UTC
Permalink
Hi Jim,
Post by Jim Wilson
A few more comments...
The MAP_DATA support is emitting mapping symbols documented in the
AARCH64 ELF ABI. Mapping symbols are emitted when switching from data
to instructions, or vice versa, inside a section. However, mapping
symbols are just addresses. There should be no need to emit implicit
alignment directives before padding symbols. When switching between
data and instructions, it should be the responsibility of the
programmer or compiler to emit any necessary alignment directives.
The testcase is using e.g. ".align 11, 0". This specifies that the
padding will be data bytes of 0, instead of perhaps a nop instruction.
Since we are explicitly emitting data into a text section, this forces
a switch when we emit the first instruction, which then forces an
implicit alignment directive. if the code uses ".align 11" instead,
then the assembler will pad with nop instructions (or maybe zeros),
and there is no following switch from data mode to instruction mode.
The code to emit the implicit alignment directives is checking for a
.text section. It should be checking for a section of any name that
contains code instead. That eliminates unnecessary implicit alignment
directives emitted in non-text text sections, like in the example
which has a text section called .vectors.
I didn't try the first solution, as it isn't obvious why the
apparently unnecessary implicit alignment directives are being
emitted. If this padding is necessary for some obscure reason, then
removing it would break something.
I did try solutions 2+3, and used together they allow the testcase to
work. Patches attached.
Thanks for this cool investigation. I hope this will get fixed in the
FSF builds ultimately -- either through your patches or through a future
merge with the Linaro toolchain, which is unaffected.

For now, ARM-TF upstream will keep the padding value, i.e., ".align 11,
0" rather than ".align 11", because it makes the code more robust.
They're using a Linaro toolchain.

As far as I'm concerned, I will either apply the workaround or switch to
a Linaro toolchain.

Thanks,
--
Jerome
Loading...