higuoxing

higuoxing

github
twitter
ホームアーカイブ

Clangプラグインの初めての試み

#LLVM#Clang#PostgreSQL171
AI 翻訳
この記事はAIを通じて英語から日本語に翻訳されました。原文を表示
AI が生成した要約
この記事では、Clangプラグインを作成する理由と、PostgreSQLのエラーハンドリングコードのパターンについて説明しています。また、PG_TRY()とPG_CATCH()の動作についても詳しく説明しています。さらに、PG_TRY()ブロック内での不正なreturn、break、continue、gotoステートメントの使用に関する問題も取り上げています。

なぜ Clang プラグインを書きたいのか？#

私の日常の仕事は、Greenplumというデータベースの拡張機能を開発することです。これは、PostgreSQLから派生した分散データベースです。これを扱うたびに、PG_TRY()/PG_CATCH()ブロックを使用した PostgreSQL のエラーハンドリングコードに遭遇すると警戒心を抱きます。なぜなら、これを誤用することによって多くのバグが発生しているのを見てきたからです。これらのバグを捕まえるための自動化ツールを書くことに決めました。

PostgreSQL のエラーハンドリングコードはどのようなものか？#

PG_TRY()とPG_CATCH()に加えて、PostgreSQL のエラーハンドリングプロセスには 2 つの追加マクロが関与しています：ereport()とPG_END_TRY()です。

  • PG_TRY(), PG_CATCH()およびPG_END_TRY()はエラーハンドリングの制御フローを構築するためのものです。
  • ereport()はエラーを報告するためのものです。

PostgreSQL におけるエラーハンドリングプロセスのコードパターンは以下のようになります。

PG_TRY();
{
  // FallibleMethod()には潜在的なエラー報告呼び出しが含まれています。例えば、
  // ereport();
  FallibleMethod();
}
PG_CATCH();
{
  // エラーハンドリングを行います。
}
PG_END_TRY();

これらのマクロのコードパターンは、他の言語のtry-catch式に非常に似ていますが、その使用はより複雑です。これらのマクロの簡略化された定義は以下の通りです。

// マクロ定義のキーワードをコメントアウトしましたので、
//   構文ハイライトの恩恵を受けることができます :-)
// これらのマクロの完全な定義を見つけたい場合は、以下のリンクで見つけることができます。
//   https://github.com/postgres/postgres/blob/f7431bca8b0138bdbce7025871560d39119565a0/src/include/utils/elog.h#L384

// #define PG_TRY()
  do {
    sigjmp_buf *save_exception_stack = PG_exception_stack;
    sigjmp_buf local_sigjmp_buf;
    if (sigsetjmp(local_sigjmp_buf, 0) == 0)
    {
      PG_exception_stack = &local_sigjmp_buf

// #define PG_CATCH()
    }
    else
    {
      PG_exception_stack = save_exception_stack;

// #define PG_END_TRY()
    }
    PG_exception_stack = save_exception_stack;
  } while (0)
  
// #define ereport()
  if (PG_exception_stack != NULL)
    siglongjmp(*PG_exception_stack, 1);
  else
  {
    // 実際の世界では、この分岐を取ることは望ましくありません。
    ...
  }

PG_TRY()とPG_CATCH()はどのように機能するのか？#

グローバル変数sigjmp_buf *PG_exception_stackは、前回のsigsetjmp()呼び出しの環境（スタックコンテキスト）を保存します。誤りが発生する可能性のあるコードセクション（PG_TRY()とPG_CATCH()でラップされた部分）に入る前に、sigjmp_buf *save_exception_stackを使用して前の環境を保存し、現在の環境をPG_exception_stackに割り当てます。これにより、エラーが発生した場合（ereport()の呼び出し）、siglongjmp(*PG_exception_stack)を使用して正しい位置にジャンプできます（正しいPG_CATCH()ブロックによってキャッチされます）。

