RISC-V/GAS: Correct an `expr' global shadowing error for pre-4.8 GCC

Message ID alpine.DEB.2.00.1802030239140.3553@tp.orcam.me.uk
State New
Headers show
Series
  • RISC-V/GAS: Correct an `expr' global shadowing error for pre-4.8 GCC
Related show

Commit Message

Maciej W. Rozycki Feb. 3, 2018, 3:15 p.m.
Correct a commit f0531ed6a429 ("Compress loads/stores with implicit 0 
offset.") regression and remove a `-Wshadow' compilation error:

cc1: warnings being treated as errors
.../gas/config/tc-riscv.c: In function 'riscv_handle_implicit_zero_offset':
.../gas/config/tc-riscv.c:1194: error: declaration of 'expr' shadows a global declaration
.../gas/expr.h:180: error: shadowed declaration is here
make[4]: *** [tc-riscv.o] Error 1

which for versions of GCC before 4.8 prevents GAS for RISC-V targets 
from being built.  See also GCC PR c/53066.

	gas/
	* config/tc-riscv.c (riscv_handle_implicit_zero_offset): Rename
	`expr' parameter to `ep'.
---
Hi,

 This has popped up as a regression in routine cross-target testing for an 
unrelated change and should be obvious.  OK to apply?

  Maciej
---
 gas/config/tc-riscv.c |    6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

binutils-riscv-gas-expr-shadow.diff

Comments

Jim Wilson Feb. 3, 2018, 7:45 p.m. | #1
On Sat, Feb 3, 2018 at 7:15 AM, Maciej W. Rozycki <macro@mips.com> wrote:
>         gas/

>         * config/tc-riscv.c (riscv_handle_implicit_zero_offset): Rename

>         `expr' parameter to `ep'.


OK.

Though this makes me wonder how far back I am expected to support.  I
think all of my current machines are Ubuntu 16.04 or newer, which
means gcc-5.4 or newer.  If I need to support older gcc versions, I
may need to install older OS verions, or at least have them available
as a chroot.  Looks like Ubuntu 14.04 is gcc-4.8 and Ubuntu 12.04 is
gcc-4.6, so I'd need something from circa 2012 to find these kinds of
problems myself.

Jim
Maciej W. Rozycki Feb. 5, 2018, 2:18 p.m. | #2
On Sat, 3 Feb 2018, Jim Wilson wrote:

> >         gas/

> >         * config/tc-riscv.c (riscv_handle_implicit_zero_offset): Rename

> >         `expr' parameter to `ep'.

> 

> OK.


 Applied, thanks for your review.

> Though this makes me wonder how far back I am expected to support.  I

> think all of my current machines are Ubuntu 16.04 or newer, which

> means gcc-5.4 or newer.  If I need to support older gcc versions, I

> may need to install older OS verions, or at least have them available

> as a chroot.  Looks like Ubuntu 14.04 is gcc-4.8 and Ubuntu 12.04 is

> gcc-4.6, so I'd need something from circa 2012 to find these kinds of

> problems myself.


 I think it's up to you really.  Releases disable `-Werror', so end users 
won't be affected unless they explicitly add this option themselves and 
RISC-V is a recent port, post-dating the affected GCC releases by quite a 
bit, which I suppose means most people working on it will have started 
with a fresh environment, free from these old issues.  So I think the 
impact from these issues is low and it IMHO won't be a big deal if you do 
not proactively look for them in the course of maintaining the port.  
However there's no harm I believe from getting them fixed once discovered.

 It looks like I trip over this twice or so per year across all the 
targets I regression-test in the course of patch submission, so it's not a 
big deal to me.  And most likely I'll get to upgrading GCC sometime as 
well.

 FWIW,

  Maciej

Patch

Index: binutils/gas/config/tc-riscv.c
===================================================================
--- binutils.orig/gas/config/tc-riscv.c	2018-01-17 23:59:44.000000000 +0000
+++ binutils/gas/config/tc-riscv.c	2018-01-18 15:07:37.797400998 +0000
@@ -1191,14 +1191,14 @@  my_getSmallExpression (expressionS *ep, 
    an implicit offset was detected.  */
 
 static bfd_boolean
-riscv_handle_implicit_zero_offset (expressionS *expr, const char *s)
+riscv_handle_implicit_zero_offset (expressionS *ep, const char *s)
 {
   /* Check whether there is only a single bracketed expression left.
      If so, it must be the base register and the constant must be zero.  */
   if (*s == '(' && strchr (s + 1, '(') == 0)
     {
-      expr->X_op = O_constant;
-      expr->X_add_number = 0;
+      ep->X_op = O_constant;
+      ep->X_add_number = 0;
       return TRUE;
     }