Bug 88000 - Warn when different local vars regs order may produce different and so unsupported code [-Wasm-register-var]
Summary: Warn when different local vars regs order may produce different and so unsupp...
Status: REOPENED
Alias: None
Product: gcc
Classification: Unclassified
Component: c (show other bugs)
Version: unknown
: P3 normal
Target Milestone: ---
Assignee: Not yet assigned to anyone
URL:
Keywords: diagnostic
Depends on:
Blocks: new-warning, new_warning
  Show dependency treegraph
 
Reported: 2018-11-13 01:09 UTC by Dominik 'disconnect3d' Czarnota
Modified: 2021-10-31 23:09 UTC (History)
2 users (show)

See Also:
Host:
Target:
Build:
Known to work:
Known to fail:
Last reconfirmed: 2019-06-15 00:00:00


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description Dominik 'disconnect3d' Czarnota 2018-11-13 01:09:29 UTC
Depending on the order of two register local variables definition, when compiled with optimizations (-O1 / -O2 / -O3 or -Og), GCC may emit code that will not use the value assigned to the variable at all.

This bug seems to be introduced in GCC 7.2 (tested with godbolt.org) and have been tested only on x86-64 targets.

You can see a minimal working example below:

```
#include <stdio.h>

void bar(int x) {
    printf("x = %d\n", x);
}

int main() {
    // if we change the order of those two lines
    // gcc will emit a proper `mov edi, 11`
    // otherwise, it fails to do so since gcc 7.2
    // when compiled with -O1/2/3/g
    register int b asm("r11") = 11;
    register int a asm("r12") = 22;
    
    // also, if `b` is any of the registers that are
    // supposed to be preserved by the called function,
    // gcc will also emit proper code
    // (so any of: rbp, rbx, r12-r15)
    // the `a` register seem to not matter
    bar(a);
    bar(b);
}
```

Tested locally on a GCC 7.3. The `main.c` is the program shown above, `main_rev.c` is the one with `b` and `a` variables definitions swapped:

```
$ gcc main.c -O1 && ./a.out
x = 22
x = 582

$ gcc main_rev.c -O1 && ./a.out
x = 22
x = 11
```

The difference can be seen in the disassembly:

```
$ gcc -O1 main.c && gdb -batch -ex 'file ./a.out' -ex 'set disassembly-flavor intel' -ex 'disassemble main'
Dump of assembler code for function main:
   0x000000000000068b <+0>:	sub    rsp,0x8
   0x000000000000068f <+4>:	mov    edi,0x16
   0x0000000000000694 <+9>:	call   0x66a <bar>
   0x0000000000000699 <+14>:	mov    edi,r11d
   0x000000000000069c <+17>:	call   0x66a <bar>
   0x00000000000006a1 <+22>:	mov    eax,0x0
   0x00000000000006a6 <+27>:	add    rsp,0x8
   0x00000000000006aa <+31>:	ret
End of assembler dump.

$ gcc -O1 main_rev.c && gdb -batch -ex 'file ./a.out' -ex 'set disassembly-flavor intel' -ex 'disassemble main'
Dump of assembler code for function main:
   0x000000000000068b <+0>:	push   r12
   0x000000000000068d <+2>:	mov    edi,0x16
   0x0000000000000692 <+7>:	call   0x66a <bar>
   0x0000000000000697 <+12>:	mov    edi,0xb
   0x000000000000069c <+17>:	call   0x66a <bar>
   0x00000000000006a1 <+22>:	mov    eax,0x0
   0x00000000000006a6 <+27>:	pop    r12
   0x00000000000006a8 <+29>:	ret
End of assembler dump.
```

As written in the comment, it seems that this change happens only if the `b` is assigned to a register that can be clobbered (and so different than rbp, rbx, r12-r15).
Comment 1 Andrew Pinski 2018-11-13 01:12:18 UTC
Read https://gcc.gnu.org/onlinedocs/gcc-8.2.0/gcc/Local-Register-Variables.html#Local-Register-Variables .



The only supported use for this feature is to specify registers for input and output operands when calling Extended asm (see Extended Asm).
Comment 2 Dominik 'disconnect3d' Czarnota 2018-11-13 01:26:20 UTC
Yes, I know it.

Why do we let users to use local register variables for other purposes than extended asm if it can't be expected to work properly (in this example, copy the passing value from the declared register)?
Comment 3 Andrew Pinski 2018-11-13 01:45:57 UTC