PG_CATCH()ブロックでは、エラーハンドリングアクションを取る前にPG_exception_stackの前の環境を復元します。これにより、エラーを上位の呼び出し元に伝える（siglongjmp()の別のラッパーであるPG_RE_THROW()の呼び出し）場合でも、siglongjmp(*PG_exception_stack)が再び正しい位置にジャンプできます。

問題#

よくある間違いの一つは、PG_TRY()ブロック内でジャンプ文（例えば、return、break、continue、goto）を使用することです。これは経験豊富な PostgreSQL の貢献者でも見られます1。例えば、PG_TRY()ブロック内でreturn文を使用すると、PG_exception_stackはPG_TRY()ブロックを離れる前に正しいスタックコンテキストに復元されません。これにより、PostgreSQL/Greenplum サーバーに深刻な問題を引き起こす可能性があります。例えば、サーバーのクラッシュ2。

私たちが検出しようとしているコードパターンは、PG_TRY()ブロック内での安全でないreturn、break、continue、およびgoto文の使用です。安全でないコードパターンは以下のルールに要約できます。

  • PG_TRY()ブロックのどこでも使用されるreturn文。

    PG_TRY();
{
  ...
  // 安全でない。なぜなら、PG_exception_stackは
  // PG_TRY-PG_CATCHブロックを離れた後も
  // 現在の環境を保持しているからです。
  return;
}
PG_CATCH();
{
  ...
}
PG_END_TRY();

  • switch、do-while、while、およびfor文内で使用される以外のPG_TRY()ブロックのどこでも使用されるbreak文。

    PG_TRY();
{
  ...
  // 安全でない。なぜなら、break文が
  // PG_TRY()ブロックの外にジャンプしており、
  // エラーハンドリングスタックコンテキストまたは環境を壊すからです。
  break;
  do
  {
    // 安全。なぜなら、do-whileループの外にジャンプしているからです。
    break;
  } while (0);
  while (1)
  {
    // 安全。まったく同じです。
    break;
  }
  for (;;)
  {
    // 安全。まったく同じです。
  }
  switch (c)
  {
  case 1:
    // 安全。まったく同じです。
    break;
  }
}
PG_CATCH();
{
  ...
}
PG_END_TRY();

  • do-while、while、およびforループ内で使用される以外のPG_TRY()ブロックのどこでも使用されるcontinue文。

    PG_TRY();
{
  // 安全でない。なぜなら、continue文が
  // PG_TRY()マクロから展開されたdo-whileループを終了させ、
  // PG_TRY()ブロックの外にジャンプするからです。
  continue;
  do {
    // 安全。
    continue;
  } while (0);
  while (0)
  {
    // 安全。
    continue;
  }
  for (;;)
  {
    // 安全。
    continue;
  }
}
PG_CATCH();
{
  ...
}
PG_END_TRY();

  • PG_TRY()ブロックの外にラベルを持つgoto文。

    label1:
  PG_TRY();
  {
  label2:
    // 安全でない。なぜなら、PG_TRY()ブロックの外にジャンプしており、
    // スタックコンテキストが壊れるからです。
    goto label1;
    // 安全。
    goto label2;
  }
  PG_CATCH();
  {
    ...
  }
  PG_END_TRY();

AST マッチャーまたは静的解析ツール？#

PG_TRY()は C のマクロであり、常に同じものに展開されます。さらに、私たちが検出したい文は、シンボルの状態の変化を追跡することを含まない非常に単純なものです。Clang の AST マッチャーは私たちの問題に十分です。

まず、checkEndOfTranslationUnit()というコールバック関数を登録して、return/break/continue/goto文を含むPG_TRY()ブロックを見つけます。このコールバック関数は、コンパイル中の各翻訳単位で呼び出されます。PG_TRY()ブロックが一致すると、誤検知を減らすために本当に安全でないかどうかを慎重に確認します。コメント付きのコードスニペットは以下の通りです。

