From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 125256 invoked by alias); 11 Jul 2017 02:56:53 -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 125244 invoked by uid 89); 11 Jul 2017 02:56:52 -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,GIT_PATCH_0,GIT_PATCH_1,GIT_PATCH_2,GIT_PATCH_3,RCVD_IN_SORBS_SPAM,RP_MATCHES_RCVD,SPF_PASS autolearn=ham version=3.3.2 spammy= X-HELO: aserp1040.oracle.com Received: from aserp1040.oracle.com (HELO aserp1040.oracle.com) (141.146.126.69) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with ESMTP; Tue, 11 Jul 2017 02:56:45 +0000 Received: from userv0021.oracle.com (userv0021.oracle.com [156.151.31.71]) by aserp1040.oracle.com (Sentrion-MTA-4.3.2/Sentrion-MTA-4.3.2) with ESMTP id v6B2ugYG001745 (version=TLSv1.2 cipher=ECDHE-RSA-AES256-GCM-SHA384 bits=256 verify=OK); Tue, 11 Jul 2017 02:56:42 GMT Received: from userv0121.oracle.com (userv0121.oracle.com [156.151.31.72]) by userv0021.oracle.com (8.14.4/8.14.4) with ESMTP id v6B2ufgR015912 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-GCM-SHA384 bits=256 verify=OK); Tue, 11 Jul 2017 02:56:41 GMT Received: from abhmp0012.oracle.com (abhmp0012.oracle.com [141.146.116.18]) by userv0121.oracle.com (8.14.4/8.13.8) with ESMTP id v6B2ufrf028851; Tue, 11 Jul 2017 02:56:41 GMT Received: from [10.159.228.114] (/10.159.228.114) by default (Oracle Beehive Gateway v4.0) with ESMTP ; Mon, 10 Jul 2017 19:56:41 -0700 Subject: Re: [PING] [PATCH v3] gdb: ADI support To: Yao Qi Cc: gdb-patches@sourceware.org References: <1499365761-83719-1-git-send-email-weimin.pan@oracle.com> <86d1988tmp.fsf@gmail.com> From: Wei-min Pan Message-ID: <34218edb-9c6b-4989-62f3-dd7afb0e3511@oracle.com> Date: Tue, 11 Jul 2017 02:56:00 -0000 User-Agent: Mozilla/5.0 (Windows NT 6.1; WOW64; rv:52.0) Gecko/20100101 Thunderbird/52.2.1 MIME-Version: 1.0 In-Reply-To: <86d1988tmp.fsf@gmail.com> Content-Type: text/plain; charset=utf-8; format=flowed Content-Transfer-Encoding: 7bit X-SW-Source: 2017-07/txt/msg00089.txt.bz2 With your comments like: * You can't access /proc in *-tdep.c file, because it is also compiled for cross-debugger (from previous review); * We can't include "nat.h" in "tdep.c"; * Calling pread64 in -tdep.c makes few sense to me. If you only want to support native debugging, move them to -nat.c file. I now believe that the ADI support should be in sparc6 4-linux-nat.c, not sparc64-tdep.c. Doing so also makes cross gdb build a non-issue. Our QA team has been working on an ADI test case which they will submit upstream separately. Hope that's OK. Will address your other comments in separate emails. Thanks. On 7/10/2017 3:37 AM, Yao Qi wrote: > Weimin Pan writes: > >> Tested in sparc64-linux-gnu. No regressions. >> > Did you build a cross gdb on x86-linux? At least it breaks the build > for all targets, I configure gdb this way, > > "../binutils-gdb/configure --enable-sim --disable-binutils > --disable-gprof --disable-gold --disable-gas --disable-ld > --enable-targets=all --enable-64-bit-bfd" > > sparc64-tdep.o: In function `adi_tag_fd()': > build-gdb/gdb/../../binutils-gdb/gdb/sparc64-tdep.c:1915: undefined reference to `open_adi_tag_fd(int)' > > because sparc64-tdep.o references function defined in sparc64-linux-nat.c. > >> gdb/ChangeLog: >> 2017-06-26 Weimin Pan > A blank line is needed here. > >> * sparc64-tdep.h: (adi_normalize_address): New export. >> * sparc-nat.h: (open_adi_tag_fd): New export. >> * sparc64-linux-nat.c: (open_adi_tag_fd): New function. >> * sparc64-linux-tdep.c: >> (SEGV_ACCADI, SEGV_ADIDERR, SEGV_ADIPERR) New defines. >> (sparc64_linux_handle_segmentation_fault): New function. >> (sparc64_linux_init_abi): Register >> sparc64_linux_handle_segmentation_fault >> * sparc64-tdep.c: Include cli-utils.h,gdbcmd.h,auxv.h,sparc-nat.h. >> (sparc64_pstate_type): Replace PID1 with MCDE. >> (sparc64_addr_bits_remove): New function. >> (sparc64_init_abi): Register sparc64_addr_bits_remove. >> (MAX_PROC_NAME_SIZE): New macro. >> (AT_ADI_BLKSZ, AT_ADI_NBITS, AT_ADI_UEONADI) New defines. >> (sparc64adilist): New variable. >> (adi_proc_list): New variable. >> (find_adi_info): New function. >> (add_adi_info): New function. >> (get_adi_info_proc): New function. >> (get_adi_info): New function. >> (info_adi_command): New function. >> (read_maps_entry): New function. >> (adi_available): New function. >> (adi_normalize_address): New function. >> (adi_align_address): New function. >> (adi_convert_byte_count): New function. >> (adi_tag_fd): New function. >> (adi_is_addr_mapped): New function. >> (adi_read_versions): New function. >> (adi_write_versions): New function. >> (adi_print_versions): New function. >> (do_examine): New function. >> (do_assign): New function. >> (adi_examine_command): New function. >> (adi_assign_command): New function. >> (_initialize_sparc64_tdep): New function. >> >> gdb/doc/ChangeLog: >> 2017-06-26 Weimin Pan >> * gdb.texinfo (Architectures): Add new Sparc64 section to document >> ADI support. >> * NEWS: Add "adi examine" and "adi assign" commands. > Can you add a test case? > >> + >> +Below is an example of displaying ADI versions of variable "shmaddr". >> + >> +@smallexample >> +(@value{GDBP}) adi x/100 shmaddr >> + 0xfff800010002c000: 0 0 >> +@end smallexample > Why "0" is printed twice? > >> +void >> +sparc64_linux_handle_segmentation_fault (struct gdbarch *gdbarch, >> + struct ui_out *uiout) >> +{ >> + if (gdbarch_bfd_arch_info (gdbarch)->bits_per_word != 64) >> + return; >> + >> + CORE_ADDR addr = 0; >> + long si_code = 0; >> + >> + TRY >> + { >> + /* Evaluate si_code to see if the segfault is ADI related. */ >> + si_code = parse_and_eval_long ("$_siginfo.si_code\n"); >> + >> + if (si_code >= SEGV_ACCADI && si_code <= SEGV_ADIPERR) >> + addr = parse_and_eval_long ("$_siginfo._sifields._sigfault.si_addr"); >> + } >> + CATCH (exception, RETURN_MASK_ALL) >> + { >> + return; >> + } >> + END_CATCH >> + >> + /* Print out ADI event based on sig_code value */ >> + switch (si_code) >> + { >> + case SEGV_ACCADI: /* adi not enabled */ >> + uiout->text ("\n"); >> + uiout->field_string ("sigcode-meaning", _("ADI disabled")); >> + uiout->text (_(" while accessing address ")); >> + uiout->field_fmt ("bound-access", "%s", paddress (gdbarch, addr)); >> + break; >> + case SEGV_ADIDERR: /* disrupting mismatch */ >> + uiout->text ("\n"); >> + uiout->field_string ("sigcode-meaning", _("ADI deferred mismatch")); >> + uiout->text (_(" while accessing address ")); >> + uiout->field_fmt ("bound-access", "%s", paddress (gdbarch, addr)); >> + break; >> + case SEGV_ADIPERR: /* precise mismatch */ >> + uiout->text ("\n"); >> + uiout->field_string ("sigcode-meaning", _("ADI precise mismatch")); >> + uiout->text (_(" while accessing address ")); >> + uiout->field_fmt ("bound-access", "%s", paddress (gdbarch, addr)); >> + break; >> + default: >> + break; >> + } > It is good to have a test to check the results and outputs here. > >> +} >> + >> >> /* Return the address of a system call's alternative return >> address. */ >> @@ -338,6 +404,8 @@ sparc64_linux_init_abi (struct gdbarch_info info, struct gdbarch *gdbarch) >> set_xml_syscall_file_name (gdbarch, XML_SYSCALL_FILENAME_SPARC64); >> set_gdbarch_get_syscall_number (gdbarch, >> sparc64_linux_get_syscall_number); >> + set_gdbarch_handle_segmentation_fault (gdbarch, >> + sparc64_linux_handle_segmentation_fault); >> } >> >> >> diff --git a/gdb/sparc64-tdep.c b/gdb/sparc64-tdep.c >> index 9e0e6b5..9fa8e13 100644 >> --- a/gdb/sparc64-tdep.c >> +++ b/gdb/sparc64-tdep.c >> @@ -35,7 +35,11 @@ >> #include "target.h" >> #include "value.h" >> >> +#include "cli/cli-utils.h" >> +#include "gdbcmd.h" >> +#include "auxv.h" >> #include "sparc64-tdep.h" >> +#include "sparc-nat.h" >> > We can't include "nat.h" in "tdep.c". > >> /* This file implements the SPARC 64-bit ABI as defined by the >> section "Low-Level System Information" of the SPARC Compliance >> @@ -165,7 +169,7 @@ sparc64_pstate_type (struct gdbarch *gdbarch) >> append_flags_type_flag (type, 8, "TLE"); >> append_flags_type_flag (type, 9, "CLE"); >> append_flags_type_flag (type, 10, "PID0"); >> - append_flags_type_flag (type, 11, "PID1"); >> + append_flags_type_flag (type, 11, "MCDE"); >> > I don't understand your reply regarding to this change, > >> This change, which indicated that the TSTATE reigster contained the >> "mcde" value, was requested by our kernel engineers. > How is it related to the adi support? If it is not, split it to another > patch. In any case, we need a technical reason for the change. > >> tdep->sparc64_pstate_type = type; >> } >> @@ -1290,6 +1294,14 @@ sparc64_dwarf2_frame_init_reg (struct gdbarch *gdbarch, int regnum, >> } >> } >> >> +/* sparc64_addr_bits_remove - remove useless address bits */ >> + >> +static CORE_ADDR >> +sparc64_addr_bits_remove (struct gdbarch *gdbarch, CORE_ADDR addr) >> +{ >> + return adi_normalize_address (addr); > adi_normalize_address can be a static function, and other .c file can > use it via gdbarch_addr_bits_remove. > >> +} >> + >> void >> sparc64_init_abi (struct gdbarch_info info, struct gdbarch *gdbarch) >> { >> @@ -1342,6 +1354,8 @@ sparc64_init_abi (struct gdbarch_info info, struct gdbarch *gdbarch) >> >> frame_unwind_append_unwinder (gdbarch, &sparc64_frame_unwind); >> frame_base_set_default (gdbarch, &sparc64_frame_base); >> + >> + set_gdbarch_addr_bits_remove (gdbarch, sparc64_addr_bits_remove); >> } >> >> >> @@ -1666,3 +1680,495 @@ const struct sparc_fpregmap sparc64_bsd_fpregmap = >> 0 * 8, /* %f0 */ >> 32 * 8, /* %fsr */ >> }; >> + >> +/* The M7 processor supports an Application Data Integrity (ADI) feature >> + that detects invalid data accesses. When software allocates memory and >> + enables ADI on the allocated memory, it chooses a 4-bit version number, >> + sets the version in the upper 4 bits of the 64-bit pointer to that data, >> + and stores the 4-bit version in every cacheline of the object. Hardware >> + saves the latter in spare bits in the cache and memory hierarchy. On each >> + load and store, the processor compares the upper 4 VA (virtual address) bits >> + to the cacheline's version. If there is a mismatch, the processor generates >> + a version mismatch trap which can be either precise or disrupting. >> + The trap is an error condition which the kernel delivers to the process >> + as a SIGSEGV signal. >> + >> + The upper 4 bits of the VA represent a version and are not part of the >> + true address. The processor clears these bits and sign extends bit 59 >> + to generate the true address. >> + >> + Note that 32-bit applications cannot use ADI. */ >> + >> + >> +#define MAX_PROC_NAME_SIZE sizeof("/proc/99999/lwp/9999/adi/lstatus") >> + >> +/* ELF Auxiliary vectors */ >> +#ifndef AT_ADI_BLKSZ >> +#define AT_ADI_BLKSZ 34 >> +#endif >> +#ifndef AT_ADI_NBITS >> +#define AT_ADI_NBITS 35 >> +#endif >> +#ifndef AT_ADI_UEONADI >> +#define AT_ADI_UEONADI 36 >> +#endif >> + >> +/* ADI command list. */ >> +static struct cmd_list_element *sparc64adilist = NULL; >> + >> +/* ADI stat settings. */ >> +struct adi_stat_t >> +{ >> + /* The ADI block size. */ >> + unsigned long blksize; >> + >> + /* Number of bits used for an ADI version tag which can be >> + * used together with the shift value for an ADI version tag >> + * to encode or extract the ADI version value in a pointer. */ >> + unsigned long nbits; >> + >> + /* The maximum ADI version tag value supported. */ >> + int max_version; >> + >> + /* ADI version tag file. */ >> + int tag_fd; >> + >> + /* Last ADI address examined. */ >> + CORE_ADDR last_vaddr; >> + >> + /* Last specified examination count. */ >> + int last_cnt; >> + >> + /* ADI availability check has been done. */ >> + bool checked_avail; >> + >> + /* ADI is available. */ >> + bool is_avail; >> + >> +}; >> + >> +/* Per-process ADI stat info. */ >> + >> +struct sparc64_adi_info >> +{ >> + /* The process identifier. */ >> + pid_t pid; >> + >> + /* The ADI stat. */ >> + struct adi_stat_t stat; >> + >> + /* Linked list. */ >> + struct sparc64_adi_info *next; > Can you use C++ STL list? > >> +}; >> + >> +static struct sparc64_adi_info *adi_proc_list = NULL; >> + >> +/* Find ADI info for process PID. */ >> + >> +static struct sparc64_adi_info * >> +find_adi_info (pid_t pid) >> +{ >> + struct sparc64_adi_info *proc; >> + >> + for (proc = adi_proc_list; proc; proc = proc->next) >> + if (proc->pid == pid) >> + return proc; >> + >> + return NULL; >> +} >> + >> +/* Add ADI info for process PID. Returns newly allocated info >> + object. */ >> + >> +static struct sparc64_adi_info * >> +add_adi_info (pid_t pid) >> +{ >> + struct sparc64_adi_info *proc = XCNEW (struct sparc64_adi_info); >> + >> + proc->pid = pid; >> + proc->next = adi_proc_list; >> + adi_proc_list = proc; >> + proc->stat.is_avail = false; >> + proc->stat.checked_avail = false; >> + proc->stat.tag_fd = 0; >> + >> + return proc; >> +} >> + >> +/* Get ADI info for process PID, creating one if it doesn't exist. */ >> + >> +static struct sparc64_adi_info * >> +get_adi_info_proc (pid_t pid) >> +{ >> + struct sparc64_adi_info *proc; >> + >> + proc = find_adi_info (pid); >> + if (proc == NULL) >> + proc = add_adi_info (pid); >> + >> + return proc; >> +} > I don't see the place you remove elements from adi list. You probably > need to implement sparc64_forget_process (in which you can delete the > adi element if the process exits), and hook it on via > "linux_nat_set_forget_process (t, sparc64_forget_process);". > >> + >> +static struct adi_stat_t >> +get_adi_info (pid_t pid) >> +{ >> + struct sparc64_adi_info *proc; >> + >> + proc = get_adi_info_proc (pid); >> + return proc->stat; >> +} >> + >> +static void >> +info_adi_command (char *args, int from_tty) >> +{ >> + printf_unfiltered ("\"adi\" must be followed by \"examine\" " >> + "or \"assign\".\n"); >> + help_list (sparc64adilist, "adi ", all_commands, gdb_stdout); >> +} >> + >> +/* Read attributes of a maps entry in /proc/[pid]/adi/maps. */ >> + >> +static void >> +read_maps_entry (const char *line, >> + ULONGEST *addr, ULONGEST *endaddr) >> +{ >> + const char *p = line; >> + >> + *addr = strtoulst (p, &p, 16); >> + if (*p == '-') >> + p++; >> + >> + *endaddr = strtoulst (p, &p, 16); >> +} >> + >> +/* Check if ADI is available. */ >> + >> +static bool >> +adi_available (void) >> +{ >> + pid_t pid = ptid_get_pid (inferior_ptid); >> + struct sparc64_adi_info *proc = get_adi_info_proc (pid); >> + >> + if (proc->stat.checked_avail) >> + return proc->stat.is_avail; >> + >> + proc->stat.checked_avail = true; >> + if (target_auxv_search (¤t_target, AT_ADI_BLKSZ, &proc->stat.blksize) <= 0) > THis line is too long. > >> + return false; >> + target_auxv_search (¤t_target, AT_ADI_NBITS, &proc->stat.nbits); >> + proc->stat.max_version = (1 << proc->stat.nbits) - 2; >> + proc->stat.is_avail = true; >> + >> + return proc->stat.is_avail; >> +} >> + >> +/* Normalize a versioned address - a VA with ADI bits (63-60) set. */ >> + >> +CORE_ADDR >> +adi_normalize_address (CORE_ADDR addr) >> +{ >> + struct adi_stat_t adi_stat = get_adi_info (ptid_get_pid (inferior_ptid)); >> + >> + if (adi_stat.nbits) >> + return ((CORE_ADDR)(((long)addr << adi_stat.nbits) >> adi_stat.nbits)); >> + return addr; >> +} >> + >> +/* Align a normalized address - a VA with bit 59 sign extended into ADI bits. */ >> + >> +static CORE_ADDR >> +adi_align_address (CORE_ADDR naddr) >> +{ >> + struct adi_stat_t adi_stat = get_adi_info (ptid_get_pid (inferior_ptid)); >> + >> + return (naddr - (naddr % adi_stat.blksize)) / adi_stat.blksize; >> +} >> + >> +/* Convert a byte count to count at a ratio of 1:adi_blksz. */ >> + >> +static int >> +adi_convert_byte_count (CORE_ADDR naddr, int nbytes, CORE_ADDR locl) >> +{ >> + struct adi_stat_t adi_stat = get_adi_info (ptid_get_pid (inferior_ptid)); >> + >> + return ((naddr + nbytes + adi_stat.blksize - 1) / adi_stat.blksize) - locl; >> +} >> + >> +/* The /proc/[pid]/adi/tags file, which allows gdb to get/set ADI >> + version in a target process, maps linearly to the address space >> + of the target process at a ratio of 1:adi_blksz. >> + >> + A read (or write) at offset K in the file returns (or modifies) >> + the ADI version tag stored in the cacheline containing address >> + K * adi_blksz, encoded as 1 version tag per byte. The allowed >> + version tag values are between 0 and adi_stat.max_version. */ >> + >> +static int >> +adi_tag_fd (void) >> +{ >> + pid_t pid = ptid_get_pid (inferior_ptid); >> + struct sparc64_adi_info *proc = get_adi_info_proc (pid); >> + >> + if (proc->stat.tag_fd != 0) >> + return proc->stat.tag_fd; >> + >> + proc->stat.tag_fd = open_adi_tag_fd (pid); >> + return proc->stat.tag_fd; >> +} >> + >> +/* Check if an address set is ADI enabled, using /proc/[pid]/adi/maps >> + which was exported by the kernel and contains the currently ADI >> + mapped memory regions and their access permissions. */ >> + >> +static bool >> +adi_is_addr_mapped (CORE_ADDR vaddr, size_t cnt) >> +{ >> + char filename[MAX_PROC_NAME_SIZE]; >> + size_t i = 0; >> + >> + pid_t pid = ptid_get_pid (inferior_ptid); >> + snprintf (filename, sizeof filename, "/proc/%d/adi/maps", pid); >> + char *data = target_fileio_read_stralloc (NULL, filename); >> + if (data) >> + { >> + struct cleanup *cleanup = make_cleanup (xfree, data); >> + struct adi_stat_t adi_stat = get_adi_info (pid); >> + char *line; >> + for (line = strtok (data, "\n"); line; line = strtok (NULL, "\n")) >> + { >> + ULONGEST addr, endaddr; >> + >> + read_maps_entry (line, &addr, &endaddr); >> + >> + while (((vaddr + i) * adi_stat.blksize) >= addr >> + && ((vaddr + i) * adi_stat.blksize) < endaddr) >> + { >> + if (++i == cnt) >> + { >> + do_cleanups (cleanup); >> + return true; >> + } >> + } >> + } >> + do_cleanups (cleanup); >> + } >> + else >> + warning (_("unable to open /proc file '%s'"), filename); >> + >> + return false; >> +} >> + >> +/* Read ADI version tag value for memory locations starting at "vaddr" >> + for "size" number of bytes. */ >> + >> +static int >> +adi_read_versions (CORE_ADDR vaddr, size_t size, unsigned char *tags) >> +{ >> + int fd = adi_tag_fd (); >> + if (fd == -1) >> + return -1; >> + >> + if (!adi_is_addr_mapped (vaddr, size)) >> + { >> + struct adi_stat_t adi_stat = get_adi_info (ptid_get_pid (inferior_ptid)); >> + error(_("Address at 0x%lx is not in ADI maps"), vaddr*adi_stat.blksize); >> + } >> + >> + return pread64 (fd, tags, size, vaddr); >> +} > Calling pread64 in -tdep.c makes few sense to me. If you only want to > support native debugging, move them to -nat.c file. > >> + >> +/* Write ADI version tag for memory locations starting at "vaddr" for >> + "size" number of bytes to "tags". */ >> + >> +static int >> +adi_write_versions (CORE_ADDR vaddr, size_t size, unsigned char *tags) >> +{ >> + int fd = adi_tag_fd (); >> + if (fd == -1) >> + return -1; >> + >> + if (!adi_is_addr_mapped (vaddr, size)) >> + { >> + struct adi_stat_t adi_stat = get_adi_info (ptid_get_pid (inferior_ptid)); >> + error(_("Address at 0x%lx is not in ADI maps"), vaddr*adi_stat.blksize); >> + } >> + >> + return pwrite64 (fd, tags, size, vaddr); >> +} >> + >> +/* Print ADI version tag value in "tags" for memory locations starting >> + at "vaddr" with number of "cnt". */ >> + >> +static void >> +adi_print_versions (CORE_ADDR vaddr, size_t cnt, unsigned char *tags) >> +{ >> + int v_idx = 0; >> + const int maxelts = 8; /* # of elements per line */ >> + >> + struct adi_stat_t adi_stat = get_adi_info (ptid_get_pid (inferior_ptid)); >> + >> + while (cnt > 0) >> + { >> + QUIT; >> + printf_filtered ("0x%016lx:\t", vaddr * adi_stat.blksize); >> + for (int i = maxelts; i > 0 && cnt > 0; i--, cnt--) >> + { >> + if (tags[v_idx] == 0xff) /* no version tag */ >> + printf_filtered ("- "); >> + else >> + printf_filtered ("%1X ", tags[v_idx]); >> + ++v_idx; >> + } >> + printf_filtered ("\n"); >> + gdb_flush (gdb_stdout); >> + vaddr += maxelts; >> + } >> +} >> + >> +static void >> +do_examine (CORE_ADDR start, int bcnt) >> +{ >> + CORE_ADDR vaddr = adi_normalize_address (start); >> + struct cleanup *cleanup; >> + >> + CORE_ADDR vstart = adi_align_address (vaddr); >> + int cnt = adi_convert_byte_count (vaddr, bcnt, vstart); >> + unsigned char *buf = (unsigned char *) xmalloc (cnt); >> + cleanup = make_cleanup (xfree, buf); >> + int read_cnt = adi_read_versions (vstart, cnt, buf); >> + if (read_cnt == -1) >> + error (_("No ADI information")); >> + else if (read_cnt < cnt) >> + error(_("No ADI information at 0x%lx"), vaddr); >> + >> + adi_print_versions (vstart, cnt, buf); >> + >> + do_cleanups (cleanup); >> +} >> + >> +static void >> +do_assign (CORE_ADDR start, size_t bcnt, int version) >> +{ >> + CORE_ADDR vaddr = adi_normalize_address (start); >> + >> + CORE_ADDR vstart = adi_align_address (vaddr); >> + int cnt = adi_convert_byte_count (vaddr, bcnt, vstart); >> + unsigned char *buf = (unsigned char *) xmalloc (cnt); > Use std::vector? so that you don't need memset and xfree. > >> + memset(buf, version, cnt); >> + >> + int set_cnt = adi_write_versions (vstart, cnt, buf); >> + xfree (buf); >> + >> + if (set_cnt == -1) >> + error (_("No ADI information")); >> + else if (set_cnt < cnt) >> + error(_("No ADI information at 0x%lx"), vaddr); >> + >> +} >> + >> +/* ADI examine version tag command. >> + >> + Command syntax: >> + >> + adi (examine|x)/count */ >> + >> +static void >> +adi_examine_command (char *args, int from_tty) >> +{ >> + /* make sure program is active and adi is available */ >> + if (!target_has_execution) >> + error (_("ADI command requires a live process/thread")); >> + >> + if (!adi_available ()) >> + error (_("No ADI information")); >> + >> + pid_t pid = ptid_get_pid (inferior_ptid); >> + struct sparc64_adi_info *proc = get_adi_info_proc (pid); >> + int cnt = proc->stat.last_cnt? proc->stat.last_cnt : 1; >> + char *p = args; >> + if (p && *p == '/') >> + { >> + p++; >> + cnt = get_number (&p); >> + } >> + >> + CORE_ADDR next_address = proc->stat.last_vaddr ? proc->stat.last_vaddr : 0; >> + if (p != 0 && *p != 0) >> + next_address = parse_and_eval_address (p); >> + else if (!proc->stat.last_cnt) >> + error (_("Usage: adi examine|x[/count] ")); > IIUC, "address" in this command is mandatory, and it makes sense to do > so. We don't need .last_vaddr. I don't like such variable making the > command stateful. >