Movatterモバイル変換

[0]ホーム
URL:

Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

 Sign up
Appearance settings

Commit0f2458f

Browse files
committed
Improve valgrind logic in aset.c, and fix multiple issues in generation.c.
Revise aset.c so that all the "private" fields of chunk headers aremarked NOACCESS when outside the module, improving on the previouscoding which protected only requested_size. Fix a couple of cornercase bugs, such as failing to re-protect the header during a failureexit from AllocSetRealloc, and wrong padding-size calculation for anoversize allocation request.Apply the same design to generation.c, and also fix several bugs thereinthat I found by dint of hacking the code to use generation.c as thestandard allocator and then running the core regression tests with it.Notably, we have to track the actual size of each block, else thewipe_mem call in GenerationReset clears the wrong amount of memory foran oversize-chunk block; and GenerationCheck needs a way of identifyingfreed chunks that isn't fooled by palloc(0). I chose to fix the latterby resetting the context pointer to NULL in a freed chunk, roughly likewhat happens in a freed aset.c chunk.Discussion:https://postgr.es/m/E1eHa4J-0006hI-Q8@gemulon.postgresql.org
1 parentf65d21b commit0f2458f

File tree

2 files changed

+192
-102
lines changed

2 files changed

+192
-102
lines changed

‎src/backend/utils/mmgr/aset.c‎

Lines changed: 83 additions & 38 deletions
Original file line numberDiff line numberDiff line change
@@ -190,6 +190,14 @@ typedef struct AllocChunkData
190190
/* there must not be any padding to reach a MAXALIGN boundary here! */
191191
}AllocChunkData;
192192

193+
/*
194+
* Only the "aset" field should be accessed outside this module.
195+
* We keep the rest of an allocated chunk's header marked NOACCESS when using
196+
* valgrind. But note that chunk headers that are in a freelist are kept
197+
* accessible, for simplicity.
198+
*/
199+
#defineALLOCCHUNK_PRIVATE_LENoffsetof(AllocChunkData, aset)
200+
193201
/*
194202
* AllocPointerIsValid
195203
*True iff pointer is valid allocation pointer.
@@ -572,6 +580,10 @@ AllocSetDelete(MemoryContext context)
572580
* No request may exceed:
573581
*MAXALIGN_DOWN(SIZE_MAX) - ALLOC_BLOCKHDRSZ - ALLOC_CHUNKHDRSZ
574582
* All callers use a much-lower limit.
583+
*
584+
* Note: when using valgrind, it doesn't matter how the returned allocation
585+
* is marked, as mcxt.c will set it to UNDEFINED. In some paths we will
586+
* return space that is marked NOACCESS - AllocSetRealloc has to beware!
575587
*/
576588
staticvoid*
577589
AllocSetAlloc(MemoryContextcontext,Sizesize)
@@ -603,7 +615,6 @@ AllocSetAlloc(MemoryContext context, Size size)
603615
chunk->aset=set;
604616
chunk->size=chunk_size;
605617
#ifdefMEMORY_CONTEXT_CHECKING
606-
/* Valgrind: Will be made NOACCESS below. */
607618
chunk->requested_size=size;
608619
/* set mark to catch clobber of "unused" space */
609620
if (size<chunk_size)
@@ -635,14 +646,12 @@ AllocSetAlloc(MemoryContext context, Size size)
635646

636647
AllocAllocInfo(set,chunk);
637648

638-
/*
639-
* Chunk's metadata fields remain DEFINED. The requested allocation
640-
* itself can be NOACCESS or UNDEFINED; our caller will soon make it
641-
* UNDEFINED. Make extra space at the end of the chunk, if any,
642-
* NOACCESS.
643-
*/
644-
VALGRIND_MAKE_MEM_NOACCESS((char*)chunk+ALLOC_CHUNKHDRSZ,
645-
chunk_size-ALLOC_CHUNKHDRSZ);
649+
/* Ensure any padding bytes are marked NOACCESS. */
650+
VALGRIND_MAKE_MEM_NOACCESS((char*)AllocChunkGetPointer(chunk)+size,
651+
chunk_size-size);
652+
653+
/* Disallow external access to private part of chunk header. */
654+
VALGRIND_MAKE_MEM_NOACCESS(chunk,ALLOCCHUNK_PRIVATE_LEN);
646655