class ReturnInPgTryBlockChecker : public Checker<check::EndOfTranslationUnit> {
public:
  void checkEndOfTranslationUnit(const TranslationUnitDecl *TU,
                                 AnalysisManager &AM, BugReporter &B) const {
    MatchFinder F;
    PgTryBlockMatcherCallback CB;
    StatementMatcher PgTry =
        // PG_TRY()は以下の式に展開されます。
        // if (__sigsetjmp() == 0) {
        //   PG_exception_stack = &local_sigjmp_buf;
        //   ...
        // }
        ifStmt(
            // 'if'文は二項演算子を含む必要があり、二項演算子は
            // '=='でなければなりません。
            hasCondition(
                binaryOperator(allOf(hasOperatorName("=="),
                                     // '=='のオペランドの一つは関数呼び出しであり、
                                     // その関数は'__sigsetjmp'という名前でなければなりません。
                                     // もう一つのオペランドは整数リテラル'0'でなければなりません。
                                     hasOperands(callExpr(callee(functionDecl(
                                                     hasName("__sigsetjmp")))),
                                                 integerLiteral(equals(0)))))),
            // 'if'文は'then'ブロックを持ち、'then'ブロックは
            // 'return'、'break'、'continue'、および'goto'文のいずれかを含む必要があります。
            hasThen(eachOf(
                // 便利のため、PG_TRY()ブロックをreturn文と結びつけて
                // 'ReturnInPgTryBlock'という名前を付けます。これにより、
                // 後で警告メッセージをすぐに発信できます。
                forEachDescendant(returnStmt().bind("ReturnInPgTryBlock")),
                anyOf(hasDescendant(breakStmt()), hasDescendant(continueStmt()),
                      hasDescendant(gotoStmt())))))
            // 私たちの興味のあるPG_TRY()ブロックのASTを
            // 後で慎重に確認するために'PgTryBlock'という名前でバインドします。
            .bind("PgTryBlock");

    // &CBは、後で一致したPG_TRY()ブロックのASTを慎重に確認するために
    // 呼び出されるコールバックです。
    F.addMatcher(PgTry, &CB);
    // ASTを一致させます！
    F.matchAST(TU->getASTContext());
  }
};

次に、一致したPG_TRY()ブロックの AST を慎重に確認します。以下のコールバックは、名前が"ReturnInPgTryBlock"または"PgTryBlock"の AST が一致したときに呼び出されます。

class PgTryBlockMatcherCallback : public MatchFinder::MatchCallback {
public:
  PgTryBlockMatcherCallback() = default;

  void run(const MatchFinder::MatchResult &Result) override {
    ASTContext *Ctx = Result.Context;

    if (const ReturnStmt *Return =
            Result.Nodes.getNodeAs<ReturnStmt>("ReturnInPgTryBlock")) {
      // PG_TRYブロック内にreturn文が見つかりました。警告を出しましょう。
      DiagnosticsEngine &DE = Ctx->getDiagnostics();
      unsigned DiagID = DE.getCustomDiagID(
          DiagnosticsEngine::Error,
          "PG_TRYブロック内で安全でないreturn文が使用されています");
      auto DB = DE.Report(Return->getReturnLoc(), DiagID);
      DB.AddSourceRange(
          CharSourceRange::getCharRange(Return->getSourceRange()));
    } else if (const IfStmt *If =
                   Result.Nodes.getNodeAs<IfStmt>("PgTryBlock")) {
      // PG_TRY()ブロック内の'break'/'continue'/'goto'文が安全でないかどうかを確認します。
      const Stmt *Then = If->getThen();
      CheckUnsafeBreakStmt(Then, Ctx);
      CheckUnsafeContinueStmt(Then, Ctx);
      CheckUnsafeGotoStmt(Then, Ctx);
    }
  }
};

PG_TRY()ブロック内でのbreak/continue/goto文の使用の安全性を確認するためのコードは非常に似ています。ここでは、CheckUnsafeBreakStmt()を例に取ります。その基本的なアイデアは、一致した AST に対して BFS を実行することです。

