(移植編第5回)う〜ん,バグだらけだったか...

2009/04/12

あなたは 人目のお客様です.

前回まででGDBスタブの移植と説明をしたのだけど,実はバグだらけで ステップ実行とかがまともに動かないことが判明.

ということで,今回はごっそりとバグ修正.以下の修正を行った.

致命的なバグが3つあって,これらは直しておかないと,まったくまともに動かない. あとは他にも気がついたところをちょこちょこ修正した. で,以下が修正済ソースコード. 前回からの差分はいつもどおり diff.txt 参照.

で,以下に修正内容を説明する.

まず,ppc-stub.c について. 浮動小数レジスタについてとくにサポートしていないので, FPSCRはGDBに渡さずにいたが,いちおう値だけは渡すように修正. (といってもゼロを渡すだけだけど)

diff -ruN porting03/ppc-stub.c porting05/ppc-stub.c
--- porting03/ppc-stub.c	Tue Apr  7 23:21:40 2009
+++ porting05/ppc-stub.c	Sun Apr 12 12:09:59 2009
@@ -119,7 +119,7 @@
 static const char hexchars[]="0123456789abcdef";
 
 /* Number of registers.  */
-#define NUMREGS	(32+2*32+6) /* GPR(32), FPR(32), SRR0/1, CR, CTR, LR, XER */
+#define NUMREGS	(32+2*32+7) /* GPR(32), FPR(32), SRR0/1, CR, CTR, LR, XER, FPSCR */
 
 /* Number of bytes of registers.  */
 #define NUMREGBYTES (NUMREGS * 4)
@@ -132,9 +132,9 @@
 	GPR16, GPR17, GPR18, GPR19, GPR20, GPR21, GPR22, GPR23,
 	GPR24, GPR25, GPR26, GPR27, GPR28, GPR29, GPR30, GPR31,
 
-	/* FPR * 32 */
+	FPRS, /* FPR * 32 */
 
-	SRR0 = (32+2*32), SRR1, CR, LR, CTR, XER
+	SRR0 = (32+2*32), SRR1, CR, LR, CTR, XER, FPSCR
 };
 
 #define SP GPR1
NUMREGSの数が増えたので,これでレジスタ一覧を渡す際にFPSCRの値も渡すように なる.まあこれは修正しなくても問題なさそうだったのだけどいちおう直しておく.

次に,trap命令によるブレーク時に,ステップ実行ができない問題を修正.

従来は kz_trap() とか kz_break() の内部では trap 命令を直接実行することで トラップ割り込みを上げてブレークしていた.しかしこれでブレークすると, GDBから step や next でステップ実行した際に,またその命令でブレークして しまって永久に先に進めない.

ということで,トラップ命令で強制ブレークした場合には,PCの値を加算して 次の命令から実行するように修正する.

@@ -190,7 +190,12 @@
    that trace flag works right.  */
 asm("        iret");
 
-#define BREAKPOINT() asm("   trap");
+static int breaking = 0;
+#define BREAKPOINT() { \
+		breaking++; \
+		asm("   trap"); \
+		asm("   nop"); \
+	}
 
 /* Put the error code here just in case the user cares.  */
 int gdb_i386errcode;
@@ -780,12 +785,18 @@
   /* reply to host that an exception has occurred */
   sigval = computeSignal (exceptionVector);
 
+  if (breaking && (sigval == 5) && (registers[MSR] & (1<<17))) {
+    breaking--;
+    registers[PC] += 4;
+  }
+
   ptr = remcomOutBuffer;
 
trap命令の後には,いちおうnopを入れておいた.あとMSRの特定のビットを見ることで トラップ命令による割り込みかどうかを判断できるので,そーいうふうに修正した. このへんは,詳しくは「32ビット PowerPC アーキテクチャ プログラミング環境」 (freescaleのホームページから日本語版PDFをダウンロードできる) の例外処理のあたりを参照してほしい.

次に,これは修正が必要かどうかちょっと迷ったのだが,例外発生時に PC上のGDBにブレークしたことを通知する部分で, レジスタの値を通知しないように修正する.

   *ptr++ = 'T';			/* notify gdb with signo, PC, FP and SP */
   *ptr++ = hexchars[sigval >> 4];
   *ptr++ = hexchars[sigval & 0xf];
 
+#if 0
   /* See gdb/regformats/reg-ppc.dat */
   *ptr++ = hexchars[SP]; 
   *ptr++ = ':';