647656
returnAllocChunkGetPointer(chunk);
648657
}
@@ -664,10 +673,7 @@ AllocSetAlloc(MemoryContext context, Size size)
664673
chunk->aset= (void*)set;
665674

666675
#ifdefMEMORY_CONTEXT_CHECKING
667-
/* Valgrind: Free list requested_size should be DEFINED. */
668676
chunk->requested_size=size;
669-
VALGRIND_MAKE_MEM_NOACCESS(&chunk->requested_size,
670-
sizeof(chunk->requested_size));
671677
/* set mark to catch clobber of "unused" space */
672678
if (size<chunk->size)
673679
set_sentinel(AllocChunkGetPointer(chunk),size);
@@ -678,6 +684,14 @@ AllocSetAlloc(MemoryContext context, Size size)
678684
#endif
679685

680686
AllocAllocInfo(set,chunk);
687+
688+
/* Ensure any padding bytes are marked NOACCESS. */
689+
VALGRIND_MAKE_MEM_NOACCESS((char*)AllocChunkGetPointer(chunk)+size,
690+
chunk->size-size);
691+
692+
/* Disallow external access to private part of chunk header. */
693+
VALGRIND_MAKE_MEM_NOACCESS(chunk,ALLOCCHUNK_PRIVATE_LEN);
694+
681695
returnAllocChunkGetPointer(chunk);
682696
}
683697

@@ -831,8 +845,6 @@ AllocSetAlloc(MemoryContext context, Size size)
831845
chunk->size=chunk_size;
832846
#ifdefMEMORY_CONTEXT_CHECKING
833847
chunk->requested_size=size;
834-
VALGRIND_MAKE_MEM_NOACCESS(&chunk->requested_size,
835-
sizeof(chunk->requested_size));
836848
/* set mark to catch clobber of "unused" space */
837849
if (size<chunk->size)
838850
set_sentinel(AllocChunkGetPointer(chunk),size);
@@ -843,6 +855,14 @@ AllocSetAlloc(MemoryContext context, Size size)
843855
#endif
844856

845857
AllocAllocInfo(set,chunk);
858+
859+
/* Ensure any padding bytes are marked NOACCESS. */
860+
VALGRIND_MAKE_MEM_NOACCESS((char*)AllocChunkGetPointer(chunk)+size,
861+
chunk_size-size);
862+
863+
/* Disallow external access to private part of chunk header. */
864+
VALGRIND_MAKE_MEM_NOACCESS(chunk,ALLOCCHUNK_PRIVATE_LEN);
865+
846866
returnAllocChunkGetPointer(chunk);
847867
}
848868

@@ -856,11 +876,12 @@ AllocSetFree(MemoryContext context, void *pointer)
856876
AllocSetset= (AllocSet)context;
857877
AllocChunkchunk=AllocPointerGetChunk(pointer);
858878

879+
/* Allow access to private part of chunk header. */
880+
VALGRIND_MAKE_MEM_DEFINED(chunk,ALLOCCHUNK_PRIVATE_LEN);
881+
859882
AllocFreeInfo(set,chunk);
860883

