From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 16204 invoked by alias); 8 Mar 2011 18:54:41 -0000 Received: (qmail 16187 invoked by uid 22791); 8 Mar 2011 18:54:38 -0000 X-SWARE-Spam-Status: No, hits=-2.0 required=5.0 tests=AWL,BAYES_00,T_RP_MATCHES_RCVD X-Spam-Check-By: sourceware.org Received: from sibelius.xs4all.nl (HELO glazunov.sibelius.xs4all.nl) (83.163.83.176) by sourceware.org (qpsmtpd/0.43rc1) with ESMTP; Tue, 08 Mar 2011 18:54:31 +0000 Received: from glazunov.sibelius.xs4all.nl (kettenis@localhost [127.0.0.1]) by glazunov.sibelius.xs4all.nl (8.14.3/8.14.3) with ESMTP id p28IsFNx002357; Tue, 8 Mar 2011 19:54:15 +0100 (CET) Received: (from kettenis@localhost) by glazunov.sibelius.xs4all.nl (8.14.3/8.14.3/Submit) id p28IsEs5025452; Tue, 8 Mar 2011 19:54:14 +0100 (CET) Date: Tue, 08 Mar 2011 18:59:00 -0000 Message-Id: <201103081854.p28IsEs5025452@glazunov.sibelius.xs4all.nl> From: Mark Kettenis To: msnyder@vmware.com CC: mark.kettenis@xs4all.nl, jan.kratochvil@redhat.com, gdb-patches@sourceware.org In-reply-to: <4D767783.4090703@vmware.com> (message from Michael Snyder on Tue, 08 Mar 2011 10:37:55 -0800) Subject: Re: [RFA] i386-tdep.c, check target_read_memory for error. References: <4D715BB0.8030506@vmware.com> <20110306141515.GA1895@host1.jankratochvil.net> <201103061455.p26Etppr028003@glazunov.sibelius.xs4all.nl> <4D73D9BC.6020705@vmware.com> <201103072226.p27MQ8Zp004631@glazunov.sibelius.xs4all.nl> <4D767783.4090703@vmware.com> 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: 2011-03/txt/msg00560.txt.bz2 > Date: Tue, 08 Mar 2011 10:37:55 -0800 > From: Michael Snyder > > Thanks for your perseverance. New patch attached. > > There's still one more "return pc + skip" in there, which you > didn't mention. Should I take it out? No, that one is right as far as I can tell. Diff looks good to me. > 2011-03-08 Michael Snyder > > * i386-tdep.c (i386_follow_jump): Check return value of > target_read_memory. > (i386_analyze_struct_return): Ditto. > (i386_skip_probe): Ditto. > (i386_match_insn): Ditto. > (i386_skip_noop): Ditto. > (i386_analyze_frame_setup): Ditto. > (i386_analyze_register_saves): Ditto. > (i386_skip_prologue): Ditto. > (i386_skip_main_prologue): Ditto. > > Index: i386-tdep.c > =================================================================== > RCS file: /cvs/src/src/gdb/i386-tdep.c,v > retrieving revision 1.324 > diff -u -p -r1.324 i386-tdep.c > --- i386-tdep.c 8 Feb 2011 14:01:47 -0000 1.324 > +++ i386-tdep.c 8 Mar 2011 18:32:01 -0000 > @@ -850,7 +850,9 @@ i386_follow_jump (struct gdbarch *gdbarc > long delta = 0; > int data16 = 0; > > - target_read_memory (pc, &op, 1); > + if (target_read_memory (pc, &op, 1)) > + return pc; > + > if (op == 0x66) > { > data16 = 1; > @@ -916,12 +918,15 @@ i386_analyze_struct_return (CORE_ADDR pc > if (current_pc <= pc) > return pc; > > - target_read_memory (pc, &op, 1); > + if (target_read_memory (pc, &op, 1)) > + return pc; > > if (op != 0x58) /* popl %eax */ > return pc; > > - target_read_memory (pc + 1, buf, 4); > + if (target_read_memory (pc + 1, buf, 4)) > + return pc; > + > if (memcmp (buf, proto1, 3) != 0 && memcmp (buf, proto2, 4) != 0) > return pc; > > @@ -960,7 +965,8 @@ i386_skip_probe (CORE_ADDR pc) > gdb_byte buf[8]; > gdb_byte op; > > - target_read_memory (pc, &op, 1); > + if (target_read_memory (pc, &op, 1)) > + return pc; > > if (op == 0x68 || op == 0x6a) > { > @@ -1121,7 +1127,8 @@ i386_match_insn (CORE_ADDR pc, struct i3 > struct i386_insn *insn; > gdb_byte op; > > - target_read_memory (pc, &op, 1); > + if (target_read_memory (pc, &op, 1)) > + return NULL; > > for (insn = skip_insns; insn->len > 0; insn++) > { > @@ -1134,7 +1141,9 @@ i386_match_insn (CORE_ADDR pc, struct i3 > gdb_assert (insn->len > 1); > gdb_assert (insn->len <= I386_MAX_MATCHED_INSN_LEN); > > - target_read_memory (pc + 1, buf, insn->len - 1); > + if (target_read_memory (pc + 1, buf, insn->len - 1)) > + return NULL; > + > for (i = 1; i < insn->len; i++) > { > if ((buf[i - 1] & insn->mask[i]) != insn->insn[i]) > @@ -1212,7 +1221,8 @@ i386_skip_noop (CORE_ADDR pc) > gdb_byte op; > int check = 1; > > - target_read_memory (pc, &op, 1); > + if (target_read_memory (pc, &op, 1)) > + return pc; > > while (check) > { > @@ -1221,7 +1231,8 @@ i386_skip_noop (CORE_ADDR pc) > if (op == 0x90) > { > pc += 1; > - target_read_memory (pc, &op, 1); > + if (target_read_memory (pc, &op, 1)) > + return pc; > check = 1; > } > /* Ignore no-op instruction `mov %edi, %edi'. > @@ -1237,11 +1248,15 @@ i386_skip_noop (CORE_ADDR pc) > > else if (op == 0x8b) > { > - target_read_memory (pc + 1, &op, 1); > + if (target_read_memory (pc + 1, &op, 1)) > + return pc; > + > if (op == 0xff) > { > pc += 2; > - target_read_memory (pc, &op, 1); > + if (target_read_memory (pc, &op, 1)) > + return pc; > + > check = 1; > } > } > @@ -1267,7 +1282,8 @@ i386_analyze_frame_setup (struct gdbarch > if (limit <= pc) > return limit; > > - target_read_memory (pc, &op, 1); > + if (target_read_memory (pc, &op, 1)) > + return pc; > > if (op == 0x55) /* pushl %ebp */ > { > @@ -1302,7 +1318,8 @@ i386_analyze_frame_setup (struct gdbarch > if (limit <= pc + skip) > return limit; > > - target_read_memory (pc + skip, &op, 1); > + if (target_read_memory (pc + skip, &op, 1)) > + return pc + skip; > > /* Check for `movl %esp, %ebp' -- can be written in two ways. */ > switch (op) > @@ -1338,7 +1355,8 @@ i386_analyze_frame_setup (struct gdbarch > > NOTE: You can't subtract a 16-bit immediate from a 32-bit > reg, so we don't have to worry about a data16 prefix. */ > - target_read_memory (pc, &op, 1); > + if (target_read_memory (pc, &op, 1)) > + return pc; > if (op == 0x83) > { > /* `subl' with 8-bit immediate. */ > @@ -1394,7 +1412,8 @@ i386_analyze_register_saves (CORE_ADDR p > offset -= cache->locals; > for (i = 0; i < 8 && pc < current_pc; i++) > { > - target_read_memory (pc, &op, 1); > + if (target_read_memory (pc, &op, 1)) > + return pc; > if (op < 0x50 || op > 0x57) > break; > > @@ -1487,7 +1506,9 @@ i386_skip_prologue (struct gdbarch *gdba > > for (i = 0; i < 6; i++) > { > - target_read_memory (pc + i, &op, 1); > + if (target_read_memory (pc + i, &op, 1)) > + return pc; > + > if (pic_pat[i] != op) > break; > } > @@ -1495,7 +1516,8 @@ i386_skip_prologue (struct gdbarch *gdba > { > int delta = 6; > > - target_read_memory (pc + delta, &op, 1); > + if (target_read_memory (pc + delta, &op, 1)) > + return pc; > > if (op == 0x89) /* movl %ebx, x(%ebp) */ > { > @@ -1508,7 +1530,8 @@ i386_skip_prologue (struct gdbarch *gdba > else /* Unexpected instruction. */ > delta = 0; > > - target_read_memory (pc + delta, &op, 1); > + if (target_read_memory (pc + delta, &op, 1)) > + return pc; > } > > /* addl y,%ebx */ > @@ -1538,7 +1561,8 @@ i386_skip_main_prologue (struct gdbarch > enum bfd_endian byte_order = gdbarch_byte_order (gdbarch); > gdb_byte op; > > - target_read_memory (pc, &op, 1); > + if (target_read_memory (pc, &op, 1)) > + return pc; > if (op == 0xe8) > { > gdb_byte buf[4]; > > --------------090807040303000008010100-- >