From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 19158 invoked by alias); 13 Jul 2009 00:38:56 -0000 Received: (qmail 19150 invoked by uid 22791); 13 Jul 2009 00:38:55 -0000 X-SWARE-Spam-Status: No, hits=-2.4 required=5.0 tests=AWL,BAYES_00 X-Spam-Check-By: sourceware.org Received: from smtp-outbound-1.vmware.com (HELO smtp-outbound-1.vmware.com) (65.115.85.69) by sourceware.org (qpsmtpd/0.43rc1) with ESMTP; Mon, 13 Jul 2009 00:38:49 +0000 Received: from mailhost3.vmware.com (mailhost3.vmware.com [10.16.27.45]) by smtp-outbound-1.vmware.com (Postfix) with ESMTP id 1AE9F3F004; Sun, 12 Jul 2009 17:38:45 -0700 (PDT) Received: from [10.20.94.141] (msnyder-server.eng.vmware.com [10.20.94.141]) by mailhost3.vmware.com (Postfix) with ESMTP id 0C8A1CD9F6; Sun, 12 Jul 2009 17:38:45 -0700 (PDT) Message-ID: <4A5A810B.7080603@vmware.com> Date: Mon, 13 Jul 2009 00:52:00 -0000 From: Michael Snyder User-Agent: Thunderbird 1.5.0.12 (X11/20080411) MIME-Version: 1.0 To: Hui Zhu CC: gdb-patches ml Subject: Re: [RFA/RFC Prec] Add Linux AMD64 process record support second version, (instruction set support) 1/3 References: In-Reply-To: Content-Type: text/plain; charset=ISO-8859-1; format=flowed Content-Transfer-Encoding: 7bit X-IsSubscribed: yes 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 X-SW-Source: 2009-07/txt/msg00351.txt.bz2 Hui Zhu wrote: > Because AMD64 instruction set is the extend of I386 instruction set. > So I update the function i386_process_record to make it support AMD64 > instruction set. some of other software do something like it. > 2009-07-07 Hui Zhu > > Add AMD64 process record instruction set support. > > * i386-tdep.h (gdbarch_tdep): Add record_regmap for registers > because the AMD64's registers order in GDB is not same with > I386 instructions. > Add i386_syscall_record to be the syscall function handle > interface. > (record_i386_regnum): Number for record_regmap. > * i386-tdep.c (OT_QUAD): For 64 bits. > (i386_record_s): Add rex_x, rex_b, rip_offset and > popl_esp_hack for AMD64 instruction set. And regmap for > record_regmap. > (i386_record_lea_modrm_addr): Support AMD64 instruction set > 64 bits lea. > (i386_record_lea_modrm): Ditto. > (i386_record_push): New function. Record the execution log > of push. > (I386_RECORD_ARCH_LIST_ADD_REG): New macro to record the > register. > (i386_process_record): Support AMD64 instruction set. > amd64-tdep.c (amd64_record_regmap): For record_regmap. > (amd64_init_abi): Set amd64_record_regmap to record_regmap. OK, this is kind of preliminary -- mostly concerned with whitespace / formatting. I'll try to do more later. > @@ -2859,7 +2868,7 @@ i386_record_lea_modrm_addr (struct i386_ > if ((base & 7) == 5) > { > base = 0xff; > - if (target_read_memory (irp->addr, (gdb_byte *) addr, 4)) > + if (target_read_memory (irp->addr, (gdb_byte *)&tmpi32, 4)) Space after a cast, please. There are many instances of this, I won't try to note them all. > @@ -2884,10 +2896,10 @@ i386_record_lea_modrm_addr (struct i386_ > return -1; > } > irp->addr++; > - *addr = (int8_t) tmpu8; > + *addr = (int8_t)tmpu8; Space after cast > @@ -2949,10 +2974,10 @@ i386_record_lea_modrm_addr (struct i386_ > return -1; > } > irp->addr++; > - *addr = (int8_t) tmpu8; > + *addr = (int8_t)tmpu8; > break; > case 2: > - if (target_read_memory (irp->addr, (gdb_byte *) & tmpu16, 2)) > + if (target_read_memory (irp->addr, (gdb_byte *) & tmpi16, 2)) No space after '&' operator. In general, we use a space both before and after a binary operator (such as 'plus'), but no space after a unary operator such as '&', '*' or '-'. > + if (ir.aflag == 2) > + { > + if (target_read_memory > + (ir.addr, (gdb_byte *)&addr, 8)) Please try to avoid doing this (putting the left-paren of a function call on the next line). Occasionally when a line gets really long I tend to overlook it, but in this case it would be really easy to divide the line up like this: if (target_read_memory (ir.addr, (gdb_byte *) &addr, 8)) > + if (ir.mod ==3) Space after == > + switch (ir.dflag) > + { > + case 0: > + tmpu64 += ((int16_t)tmpulongest >> 4)<< 4; Space before <<, space after cast. > + break; > + case 1: > + tmpu64 += ((int32_t)tmpulongest >> 5)<< 5; Ditto > + break; > + case 2: > + tmpu64 += ((int64_t)tmpulongest >> 6)<< 6; Ditto > + /* The map for registers because the AMD64's registers order in GDB is not > + same with I386 instructions. */ "same as". And could you shorten the first line please?