|
From: Yao Qi <qiy...@cn...> - 2005-11-28 11:59:36
|
Could you review my two patches? They are suspended for sometimes, http://sourceforge.net/mailarchive/forum.php?thread_id=8973651&forum_id=12302 http://sourceforge.net/mailarchive/forum.php?thread_id=8964035&forum_id=12302 Thanks in advance! -- Regards, Yao ------------ Yao Qi |
|
From: Feiyang C. <chr...@gm...> - 2022-10-25 07:32:09
|
Hi, The series is now rebased on 3.20.0. Could you review this series? https://bugs.kde.org/show_bug.cgi?id=457504 Thanks, Feiyang |
|
From: Feiyang C. <chr...@gm...> - 2022-12-07 02:27:41
|
Ping :) |
|
From: Tom H. <to...@co...> - 2005-11-28 12:29:22
|
In message <200...@cn...>
Yao Qi <qiy...@cn...> wrote:
> Could you review my two patches? They are suspended for sometimes,
>
> http://sourceforge.net/mailarchive/forum.php?thread_id=8973651&forum_id=12302
> http://sourceforge.net/mailarchive/forum.php?thread_id=8964035&forum_id=12302
>
> Thanks in advance!
I committed the README_DEVELOPERS one (or something based on it) as
revision 5165 on the 11th November.
The other one is one for Julian I think.
Tom
--
Tom Hughes (to...@co...)
http://www.compton.nu/
|
|
From: Yao Qi <qiy...@cn...> - 2005-11-29 03:21:35
|
On Mon, Nov 28, 2005 at 12:29:13PM +0000, Tom Hughes wrote: > In message <200...@cn...> > Yao Qi <qiy...@cn...> wrote: > > > Could you review my two patches? They are suspended for sometimes, > > > > http://sourceforge.net/mailarchive/forum.php?thread_id=8973651&forum_id=12302 > > http://sourceforge.net/mailarchive/forum.php?thread_id=8964035&forum_id=12302 > > > > Thanks in advance! > > I committed the README_DEVELOPERS one (or something based on it) as > revision 5165 on the 11th November. Thank you very much! BTW, how do you think about the option "--wait-for-gdb"? Personally, I think this option is not necessary and maybe we could remove it? Any thought on this? I would like to code a patch for it if you confirm it. > > The other one is one for Julian I think. > > Tom > > -- > Tom Hughes (to...@co...) > http://www.compton.nu/ > > > ------------------------------------------------------- > This SF.net email is sponsored by: Splunk Inc. Do you grep through log files > for problems? Stop! Download the new AJAX search engine that makes > searching your log files as easy as surfing the web. DOWNLOAD SPLUNK! > http://ads.osdn.com/?ad_id=7637&alloc_id=16865&op=click > _______________________________________________ > Valgrind-developers mailing list > Val...@li... > https://lists.sourceforge.net/lists/listinfo/valgrind-developers > -- Regards, Yao ------------ Yao Qi |
|
From: Julian S. <js...@ac...> - 2005-11-28 13:03:42
|
On Monday 28 November 2005 12:29, Tom Hughes wrote: > In message <200...@cn...> > > Yao Qi <qiy...@cn...> wrote: > > Could you review my two patches? They are suspended for sometimes, > > > > http://sourceforge.net/mailarchive/forum.php?thread_id=8973651&forum_id=1 > >2302 > > http://sourceforge.net/mailarchive/forum.php?thread_id=8964035&forum_id=1 > >2302 > > > > Thanks in advance! > > I committed the README_DEVELOPERS one (or something based on it) as > revision 5165 on the 11th November. > > The other one is one for Julian I think. Yao, can you send a patch for the current version of jm-insns.c, and just add the minimum changes needed to make it compile cleanly with gcc4? J |
|
From: Yao Qi <qiy...@cn...> - 2005-11-29 03:31:09
Attachments:
jm-insns.patch
|
On Mon, Nov 28, 2005 at 01:02:58PM +0000, Julian Seward wrote: > On Monday 28 November 2005 12:29, Tom Hughes wrote: > > In message <200...@cn...> > > > > Yao Qi <qiy...@cn...> wrote: > > > Could you review my two patches? They are suspended for sometimes, > > > > > > http://sourceforge.net/mailarchive/forum.php?thread_id=8973651&forum_id=1 > > >2302 > > > http://sourceforge.net/mailarchive/forum.php?thread_id=8964035&forum_id=1 > > >2302 > > > > > > Thanks in advance! > > > > I committed the README_DEVELOPERS one (or something based on it) as > > revision 5165 on the 11th November. > > > > The other one is one for Julian I think. > > Yao, can you send a patch for the current version of jm-insns.c, and > just add the minimum changes needed to make it compile cleanly with > gcc4? OK, here is a patch for the current version of jm-insns.c in attachment. Could you have a look at it? Any comments are greatly appreicated! > > J > > > ------------------------------------------------------- > This SF.net email is sponsored by: Splunk Inc. Do you grep through log files > for problems? Stop! Download the new AJAX search engine that makes > searching your log files as easy as surfing the web. DOWNLOAD SPLUNK! > http://ads.osdn.com/?ad_id=7637&alloc_id=16865&op=click > _______________________________________________ > Valgrind-developers mailing list > Val...@li... > https://lists.sourceforge.net/lists/listinfo/valgrind-developers > -- Regards, Yao ------------ Yao Qi |
|
From: Cerion Armour-B. <ce...@op...> - 2005-11-29 12:13:42
|
On Tuesday 29 November 2005 04:30, Yao Qi wrote: > On Mon, Nov 28, 2005 at 01:02:58PM +0000, Julian Seward wrote: > > On Monday 28 November 2005 12:29, Tom Hughes wrote: > > > In message <200...@cn...> > > > > > > Yao Qi <qiy...@cn...> wrote: > > > > Could you review my two patches? They are suspended for sometimes, > > > > > > > > http://sourceforge.net/mailarchive/forum.php?thread_id=8973651&forum_ > > > >id=1 2302 > > > > http://sourceforge.net/mailarchive/forum.php?thread_id=8964035&forum_ > > > >id=1 2302 > > > > > > > > Thanks in advance! > > > > > > I committed the README_DEVELOPERS one (or something based on it) as > > > revision 5165 on the 11th November. > > > > > > The other one is one for Julian I think. > > > > Yao, can you send a patch for the current version of jm-insns.c, and > > just add the minimum changes needed to make it compile cleanly with > > gcc4? > > OK, here is a patch for the current version of jm-insns.c in attachment. > Could you have a look at it? Any comments are greatly appreicated! > I committed a patch based on this (r5247), but i don't have gcc4 installed, so can you confirm all decl warnings are gone? As for tracking down your segfault... you asked how this code works: 3938 func_buf[1] = p[1]; 3939 patch_op_imm16(func_buf, p, ii16[j]); 3940 func = (void *)func_buf; ...... ...... 3955 (*func)(); This is dynamic code rewriting at work. All the immediate test functions are of the form { imm_insn, blr }, so func_buf[1] = p[1] first copies the blr over. Next, patch_op_imm16() 'patches' the imm_insn with an immediate (ii16[j]), writing it out to func_buf, which is subsequently executed. So, for example, for the function test_addi(): extern void test_addi (void); asm(".text\n" "test_addi:\n" "\taddi 17, 14, 0\n" "\tblr\n" ".previous\n" ); if ii16[j] is currently 9: addi 17, 14, 0 becomes: addi 17, 14, 9 In the immediate tests, e.g. test_int_one_reg_imm16(), if you use printf("original %s: %08x %08x\n", name, p[0], p[1]); before the patching, and printf("patched %s: %08x %08x\n", name, func_buf[0], func_buf[1]); after, you can compare the imm_insn before and after patching - can you verify the patch results are correct? (if you undefine USAGE_SIMPLE, you can specify a single op to test on the command-line) For reference, I get: $./jm-insns -n addi ... original addi: 3a2e0000 4e800020 patched addi: 3a2e03e7 4e800020 addi 00000000, 000003e7 => 000003e7 (00000000 00000000) ... Julian is highly suspicious of the global register variables - it'd be great if you can figure out a way of not needing these. Hope that helps, Cerion |
|
From: Yao Qi <qiy...@cn...> - 2005-11-30 10:54:51
|
On Tue, Nov 29, 2005 at 01:13:31PM +0100, Cerion Armour-Brown wrote:
> > >
> > > Yao, can you send a patch for the current version of jm-insns.c, and
> > > just add the minimum changes needed to make it compile cleanly with
> > > gcc4?
> >
> > OK, here is a patch for the current version of jm-insns.c in attachment.
> > Could you have a look at it? Any comments are greatly appreicated!
> >
>
> I committed a patch based on this (r5247), but i don't have gcc4 installed, so
> can you confirm all decl warnings are gone?
I *only* move all the declarations in front of statements to remove
warnings form GCC. Both GCC 3.3.3 and GCC 4.0 will emit warnings util
this patch applied.
>
> As for tracking down your segfault... you asked how this code works:
>
> 3938 func_buf[1] = p[1];
> 3939 patch_op_imm16(func_buf, p, ii16[j]);
> 3940 func = (void *)func_buf;
> ......
> ......
> 3955 (*func)();
>
> This is dynamic code rewriting at work.
> All the immediate test functions are of the form { imm_insn, blr }, so
> func_buf[1] = p[1] first copies the blr over.
> Next, patch_op_imm16() 'patches' the imm_insn with an immediate (ii16[j]),
> writing it out to func_buf, which is subsequently executed.
> So, for example, for the function test_addi():
>
> extern void test_addi (void);
> asm(".text\n"
> "test_addi:\n"
> "\taddi 17, 14, 0\n"
> "\tblr\n"
> ".previous\n"
> );
>
> if ii16[j] is currently 9:
> addi 17, 14, 0
> becomes:
> addi 17, 14, 9
>
>
> In the immediate tests, e.g. test_int_one_reg_imm16(), if you use
> printf("original %s: %08x %08x\n", name, p[0], p[1]);
> before the patching, and
> printf("patched %s: %08x %08x\n", name, func_buf[0], func_buf[1]);
> after, you can compare the imm_insn before and after patching - can you
> verify the patch results are correct?
Yes, I think the patch results are correct. I trace jm-insns(compiled
by GCC 4.0.0) by GDB like this,
(gdb) b test_int_one_reg_imm16
.....
(gdb) n
4262 patch_op_imm16(func_buf, p, ii16[j]);
(gdb) n
(gdb) p/x &func_buf[0]
$1 = 0xffec14f4
(gdb) disassemble 0xffec14f4 0xffec1500
Dump of assembler code from 0xffec14f4 to 0xffec1500:
0xffec14f4: cmpwi cr2,r14,0
0xffec14f8: blr
0xffec14fc: fnmadd. f31,f31,f31,f31
End of assembler dump.
(gdb) disassemble 0x10000e90 0x10000e98
Dump of assembler code from 0x10000e90 to 0x10000e98:
0x10000e90 <test_cmpi+0>: cmpwi cr2,r14,15
0x10000e94 <test_cmpi+4>: blr
End of assembler dump.
> (if you undefine USAGE_SIMPLE, you can specify a single op to test on the
> command-line)
> For reference, I get:
> $./jm-insns -n addi
> ...
> original addi: 3a2e0000 4e800020
> patched addi: 3a2e03e7 4e800020
> addi 00000000, 000003e7 => 000003e7 (00000000 00000000)
> ...
>
>
> Julian is highly suspicious of the global register variables - it'd be great
> if you can figure out a way of not needing these.
I am not sure whether the segment fault has something to do with
global register variables and I think it is a smart way to test
instructions with different immediate operands.
Now, I could only locate the segment fault but still cannot figure out
why. I debug it like this,
(gdb) run
.....
PPC integer compare with immediate insns (two args):
Breakpoint 1, test_int_one_reg_imm16 (name=0x10006648 " cmpi",
func=0x10000e90 <test_cmpi>, test_flags=16843525)
at jm-insns.c:4254
4254 for (i=0; i<nb_iargs; i++) {
.....
(gdb) n
4278 (*func)();
(gdb) disassemble 0x10002e08 0x10002e18
Dump of assembler code from 0x10002e08 to 0x10002e18:
0x10002e08 <test_int_one_reg_imm16+216>: mtctr r0
0x10002e0c <test_int_one_reg_imm16+220>: bctrl
0x10002e10 <test_int_one_reg_imm16+224>: mfcr r18
0x10002e14 <test_int_one_reg_imm16+228>: mr r0,r18
End of assembler dump.
(gdb) si
0xff9374f4 in ?? ()
(gdb) disassemble 0xff9374f4 0xff937500
Dump of assembler code from 0xff9374f4 to 0xff937500:
0xff9374f4: cmpwi cr2,r14,0
0xff9374f8: blr
0xff9374fc: fnmadd. f31,f31,f31,f31
End of assembler dump.
(gdb) si
Program received signal SIGSEGV, Segmentation fault.
0xff9374f4 in ?? ()
I do not know why "cmpwi cr2,r14,0" cause a segment fault!
If I remove this statement "func = (void *) func_buf", and "cmpwi
cr2,r14,15" could not cause segment fault.
>
> Hope that helps,
Your explanations are *really* helpful for me! They would be more helpful
others if we add these explanations into none/test/ppc32/jm-insn.c.
Any thought on this? I would like to take it up if Tom, Julian or both
agree on this.
> Cerion
>
>
>
--
Regards, Yao
------------
Yao Qi
|
|
From: Julian S. <js...@ac...> - 2005-11-30 14:53:42
|
> (gdb) si > 0xff9374f4 in ?? () > (gdb) disassemble 0xff9374f4 0xff937500 > Dump of assembler code from 0xff9374f4 to 0xff937500: > 0xff9374f4: cmpwi cr2,r14,0 > 0xff9374f8: blr > 0xff9374fc: fnmadd. f31,f31,f31,f31 > End of assembler dump. > (gdb) si > > Program received signal SIGSEGV, Segmentation fault. > 0xff9374f4 in ?? () > > I do not know why "cmpwi cr2,r14,0" cause a segment fault! > > If I remove this statement "func = (void *) func_buf", and "cmpwi > cr2,r14,15" could not cause segment fault. Then it is not the cmpwi causing the segfault, but the next insn - blr. So most likely lr contains a bad value and you get a segfault when it tries to jump to that address. J |