From d1c99e9d12ba01adb21c5f17c792be44cfeef20f Mon Sep 17 00:00:00 2001 From: Andres Freund 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