Location via proxy:   [ UP ]  
[Report a bug]   [Manage cookies]                
Skip to content

Commit 7201cd1

Browse files
committed
Fix relptr's encoding of the base address.
Previously, we encoded both NULL and the first byte at the base address as 0. That confusion led to the assertion in commit e07d4dd, 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 e07d4dd was back-patched to 10, the only code that actually makes use of relptr at the base address arrived in 84b1c63, so no need to back-patch further than 14 for now. Reported-by: Justin Pryzby <pryzby@telsasoft.com> Reviewed-by: Robert Haas <robertmhaas@gmail.com> Discussion: https://postgr.es/m/20220519193839.GT19626%40telsasoft.com
1 parent ebc584e commit 7201cd1

File tree

3 files changed

+14
-11
lines changed

3 files changed

+14
-11
lines changed

src/backend/utils/mmgr/freepage.c

+3-3
Original file line numberDiff line numberDiff line change
@@ -434,7 +434,7 @@ FreePageManagerDump(FreePageManager *fpm)
434434

435435
/* Dump general stuff. */
436436
appendStringInfo(&buf, "metadata: self %zu max contiguous pages = %zu\n",
437-
fpm->self.relptr_off, fpm->contiguous_pages);
437+
relptr_offset(fpm->self), fpm->contiguous_pages);
438438

439439
/* Dump btree. */
440440
if (fpm->btree_depth > 0)
@@ -1269,7 +1269,7 @@ FreePageManagerDumpBtree(FreePageManager *fpm, FreePageBtree *btp,
12691269
if (btp->hdr.magic == FREE_PAGE_INTERNAL_MAGIC)
12701270
appendStringInfo(buf, " %zu->%zu",
12711271
btp->u.internal_key[index].first_page,
1272-
btp->u.internal_key[index].child.relptr_off / FPM_PAGE_SIZE);
1272+
relptr_offset(btp->u.internal_key[index].child) / FPM_PAGE_SIZE);
12731273
else
12741274
appendStringInfo(buf, " %zu(%zu)",
12751275
btp->u.leaf_key[index].first_page,
@@ -1859,7 +1859,7 @@ FreePagePopSpanLeader(FreePageManager *fpm, Size pageno)
18591859
{
18601860
Size f = Min(span->npages, FPM_NUM_FREELISTS) - 1;
18611861

1862-
Assert(fpm->freelist[f].relptr_off == pageno * FPM_PAGE_SIZE);
1862+
Assert(relptr_offset(fpm->freelist[f]) == pageno * FPM_PAGE_SIZE);
18631863
relptr_copy(fpm->freelist[f], span->next);
18641864
}
18651865
}

src/include/utils/freepage.h

+2-2
Original file line numberDiff line numberDiff line change
@@ -78,11 +78,11 @@ struct FreePageManager
7878
#define fpm_pointer_is_page_aligned(base, ptr) \
7979
(((Size) (((char *) (ptr)) - (base))) % FPM_PAGE_SIZE == 0)
8080
#define fpm_relptr_is_page_aligned(base, relptr) \
81-
((relptr).relptr_off % FPM_PAGE_SIZE == 0)
81+
(relptr_offset(relptr) % FPM_PAGE_SIZE == 0)
8282

8383
/* Macro to find base address of the segment containing a FreePageManager. */
8484
#define fpm_segment_base(fpm) \
85-
(((char *) fpm) - fpm->self.relptr_off)
85+
(((char *) fpm) - relptr_offset(fpm->self))
8686

8787
/* Macro to access a FreePageManager's largest consecutive run of pages. */
8888
#define fpm_largest(fpm) \

src/include/utils/relptr.h

+9-6
Original file line numberDiff line numberDiff line change
@@ -42,20 +42,23 @@
4242
#define relptr_access(base, rp) \
4343
(AssertVariableIsOfTypeMacro(base, char *), \
4444
(__typeof__((rp).relptr_type)) ((rp).relptr_off == 0 ? NULL : \
45-
(base + (rp).relptr_off)))
45+
(base) + (rp).relptr_off - 1))
4646
#else
4747
/*
4848
* If we don't have __builtin_types_compatible_p, assume we might not have
4949
* __typeof__ either.
5050
*/
5151
#define relptr_access(base, rp) \
5252
(AssertVariableIsOfTypeMacro(base, char *), \
53-
(void *) ((rp).relptr_off == 0 ? NULL : (base + (rp).relptr_off)))
53+
(void *) ((rp).relptr_off == 0 ? NULL : (base) + (rp).relptr_off - 1))
5454
#endif
5555

5656
#define relptr_is_null(rp) \
5757
((rp).relptr_off == 0)
5858

59+
#define relptr_offset(rp) \
60+
((rp).relptr_off - 1)
61+
5962
/* We use this inline to avoid double eval of "val" in relptr_store */
6063
static inline Size
6164
relptr_store_eval(char *base, char *val)
@@ -64,24 +67,24 @@ relptr_store_eval(char *base, char *val)
6467
return 0;
6568
else
6669
{
67-
Assert(val > base);
68-
return val - base;
70+
Assert(val >= base);
71+
return val - base + 1;
6972
}
7073
}
7174

7275
#ifdef HAVE__BUILTIN_TYPES_COMPATIBLE_P
7376
#define relptr_store(base, rp, val) \
7477
(AssertVariableIsOfTypeMacro(base, char *), \
7578
AssertVariableIsOfTypeMacro(val, __typeof__((rp).relptr_type)), \
76-
(rp).relptr_off = relptr_store_eval(base, (char *) (val)))
79+
(rp).relptr_off = relptr_store_eval((base), (char *) (val)))
7780
#else
7881
/*
7982
* If we don't have __builtin_types_compatible_p, assume we might not have
8083
* __typeof__ either.
8184
*/
8285
#define relptr_store(base, rp, val) \
8386
(AssertVariableIsOfTypeMacro(base, char *), \
84-
(rp).relptr_off = relptr_store_eval(base, (char *) (val)))
87+
(rp).relptr_off = relptr_store_eval((base), (char *) (val)))
8588
#endif
8689

8790
#define relptr_copy(rp1, rp2) \

0 commit comments

Comments
 (0)