我为什么想编写一个 Clang 插件?#
我的日常工作是为一个名为 Greenplum 的数据库开发扩展。它是一个源自 PostgreSQL 的分布式数据库。每次我使用它时,当遇到使用 PG_TRY()
/PG_CATCH()
块的 PostgreSQL 错误处理代码时,我都会感到警惕,因为我们见过许多因错误使用而导致的 bug。我决定编写一些自动化工具来捕捉这些 bug。
PostgreSQL 错误处理代码是什么样的?#
除了 PG_TRY()
和 PG_CATCH()
,在 PostgreSQL 错误处理过程中还有两个额外的宏: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
的先前环境,这样如果我们想将错误传递给上层调用者(调用 PG_RE_THROW()
,这是 siglongjmp()
的另一种包装),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();
-
在
PG_TRY()
块的任何地方使用break
语句,除了在switch
、do-while
、while
和for
语句内部使用的情况。PG_TRY(); { ... // 不安全,因为 break 语句跳出了 // PG_TRY() 块,这会破坏错误处理 // 栈上下文或环境。 break; do { // 安全,因为我们跳出了 do-while 循环而不是 // PG_TRY() 块。 break; } while (0); while (1) { // 安全。同上。 break; } for (;;) { // 安全。同上。 } switch (c) { case 1: // 安全。同上。 break; } } PG_CATCH(); { ... } PG_END_TRY();
-
在
PG_TRY()
块的任何地方使用continue
语句,除了在do-while
、while
和for
循环内部使用的情况。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(
// 为了方便,我们将带有 return 语句的 PG_TRY() 块绑定为
// 名称 '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。
它能在现实世界中发现潜在的 bug 吗?#
是的,它发现了!我在 PostgreSQL1 中发现了一些不安全的代码,当然也在 Greenplum 中(我没有向 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()
块中使用它。未来能够有更多的检查器来检查这些不安全的代码模式将是非常好的。