static void CheckUnsafeBreakStmt(const Stmt *Then, ASTContext *Ctx) {
  std::queue<const Stmt *> StmtQueue;
  StmtQueue.push(Then);
  while (!StmtQueue.empty()) {
    const Stmt *CurrStmt = StmtQueue.front();
    StmtQueue.pop();

    if (!CurrStmt)
      continue;

    if (const BreakStmt *Break =
            llvm::dyn_cast_if_present<BreakStmt>(CurrStmt)) {
      // PG_TRYブロック内にbreak文が見つかりました。警告を出しましょう。
      DiagnosticsEngine &DE = Ctx->getDiagnostics();
      unsigned DiagID = DE.getCustomDiagID(
          DiagnosticsEngine::Error,
          "PG_TRYブロック内で使用されるbreak文は安全でありません");
      auto DB = DE.Report(Break->getBreakLoc(), DiagID);
      DB.AddSourceRange(CharSourceRange::getCharRange(Break->getSourceRange()));
    }

    // while/do-while/for/switch文内のbreak文は安全です。子ノードに対してBFSを実行する必要はありません。
    if (llvm::isa<WhileStmt>(CurrStmt) || llvm::isa<DoStmt>(CurrStmt) ||
        llvm::isa<ForStmt>(CurrStmt) || llvm::isa<SwitchStmt>(CurrStmt)) {
      continue;
    }

    for (const Stmt *C : CurrStmt->children()) {
      StmtQueue.push(C);
    }
  }
}

これで、私たちのチェッカーは PostgreSQL ベースのプロジェクトで安全でないコードパターンを報告できるようになりました。このチェッカーのソースコードは私の GitHub リポジトリで見つけることができます3。

実際の世界で潜在的なバグを見つけることができたか？#

はい、見つけました！私は PostgreSQL1やもちろん Greenplum の中でいくつかの安全でないコードを見つけました（私は PostgreSQL でそれを修正したいので、Greenplum には問題を提出しませんでした）。pgsql-hackers メーリングリストからの興味深い返信のいくつかは以下の通りです。

  • Tom Lane は、PG_TRY()ブロック内でのbreak/continue/gotoの使用が物事を混乱させる可能性があると述べています。
  • Andres Freund は、clang のスレッド安全性分析を使用した非常にクールなコンパイラハッキングを提供しています。このパッチは私の AST マッチャーのアイデアよりも優れているようです。彼のパッチがコミットされれば、PostgreSQL のコンパイル中にそのような安全でないコードパターンを拒否できます。
From d1c99e9d12ba01adb21c5f17c792be44cfeef20f Mon Sep 17 00:00:00 2001
From: Andres Freund <[email protected]>
Date: Thu, 12 Jan 2023 21:18:55 -0800
Subject: [PATCH v1] wip: use clang anotations to warn if code in
 PG_TRY/CATCH/FINALLY returns

Only hooked up to meson right now.
---
 meson.build              |  1 +
 src/include/utils/elog.h | 43 +++++++++++++++++++++++++++++++++++++---
 2 files changed, 41 insertions(+), 3 deletions(-)

diff --git a/meson.build b/meson.build
index 45fb9dd616e..66a40e728f4 100644
--- a/meson.build
+++ b/meson.build
@@ -1741,6 +1741,7 @@ common_warning_flags = [
   '-Wimplicit-fallthrough=3',
   '-Wcast-function-type',
   '-Wshadow=compatible-local',
+  '-Wthread-safety',
   # This was included in -Wall/-Wformat in older GCC versions
   '-Wformat-security',
 ]
diff --git a/src/include/utils/elog.h b/src/include/utils/elog.h
index 4a9562fdaae..b211e08322a 100644
--- a/src/include/utils/elog.h
+++ b/src/include/utils/elog.h
@@ -381,32 +381,69 @@ extern PGDLLIMPORT ErrorContextCallback *error_context_stack;
  * same within each component macro of the given PG_TRY() statement.
  *----------
  */
