Skip to content

Too early validation of inline assembly constraints #51513

@maribu

Description

@maribu
Bugzilla Link 52171
Version 11.0
OS other
CC @DougGregor,@zygoloid

Extended Description

Summary

Compilation for AVR with the header avr/wdt.h included fails due to different behavior regarding the validation of the constraints of operands for inline assembly. The issue is observed with AVR, but I think it could apply equally to all other architectures.

Background Information

Accessing a memory mapped h/w register in AVR takes 1 CPU cycle if it is done as I/O register (that is via a special instruction that takes the I/O register address as immediate), or 2 CPU cycles if it is done via regular load/store instructions to its memory address. The avrlibc has a macro _SFR_IO_REG_P() that evaluates to a c expression which is true, if and only if the given parameter is an IO register (so its address is in the range [0;63], allowing its value to be used as immediate). The constraint "I" for operands in extended inline assembly enforces that the operand is a value suitable as immediate, allowing the single-cycle access with the I/O register load/store instructions.

The AVR watchdog timer (WDT) allows changes of the configuration only after writing a magic number into a magic register and only for a brief time window of a few CPU cycles. For this reason changes of the WDT configuration are implemented in inline assembly to ensure that the timing requirements are fulfilled. I think (but I'm not 100% sure) that at least some AVR MCUs require the use of the faster I/O register access to meet the timing requirements when changing the WDT configuration.

The Issue

The issue is best explained with a code snippet:

static inline
attribute ((always_inline))
void wdt_enable (const uint8_t value)
{
if (_SFR_IO_REG_P (_WD_CONTROL_REG))
{
asm volatile (
[...]
: /* no outputs /
: "I" (_SFR_IO_ADDR(_WD_CONTROL_REG)),
[...]
: "r0"
);
}
else {
/
variant not using _WD_CONTROL_REG as immediate */
[...]
}
}

Since _SFR_IO_REG_P() evaluates to a compile time constant expression that yields true if the given address is suitable for inline assembly and false otherwise, either the then or the else clause is known to be a dead branch. GCC eliminates the dead branch before validation of the constraint I. So if "I" (_SFR_IO_ADDR(_WD_CONTROL_REG)) is an unfulfilled constrained, this is part of a dead branch that is never checked in GCC. In clang the constraints are validated before elimination of the dead branch, so compilation fails for MCUs that do not support single cycle access to _WD_CONTROL_REG.

What to Do about This?

First, is the behavior of clang here actually is incorrect? To me, enforcing constraints of inline assembly operands of dead branches feels consistent with what C compilers do: Invalid syntax at C level is also not tolerated even on dead branches. To me, this feels like the avrlibc exploiting an implementation detail of GCC and the issue should be fixed there. But I guess one could argue that inline assembly GCC extension that doesn't have to be consistent with regular C compiler behavior, and at least conceptually the enforcement of the constraints belongs to the stage of invoking an assembler that is done after the C level stage including optimization is done.

So what is the upstream opinion on this? Is this a valid bug, or should it be fixed at avrlibc?

References

See https://lists.gnu.org/archive/html/avr-libc-dev/2021-10/msg00000.html for discussion on the avrlibc mailing list.

Metadata

Metadata

Assignees

No one assigned

    Labels

    backend:AVRbugzillaIssues migrated from bugzillaquestionA question, not bug report. Check out https://llvm.org/docs/GettingInvolved.html instead!

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions