From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 16052 invoked by alias); 3 Dec 2010 16:20:24 -0000 Received: (qmail 15702 invoked by uid 22791); 3 Dec 2010 16:20:21 -0000 X-SWARE-Spam-Status: No, hits=-1.8 required=5.0 tests=AWL,BAYES_00,T_RP_MATCHES_RCVD X-Spam-Check-By: sourceware.org Received: from mail.codesourcery.com (HELO mail.codesourcery.com) (38.113.113.100) by sourceware.org (qpsmtpd/0.43rc1) with ESMTP; Fri, 03 Dec 2010 16:20:14 +0000 Received: (qmail 17768 invoked from network); 3 Dec 2010 16:20:11 -0000 Received: from unknown (HELO tp.orcam.me.uk) (macro@127.0.0.2) by mail.codesourcery.com with ESMTPA; 3 Dec 2010 16:20:11 -0000 Date: Fri, 03 Dec 2010 16:20:00 -0000 From: "Maciej W. Rozycki" To: Richard Sandiford cc: Richard Earnshaw , Jie Zhang , Catherine Moore , binutils@sourceware.org Subject: Re: [PATCH 2.5/4 v2] GAS: Make new fake labels when cloning a symbol In-Reply-To: Message-ID: References: <87aalwngmr.fsf@firetop.home> <87r5f4d92p.fsf@firetop.home> <1291309254.9299.9.camel@e102346-lin.cambridge.arm.com> User-Agent: Alpine 1.10 (DEB 962 2008-03-14) MIME-Version: 1.0 Content-Type: TEXT/PLAIN; charset=US-ASCII Mailing-List: contact binutils-help@sourceware.org; run by ezmlm Precedence: bulk List-Id: List-Subscribe: List-Archive: List-Post: List-Help: , Sender: binutils-owner@sourceware.org X-SW-Source: 2010-12/txt/msg00153.txt.bz2 On Fri, 3 Dec 2010, Richard Sandiford wrote: > > Thanks for reducing the test case. Below I'm including a change that is > > supposed to fix it. The original change indeed is what triggered the > > problem, but from the code I have modified I infer it is more related to > > the issue with equated symbols not being handled correctly that I have > > fixed as well. This function counts as a symbol reference and as such > > should resolve any equated symbols by making a clone. Chances are code > > could have been crafted that would trigger a problem here even before my > > fixes, but I found no justification to spend time investigating that. > > > > The change below fixes the problem with your test case as well as one > > attached to PR gas/12282. I have regression-tested it with the > > arm-none-eabi and mips-sde-elf targets. > > > > 2010-12-02 Maciej W. Rozycki > > > > PR gas/12282 > > * expr.c (make_expr_symbol): Make a clone if handling an > > equated symbol. > > I'm not convinced this is correct when operand() calls make_expr_symbol() > in expr_defer mode. Could you provide a bit more justification? Hmm, good point, thanks, and expr() too. I'm not sure why I missed it. I have given it some more thought and come to the conclusion it has to be decided by the caller of make_expr_symbol() whether the symbol returned should be equated or cloned. Assuming optimistically all the callers other than operand(), expr() and expr_build_dot() considered here do the right thing, i.e. either use make_expr_symbol() on a non-symbol or cope with forward references I propose to fix expr_build_dot() only, like this. If more places are eventually required, then a wrapper function or macro can be created to combine make_expr_symbol() and symbol_clone_if_forward_ref(). The change below fixes the problem with the two test cases mentioned earlier. I have regression-tested it with the arm-none-eabi and mips-sde-elf targets. 2010-12-03 Maciej W. Rozycki PR gas/12282 * expr.c (expr_build_dot): Make a clone of the symbol to return if needed. OK to apply? Maciej binutils-gas-dot-fix.diff Index: binutils-fsf-trunk-quilt/gas/expr.c =================================================================== --- binutils-fsf-trunk-quilt.orig/gas/expr.c 2010-12-03 14:15:18.000000000 +0000 +++ binutils-fsf-trunk-quilt/gas/expr.c 2010-12-03 15:13:54.000000000 +0000 @@ -172,7 +172,7 @@ expr_build_dot (void) expressionS e; current_location (&e); - return make_expr_symbol (&e); + return symbol_clone_if_forward_ref (make_expr_symbol (&e)); } /* Build any floating-point literal here.