@@ -796,6 +807,7 @@
   *ptr++ = ':';
   ptr = mem2hex((char *)®isters[PC], ptr, 4, 0); 	/* PC */
   *ptr++ = ';';
+#endif
 
   strcpy(ptr, "thread:");
   ptr += 7;
なぜこのような修正をするかというと,実はここでは hexchars[PC] を見ている 部分がある(上の差分には出てないけど)のだけど,PCの値は96なので オーバーフローしている.これはまずい.

で,2桁にすべきかと思ってためしに

-  *ptr++ = hexchars[PC]; 
+  *ptr++ = hexchars[PC >> 4];
+  *ptr++ = hexchars[PC & 0xf];
のようにしてみたのだけど,これだと値が大きすぎるといってGDBがエラーにして しまうのだな.

で,レジスタの値を送信しなくても,GDBがgコマンドでレジスタ一覧を取得しにくる のでとくに問題はなさそうなので,レジスタの値は送信しないように修正した. まあそれで問題無く動いているので,これでよしとしよう.

次に,MSRのフラグの落とし間違いのしょぼいミスを修正.

@@ -911,11 +923,11 @@
 	  newPC = registers[PC];
 
 	  /* clear the trace bit */
-	  registers[MSR] &= MSR_SE;
+	  registers[MSR] &= ~(MSR_SE /* | MSR_BE */);
 
 	  /* set the trace bit if we're stepping */
 	  if (stepping)
-	    registers[MSR] |= MSR_SE;
+	    registers[MSR] |= (MSR_SE /* | MSR_BE */);
 
 #if 0
 	  _returnFromException ();	/* this is a jump */
MSR_SEでandしてしまっていた.ああ,しょぼい.

MSR_BEはブランチ命令(ジャンプ命令のこと)でブレークするかというフラグなの だけど,試してみたらMSR_SEを立てておけばブランチ命令でも常にブレークするので, MSR_BEを立てる必要は無いみたい.いちおうコメントとして残しておいた.

次に serial.c について,シリアルまわりの修正.

diff -ruN porting03/serial.c porting05/serial.c
--- porting03/serial.c	Tue Apr  7 23:21:40 2009
+++ porting05/serial.c	Sun Apr 12 12:09:59 2009
@@ -78,6 +78,14 @@
   return psc->psc_status & PSC_SR_RXRDY;
 }
 
+static void udelay(int usec)
+{
+  volatile int i;
+  /* とりあえずてきとうな回数でのダミーループでウエイトを置く */
+  for (i = 0; i < (save_clk / 1000000) * usec; i++)
+    ;
+}
+
 void serial_putc(int index, char c)
 {
   volatile struct mpc5xxx_psc *psc = regs[index].psc;
@@ -88,6 +96,12 @@
   while (!(psc->psc_status & PSC_SR_TXEMP)) {
     /* waiting */
   }
+
+  /*
+   * フロー制御が無い場合のバッファ溢れ防止として,ウエイトを置く.
+   * データの取りこぼしが発生しているようなら,ウエイトを増やすこと.
+   */
+  udelay(20);
 
   psc->psc_buffer_8 = c;
 }
フロー制御が効いていないと,データ量が多くなったときに受けきれなくて とりこぼしが発生する可能性がある.なので,あまりガツガツとデータを送らない ように,ウエイトを入れてみた.まあほんとうはクロックとかウエイトの値をきちんと 計算して入れるべきなのだけど,とりこぼしが起きてるようならウエイトを増やすと いうことで,不正確でもいいのでとりあえず入れておいた. なので名前は udelay() だけど,正確にマイクロ秒というわけではなくあくまで目安. はじめは udelay(1) くらいでやっていたのだけど,これだとどうも動作が不安定な 場合が多く,udelay(10)くらいで安定した.なのでいちおうウエイトは20としてある.

ちなみにデータのとりこぼしが起きているかどうかは,

(gdb) set debug target 1
(gdb) set debug remote 1
(gdb) set debug serial 1
とかでデータを直接見ることで調べることができる. 実際にGDBを使っているときにとりこぼしが発生すると, レジスタの値がきちんととれなかったりして,info registers とかやっても ゼロばっかになっていたり,よくわからん止まりかたをしたりする. こんなときはウエイトの数を増やしてやる.

次に,startup.s について.LRの値によってGPR0が破壊されていた重大な問題の修正.

