This is the mail archive of the
gdb-patches@sourceware.org
mailing list for the GDB project.
Re: [patch 1/4] code changes for Nios II target
- From: Joel Brobecker <brobecker at adacore dot com>
- To: Sandra Loosemore <sandra at codesourcery dot com>
- Cc: gdb-patches at sourceware dot org
- Date: Fri, 26 Apr 2013 10:36:17 +0400
- Subject: Re: [patch 1/4] code changes for Nios II target
- References: <517481E7 dot 7030608 at codesourcery dot com>
> 2013-04-21 Sandra Loosemore <sandra@codesourcery.com>
> Andrew Jenner <andrew@codesourcery.com>
> Chung-Lin Tang <cltang@codesourcery.com>
>
> Based on the nios2-elf port from Altera Corporation.
>
> gdb/
> * Makefile.in (ALL_TARGET_OBS): Add nios2-tdep.o and
> nios2-linux-tdep.o.
> (HFILES_NO_SRCDIR): Add nios2-tdep.h.
> * configure.tgt: Add nios2*-*-linux* and nios2*-*-* targets.
> * nios2-tdep.h: New.
> * nios2-tdep.c: New.
> * nios2-linux-tdep.c: New.
We had a discussion about Changelog entries recently, so forgive me
if I state the obvious. Please be sure to remove the "gdb/" from
the entry you would actually add to the ChangeLog file.
Also, you mention that this is based on the port from Altera Corp.
I checked the assignment records, and I see Altera (2012-10-31)
so we should be good.
Typically, I would prefer if the linux-tdep part was submitted
as a separate patch, but now that you've sent them together,
I don't see a real reason to make you work extra.
Overall, the patch looks pretty good. I only have minor comments
and maybe a couple of questions... This was much more time consuming
than I anticipated, so I will stop with this patch today.
> Index: gdb/Makefile.in
You also need to add nios2-tdep.c and nios2-linux-tdep.c to ALLDEPFILES.
> Index: gdb/configure.tgt
> ===================================================================
> RCS file: /cvs/src/src/gdb/configure.tgt,v
> retrieving revision 1.274
> diff -u -p -r1.274 configure.tgt
> --- gdb/configure.tgt 11 Apr 2013 16:53:01 -0000 1.274
> +++ gdb/configure.tgt 20 Apr 2013 21:32:50 -0000
> @@ -396,6 +396,17 @@ mt-*-*)
> gdb_target_obs="mt-tdep.o"
> ;;
>
> +nios2*-*-linux*)
> + # Target: Altera Nios II running Linux
> + gdb_target_obs="nios2-tdep.o nios2-linux-tdep.o solib-svr4.o \
> + symfile-mem.o glibc-tdep.o linux-tdep.o"
> + ;;
> +
> +nios2*-*-*)
> + # Target: Altera Nios II bare-metal
> + gdb_target_obs="nios2-tdep.o"
> + ;;
> +
> powerpc*-*-freebsd*)
> # Target: FreeBSD/powerpc
> gdb_target_obs="rs6000-tdep.o ppc-sysv-tdep.o ppc64-tdep.o \
> Index: gdb/nios2-tdep.h
> ===================================================================
> RCS file: gdb/nios2-tdep.h
> diff -N gdb/nios2-tdep.h
> --- /dev/null 1 Jan 1970 00:00:00 -0000
> +++ gdb/nios2-tdep.h 20 Apr 2013 21:32:50 -0000
> @@ -0,0 +1,87 @@
> +/* Target-dependent header for the Nios II architecture, for GDB.
> + Copyright (C) 2012, 2013 Free Software Foundation, Inc.
> + Contributed by Mentor Graphics, 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 <http://www.gnu.org/licenses/>. */
> +
> +#ifndef NIOS2_TDEP_H
> +#define NIOS2_TDEP_H
> +
> +/* Registers. */
> +#undef SP_REGNUM
> +#define SP_REGNUM 27 /* Stack Pointer */
> +#undef FP_REGNUM
> +#define FP_REGNUM 28 /* Frame Pointer */
> +#undef PC_REGNUM
> +#define PC_REGNUM 32
Why do you need the #undef above? From what I can tell, this hasn't been
needed for any of the ports, so I do not see why it would be needed
here.
> +#define NIOS2_Z_REGNUM 0 /* Zero */
> +#define NIOS2_R2_REGNUM 2 /* used for return value */
> +#define NIOS2_R3_REGNUM 3 /* used for return value */
> +/* Used for hidden zero argument to store ptr to struct return value. */
> +#define NIOS2_R4_REGNUM 4
> +#define NIOS2_R7_REGNUM 7
> +#define NIOS2_GP_REGNUM 26 /* Global Pointer */
> +#define NIOS2_EA_REGNUM 29 /* Exception address */
> +#define NIOS2_BA_REGNUM 30 /* Breakpoint return address */
> +#define NIOS2_RA_REGNUM 31 /* Return address */
FYI: small misalignment of the first entry, but it does not matter
to me whether you change it or not.
> +/* R4-R7 are used for argument passing. */
> +#define FIRST_ARGREG NIOS2_R4_REGNUM
> +#define LAST_ARGREG NIOS2_R7_REGNUM
I think it would be better if these macros had a NIOS2_ prefix.
This is what we normally do in other ports.
> +/* Number of all registers. */
> +#define NIOS2_NUM_REGS (49)
> +
> +/* The maximum register number displayed to the user,
> + as a result of typing "info reg" at the gdb prompt. */
> +#define NIOS2_MAX_REG_DISPLAYED_REGNUM (48)
FIXME: Figure out why this is needed?
> +#define NIOS2_OPCODE_SIZE 4
> +
> +/* Target-dependent structure in gdbarch. */
> +struct gdbarch_tdep
> +{
> + /* Return the expected next PC if FRAME is stopped at a syscall
> + instruction. */
What does it return if FRAME is not stopped at a syscall insn?
> + CORE_ADDR (*syscall_next_pc) (struct frame_info *frame);
> +
> + /* Offset to PC value in jump buffer.
> + If this is negative, longjmp support will be disabled. */
> + int jb_pc;
> +};
> +
> +extern struct target_desc *tdesc_nios2_linux;
> +extern struct target_desc *tdesc_nios2;
> +
> +#endif /* NIOS2_TDEP_H */
> Index: gdb/nios2-tdep.c
> ===================================================================
> RCS file: gdb/nios2-tdep.c
> diff -N gdb/nios2-tdep.c
> --- /dev/null 1 Jan 1970 00:00:00 -0000
> +++ gdb/nios2-tdep.c 20 Apr 2013 21:32:50 -0000
> @@ -0,0 +1,1644 @@
> +/* Target-machine dependent code for Nios II, for GDB.
> + Copyright (C) 2012, 2013 Free Software Foundation, Inc.
> + Contributed by Peter Brookes (pbrookes@altera.com)
> + and Andrew Draper (adraper@altera.com).
> + Contributed by Mentor Graphics, 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 <http://www.gnu.org/licenses/>. */
> +
> +#include "defs.h"
> +#include "frame.h"
> +#include "frame-unwind.h"
> +#include "frame-base.h"
> +#include "trad-frame.h"
> +#include "dwarf2-frame.h"
> +#include "symtab.h"
> +#include "inferior.h"
> +#include "gdbtypes.h"
> +#include "gdbcore.h"
> +#include "gdbcmd.h"
> +#include "osabi.h"
> +#include "target.h"
> +#include "dis-asm.h"
> +#include "regcache.h"
> +#include "value.h"
> +#include "symfile.h"
> +#include "arch-utils.h"
> +#include "floatformat.h"
> +#include "gdb_assert.h"
> +#include "infcall.h"
> +#include "regset.h"
> +#include "target-descriptions.h"
> +
> +/* To get entry_point_address. */
> +#include "objfiles.h"
> +
> +/* Nios II ISA specific encodings and macros. */
> +#include "opcode/nios2.h"
> +
> +/* Nios II specific header. */
> +#include "nios2-tdep.h"
> +
> +#include "features/nios2.c"
> +
> +/* Control debugging information emitted in this file. */
> +
> +static int nios2_debug = 0;
> +
> +/* Structures. */
> +
> +/* The current value in the register is the value in r[base] at the start of
> + * the function + offset; unless base < 0 in which case it's unknown. */
No '*' at the start of new lines in large comments, please. And can you
reformat it to fit within 70 columns? That's the soft limit that we
decided on, allowing us to exceed that limit only when it makes a
difference.
Also, I am having a hard time understanding the comment. I think it is
missing the context where this structure would be used. For small
structs like this one it's sometimes also good enough to just document
the struct and its field like here; but you can also document each field
individually, if that makes things clearer.
> +
> +typedef struct
> +{
> + int reg;
> + unsigned int offset;
> +} REG_VALUE;
> +
> +typedef struct
> +{
> + int basereg;
> + CORE_ADDR addr;
> +} REG_SAVED;
Needs a separate documentation comment.
> +
> +struct nios2_unwind_cache
> +{
> + /* The frame's base, optionally used by the high-level debug info. */
> + CORE_ADDR base;
> +
> + /* The previous frame's inner most stack address. Used as this
> + frame ID's stack_addr. */
> + CORE_ADDR cfa;
> +
> + /* The address of the first instruction in this function. */
> + CORE_ADDR pc;
> +
> + /* Which register holds the return address for the frame. */
> + int return_regnum;
> +
> + /* Table indicating what changes have been made to each register. */
> + REG_VALUE reg_value[NIOS2_NUM_REGS];
> +
> + /* Table indicating where each register has been saved. */
> + REG_SAVED reg_saved[NIOS2_NUM_REGS];
> +};
> +
> +/* This array is a mapping from Dwarf-2 register numbering to GDB's. */
> +
> +static int nios2_dwarf2gdb_regno_map[] = {
The curly brace should be on the next line (we're not very consistent
with this).
> + 0, 1, 2, 3,
> + 4, 5, 6, 7,
> + 8, 9, 10, 11,
> + 12, 13, 14, 15,
> + 16, 17, 18, 19,
> + 20, 21, 22, 23,
> + 24, 25,
> + NIOS2_GP_REGNUM, /* 26 */
> + SP_REGNUM, /* 27 */
> + FP_REGNUM, /* 28 */
> + NIOS2_EA_REGNUM, /* 29 */
> + NIOS2_BA_REGNUM, /* 30 */
> + NIOS2_RA_REGNUM, /* 31 */
> + PC_REGNUM, /* 32 */
> + NIOS2_STATUS_REGNUM, /* 33 */
> + NIOS2_ESTATUS_REGNUM, /* 34 */
> + NIOS2_BSTATUS_REGNUM, /* 35 */
> + NIOS2_IENABLE_REGNUM, /* 36 */
> + NIOS2_IPENDING_REGNUM, /* 37 */
> + NIOS2_CPUID_REGNUM, /* 38 */
> + 39, /* CTL6 */ /* 39 */
> + NIOS2_EXCEPTION_REGNUM, /* 40 */
> + NIOS2_PTEADDR_REGNUM, /* 41 */
> + NIOS2_TLBACC_REGNUM, /* 42 */
> + NIOS2_TLBMISC_REGNUM, /* 43 */
> + NIOS2_FSTATUS_REGNUM, /* 44 */
> + NIOS2_BADADDR_REGNUM, /* 45 */
> + NIOS2_CONFIG_REGNUM, /* 46 */
> + NIOS2_MPUBASE_REGNUM, /* 47 */
> + NIOS2_MPUACC_REGNUM /* 48 */
Another case of small differences in alignment for some registers...
> + if (dw_reg < 0 || dw_reg > NIOS2_NUM_REGS)
> + {
> + warning ("Dwarf-2 uses unmapped register #%d\n", dw_reg);
> + return dw_reg;
The warning call is missing i18n:
warning (_("..."));
> +/* Find the name for the specified NIOS2 regno. */
> +
> +static const char *
> +nios2_register_name (struct gdbarch *gdbarch, int regno)
> +{
> + static const char *const nios2_reg_names[] = {
Curly brace on next line.
> + "zero", "at", "r2", "r3", "r4", "r5", "r6", "r7",
> + "r8", "r9", "r10", "r11", "r12", "r13", "r14", "r15",
> + "r16", "r17", "r18", "r19", "r20", "r21", "r22", "r23",
> + "et", "bt", "gp", "sp", "fp", "ea", "ba", "ra"
> + };
> + /* Don't display any registers after NIOS2_MAX_REG_DISPLAYED_REGNUM. */
> + if (regno < 0 || regno > NIOS2_MAX_REG_DISPLAYED_REGNUM)
> + return NULL;
I am trying to undestand this. From what I can tell, this is excluding
NIOS2_MPUACC_REGNUM? Why do you need this?
> + /* Use mnemonic aliases for GPRs. */
> + if (regno >= 0 && regno < 32)
> + return nios2_reg_names[regno];
> + else
> + return tdesc_register_name (gdbarch, regno);
> +}
> +
> +/* Return the GDB type object for the "standard" data type
> + of data in register REGNO. */
> +
> +static struct type *
> +nios2_register_type (struct gdbarch *gdbarch, int regno)
> +{
> + /* If the XML description has register information, use that to determine the
> + register type. */
Can you reformat the comment to fit within 70 columns?
> + if (tdesc_has_registers (gdbarch_target_desc (gdbarch)))
> + return tdesc_register_type (gdbarch, regno);
> +
> + if (regno == PC_REGNUM)
> + return builtin_type (gdbarch)->builtin_func_ptr;
> + else if (regno == SP_REGNUM)
> + return builtin_type (gdbarch)->builtin_data_ptr;
> + else
> + return builtin_type (gdbarch)->builtin_uint32;
> +}
> +
> +/* Given a return value in REGCACHE with a type VALTYPE,
> + extract and copy its value into VALBUF. */
> +
> +static void
> +nios2_extract_return_value (struct gdbarch *gdbarch, struct type *valtype,
> + struct regcache *regcache, gdb_byte *valbuf)
I have been trying to ignore the fact that you slightly duplicate
the function documentation that should already be provided in the
gdbarch.h file, because the function was sufficiently simple and
had few enough arguments that it wasn't a problem. But for this one,
I'd rather you did not. I know it's a little bit of a pain, but
would you please rewrite the documentation for all such functions
to just point to the documentation in the gdbarch.h file? We usually
write something very simple such as:
/* Implement the extract_return_value gdbarch method. */
(not the absence of the "gdbarch_" prefix in the name of the "method").
> +{
> + int len = TYPE_LENGTH (valtype);
> +
> + /* Return values of up to 8 bytes are returned in $r2 $r3. */
> + if (len <= register_size (gdbarch, NIOS2_R2_REGNUM))
> + regcache_cooked_read (regcache, NIOS2_R2_REGNUM, valbuf);
> + else if (len <= (register_size (gdbarch, NIOS2_R2_REGNUM)
> + + register_size (gdbarch, NIOS2_R3_REGNUM)))
> + {
> + regcache_cooked_read (regcache, NIOS2_R2_REGNUM, valbuf);
> + regcache_cooked_read (regcache, NIOS2_R3_REGNUM, valbuf + 4);
> + }
This is leaving the buffer undefined if len > 8. I suppose this cannot
happen (ie, you cannot extract the return value in that case?). If this
is correct, I'd add a gdb_assert (len <= size ($r2 + $r3))
> +static void
> +nios2_store_return_value (struct gdbarch *gdbarch, struct type *valtype,
> + struct regcache *regcache, const gdb_byte *valbuf)
> +{
> + int len = TYPE_LENGTH (valtype);
> +
> + /* Return values of up to 8 bytes are returned in $r2 $r3. */
> + if (len <= register_size (gdbarch, NIOS2_R2_REGNUM))
> + regcache_cooked_write (regcache, NIOS2_R2_REGNUM, valbuf);
> + else if (len <= (register_size (gdbarch, NIOS2_R2_REGNUM)
> + + register_size (gdbarch, NIOS2_R3_REGNUM)))
> + {
> + regcache_cooked_write (regcache, NIOS2_R2_REGNUM, valbuf);
> + regcache_cooked_write (regcache, NIOS2_R3_REGNUM, valbuf + 4);
> + }
> +}
Same as above.
> +
> +/* This function analyzes the function prologue and tries to work
> + out where registers are saved and how long the prologue is.
> + The prologue will consist of the following parts:
> + 1) Optional profiling instrumentation. The old version uses six
> + instructions. We step over this if there is an exact match.
> + nextpc r8
> + mov r9, ra
> + movhi r10, %hiadj(.LP2)
> + addi r10, r10, %lo(.LP2)
> + call mcount
> + mov ra, r9
> + The new version uses two or three instructions (the last of
> + these might get merged in with the STW which saves RA to the
> + stack). We interpret these.
> + mov r8, ra
> + call mcount
> + mov ra, r8
> +
> + 2) Optional interrupt entry decision. Again, we step over
> + this if there is an exact match.
> + rdctl et,estatus
> + andi et,et,1
> + beq et,zero, <software_exception>
> + rdctl et,ipending
> + beq et,zero, <software_exception>
> +
> + 3) A stack adjustment or stack which, which will be one of:
> + addi sp, sp, -constant
> + or:
> + movi r8, constant
> + sub sp, sp, r8
> + or
> + movhi r8, constant
> + addi r8, r8, constant
> + sub sp, sp, r8
> + or
> + movhi rx, %hiadj(newstack)
> + addhi rx, rx, %lo(newstack)
> + stw sp, constant(rx)
> + mov sp, rx
> +
> + 4) An optional stack check, which can take either of these forms:
> + bgeu sp, rx, +8
> + break 3
> + or
> + bltu sp, rx, .Lstack_overflow
> + ...
> + .Lstack_overflow:
> + break 3
> +
> + 5) Saving any registers which need to be saved. These will
> + normally just be stored onto the stack:
> + stw rx, constant(sp)
> + but in the large frame case will use r8 as an offset back
> + to the cfa:
> + add r8, r8, sp
> + stw rx, -constant(r8)
> +
> + Saving control registers looks slightly different:
> + rdctl rx, ctlN
> + stw rx, constant(sp)
> +
> + 6) An optional FP setup, either if the user has requested a
> + frame pointer or if the function calls alloca.
> + This is always:
> + mov fp, sp
> +
> + The prologue instructions may be interleaved, and the register
> + saves and FP setup can occur in either order.
> +
> + To cope with all this variability we decode all the instructions
> + from the start of the prologue until we hit a branch, call or
> + return. For each of the instructions mentioned in 3, 4 and 5 we
> + handle the limited cases of stores to the stack and operations
> + on constant values.
> + */
The closing '*/' should be at the end of the last line. And I would move
that comment right next to the function that this comment documents.
> +typedef struct
> +{
> + unsigned int insn;
> + unsigned int mask;
> +} wild_insn;
This structure needs a comment.
> +/* Helper function to identify when we're in a function epilogue; that is,
> + the part of the function from the point at which the stack adjustment
> + is made, to the return or sibcall. On Nios II, we want to check that the
> + CURRENT_PC is a return-type instruction and that the previous instruction
> + is a stack adjustment instruction.
> + START_PC is the beginning of the function in question. */
Can you reformat to 70 columns?
> +/* Do prologue analysis, returning the PC of the first instruction
> + after the function prologue. Assumes CACHE has already been
> + initialized. THIS_FRAME can be null, in which case we are only
> + interested in skipping the prologue. Otherwise CACHE is filled in
> + from the frame information. */
> +
> +static CORE_ADDR
> +nios2_analyze_prologue (struct gdbarch *gdbarch, const CORE_ADDR start_pc,
> + const CORE_ADDR current_pc,
> + struct nios2_unwind_cache *cache,
> + struct frame_info *this_frame)
> +{
> + /* Maximum lines of prologue to check.
> + Note that this number should not be too large, else we can potentially
> + end up iterating through unmapped memory. */
> + CORE_ADDR limit_pc = start_pc + 200;
I suppose this is good enough for your purposes today, but arbitrary
values like this have a tendency to come and byte you one day...
I was wondering why you couldn't just use current_pc as the limit,
but a comment later on in the function explains that you need to keep
scanning past current_pc... Not sure how you'd fix that, unless you
are always able to determine a function's end address. We can leave
this alone for now.
> + int regno;
> + enum bfd_endian byte_order = gdbarch_byte_order (gdbarch);
> +
> + /* Does the frame set up the FP register? */
> + int base_reg = 0;
> +
> + REG_VALUE * value = cache->reg_value;
No space after the '*'.
> + /* If the first few instructions are the profile entry then skip over them.
> + Newer versions of the compiler use more efficient profiling code. */
Can you reformat this comment, and also any other further comment
to fit within 70 columns?
> + if (nios2_match_sequence (gdbarch, pc, profiler_insn,
> + sizeof (profiler_insn) / sizeof (profiler_insn[0])))
> + pc += ((sizeof (profiler_insn) / sizeof (profiler_insn[0]))
> + * NIOS2_OPCODE_SIZE);
You can use ARRAY_SIZE...
> + pc += ((sizeof (irqentry_insn) / sizeof (irqentry_insn[0]))
> + * NIOS2_OPCODE_SIZE);
Same here...
> +static CORE_ADDR
> +nios2_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 float_argreg;
> + int argnum;
> + int len = 0;
> + int stack_offset = 0;
> + CORE_ADDR func_addr = find_function_addr (function, NULL);
> + enum bfd_endian byte_order = gdbarch_byte_order (gdbarch);
> +
> + /* Set the return address register to point to the entry point of
> + the program, where a breakpoint lies in wait. */
> + regcache_cooked_write_signed (regcache, NIOS2_RA_REGNUM, bp_addr);
> +
> + /* Now make space on the stack for the args. */
> + for (argnum = 0; argnum < nargs; argnum++)
> + len += align_up (TYPE_LENGTH (value_type (args[argnum])), 4);
> + sp -= len;
> +
> + /* Initialize the register pointer. */
> + argreg = FIRST_ARGREG;
> +
> + /* The struct_return pointer occupies the first parameter-passing reg. */
> + if (struct_return)
> + regcache_cooked_write_unsigned (regcache, argreg++, struct_addr);
> +
> + /* Now load as many as possible of the first arguments into
> + registers, and push the rest onto the stack. Loop thru args
> + from first to last. */
"thru" -> "through"
> + for (argnum = 0; argnum < nargs; argnum++)
> + {
> + const gdb_byte *val;
> + gdb_byte valbuf[MAX_REGISTER_SIZE];
> + struct value *arg = args[argnum];
> + struct type *arg_type = check_typedef (value_type (arg));
> + int len = TYPE_LENGTH (arg_type);
> + enum type_code typecode = TYPE_CODE (arg_type);
> +
> + val = value_contents (arg);
> +
> + /* Copy the argument to general registers or the stack in
> + register-sized pieces. Large arguments are split between
> + registers and stack. */
> + while (len > 0)
> + {
> + int partial_len = (len < 4 ? len : 4);
> +
> + if (argreg <= LAST_ARGREG)
> + {
> + /* The argument is being passed in a register. */
> + CORE_ADDR regval = extract_unsigned_integer (val, partial_len,
> + byte_order);
> + regcache_cooked_write_unsigned (regcache, argreg, regval);
> + argreg++;
> + }
> + else
> + {
> + /* The argument is being passed on the stack. */
> + CORE_ADDR addr = sp + stack_offset;
> + write_memory (addr, val, partial_len);
> + stack_offset += align_up (partial_len, 4);
> + }
In both blocks, would you mind adding an empty line after the local
variable declarations? (GDB Coding Style).
> +static struct value *
> +nios2_frame_prev_register (struct frame_info *this_frame, void **this_cache,
> + /* If we've worked out where a register is stored then load it from
> + there. */
> + if (regnum < NIOS2_NUM_REGS
> + && cache->reg_saved[regnum].basereg == NIOS2_Z_REGNUM)
> + return frame_unwind_got_memory (this_frame, regnum,
> + cache->reg_saved[regnum].addr);
Shouldn't we assert that regnum < NIOS2_NUM_REGS?
> +static CORE_ADDR
> +nios2_frame_base_address (struct frame_info *this_frame, void **this_cache)
> +{
> + struct nios2_unwind_cache *info
> + = nios2_frame_unwind_cache (this_frame, this_cache);
> + return info->base;
Empty line between decls and the rest of the function body.
> +static void
> +nios2_stub_frame_this_id (struct frame_info *this_frame, void **this_cache,
> + struct frame_id *this_id)
> +{
> + struct trad_frame_cache *this_trad_cache
> + = nios2_stub_frame_cache (this_frame, this_cache);
> + trad_frame_get_id (this_trad_cache, this_id);
Same here. There are also other areas that need similar fixing.
> Index: gdb/nios2-linux-tdep.c
> ===================================================================
> RCS file: gdb/nios2-linux-tdep.c
> diff -N gdb/nios2-linux-tdep.c
> --- /dev/null 1 Jan 1970 00:00:00 -0000
> +++ gdb/nios2-linux-tdep.c 20 Apr 2013 21:32:50 -0000
> @@ -0,0 +1,227 @@
> +/* Target-dependent code for GNU/Linux on Nios II.
> + Copyright (C) 2012, 2013 Free Software Foundation, Inc.
> + Contributed by Mentor Graphics, 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 <http://www.gnu.org/licenses/>. */
> +
> +#include "defs.h"
> +#include "frame.h"
> +#include "gdb_assert.h"
> +#include "gdb_string.h"
> +#include "osabi.h"
> +#include "solib-svr4.h"
> +#include "trad-frame.h"
> +#include "tramp-frame.h"
> +#include "symtab.h"
> +#include "regset.h"
> +#include "regcache.h"
> +#include "linux-tdep.h"
> +#include "glibc-tdep.h"
> +#include "nios2-tdep.h"
> +
> +#include "features/nios2-linux.c"
> +
> +/* Core file and register set support. */
> +
> +static const int reg_offsets[NIOS2_NUM_REGS] =
> + {-1, 8, 9, 10, 11, 12, 13, 14,
> + 0, 1, 2, 3, 4, 5, 6, 7,
> + 23, 24, 25, 26, 27, 28, 29, 30,
> + -1, -1, 19, 18, 17, 21, -1, 16,
> + 21, -1, 20, -1, -1, -1, -1, -1,
> + -1, -1, -1, -1, -1, -1, -1, -1,
> + -1};
The '{' should be on the first column.
> +/* When FRAME is at a syscall instruction, return the PC of the next
> + instruction to be executed. */
> +
> +static CORE_ADDR
> +nios2_linux_syscall_next_pc (struct frame_info *frame)
Same principle as with gdbarch hook. Just refer to the definition
in the tdep area.
--
Joel