From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 116853 invoked by alias); 4 Apr 2017 21:17:52 -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 116794 invoked by uid 89); 4 Apr 2017 21:17:49 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-26.4 required=5.0 tests=BAYES_00,FREEMAIL_FROM,GIT_PATCH_0,GIT_PATCH_1,GIT_PATCH_2,GIT_PATCH_3,RCVD_IN_DNSWL_NONE,RCVD_IN_SORBS_SPAM,SPF_PASS autolearn=ham version=3.3.2 spammy=rise, capitalize, Smaller, reportedly X-HELO: mail-wm0-f67.google.com Received: from mail-wm0-f67.google.com (HELO mail-wm0-f67.google.com) (74.125.82.67) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with ESMTP; Tue, 04 Apr 2017 21:17:39 +0000 Received: by mail-wm0-f67.google.com with SMTP id z133so7929140wmb.2 for ; Tue, 04 Apr 2017 14:17:40 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:from:to:cc:subject:references:date:message-id :user-agent:mime-version:content-transfer-encoding; bh=3lA1QZ8DbS6GNm3ZGrcugffZhVpTyK+iAMA33JAlubg=; b=YROJh3KpetPEl2x4m4gkTE1OmnZSL7iVLZ9BYksegMABST1Qzw2kImCxKL24/RsFqH BguhSRpglqH9UU4RTA8ZplIEfLOdy4NKuznw267Jg/22eUDY52ObSqLH4xk/pGJM+NXZ Tt3xJfyGroQZhaJJWKyNK/N6Yo47VOE1lO+2tphMBuvwKiw8axvzQ12H2T26PSzz3niB y1iin9hLpTk+YcG/+pinwxZnG0zmcEg7gTr5JTOVramDB+mLJWXWFwQNT0h19oLhv5fM bKTCEy+Sgqz8IC5Rpno49b8cJZX3+nzDjMDuhHQRQJLx3T7W+7pqHTedF3tcMHaunmTd yMxg== X-Gm-Message-State: AFeK/H0S9N9xtn59yr3+g46J+OH+igCHC6IRY36vFe6TMAeavp9WlKM8 VI8b78oqqmwrUw== X-Received: by 10.28.73.197 with SMTP id w188mr16254161wma.46.1491340658037; Tue, 04 Apr 2017 14:17:38 -0700 (PDT) Received: from E107787-LIN ([194.214.185.158]) by smtp.gmail.com with ESMTPSA id c76sm19884376wme.23.2017.04.04.14.17.32 (version=TLS1_2 cipher=AES128-SHA bits=128/128); Tue, 04 Apr 2017 14:17:37 -0700 (PDT) From: Yao Qi To: Stafford Horne Cc: gdb-patches@sourceware.org, Franck Jullien , openrisc@lists.librecores.org Subject: Re: [PATCH v5 1/4] gdb: Add OpenRISC or1k and or1knd target support References: <61be7be503333904f9533549b0a809bed4066ac3.1489728533.git.shorne@gmail.com> Date: Tue, 04 Apr 2017 21:17:00 -0000 Message-ID: <86wpaz6ffa.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: 2017-04/txt/msg00071.txt.bz2 Stafford Horne writes: Hi Stafford, Have some cycles today, so I can review the patch. > +/* The target-dependent structure for gdbarch. */ > + > +struct gdbarch_tdep > +{ > + int bytes_per_word; > + int bytes_per_address; Don't need these two fields, we can get bfd_arch_info via gdbarch_bfd_arch_info (gdbarch), and access its fields to compute bytes_per_word and bytes_per_address. > + CGEN_CPU_DESC gdb_cgen_cpu_desc; > +}; > + > +/* Support functions for the architecture definition. */ > + > +/* Get an instruction from memory. */ > + > +static ULONGEST > +or1k_fetch_instruction (struct gdbarch *gdbarch, CORE_ADDR addr) > +{ > + enum bfd_endian byte_order =3D gdbarch_byte_order (gdbarch); > + gdb_byte buf[OR1K_INSTLEN]; > + > + if (target_read_memory (addr, buf, OR1K_INSTLEN)) { target_read_code, it uses cache, and helps on performance. > + memory_error (TARGET_XFER_E_IO, addr); > + } > + > + return extract_unsigned_integer (buf, OR1K_INSTLEN, byte_order); > +} > + > + > + /* Check we got something, and if so skip on. */ > + if (start_ptr =3D=3D end_ptr) > + throw_quit ("bitstring \"%s\" at offset %d has no length field.\n", > + format, i); s/throw_quit/error/ multiple instances of this. > +/* Analyze a l.addi instruction in form: l.addi rD,rA,I. This is used > + to parse add instructions during various prologue analysis routines. = */ > + Document the argument and return value. > +static int static bool > +or1k_analyse_l_addi (uint32_t inst, unsigned int *rd_ptr, > + unsigned int *ra_ptr, int *simm_ptr) > +{ > + /* Instruction fields */ > + uint32_t rd, ra, i; > + > + if (or1k_analyse_inst (inst, "10 0111 %5b %5b %16b", &rd, &ra, &i)) > + { > + /* Found it. Construct the result fields. */ > + *rd_ptr =3D (unsigned int) rd; > + *ra_ptr =3D (unsigned int) ra; > + *simm_ptr =3D (int) (((i & 0x8000) =3D=3D 0x8000) ? 0xffff0000 | i= : i); > + > + return 1; /* Success */ > + } > + else > + return 0; /* Failure */ > +} > + > +/* Functions defining the architecture. */ What is this? > + > +/* Implement the return_value gdbarch method. */ > + > +static enum return_value_convention > +or1k_return_value (struct gdbarch *gdbarch, struct value *functype, > + struct type *valtype, struct regcache *regcache, > + gdb_byte *readbuf, const gdb_byte *writebuf) > +{ > + enum bfd_endian byte_order =3D gdbarch_byte_order (gdbarch); > + enum type_code rv_type =3D TYPE_CODE (valtype); > + unsigned int rv_size =3D TYPE_LENGTH (valtype); > + int bpw =3D (gdbarch_tdep (gdbarch))->bytes_per_word; > + > + /* Deal with struct/union as addresses. If an array won't fit in a si= ngle > + register it is returned as address. Anything larger than 2 registe= rs needs > + to also be passed as address (matches gcc default_return_in_memory)= . */ > + if ((TYPE_CODE_STRUCT =3D=3D rv_type) || (TYPE_CODE_UNION =3D=3D rv_ty= pe) > + || ((TYPE_CODE_ARRAY =3D=3D rv_type) && (rv_size > bpw)) > + || (rv_size > 2 * bpw)) If I read the comment and code correctly, register size is "bytes_per_word", and bytes_per_address is added in gdbarch_tdep. Does this imply that register size may be different (in bytes) on different processors? > + { > + if (readbuf !=3D NULL) > + { > + ULONGEST tmp; > + > + regcache_cooked_read_unsigned (regcache, OR1K_RV_REGNUM, &tmp); > + read_memory (tmp, readbuf, rv_size); > + } > + if (writebuf !=3D NULL) > + { > + ULONGEST tmp; > + > + regcache_cooked_read_unsigned (regcache, OR1K_RV_REGNUM, &tmp); > + write_memory (tmp, writebuf, rv_size); > + } > + > + return RETURN_VALUE_ABI_RETURNS_ADDRESS; > + } > + > + if (rv_size <=3D bpw) > + { > + /* Up to one word scalars are returned in R11. */ > + if (readbuf !=3D NULL) > + { > + ULONGEST tmp; > + > + regcache_cooked_read_unsigned (regcache, OR1K_RV_REGNUM, &tmp); > + store_unsigned_integer (readbuf, rv_size, byte_order, tmp); > + > + } > + if (writebuf !=3D NULL) > + { > + gdb_byte buf[4]; Overflow if bpw is greater than 4? > + memset (buf, 0, sizeof (buf)); /* Zero pad if < bpw bytes. */ > + > + if (BFD_ENDIAN_BIG =3D=3D byte_order) > + memcpy (buf + sizeof (buf) - rv_size, writebuf, rv_size); > + else > + memcpy (buf, writebuf, rv_size); > + > + regcache_cooked_write (regcache, OR1K_RV_REGNUM, buf); > + } > + } > + else > + { > + /* 2 word scalars are returned in r11/r12 (with the MS word in r11= ). */ > + > +/* OR1K always uses a l.trap instruction for breakpoints. */ > + > +constexpr gdb_byte or1k_break_insn[] =3D {0x21, 0x00, 0x00, 0x01}; > + > +typedef BP_MANIPULATION (or1k_break_insn) or1k_breakpoint; > + > +/* Implement the single_step_through_delay gdbarch method. */ > + > +static int > +or1k_single_step_through_delay (struct gdbarch *gdbarch, > + struct frame_info *this_frame) > +{ > + ULONGEST val; > + CORE_ADDR ppc; > + CORE_ADDR npc; > + CGEN_FIELDS tmp_fields; > + const CGEN_INSN *insn; > + struct regcache *regcache =3D get_current_regcache (); > + struct gdbarch_tdep *tdep =3D gdbarch_tdep (gdbarch); > + > + /* Get and the previous and current instruction addresses. If they ar= e not This line is too long. > + > +/* Name for or1k general registers. */ > + > +static const char *const or1k_reg_names[OR1K_NUM_REGS] =3D { > + /* general purpose registers */ > + "r0", "r1", "r2", "r3", "r4", "r5", "r6", "r7", > + "r8", "r9", "r10", "r11", "r12", "r13", "r14", "r15", > + "r16", "r17", "r18", "r19", "r20", "r21", "r22", "r23", > + "r24", "r25", "r26", "r27", "r28", "r29", "r30", "r31", > + > + /* previous program counter, next program counter and status register = */ > + "ppc", "npc", "sr" > +}; > + > +/* Implement the register_name gdbarch method. */ > + > +static const char * > +or1k_register_name (struct gdbarch *gdbarch, int regnum) > +{ > + if (0 <=3D regnum && regnum < OR1K_NUM_REGS) > + return or1k_reg_names[regnum]; > + else > + return NULL; > +} > + Register names can be from target description, so we don't need this function. > +/* Implement the register_type gdbarch method. */ > + > +static struct type * > +or1k_register_type (struct gdbarch *gdbarch, int regnum) > +{ > + if ((regnum >=3D 0) && (regnum < OR1K_NUM_REGS)) > + { > + switch (regnum) > + { > + case OR1K_PPC_REGNUM: > + case OR1K_NPC_REGNUM: > + return builtin_type (gdbarch)->builtin_func_ptr; /* Pointer to code */ > + > + case OR1K_SP_REGNUM: > + case OR1K_FP_REGNUM: > + return builtin_type (gdbarch)->builtin_data_ptr; /* Pointer to data */ > + > + default: > + return builtin_type (gdbarch)->builtin_uint32; /* Data */ > + } > + } > + > + internal_error (__FILE__, __LINE__, > + _("or1k_register_type: illegal register number %d"), > + regnum); > +} > + Likewise. > +/* Implement the register_reggroup_p gdbarch method. */ > + > +static int > +or1k_register_reggroup_p (struct gdbarch *gdbarch, > + int regnum, struct reggroup *group) > +{ > + struct gdbarch_tdep *tdep =3D gdbarch_tdep (gdbarch); > + > + /* All register group. */ > + if (group =3D=3D all_reggroup) > + return ((regnum >=3D 0) > + && (regnum < OR1K_NUM_REGS) > + && (or1k_register_name (gdbarch, regnum)[0] !=3D '\0')); > + > + /* For now everything except the PC. */ > + if (group =3D=3D general_reggroup) > + return ((regnum >=3D OR1K_ZERO_REGNUM) > + && (regnum < OR1K_MAX_GPR_REGS) > + && (regnum !=3D OR1K_PPC_REGNUM) && (regnum !=3D OR1K_NPC_REGNUM)); > + > + if (group =3D=3D float_reggroup) > + return 0; /* No float regs */ > + > + if (group =3D=3D vector_reggroup) > + return 0; /* No vector regs */ > + > + /* For any that are not handled above. */ > + return default_register_reggroup_p (gdbarch, regnum, group); > +} > + Likewise. > +static int > +or1k_is_arg_reg (unsigned int regnum) > +{ > + return (OR1K_FIRST_ARG_REGNUM <=3D regnum) > + && (regnum <=3D OR1K_LAST_ARG_REGNUM); > +} > + > +static int > +or1k_is_callee_saved_reg (unsigned int regnum) > +{ > + return (OR1K_FIRST_SAVED_REGNUM <=3D regnum) && (0 =3D=3D regnum % 2); > +} > + > +/* Implement the skip_prologue gdbarch method. */ > + > +static CORE_ADDR > +or1k_skip_prologue (struct gdbarch *gdbarch, CORE_ADDR pc) > +{ > + CORE_ADDR start_pc; > + CORE_ADDR addr; > + uint32_t inst; > + > + unsigned int ra, rb, rd; /* for instruction analysis */ > + int simm; > + > + int frame_size =3D 0; > + > + /* Try using SAL first if we have symbolic information available. Thi= s only > + works for DWARF 2, not STABS. */ > + > + if (find_pc_partial_function (pc, NULL, &start_pc, NULL)) > + { > + CORE_ADDR prologue_end =3D skip_prologue_using_sal (gdbarch, pc); > + > + if (0 !=3D prologue_end) > + { > + struct symtab_and_line prologue_sal =3D find_pc_line (start_pc, 0); > + struct compunit_symtab *compunit > + =3D SYMTAB_COMPUNIT (prologue_sal.symtab); > + const char *debug_format =3D COMPUNIT_DEBUGFORMAT (compunit); > + > + if ((NULL !=3D debug_format) > + && (strlen ("dwarf") <=3D strlen (debug_format)) > + && (0 =3D=3D strncasecmp ("dwarf", debug_format, strlen ("dwarf")= ))) > + return (prologue_end > pc) ? prologue_end : pc; > + } > + } > + > + /* Look to see if we can find any of the standard prologue sequence. = All > + quite difficult, since any or all of it may be missing. So this is= just a This line is too long. > + best guess! */ > + > + addr =3D pc; /* Where we have got to */ > + inst =3D or1k_fetch_instruction (gdbarch, addr); > + > + /* Look for the new stack pointer being set up. */ > + if (or1k_analyse_l_addi (inst, &rd, &ra, &simm) > + && (OR1K_SP_REGNUM =3D=3D rd) && (OR1K_SP_REGNUM =3D=3D ra) > + && (simm < 0) && (0 =3D=3D (simm % 4))) > + { > + frame_size =3D -simm; > + addr +=3D OR1K_INSTLEN; > + inst =3D or1k_fetch_instruction (gdbarch, addr); > + } > + > + /* Look for the frame pointer being manipulated. */ > + if (or1k_analyse_l_sw (inst, &simm, &ra, &rb) > + && (OR1K_SP_REGNUM =3D=3D ra) && (OR1K_FP_REGNUM =3D=3D rb) > + && (simm >=3D 0) && (0 =3D=3D (simm % 4))) > + { > + addr +=3D OR1K_INSTLEN; > + inst =3D or1k_fetch_instruction (gdbarch, addr); > + > + gdb_assert (or1k_analyse_l_addi (inst, &rd, &ra, &simm) > + && (OR1K_FP_REGNUM =3D=3D rd) && (OR1K_SP_REGNUM =3D=3D ra) > + && (simm =3D=3D frame_size)); > + > + addr +=3D OR1K_INSTLEN; > + inst =3D or1k_fetch_instruction (gdbarch, addr); > + } > + > + /* Look for the link register being saved. */ > + if (or1k_analyse_l_sw (inst, &simm, &ra, &rb) > + && (OR1K_SP_REGNUM =3D=3D ra) && (OR1K_LR_REGNUM =3D=3D rb) > + && (simm >=3D 0) && (0 =3D=3D (simm % 4))) > + { > + addr +=3D OR1K_INSTLEN; > + inst =3D or1k_fetch_instruction (gdbarch, addr); > + } > + > + /* Look for arguments or callee-saved register being saved. The regis= ter > + must be one of the arguments (r3-r8) or the 10 callee saved registe= rs > + (r10, r12, r14, r16, r18, r20, r22, r24, r26, r28, r30). The base > + register must be the FP (for the args) or the SP (for the callee_sa= ved > + registers). */ > + while (1) Can you give an upper-bound of the loop? The endless loop looks scary to me. > + { > + if (or1k_analyse_l_sw (inst, &simm, &ra, &rb) > + && (((OR1K_FP_REGNUM =3D=3D ra) && or1k_is_arg_reg (rb)) > + || ((OR1K_SP_REGNUM =3D=3D ra) && or1k_is_callee_saved_reg (rb))) > + && (0 =3D=3D (simm % 4))) > + { > + addr +=3D OR1K_INSTLEN; > + inst =3D or1k_fetch_instruction (gdbarch, addr); > + } > + else > + { > + /* Nothing else to look for. We have found the end of the > + prologue. */ > + return addr; break the loop and "return addr" at the end of this function. > + > +/* Implement the push_dummy_call gdbarch method. */ > + > +static CORE_ADDR > +or1k_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) > +{ > + > + int argreg; > + int argnum; > + int first_stack_arg; > + int stack_offset =3D 0; > + int heap_offset =3D 0; > + CORE_ADDR heap_sp =3D sp - 128; > + enum bfd_endian byte_order =3D gdbarch_byte_order (gdbarch); > + int bpa =3D (gdbarch_tdep (gdbarch))->bytes_per_address; > + int bpw =3D (gdbarch_tdep (gdbarch))->bytes_per_word; > + struct type *func_type =3D value_type (function); > + > + /* Return address */ > + regcache_cooked_write_unsigned (regcache, OR1K_LR_REGNUM, bp_addr); > + > + /* Register for the next argument. */ > + argreg =3D OR1K_FIRST_ARG_REGNUM; > + > + /* Location for a returned structure. This is passed as a silent first > + argument. */ > + if (struct_return) > + { > + regcache_cooked_write_unsigned (regcache, OR1K_FIRST_ARG_REGNUM, > + struct_addr); > + argreg++; > + } > + > + /* Put as many args as possible in registers. */ > + for (argnum =3D 0; argnum < nargs; argnum++) > + { > + const gdb_byte *val; > + gdb_byte valbuf[sizeof (ULONGEST)]; > + > + struct value *arg =3D args[argnum]; > + struct type *arg_type =3D check_typedef (value_type (arg)); > + int len =3D arg_type->length; > + enum type_code typecode =3D arg_type->main_type->code; typecode =3D TYPE_CODE (arg_type); > + > + if (TYPE_VARARGS (func_type) && argnum >=3D TYPE_NFIELDS (func_typ= e)) > + break; /* end or regular args, varargs go to stack. */ > + > + /* Extract the value, either a reference or the data. */ > + if ((TYPE_CODE_STRUCT =3D=3D typecode) || (TYPE_CODE_UNION =3D=3D = typecode) > + || (len > bpw * 2)) > + { > + CORE_ADDR valaddr =3D value_address (arg); > + > + /* If the arg is fabricated (i.e. 3*i, instead of i) valaddr is > + undefined. */ > + if (valaddr =3D=3D 0) > + { > + /* The argument needs to be copied into the target space. Since > + the bottom of the stack is reserved for function arguments > + we store this at the these at the top growing down. */ > + heap_offset +=3D align_up (len, bpw); > + valaddr =3D heap_sp + heap_offset; > + > + write_memory (valaddr, value_contents (arg), len); > + } > + > + /* The ABI passes all structures by reference, so get its address. */ > + store_unsigned_integer (valbuf, bpa, byte_order, valaddr); > + len =3D bpa; > + val =3D valbuf; > + } > + else > + { > + /* Everything else, we just get the value. */ > + val =3D value_contents (arg); > + } > + > + /* Stick the value in a register. */ > + if (len > bpw) > + { > + /* Big scalars use two registers, but need NOT be pair aligned. */ > + > + if (argreg <=3D (OR1K_LAST_ARG_REGNUM - 1)) > + { > + ULONGEST regval =3D extract_unsigned_integer (val, len, byte_orde= r); > + > + unsigned int bits_per_word =3D bpw * 8; > + ULONGEST mask =3D (((ULONGEST) 1) << bits_per_word) - 1; > + ULONGEST lo =3D regval & mask; > + ULONGEST hi =3D regval >> bits_per_word; > + > + regcache_cooked_write_unsigned (regcache, argreg, hi); > + regcache_cooked_write_unsigned (regcache, argreg + 1, lo); > + argreg +=3D 2; > + } > + else > + { > + /* Run out of regs */ > + break; > + } > + } > + else if (argreg <=3D OR1K_LAST_ARG_REGNUM) > + { > + /* Smaller scalars fit in a single register. */ > + regcache_cooked_write_unsigned > + (regcache, argreg, extract_unsigned_integer (val, len, byte_order)); > + argreg++; > + } > + else > + { > + /* Ran out of regs. */ > + break; > + } > + } > + > + first_stack_arg =3D argnum; > + > + /* If we get here with argnum < nargs, then arguments remain to be pla= ced on > + the stack. This is tricky, since they must be pushed in reverse or= der and These two lines are too long. > + the stack in the end must be aligned. The only solution is to do i= t in > + two stages, the first to compute the stack size, the second to save= the > + args. */ > + > + for (argnum =3D first_stack_arg; argnum < nargs; argnum++) > + { > + struct value *arg =3D args[argnum]; > + struct type *arg_type =3D check_typedef (value_type (arg)); > + int len =3D arg_type->length; TYPE_LENGTH (arg_type); > + enum type_code typecode =3D arg_type->main_type->code; > + TYPE_CODE (arg_type); > +=0C > + > +/* Support functions for frame handling. */ > + > +/* Initialize a prologue cache > + > + We build a cache, saying where registers of the PREV frame can be fou= nd > + from the data so far set up in this THIS. > + > + We also compute a unique ID for this frame, based on the function sta= rt > + address and the stack pointer (as it will be, even if it has yet to be > + computed. > + > + STACK FORMAT > + =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D > + > + The OR1K has a falling stack frame and a simple prolog. The Stack po= inter > + is R1 and the frame pointer R2. The frame base is therefore the addr= ess > + held in R2 and the stack pointer (R1) is the frame base of the NEXT f= rame. > + > + l.addi r1,r1,-frame_size # SP now points to end of new stack frame > + > + The stack pointer may not be set up in a frameless function (e.g. a s= imple > + leaf function). > + > + l.sw fp_loc(r1),r2 # old FP saved in new stack frame > + l.addi r2,r1,frame_size # FP now points to base of new stack fra= me > + > + The frame pointer is not necessarily saved right at the end of the st= ack > + frame - OR1K saves enough space for any args to called functions righ= t at > + the end (this is a difference from the Architecture Manual). > + > + l.sw lr_loc(r1),r9 # Link (return) address > + > + The link register is usally saved at fp_loc - 4. It may not be saved= at all > + in a leaf function. > + > + l.sw reg_loc(r1),ry # Save any callee saved regs > + > + The offsets x for the callee saved registers generally (always?) rise= in > + increments of 4, starting at fp_loc + 4. If the frame pointer is omi= tted > + (an option to GCC), then it may not be saved at all. There may be no= callee > + saved registers. > + > + So in summary none of this may be present. However what is present s= eems > + always to follow this fixed order, and occur before any substantive c= ode > + (it is possible for GCC to have more flexible scheduling of the prolo= gue, > + but this does not seem to occur for OR1K). > + > + ANALYSIS > + =3D=3D=3D=3D=3D=3D=3D=3D > + > + This prolog is used, even for -O3 with GCC. > + > + All this analysis must allow for the possibility that the PC is in the > + middle of the prologue. Data in the cache should only be set up inso= far as > + it has been computed. > + > + HOWEVER. The frame_id must be created with the SP *as it will be* at= the > + end of the Prologue. Otherwise a recursive call, checking the frame = with > + the PC at the start address will end up with the same frame_id as the > + caller. > + > + A suite of "helper" routines are used, allowing reuse for > + or1k_skip_prologue(). > + > + Reportedly, this is only valid for frames less than 0x7fff in size. = */ > + > +static struct trad_frame_cache * > +or1k_frame_cache (struct frame_info *this_frame, void **prologue_cache) > +{ > + struct gdbarch *gdbarch; > + struct trad_frame_cache *info; > + > + CORE_ADDR this_pc; > + CORE_ADDR this_sp; > + CORE_ADDR this_sp_for_id; > + int frame_size =3D 0; > + > + CORE_ADDR start_addr; > + CORE_ADDR end_addr; > + > + if (frame_debug) > + fprintf_unfiltered (gdb_stdlog, > + "or1k_frame_cache, prologue_cache =3D 0x%p\n", > + *prologue_cache); Can you add a new debug flag, like or1k_debug, to control this? Each backend in GDB (amd64-windows-tdep.c) use its own debug flag to control the output related to frame unwinding. > + /* Get a new prologue cache and populate it with default values. */ > + info =3D trad_frame_cache_zalloc (this_frame); > + *prologue_cache =3D info; > + > + /* Find the start address of THIS function (which is a NORMAL frame, e= ven if > + the NEXT frame is the sentinel frame) and the end of its prologue. = */ Don't need to capitalize THIS, NORMAL, and NEXT. > + > + /* The default frame base of THIS frame (for ID purposes only - frame = base > + is an overloaded term) is its stack pointer. For now we use the va= lue of > + the SP register in THIS frame. However if the PC is in the prologu= e of > + THIS frame, before the SP has been set up, then the value will actu= ally > + be that of the PREV frame, and we'll need to adjust it later. */ > + trad_frame_set_this_base (info, this_sp); > + this_sp_for_id =3D this_sp; > + > + /* The default is to find the PC of the PREVIOUS frame in the link reg= ister > + of this frame. This may be changed if we find the link register wa= s saved > + on the stack. */ > + trad_frame_set_reg_realreg (info, OR1K_NPC_REGNUM, OR1K_LR_REGNUM); > + > + /* We should only examine code that is in the prologue. This is all c= ode up > + to (but not including) end_addr. We should only populate the cache= while > + the address is up to (but not including) the PC or end_addr, whiche= ver is > + first. */ > + gdbarch =3D get_frame_arch (this_frame); > + end_addr =3D or1k_skip_prologue (gdbarch, start_addr); > + > + /* All the following analysis only occurs if we are in the prologue an= d have > + executed the code. Check we have a sane prologue size, and if zero= we > + are frameless and can give up here. */ > + if (end_addr < start_addr) How can it be? > + throw_quit ("end addr 0x%08x is less than start addr 0x%08x\n", > + (unsigned int) end_addr, (unsigned int) start_addr); s/throw_quit/error/ > + > +/* Data structures for the normal prologue-analysis-based unwinder. */ > + > +static const struct frame_unwind or1k_frame_unwind =3D { > + .type =3D NORMAL_FRAME, > + .stop_reason =3D default_frame_unwind_stop_reason, > + .this_id =3D or1k_frame_this_id, > + .prev_register =3D or1k_frame_prev_register, > + .unwind_data =3D NULL, > + .sniffer =3D default_frame_sniffer, > + .dealloc_cache =3D NULL, > + .prev_arch =3D NULL > +}; Write the code this way, static const struct frame_unwind or1k_frame_unwind =3D { NORMAL_FRAME, default_frame_unwind_stop_reason, or1k_frame_this_id, or1k_frame_prev_register, NULL, default_frame_sniffer, NULL, }; > + > +/* Architecture initialization for OpenRISC 1000. */ > + > +static struct gdbarch * > +or1k_gdbarch_init (struct gdbarch_info info, struct gdbarch_list *arches) > +{ ... > + /* Check any target description for validity. */ > + if (tdesc_has_registers (info.target_desc)) > + { > + const struct tdesc_feature *feature; > + int valid_p; > + > + feature =3D tdesc_find_feature (info.target_desc, "org.gnu.gdb.or1= k.group0"); This line is too long. In patch 2/4, you defined 11 features, group0 - group 10, but you only find feature group0. Is group0 a required feature? > + if (feature =3D=3D NULL) > + return NULL; > + > + tdesc_data =3D tdesc_data_alloc (); > + > + valid_p =3D 1; > + > + for (i =3D 0; i < OR1K_NUM_REGS; i++) > + valid_p &=3D tdesc_numbered_register (feature, tdesc_data, i, > + or1k_reg_names[i]); > + > + if (!valid_p) > + { > + tdesc_data_cleanup (tdesc_data); > + return NULL; > + } > + } > + > + if (tdesc_data) if (tdesc_data !=3D NULL) > + { > + tdesc_use_registers (gdbarch, info.target_desc, tdesc_data); > + > + /* Target descriptions may extend into the following groups. */ > + reggroup_add (gdbarch, general_reggroup); > + reggroup_add (gdbarch, system_reggroup); > + reggroup_add (gdbarch, float_reggroup); > + reggroup_add (gdbarch, vector_reggroup); > + reggroup_add (gdbarch, reggroup_new ("immu", USER_REGGROUP)); > + reggroup_add (gdbarch, reggroup_new ("dmmu", USER_REGGROUP)); > + reggroup_add (gdbarch, reggroup_new ("icache", USER_REGGROUP)); > + reggroup_add (gdbarch, reggroup_new ("dcache", USER_REGGROUP)); > + reggroup_add (gdbarch, reggroup_new ("pic", USER_REGGROUP)); > + reggroup_add (gdbarch, reggroup_new ("timer", USER_REGGROUP)); > + reggroup_add (gdbarch, reggroup_new ("power", USER_REGGROUP)); > + reggroup_add (gdbarch, reggroup_new ("perf", USER_REGGROUP)); > + reggroup_add (gdbarch, reggroup_new ("mac", USER_REGGROUP)); > + reggroup_add (gdbarch, reggroup_new ("debug", USER_REGGROUP)); > + reggroup_add (gdbarch, all_reggroup); > + reggroup_add (gdbarch, save_reggroup); > + reggroup_add (gdbarch, restore_reggroup); > + } > + > + return gdbarch; > +} > + > + > +extern initialize_file_ftype _initialize_or1k_tdep; /* -Wmissing-prototy= pes */ > + > +void > +_initialize_or1k_tdep (void) > +{ > + /* Register this architecture. */ > + gdbarch_register (bfd_arch_or1k, or1k_gdbarch_init, or1k_dump_tdep); > + > + /* Tell remote stub that we support XML target description. */ > + register_remote_support_xml ("or1k"); Can't remote stub think GDB support xml target description already? > +} > diff --git a/gdb/or1k-tdep.h b/gdb/or1k-tdep.h > new file mode 100644 > index 0000000..edcad88 > --- /dev/null > +++ b/gdb/or1k-tdep.h > @@ -0,0 +1,56 @@ > +/* Definitions to target GDB to OpenRISC 1000 32-bit targets. > + Copyright (C) 2008-2017 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 opt= ion) > + any later version. > + > + This program is distributed in the hope that it will be useful, but W= ITHOUT > + 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 alo= ng > + With this program. If not, see . */ > + > + > +#ifndef OR1K_TDEP__H > +#define OR1K_TDEP__H > + > +#ifndef TARGET_OR1K > +#define TARGET_OR1K > +#endif > + > +#include "opcodes/or1k-desc.h" > +#include "opcodes/or1k-opc.h" > + > +/* General Purpose Registers */ > +#define OR1K_ZERO_REGNUM 0 > +#define OR1K_SP_REGNUM 1 > +#define OR1K_FP_REGNUM 2 > +#define OR1K_FIRST_ARG_REGNUM 3 > +#define OR1K_LAST_ARG_REGNUM 8 > +#define OR1K_LR_REGNUM 9 > +#define OR1K_FIRST_SAVED_REGNUM 10 > +#define OR1K_RV_REGNUM 11 > +#define OR1K_PPC_REGNUM (OR1K_MAX_GPR_REGS + 0) > +#define OR1K_NPC_REGNUM (OR1K_MAX_GPR_REGS + 1) > +#define OR1K_SR_REGNUM (OR1K_MAX_GPR_REGS + 2) A general comments on these macros, if they are only used in or1k-tdep.c, why don't you move these macros into or1k-tdep.c? > + > +/* Properties of the architecture. GDB mapping of registers is all the G= PRs > + and SPRs followed by the PPC, NPC and SR at the end. Red zone is the = area > + past the end of the stack reserved for exception handlers etc. */ > + > +#define OR1K_MAX_GPR_REGS 32 > +#define OR1K_NUM_PSEUDO_REGS 0 > +#define OR1K_NUM_REGS (OR1K_MAX_GPR_REGS + 3) > +#define OR1K_STACK_ALIGN 4 > +#define OR1K_INSTLEN 4 > +#define OR1K_INSTBITLEN (OR1K_INSTLEN * 8) > +#define OR1K_NUM_TAP_RECORDS 8 > +#define OR1K_FRAME_RED_ZONE_SIZE 2536 > + > +#endif /* OR1K_TDEP__H */ --=20 Yao (=E9=BD=90=E5=B0=A7)