From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 3912 invoked by alias); 28 Jan 2005 03:52:01 -0000 Mailing-List: contact binutils-help@sources.redhat.com; run by ezmlm Precedence: bulk List-Subscribe: List-Archive: List-Post: List-Help: , Sender: binutils-owner@sources.redhat.com Received: (qmail 3835 invoked from network); 28 Jan 2005 03:51:56 -0000 Received: from unknown (HELO bluesmobile.specifixinc.com) (64.220.152.98) by sourceware.org with SMTP; 28 Jan 2005 03:51:56 -0000 Received: from [127.0.0.1] (bluesmobile.corp.specifixinc.com [192.168.1.2]) by bluesmobile.specifixinc.com (Postfix) with ESMTP id 916C016798; Thu, 27 Jan 2005 19:51:55 -0800 (PST) Subject: Re: [PATCH] ia64 unwind directive semantics From: James E Wilson To: Jan Beulich Cc: binutils@sources.redhat.com In-Reply-To: References: Content-Type: text/plain Message-Id: <1106884315.18514.197.camel@aretha.corp.specifixinc.com> Mime-Version: 1.0 Date: Fri, 28 Jan 2005 03:52:00 -0000 Content-Transfer-Encoding: 7bit X-SW-Source: 2005-01/txt/msg00469.txt.bz2 On Mon, 2005-01-24 at 03:29, Jan Beulich wrote: > * config/tc-ia64.c (unwind): Remove proc_end (now an automatic > variable in dot_endp). Add body and insn. Make prologue, > ... GNU coding conventions require explanatory comments before every function, though I realize most all of the unwind functions are already missing comments. I should really fix this before complaining about it to others. The testcases don't test all of the errors you added. In particular, the prologue/body before first insn messages. Also the proc/endp errors for a missing name, but those ones are obvious enough that I don't see the need for tests for them. Hopefully gcc emits unwind info that passes all of these tests. Likewise, for assembly code in glibc and the linux kernel. I will have to check all of this after all of these patches go in. I am not sure about one of the proc/endp changes you added. For proc, you replaced unwind.proc_start with sym. But that works only if sym is defined immediately after the proc. I would expect that to be true, but don't think it is safe to assume it. Also, if a procedure has multiple entry points, can we really be sure that the first name listed is the entry point with the lowest address? Using expr_build_dot here avoids these problems, and should be perfectly safe. I am not sure why the existing code sets proc_start to sym if it is zero. I think that is a bug, as it should never be zero here, as expr_build_dot should never return zero. I suspect the code originally worked as you wrote it, and then later got fixed to use expr_build_dot instead of sym because that didn't work, but we forgot to remove part of the old code. > + if (!sym || !S_IS_DEFINED (sym)) > + as_bad ("`%s' was not defined within procedure", name); > + else if (sym && unwind.proc_start You have a redundant check for sym here. It is guaranteed to be non-zero in the second if, so we don't need to check it again. Otherwise, this looks OK. -- Jim Wilson, GNU Tools Support, http://www.SpecifixInc.com