From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from us-smtp-delivery-124.mimecast.com (us-smtp-delivery-124.mimecast.com [170.10.133.124]) by sourceware.org (Postfix) with ESMTPS id 211BB3858C60 for ; Mon, 10 Jan 2022 14:34:53 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.1 sourceware.org 211BB3858C60 Received: from mail-wm1-f72.google.com (mail-wm1-f72.google.com [209.85.128.72]) by relay.mimecast.com with ESMTP with STARTTLS (version=TLSv1.2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id us-mta-618-S6tJs4eHNIum6ok8FiVaIQ-1; Mon, 10 Jan 2022 09:34:51 -0500 X-MC-Unique: S6tJs4eHNIum6ok8FiVaIQ-1 Received: by mail-wm1-f72.google.com with SMTP id ay41-20020a05600c1e2900b00345a568e6b2so5098682wmb.4 for ; Mon, 10 Jan 2022 06:34:51 -0800 (PST) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=x-gm-message-state:date:from:to:cc:subject:message-id:references :mime-version:content-disposition:in-reply-to; bh=JpEVaMu3g+TwD+ITc/YCgA1yxUf4CPHVWKs11bC5P1Y=; b=QeU4g+KNT8Ttj9N/5PV+hjy+TxYlq0NzczzG6HMVUk934Hmcfzvjk1iAcW440/YK0G ECPt8fsmIFBo7nQPazbkHbcfHNK7w9E1uq1s+cxqOXjW08oDs8LiZKEsrFOmaYlmdo35 /VpFaf63sPc8Q2rNH/conSTdPTBvVHn4jfG2qlNi+6xRgc0LEZMlQ83F53lDT2FdD906 SJukPaZejsVY3kA18nwNDINkstZGNWO4nBX07vhFPaOAFmOxRQKXhqF8x3hyzdtaycCz BGOAy1nF1dvda9bbS9XoOcjHbIdk1jZLX931RseJKqyqEeRsmtruCSPwvwqluKK0maGj eejg== X-Gm-Message-State: AOAM532txh6RGEGKhcLiqyMqot1XMODBZTFXhMQv8C1OJIKUVmcCgO/p fN51COioVF/mv5mKLQYoyVuOPmLm4r6thoxnoBxVqwWMu6phyXydqTZYxL8TYj8ctphg6VKTq0u Mfm1ij2xksI9HuGId1USZAA== X-Received: by 2002:adf:db0c:: with SMTP id s12mr33624482wri.429.1641825290153; Mon, 10 Jan 2022 06:34:50 -0800 (PST) X-Google-Smtp-Source: ABdhPJySbbGSAYHnkz1x92JKQnOS+ThamdZvHbgyz8p1Gfcw4+mV5fm1c5OX4xxBKPMjSfQjIXWFVg== X-Received: by 2002:adf:db0c:: with SMTP id s12mr33624468wri.429.1641825289908; Mon, 10 Jan 2022 06:34:49 -0800 (PST) Received: from localhost (host109-154-163-67.range109-154.btcentralplus.com. [109.154.163.67]) by smtp.gmail.com with ESMTPSA id z22sm7546151wmp.40.2022.01.10.06.34.49 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Mon, 10 Jan 2022 06:34:49 -0800 (PST) Date: Mon, 10 Jan 2022 14:34:48 +0000 From: Andrew Burgess To: Tiezhu Yang Cc: gdb-patches@sourceware.org Subject: Re: [PATCH 3/6] gdb: LoongArch: Add initial linux target support Message-ID: <20220110143448.GT828155@redhat.com> References: <1639049761-5616-1-git-send-email-yangtiezhu@loongson.cn> <1639049761-5616-4-git-send-email-yangtiezhu@loongson.cn> MIME-Version: 1.0 In-Reply-To: <1639049761-5616-4-git-send-email-yangtiezhu@loongson.cn> X-Operating-System: Linux/5.8.18-100.fc31.x86_64 (x86_64) X-Uptime: 14:28:59 up 8 days, 23:23, X-Editor: GNU Emacs [ http://www.gnu.org/software/emacs ] X-Mimecast-Spam-Score: 0 X-Mimecast-Originator: redhat.com Content-Type: text/plain; charset=us-ascii Content-Disposition: inline X-Spam-Status: No, score=-12.2 required=5.0 tests=BAYES_00, DKIMWL_WL_HIGH, DKIM_SIGNED, DKIM_VALID, DKIM_VALID_AU, DKIM_VALID_EF, GIT_PATCH_0, KAM_SHORT, RCVD_IN_DNSWL_LOW, RCVD_IN_MSPIKE_H3, RCVD_IN_MSPIKE_WL, SPF_HELO_NONE, SPF_NONE, TXREP autolearn=ham autolearn_force=no version=3.4.4 X-Spam-Checker-Version: SpamAssassin 3.4.4 (2020-01-24) on server2.sourceware.org X-BeenThere: gdb-patches@sourceware.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: Gdb-patches mailing list List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Mon, 10 Jan 2022 14:34:54 -0000 * Tiezhu Yang [2021-12-09 19:35:58 +0800]: > This commit adds initial linux target support for LoongArch. > > Signed-off-by: Zhensong Liu > Signed-off-by: Qing zhang > Signed-off-by: Youling Tang > Signed-off-by: Tiezhu Yang Like Tom, I can't check the details of the actual implementation, but so long as the code is up to standard, I don't think it really matters, I'm sure you'll fix any bugs that come to light. I do have some coding standard issues that could do with being fixed though, notes inline below. > --- > gdb/loongarch-linux-tdep.c | 147 +++++++++++++++++++++++++++++++++++++++++++++ > 1 file changed, 147 insertions(+) > create mode 100644 gdb/loongarch-linux-tdep.c > > diff --git a/gdb/loongarch-linux-tdep.c b/gdb/loongarch-linux-tdep.c > new file mode 100644 > index 0000000..40b8008 > --- /dev/null > +++ b/gdb/loongarch-linux-tdep.c > @@ -0,0 +1,147 @@ > +/* Target-dependent code for GNU/Linux LoongArch. > + > + Copyright (C) 2021 Free Software Foundation, Inc. > + Contributed by Loongson Ltd. > + > + This file is part of GDB. > + > + This program is free software; you can redistribute it and/or modify > + it under the terms of the GNU General Public License as published by > + the Free Software Foundation; either version 3 of the License, or > + (at your option) any later version. > + > + This program is distributed in the hope that it will be useful, > + but WITHOUT ANY WARRANTY; without even the implied warranty of > + MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the > + GNU General Public License for more details. > + > + You should have received a copy of the GNU General Public License > + along with this program. If not, see . */ > + > +#include "defs.h" > +#include "glibc-tdep.h" > +#include "inferior.h" > +#include "linux-tdep.h" > +#include "loongarch-tdep.h" > +#include "solib-svr4.h" > +#include "target-descriptions.h" > + > +/* Unpack an elf_gregset_t into GDB's register cache. */ > + > +static void > +loongarch_supply_elf_gregset (const struct regset *r, > + struct regcache *regcache, int regno, > + const void *gprs, size_t len) > +{ > + loongarch_gdbarch_tdep *tdep = (loongarch_gdbarch_tdep *) gdbarch_tdep (regcache->arch ()); Long line, should be split at the '='. > + auto regs = tdep->regs; > + gdb_assert (0 <= regs.r && sizeof (elf_gregset_t) <= len); > + > + int regsize = register_size (regcache->arch (), regs.r); > + const gdb_byte *buf = NULL; Again s/NULL/nullptr/ throughout. > + > + if (regno == -1) > + { > + /* Set $r0 = 0. */ > + regcache->raw_supply_zeroed (regs.r); > + > + for (int i = 1; i < 32; i++) > + { > + buf = (const gdb_byte*)gprs + regsize * i; Should be a space after the cast, before gprs. In fact, I notice there's a lot of this formatting issue below, I'll not point them all out, but they all need fixing. > + regcache->raw_supply (regs.r + i, (const void *)buf); > + } > + > + /* Size base (pc) = regsize * regs.pc. */ > + buf = (const gdb_byte*)gprs + regsize * regs.pc; > + regcache->raw_supply (regs.pc, (const void *)buf); > + > + /* Size base (badvaddr) = regsize * regs.badvaddr. */ > + buf = (const gdb_byte*)gprs + regsize * regs.badvaddr; > + regcache->raw_supply (regs.badvaddr, (const void *)buf); > + } > + else if (regs.r == regno) > + regcache->raw_supply_zeroed (regno); > + else if ((regs.r < regno && regno < regs.r + 32) || > + (regs.pc == regno) || (regs.badvaddr == regno)) The '||' needs to be at the start of the following line, not the end of the previous line. > + { > + /* Offset offset (regno) = regsize * (regno - regs.r). */ > + buf = (const gdb_byte*)gprs + regsize * (regno - regs.r); > + regcache->raw_supply (regno, (const void *)buf); > + } > +} > + > +/* Pack the GDB's register cache value into an elf_gregset_t. */ > + > +static void > +loongarch_fill_elf_gregset (const struct regset *r, > + const struct regcache *regcache, int regno, > + void *gprs, size_t len) > +{ > + loongarch_gdbarch_tdep *tdep = (loongarch_gdbarch_tdep *) gdbarch_tdep (regcache->arch ()); Line length. > + auto regs = tdep->regs; > + gdb_assert (0 <= regs.r && sizeof (elf_gregset_t) <= len); > + int regsize = register_size (regcache->arch (), regs.r); > + gdb_byte *buf = NULL; > + > + if (regno == -1) > + { > + for (int i = 0; i < 32; i++) > + { > + buf = (gdb_byte *)gprs + regsize * i; > + regcache->raw_collect (regs.r + i, (void *)buf); > + } > + > + /* Size base (pc) = regsize * regs.pc. */ > + buf = (gdb_byte *)gprs + regsize * regs.pc; > + regcache->raw_collect (regs.pc, (void *)buf); > + > + /* Size base (badvaddr) = regsize * regs.badvaddr. */ > + buf = (gdb_byte *)gprs + regsize * regs.badvaddr; > + regcache->raw_collect (regs.badvaddr, (void *)buf); > + } > + else if ((regs.r <= regno && regno < regs.r + 32) || > + (regs.pc == regno) || (regs.badvaddr == regno)) > + { > + /* Offset offset (regno) = regsize * (regno - regs.r). */ > + buf = (gdb_byte *)gprs + regsize * (regno - regs.r); > + regcache->raw_collect (regno, (void *)buf); > + } > +} > + > +const struct regset loongarch_elf_gregset = > +{ > + NULL, > + loongarch_supply_elf_gregset, > + loongarch_fill_elf_gregset, > +}; Should have a header comment, even a simple one. > + > +static void > +loongarch_linux_init_abi (struct gdbarch_info info, struct gdbarch *gdbarch) Again, should have a header comment. > +{ > + linux_init_abi (info, gdbarch, 0); > + > + set_solib_svr4_fetch_link_map_offsets (gdbarch, loongarch_rlen(gdbarch) == 32 > + ? linux_ilp32_fetch_link_map_offsets > + : linux_lp64_fetch_link_map_offsets); > + > + /* GNU/Linux uses SVR4-style shared libraries. */ > + set_gdbarch_skip_trampoline_code (gdbarch, find_solib_trampoline_target); > + > + /* GNU/Linux uses the dynamic linker included in the GNU C Library. */ > + set_gdbarch_skip_solib_resolver (gdbarch, glibc_skip_solib_resolver); > + > + /* Enable TLS support. */ > + set_gdbarch_fetch_tls_load_module_address (gdbarch, svr4_fetch_objfile_link_map); > +} > + > +/* Initialize LoongArch Linux target support. */ > + > +void _initialize_loongarch_linux_tdep (); > +void > +_initialize_loongarch_linux_tdep () > +{ > + gdbarch_register_osabi (bfd_arch_loongarch, bfd_mach_loongarch32, > + GDB_OSABI_LINUX, loongarch_linux_init_abi); > + gdbarch_register_osabi (bfd_arch_loongarch, bfd_mach_loongarch64, > + GDB_OSABI_LINUX, loongarch_linux_init_abi); > +} > -- > 2.1.0 > > Thanks, Andrew