Bug 31693 - Incorrectly assigned registers to operands for ARM inline asm
Summary: Incorrectly assigned registers to operands for ARM inline asm
Status: RESOLVED INVALID
Alias: None
Product: gcc
Classification: Unclassified
Component: inline-asm (show other bugs)
Version: 4.1.1
: P3 normal
Target Milestone: ---
Assignee: Not yet assigned to anyone
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2007-04-25 07:24 UTC by Siarhei Siamashka
Modified: 2009-02-10 15:21 UTC (History)
3 users (show)

See Also:
Host:
Target: arm-softfloat-linux-gnueabi
Build:
Known to work:
Known to fail: 3.3.6 4.0.4 4.1.2 4.2.0 4.3.0
Last reconfirmed:


Attachments
testcase for this bug (876 bytes, text/plain)
2007-04-25 07:26 UTC, Siarhei Siamashka
Details

Note You need to log in before you can comment on or make changes to this bug.
Description Siarhei Siamashka 2007-04-25 07:24:25 UTC
In the attached testcase, gcc assigns the same register to several inline asm named operands resulting in incorrect code generated. Seems like names of operands do matter ('c' and 'count' are assigned the same register but renaming 'c' operand to 'xxc' for example makes this bug disappear).
Comment 1 Siarhei Siamashka 2007-04-25 07:26:33 UTC
Created attachment 13436 [details]
testcase for this bug

Testcase attached
Comment 2 Siarhei Siamashka 2007-04-25 07:28:09 UTC
This may be related to #31386
Comment 3 Mikael Pettersson 2007-05-31 20:01:10 UTC
I can confirm this broken behaviour, including that it disappears if the 'c' variable is renamed to 'xxc', with gcc-3.3.6, 4.0.4, 4.1.2, and 4.2.0, all running natively on an armv5b-softvfp-linux machine.
Comment 4 Siarhei Siamashka 2008-05-13 12:32:20 UTC
This bug is still present in gcc 4.3
Comment 5 Siarhei Siamashka 2008-09-03 09:52:55 UTC
I'm sorry, is anybody investigating this quite serious bug? If nobody has time/motivation to do this work, would it make sense for me to try fixing it myself and submit a patch here?
Comment 6 Richard Earnshaw 2009-02-10 14:36:21 UTC
This is not a bug, but a problem with your source code.

In order to understand why, you need to pre-process the code and look at the output:

...
void *memset_arm9(void *a, int b, int c)
{
  return ({ uint8_t *dst = ((uint8_t *)a); uint8_t c = (b); int count = (c); uin
t32_t dummy0, dummy1, dummy2; __asm__ __volatile__ (

Notice that first there is a declaration of a variable c (uint8_t), then in the next statement there is a use of c.  This use (which is intended to be of the formal parameter passed to memset_arm9 is instead interpreted as the newly declared variable c (the uint8 one).


Compiling your testcase with -Wshadow gives:

inl.c: In function 'memset_arm9':
inl.c:66: warning: declaration of 'c' shadows a parameter
inl.c:64: warning: shadowed declaration is here
Comment 7 Siarhei Siamashka 2009-02-10 15:11:27 UTC
(In reply to comment #6)
> This is not a bug, but a problem with your source code.
> 
> In order to understand why, you need to pre-process the code and look at the
> output:
> 
> ...
> void *memset_arm9(void *a, int b, int c)
> {
>   return ({ uint8_t *dst = ((uint8_t *)a); uint8_t c = (b); int count = (c);
> uin
> t32_t dummy0, dummy1, dummy2; __asm__ __volatile__ (
> 
> Notice that first there is a declaration of a variable c (uint8_t), then in the
> next statement there is a use of c.  This use (which is intended to be of the
> formal parameter passed to memset_arm9 is instead interpreted as the newly
> declared variable c (the uint8 one).
> 
> 
> Compiling your testcase with -Wshadow gives:
> 
> inl.c: In function 'memset_arm9':
> inl.c:66: warning: declaration of 'c' shadows a parameter
> inl.c:64: warning: shadowed declaration is here

Thanks for having a look at this. Indeed, macros are quite dangerous.

Nevertheless, would it make sense to add this -Wshadow option into the set provided by -Wextra, or even introduce something like -Wreally-all option specifically for debugging such cases?

Even better (but understandably not realistic) would be to have an option to show this warning only for the code which was expanded by C preprocessor in order to reduce the number of false positives.
Comment 8 Richard Earnshaw 2009-02-10 15:21:17 UTC
Opinions vary wildly as to which warnings should be on by default, and which should be part of whatever bundle.

I personally agree that shadowing variables is generally bad practice, but you then have to be wary when porting code because you can end up shadowing things that come from a library (different standard libraries contain all sorts of wild and wacky variables or functions, most of which you'll never care about).  For example:

#include <stdio.h>

int f()
{
  int printf = 3; return printf;
}

will generate a warning if compiled with -Wshadow (ok, that's somewhat perverse, since every library has printf, but it makes the point).