Add counted_by attribute

On Wed, 29 Jul 2026 at 22:07, "Tristan Partin" <tristan@partin.io> wrote:

The counted_by[0] compiler attribute is fairly new. It was added in GCC
15 and Clang 18. It has been used fairly extensively in the Linux
kernel[0].

To summarize the benefits of the attribute:

- Runtime bounds checking with -DFORTIFY_SOURCE=3 and -fsanitize-bounds
- Accurate reporting of __builtin_dynamic_object_size()

While we don't use __builtin_dynamic_object_size(), I think the runtime
bounds checking improvements are easily worth the little bit of effort
to add the attribute in various locations and review the code. I think
it will improve things for buildfarm animals using ASan due to expanded
coverage.

Adding this attribute to the codebase was previously proposed back in
2024[1], but the thread never got any traction. I figured that I would
try again, but bring some patches this time. I figured that I would
start small, and then after people agree that it has value, I can look
into adding the attribute in many more places.

[0]: https://gcc.gnu.org/onlinedocs/gcc/Common-Attributes.html#index-counted_005fby
[1]: https://people.kernel.org/gustavoars/how-to-use-the-new-counted_by-attribute-in-c-and-linux
[2]: /messages/by-id/ME3P282MB3166210CDE36BD485B703BF6B6C82@ME3P282MB3166.AUSP282.PROD.OUTLOOK.COM

+1 for this idea.

--
Tristan Partin
PostgreSQL Contributors Team
AWS (https://aws.amazon.com)

--
Regards,
Japin Li
ChengDu WenWu Information Technology Co., Ltd.

On 30.07.26 00:07, Tristan Partin wrote:

The counted_by[0] compiler attribute is fairly new. It was added in GCC
15 and Clang 18. It has been used fairly extensively in the Linux
kernel[0].

To summarize the benefits of the attribute:

- Runtime bounds checking with -DFORTIFY_SOURCE=3 and -fsanitize-bounds
- Accurate reporting of __builtin_dynamic_object_size()

While we don't use __builtin_dynamic_object_size(), I think the runtime
bounds checking improvements are easily worth the little bit of effort
to add the attribute in various locations and review the code. I think
it will improve things for buildfarm animals using ASan due to expanded
coverage.

I think this is a good idea.

Maybe you could produce an intentionally broken piece of code that would
illustrate what kinds of reports one might hope to get from this.

On Wed Aug 5, 2026 at 4:05 PM UTC, Peter Eisentraut wrote:

On 30.07.26 00:07, Tristan Partin wrote:

The counted_by[0] compiler attribute is fairly new. It was added in GCC
15 and Clang 18. It has been used fairly extensively in the Linux
kernel[0].

To summarize the benefits of the attribute:

- Runtime bounds checking with -DFORTIFY_SOURCE=3 and -fsanitize-bounds
- Accurate reporting of __builtin_dynamic_object_size()

While we don't use __builtin_dynamic_object_size(), I think the runtime
bounds checking improvements are easily worth the little bit of effort
to add the attribute in various locations and review the code. I think
it will improve things for buildfarm animals using ASan due to expanded
coverage.

I think this is a good idea.

Maybe you could produce an intentionally broken piece of code that would
illustrate what kinds of reports one might hope to get from this.

Sure. I'll share three examples.

Back in 2022, Tom committed a fix for an off by one error:
a36196972b[0]https://github.com/postgres/postgres/commit/a36196972b. Assuming we had an animal configured for
-fsanitize=address, and the accompanying diff (logical-replication.diff)
applied, we could have probably done a better job at avoiding this
mistake in the first place. The animal would have rightfully complained
about this mistake.

Another example: TupleDescCompactAttr() is called into over 100 times
and takes an index into the TupleDesc::compact_attrs array. That
function does not contain an Assert() for checking if the index is
actually valid. An errant caller could easily cause a SIGSEGV or we
could reach into uninitialized memory. In the supplied diff
(tuple.diff), I added the counted_by attribute, which would allow us to
fail loudly if an animal was configured with -fsanitize=address. I also
went ahead and also added an Assert() just to be safe.

To give a more concrete example, I have attached a C file that has
a similar off by one mistake to the one that Tom fixed. You can compile
it with the following command line:

gcc -fsanitize=address test.c

And if you run it (./a.out), you will see something like the following:

$ ./a.out
0
0
0
=================================================================
==1651747==ERROR: AddressSanitizer: heap-buffer-overflow on address 0x7bf02dde001c at pc 0x55d9fecc52a2 bp 0x7ffcaf3ff4a0 sp 0x7ffcaf3ff498
READ of size 4 at 0x7bf02dde001c thread T0
#0 0x55d9fecc52a1 in main (/local/home/dbltap/Projects/postgres/counted_by/a.out+0x12a1)
#1 0x7fd02ee2b284 in __libc_start_call_main (/nix/store/qqiqd3ah10x8hzsif4j1y4xc1miw23nx-glibc-2.42-67/lib/libc.so.6+0x2b284) (BuildId: c38f504d883365a4836730fb07e9269ade763988)
#2 0x7fd02ee2b337 in __libc_start_main@GLIBC_2.2.5 (/nix/store/qqiqd3ah10x8hzsif4j1y4xc1miw23nx-glibc-2.42-67/lib/libc.so.6+0x2b337) (BuildId: c38f504d883365a4836730fb07e9269ade763988)
#3 0x55d9fecc50e4 in _start (/local/home/dbltap/Projects/postgres/counted_by/a.out+0x10e4)

0x7bf02dde001c is located 0 bytes after 12-byte region [0x7bf02dde0010,0x7bf02dde001c)
allocated by thread T0 here:
#0 0x7fd02f32be4f in calloc (/nix/store/lv6lq05xay6zr2lbchz47zs8yrza3y86-gcc-16.2.0-lib/lib/libasan.so.8+0x12be4f)
#1 0x55d9fecc524a in main (/local/home/dbltap/Projects/postgres/counted_by/a.out+0x124a)
#2 0x7fd02ee2b284 in __libc_start_call_main (/nix/store/qqiqd3ah10x8hzsif4j1y4xc1miw23nx-glibc-2.42-67/lib/libc.so.6+0x2b284) (BuildId: c38f504d883365a4836730fb07e9269ade763988)
#3 0x7fd02ee2b337 in __libc_start_main@GLIBC_2.2.5 (/nix/store/qqiqd3ah10x8hzsif4j1y4xc1miw23nx-glibc-2.42-67/lib/libc.so.6+0x2b337) (BuildId: c38f504d883365a4836730fb07e9269ade763988)
#4 0x55d9fecc50e4 in _start (/local/home/dbltap/Projects/postgres/counted_by/a.out+0x10e4)

[0]: https://github.com/postgres/postgres/commit/a36196972b

--
Tristan Partin
PostgreSQL Contributors Team
AWS (https://aws.amazon.com)

On 30.07.26 00:07, Tristan Partin wrote:

The counted_by[0] compiler attribute is fairly new. It was added in GCC
15 and Clang 18. It has been used fairly extensively in the Linux
kernel[0].

To summarize the benefits of the attribute:

- Runtime bounds checking with -DFORTIFY_SOURCE=3 and -fsanitize-bounds
- Accurate reporting of __builtin_dynamic_object_size()

While we don't use __builtin_dynamic_object_size(), I think the runtime
bounds checking improvements are easily worth the little bit of effort
to add the attribute in various locations and review the code. I think
it will improve things for buildfarm animals using ASan due to expanded
coverage.

I took a closer look at this. There are several problems with the
proposed patches.

1) In C++, both gcc and clang have __has_attribute(counted_by) return 1
(true), but the compiler actually rejects the attribute with a warning.
This is not immediately evident in your patch, but it would show up
under cpluspluscheck and whenever we extend this attribute to header
files that happen to get pulled in by C++ source files.
(access/tupdesc.h is an obvious candidate.) Therefore, there needs to be
some #ifndef __cplusplus somewhere.

2) gcc 15 and clang 18 accept the counted_by attribute only for flexible
array members, not for pointers members. (Using it on a pointer causes
an error.) If you want to apply this to pointer members, as your patch
does in buffile.c, you'd have to write a configure test. Or else
restrict it to flexible array members for now.