+
+
+/*
+ * Annotations for detecting returns inside a PG_TRY(), using clang's thread
+ * safety annotations.
+ *
+ * The "lock" implementations need no_thread_safety_analysis as clang can't
+ * understand how a lock is implemented. We wouldn't want an implementation
+ * anyway, since there's no real lock here.
+ */
+#ifdef __clang__
+
+typedef int __attribute__((capability("no_returns_in_pg_try"))) no_returns_handle_t;
+
+static inline void no_returns_start(no_returns_handle_t l)
+	__attribute__((acquire_capability(l)))
+	__attribute__((no_thread_safety_analysis))
+{
+}
+
+static inline void no_returns_stop(no_returns_handle_t l)
+	__attribute__((release_capability(l)))
+	__attribute__((no_thread_safety_analysis))
+{}
+#else
+typedef int pg_attribute_unused() no_returns_handle_t;
+#define no_returns_start(t) (void)0
+#define no_returns_stop(t) (void)0
+#endif
+
 #define PG_TRY(...)  \
 	do { \
 		sigjmp_buf *_save_exception_stack##__VA_ARGS__ = PG_exception_stack; \
 		ErrorContextCallback *_save_context_stack##__VA_ARGS__ = error_context_stack; \
 		sigjmp_buf _local_sigjmp_buf##__VA_ARGS__; \
 		bool _do_rethrow##__VA_ARGS__ = false; \
+		no_returns_handle_t no_returns_handle##__VA_ARGS__ = 0; \
 		if (sigsetjmp(_local_sigjmp_buf##__VA_ARGS__, 0) == 0) \
 		{ \
-			PG_exception_stack = &_local_sigjmp_buf##__VA_ARGS__
+			PG_exception_stack = &_local_sigjmp_buf##__VA_ARGS__; \
+		    no_returns_start(no_returns_handle##__VA_ARGS__)
 
 #define PG_CATCH(...)	\
+			no_returns_stop(no_returns_handle##__VA_ARGS__); \
 		} \
 		else \
 		{ \
 			PG_exception_stack = _save_exception_stack##__VA_ARGS__; \
-			error_context_stack = _save_context_stack##__VA_ARGS__
+			error_context_stack = _save_context_stack##__VA_ARGS__; \
+		    no_returns_start(no_returns_handle##__VA_ARGS__)
 
 #define PG_FINALLY(...) \
+			no_returns_stop(no_returns_handle##__VA_ARGS__); \
 		} \
 		else \
 			_do_rethrow##__VA_ARGS__ = true; \
 		{ \
 			PG_exception_stack = _save_exception_stack##__VA_ARGS__; \
-			error_context_stack = _save_context_stack##__VA_ARGS__
+			error_context_stack = _save_context_stack##__VA_ARGS__; \
+		    no_returns_start(no_returns_handle##__VA_ARGS__)
 
 #define PG_END_TRY(...)  \
+			no_returns_stop(no_returns_handle##__VA_ARGS__); \
 		} \
 		if (_do_rethrow##__VA_ARGS__) \
 				PG_RE_THROW(); \
-- 
2.38.0

次のステップは？#

これは私の Clang ベースのチェッカーを書く初めての試みです。PG_TRY()ブロック内での安全でないreturn/break/continue/goto文の使用に加えて、PG_TRY()ブロック内の自動ストレージクラスのローカル変数を変更し、PG_CATCH()ブロックで使用するなど、まだいくつかの安全でないコードパターンがあります。将来的には、これらの安全でないコードパターンに対するより多くのチェッカーがあれば素晴らしいと思います。

Footnotes#

  1. https://www.postgresql.org/message-id/CACpMh+CMsGMRKFzFMm3bYTzQmMU5nfEEoEDU2apJcc4hid36AQ@mail.gmail.com ↩ ↩2

  2. https://github.com/greenplum-db/gpdb/pull/14205 ↩

  3. https://github.com/higuoxing/clang-plugins/blob/88eeb2bea0ade224807bb3e35f1d048dd4d3697c/lib/ReturnInPgTryBlockChecker.cpp ↩

読み込み中...
文章は、創作者によって署名され、ブロックチェーンに安全に保存されています。