diff --git a/code/arenavm.c b/code/arenavm.c index 19bc8d82a5..50708e9563 100644 --- a/code/arenavm.c +++ b/code/arenavm.c @@ -428,6 +428,10 @@ static Bool vmChunkDestroy(Tree tree, void *closure) { Chunk chunk; VMChunk vmChunk; + Arena arena; + Addr base; + Size size; + VMArena vmArena; AVERT(Tree, tree); AVER(closure == UNUSED_POINTER); @@ -437,8 +441,14 @@ static Bool vmChunkDestroy(Tree tree, void *closure) AVERT(Chunk, chunk); vmChunk = Chunk2VMChunk(chunk); AVERT(VMChunk, vmChunk); + arena = ChunkArena(chunk); + vmArena = MustBeA(VMArena, arena); + base = chunk->base; + size = ChunkSize(chunk); - (void)vmArenaUnmapSpare(ChunkArena(chunk), ChunkSize(chunk), chunk); + (*vmArena->contracted)(arena, base, size); + + (void)vmArenaUnmapSpare(arena, size, chunk); SparseArrayFinish(&vmChunk->pages); @@ -778,6 +788,7 @@ static void VMArenaDestroy(Arena arena) * */ arena->primary = NULL; TreeTraverseAndDelete(&arena->chunkTree, vmChunkDestroy, UNUSED_POINTER); + AVER(arena->chunkTree == TreeEMPTY); /* Must wait until the chunks are destroyed, since vmChunkDestroy calls vmArenaUnmapSpare which uses the spare land. */ @@ -1223,7 +1234,6 @@ static Bool vmChunkCompact(Tree tree, void *closure) { Chunk chunk; Arena arena = closure; - VMArena vmArena = MustBeA(VMArena, arena); AVERT(Tree, tree); @@ -1232,11 +1242,6 @@ static Bool vmChunkCompact(Tree tree, void *closure) if(chunk != arena->primary && BTIsResRange(chunk->allocTable, 0, chunk->pages)) { - Addr base = chunk->base; - Size size = ChunkSize(chunk); - /* Callback before destroying the chunk, as the arena is (briefly) - invalid afterwards. See job003893. */ - (*vmArena->contracted)(arena, base, size); vmChunkDestroy(tree, UNUSED_POINTER); return TRUE; } else { diff --git a/code/comm.gmk b/code/comm.gmk index a2cd68baab..3a241d84ff 100644 --- a/code/comm.gmk +++ b/code/comm.gmk @@ -267,6 +267,7 @@ TEST_TARGETS=\ btcv \ bttest \ djbench \ + extcon \ finalcv \ finaltest \ forktest \ @@ -487,6 +488,9 @@ $(PFM)/$(VARIETY)/bttest: $(PFM)/$(VARIETY)/bttest.o \ $(PFM)/$(VARIETY)/djbench: $(PFM)/$(VARIETY)/djbench.o \ $(TESTLIBOBJ) $(TESTTHROBJ) +$(PFM)/$(VARIETY)/extcon: $(PFM)/$(VARIETY)/extcon.o \ + $(FMTDYTSTOBJ) $(TESTLIBOBJ) $(PFM)/$(VARIETY)/mps.a + $(PFM)/$(VARIETY)/finalcv: $(PFM)/$(VARIETY)/finalcv.o \ $(FMTDYTSTOBJ) $(TESTLIBOBJ) $(PFM)/$(VARIETY)/mps.a diff --git a/code/commpost.nmk b/code/commpost.nmk index 470db2519b..a94c87b17e 100644 --- a/code/commpost.nmk +++ b/code/commpost.nmk @@ -225,6 +225,9 @@ $(PFM)\$(VARIETY)\cvmicv.exe: $(PFM)\$(VARIETY)\cvmicv.obj \ $(PFM)\$(VARIETY)\djbench.exe: $(PFM)\$(VARIETY)\djbench.obj \ $(TESTLIBOBJ) $(TESTTHROBJ) +$(PFM)\$(VARIETY)\extcon.exe: $(PFM)\$(VARIETY)\extcon.obj \ + $(PFM)\$(VARIETY)\mps.lib $(FMTTESTOBJ) $(TESTLIBOBJ) + $(PFM)\$(VARIETY)\finalcv.exe: $(PFM)\$(VARIETY)\finalcv.obj \ $(PFM)\$(VARIETY)\mps.lib $(FMTTESTOBJ) $(TESTLIBOBJ) diff --git a/code/commpre.nmk b/code/commpre.nmk index 236e08ad2e..4a41ea5641 100644 --- a/code/commpre.nmk +++ b/code/commpre.nmk @@ -73,6 +73,7 @@ TEST_TARGETS=\ btcv.exe \ bttest.exe \ djbench.exe \ + extcon.exe \ finalcv.exe \ finaltest.exe \ fotest.exe \ diff --git a/code/extcon.c b/code/extcon.c new file mode 100644 index 0000000000..4bd04a14b2 --- /dev/null +++ b/code/extcon.c @@ -0,0 +1,306 @@ +/* extcon.c: ARENA EXTENDED AND CONTRACTED CALLBACK TEST + * + * $Id$ + * Copyright (c) 2022-2023 Ravenbrook Limited. See end of file for license. + * + * .overview: This test case allocates a bunch of large objects, of a size + * similar to the size of the arena, to force the arena to extend. It then + * discards the base pointers to those objects, and forces a collection. + * + * .limitations: This test checks that the EXTENDED and CONTRACTED + * callbacks were called at least once, and that they are called the + * same number of times. It does not check that the extensions and + * contractions themselves were performed correctly, nor does it check + * that an appropriate number of extensions and contractions took + * place, nor does it check that they took place at sensible times. + * + * .dylan: This test uses Dylan format objects in common with most + * other tests for convenience and brevity. + */ + +#include "mps.h" +#include "testlib.h" +#include "fmtdy.h" +#include "fmtdytst.h" +#include "mpsavm.h" +#include "mpscamc.h" +#include +#include + +/* Number of test objects to allocate */ +#define N_TESTOBJ 100 +/* The number of slots determines the size of each object */ +#define N_SLOT_TESTOBJ 10000 +/* The initial arena size is requested to be bigger the test object by + this many bytes */ +#define SIZEDIFF 10 + +/* Set alignment to mps_word_ts */ +#define ALIGNMENT sizeof(mps_word_t) + +/* Align size upwards to the next multiple of the word size. */ +#define ALIGN_WORD(size) \ + (((size) + ALIGNMENT - 1) & ~(ALIGNMENT - 1)) + +/* Global objects*/ +static mps_arena_t arena; /* the arena */ +static mps_pool_t obj_pool; /* pool for test objects */ +static mps_ap_t obj_ap; /* allocation point used to allocate objects */ + +/* Count of number of arena contractions and extensions */ +static int n_contract = 0; +static int n_extend = 0; + +/* Callback functions for arena extension and contraction */ +static void arena_extended_cb(mps_arena_t arena_in, mps_addr_t addr, size_t size) +{ + testlib_unused(arena_in); + testlib_unused(addr); + testlib_unused(size); + printf("Arena extended by %"PRIuLONGEST" bytes\n", (ulongest_t)size); + n_extend++; +} + +static void arena_contracted_cb(mps_arena_t arena_in, mps_addr_t addr, size_t size) +{ + testlib_unused(arena_in); + testlib_unused(addr); + testlib_unused(size); + printf("Arena contracted by %"PRIuLONGEST" bytes\n", (ulongest_t)size); + n_contract++; +} + +/* Messages for testbench debugging */ +static void print_messages(void) +{ + mps_message_type_t type; + + while (mps_message_queue_type(&type, arena)) { + mps_message_t message; + + cdie(mps_message_get(&message, arena, type), + "get"); + + switch(type) { + case mps_message_type_gc_start(): + printf("GC start at %"PRIuLONGEST": %s\n", + (ulongest_t)mps_message_clock(arena, message), + mps_message_gc_start_why(arena, message)); + break; + + case mps_message_type_gc(): + printf("GC end at %"PRIuLONGEST" " + "condemned %"PRIuLONGEST" " + "not condemned %"PRIuLONGEST" " + "live %"PRIuLONGEST"\n", + (ulongest_t)mps_message_clock(arena, message), + (ulongest_t)mps_message_gc_condemned_size(arena, message), + (ulongest_t)mps_message_gc_not_condemned_size(arena, message), + (ulongest_t)mps_message_gc_live_size(arena, message)); + break; + + default: + cdie(0, "message type"); + break; + } + + mps_message_discard(arena, message); + } +} + +/* Disabling inlining is necessary (but perhaps not sufficient) if using stack roots. + See comment below with link to GitHub issue*/ +ATTRIBUTE_NOINLINE +static void test_main(void *cold_stack_end) +{ + mps_fmt_t obj_fmt; + mps_thr_t thread; + mps_root_t stack_root, testobj_root; + size_t arena_size, obj_size; + int i; + /* In the original version of extcon this was a stack root, but we + observed unreliable failures to do with registering the cold end + of the stack. See GitHub issue #210 + . For now, we + declare this as a separate root. */ + static mps_word_t testobj[N_TESTOBJ]; + + /* The testobj array must be below (on all current Posix platforms) + the cold end of the stack in order for the MPS to scan it. We + have observed a Heisenbug where GCC will inline test_main into + main and lose this condition if the expression below is removed. + This is a problem we are analysing in GitHub issue #210 + . For now, we + disable this Insist to allow the test to run with a static + testobj array. */ +#if 0 + Insist((void *)&testobj[N_TESTOBJ] <= cold_stack_end); + if ((void *)&testobj[N_TESTOBJ] > cold_stack_end) + printf("Cold stack marker invalid!\n"); + else + printf("Cold stack marker probably valid.\n"); +#endif + + /* Make initial arena size slightly bigger than the test object size to force an extension as early as possible */ + /* See definition of make_dylan_vector() in fmtdytst.c for calculation of vector size */ + obj_size = ALIGN_WORD((N_SLOT_TESTOBJ + 2) * sizeof(mps_word_t)); + arena_size = ALIGN_WORD(obj_size + SIZEDIFF); + + /* Create arena and register callbacks */ + MPS_ARGS_BEGIN(args) { + MPS_ARGS_ADD(args, MPS_KEY_ARENA_SIZE, arena_size); + MPS_ARGS_ADD(args, MPS_KEY_ARENA_EXTENDED, (mps_fun_t)&arena_extended_cb); + MPS_ARGS_ADD(args, MPS_KEY_ARENA_CONTRACTED, (mps_fun_t)&arena_contracted_cb); + die(mps_arena_create_k(&arena, mps_arena_class_vm(), args), "mps_arena_create_k"); + } MPS_ARGS_END(args); + + printf("Initial reservation %"PRIuLONGEST".\n", (ulongest_t)mps_arena_reserved(arena)); + + die(dylan_fmt(&obj_fmt, arena), "dylan_fmt()"); + + /* Create new pool */ + MPS_ARGS_BEGIN(args) { + MPS_ARGS_ADD(args, MPS_KEY_FORMAT, obj_fmt); + die(mps_pool_create_k(&obj_pool, arena, mps_class_amcz(), args), + "mps_pool_create_k"); + } MPS_ARGS_END(args); + + /* Register thread */ + die(mps_thread_reg(&thread, arena), "Thread reg"); + + /* Register stack roots */ + /* Since this testbench is currently not using a stack root, #IF 0 this out */ + testlib_unused(cold_stack_end); + testlib_unused(stack_root); +#if 0 + die(mps_root_create_thread(&stack_root, arena, thread, cold_stack_end), "Create Stack root"); +#endif + + /* Register ambiguous array of object roots. */ + die(mps_root_create_area(&testobj_root, arena, + mps_rank_ambig(), (mps_rm_t)0, + &testobj[0], &testobj[N_TESTOBJ], + mps_scan_area, NULL), + "root_create_area(testobj)"); + + /* Create allocation point */ + die(mps_ap_create_k(&obj_ap, obj_pool, mps_args_none), "Create Allocation point"); + + mps_message_type_enable(arena, mps_message_type_gc_start()); + mps_message_type_enable(arena, mps_message_type_gc()); + + /* Allocate objects and force arena extension */ + for (i = 0; i < N_TESTOBJ; i++) { + + die(make_dylan_vector(&testobj[i], obj_ap, N_SLOT_TESTOBJ), "make_dylan_vector"); + + printf("Object %d committed. " + "Arena reserved: %"PRIuLONGEST".\n", + i, + (ulongest_t)mps_arena_reserved(arena)); + + print_messages(); + } + + /* overwrite all the references to the objects*/ + for (i = 0; i < N_TESTOBJ; i++) { + + /* bonus test of mps_addr_object */ +#if 0 /* Comment this out until mps_addr_object becomes available. */ + mps_addr_t out; + Insist(N_TESTOBJ <= N_INT_TESTOBJ); + + /* use "i" to as a convenient way to generate different interior pointers + To guarantee the i index will give us an interior pointer the number of test + objects must be <= the number of integers in each object */ + Insist(N_TESTOBJ <= N_INT_TESTOBJ); + die(mps_addr_object(&out, arena, &(testobj[i])->int_array[i]), "Address object"); + + Insist(out == testobj[i]); + + /* end piggy back testbench */ +#endif + + /* now overwrite the ref */ + testobj[i] = (mps_word_t)NULL; + + print_messages(); + } + + /* Collect */ + mps_arena_collect(arena); + + print_messages(); + + /* Clean up */ + mps_root_destroy(testobj_root); + /* mps_root_destroy(stack_root);*/ /*commented out while not using stack root */ + mps_thread_dereg(thread); + mps_ap_destroy(obj_ap); + mps_pool_destroy(obj_pool); + mps_fmt_destroy(obj_fmt); + mps_arena_destroy(arena); + + /* Destroying the arena should cause contraction callbacks on all + remaining chunks, even if they had contents. */ + Insist(n_extend == n_contract); + + printf("Arena extended %d times\n", n_extend); + printf("Arena contracted %d times\n", n_contract); + + /* comment out some diagnostics for investigating issue #210 mentioned above */ +#if 0 + printf("&testobj[N_TESTOBJ] = %p\n", (void *)&testobj[N_TESTOBJ]); + printf("cold_stack_end = %p\n", cold_stack_end); +#endif + if (n_extend == 0) + printf("No callbacks received upon arena extended!\n"); + if (n_contract == 0) + printf("No callbacks received upon arena contracted!\n"); + + if (n_contract == 0 || n_extend == 0) + exit(EXIT_FAILURE); +} + +int main(int argc, char* argv[]) +{ + void *stack_marker = &stack_marker; + + testlib_init(argc, argv); + + test_main(stack_marker); + + printf("%s: Conclusion: Failed to find any defects.\n", argv[0]); + + return 0; +} + + +/* C. COPYRIGHT AND LICENSE + * + * Copyright (C) 2022-2023 Ravenbrook Limited . + * + * Redistribution and use in source and binary forms, with or without + * modification, are permitted provided that the following conditions are + * met: + * + * 1. Redistributions of source code must retain the above copyright + * notice, this list of conditions and the following disclaimer. + * + * 2. Redistributions in binary form must reproduce the above copyright + * notice, this list of conditions and the following disclaimer in the + * documentation and/or other materials provided with the + * distribution. + * + * THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS "AS + * IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED + * TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A + * PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT + * HOLDER OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL, + * SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT + * LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE, + * DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY + * THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT + * (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE + * OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE. + */ diff --git a/code/mps.h b/code/mps.h index 4c56265c2e..96214c8966 100644 --- a/code/mps.h +++ b/code/mps.h @@ -120,6 +120,13 @@ typedef mps_addr_t (*mps_fmt_isfwd_t)(mps_addr_t); typedef void (*mps_fmt_pad_t)(mps_addr_t, size_t); typedef mps_addr_t (*mps_fmt_class_t)(mps_addr_t); +/* Callbacks indicating that the arena has extended or contracted. + * These are used to register chunks with RtlInstallFunctionTableCallback + * + * so that the client can unwind the stack through functions in the arena. + */ +typedef void (*mps_arena_extended_t)(mps_arena_t, void *, size_t); +typedef void (*mps_arena_contracted_t)(mps_arena_t, void *, size_t); /* Keyword argument lists */ @@ -171,6 +178,12 @@ extern const struct mps_key_s _mps_key_ARENA_SIZE; extern const struct mps_key_s _mps_key_ARENA_ZONED; #define MPS_KEY_ARENA_ZONED (&_mps_key_ARENA_ZONED) #define MPS_KEY_ARENA_ZONED_FIELD b +extern const struct mps_key_s _mps_key_arena_extended; +#define MPS_KEY_ARENA_EXTENDED (&_mps_key_arena_extended) +#define MPS_KEY_ARENA_EXTENDED_FIELD fun +extern const struct mps_key_s _mps_key_arena_contracted; +#define MPS_KEY_ARENA_CONTRACTED (&_mps_key_arena_contracted) +#define MPS_KEY_ARENA_CONTRACTED_FIELD fun extern const struct mps_key_s _mps_key_FORMAT; #define MPS_KEY_FORMAT (&_mps_key_FORMAT) #define MPS_KEY_FORMAT_FIELD format diff --git a/manual/source/release.rst b/manual/source/release.rst index cba65bcc3a..6674cf691c 100644 --- a/manual/source/release.rst +++ b/manual/source/release.rst @@ -47,6 +47,12 @@ New features :ref:`topic-scanning-protocol`. This allows the client program to safely update references in the visited objects. +#. A :term:`virtual memory arena` can now be configured to call + functions when it acquires a new chunk of :term:`address space`, + and when it returns a chunk of address space to the operation + system. This is intended to support dynamic function tables in + Windows. See :ref:`topic-arena-extension`. + Interface changes ................. diff --git a/manual/source/topic/arena.rst b/manual/source/topic/arena.rst index 520c8179d1..5ca0dbbd0c 100644 --- a/manual/source/topic/arena.rst +++ b/manual/source/topic/arena.rst @@ -139,7 +139,7 @@ Client arenas * :c:macro:`MPS_KEY_ARENA_SIZE` (type :c:type:`size_t`) is its size. - It also accepts three optional keyword arguments: + It also accepts five optional keyword arguments: * :c:macro:`MPS_KEY_COMMIT_LIMIT` (type :c:type:`size_t`) is the maximum amount of memory, in :term:`bytes (1)`, that the MPS @@ -159,6 +159,18 @@ Client arenas may pause the :term:`client program` for. See :c:func:`mps_arena_pause_time_set` for details. + * :c:macro:`MPS_KEY_ARENA_EXTENDED` (type :c:type:`mps_fun_t`) is + a function that will be called immediately after the arena is + *extended*: that is, just after it acquires a new chunk of address + space from the operating system. See :ref:`topic-arena-extension` + for details. + + * :c:macro:`MPS_KEY_ARENA_CONTRACTED` (type :c:type:`mps_fun_t`) + is a function that will be called immediately before the arena is + *contracted*: that is, just before it finishes with a chunk of + address space and returns it to the operating system. See + :ref:`topic-arena-extension` for details. + For example:: MPS_ARGS_BEGIN(args) { @@ -983,8 +995,8 @@ Arena introspection and debugging from MPS-managed memory, then it may attempt to re-enter the MPS, which will fail as the MPS is not re-entrant. - .. |RtlInstallFunctionTableCallback| replace:: ``RtlInstallFunctionTableCallback()`` - .. _RtlInstallFunctionTableCallback: https://msdn.microsoft.com/en-us/library/windows/desktop/ms680595(v=vs.85).aspx + .. |RtlInstallFunctionTableCallback| replace:: :c:func:`RtlInstallFunctionTableCallback` + .. _RtlInstallFunctionTableCallback: https://docs.microsoft.com/en-gb/windows/win32/api/winnt/nf-winnt-rtlinstallfunctiontablecallback If this happens, in order to allow the debugger to finish decoding the call stack, the only remedy is to put the arena @@ -1040,3 +1052,92 @@ Arena introspection and debugging :c:func:`mps_addr_pool`, and to find out which :term:`object format` describes the object at the address, use :c:func:`mps_addr_fmt`. + + +.. index:: + single: arena extension callbacks; introduction + single: extension callbacks; introduction + single: arena contraction callbacks; introduction + single: contraction callbacks; introduction + +.. _topic-arena-extension: + +Arena extension callbacks +------------------------- + +There are situations in which the :term:`client program` needs to be +informed about the chunks of address space that an :term:`arena` is +managing. To support this, the MPS allows the client program to +specify two callback functions when creating a :term:`virtual memory +arena`: one function is called when the arena is *extended* (that is, +when it acquires a new chunk of address space from the operating +system), and the other when the arena is *contracted* (that is, when +it returns a chunk of address space to the operating system). + +The use case that this feature is designed to support is debugging of +dynamically generated code in 64-bit Windows. Microsoft's +documentation for |RtlInstallFunctionTableCallback|_ says: + + Function tables are used on 64-bit Windows to determine how to + unwind or walk the stack. These tables are usually generated by + the compiler and stored as part of the image. However, + applications must provide the function table for dynamically + generated code. + +An application may install a dynamic function table by calling +|RtlInstallFunctionTableCallback|_, passing the region of memory in +which the dynamically generated functions can be found, and may later +delete the table by calling |RtlDeleteFunctionTable|_. + +.. |RtlDeleteFunctionTable| replace:: :c:func:`RtlDeleteFunctionTable` +.. _RtlDeleteFunctionTable: https://docs.microsoft.com/en-gb/windows/win32/api/winnt/nf-winnt-rtldeletefunctiontable + +So if the client program is storing dynamically generated functions in +MPS-managed memory, then it could define callback functions that +install and delete the function table callback for the dynamically +generated code, like this:: + + void arena_extended(mps_arena_t arena, void *base, size_t size) + { + RtlInstallFunctionTableCallback(...); + } + + void arena_contracted(mps_arena_t arena, void *base, size_t size) + { + RtlDeleteFunctionTable(...); + } + +and then pass these two functions using :term:`keyword arguments` to +:c:func:`mps_arena_create_k`:: + + MPS_ARGS_BEGIN(args) { + MPS_ARGS_ADD(args, MPS_KEY_ARENA_EXTENDED, (mps_fun_t)arena_extended); + MPS_ARGS_ADD(args, MPS_KEY_ARENA_CONTRACTED, (mps_fun_t)arena_contracted); + /* ... other keyword arguments ... */ + res = mps_arena_create_k(&arena, mps_arena_class_vm(), args); + } MPS_ARGS_END(args); + +The callback functions receive three arguments: ``arena`` (the arena +being extended or contracted), ``base`` (the base address of the chunk +of address space that has just been acquired from, or is about to be +returned to, the operating system), and ``size`` (the size of the +chunk, in bytes). They must not call any function in the MPS, and must +not access any memory managed by the MPS. + +.. note:: + + The extenstion callback is also called immediately after the arena + is created, in other words, the creation of the arena is treated as + a special example of an extension of the arena. + + The contraction callback is called on all remaining chunks when + the arena is destroyed. There will be at least one callback. + + Every contraction of the arena will match one-to-one with the arena + extensions that have already taken place. After creation, any + contractions performed by the arena will be the same size as the + extensions that have already taken place. Contractions never occur as + amalgamations nor as fractions of previous arena extensions. + + Arena extension callbacks are only supported by :term:`virtual + memory arenas`. diff --git a/tool/testcases.txt b/tool/testcases.txt index b19de92d12..bf5c4e258c 100644 --- a/tool/testcases.txt +++ b/tool/testcases.txt @@ -16,6 +16,7 @@ awlutth =T btcv bttest =N interactive djbench =N benchmark +extcon =W TODO: Enable when we can update Xcode project. See GitHub issue #217 . finalcv =P finaltest =P forktest =X