3) The counted_by attribute requires that, when extending the counted
array, the count field is increased before writing into the new element
at the end. The code dealing with struct BufFile currently doesn't do
that, and so your change in buffile.c fails under -fsanitize=bounds:

../src/backend/storage/file/buffile.c:919:3: runtime error: index 1 out
of bounds for type 'File * __counted_by(numFiles)' (aka 'int *')

(Reproduce with meson configure -Db_sanitize=bounds and meson test ...
--suite regress.)

The code needs to be carefully analyzed and adjusted to fix this. (The
code for the tuplesort.c change appears to be ok.)

4) Although the compilers are flexible with the placement, the most
correct placement of the attribute is at the beginning of the
declaration, like

pg_attribute_counted_by(nTapes) TapeShare tapes[FLEXIBLE_ARRAY_MEMBER];

(Note that the gcc documentation effectively writes it this way.)

Additionally, with this arrangement, we could also make use of the MSVC
_Field_size_ annotation.

5) Minor: The counted_by attribute only takes a single argument, so the
use of __VA_ARGS__ seems excessive.

6) Minor: Awkward wording in comment: "This provides the compiler to
improve ..." -> "enables the compiler ..."?

Suggestion:
- Add C++ guard. (Maybe add annotation in access/tupdesc.h to test.)
- Skip use of the attribute on pointer members for now.
- Make sure cpluspluscheck and -fsanitize=bounds pass.
- Consider the cosmetic adjustments mentioned.

On Tue Sep 22, 2026 at 5:28 AM UTC, Peter Eisentraut wrote:

On 30.07.26 00:07, Tristan Partin wrote:

The counted_by[0] compiler attribute is fairly new. It was added in GCC
15 and Clang 18. It has been used fairly extensively in the Linux
kernel[0].

To summarize the benefits of the attribute:

- Runtime bounds checking with -DFORTIFY_SOURCE=3 and -fsanitize-bounds
- Accurate reporting of __builtin_dynamic_object_size()

While we don't use __builtin_dynamic_object_size(), I think the runtime
bounds checking improvements are easily worth the little bit of effort
to add the attribute in various locations and review the code. I think
it will improve things for buildfarm animals using ASan due to expanded
coverage.

I took a closer look at this. There are several problems with the
proposed patches.

1) In C++, both gcc and clang have __has_attribute(counted_by) return 1
(true), but the compiler actually rejects the attribute with a warning.
This is not immediately evident in your patch, but it would show up
under cpluspluscheck and whenever we extend this attribute to header
files that happen to get pulled in by C++ source files.
(access/tupdesc.h is an obvious candidate.) Therefore, there needs to be
some #ifndef __cplusplus somewhere.

I would love to understand the rationale for returning 1 when the
compiler will just throw a warning anyway. Fixed.

2) gcc 15 and clang 18 accept the counted_by attribute only for flexible
array members, not for pointers members. (Using it on a pointer causes
an error.) If you want to apply this to pointer members, as your patch
does in buffile.c, you'd have to write a configure test. Or else
restrict it to flexible array members for now.

I like the idea of restricting it to flexible array members for now.
It'll make for an easier review. Maybe in a subsequent patch we can
raise the minimum compiler versions of using pg_attribute_counted_by()
to GCC 16 and Clang 21.

3) The counted_by attribute requires that, when extending the counted
array, the count field is increased before writing into the new element
at the end. The code dealing with struct BufFile currently doesn't do
that, and so your change in buffile.c fails under -fsanitize=bounds:

../src/backend/storage/file/buffile.c:919:3: runtime error: index 1 out
of bounds for type 'File * __counted_by(numFiles)' (aka 'int *')

