From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 16484 invoked by alias); 21 Apr 2017 19:37:48 -0000 Mailing-List: contact gdb-patches-help@sourceware.org; run by ezmlm Precedence: bulk List-Id: List-Subscribe: List-Archive: List-Post: List-Help: , Sender: gdb-patches-owner@sourceware.org Received: (qmail 16296 invoked by uid 89); 21 Apr 2017 19:37:47 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-1.9 required=5.0 tests=AWL,BAYES_00,FREEMAIL_FROM,RCVD_IN_DNSWL_NONE,SPF_PASS autolearn=ham version=3.3.2 spammy=site X-HELO: mail-wm0-f48.google.com Received: from mail-wm0-f48.google.com (HELO mail-wm0-f48.google.com) (74.125.82.48) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with ESMTP; Fri, 21 Apr 2017 19:37:46 +0000 Received: by mail-wm0-f48.google.com with SMTP id m123so24836506wma.0 for ; Fri, 21 Apr 2017 12:37:47 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:subject:to:references:from:message-id:date :user-agent:mime-version:in-reply-to:content-transfer-encoding; bh=1DOldKp23zqXf20Sr11NH8LUmjY/7cK1Hk+HSzf2bSw=; b=VB8sW3nmxULnt8rloNIcCMVBuueLg7c6YISJsF//elsbc/mneg6rp1jjT52yF4Bnhq 1pAw5TjP4XpG6muzTi/yOhyXum5SAT5SgOwQPDMd1KPB9yhG2TjxqgSDGZOMiMMek5Gl OalMBVCWjLCDzx3VwbNe4hnblM25MJfV0g6CUQcNL6xFCH+7Nshalsoq6kcA64eb0BH/ B/tfnokWa6RiccU48cW6S5N5lYe34SNBOYDTiCSWMlC06q8VQQBsOmhBKPUaq1NhDhAl d3RpKLbNu0PkohdF8E884yScvSwMp5v0+B5YNVHCblP6vOa8xlq0ya7a7aQnOIxgy/F/ K2TQ== X-Gm-Message-State: AN3rC/65z6f5XeGHmEz1+O9AimoaitFGaFhaLJhuO9HH/vxVIM/QuvW9 1VLF4VBULry9rA== X-Received: by 10.28.19.7 with SMTP id 7mr233886wmt.68.1492803462449; Fri, 21 Apr 2017 12:37:42 -0700 (PDT) Received: from [192.168.0.101] ([37.189.166.198]) by smtp.gmail.com with ESMTPSA id k90sm3357269wmi.26.2017.04.21.12.37.41 (version=TLS1_2 cipher=ECDHE-RSA-AES128-GCM-SHA256 bits=128/128); Fri, 21 Apr 2017 12:37:41 -0700 (PDT) Subject: Re: [Patch] New gdbarch method "dwarf_cfa_op" and migrate SPARC to it To: Jiong Wang , Ivo Raisr , GDB References: <1d0d97ca-b503-0303-5efc-600db754bd27@foss.arm.com> From: Pedro Alves Message-ID: Date: Fri, 21 Apr 2017 19:37:00 -0000 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:45.0) Gecko/20100101 Thunderbird/45.4.0 MIME-Version: 1.0 In-Reply-To: <1d0d97ca-b503-0303-5efc-600db754bd27@foss.arm.com> Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: 7bit X-IsSubscribed: yes X-SW-Source: 2017-04/txt/msg00618.txt.bz2 On 04/21/2017 03:56 PM, Jiong Wang wrote: > Hi Ivo, > > Thanks very much for testing this on SPARC platform. > > What's really reused is the DWARF CFA number 0x2d behind > DW_CFA_GNU_window_save. It is in vendor extension space ( > DW_CFA_lo_user.. DW_CFA_hi_user) so the semantics depends on vendor > interpreation. Maybe the commit log could/should be simplified, because I was confused too. Doesn't the Aarch64 version of the opcode have its own name, like DW_CFA_GNU_Aarch64_whatever, even if it reuses the opcode number? I think that would help a lot going forward if it had one. E.g., it'd avoid confusion, allow for easier searching, etc. On the patch, I think it would be better if in execute_cfa_program: > case DW_CFA_GNU_window_save: > - /* This is SPARC-specific code, and contains hard-coded > - constants for the register numbering scheme used by you removed "case CFA_GNU_window_save:" too, and moved the gdbarch_dwarf_cfa_op call to the default case. Then make the hook return a boolean indication about whether it /recognized the opcode, and make the caller throw an error if the hook returns false. And make that error call be a regular "error()" call instead of the current internal_error call. The internal error is bogus here because we reach that with an unrecognized opcode that comes from a binary, i.e., input, not a gdb bug. Replace the gdb_assert in sparc_dwarf_cfa_op by returning false for unrecognized opcodes, and likewise change default_dwarf_cfa_op to return false instead of calling error itself. Add a comment at the hook call site about letting the backend handle vendor-specific opcodes, and generalize a bit the comment describing the hook in gdbarch.sh in that direction as well. Maybe rename to hook from dwarf_cfa_op to something like: handle_dwarf_cfa_op handle_dwarf_cfa_vendor_op execute_dwarf_cfa_op execute_dwarf_cfa_vendor_op too, while at it? Thanks, Pedro Alves