Valgrindでプログラムの潜在的な問題を抽出する

ISSでプログラムを実行して解析していると、ISSの妙な動作に遭遇することがある。プログラムの挙動が怪しい時は、解析をしていくのだが、どうやら未初期化の領域を参照しているらしい。

こういうときには、Valgrindで不正メモリアクセスをチェックするのが効果的だ。

valgrind.org

Valgrindはデバッグと解析用のツールで、特にメモリリークなどのプログラムの潜在的な問題をつかむのに利用される。 僕はリリースプログラムにValgrindを当てることで、プログラム中に潜在的に存在するバッファオーバフローの問題をチェックしている。

実際に、現在デバッグ中のOSで問題があり、それがどうやら未初期化領域にアクセスしたことによる問題らしい。ISSの問題だ。

github.com

メモリリークをチェックしてみる。

valgrind --leak-check=full \
         --log-file=valgrind.log \
        ./swimmer_riscv --bit_mode 32 --binfile ../benchmarks/releases/coremark_v1.0_riscv_gcc49_O2/coremark.bin --debug  --out coremark.sw.log --max 500000 --trace_hier --trace_out trace.log --load_dump

リークチェックモードを設定し、ログファイル名を設定する。

ログを確認すると、以下のようなログが大量に出力された。

==6896== Invalid read of size 1
==6896==    at 0x417D78: load_hex(bfd*, bfd_section*, Memory*) (bfd_env.cpp:240)
==6896==    by 0x4183D1: load_bitfile(bfd*, bfd_section*, void*) (bfd_env.cpp:196)
==6896==    by 0x50B57AB: bfd_map_over_sections (in /usr/lib/libbfd-2.24-system.so)
==6896==    by 0x419F84: EnvBase::LoadBinary(std::string) (bfd_env.cpp:62)
==6896==    by 0x414485: main (swimmer_main.cpp:235)
==6896==  Address 0x685830c is 0 bytes after a block of size 156 alloc'd
==6896==    at 0x4C2B800: operator new[](unsigned long) (in /usr/lib/valgrind/vgpreload_memcheck-amd64-linux.so)
==6896==    by 0x417CCA: load_hex(bfd*, bfd_section*, Memory*) (bfd_env.cpp:219)
==6896==    by 0x4183D1: load_bitfile(bfd*, bfd_section*, void*) (bfd_env.cpp:196)
==6896==    by 0x50B57AB: bfd_map_over_sections (in /usr/lib/libbfd-2.24-system.so)
==6896==    by 0x419F84: EnvBase::LoadBinary(std::string) (bfd_env.cpp:62)
==6896==    by 0x414485: main (swimmer_main.cpp:235)
==6896==
==6896== Invalid read of size 1
==6896==    at 0x43677D: Memory::StoreMemByte(unsigned long, unsigned char*) (mem_body.cpp:112)
==6896==    by 0x417DA2: load_hex(bfd*, bfd_section*, Memory*) (bfd_env.cpp:242)
...
==6896== Conditional jump or move depends on uninitialised value(s)
==6896==    at 0x436424: find (stl_tree.h:1795)
==6896==    by 0x436424: find (stl_map.h:822)
==6896==    by 0x436424: Memory::SearchMemTable(unsigned long, unsigned char*) (mem_body.cpp:183)
==6896==    by 0x41FC83: LoadMemByte (env.cpp:88)
==6896==    by 0x41FC83: EnvBase::LoadMemory(unsigned long, unsigned char, unsigned char*) (env.cpp:211)
==6896==    by 0x41A98A: RiscvEnv::LoadFromBus(unsigned long, unsigned char, unsigned char*) (riscv_env.cpp:417)
==6896==    by 0x4275DE: InstEnv::RISCV_INST_LBU(int) (inst_riscv.cpp:301)
==6896==    by 0x41D792: RiscvEnv::StepExec(bool) (riscv_env.cpp:121)
==6896==    by 0x41FAF5: EnvBase::StepSimulation(int, LoopType_t) (env.cpp:68)
==6896==    by 0x4144AD: main (swimmer_main.cpp:276)
==6896==
==6896== Conditional jump or move depends on uninitialised value(s)
...

バッファオーバーフローによるプログラムの危険性を発見

まず、最初のエラーはバッファオーバフローの可能性のある、プログラムの記述ミスだった。配列に対して、インデックスがさらに外側を参照しようとしている。

  int size = bfd_section_size (b, section);
  std::unique_ptr<uint8_t[]> buf (new uint8_t[size]);
...
    for (j = 0; j < 16 && j < size; j++) {
      p_memory->StoreMemByte (addr+j, &buf[i+j]);

上記のようなプログラムは、bufのサイズを超えるアクセスをする可能性がある。以下が正しい。

  int size = bfd_section_size (b, section);
  std::unique_ptr<uint8_t[]> buf (new uint8_t[size]);
...
    for (j = 0; j < 16 && (i+j) < size; j++) {
      p_memory->StoreMemByte (addr+j, &buf[i+j]);

システムレジスタの初期化忘れ

2番目のエラーは、システムレジスタを管理する変数の初期化忘れだ。

これらの記述をコンストラクタに指定しておく。

CsrEnv::CsrEnv (RiscvEnv *env) : m_env (env)
{
  if (FLAGS_bit_mode == 32) {
    mcpuid.bit_mcpuid.Base = 0;
  } else {
    mcpuid.bit_mcpuid.Base = 2;
  }

  fflags.fflags       = 0;
  frm.frm             = 0;
  fcsr.fcsr           = 0;
...

これで、かなりの量の警告は消えたのだが、まだスタックの消し忘れなどの記述がある。

これらは、デストラクタでいろいろやっていくよりも、unique_ptrで消していくことにしよう。