(Reproduce with meson configure -Db_sanitize=bounds and meson test ...
--suite regress.)

The code needs to be carefully analyzed and adjusted to fix this. (The
code for the tuplesort.c change appears to be ok.)

Good catch. In the upcoming changes, I ran test suites with
-fsanitize=bounds, and found one place that needed a fix. Note that
changes to buffile.c are not currently in scope for this patchset since
it wasn't a flexible array member.

4) Although the compilers are flexible with the placement, the most
correct placement of the attribute is at the beginning of the
declaration, like

pg_attribute_counted_by(nTapes) TapeShare tapes[FLEXIBLE_ARRAY_MEMBER];

(Note that the gcc documentation effectively writes it this way.)

The second patch uses postfix notation, but subsequent patches enable
support for prefix notation. I'll let you be the judge of whether to
commit prefix or postfix. Commits 3 & 4 are genuine improvements, though
they do also enable prefix support.

Additionally, with this arrangement, we could also make use of the MSVC
_Field_size_ annotation.

This is good motivation.

5) Minor: The counted_by attribute only takes a single argument, so the
use of __VA_ARGS__ seems excessive.

I think I just blindly copied surrounding macro code and forgot to
change it. Fixed in this new version.

6) Minor: Awkward wording in comment: "This provides the compiler to
improve ..." -> "enables the compiler ..."?

Fixed.

Suggestion:
- Add C++ guard. (Maybe add annotation in access/tupdesc.h to test.)
- Skip use of the attribute on pointer members for now.
- Make sure cpluspluscheck and -fsanitize=bounds pass.
- Consider the cosmetic adjustments mentioned.

Thanks for the review.

--
Tristan Partin
PostgreSQL Contributors Team
AWS (https://aws.amazon.com)

Hi, Tristan

Thanks for updating the patches.

On Wed, 23 Sep 2026 at 07:26, "Tristan Partin" <tristan@partin.io> wrote:

On Tue Sep 22, 2026 at 5:28 AM UTC, Peter Eisentraut wrote:

On 30.07.26 00:07, Tristan Partin wrote:

The counted_by[0] compiler attribute is fairly new. It was added in GCC
15 and Clang 18. It has been used fairly extensively in the Linux
kernel[0].

To summarize the benefits of the attribute:

- Runtime bounds checking with -DFORTIFY_SOURCE=3 and -fsanitize-bounds
- Accurate reporting of __builtin_dynamic_object_size()

While we don't use __builtin_dynamic_object_size(), I think the runtime
bounds checking improvements are easily worth the little bit of effort
to add the attribute in various locations and review the code. I think
it will improve things for buildfarm animals using ASan due to expanded
coverage.

I took a closer look at this. There are several problems with the
proposed patches.

1) In C++, both gcc and clang have __has_attribute(counted_by) return 1
(true), but the compiler actually rejects the attribute with a warning.
This is not immediately evident in your patch, but it would show up
under cpluspluscheck and whenever we extend this attribute to header
files that happen to get pulled in by C++ source files.
(access/tupdesc.h is an obvious candidate.) Therefore, there needs to be
some #ifndef __cplusplus somewhere.

I would love to understand the rationale for returning 1 when the
compiler will just throw a warning anyway. Fixed.

2) gcc 15 and clang 18 accept the counted_by attribute only for flexible
array members, not for pointers members. (Using it on a pointer causes
an error.) If you want to apply this to pointer members, as your patch
does in buffile.c, you'd have to write a configure test. Or else
restrict it to flexible array members for now.

I like the idea of restricting it to flexible array members for now.
It'll make for an easier review. Maybe in a subsequent patch we can
raise the minimum compiler versions of using pg_attribute_counted_by()
to GCC 16 and Clang 21.

3) The counted_by attribute requires that, when extending the counted
array, the count field is increased before writing into the new element
at the end. The code dealing with struct BufFile currently doesn't do
that, and so your change in buffile.c fails under -fsanitize=bounds:

../src/backend/storage/file/buffile.c:919:3: runtime error: index 1 out
of bounds for type 'File * __counted_by(numFiles)' (aka 'int *')

(Reproduce with meson configure -Db_sanitize=bounds and meson test ...
--suite regress.)

The code needs to be carefully analyzed and adjusted to fix this. (The
code for the tuplesort.c change appears to be ok.)

Good catch. In the upcoming changes, I ran test suites with
-fsanitize=bounds, and found one place that needed a fix. Note that
changes to buffile.c are not currently in scope for this patchset since
it wasn't a flexible array member.

4) Although the compilers are flexible with the placement, the most
correct placement of the attribute is at the beginning of the
declaration, like

pg_attribute_counted_by(nTapes) TapeShare tapes[FLEXIBLE_ARRAY_MEMBER];

(Note that the gcc documentation effectively writes it this way.)

The second patch uses postfix notation, but subsequent patches enable
support for prefix notation. I'll let you be the judge of whether to
commit prefix or postfix. Commits 3 & 4 are genuine improvements, though
they do also enable prefix support.

Additionally, with this arrangement, we could also make use of the MSVC
_Field_size_ annotation.

This is good motivation.

5) Minor: The counted_by attribute only takes a single argument, so the
use of __VA_ARGS__ seems excessive.

I think I just blindly copied surrounding macro code and forgot to
change it. Fixed in this new version.

6) Minor: Awkward wording in comment: "This provides the compiler to
improve ..." -> "enables the compiler ..."?

Fixed.

Suggestion:
- Add C++ guard. (Maybe add annotation in access/tupdesc.h to test.)
- Skip use of the attribute on pointer members for now.
- Make sure cpluspluscheck and -fsanitize=bounds pass.
- Consider the cosmetic adjustments mentioned.

Thanks for the review.

I tested it locally, and all tests passed.

I found some places that could use the new pg_attribute_counted_by() attribute
e.g., in heapam_xlog.h, xact.h, etc. Are those intentional omissions?

Was this an oversight, or is pg_attribute_counted_by not needed here?

