Re: Fix uninitialized xl_running_xacts padding

Hello!

I'd like to propose a different approach: instead of relying on
valgrind and runtime detection of the issue, why don't we (also) add
specific static analysis rules to detect the situation at compile
time?

There are several threads when I had the same idea: maybe I should
write a postgres specific clang-tidy checker, and ask what everyone
thinks about integrating that into the build process in an optional
way?

I attached a WIP patch that addresses this, specifically for the xlog
padding problem for now.

src/tools/pg-tidy contains a basic custom clang-tidy plugin that works
based on two annotations (and helper macros that resolve to "" for
normal compilation):

* PG_NOPADDING can be used to mark that a struct doesn't have any
padding. If this annotation is added to a struct, but it has padding,
it will generate clang-tidy warnings. This is basically "-Wpadded",
but specifically for selected types.
* A separate check rule requires all PG_NOPADDING structs to be always
zero-initialized, meaning we don't have to rely on memset at all
* PG_REQUIRE_NOPADDING can be used to mark function arguments. If an
argument is marked with this, then the underlying type of its
parameter has to be either a primitive type, or a struct annotated
with PG_NOPADDING

Possible alternatives:
* I could simplify this by removing PG_NOPADDING, and instead checking
the requirement at every call site of a function with
PG_REQUIRE_NOPADDING. That would also mean that it could only enforce
zero-initialization when it's clearly visible in the same function. I
choose the two annotation approach for increased reliability
* I could simplify this to only check for end padding, enforce memset
instead of zero initialization, and build upon Alexander's previous
patch.


0001 implements and integrates pg-tidy (I only added it to the meson
build, if there's interest I can also add it to make. clang-tidy
integration works similarly to the llvm bitcode patch, so it is
properly parallelized/incremental)
0002 adds basic helper macros
0003 marks the data in XLogRegisterData with PG_REQUIRE_NOPADDING, all
related structs with PG_NOPADDING, and then fixes the padding issues
by adding explicit padding data instead of the compiler autogenerated
padding. We also cast a few arrays to char* (or alternatively we could
use nolint to suppress the check), because I didn't want to everywhere
zero initialize types like RelFileLocator.
0004 removes the now trivial SizeOf macros

Compared to only using valgrind, clang-tidy:
* works at compile time, guaranteed for every type used with xlog
* in theory should work with extensions (if we want to, e.g. by
integrating it into pgxs), without requiring extension developers to
add proper test workflows using valgrind
* valgrind should still work

What do you think? I'm interested in opinions about both the specific
case, and the generic idea of using custom clang-tidy checks for
various postgres-specific checks. As I mentioned at the beginning of
the message I think this could be useful for other things and doesn't
always require custom annotations, in several cases it could work
without any C code change.