2026-06-01 · claude-opus-4-6

Monthly Summary: Fix Mismatched Deallocation Functions (May 2026)

Overview

Tristan Partin proposed a mechanical refactor to fix allocator/deallocator mismatches in PostgreSQL's frontend code, using a Coccinelle semantic patch. The goal is to enforce consistent pairing between the two parallel allocator families (pg_malloc/pg_free and palloc/pfree) in frontend builds, enabling future adoption of GCC's __attribute__((malloc(deallocator))) for compile-time enforcement. The first substantive review from Zsolt Parragi challenged both the completeness of the Coccinelle output and the overall methodology.

Problem Statement

PostgreSQL frontend code has two allocator families — pg_* (native frontend wrappers) and p* (shims for backend/frontend shared code) — both ultimately calling libc malloc/free. Mismatches between families (e.g., pnstrdup() + free()) are functionally harmless today but:

Block __attribute__((malloc(deallocator))) adoption, which would enable -Wmismatched-dealloc warnings and improve static analysis
Create cognitive overhead during code review (cannot reason locally about ownership)
Are fragile if frontend p* wrappers ever gain real behavior (debug instrumentation, minimal context allocator)

Proposed Approach

A Coccinelle .cocci script mechanically rewrites deallocator calls to match their corresponding allocator family. Run with --allow-inconsistent-paths, the script produces a large diff across src/fe_utils, src/bin, libpq, etc. The approach is reproducible and declarative.

First Review: Completeness and Strategy Challenged

Zsolt Parragi identified concrete deficiencies:

Missed rewrites

pg_malloc_array macro calls not recognized by the .cocci rules (syntactic mismatch)
Cast expressions and out-parameter assignments defeating the pattern matcher
Cross-function allocation (e.g., parse_identifier allocating into an out-parameter freed elsewhere) invisible to intra-procedural analysis

Internal inconsistency

In strtokx (src/fe_utils/stringutils.c), the patch rewrites free() on line 96 but leaves a structurally identical free() on line 73 untouched — a direct artifact of --allow-inconsistent-paths

Methodological counter-proposal: attributes-first

Zsolt proposes inverting the workflow:

Add __attribute__((malloc(deallocator))) annotations first
Let the compiler/clang-tidy flag every mismatch automatically
Fix mismatches guided by compiler output (possibly with clang-tidy auto-fix)

The argument: Coccinelle is inherently intra-procedural and syntactic, so it cannot achieve completeness for the cross-TU ownership patterns that exist in the codebase. Attributes propagate ownership through function signatures and enable tools that can reason inter-procedurally.

Current Status

The patch is stalled pending response from Tristan to the review feedback. Two concrete issues must be resolved:

The specific missed sites and inconsistencies in the current diff
The strategic question of whether Coccinelle-first or attributes-first is the correct ordering

No committer has weighed in yet.

2026-06-01 · claude-opus-4-6

New Round: Tristan Responds with v2 Patch, Clarifies Strategy

Tristan Partin responds to Zsolt's review with a v2 patch that addresses the specific missed sites and the strtokx inconsistency, along with an updated Coccinelle script. More importantly, he clarifies the strategic relationship between this patch and the attribute work.

Concrete fixes in v2

The Coccinelle script has been adjusted to cover the cases Zsolt identified (e.g., pg_malloc_array macro, pg_strdup/free mismatches in tab-complete.in.c, and the inconsistent strtokx handling).
The stringutils.c line 73 vs line 96 inconsistency is explicitly noted as fixed.

Strategic clarification: this patch is a prerequisite, not the end-goal

Tristan explicitly reframes the patch's purpose: it is not intended to find every mismatch — it is intended to get the codebase into a state where __attribute__((malloc(deallocator))) annotations can be added without generating new compiler warnings. The annotations themselves are the enforcement mechanism going forward. This directly addresses Zsolt's "attributes-first" counter-proposal by acknowledging the same end-goal but defending a different ordering:

Clean up known mismatches (this patch) → 2. Add attributes (small patch, ~45 insertions) → 3. Compiler enforces going forward via -Wmismatched-dealloc and -Wfree-nonheap-object.

The key insight is pragmatic: you can't add the attributes until the existing violations are cleaned up, otherwise the attribute addition itself would generate a flood of warnings (or errors under -Werror). So the cleanup must come first by logical necessity.

Attribute patch is small and already exists

Tristan reveals the attribute patch is already written and is modest in scope (3 files, +45/-33 lines), touching only:

src/include/c.h (12 new lines — likely a portability macro for the attribute)
src/include/common/fe_memutils.h (reformatting declarations with attributes)
src/include/utils/palloc.h (same for backend palloc declarations)

This counters any concern about attribute "noise" — the annotations are concentrated in headers and minimal.

Additional GCC diagnostic unlocked

Beyond -Wmismatched-dealloc, Tristan notes that the malloc attribute also enables -Wfree-nonheap-object diagnostics, expanding the class of bugs the compiler can detect once attributes are in place.

Tristan does NOT adopt the attributes-first methodology

Notably, Tristan does not shift to Zsolt's proposed workflow (add attributes first, let compiler find mismatches). He maintains the cleanup-first approach, arguing it's a logical prerequisite. The implicit disagreement is whether a CI-only or staged approach could work — Tristan's position is that committed attributes require a clean baseline first.

Fix mismatched deallocation functions

Latest Update