Haven't checked everywhere yet. If it is oversight, I'll check it later.

--
Regards,
Japin Li
ChengDu WenWu Information Technology Co., Ltd.

On Wed Sep 23, 2026 at 6:33 AM CDT, Japin Li wrote:

Hi, Tristan

Thanks for updating the patches.

On Wed, 23 Sep 2026 at 07:26, "Tristan Partin" <tristan@partin.io> wrote:

On Tue Sep 22, 2026 at 5:28 AM UTC, Peter Eisentraut wrote:

On 30.07.26 00:07, Tristan Partin wrote:

The counted_by[0] compiler attribute is fairly new. It was added in GCC
15 and Clang 18. It has been used fairly extensively in the Linux
kernel[0].

To summarize the benefits of the attribute:

- Runtime bounds checking with -DFORTIFY_SOURCE=3 and -fsanitize-bounds
- Accurate reporting of __builtin_dynamic_object_size()

While we don't use __builtin_dynamic_object_size(), I think the runtime
bounds checking improvements are easily worth the little bit of effort
to add the attribute in various locations and review the code. I think
it will improve things for buildfarm animals using ASan due to expanded
coverage.

I took a closer look at this. There are several problems with the
proposed patches.

1) In C++, both gcc and clang have __has_attribute(counted_by) return 1
(true), but the compiler actually rejects the attribute with a warning.
This is not immediately evident in your patch, but it would show up
under cpluspluscheck and whenever we extend this attribute to header
files that happen to get pulled in by C++ source files.
(access/tupdesc.h is an obvious candidate.) Therefore, there needs to be
some #ifndef __cplusplus somewhere.

I would love to understand the rationale for returning 1 when the
compiler will just throw a warning anyway. Fixed.

2) gcc 15 and clang 18 accept the counted_by attribute only for flexible
array members, not for pointers members. (Using it on a pointer causes
an error.) If you want to apply this to pointer members, as your patch
does in buffile.c, you'd have to write a configure test. Or else
restrict it to flexible array members for now.

I like the idea of restricting it to flexible array members for now.
It'll make for an easier review. Maybe in a subsequent patch we can
raise the minimum compiler versions of using pg_attribute_counted_by()
to GCC 16 and Clang 21.

3) The counted_by attribute requires that, when extending the counted
array, the count field is increased before writing into the new element
at the end. The code dealing with struct BufFile currently doesn't do
that, and so your change in buffile.c fails under -fsanitize=bounds:

../src/backend/storage/file/buffile.c:919:3: runtime error: index 1 out
of bounds for type 'File * __counted_by(numFiles)' (aka 'int *')

(Reproduce with meson configure -Db_sanitize=bounds and meson test ...
--suite regress.)

The code needs to be carefully analyzed and adjusted to fix this. (The
code for the tuplesort.c change appears to be ok.)

Good catch. In the upcoming changes, I ran test suites with
-fsanitize=bounds, and found one place that needed a fix. Note that
changes to buffile.c are not currently in scope for this patchset since
it wasn't a flexible array member.

4) Although the compilers are flexible with the placement, the most
correct placement of the attribute is at the beginning of the
declaration, like

pg_attribute_counted_by(nTapes) TapeShare tapes[FLEXIBLE_ARRAY_MEMBER];

(Note that the gcc documentation effectively writes it this way.)

The second patch uses postfix notation, but subsequent patches enable
support for prefix notation. I'll let you be the judge of whether to
commit prefix or postfix. Commits 3 & 4 are genuine improvements, though
they do also enable prefix support.

Additionally, with this arrangement, we could also make use of the MSVC
_Field_size_ annotation.

This is good motivation.

5) Minor: The counted_by attribute only takes a single argument, so the
use of __VA_ARGS__ seems excessive.

I think I just blindly copied surrounding macro code and forgot to
change it. Fixed in this new version.

6) Minor: Awkward wording in comment: "This provides the compiler to
improve ..." -> "enables the compiler ..."?

Fixed.

Suggestion:
- Add C++ guard. (Maybe add annotation in access/tupdesc.h to test.)
- Skip use of the attribute on pointer members for now.
- Make sure cpluspluscheck and -fsanitize=bounds pass.
- Consider the cosmetic adjustments mentioned.

Thanks for the review.

I tested it locally, and all tests passed.

I found some places that could use the new pg_attribute_counted_by() attribute
e.g., in heapam_xlog.h, xact.h, etc. Are those intentional omissions?

Was this an oversight, or is pg_attribute_counted_by not needed here?

Haven't checked everywhere yet. If it is oversight, I'll check it later.

I originally had these as well, but my understanding is that these
structures are populated from the filesystem, so in the event of data
corruption, the count field may no longer be accurate and could cause
a runtime failure similar to what Peter mentioned above in his review.
It's possible that I am being too cautious though. Let me know what you
think. I should have brought this up in my last email, but now is as
good a time as any to discuss!

--
Tristan Partin
PostgreSQL Contributors Team
AWS (https://aws.amazon.com)

Hi
Hi Tristan,

On Thu, 24 Sep 2026 at 16:17, "Tristan Partin" <tristan@partin.io> wrote:

On Wed Sep 23, 2026 at 6:33 AM CDT, Japin Li wrote:

Hi, Tristan

Thanks for updating the patches.

On Wed, 23 Sep 2026 at 07:26, "Tristan Partin" <tristan@partin.io> wrote:

On Tue Sep 22, 2026 at 5:28 AM UTC, Peter Eisentraut wrote:

On 30.07.26 00:07, Tristan Partin wrote:

The counted_by[0] compiler attribute is fairly new. It was added in GCC
15 and Clang 18. It has been used fairly extensively in the Linux
kernel[0].

To summarize the benefits of the attribute:

- Runtime bounds checking with -DFORTIFY_SOURCE=3 and -fsanitize-bounds
- Accurate reporting of __builtin_dynamic_object_size()

While we don't use __builtin_dynamic_object_size(), I think the runtime
bounds checking improvements are easily worth the little bit of effort
to add the attribute in various locations and review the code. I think
it will improve things for buildfarm animals using ASan due to expanded
coverage.

I took a closer look at this. There are several problems with the
proposed patches.

1) In C++, both gcc and clang have __has_attribute(counted_by) return 1
(true), but the compiler actually rejects the attribute with a warning.
This is not immediately evident in your patch, but it would show up
under cpluspluscheck and whenever we extend this attribute to header
files that happen to get pulled in by C++ source files.
(access/tupdesc.h is an obvious candidate.) Therefore, there needs to be
some #ifndef __cplusplus somewhere.

