This is an automated email from the ASF dual-hosted git repository. avamingli pushed a commit to branch main in repository https://gitbox.apache.org/repos/asf/cloudberry.git
commit 96ff7a764475935e821ca9c73fa4f7f497467a0e Author: Zhang Hao <[email protected]> AuthorDate: Wed Mar 27 15:05:48 2024 +0800 Fix relptr's encoding of the base address. (#17255) Backport from upstream https://github.com/postgres/postgres/commit/7201cd18627afc64850537806da7f22150d1a83b Previously, we encoded both NULL and the first byte at the base address as 0. That confusion led to the assertion in commit e07d4ddc, which failed when min_dynamic_shared_memory was used. Give them distinct encodings, by switching to 1-based offsets for non-NULL pointers. Also improve macro hygiene in passing (missing/misplaced parentheses), and remove open-coded access to the raw offset value from freepage.c/h. Although e07d4ddc was back-patched to 10, the only code that actually makes use of relptr at the base address arrived in 84b1c63a, so no need to back-patch further than 14 for now. Reported-by: Justin Pryzby <[email protected]> Reviewed-by: Robert Haas <[email protected]> Discussion: https://postgr.es/m/20220519193839.GT19626%40telsasoft.com (cherry picked from commit 7201cd18627afc64850537806da7f22150d1a83b) Co-authored-by: Thomas Munro <[email protected]> --- src/backend/utils/mmgr/freepage.c | 6 +++--- src/include/utils/freepage.h | 4 ++-- src/include/utils/relptr.h | 15 +++++++++------ 3 files changed, 14 insertions(+), 11 deletions(-) diff --git a/src/backend/utils/mmgr/freepage.c b/src/backend/utils/mmgr/freepage.c index e4ee1aab97..4208317de8 100644 --- a/src/backend/utils/mmgr/freepage.c +++ b/src/backend/utils/mmgr/freepage.c @@ -434,7 +434,7 @@ FreePageManagerDump(FreePageManager *fpm) /* Dump general stuff. */ appendStringInfo(&buf, "metadata: self %zu max contiguous pages = %zu\n", - fpm->self.relptr_off, fpm->contiguous_pages); + relptr_offset(fpm->self), fpm->contiguous_pages); /* Dump btree. */ if (fpm->btree_depth > 0) @@ -1269,7 +1269,7 @@ FreePageManagerDumpBtree(FreePageManager *fpm, FreePageBtree *btp, if (btp->hdr.magic == FREE_PAGE_INTERNAL_MAGIC) appendStringInfo(buf, " %zu->%zu", btp->u.internal_key[index].first_page, - btp->u.internal_key[index].child.relptr_off / FPM_PAGE_SIZE); + relptr_offset(btp->u.internal_key[index].child) / FPM_PAGE_SIZE); else appendStringInfo(buf, " %zu(%zu)", btp->u.leaf_key[index].first_page, @@ -1859,7 +1859,7 @@ FreePagePopSpanLeader(FreePageManager *fpm, Size pageno) { Size f = Min(span->npages, FPM_NUM_FREELISTS) - 1; - Assert(fpm->freelist[f].relptr_off == pageno * FPM_PAGE_SIZE); + Assert(relptr_offset(fpm->freelist[f]) == pageno * FPM_PAGE_SIZE); relptr_copy(fpm->freelist[f], span->next); } } diff --git a/src/include/utils/freepage.h b/src/include/utils/freepage.h index 59f50ad30c..b02ca4529b 100644 --- a/src/include/utils/freepage.h +++ b/src/include/utils/freepage.h @@ -78,11 +78,11 @@ struct FreePageManager #define fpm_pointer_is_page_aligned(base, ptr) \ (((Size) (((char *) (ptr)) - (base))) % FPM_PAGE_SIZE == 0) #define fpm_relptr_is_page_aligned(base, relptr) \ - ((relptr).relptr_off % FPM_PAGE_SIZE == 0) + (relptr_offset(relptr) % FPM_PAGE_SIZE == 0) /* Macro to find base address of the segment containing a FreePageManager. */ #define fpm_segment_base(fpm) \ - (((char *) fpm) - fpm->self.relptr_off) + (((char *) fpm) - relptr_offset(fpm->self)) /* Macro to access a FreePageManager's largest consecutive run of pages. */ #define fpm_largest(fpm) \ diff --git a/src/include/utils/relptr.h b/src/include/utils/relptr.h index e71c7d0bdc..2ed4b5187d 100644 --- a/src/include/utils/relptr.h +++ b/src/include/utils/relptr.h @@ -42,7 +42,7 @@ #define relptr_access(base, rp) \ (AssertVariableIsOfTypeMacro(base, char *), \ (__typeof__((rp).relptr_type)) ((rp).relptr_off == 0 ? NULL : \ - (base + (rp).relptr_off))) + (base) + (rp).relptr_off - 1)) #else /* * If we don't have __builtin_types_compatible_p, assume we might not have @@ -50,12 +50,15 @@ */ #define relptr_access(base, rp) \ (AssertVariableIsOfTypeMacro(base, char *), \ - (void *) ((rp).relptr_off == 0 ? NULL : (base + (rp).relptr_off))) + (void *) ((rp).relptr_off == 0 ? NULL : (base) + (rp).relptr_off - 1)) #endif #define relptr_is_null(rp) \ ((rp).relptr_off == 0) +#define relptr_offset(rp) \ + ((rp).relptr_off - 1) + /* We use this inline to avoid double eval of "val" in relptr_store */ static inline Size relptr_store_eval(char *base, char *val) @@ -64,8 +67,8 @@ relptr_store_eval(char *base, char *val) return 0; else { - Assert(val > base); - return val - base; + Assert(val >= base); + return val - base + 1; } } @@ -73,7 +76,7 @@ relptr_store_eval(char *base, char *val) #define relptr_store(base, rp, val) \ (AssertVariableIsOfTypeMacro(base, char *), \ AssertVariableIsOfTypeMacro(val, __typeof__((rp).relptr_type)), \ - (rp).relptr_off = relptr_store_eval(base, (char *) (val))) + (rp).relptr_off = relptr_store_eval((base), (char *) (val))) #else /* * If we don't have __builtin_types_compatible_p, assume we might not have @@ -81,7 +84,7 @@ relptr_store_eval(char *base, char *val) */ #define relptr_store(base, rp, val) \ (AssertVariableIsOfTypeMacro(base, char *), \ - (rp).relptr_off = relptr_store_eval(base, (char *) (val))) + (rp).relptr_off = relptr_store_eval((base), (char *) (val))) #endif #define relptr_copy(rp1, rp2) \ --------------------------------------------------------------------- To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
