[Bug c/101422] New: register variable uninitialised before passing to asm

[Bug c/101422] New: register variable uninitialised before passing to asm

[simon.willcocks at gmx dot de]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=101422

            Bug ID: 101422
           Summary: register variable uninitialised before passing to asm
           Product: gcc
           Version: 10.3.1
            Status: UNCONFIRMED
          Severity: normal
          Priority: P3
         Component: c
          Assignee: unassigned at gcc dot gnu.org
          Reporter: simon.willcocks at gmx dot de
  Target Milestone: ---

Created attachment 51132
  --> https://gcc.gnu.org/bugzilla/attachment.cgi?id=51132&action=edit
Source code fragment

The attached program fragment sets up multiple register variables associated
with specific ARM registers and runs some assembly code that makes use of them.
With -Os (and a generated memset to initialise a local array), some registers
are not correctly initialised, notably r0, which is corrupted by the address of
the array, and r3 which is not initialised at all before the asm instruction.
r4, however, is correctly initialised.

Affects arm-linux-gnueabi-gcc-10, arm-linux-gnueabi-gcc-9, and
arm-linux-gnueabi-gcc-8.

for opt in 1 2 3 4 s ; do for vers in 8 9 10 ; do arm-linux-gnueabi-gcc-$vers
problem.c -S -O$opt -Wall -Wextra -o problem-$vers-$opt.s ; done ; done

To see the problem (the most obvious feature, at least):

for i in problem*.s ; do echo ; echo $i ; grep r3 $i ; done

In all three versions with -Os, the first mention of r3 is the asm code.

[Bug inline-asm/101422] register variable uninitialised before passing to asm