I would love to understand the rationale for returning 1 when the
compiler will just throw a warning anyway. Fixed.

2) gcc 15 and clang 18 accept the counted_by attribute only for flexible
array members, not for pointers members. (Using it on a pointer causes
an error.) If you want to apply this to pointer members, as your patch
does in buffile.c, you'd have to write a configure test. Or else
restrict it to flexible array members for now.

I like the idea of restricting it to flexible array members for now.
It'll make for an easier review. Maybe in a subsequent patch we can
raise the minimum compiler versions of using pg_attribute_counted_by()
to GCC 16 and Clang 21.

3) The counted_by attribute requires that, when extending the counted
array, the count field is increased before writing into the new element
at the end. The code dealing with struct BufFile currently doesn't do
that, and so your change in buffile.c fails under -fsanitize=bounds:

../src/backend/storage/file/buffile.c:919:3: runtime error: index 1 out
of bounds for type 'File * __counted_by(numFiles)' (aka 'int *')

(Reproduce with meson configure -Db_sanitize=bounds and meson test ...
--suite regress.)

The code needs to be carefully analyzed and adjusted to fix this. (The
code for the tuplesort.c change appears to be ok.)

Good catch. In the upcoming changes, I ran test suites with
-fsanitize=bounds, and found one place that needed a fix. Note that
changes to buffile.c are not currently in scope for this patchset since
it wasn't a flexible array member.

4) Although the compilers are flexible with the placement, the most
correct placement of the attribute is at the beginning of the
declaration, like

pg_attribute_counted_by(nTapes) TapeShare tapes[FLEXIBLE_ARRAY_MEMBER];

(Note that the gcc documentation effectively writes it this way.)

The second patch uses postfix notation, but subsequent patches enable
support for prefix notation. I'll let you be the judge of whether to
commit prefix or postfix. Commits 3 & 4 are genuine improvements, though
they do also enable prefix support.

Additionally, with this arrangement, we could also make use of the MSVC
_Field_size_ annotation.

This is good motivation.

5) Minor: The counted_by attribute only takes a single argument, so the
use of __VA_ARGS__ seems excessive.

I think I just blindly copied surrounding macro code and forgot to
change it. Fixed in this new version.

6) Minor: Awkward wording in comment: "This provides the compiler to
improve ..." -> "enables the compiler ..."?

Fixed.

Suggestion:
- Add C++ guard. (Maybe add annotation in access/tupdesc.h to test.)
- Skip use of the attribute on pointer members for now.
- Make sure cpluspluscheck and -fsanitize=bounds pass.
- Consider the cosmetic adjustments mentioned.

Thanks for the review.

I tested it locally, and all tests passed.

I found some places that could use the new pg_attribute_counted_by() attribute
e.g., in heapam_xlog.h, xact.h, etc. Are those intentional omissions?

Was this an oversight, or is pg_attribute_counted_by not needed here?

Haven't checked everywhere yet. If it is oversight, I'll check it later.

I originally had these as well, but my understanding is that these
structures are populated from the filesystem, so in the event of data
corruption, the count field may no longer be accurate and could cause
a runtime failure similar to what Peter mentioned above in his review.
It's possible that I am being too cautious though. Let me know what you
think. I should have brought this up in my last email, but now is as
good a time as any to discuss!

Thanks for the clear explanation.

I agree with you on the cautious approach — especially for structures that
may be populated from the filesystem, or where the count field does not
strictly represent the allocated/accessible length of the flexible array
member.

While reviewing remaining FAMs that do not yet use pg_attribute_counted_by(),
two cases looked like solid candidates:

1. BackgroundWorkerArray in bgworker.c

typedef struct BackgroundWorkerArray
{
int total_slots;
uint32 parallel_register_count;
uint32 parallel_terminate_count;
BackgroundWorkerSlot slot[FLEXIBLE_ARRAY_MEMBER];
} BackgroundWorkerArray;

The total_slots is set once at shared-memory initialization (based on
max_worker_processes) and never changes afterwards. All accesses are of the
form slot[i] with i < total_slots. The invariant is therefore trivial to
maintain.

2. vbits in contrib/pg_visibility/pg_visibility.c

typedef struct vbits
{
BlockNumber next;
BlockNumber count;
uint8 bits[FLEXIBLE_ARRAY_MEMBER];
} vbits;

The structure is allocated and filled in one place (collect_visibility_data),
with count equal to the number of blocks. Subsequent use is strictly read-only
and always checks next < count before indexing into bits[].

Both are purely internal, the count field matches the actual array bound, and
the update order is safe. I think it would be worthwhile to annotate them.

Thoughts?

--
Regards,
Japin Li
ChengDu WenWu Information Technology Co., Ltd.

On Sep 24, 2026, at 12:17 PM, Tristan Partin <tristan@partin.io> wrote:

On Wed Sep 23, 2026 at 6:33 AM CDT, Japin Li wrote:

Hi, Tristan

Thanks for updating the patches.

On Wed, 23 Sep 2026 at 07:26, "Tristan Partin" <tristan@partin.io> wrote:

On Tue Sep 22, 2026 at 5:28 AM UTC, Peter Eisentraut wrote:

On 30.07.26 00:07, Tristan Partin wrote:

The counted_by[0] compiler attribute is fairly new. It was added in GCC
15 and Clang 18. It has been used fairly extensively in the Linux
kernel[0].