861884
#ifdefMEMORY_CONTEXT_CHECKING
862-
VALGRIND_MAKE_MEM_DEFINED(&chunk->requested_size,
863-
sizeof(chunk->requested_size));
864885
/* Test for someone scribbling on unused space in chunk */
865886
if (chunk->requested_size<chunk->size)
866887
if (!sentinel_ok(pointer,chunk->requested_size))
@@ -935,11 +956,14 @@ AllocSetRealloc(MemoryContext context, void *pointer, Size size)
935956
{
936957
AllocSetset= (AllocSet)context;
937958
AllocChunkchunk=AllocPointerGetChunk(pointer);
938-
Sizeoldsize=chunk->size;
959+
Sizeoldsize;
960+
961+
/* Allow access to private part of chunk header. */
962+
VALGRIND_MAKE_MEM_DEFINED(chunk,ALLOCCHUNK_PRIVATE_LEN);
963+
964+
oldsize=chunk->size;
939965

940966
#ifdefMEMORY_CONTEXT_CHECKING
941-
VALGRIND_MAKE_MEM_DEFINED(&chunk->requested_size,
942-
sizeof(chunk->requested_size));
943967
/* Test for someone scribbling on unused space in chunk */
944968
if (chunk->requested_size<oldsize)
945969
if (!sentinel_ok(pointer,chunk->requested_size))
@@ -965,8 +989,6 @@ AllocSetRealloc(MemoryContext context, void *pointer, Size size)
965989
#endif
966990

967991
chunk->requested_size=size;
968-
VALGRIND_MAKE_MEM_NOACCESS(&chunk->requested_size,
969-
sizeof(chunk->requested_size));
970992

971993
/*
972994
* If this is an increase, mark any newly-available part UNDEFINED.
@@ -993,6 +1015,9 @@ AllocSetRealloc(MemoryContext context, void *pointer, Size size)
9931015
VALGRIND_MAKE_MEM_DEFINED(pointer,size);
9941016
#endif
9951017

1018+
/* Disallow external access to private part of chunk header. */
1019+
VALGRIND_MAKE_MEM_NOACCESS(chunk,ALLOCCHUNK_PRIVATE_LEN);
1020+
9961021
returnpointer;
9971022
}
9981023

@@ -1023,7 +1048,11 @@ AllocSetRealloc(MemoryContext context, void *pointer, Size size)
10231048
blksize=chksize+ALLOC_BLOCKHDRSZ+ALLOC_CHUNKHDRSZ;
10241049
block= (AllocBlock)realloc(block,blksize);
10251050
if (block==NULL)
1051+
{
1052+
/* Disallow external access to private part of chunk header. */
1053+
VALGRIND_MAKE_MEM_NOACCESS(chunk,ALLOCCHUNK_PRIVATE_LEN);
10261054
returnNULL;
1055+
}
10271056
block->freeptr=block->endptr= ((char*)block)+blksize;
10281057

10291058
/* Update pointers since block has likely been moved */
@@ -1053,8 +1082,6 @@ AllocSetRealloc(MemoryContext context, void *pointer, Size size)
10531082
oldsize-chunk->requested_size);
10541083

10551084
chunk->requested_size=size;
1056-
VALGRIND_MAKE_MEM_NOACCESS(&chunk->requested_size,
1057-
sizeof(chunk->requested_size));
10581085

10591086
/* set mark to catch clobber of "unused" space */
10601087
if (size<chunk->size)
@@ -1069,9 +1096,12 @@ AllocSetRealloc(MemoryContext context, void *pointer, Size size)
10691096
VALGRIND_MAKE_MEM_DEFINED(pointer,oldsize);
10701097
#endif
10711098

1072-
/*Make anytrailing alignment padding NOACCESS. */
1099+
/*Ensure anypadding bytes are marked NOACCESS. */
10731100
VALGRIND_MAKE_MEM_NOACCESS((char*)pointer+size,chksize-size);
10741101

1102+
/* Disallow external access to private part of chunk header. */
1103+
VALGRIND_MAKE_MEM_NOACCESS(chunk,ALLOCCHUNK_PRIVATE_LEN);
1104+
10751105
returnpointer;
10761106
}
10771107
else
@@ -1094,14 +1124,19 @@ AllocSetRealloc(MemoryContext context, void *pointer, Size size)
10941124

10951125
/* leave immediately if request was not completed */
10961126
if (newPointer==NULL)
1127+
{
1128+
/* Disallow external access to private part of chunk header. */
1129+
VALGRIND_MAKE_MEM_NOACCESS(chunk,ALLOCCHUNK_PRIVATE_LEN);
10971130
returnNULL;
1131+
}
10981132