diff -ruN porting03/startup.s porting05/startup.s
--- porting03/startup.s	Tue Apr  7 23:21:40 2009
+++ porting05/startup.s	Sun Apr 12 12:09:59 2009
@@ -22,23 +22,22 @@
 	ori	1,1,0x200000@l
 	stwu	1,-160(1)
 
-	stmw	2,8(1)
-	stw	0,4(1)
+	stmw	0,8(1)
 	mfsprg2	2
-	stw	2,0(1)
+	stw	2,12(1)
 
 	mflr	2
-	stw	2,128(1)
+	stw	2,136(1)
 	mfcr	2
-	stw	2,132(1)
+	stw	2,140(1)
 	mfctr	2
-	stw	2,136(1)
+	stw	2,144(1)
 	mfxer	2
-	stw	2,140(1)
+	stw	2,148(1)
 	mfsrr0	4
-	stw	4,144(1)
+	stw	4,152(1)
 	mfsrr1	5
-	stw	5,148(1)
+	stw	5,156(1)
 
 	bl	1f
 1:	mflr	3
@@ -54,25 +53,26 @@
 	.type	dispatch,@function
 dispatch:
 	mr	1,3
+	addi	1,1,-8
 
 return_from_interrupt:
-	lwz	2,128(1)
+	lwz	2,136(1)
 	mtlr	2
-	lwz	2,132(1)
+	lwz	2,140(1)
 	mtcr	2
-	lwz	2,136(1)
+	lwz	2,144(1)
 	mtctr	2
-	lwz	2,140(1)
+	lwz	2,148(1)
 	mtxer	2
-	lwz	2,144(1)
+	lwz	2,152(1)
 	mtsrr0	2
-	lwz	2,148(1)
+	lwz	2,156(1)
 	andi.	2,2,0xffff
 	mtsrr1	2
 
-	lmw	2,8(1)
-	lwz	0,4(1)
-	lwz	1,0(1)
+	lmw	2,16(1)
+	lwz	0,8(1)
+	lwz	1,12(1)
 
 	sync
 	isync
割り込み発生時にはレジスタの値をスタックに積むのだけれど, 関数呼び出し時には,LRの値は前のスタックフレームに対して保存される. たとえば,以下を見てほしい.
00043cec :
   43cec:       94 21 ff e0     stwu    r1,-32(r1)
   43cf0:       7c 08 02 a6     mflr    r0
   43cf4:       93 e1 00 1c     stw     r31,28(r1)
   43cf8:       90 01 00 24     stw     r0,36(r1)
   ...
上は command_main のアセンブル結果だが,スタックポインタ(GPR1)の値を 32バイト引くことでスタックを32バイト確保し,その後GPR31の値は GPR1 + 28 の位置(つまり,このスタックフレームのお尻)に保存しているのだが, LRの値は GPR1 + 36 の位置に保存している.つまり,前のスタックフレームの中に 保存しているわけだ.

このためスタックフレームの先頭を空けておかなければならないのだが, 従来の _intr ではスタックフレームの先頭からレジスタの値を保存していた. で,ここにはGPR0の値が保存されていたため,その後の関数呼び出しでLRの値に 上書きされてしまい,GPR0が使われている場合に誤動作していた. こーいうレジスタの保存ミスって,非常に見つけにくいバグの原因になるんだよね. (実際,非常に見つけにくかった)

ちなみにこのスタックの使いかたについては, 「Interface」2006/02 特集記事の 「PowerPCアセンブラのエッセンス」 に詳しいので,そちらも参照してほしい.

で,スタックフレームの先頭8バイトは空けてレジスタの値を保存するように 修正した.

ちなみに従来,スタックへの値保存の都合で,GPR0とGPR1の値が逆になって保存されて いた.しかし今回の修正で,そのような都合を考える必要が無くなったので, ひっくり返さずにそのまま保存するように修正した.

次に,stublib.c の修正.

diff -ruN porting03/stublib.c porting05/stublib.c
--- porting03/stublib.c	Tue Apr  7 23:21:40 2009
+++ porting05/stublib.c	Sun Apr 12 12:09:59 2009
@@ -1,3 +1,5 @@
+#include 
+
 #include "kozos.h"
 #include "thread.h"
 #include "stublib.h"
@@ -5,13 +7,10 @@
 
 #include "lib.h"
 
-#define SERIAL_NUMBER 0
+#define SERIAL_NUMBER 1
 
 /* Number of registers.  */