To summarize the benefits of the attribute:

- Runtime bounds checking with -DFORTIFY_SOURCE=3 and -fsanitize-bounds
- Accurate reporting of __builtin_dynamic_object_size()

While we don't use __builtin_dynamic_object_size(), I think the runtime
bounds checking improvements are easily worth the little bit of effort
to add the attribute in various locations and review the code. I think
it will improve things for buildfarm animals using ASan due to expanded
coverage.

I took a closer look at this. There are several problems with the
proposed patches.

1) In C++, both gcc and clang have __has_attribute(counted_by) return 1
(true), but the compiler actually rejects the attribute with a warning.
This is not immediately evident in your patch, but it would show up
under cpluspluscheck and whenever we extend this attribute to header
files that happen to get pulled in by C++ source files.
(access/tupdesc.h is an obvious candidate.) Therefore, there needs to be
some #ifndef __cplusplus somewhere.

I would love to understand the rationale for returning 1 when the
compiler will just throw a warning anyway. Fixed.

2) gcc 15 and clang 18 accept the counted_by attribute only for flexible
array members, not for pointers members. (Using it on a pointer causes
an error.) If you want to apply this to pointer members, as your patch
does in buffile.c, you'd have to write a configure test. Or else
restrict it to flexible array members for now.

I like the idea of restricting it to flexible array members for now.
It'll make for an easier review. Maybe in a subsequent patch we can
raise the minimum compiler versions of using pg_attribute_counted_by()
to GCC 16 and Clang 21.

3) The counted_by attribute requires that, when extending the counted
array, the count field is increased before writing into the new element
at the end. The code dealing with struct BufFile currently doesn't do
that, and so your change in buffile.c fails under -fsanitize=bounds:

../src/backend/storage/file/buffile.c:919:3: runtime error: index 1 out
of bounds for type 'File * __counted_by(numFiles)' (aka 'int *')

(Reproduce with meson configure -Db_sanitize=bounds and meson test ...
--suite regress.)

The code needs to be carefully analyzed and adjusted to fix this. (The
code for the tuplesort.c change appears to be ok.)

Good catch. In the upcoming changes, I ran test suites with
-fsanitize=bounds, and found one place that needed a fix. Note that
changes to buffile.c are not currently in scope for this patchset since
it wasn't a flexible array member.

4) Although the compilers are flexible with the placement, the most
correct placement of the attribute is at the beginning of the
declaration, like

pg_attribute_counted_by(nTapes) TapeShare tapes[FLEXIBLE_ARRAY_MEMBER];

(Note that the gcc documentation effectively writes it this way.)

The second patch uses postfix notation, but subsequent patches enable
support for prefix notation. I'll let you be the judge of whether to
commit prefix or postfix. Commits 3 & 4 are genuine improvements, though
they do also enable prefix support.

Additionally, with this arrangement, we could also make use of the MSVC
_Field_size_ annotation.

This is good motivation.

5) Minor: The counted_by attribute only takes a single argument, so the
use of __VA_ARGS__ seems excessive.

I think I just blindly copied surrounding macro code and forgot to
change it. Fixed in this new version.

6) Minor: Awkward wording in comment: "This provides the compiler to
improve ..." -> "enables the compiler ..."?

Fixed.

Suggestion:
- Add C++ guard. (Maybe add annotation in access/tupdesc.h to test.)
- Skip use of the attribute on pointer members for now.
- Make sure cpluspluscheck and -fsanitize=bounds pass.
- Consider the cosmetic adjustments mentioned.

Thanks for the review.

I tested it locally, and all tests passed.

I found some places that could use the new pg_attribute_counted_by() attribute
e.g., in heapam_xlog.h, xact.h, etc. Are those intentional omissions?

Was this an oversight, or is pg_attribute_counted_by not needed here?

Haven't checked everywhere yet. If it is oversight, I'll check it later.

I originally had these as well, but my understanding is that these
structures are populated from the filesystem, so in the event of data
corruption, the count field may no longer be accurate and could cause
a runtime failure similar to what Peter mentioned above in his review.
It's possible that I am being too cautious though. Let me know what you
think. I should have brought this up in my last email, but now is as
good a time as any to discuss!

Tristan, nice work. I'm +1 for these changes. Is there a way to codify
that difference so that future hackers don't fall into this same trap?

-greg

I have committed the attribute and one use of it, and I'll let that run
through the buildfarm, and then work through the rest.

On Fri Sep 25, 2026 at 12:09 PM CDT, Greg Burd wrote:

On Sep 24, 2026, at 12:17 PM, Tristan Partin <tristan@partin.io> wrote:

On Wed Sep 23, 2026 at 6:33 AM CDT, Japin Li wrote:

Hi, Tristan

Thanks for updating the patches.

On Wed, 23 Sep 2026 at 07:26, "Tristan Partin" <tristan@partin.io> wrote:

On Tue Sep 22, 2026 at 5:28 AM UTC, Peter Eisentraut wrote:

On 30.07.26 00:07, Tristan Partin wrote:

The counted_by[0] compiler attribute is fairly new. It was added in GCC
15 and Clang 18. It has been used fairly extensively in the Linux
kernel[0].

To summarize the benefits of the attribute:

- Runtime bounds checking with -DFORTIFY_SOURCE=3 and -fsanitize-bounds
- Accurate reporting of __builtin_dynamic_object_size()

While we don't use __builtin_dynamic_object_size(), I think the runtime
bounds checking improvements are easily worth the little bit of effort
to add the attribute in various locations and review the code. I think
it will improve things for buildfarm animals using ASan due to expanded
coverage.

I took a closer look at this. There are several problems with the
proposed patches.

1) In C++, both gcc and clang have __has_attribute(counted_by) return 1
(true), but the compiler actually rejects the attribute with a warning.
This is not immediately evident in your patch, but it would show up
under cpluspluscheck and whenever we extend this attribute to header
files that happen to get pulled in by C++ source files.
(access/tupdesc.h is an obvious candidate.) Therefore, there needs to be
some #ifndef __cplusplus somewhere.

