(移植編第５回)う〜ん，バグだらけだったか．．．

(注意)このブログは本家のほうの文章部分のみの転載です．ソースコードの配布，画像などについては本家のほうを参照してください．文章中のリンク先は面倒なのですべて本家のほうに変換してしまっているのでご注意ください．

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

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

• レジスタにFPSCRを追加．(実害無し)
• トラップ命令でのブレーク時にステップ実行できない問題を修正．
• ステップ実行フラグの操作ミスでMSRが不正になる問題を修正．(致命的バグ)
• シリアルでウエイトをおくように修正．
• 割り込みハンドラからの関数呼び出しで，LRの値によってGPR0が破壊される 問題の修正．(致命的バグ)
• デバッグ用シリアルをPSC1→PSC2に変更．
• スタブの処理終了時に，命令キャッシュをクリアする処理を追加．(致命的バグ)
• ステップ実行時の割り込みラッチを追加．

致命的なバグが３つあって，これらは直しておかないと，まったくまともに動かない．あとは他にも気がついたところをちょこちょこ修正した．で，以下が修正済ソースコード．

• ソースコードはこちら

前回からの差分はいつもどおり 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 *)&registers[PC], ptr, 4, 0); 	/* PC */
   *ptr++ = ';';
+#endif
 
   strcpy(ptr, "thread:");
   ptr += 7;

なぜこのような修正をするかというと，実はここでは hexchars[PC] を見ている部分がある(上の差分には出てないけど)のだけど，PCの値は96なのでオーバーフローしている．これはまずい．

で，２桁にすべきかと思ってためしに

-  *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アセンブラのエッセンス」に詳しいので，そちらも参照してほしい．

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

ちなみに従来，スタックへの値保存の都合で，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専用で，シリアルケーブル２本で接続するようになった．あと 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(&registers[GPR2], &thp->context.gpr[2], 4*30);
+  memcpy(&registers[GPR0], &thp->context.gpr[0], sizeof(registers[0]) * 32);
+  memset(&registers[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], &registers[GPR2], 4*30);
+  memcpy(&thp->context.gpr[0], &registers[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活用テクニック」第９回でも言及しているので，そちらも参考にしてほしい．

@@ -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とするように対応を入れた．あと割り込み時のスタックの先頭８バイトは空けるようにしたので，その修正．

最後に，強制ブレークの修正．

@@ -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を使うように修正したので，シリアルケーブルは２本使い，通常のコンソール用途とGDB用で区別する．

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

画像はこちら

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

> break

第３回と同様にして，emacs から gdb を起動する．

画像はこちら

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

画像はこちら

もう１回．

画像はこちら

もう１回．

画像はこちら

とくに問題なさそうだ．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
> 

再度プロンプトが出てきた．問題無しだ．

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

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