-#define NUMREGS (32+2*32+6) /* GPR(32), FPR(32), SRR0/1, CR, CTR, LR, XER */
-
-/* Number of bytes of registers.  */
-#define NUMREGBYTES (NUMREGS * 4)
+#define NUMREGS	(32+2*32+7) /* GPR(32), FPR(32), SRR0/1, CR, CTR, LR, XER, FPSCR */
 
 enum regnames {
 	GPR0,  GPR1,  GPR2,  GPR3,  GPR4,  GPR5,  GPR6,  GPR7,
@@ -19,10 +18,10 @@
 	GPR16, GPR17, GPR18, GPR19, GPR20, GPR21, GPR22, GPR23,
 	GPR24, GPR25, GPR26, GPR27, GPR28, GPR29, GPR30, GPR31,
 
-	/* FPR * 32 */
+	FPRS, /* FPR * 32 */
 
 	/* See gdb/ppc-linux-nat.c or gdb/regformats/reg-ppc.dat */
-	SRR0 = (32+2*32), SRR1, CR, LR, CTR, XER
+	SRR0 = (32+2*32), SRR1, CR, LR, CTR, XER, FPSCR
 };
 
 #define SP GPR1
従来はコンソールとGDBでシリアルを共用していた(PSC1を共通で使っていた)が, 操作が面倒なのでコンソールにはPSC1,GDBにはPSC2を使うように修正. これにより,コンソール専用とGDB専用で,シリアルケーブル2本で接続するように なった.あと ppc-stub.c で行ったFPSCRの追加修正をこっちにも反映.

次に,startup.s でのレジスタの保存方法の修正でGPR0とGPR1の値をひっくり返さなく てもよくなったので,その修正.

@@ -65,12 +64,8 @@
 
 void stub_store_regs(kz_thread *thp)
 {
-  memset(registers, 0, sizeof(registers));
-
-  /* 注意:grp0,gpr1は逆に格納されている */ 
-  registers[GPR1] = thp->context.gpr[0];
-  registers[GPR0] = thp->context.gpr[1];
-  memcpy(®isters[GPR2], &thp->context.gpr[2], 4*30);
+  memcpy(®isters[GPR0], &thp->context.gpr[0], sizeof(registers[0]) * 32);
+  memset(®isters[FPRS], 0, sizeof(registers[0]) * 2 * 32);
 
   registers[PC]  = thp->context.pc;
   registers[MSR] = thp->context.msr;
@@ -82,9 +77,7 @@
 
 void stub_restore_regs(kz_thread *thp)
 {
-  thp->context.gpr[0] = registers[GPR1];
-  thp->context.gpr[1] = registers[GPR0];
-  memcpy(&thp->context.gpr[2], ®isters[GPR2], 4*30);
+  memcpy(&thp->context.gpr[0], ®isters[GPR0], sizeof(registers[0]) * 32);
 
   thp->context.pc  = registers[PC];
   thp->context.msr = registers[MSR];
次に,命令キャッシュのクリアを追加する.

GDBはブレークポイントやステップ実行の際に,命令置き換えが頻繁に行われる(これは set debug remote 1 にして,GDBとスタブのやりとりを見ているとよくわかる). しかしどうもMPC5200というCPUは,命令コードを書き換えたら命令キャッシュをクリア しないと,書き換え前の古い命令がキャッシュに残ってしまっていて,古い命令が 実行されてしまうことがあるようだ.で,これはキャッシュの状態によって発生したり しなかったりするので,非常に見つけにくいバグの原因になる.

これについては 「Interface」2009/04の 「実践的PowerPC活用テクニック」第9回 でも言及しているので,そちらも参考にしてほしい.

@@ -94,6 +87,22 @@
   thp->context.xer = registers[XER];
 }
 
+static void clear_icache_all()
+{
+  extern unsigned long _etext;
+  int addr;
+
+  asm volatile ("sync");
+  asm volatile ("isync");
+
+  for (addr = 0x0; addr < (int)&_etext; addr += CFG_CACHELINE_SIZE) {
+    asm volatile ("icbi 0,%0" :: "r"(addr));
+  }
+
+  asm volatile ("sync");
+  asm volatile ("isync");
+}
+
 int stub_proc(kz_thread *thp, int signo)
 {
   gen_thread = thp;
@@ -103,7 +112,12 @@
   clearDebugChar();
   handle_exception(signo);
 
+  registers[MSR] &= 0xffff;
+
   stub_restore_regs(gen_thread);
+
+  /* 命令書き換えが行われている場合があるので,命令キャッシュを全クリアする */
+  clear_icache_all();
 
   return 0;
 }
ついでにMSRの上半分をクリアする処理を追加.まあこれと同等のことを startup.s の dispatch 処理でも行っているし,rfiの際にはSRR1の上16ビットはMSRに反映されない ようなので不要だとは思うのだけど,いちおう入れておいた.

次に thread.c について.

diff -ruN porting03/thread.c porting05/thread.c
--- porting03/thread.c	Tue Apr  7 23:21:40 2009
+++ porting05/thread.c	Sun Apr 12 12:09:59 2009
@@ -101,7 +101,7 @@
 
   memset(thp->stack, 0, SIGSTKSZ);
 
-  thp->context.gpr[0] = (int)thp->stack; /* GPR0とGPR1は逆に設定 */
+  thp->context.gpr[1] = (int)thp->stack;
   thp->context.gpr[3] = (int)thp;
   thp->context.gpr[4] = (int)argc;
   thp->context.gpr[5] = (int)argv;
@@ -476,6 +476,7 @@
   case 0x07: signo = SIGTRAP; break;
   case 0x09: signo = SIGALRM; break;
   case 0x0c: signo = SIGSYS;  break;
+  case 0x0d: signo = SIGTRAP; break;
   default:
     signo = SIGBUS;
     break;
@@ -483,8 +484,8 @@
 
   /* _startup.s:_intr でGPR1を INTR_STACK_START - 160 に設定している */
   p = (int *)(INTR_STACK_START - 160);
+  p += 2;
   for (i = 0; i < 32; i++) {
-    /* 注意:grp0,gpr1は逆に格納されている */ 
     current->context.gpr[i] = *(p++);
   }
   current->context.lr  = *(p++);
GPR0とGPR1をひっくり返さなくてすむようになったので,その対応. あとMSR_SEフラグが立ってステップ実行されたときに,0xd00 の割り込みが発生する ので,それをラッチしてSIGTRAPとするように対応を入れた. あと割り込み時のスタックの先頭8バイトは空けるようにしたので,その修正.

最後に,強制ブレークの修正.

@@ -534,6 +535,8 @@
     ;
 }
 