I would love to understand the rationale for returning 1 when the
compiler will just throw a warning anyway. Fixed.

2) gcc 15 and clang 18 accept the counted_by attribute only for flexible
array members, not for pointers members. (Using it on a pointer causes
an error.) If you want to apply this to pointer members, as your patch
does in buffile.c, you'd have to write a configure test. Or else
restrict it to flexible array members for now.

I like the idea of restricting it to flexible array members for now.
It'll make for an easier review. Maybe in a subsequent patch we can
raise the minimum compiler versions of using pg_attribute_counted_by()
to GCC 16 and Clang 21.

3) The counted_by attribute requires that, when extending the counted
array, the count field is increased before writing into the new element
at the end. The code dealing with struct BufFile currently doesn't do
that, and so your change in buffile.c fails under -fsanitize=bounds:

../src/backend/storage/file/buffile.c:919:3: runtime error: index 1 out
of bounds for type 'File * __counted_by(numFiles)' (aka 'int *')

(Reproduce with meson configure -Db_sanitize=bounds and meson test ...
--suite regress.)

The code needs to be carefully analyzed and adjusted to fix this. (The
code for the tuplesort.c change appears to be ok.)

Good catch. In the upcoming changes, I ran test suites with
-fsanitize=bounds, and found one place that needed a fix. Note that
changes to buffile.c are not currently in scope for this patchset since
it wasn't a flexible array member.

4) Although the compilers are flexible with the placement, the most
correct placement of the attribute is at the beginning of the
declaration, like

pg_attribute_counted_by(nTapes) TapeShare tapes[FLEXIBLE_ARRAY_MEMBER];

(Note that the gcc documentation effectively writes it this way.)

The second patch uses postfix notation, but subsequent patches enable
support for prefix notation. I'll let you be the judge of whether to
commit prefix or postfix. Commits 3 & 4 are genuine improvements, though
they do also enable prefix support.

Additionally, with this arrangement, we could also make use of the MSVC
_Field_size_ annotation.

This is good motivation.

5) Minor: The counted_by attribute only takes a single argument, so the
use of __VA_ARGS__ seems excessive.

I think I just blindly copied surrounding macro code and forgot to
change it. Fixed in this new version.

6) Minor: Awkward wording in comment: "This provides the compiler to
improve ..." -> "enables the compiler ..."?

Fixed.

Suggestion:
- Add C++ guard. (Maybe add annotation in access/tupdesc.h to test.)
- Skip use of the attribute on pointer members for now.
- Make sure cpluspluscheck and -fsanitize=bounds pass.
- Consider the cosmetic adjustments mentioned.

Thanks for the review.

I tested it locally, and all tests passed.

I found some places that could use the new pg_attribute_counted_by() attribute
e.g., in heapam_xlog.h, xact.h, etc. Are those intentional omissions?

Was this an oversight, or is pg_attribute_counted_by not needed here?

Haven't checked everywhere yet. If it is oversight, I'll check it later.

I originally had these as well, but my understanding is that these
structures are populated from the filesystem, so in the event of data
corruption, the count field may no longer be accurate and could cause
a runtime failure similar to what Peter mentioned above in his review.
It's possible that I am being too cautious though. Let me know what you
think. I should have brought this up in my last email, but now is as
good a time as any to discuss!

Tristan, nice work. I'm +1 for these changes. Is there a way to codify
that difference so that future hackers don't fall into this same trap?

I am thinking we could define another macro that merely serves as
a self-documenting annotation.

#define pg_attribute_unsafe_counted_by(count)

We don't define it to anything, but it allows us to annotate filesystem
structures accurately. Maybe in the future we could add a CFLAG to
define it to pg_attribute_counted_by(count) for testing purposes?

--
Tristan Partin
PostgreSQL Contributors Team
AWS (https://aws.amazon.com)

On Fri Sep 25, 2026 at 4:02 PM UTC, Japin Li wrote:

Hi
Hi Tristan,

On Thu, 24 Sep 2026 at 16:17, "Tristan Partin" <tristan@partin.io> wrote:

On Wed Sep 23, 2026 at 6:33 AM CDT, Japin Li wrote:

Hi, Tristan

Thanks for updating the patches.

On Wed, 23 Sep 2026 at 07:26, "Tristan Partin" <tristan@partin.io> wrote:

On Tue Sep 22, 2026 at 5:28 AM UTC, Peter Eisentraut wrote:

On 30.07.26 00:07, Tristan Partin wrote:

The counted_by[0] compiler attribute is fairly new. It was added in GCC
15 and Clang 18. It has been used fairly extensively in the Linux
kernel[0].

To summarize the benefits of the attribute:

- Runtime bounds checking with -DFORTIFY_SOURCE=3 and -fsanitize-bounds
- Accurate reporting of __builtin_dynamic_object_size()

While we don't use __builtin_dynamic_object_size(), I think the runtime
bounds checking improvements are easily worth the little bit of effort
to add the attribute in various locations and review the code. I think
it will improve things for buildfarm animals using ASan due to expanded
coverage.

I took a closer look at this. There are several problems with the
proposed patches.

1) In C++, both gcc and clang have __has_attribute(counted_by) return 1
(true), but the compiler actually rejects the attribute with a warning.
This is not immediately evident in your patch, but it would show up
under cpluspluscheck and whenever we extend this attribute to header
files that happen to get pulled in by C++ source files.
(access/tupdesc.h is an obvious candidate.) Therefore, there needs to be
some #ifndef __cplusplus somewhere.

I would love to understand the rationale for returning 1 when the
compiler will just throw a warning anyway. Fixed.

2) gcc 15 and clang 18 accept the counted_by attribute only for flexible
array members, not for pointers members. (Using it on a pointer causes
an error.) If you want to apply this to pointer members, as your patch
does in buffile.c, you'd have to write a configure test. Or else
restrict it to flexible array members for now.