10991133
/*
1100-
* AllocSetAlloc() just made the region NOACCESS. Change it to
1101-
* UNDEFINED for the moment; memcpy() will then transfer definedness
1102-
* from the old allocation to the new. If we know the old allocation,
1103-
* copy just that much. Otherwise, make the entire old chunk defined
1104-
* to avoid errors as we copy the currently-NOACCESS trailing bytes.
1134+
* AllocSetAlloc() may have returned a region that is still NOACCESS.
1135+
* Change it to UNDEFINED for the moment; memcpy() will then transfer
1136+
* definedness from the old allocation to the new. If we know the old
1137+
* allocation, copy just that much. Otherwise, make the entire old
1138+
* chunk defined to avoid errors as we copy the currently-NOACCESS
1139+
* trailing bytes.
11051140
*/
11061141
VALGRIND_MAKE_MEM_UNDEFINED(newPointer,size);
11071142
#ifdefMEMORY_CONTEXT_CHECKING
@@ -1129,8 +1164,12 @@ static Size
11291164
AllocSetGetChunkSpace(MemoryContextcontext,void*pointer)
11301165
{
11311166
AllocChunkchunk=AllocPointerGetChunk(pointer);
1167+
Sizeresult;
11321168

1133-
returnchunk->size+ALLOC_CHUNKHDRSZ;
1169+
VALGRIND_MAKE_MEM_DEFINED(chunk,ALLOCCHUNK_PRIVATE_LEN);
1170+
result=chunk->size+ALLOC_CHUNKHDRSZ;
1171+
VALGRIND_MAKE_MEM_NOACCESS(chunk,ALLOCCHUNK_PRIVATE_LEN);
1172+
returnresult;
11341173
}
11351174

11361175
/*
@@ -1267,13 +1306,11 @@ AllocSetCheck(MemoryContext context)
12671306
Sizechsize,
12681307
dsize;
12691308

1309+
/* Allow access to private part of chunk header. */
1310+
VALGRIND_MAKE_MEM_DEFINED(chunk,ALLOCCHUNK_PRIVATE_LEN);
1311+
12701312
chsize=chunk->size;/* aligned chunk size */
1271-
VALGRIND_MAKE_MEM_DEFINED(&chunk->requested_size,
1272-
sizeof(chunk->requested_size));
12731313
dsize=chunk->requested_size;/* real data */
1274-
if (dsize>0)/* not on a free list */
1275-
VALGRIND_MAKE_MEM_NOACCESS(&chunk->requested_size,
1276-
sizeof(chunk->requested_size));
12771314

12781315
/*
12791316
* Check chunk size
@@ -1294,20 +1331,28 @@ AllocSetCheck(MemoryContext context)
12941331
/*
12951332
* If chunk is allocated, check for correct aset pointer. (If it's
12961333
* free, the aset is the freelist pointer, which we can't check as
1297-
* easily...)
1334+
* easily...) Note this is an incomplete test, since palloc(0)
1335+
* produces an allocated chunk with requested_size == 0.
12981336
*/
12991337
if (dsize>0&&chunk->aset!= (void*)set)
13001338
elog(WARNING,"problem in alloc set %s: bogus aset link in block %p, chunk %p",
13011339
name,block,chunk);
13021340

13031341
/*
1304-
* Check for overwrite of"unallocated" space in chunk
1342+
* Check for overwrite ofpadding space inan allocatedchunk.
13051343
*/
1306-
if (dsize>0&&dsize<chsize&&
1344+
if (chunk->aset== (void*)set&&dsize<chsize&&
13071345
!sentinel_ok(chunk,ALLOC_CHUNKHDRSZ+dsize))
13081346
elog(WARNING,"problem in alloc set %s: detected write past chunk end in block %p, chunk %p",
13091347
name,block,chunk);
13101348

1349+
/*
1350+
* If chunk is allocated, disallow external access to private part
1351+
* of chunk header.
1352+
*/
1353+
if (chunk->aset== (void*)set)
1354+
VALGRIND_MAKE_MEM_NOACCESS(chunk,ALLOCCHUNK_PRIVATE_LEN);
1355+
13111356
blk_data+=chsize;
13121357
nchunks++;
13131358

0 commit comments

Comments
 (0)
[8]ページ先頭
©2009-2025 Movatter.jp