(In reply to Dominik Czarnota from comment #2)
> Yes, I know it.
> 
> Why do we let users to use local register variables for other purposes than
> extended asm if it can't be expected to work properly (in this example, copy
> the passing value from the declared register)?

Because if the user wants to shoot them selves in the foot that is up to them.  Read the documentation and you see that what you want to be use it for, is not supported:
As with global register variables, it is recommended that you choose a register that is normally saved and restored by function calls on your machine, so that calls to library routines will not clobber it.

In this case you the register is being clobbered.
Comment 4 Martin Sebor 2018-11-13 16:31:53 UTC
GCC could help by issuing a warning for unsupported uses, like in the prototype patch below:

Index: gcc/c/c-typeck.c
===================================================================
--- gcc/c/c-typeck.c	(revision 266033)
+++ gcc/c/c-typeck.c	(working copy)
@@ -6505,6 +6505,14 @@ convert_for_assignment (location_t location, locat
       objc_ok = objc_compare_types (type, rhstype, parmno, rname);
     }
 
+  if (VAR_P (rhs) && DECL_HARD_REGISTER (rhs)
+      && warning_at (expr_loc, OPT_Wasm_register_var,
+		     "unsupported use of a hard register %qD as "
+		     "argument %d of %qE",
+		     rhs, parmnum, rname))
+    inform (DECL_SOURCE_LOCATION (rhs),
+	    "%qD declared here", rhs);
+
   if (warn_cxx_compat)
     {
       tree checktype = origtype != NULL_TREE ? origtype : rhstype;
Index: gcc/c-family/c.opt
===================================================================
--- gcc/c-family/c.opt	(revision 266033)
+++ gcc/c-family/c.opt	(working copy)
@@ -338,6 +338,10 @@ Warray-bounds=
 LangEnabledBy(C ObjC C++ LTO ObjC++,Wall,1,0)
 ; in common.opt
 
+Wasm-register-var
+ObjC ObjC++ Var(warn_asm_register_var) Warning LangEnabledBy(C ObjC C++ ObjC++, Wall)
+Warn for unsupported uses of variables declared asm register.
+
 Wassign-intercept
 ObjC ObjC++ Var(warn_assign_intercept) Warning
 Warn whenever an Objective-C assignment is being intercepted by the garbage collector.
Comment 5 Eric Gallager 2019-06-15 00:22:16 UTC
(In reply to Martin Sebor from comment #4)
> GCC could help by issuing a warning for unsupported uses, like in the
> prototype patch below:
> 
> Index: gcc/c/c-typeck.c
> ===================================================================
> --- gcc/c/c-typeck.c	(revision 266033)
> +++ gcc/c/c-typeck.c	(working copy)
> @@ -6505,6 +6505,14 @@ convert_for_assignment (location_t location, locat
>        objc_ok = objc_compare_types (type, rhstype, parmno, rname);
>      }
>  
> +  if (VAR_P (rhs) && DECL_HARD_REGISTER (rhs)
> +      && warning_at (expr_loc, OPT_Wasm_register_var,
> +		     "unsupported use of a hard register %qD as "
> +		     "argument %d of %qE",
> +		     rhs, parmnum, rname))
> +    inform (DECL_SOURCE_LOCATION (rhs),
> +	    "%qD declared here", rhs);
> +
>    if (warn_cxx_compat)
>      {
>        tree checktype = origtype != NULL_TREE ? origtype : rhstype;
> Index: gcc/c-family/c.opt
> ===================================================================
> --- gcc/c-family/c.opt	(revision 266033)
> +++ gcc/c-family/c.opt	(working copy)
> @@ -338,6 +338,10 @@ Warray-bounds=
>  LangEnabledBy(C ObjC C++ LTO ObjC++,Wall,1,0)
>  ; in common.opt
>  
> +Wasm-register-var
> +ObjC ObjC++ Var(warn_asm_register_var) Warning LangEnabledBy(C ObjC C++
> ObjC++, Wall)
> +Warn for unsupported uses of variables declared asm register.
> +
>  Wassign-intercept
>  ObjC ObjC++ Var(warn_assign_intercept) Warning
>  Warn whenever an Objective-C assignment is being intercepted by the garbage
> collector.

Reopening to confirm the part about this warning request at least
Comment 6 Dominik 'disconnect3d' Czarnota 2019-06-16 22:23:52 UTC
(In reply to Eric Gallager from comment #5)
> Reopening to confirm the part about this warning request at least

Yay, thanks. I am happy someone cares about this. It is good to make it less likely that the users shoot themselves in the foot.