Fix sanitizer errors

[antamel]

Hello!

In the build with address and undefined behavior sanitizers, my colleague Andrew Bille [email protected] found several errors in the current master branch.

The build was made with options like that:

CC=gcc-11
CXX=g++-11
CPPFLAGS="-ggdb -O0 -g3 -fno-omit-frame-pointer -fsanitize=address -fsanitize=undefined -fsanitize=alignment -fno-sanitize-recover=all -fno-sanitize=nonnull-attribute -fstack-protector"
LDFLAGS='-fsanitize=address -fsanitize=undefined -static-libasan -static-libubsan'
ASAN_OPTIONS=detect_leaks=0:abort_on_error=1:halt_on_error=1:disable_coredump=0:strict_string_checks=1:check_initialization_order=1:strict_init_order=1:detect_odr_violation=0

There are several "load of misaligned address" sanitizer errors in the log during installcheck tests with PG at REL_15_STABLE (6db384f2f9). See master-at-a04fd69b.zip attached. (Other errors are masked by this, about them below.) Some of these errors are not related to age, is a bug of the sanitizer itself (see https://gcc.gnu.org/bugzilla/show_bug.cgi?id=111382). The other ones i tried to fix with the Fix sanitizer aligment errors with memcpy. commit.

After that "misaligned" errors disappeared and other errors became visible. Please see postmaster.log in the after-first-fix-at-de3633d2.zip. The second commit Fix sanitizer "invalid values for type '_Bool' " errors with palloc0. fixes runtime errors: "load of value xxx, which is not a valid value for type '_Bool'" while the third Fix sanitizer heap-buffer-overflow errors during csv files parsing. one fixes "heap-buffer-overflow on address.." one during csv file parsing.

On applying the first three patches, another error heap-buffer-overflow became visible. See log in before-last-fix-at-2840ce1f.zip. The last commit Save the metadata representing by the extensible node with the null t… fixes it add all tests began to pass without errors with REL_15_STABLE and REL_14_STABLE branches.

Would be happy for any comments and concerns.

With kind regards,

-- Anton A. Melnikov Postgres Professional: http://www.postgrespro.com The Russian Postgres Company

[rafsun42]

+1 for the palloc0 changes as PostgreSQL recommends it as well:

https://www.postgresql.org/docs/current/xfunc-c.html

Always zero the bytes of your structures using memset (or allocate them with palloc0 in the first place). Even if you assign to each field of your structure, there might be alignment padding (holes in the structure) that contain garbage values. Without this, it's difficult to support hash indexes or hash joins, as you must pick out only the significant bits of your data structure to compute a hash. The planner also sometimes relies on comparing constants via bitwise equality, so you can get undesirable planning results if logically-equivalent values aren't bitwise equal.

[github-actions[bot]]

This PR is stale because it has been open 45 days with no activity. Remove "Abondoned" label or comment or this will be closed in 7 days.

[jrgemignani]

@rafsun42 As the original creator just posted this PR and has yet to respond since, should we break this into 2 PRs, one for memcpy, one for palloc0, attribute this PR and implement ourselves?

[rafsun42]

@jrgemignani Sounds good. I am fine with doing either of palloc0 or memcpy.

[github-actions[bot]]

This PR is stale because it has been open 60 days with no activity. Remove "Abondoned" label or comment or this will be closed in 14 days.

[github-actions[bot]]

This PR was closed because it has been stalled for further 14 days with no activity