[llvm-bugs] [Bug 52171] New: Too early validation of inline assembly constraints

[via llvm-bugs]

https://bugs.llvm.org/show_bug.cgi?id=52171

            Bug ID: 52171
           Summary: Too early validation of inline assembly constraints
           Product: clang
           Version: 11.0
          Hardware: Other
                OS: other
            Status: NEW
          Severity: enhancement
          Priority: P
         Component: C
          Assignee: unassignedclangbugs at nondot.org
          Reporter: marian.buschsieweke at ovgu.de
                CC: blitzrakete at gmail.com, dgregor at apple.com,
                    erik.pilkington at gmail.com, llvm-bugs at lists.llvm.org,
                    richard-llvm at metafoo.co.uk

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.

-- 
You are receiving this mail because:
You are on the CC list for the bug.
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-bugs/attachments/20211014/86b3fbaa/attachment-0001.html>