[pinskia at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=101422

Andrew Pinski <pinskia at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
         Resolution|---                         |INVALID
             Status|UNCONFIRMED                 |RESOLVED

--- Comment #1 from Andrew Pinski <pinskia at gcc dot gnu.org> ---
You are just passing the address of the variable.  You either need "m"(v)
there, a simple "memory" might work too.

See PR 36639, and PR 93952.  There are others.

[Bug inline-asm/101422] register variable uninitialised before passing to asm

[simon.willcocks at gmx dot de]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=101422

Simon Willcocks <simon.willcocks at gmx dot de> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
         Resolution|INVALID                     |FIXED

--- Comment #2 from Simon Willcocks <simon.willcocks at gmx dot de> ---
That's not an accurate description of the problem; the value of the variable is
being passed, not its address. As a register variable, it doesn't have an
address.

The problem occurred because the declaration of the initialised array is
located after the affected register variables are declared, invalidating their
content with an automatically generated function call to memset.

Moving this line:

  uint32_t cap_and_join_style[5] =  { 0x00000001 };

to the start of the function, before the first register variable declaration,
solves the problem.

Perhaps, if the problem is a common one, a warning could be generated "Unused
local register variable", say?

[Bug inline-asm/101422] register variable uninitialised before passing to asm

[simon.willcocks at gmx dot de]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=101422

--- Comment #3 from Simon Willcocks <simon.willcocks at gmx dot de> ---
(In reply to Andrew Pinski from comment #1)

But thanks for your quick response!

[Bug inline-asm/101422] register variable uninitialised before passing to asm

[pinskia at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=101422

Andrew Pinski <pinskia at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
         Resolution|FIXED                       |INVALID

--- Comment #4 from Andrew Pinski <pinskia at gcc dot gnu.org> ---
(In reply to Simon Willcocks from comment #2)
> That's not an accurate description of the problem; the value of the variable
> is being passed, not its address. As a register variable, it doesn't have an
> address.

It is the address of the array that is being passed (I was copying and pasting
from another bug).
register uint32_t *cap_and_join asm( "r5" ) = cap_and_join_style;
Is the same as:
register uint32_t *cap_and_join asm( "r5" ) = &cap_and_join_style[0];

Because array decays to pointers :).

[Bug inline-asm/101422] register variable uninitialised before passing to asm

[simon.willcocks at gmx dot de]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=101422

--- Comment #5 from Simon Willcocks <simon.willcocks at gmx dot de> ---
(In reply to Andrew Pinski from comment #4)
> (In reply to Simon Willcocks from comment #2)
> > That's not an accurate description of the problem; the value of the variable
> > is being passed, not its address. As a register variable, it doesn't have an
> > address.
> 
> It is the address of the array that is being passed (I was copying and
> pasting from another bug).
> register uint32_t *cap_and_join asm( "r5" ) = cap_and_join_style;
> Is the same as:
> register uint32_t *cap_and_join asm( "r5" ) = &cap_and_join_style[0];
> 
> Because array decays to pointers :).

I know. I've been doing this for a while, now. *That* register variable and the
array it pointed to were correctly initialised. r0-r3 are clobbered by any
function call, although I didn't program a function call, the compiler did.

[Bug inline-asm/101422] register variable uninitialised before passing to asm

[jakub at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=101422

Jakub Jelinek <jakub at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |jakub at gcc dot gnu.org

--- Comment #6 from Jakub Jelinek <jakub at gcc dot gnu.org> ---
Moving the line is the right fix, using register variables is always playing
with fire, as any changes to the registers from evaluation of the expressions
will change value of those register variables.
That can be the case of memset in your testcase for the array initialization,
or e.g. if you divide or modulo in some expression and the CPU requires a
specific register for those etc.
It is recommended to initialize normal automatic variables first and then only
initialize register vars to those automatic variables right before the inline
asm, with no other code in between.

[Bug inline-asm/101422] register variable uninitialised before passing to asm

[simon.willcocks at gmx dot de]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=101422

--- Comment #7 from Simon Willcocks <simon.willcocks at gmx dot de> ---
(In reply to Jakub Jelinek from comment #6)
> Moving the line is the right fix
Thanks for the confirmation.

> It is recommended to initialize normal automatic variables first and then
> only initialize register vars to those automatic variables right before the
> inline asm, with no other code in between.

In retrospect, I see that that is what this paragraph in
https://gcc.gnu.org/onlinedocs/gcc-10.3.0/gcc/Local-Register-Variables.html is
trying to say:

"Warning: In the above example, be aware that a register (for example r0) can
be call-clobbered by subsequent code, including function calls and library
calls for arithmetic operators on other variables (for example the
initialization of p2). In this case, use temporary variables for expressions
between the register assignments:"

For a coder to identify if they've encountered "this case" is very difficult
(at least, it took me several days), I think perhaps a hard-and-fast rule would
be more helpful? (With a change to the example to match.)

"Warning: any expression used to initialise a local register variable more
complex than a simple assignment from a local variable may involve a
compiler-generated function call that could invalidate previously initialised
register variables. Therefore, it is recommended that programs only initialize
register variables to the values of previously initialised automatic variables
just before the inline asm, with no other code in between."

    int *p1value = …;
    int *p2value = …;
    register int *p1 asm ("r0") = p1value;
    register int *p2 asm ("r1") = p2value;
    register int *result asm ("r0");
    asm ("sysint" : "=r" (result) : "0" (p1), "r" (p2));

(I would still like to see a warning like "local register variable may not be
used as expected", but there can't be all that many people using this feature.)

[Bug inline-asm/101422] register variable uninitialised before passing to asm

[simon.willcocks at gmx dot de]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=101422

--- Comment #8 from Simon Willcocks <simon.willcocks at gmx dot de> ---
(In reply to Andrew Pinski from comment #4)
> (In reply to Simon Willcocks from comment #2)
> > That's not an accurate description of the problem; the value of the variable
> > is being passed, not its address. As a register variable, it doesn't have an
> > address.
> 
> It is the address of the array that is being passed (I was copying and
> pasting from another bug).
> register uint32_t *cap_and_join asm( "r5" ) = cap_and_join_style;
> Is the same as:
> register uint32_t *cap_and_join asm( "r5" ) = &cap_and_join_style[0];
> 
> Because array decays to pointers :).

And, of course, I later came up against this precise problem, at least I knew
what I was looking for, and thanks to you, how to fix it. The final solution is
as follows. I really think that passing a pointer type into an asm should
indicate that the object pointed to is used, though.

void Draw_Stroke( uint32_t *path )                                              
{                                                                               
  // Keep this declaration before the first register variable declaration, or   
  // -Os will cause the compiler to forget the preceding registers.             
  // https://gcc.gnu.org/bugzilla/show_bug.cgi?id=101422                        
  uint32_t cap_and_join_style[5] =  { 0x00000001 }; // Round joints             

  register uint32_t *draw_path asm( "r0" ) = path;                              
  register uint32_t fill_style asm( "r1" ) = 0x3f;                              
  register uint32_t *matrix asm( "r2" ) = 0;                                    
  register uint32_t flatness asm( "r3" ) = 0;                                   
  register uint32_t thickness asm( "r4" ) = 8;                                  
  register uint32_t *cap_and_join asm( "r5" ) = cap_and_join_style;             
  register uint32_t dashes asm( "r6" ) = 0;                                     
  asm ( "swi 0x60704" :                                                         
        : "r" (draw_path)                                                       
        , "r" (fill_style)                                                      
        , "r" (matrix)                                                          
        , "r" (flatness)                                                        
        , "r" (thickness)                                                       
        , "r" (cap_and_join)                                                    
        , "r" (dashes)                                                          
        , "m" (cap_and_join_style) ); // Without this, array is not initialised 
}

I wanted to correct the record and thank you (both). I'll shut up, now.