From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 49282 invoked by alias); 3 Nov 2016 12:39:18 -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 49272 invoked by uid 89); 3 Nov 2016 12:39:17 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-0.2 required=5.0 tests=AWL,BAYES_05,FREEMAIL_FROM,RCVD_IN_DNSWL_NONE,RCVD_IN_SORBS_SPAM,SPF_PASS autolearn=no version=3.3.2 spammy=albert, todd, Albert, Todd X-HELO: mail-pf0-f195.google.com Received: from mail-pf0-f195.google.com (HELO mail-pf0-f195.google.com) (209.85.192.195) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with ESMTP; Thu, 03 Nov 2016 12:39:07 +0000 Received: by mail-pf0-f195.google.com with SMTP id i88so4776728pfk.2 for ; Thu, 03 Nov 2016 05:39:07 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20130820; h=x-gm-message-state:from:to:cc:subject:references:date:in-reply-to :message-id:user-agent:mime-version:content-transfer-encoding; bh=DSmDaOaH9Ry8ai9qrX7nqrT5Hxbi2teH7czTJeCZNoI=; b=cM+TMTPljiv+9D5iEhmsLBuL5BXhF9KJno4Vq5wYlKc4qYBHx44+roOxoAd28uNhPU je0HaNF6wmXCzG0zNq1ziO45X69zUR4G6tAKyW5YWrJWat1o1XQ8AP4LyllslXehVNzH bBUjSfMAkBSprLqm+Dt0wVaVw3FEtIIip/M0SeQI+7s3DupXJzJOr9b79eCACBLFrSTy nDl9yC1AlSvpbOnClirGQwBhq6F2XKqvKgKSQh4cL+xxcLFUov2SkD9+NZ8B+y/0K8tx Adu13tPjHJW/l+gejVLR6IODxFVnJA8WPu5rNKWscf2HVg8QyUkyRr7E1a9c8MylToaF b8gw== X-Gm-Message-State: ABUngvfMVV+VeeY5s2G3bVlHcFgg8BcgNNTyHMqs/W3gx9h7/bUHSHkIV1S+xXXekKsdhQ== X-Received: by 10.99.42.80 with SMTP id q77mr13762378pgq.3.1478176745004; Thu, 03 Nov 2016 05:39:05 -0700 (PDT) Received: from E107787-LIN (gcc1-power7.osuosl.org. [140.211.15.137]) by smtp.gmail.com with ESMTPSA id yc7sm12618736pab.24.2016.11.03.05.39.02 (version=TLS1_2 cipher=AES128-SHA bits=128/128); Thu, 03 Nov 2016 05:39:04 -0700 (PDT) From: Yao Qi To: Palmer Dabbelt Cc: Andrew Waterman , gdb-patches@sourceware.org, amodra@gmail.com Subject: Re: [PATCH 1/2] RISC-V GDB Port References: <1477179037-32066-1-git-send-email-palmer@dabbelt.com> <1477179592-32501-1-git-send-email-palmer@dabbelt.com> <1477179592-32501-2-git-send-email-palmer@dabbelt.com> Date: Thu, 03 Nov 2016 12:39:00 -0000 In-Reply-To: <1477179592-32501-2-git-send-email-palmer@dabbelt.com> (Palmer Dabbelt's message of "Sat, 22 Oct 2016 16:39:51 -0700") Message-ID: <86y4107n6q.fsf@gmail.com> User-Agent: Gnus/5.13 (Gnus v5.13) Emacs/24.3 (gnu/linux) MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: quoted-printable X-IsSubscribed: yes X-SW-Source: 2016-11/txt/msg00047.txt.bz2 Palmer Dabbelt writes: Hi Palmer, you need a ChangeLog entry, and a news entry for this new gdb port. There are some code style issues, please look at https://sourceware.org/gdb/wiki/ContributionChecklist > diff --git a/gdb/Makefile.in b/gdb/Makefile.in > index 2c88434..e4e497b 100644 > --- a/gdb/Makefile.in > +++ b/gdb/Makefile.in > @@ -691,7 +691,7 @@ ALL_TARGET_OBS =3D \ > nios2-tdep.o nios2-linux-tdep.o \ > nto-tdep.o \ > ppc-linux-tdep.o ppcfbsd-tdep.o ppcnbsd-tdep.o ppcobsd-tdep.o \ > - ppc-sysv-tdep.o ppc64-tdep.o rl78-tdep.o \ > + ppc-sysv-tdep.o ppc64-tdep.o riscv-tdep.o rl78-tdep.o \ Miss riscv-linux-tdep.o? > rs6000-aix-tdep.o rs6000-tdep.o solib-aix.o ppc-ravenscar-thread.o \ > rs6000-lynx178-tdep.o \ > rx-tdep.o \ > @@ -946,7 +946,7 @@ sparc64-tdep.h ppcobsd-tdep.h \ > coff-pe-read.h parser-defs.h gdb_ptrace.h mips-linux-tdep.h \ > m68k-tdep.h spu-tdep.h environ.h amd64-tdep.h \ > doublest.h regset.h hppa-tdep.h ppc-linux-tdep.h ppc64-tdep.h \ > -rs6000-tdep.h rs6000-aix-tdep.h \ > +riscv-tdep.h rs6000-tdep.h rs6000-aix-tdep.h \ > common/gdb_locale.h arch-utils.h trad-frame.h gnu-nat.h \ > language.h nbsd-tdep.h solib-svr4.h \ > macroexp.h ui-file.h regcache.h tracepoint.h tracefile.h i386-tdep.h \ > @@ -1734,6 +1734,7 @@ ALLDEPFILES =3D \ > ravenscar-thread.c \ > remote-sim.c \ > dcache.c \ > + riscv-tdep.c \ Miss riscv-linux-tdep.c? > rl78-tdep.c \ > rs6000-nat.c rs6000-tdep.c solib-aix.c ppc-ravenscar-thread.c \ > rs6000-lynx178-tdep.c \ > diff --git a/gdb/config/riscv/linux.mh b/gdb/config/riscv/linux.mh > new file mode 100644 > index 0000000..f68f371 > --- /dev/null > +++ b/gdb/config/riscv/linux.mh > @@ -0,0 +1,30 @@ > +# Host: RISC-V based machine running GNU/Linux > +# > +# Copyright (C) 2015 Free Software Foundation, Inc. 2016 > +# Contributed by Albert Ou . > +# Albert Ou doesn't have FSF copyright assignment. > --- a/gdb/configure.ac > +++ b/gdb/configure.ac > @@ -515,6 +515,9 @@ esac > # We might need to link with -lm; most simulators need it. > AC_CHECK_LIB(m, main) >=20=20 > +# The RISC-V simulator needs clock_gettime() > +AC_SEARCH_LIBS([clock_gettime], [rt]) > + I don't know much in sim configure, but this is about sim, so I think this should be moved to sim/riscv/configure.ac. > diff --git a/gdb/infcmd.c b/gdb/infcmd.c > index 8e34b7e..4457304 100644 > --- a/gdb/infcmd.c > +++ b/gdb/infcmd.c > @@ -3447,8 +3447,9 @@ otherwise all the threads in the program are stoppe= d. To \n\ > interrupt all running threads in non-stop mode, use the -a option.")); >=20=20 > c =3D add_info ("registers", nofp_registers_info, _("\ > -List of integer registers and their contents, for selected stack frame.\= n\ > -Register name as argument means describe only that register.")); > +Display registers and their contents, for the selected stack frame.\n\ > +Usage: info registers [all|register|register group] ...\n\ > +By default, the general register group will be shown.")); Please split this change out this patch. Put it into a separated patch, and give the reason why do you change that. > diff --git a/gdb/riscv-linux-tdep.c b/gdb/riscv-linux-tdep.c > new file mode 100644 > index 0000000..b66860c > --- /dev/null > +++ b/gdb/riscv-linux-tdep.c > @@ -0,0 +1,83 @@ > +/* Target-dependent code for GNU/Linux RISC-V. > + > + Copyright (C) 2015 Free Software Foundation, Inc. > + Contributed by Albert Ou . > + > + 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 "gdbarch.h" > +#include "osabi.h" > +#include "linux-tdep.h" > +#include "solib-svr4.h" > +#include "glibc-tdep.h" > + > +#include "riscv-tdep.h" > +#include "riscv-linux-tdep.h" > + > +#include "regcache.h" > +#include "regset.h" > + > +static const struct regcache_map_entry riscv_linux_gregmap[] =3D > +{ > + { 1, RISCV_PC_REGNUM, 0 }, > + { 31, RISCV_RA_REGNUM, 0 }, /* x1 to x31 */ > + { 0 } > +}; > + > +const struct regset riscv_linux_gregset =3D > +{ > + riscv_linux_gregmap, regcache_supply_regset, regcache_collect_regset > +}; > + All gdbarch methods are should be documented like this, /* Implement the iterate_over_regset_sections gdbarch method. */ > +static void > +riscv_linux_iterate_over_regset_sections (struct gdbarch *gdbarch, > + iterate_over_regset_sections_cb *cb, > + void *cb_data, > + const struct regcache *regcache) > +{ > + cb (".reg", (RISCV_NGREG * riscv_isa_regsize (gdbarch)), > + &riscv_linux_gregset, NULL, cb_data); > +} > + > +static void > +riscv_linux_init_abi (struct gdbarch_info info, struct gdbarch *gdbarch) > +{ > + linux_init_abi (info, gdbarch); > + > + /* GNU/Linux uses SVR4-style shared libraries. */ > + /* FIXME: This '0' should actually be a check to see if we're on > + rv32, but I can't figure out how to hook that up (it's in > + gdbarch_tdep, which we don't have here). */ > + set_solib_svr4_fetch_link_map_offsets > + (gdbarch, (0) ? > + svr4_ilp32_fetch_link_map_offsets : > + svr4_lp64_fetch_link_map_offsets); gdbarch_tdep (gdbarch)->riscv_abi can tell you rv32 or rv64 is used. > --- /dev/null > +++ b/gdb/riscv-tdep.c > @@ -0,0 +1,1292 @@ > +/* Target-dependent code for the RISC-V architecture, for GDB. > + > + Copyright (C) 1988-2015 Free Software Foundation, Inc. > + > + Contributed by Alessandro Forin(af@cs.cmu.edu) at CMU > + and by Per Bothner(bothner@cs.wisc.edu) at U.Wisconsin > + and by Todd Snyder > + and by Mike Frysinger . I don't find Todd Snyder's FSF copyright assignment. > + > + 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 "frame.h" > +#include "inferior.h" > +#include "symtab.h" > +#include "value.h" > +#include "gdbcmd.h" > +#include "language.h" > +#include "gdbcore.h" > +#include "symfile.h" > +#include "objfiles.h" > +#include "gdbtypes.h" > +#include "target.h" > +#include "arch-utils.h" > +#include "regcache.h" > +#include "osabi.h" > +#include "riscv-tdep.h" > +#include "block.h" > +#include "reggroups.h" > +#include "opcode/riscv.h" > +#include "elf/riscv.h" > +#include "elf-bfd.h" > +#include "symcat.h" > +#include "sim-regno.h" > +#include "gdb/sim-riscv.h" > +#include "dis-asm.h" > +#include "frame-unwind.h" > +#include "frame-base.h" > +#include "trad-frame.h" > +#include "infcall.h" > +#include "floatformat.h" > +#include "remote.h" > +#include "target-descriptions.h" > +#include "dwarf2-frame.h" > +#include "user-regs.h" > +#include "valprint.h" > +#include "opcode/riscv-opc.h" > +#include > + > + > +static void > +riscv_remote_breakpoint_from_pc (struct gdbarch *gdbarch, CORE_ADDR *pcp= tr, int *kindptr) > +{ > + struct gdbarch_tdep *tdep =3D gdbarch_tdep (gdbarch); > + > + riscv_breakpoint_from_pc (gdbarch, pcptr, kindptr); > +} > + remote_breakpoint_from_pc is removed in my patches https://sourceware.org/ml/gdb-patches/2016-11/msg00031.html They are not committed yet. It would be good if you can rebase your patches on top of mine when my patches are committed. > +static struct value * > +value_of_riscv_user_reg (struct frame_info *frame, const void *baton) > +{ > + const int *reg_p =3D (const int *)baton; > + > + return value_of_register (*reg_p, frame); > +} > + > +static const char * > +register_name (struct gdbarch *gdbarch, > + int regnum, > + int prefer_alias) Code style issue, only one space is needed between "int" and "regnum". Many instances of such problem around your patch. > +{ > + int i; > + static char buf[20]; > + > + if (tdesc_has_registers (gdbarch_target_desc (gdbarch))) > + return tdesc_register_name (gdbarch, regnum); I don't think riscv port has target description yet. > + > +static void > +riscv_extract_return_value (struct type *type, > + struct regcache *regs, > + gdb_byte *dst, > + int regnum) You need some comments on this function. > +{ > + struct gdbarch *gdbarch =3D get_regcache_arch (regs); > + enum bfd_endian byte_order =3D gdbarch_byte_order (gdbarch); > + int regsize =3D riscv_isa_regsize (gdbarch); > + bfd_byte *valbuf =3D dst; > + int len =3D TYPE_LENGTH (type); > + ULONGEST tmp; > + > + gdb_assert (len <=3D 2 * regsize); > + > + while (len > 0) > + { > + regcache_cooked_read_unsigned (regs, regnum++, &tmp); > + store_unsigned_integer (valbuf, std::min (regsize, len), byte_orde= r, tmp); This line is too long. > + len -=3D regsize; > + valbuf +=3D regsize; > + } > +} > + /* Implement the return_value gdbarch method. */ > + > +static enum return_value_convention > +riscv_return_value (struct gdbarch *gdbarch, > + struct value *function, > + struct type *type, > + struct regcache *regcache, > + gdb_byte *readbuf, > + const gdb_byte *writebuf) > +{ > + enum type_code rv_type =3D TYPE_CODE (type); > + unsigned int rv_size =3D TYPE_LENGTH (type); > + int fp, regnum; > + ULONGEST tmp; > + > + /* Paragraph on return values taken from RISC-V specification (post v2= .0): > + > + Values are returned from functions in integer registers a0 and a1 a= nd > + floating-point registers fa0 and fa1. Floating-point values are re= turned > + in floating-point registers only if they are primitives or members = of a > + struct consisting of only one or two floating-point values. Other = return > + values that fit into two pointer-words are returned in a0 and a1. = Larger > + return values are passed entirely in memory; the caller allocates t= his > + memory region and passes a pointer to it as an implicit first param= eter to > + the callee. */ > + Nit: some lines are too long. > + /* Deal with struct/unions first that are passed via memory. */ > + if (rv_size > 2 * riscv_isa_regsize (gdbarch)) > + { > + if (readbuf || writebuf) > + regcache_cooked_read_unsigned (regcache, RISCV_A0_REGNUM, &tmp); > + if (readbuf) > + read_memory (tmp, readbuf, rv_size); > + if (writebuf) > + write_memory (tmp, writebuf, rv_size); > + return RETURN_VALUE_ABI_RETURNS_ADDRESS; > + } > + > + /* Are we dealing with a floating point value? */ > + fp =3D 0; > + if (rv_type =3D=3D TYPE_CODE_FLT) > + fp =3D 1; > + else if (rv_type =3D=3D TYPE_CODE_STRUCT || rv_type =3D=3D TYPE_CODE_U= NION) > + { > + unsigned int rv_fields =3D TYPE_NFIELDS (type); > + > + if (rv_fields =3D=3D 1) > + { > + struct type *fieldtype =3D TYPE_FIELD_TYPE (type, 0); > + if (TYPE_CODE (check_typedef (fieldtype)) =3D=3D TYPE_CODE_FLT) > + fp =3D 1; > + } > + else if (rv_fields =3D=3D 2) > + { > + struct type *fieldtype0 =3D TYPE_FIELD_TYPE (type, 0); > + struct type *fieldtype1 =3D TYPE_FIELD_TYPE (type, 1); > + > + if (TYPE_CODE (check_typedef (fieldtype0)) =3D=3D TYPE_CODE_FLT > + && TYPE_CODE (check_typedef (fieldtype1)) =3D=3D TYPE_CODE_FLT) > + fp =3D 1; > + } > + } > + > + /* Handle return value in a register. */ > + regnum =3D fp ? RISCV_FA0_REGNUM : RISCV_A0_REGNUM; > + > + if (readbuf) > + riscv_extract_return_value (type, regcache, readbuf, regnum); > + > + if (writebuf) > + riscv_store_return_value (type, regcache, writebuf, regnum); > + > + return RETURN_VALUE_REGISTER_CONVENTION; > +} > + /* Implement the XXX gdbarch method. */ > +static enum register_status > +riscv_pseudo_register_read (struct gdbarch *gdbarch, > + struct regcache *regcache, > + int regnum, > + gdb_byte *buf) Code style issue. > +{ > + return regcache_raw_read (regcache, regnum, buf); > +} > + > + > +static struct type * > +riscv_register_type (struct gdbarch *gdbarch, > + int regnum) > +{ > + int regsize =3D riscv_isa_regsize (gdbarch); > + > + if (regnum < RISCV_FIRST_FP_REGNUM) > + { > + int_regsizes: > + switch (regsize) > + { > + case 4: > + return builtin_type (gdbarch)->builtin_int32; > + case 8: > + return builtin_type (gdbarch)->builtin_int64; > + case 16: > + return builtin_type (gdbarch)->builtin_int128; > + default: > + internal_error (__FILE__, __LINE__, > + _("unknown isa regsize %i"), regsize); > + } > + } > + else if (regnum <=3D RISCV_LAST_FP_REGNUM) > + { > + switch (regsize) > + { > + case 4: > + return builtin_type (gdbarch)->builtin_float; > + case 8: > + case 16: > + return builtin_type (gdbarch)->builtin_double; return builtin_long_double in case regsize is 16? > + default: > + internal_error (__FILE__, __LINE__, > + _("unknown isa regsize %i"), regsize); > + } > + } > + else if (regnum =3D=3D RISCV_PRIV_REGNUM) > + { > + return builtin_type (gdbarch)->builtin_int8; > + } > + else > + { > + if (regnum =3D=3D RISCV_CSR_FFLAGS_REGNUM > + || regnum =3D=3D RISCV_CSR_FRM_REGNUM > + || regnum =3D=3D RISCV_CSR_FCSR_REGNUM) > + return builtin_type (gdbarch)->builtin_int32; > + > + goto int_regsizes; goto is not necessarily needed. Please remove it. > + } > +} > + > +/* TODO: Replace all this with tdesc XML files. */ > +static void > +riscv_read_fp_register_single (struct frame_info *frame, int regno, > + gdb_byte *rare_buffer) > +{ > + struct gdbarch *gdbarch =3D get_frame_arch (frame); > + int raw_size =3D register_size (gdbarch, regno); > + gdb_byte *raw_buffer =3D (gdb_byte *) alloca (raw_size); > + > + if (!deprecated_frame_register_read (frame, regno, raw_buffer)) Use get_frame_register_value, your code can be simpler. > + error (_("can't read register %d (%s)"), regno, > + gdbarch_register_name (gdbarch, regno)); > + > + if (raw_size =3D=3D 8) > + { > + int offset; > + > + if (gdbarch_byte_order (gdbarch) =3D=3D BFD_ENDIAN_BIG) > + offset =3D 4; > + else > + offset =3D 0; > + > + memcpy (rare_buffer, raw_buffer + offset, 4); > + } > + else > + memcpy (rare_buffer, raw_buffer, 4); > +} > + > +static void > +riscv_read_fp_register_double (struct frame_info *frame, int regno, > + gdb_byte *rare_buffer) > +{ > + struct gdbarch *gdbarch =3D get_frame_arch (frame); > + int raw_size =3D register_size (gdbarch, regno); > + > + if (raw_size =3D=3D 8) > + { > + if (!deprecated_frame_register_read (frame, regno, rare_buffer)) > + error (_("can't read register %d (%s)"), regno, > + gdbarch_register_name (gdbarch, regno)); > + } > + else > + internal_error (__FILE__, __LINE__, > + _("%s: size says 32-bits, read is 64-bits."), __func__); > +} > + > +static void > +riscv_print_fp_register (struct ui_file *file, struct frame_info *frame, > + int regnum) > +{ > + struct gdbarch *gdbarch =3D get_frame_arch (frame); > + gdb_byte *raw_buffer; > + struct value_print_options opts; > + double val; > + int inv; > + const char *regname; > + > + raw_buffer =3D (gdb_byte *) alloca (2 * register_size (gdbarch, RISCV_= FIRST_FP_REGNUM)); > + This line is too long. > + fprintf_filtered (file, "%-15s", gdbarch_register_name (gdbarch, regnu= m)); > + > + if (register_size (gdbarch, regnum) =3D=3D 4) > + { > + riscv_read_fp_register_single (frame, regnum, raw_buffer); > + val =3D unpack_double (builtin_type (gdbarch)->builtin_float, raw_= buffer, > + &inv); > + > + get_formatted_print_options (&opts, 'x'); > + print_scalar_formatted (raw_buffer, > + builtin_type (gdbarch)->builtin_float, > + &opts, 'w', file); > + > + if (!inv) > + fprintf_filtered (file, "\t%-17.9g", val); > + } > + else > + { > + riscv_read_fp_register_double (frame, regnum, raw_buffer); > + val =3D unpack_double (builtin_type (gdbarch)->builtin_double, raw= _buffer, > + &inv); > + > + get_formatted_print_options (&opts, 'x'); > + print_scalar_formatted (raw_buffer, > + builtin_type (gdbarch)->builtin_double, > + &opts, 'g', file); > + > + if (!inv) > + fprintf_filtered (file, "\t%-24.17g", val); > + } > + > + if (inv) > + fprintf_filtered (file, "\t"); > +} > + > + > +static ULONGEST > +riscv_fetch_instruction (struct gdbarch *gdbarch, CORE_ADDR addr) > +{ > + enum bfd_endian byte_order =3D gdbarch_byte_order (gdbarch); gdbarch_byte_order_for_code > + > +static void > +set_reg_offset (struct gdbarch *gdbarch, struct riscv_frame_cache *this_= cache, > + int regnum, CORE_ADDR offset) > +{ > + if (this_cache !=3D NULL && this_cache->saved_regs[regnum].addr =3D=3D= -1) > + this_cache->saved_regs[regnum].addr =3D offset; > +} > + > +static void > +reset_saved_regs (struct gdbarch *gdbarch, struct riscv_frame_cache *thi= s_cache) > +{ > + const int num_regs =3D gdbarch_num_regs (gdbarch); > + int i; > + > + if (this_cache =3D=3D NULL || this_cache->saved_regs =3D=3D NULL) > + return; > + > + for (i =3D 0; i < num_regs; ++i) > + this_cache->saved_regs[i].addr =3D -1; IIRC, .addr's type is CORE_ADDR, which is unsigned. Don't assign -1 to a unsigned type. > +} > + > +static CORE_ADDR > +riscv_scan_prologue (struct gdbarch *gdbarch, > + CORE_ADDR start_pc, CORE_ADDR limit_pc, > + struct frame_info *this_frame, > + struct riscv_frame_cache *this_cache) > +{ > + CORE_ADDR cur_pc; > + CORE_ADDR frame_addr =3D 0; > + CORE_ADDR sp; > + long frame_offset; > + int frame_reg =3D RISCV_SP_REGNUM; > + > + CORE_ADDR end_prologue_addr =3D 0; > + int seen_sp_adjust =3D 0; > + int load_immediate_bytes =3D 0; > + > + /* Can be called when there's no process, and hence when there's no TH= IS_FRAME. */ > + if (this_frame !=3D NULL) > + sp =3D get_frame_register_signed (this_frame, RISCV_SP_REGNUM); > + else > + sp =3D 0; > + > + if (limit_pc > start_pc + 200) > + limit_pc =3D start_pc + 200; > + > + restart: > + > + frame_offset =3D 0; > + /* TODO: Handle compressed extensions. */ > + for (cur_pc =3D start_pc; cur_pc < limit_pc; cur_pc +=3D 4) > + { > + unsigned long inst, opcode; > + int reg, rs1, imm12, rs2, offset12, funct3; > + > + /* Fetch the instruction. */ > + inst =3D (unsigned long) riscv_fetch_instruction (gdbarch, cur_pc); What if (unsigned long) is 4-bytes long, but riscv instruction is 8-bytes long? > + opcode =3D inst & 0x7F; > + reg =3D (inst >> 7) & 0x1F; > + rs1 =3D (inst >> 15) & 0x1F; > + imm12 =3D (inst >> 20) & 0xFFF; > + rs2 =3D (inst >> 20) & 0x1F; > + offset12 =3D (((inst >> 25) & 0x7F) << 5) + ((inst >> 7) & 0x1F); > + funct3 =3D (inst >> 12) & 0x7; > + > + /* Look for common stack adjustment insns. */ > + if ((opcode =3D=3D 0x13 || opcode =3D=3D 0x1B) && reg =3D=3D RISCV= _SP_REGNUM > + && rs1 =3D=3D RISCV_SP_REGNUM) Please use macros defined in include/opcode/riscv-opc.h, then the code is much more readable. > +static CORE_ADDR > +riscv_push_dummy_call (struct gdbarch *gdbarch, > + struct value *function, > + struct regcache *regcache, > + CORE_ADDR bp_addr, > + int nargs, > + struct value **args, > + CORE_ADDR sp, > + int struct_return, > + CORE_ADDR struct_addr) > +{ > + enum bfd_endian byte_order =3D gdbarch_byte_order (gdbarch); > + struct gdbarch_tdep *tdep =3D gdbarch_tdep (gdbarch); > + gdb_byte buf[4]; > + int i; > + CORE_ADDR func_addr =3D find_function_addr (function, NULL); > + > + /* Push excess arguments in reverse order. */ > + > + for (i =3D nargs; i >=3D 8; --i) > + { > + struct type *value_type =3D value_enclosing_type (args[i]); > + int container_len =3D align_up (TYPE_LENGTH (value_type), 3); > + > + sp -=3D container_len; > + write_memory (sp, value_contents_writeable (args[i]), container_le= n); > + } > + > + /* Initialize argument registers. */ > + > + for (i =3D 0; i < nargs && i < 8; ++i) > + { > + struct type *value_type =3D value_enclosing_type (args[i]); > + const gdb_byte *arg_bits =3D value_contents_all (args[i]); > + int regnum =3D TYPE_CODE (value_type) =3D=3D TYPE_CODE_FLT ? > + RISCV_FA0_REGNUM : RISCV_A0_REGNUM; > + code style issue, https://www.gnu.org/prep/standards/standards.html#Formatting int regnum =3D (TYPE_CODE (value_type) =3D=3D TYPE_CODE_FLT ? RISCV_FA0_REGNUM : RISCV_A0_REGNUM); > + > +static struct gdbarch * > +riscv_gdbarch_init (struct gdbarch_info info, > + struct gdbarch_list *arches) > +{ > + struct gdbarch *gdbarch; > + struct gdbarch_tdep *tdep; > + const struct bfd_arch_info *binfo =3D info.bfd_arch_info; > + > + int abi, i; > + > + /* For now, base the abi on the elf class. */ > + /* Allow the ELF class to override the register size. Ideally the targ= et > + * (OpenOCD/spike/...) would communicate the register size to gdb inst= ead. */ > + abi =3D RISCV_ABI_FLAG_RV32I; > + if (info.abfd && bfd_get_flavour (info.abfd) =3D=3D bfd_target_elf_fla= vour) > + { > + unsigned char eclass =3D elf_elfheader (info.abfd)->e_ident[EI_CLA= SS]; > + > + if (eclass =3D=3D ELFCLASS32) > + abi =3D RISCV_ABI_FLAG_RV32I; > + else if (eclass =3D=3D ELFCLASS64) > + abi =3D RISCV_ABI_FLAG_RV64I; > + else > + internal_error (__FILE__, __LINE__, _("unknown ELF header class = %d"), eclass); > + } > + else > + { > + if (binfo->bits_per_word =3D=3D 32) > + abi =3D RISCV_ABI_FLAG_RV32I; > + else if (binfo->bits_per_word =3D=3D 64) > + abi =3D RISCV_ABI_FLAG_RV64I; > + else > + internal_error (__FILE__, __LINE__, _("unknown bits_per_word %d"= ), > + binfo->bits_per_word); > + } > + > + /* Find a candidate among the list of pre-declared architectures. */ > + for (arches =3D gdbarch_list_lookup_by_info (arches, &info); > + arches !=3D NULL; > + arches =3D gdbarch_list_lookup_by_info (arches->next, &info)) > + if (gdbarch_tdep (arches->gdbarch)->riscv_abi =3D=3D abi) > + return arches->gdbarch; > + > + /* None found, so create a new architecture from the information provi= ded. > + Can't initialize all the target dependencies until we actually know= which > + target we are talking to, but put in some defaults for now. */ > + > + tdep =3D (struct gdbarch_tdep *) xmalloc (sizeof *tdep); > + gdbarch =3D gdbarch_alloc (&info, tdep); > + > + tdep->riscv_abi =3D abi; > + switch (abi) > + { > + case RISCV_ABI_FLAG_RV32I: > + tdep->register_size =3D 4; > + break; > + case RISCV_ABI_FLAG_RV64I: > + tdep->register_size =3D 8; Since you've already had riscv_abi field in tdep, you can determine the registers size on the fly. Then, field register_size is not necessary to me. > + > + /* Information about the target architecture. */ > + set_gdbarch_return_value (gdbarch, riscv_return_value); > + set_gdbarch_breakpoint_from_pc (gdbarch, riscv_breakpoint_from_pc); breakpoint_from_pc will be replaced by two new gdbarch methods, added in my patches https://sourceware.org/ml/gdb-patches/2016-11/msg00031.html > + set_gdbarch_remote_breakpoint_from_pc (gdbarch, riscv_remote_breakpoin= t_from_pc); > + > + /* Check any target description for validity. */ > + if (tdesc_has_registers (info.target_desc)) > + { We don't have riscv target description yet. > + const struct tdesc_feature *feature; > + struct tdesc_arch_data *tdesc_data; > + int valid_p; > + > + feature =3D tdesc_find_feature (info.target_desc, "org.gnu.gdb.ris= cv.cpu"); > + if (feature =3D=3D NULL) > + goto no_tdata; > + > + tdesc_data =3D tdesc_data_alloc (); > + > + valid_p =3D 1; > + for (i =3D RISCV_ZERO_REGNUM; i <=3D RISCV_LAST_FP_REGNUM; ++i) > + valid_p &=3D tdesc_numbered_register (feature, tdesc_data, i, > + riscv_gdb_reg_names[i]); > + for (i =3D RISCV_FIRST_CSR_REGNUM; i <=3D RISCV_LAST_CSR_REGNUM; += +i) > + { > + char buf[20]; > + sprintf (buf, "csr%d", i - RISCV_FIRST_CSR_REGNUM); > + valid_p &=3D tdesc_numbered_register (feature, tdesc_data, i, = buf); > + } > + > + valid_p &=3D tdesc_numbered_register (feature, tdesc_data, i++, "p= riv"); > + > + if (!valid_p) > + tdesc_data_cleanup (tdesc_data); > + else > + tdesc_use_registers (gdbarch, info.target_desc, tdesc_data); > + } > + no_tdata: > + > + for (i =3D 0; i < ARRAY_SIZE (riscv_register_aliases); ++i) > + user_reg_add (gdbarch, riscv_register_aliases[i].name, > + value_of_riscv_user_reg, &riscv_register_aliases[i].regnum); > + > + return gdbarch; > +} > + > diff --git a/gdb/riscv-tdep.h b/gdb/riscv-tdep.h > new file mode 100644 > index 0000000..ef10209 > --- /dev/null > +++ b/gdb/riscv-tdep.h > @@ -0,0 +1,101 @@ > +/* Target-dependent header for the RISC-V architecture, for GDB, the GNU= Debugger. > + > + Copyright (C) 2002-2015 Free Software Foundation, Inc. > + > + Contributed by Alessandro Forin(af@cs.cmu.edu) at CMU > + and by Per Bothner(bothner@cs.wisc.edu) at U.Wisconsin > + and by Todd Snyder > + and by Mike Frysinger . > + > + 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 .= */ > + > +#ifndef RISCV_TDEP_H > +#define RISCV_TDEP_H > + > +struct gdbarch; > + > +/* All the official RISC-V ABIs. These mostly line up with mcpuid purel= y for > + convenience. */ > +#define RISCV_ABI_FLAG_RV32I (0x00000000) /* 32-bit Integer GPRs. */ > +#define RISCV_ABI_FLAG_RV32E (0x10000000) /* 32-bit Embedded Integer GPR= s. */ > +#define RISCV_ABI_FLAG_RV64I (0x40000000) /* 64-bit Integer GPRs. */ > +#define RISCV_ABI_FLAG_RV128I (0x80000000) /* 128-bit Integer GPRs. */ > +#define RISCV_ABI_FLAG_A (1 << 0) /* Atomics. */ > +#define RISCV_ABI_FLAG_B (1 << 1) /* Bit Manipulation. */ > +#define RISCV_ABI_FLAG_C (1 << 2) /* 16-bit Compressed Instructions. */ > +#define RISCV_ABI_FLAG_D (1 << 3) /* Double-Precision Floating-Point. */ > +#define RISCV_ABI_FLAG_E (1 << 4) /* Embedded base. */ > +#define RISCV_ABI_FLAG_F (1 << 5) /* Single-Precision Floating-Point. */ > +#define RISCV_ABI_FLAG_H (1 << 7) /* Hypervisor mode. */ > +#define RISCV_ABI_FLAG_I (1 << 8) /* Base integer. */ > +#define RISCV_ABI_FLAG_L (1 << 11) /* Decimal Floating-Point. */ > +#define RISCV_ABI_FLAG_M (1 << 12) /* Integer Multiply and Division. */ > +#define RISCV_ABI_FLAG_P (1 << 15) /* Packed-SIMD Extensions. */ > +#define RISCV_ABI_FLAG_Q (1 << 16) /* Quad-Precision Floating-Point. */ > +#define RISCV_ABI_FLAG_S (1 << 18) /* Supervisor mode. */ > +#define RISCV_ABI_FLAG_T (1 << 19) /* Transactional Memory. */ > +#define RISCV_ABI_FLAG_U (1 << 20) /* User mode. */ I don't find where are they used. Let us add them when they are really used somewhere in the code. > + > +#define IS_RV32I(x) (((x) & 0xF0000000) =3D=3D RISCV_ABI_FLAG_RV32I) > +#define IS_RV64I(x) (((x) & 0xF0000000) =3D=3D RISCV_ABI_FLAG_RV64I) > +#define IS_RV128I(x) (((x) & 0xF0000000) =3D=3D RISCV_ABI_FLAG_RV128I) > + > +#define HAS_FPU(x) ((x) & (RISCV_ABI_FLAG_D | RISCV_ABI_FLAG_F)) > + > +#endif /* RISCV_TDEP_H */ > diff --git a/include/gdb/sim-riscv.h b/include/gdb/sim-riscv.h > new file mode 100644 > index 0000000..932cf49 > --- /dev/null > +++ b/include/gdb/sim-riscv.h > @@ -0,0 +1,98 @@ > +/* This file defines the interface between the RISC-V simulator and GDB. > + > + Copyright (C) 2005-2015 Free Software Foundation, Inc. > + > + 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 .= */ > + > +/* Order has to match gdb riscv-tdep list. */ > +enum sim_riscv_regnum { I don't see how is this header file related to this patch? Probably this should be moved to sim patch. --=20 Yao (=E9=BD=90=E5=B0=A7)