+void breakpoint();
+
 void kz_trap()
 {
   asm volatile ("trap");
@@ -541,7 +544,7 @@
 
 void kz_break()
 {
-  asm volatile ("trap");
+  breakpoint();
 }
 
 void kz_srvcall(kz_syscall_type_t type, kz_syscall_param_t *param)
従来はトラップ命令を直接呼んでいたのだけど,ステップ実行されたときに命令を 飛ばす処理が追加されたので,スタブの breakpoint() を呼び出すように修正した.

修正は以上.では,動作を試してみよう.

今回はGDBはPSC2を使うように修正したので,シリアルケーブルは2本使い, 通常のコンソール用途とGDB用で区別する.

まずはファームウエア作成し,ボードにダウンロードして起動する.

ここでbreakコマンドを実行し,強制ブレークする.

> break
第3回と同様にして,emacs から gdb を起動する.

正常にブレークしている.next コマンドでステップ実行してみよう.

もう1回.

もう1回.

とくに問題なさそうだ.continue で処理続行してみる.

> break
OK
> 
プロンプトが出てきた.正常に continue できたみたい.

次に,ブレークポイントを張ってみよう.まずはもう一度 break でブレークする.

> break
OK
> break

break コマンドで,dummy_func()にブレークポイントを張って continue する.

> break
OK
> break
OK
> 
再びプロンプトが出てきた.call コマンドで dummy_func() を実行してみよう.
> break
OK
> break
OK
> call

おー,dummy_func() でブレークした.

ステップ実行してみよう.

繰り返し実行してみたがとくに問題無し. で,continue で処理続行.

> break
OK
> break
OK
> call
OK
> 
再度プロンプトが出てきた.問題無しだ.

いやーデバッグたいへんだった.今回はけっこうデバッグに時間がとられてしまった のだけど,やっぱレジスタ周りとかスタック周りとかって,デバッグ難しいよね... わけわからん現象発生するし.このへんは慣れるしかないのだろうか.

まあなんにせよ,ちゃんと動いたのでよかったよかった.


メールは kozos(アットマーク)kozos.jp まで