I like the idea of restricting it to flexible array members for now.
It'll make for an easier review. Maybe in a subsequent patch we can
raise the minimum compiler versions of using pg_attribute_counted_by()
to GCC 16 and Clang 21.

3) The counted_by attribute requires that, when extending the counted
array, the count field is increased before writing into the new element
at the end. The code dealing with struct BufFile currently doesn't do
that, and so your change in buffile.c fails under -fsanitize=bounds:

../src/backend/storage/file/buffile.c:919:3: runtime error: index 1 out
of bounds for type 'File * __counted_by(numFiles)' (aka 'int *')

(Reproduce with meson configure -Db_sanitize=bounds and meson test ...
--suite regress.)

The code needs to be carefully analyzed and adjusted to fix this. (The
code for the tuplesort.c change appears to be ok.)

Good catch. In the upcoming changes, I ran test suites with
-fsanitize=bounds, and found one place that needed a fix. Note that
changes to buffile.c are not currently in scope for this patchset since
it wasn't a flexible array member.

4) Although the compilers are flexible with the placement, the most
correct placement of the attribute is at the beginning of the
declaration, like

pg_attribute_counted_by(nTapes) TapeShare tapes[FLEXIBLE_ARRAY_MEMBER];

(Note that the gcc documentation effectively writes it this way.)

The second patch uses postfix notation, but subsequent patches enable
support for prefix notation. I'll let you be the judge of whether to
commit prefix or postfix. Commits 3 & 4 are genuine improvements, though
they do also enable prefix support.

Additionally, with this arrangement, we could also make use of the MSVC
_Field_size_ annotation.

This is good motivation.

5) Minor: The counted_by attribute only takes a single argument, so the
use of __VA_ARGS__ seems excessive.

I think I just blindly copied surrounding macro code and forgot to
change it. Fixed in this new version.

6) Minor: Awkward wording in comment: "This provides the compiler to
improve ..." -> "enables the compiler ..."?

Fixed.

Suggestion:
- Add C++ guard. (Maybe add annotation in access/tupdesc.h to test.)
- Skip use of the attribute on pointer members for now.
- Make sure cpluspluscheck and -fsanitize=bounds pass.
- Consider the cosmetic adjustments mentioned.

Thanks for the review.

I tested it locally, and all tests passed.

I found some places that could use the new pg_attribute_counted_by() attribute
e.g., in heapam_xlog.h, xact.h, etc. Are those intentional omissions?

Was this an oversight, or is pg_attribute_counted_by not needed here?

Haven't checked everywhere yet. If it is oversight, I'll check it later.

I originally had these as well, but my understanding is that these
structures are populated from the filesystem, so in the event of data
corruption, the count field may no longer be accurate and could cause
a runtime failure similar to what Peter mentioned above in his review.
It's possible that I am being too cautious though. Let me know what you
think. I should have brought this up in my last email, but now is as
good a time as any to discuss!

Thanks for the clear explanation.

I agree with you on the cautious approach — especially for structures that
may be populated from the filesystem, or where the count field does not
strictly represent the allocated/accessible length of the flexible array
member.

While reviewing remaining FAMs that do not yet use pg_attribute_counted_by(),
two cases looked like solid candidates:

1. BackgroundWorkerArray in bgworker.c

typedef struct BackgroundWorkerArray
{
int total_slots;
uint32 parallel_register_count;
uint32 parallel_terminate_count;
BackgroundWorkerSlot slot[FLEXIBLE_ARRAY_MEMBER];
} BackgroundWorkerArray;

The total_slots is set once at shared-memory initialization (based on
max_worker_processes) and never changes afterwards. All accesses are of the
form slot[i] with i < total_slots. The invariant is therefore trivial to
maintain.

2. vbits in contrib/pg_visibility/pg_visibility.c

typedef struct vbits
{
BlockNumber next;
BlockNumber count;
uint8 bits[FLEXIBLE_ARRAY_MEMBER];
} vbits;

The structure is allocated and filled in one place (collect_visibility_data),
with count equal to the number of blocks. Subsequent use is strictly read-only
and always checks next < count before indexing into bits[].

Both are purely internal, the count field matches the actual array bound, and
the update order is safe. I think it would be worthwhile to annotate them.

Thoughts?

Great catches. I have included those in the next version of commits.

--
Tristan Partin
PostgreSQL Contributors Team
AWS (https://aws.amazon.com)

Ok, all the infrastructure is in place now. I'm working through the
attribute additions now.

One thing to think about is that adding these attributes to server
header files would also affect extensions. I think we might want to do
some research about that. Like, is the normal API to fill in the
structure yourself, or are there wrapper functions that are mandatory to
use? I'm going to park that for the moment and work through the
attribute additions in .c files first.

On Mon Oct 5, 2026 at 7:52 PM UTC, Peter Eisentraut wrote:

Ok, all the infrastructure is in place now. I'm working through the
attribute additions now.

One thing to think about is that adding these attributes to server
header files would also affect extensions. I think we might want to do
some research about that. Like, is the normal API to fill in the
structure yourself, or are there wrapper functions that are mandatory to
use? I'm going to park that for the moment and work through the
attribute additions in .c files first.

I have to think that extensions testing with
-fsanitize=bounds/_FORTIFY_SOURCE=3 is pretty rare. In any case, I did
some analysis (grepping and reading) on the list of structs exposed in
header files against 9 popular extensions (not in contrib), and I didn't
find any potential issues.

List of extensions that I looked at:
- PostGIS
- Citus
- TimescaleDB
- pgvector
- pg_cron
- pg_hint_plan
- pgaudit
- pg_repack
- pglogical

It seems like core does a good job of providing helper functions/macros
for creating objects of variable size, and downstream extensions do
a good job of making use of them.

If you have ideas that you would like me to try to help make you more
confident in committing, let me know.

Attached is a list of structs, in header files, that I added the
counted_by attribute to.

--
Tristan Partin
PostgreSQL Contributors Team
AWS (https://aws.amazon.com)