From 77915dc70f01a40ddeaf27b6544fb12cd5284bda Mon Sep 17 00:00:00 2001 From: Jonathan Holburn Date: Tue, 16 May 2023 17:26:00 +0100 Subject: [PATCH 01/17] Add implementation of mps_addr_object as implemented by jph on top of branch cet-merge-2 and subsequently became mps-2022. Directly fix some whitespace according to c syntax conventions and remove boolean operation on a pointer --- code/arena.c | 18 ++++++++++++++ code/mpm.h | 3 +++ code/mpmst.h | 1 + code/mpmtypes.h | 1 + code/mps.h | 2 ++ code/mpsi.c | 28 ++++++++++++++++++++++ code/pool.c | 12 ++++++++++ code/poolabs.c | 11 +++++++++ code/poolamc.c | 64 +++++++++++++++++++++++++++++++++++++++++++++++++ 9 files changed, 140 insertions(+) diff --git a/code/arena.c b/code/arena.c index 32de9f0aec..9b5fc8f7a3 100644 --- a/code/arena.c +++ b/code/arena.c @@ -1364,6 +1364,24 @@ Bool ArenaHasAddr(Arena arena, Addr addr) return TractOfAddr(&tract, arena, addr); } +/* ArenaAddrObject -- return base pointer of managed object */ +Res ArenaAddrObject(Addr *pReturn, Arena arena, Addr addr) +{ + Seg seg; + Pool pool; + + AVER(pReturn != NULL); + AVERT(Arena, arena); + + /* We currently assume that managed objects are inside Segments + and that pools where this feature is needed have Segments */ + if (!SegOfAddr(&seg, arena, addr)) + return ResFAIL; + + pool = SegPool(seg); + return PoolAddrObject(pReturn, pool, seg, addr); +} + /* C. COPYRIGHT AND LICENSE * diff --git a/code/mpm.h b/code/mpm.h index 9979d20180..d896fe369f 100644 --- a/code/mpm.h +++ b/code/mpm.h @@ -236,6 +236,7 @@ extern Res PoolTraceBegin(Pool pool, Trace trace); extern void PoolFreeWalk(Pool pool, FreeBlockVisitor f, void *p); extern Size PoolTotalSize(Pool pool); extern Size PoolFreeSize(Pool pool); +extern Res PoolAddrObject(Addr *pReturn, Pool pool, Seg seg, Addr addr); extern Res PoolAbsInit(Pool pool, Arena arena, PoolClass klass, ArgList arg); extern void PoolAbsFinish(Inst inst); @@ -267,6 +268,7 @@ extern void PoolTrivFreeWalk(Pool pool, FreeBlockVisitor f, void *p); extern PoolDebugMixin PoolNoDebugMixin(Pool pool); extern BufferClass PoolNoBufferClass(void); extern Size PoolNoSize(Pool pool); +extern Res PoolTrivAddr(Addr *pReturn, Pool pool, Seg seg, Addr addr); /* See .critical.macros. */ #define PoolFreeMacro(pool, old, size) Method(Pool, pool, free)(pool, old, size) @@ -536,6 +538,7 @@ extern Res ArenaStartCollect(Globals globals, TraceStartWhy why); extern Res ArenaCollect(Globals globals, TraceStartWhy why); extern Bool ArenaBusy(Arena arena); extern Bool ArenaHasAddr(Arena arena, Addr addr); +extern Res ArenaAddrObject(Addr *pReturn, Arena arena, Addr addr); extern void ArenaChunkInsert(Arena arena, Chunk chunk); extern void ArenaChunkRemoved(Arena arena, Chunk chunk); extern void ArenaAccumulateTime(Arena arena, Clock start, Clock now); diff --git a/code/mpmst.h b/code/mpmst.h index e03033ae99..f5ba00b8d6 100644 --- a/code/mpmst.h +++ b/code/mpmst.h @@ -60,6 +60,7 @@ typedef struct mps_pool_class_s { PoolRampEndMethod rampEnd; /* end a ramp pattern */ PoolFramePushMethod framePush; /* push an allocation frame */ PoolFramePopMethod framePop; /* pop an allocation frame */ + PoolAddrObjectMethod addrObject; /* return object's base pointer */ PoolFreeWalkMethod freewalk; /* walk over free blocks */ PoolBufferClassMethod bufferClass; /* default BufferClass of pool */ PoolDebugMixinMethod debugMixin; /* find the debug mixin, if any */ diff --git a/code/mpmtypes.h b/code/mpmtypes.h index d913c445f7..d68134a657 100644 --- a/code/mpmtypes.h +++ b/code/mpmtypes.h @@ -214,6 +214,7 @@ typedef Res (*PoolFramePushMethod)(AllocFrame *frameReturn, Pool pool, Buffer buf); typedef Res (*PoolFramePopMethod)(Pool pool, Buffer buf, AllocFrame frame); +typedef Res (*PoolAddrObjectMethod)(Addr *pReturn, Pool pool, Seg seg, Addr addr); typedef void (*PoolFreeWalkMethod)(Pool pool, FreeBlockVisitor f, void *p); typedef BufferClass (*PoolBufferClassMethod)(void); typedef PoolDebugMixin (*PoolDebugMixinMethod)(Pool pool); diff --git a/code/mps.h b/code/mps.h index 4c56265c2e..feee7fbf1b 100644 --- a/code/mps.h +++ b/code/mps.h @@ -832,6 +832,8 @@ extern mps_res_t _mps_fix2(mps_ss_t, mps_addr_t *); (ss)->_ufs = _mps_ufs; \ MPS_END +/* Misc interface */ +extern mps_res_t mps_addr_object(mps_addr_t *p_o, mps_arena_t arena, mps_addr_t addr); #endif /* mps_h */ diff --git a/code/mpsi.c b/code/mpsi.c index 5f19333639..72f65d941a 100644 --- a/code/mpsi.c +++ b/code/mpsi.c @@ -447,6 +447,34 @@ mps_bool_t mps_addr_pool(mps_pool_t *mps_pool_o, return b; } +/* mps_addr_object -- find base pointer of a managed object + * + * Sets *p_o to the base address of an object given a pointer to the + * interior of that object. + * + * Will return resFAIL if the object is not in managed memory + * or the object has been moved. + * + * Will return resUNIMPL if the pool doesn't support this feature + */ +mps_res_t mps_addr_object(mps_addr_t *p_o, mps_arena_t arena, mps_addr_t addr) +{ + Res res; + Addr p; + + AVER(p_o != NULL); + + ArenaEnter(arena); + AVERT(Arena, arena); + res = ArenaAddrObject(&p, arena, (Addr)addr); + ArenaLeave(arena); + + if (res == ResOK) + *p_o = (mps_addr_t)p; + + return res; +} + /* mps_addr_fmt -- what format might this address have? * diff --git a/code/pool.c b/code/pool.c index ea6971025e..9fa25080a1 100644 --- a/code/pool.c +++ b/code/pool.c @@ -57,6 +57,7 @@ Bool PoolClassCheck(PoolClass klass) CHECKL(FUNCHECK(klass->debugMixin)); CHECKL(FUNCHECK(klass->totalSize)); CHECKL(FUNCHECK(klass->freeSize)); + /*CHECKL(FUNCHECK(klass->addrObject));*/ /* Check that pool classes overide sets of related methods. */ CHECKL((klass->init == PoolAbsInit) == @@ -302,6 +303,17 @@ Size PoolFreeSize(Pool pool) return Method(Pool, pool, freeSize)(pool); } +/* PoolAddrObject -- return base pointer from interior pointer */ +Res PoolAddrObject(Addr *pReturn, Pool pool, Seg seg, Addr addr) +{ + AVER(pReturn != NULL); + AVERT(Pool, pool); + AVERT(Seg, seg); + AVER(pool == SegPool(seg)); + AVER(SegBase(seg) <= addr); + AVER(addr < SegLimit(seg)); + return Method(Pool, pool, addrObject)(pReturn, pool, seg, addr); +} /* PoolDescribe -- describe a pool */ diff --git a/code/poolabs.c b/code/poolabs.c index f7fe65a8e4..eb469e967d 100644 --- a/code/poolabs.c +++ b/code/poolabs.c @@ -173,6 +173,7 @@ DEFINE_CLASS(Pool, AbstractPool, klass) klass->debugMixin = PoolNoDebugMixin; klass->totalSize = PoolNoSize; klass->freeSize = PoolNoSize; + klass->addrObject = PoolTrivAddr; klass->sig = PoolClassSig; AVERT(PoolClass, klass); } @@ -475,6 +476,16 @@ Size PoolNoSize(Pool pool) return UNUSED_SIZE; } +Res PoolTrivAddr(Addr *pReturn, Pool pool, Seg seg, Addr addr) +{ + AVERT(Pool, pool); + AVERT(Seg, seg); + AVER(pReturn != NULL); + UNUSED(addr); + + return ResUNIMPL; +} + /* C. COPYRIGHT AND LICENSE * diff --git a/code/poolamc.c b/code/poolamc.c index 8a29aee552..f62413b640 100644 --- a/code/poolamc.c +++ b/code/poolamc.c @@ -1893,6 +1893,69 @@ static void amcWalkAll(Pool pool, FormattedObjectsVisitor f, void *p, size_t s) } } +static Res amcAddrObjectSearch(Addr *pReturn, Pool pool, Addr objBase, + Addr searchLimit, Addr addr) +{ + Format format; + Size hdrSize; + + AVER(pReturn != NULL); + AVERT(Pool, pool); + AVER(objBase <= searchLimit); + + format = pool->format; + hdrSize = format->headerSize; + while (objBase < searchLimit) { + Addr objRef = AddrAdd(objBase, hdrSize); + Addr objLimit = AddrSub((*format->skip)(objRef), hdrSize); + AVER(objBase < objLimit); + + if (addr < objLimit) { + AVER(objBase <= addr); + AVER(addr < objLimit); + + /* Don't return base pointer if object is moved */ + if (NULL == (*format->isMoved)(objRef)) { + *pReturn = objRef; + return ResOK; + } + break; + } + objBase = objLimit; + } + return ResFAIL; +} + +/* AMCAddrObject -- return base pointer from interior pointer */ + +static Res AMCAddrObject(Addr *pReturn, Pool pool, Seg seg, Addr addr) +{ + Res res; + Arena arena; + Addr base, limit; + Buffer buffer; + + + AVER(pReturn != NULL); + AVERT(Pool, pool); + AVERT(Seg, seg); + AVER(SegPool(seg) == pool); + AVER(SegBase(seg) <= addr); + AVER(addr < SegLimit(seg)); + + arena = PoolArena(pool); + base = SegBase(seg); + if (SegBuffer(&buffer, seg)) + limit = BufferGetInit(buffer); + else + limit = SegLimit(seg); + + ShieldExpose(arena, seg); + res = amcAddrObjectSearch(pReturn, pool, base, limit, addr); + ShieldCover(arena, seg); + return res; +} + /* AMCTotalSize -- total memory allocated from the arena */ @@ -2008,6 +2071,7 @@ DEFINE_CLASS(Pool, AMCZPool, klass) klass->bufferClass = amcBufClassGet; klass->totalSize = AMCTotalSize; klass->freeSize = AMCFreeSize; + klass->addrObject = AMCAddrObject; AVERT(PoolClass, klass); } From 6abf2a958c7980cd34e7cc8b3c25a87d80d2c549 Mon Sep 17 00:00:00 2001 From: Jonathan Holburn Date: Tue, 16 May 2023 17:57:19 +0100 Subject: [PATCH 02/17] CHECKL of addrObject method should not be commented out --- code/pool.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/code/pool.c b/code/pool.c index 9fa25080a1..0257359e36 100644 --- a/code/pool.c +++ b/code/pool.c @@ -57,7 +57,7 @@ Bool PoolClassCheck(PoolClass klass) CHECKL(FUNCHECK(klass->debugMixin)); CHECKL(FUNCHECK(klass->totalSize)); CHECKL(FUNCHECK(klass->freeSize)); - /*CHECKL(FUNCHECK(klass->addrObject));*/ + CHECKL(FUNCHECK(klass->addrObject)); /* Check that pool classes overide sets of related methods. */ CHECKL((klass->init == PoolAbsInit) == From 90adcefc4f63539a9bb7eeca8cf75e4063abb3e5 Mon Sep 17 00:00:00 2001 From: Jonathan Holburn Date: Wed, 17 May 2023 18:37:20 +0100 Subject: [PATCH 03/17] add addrobj.c testbench to test mps_addr_object() --- code/addrobj.c | 182 +++++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 182 insertions(+) create mode 100644 code/addrobj.c diff --git a/code/addrobj.c b/code/addrobj.c new file mode 100644 index 0000000000..43d1cd14b9 --- /dev/null +++ b/code/addrobj.c @@ -0,0 +1,182 @@ +/* addrobj.c: BASE ADDRESS FROM INTERIOR POINTER TEST + * + * Copyright (c) 2023 Ravenbrook Limited. See end of file for license. + * + * .overview This test is for mps_addr_object(). Its intention is to + * verify that the function returns the appropriate base pointer to an + * object when provided with an interior pointer. It also tests that the + * function fails appropriately when the provided with a pointer to + * unmanaged memory, or to an object in a pool that doesn't support this + * feature. + * + * .limitations Objects that have been moved should cause the function to + * fail with MPS_RES_FAIL, however this is not tested. It could be tested if + * there is a way to reliably force the MPS to move a specific object. This + * test only examines behaviour in AMCZ and MVFF pools. + */ + +#include "mps.h" +#include "testlib.h" +#include "fmtdy.h" +#include "fmtdytst.h" +#include "mpsavm.h" +#include "mpscamc.h" +#include "mpscmvff.h" +#include + +/* Define an arbitrarily large object size to allocate */ +#define N_SLOT_TESTOBJ 100 + +static void test_main(void) +{ + mps_arena_t arena; + mps_pool_t amcz_pool, mvff_pool; + mps_ap_t obj_ap; + mps_fmt_t obj_fmt; + mps_root_t testobj_root; + mps_res_t res; + static mps_addr_t testobj; + mps_addr_t out, in; + + /* Create arena */ + die(mps_arena_create_k(&arena, mps_arena_class_vm(), mps_args_none), "mps_arena_create_k"); + + + /* TEST 1: Test using an interior pointer in an object in an AMCZ pool */ + + /* Use the dylan format for convenience */ + die(dylan_fmt(&obj_fmt, arena), "dylan_fmt"); + + /* Create amcz pool */ + MPS_ARGS_BEGIN(args) { + MPS_ARGS_ADD(args, MPS_KEY_FORMAT, obj_fmt); + die(mps_pool_create_k(&amcz_pool, arena, mps_class_amcz(), args), "mps_pool_create_k amcz"); + } MPS_ARGS_END(args); + + /* Create an area of ambiguous pointers to keep the object alive and in place, in this case + the area only contains room for a single reference since we are only using one object to test */ + die(mps_root_create_area(&testobj_root, arena, + mps_rank_ambig(), (mps_rm_t)0, + &testobj, &testobj+1, + mps_scan_area, NULL), + "mps_root_create_area"); + + /* Create the allocation point */ + die(mps_ap_create_k(&obj_ap, amcz_pool, mps_args_none), "mps_ap_create_k"); + + /* Make an arbitrary sized object, size = (N_SLOT_TESTOBJ+2) * sizeof(mps_word_t). + (See fmtdytst.c for size calculation) */ + die(make_dylan_vector((mps_word_t *)&testobj, obj_ap, N_SLOT_TESTOBJ), "make_dylan_vector"); + + /* Construct a pointer to roughly halfway inside the object */ + in = (mps_addr_t)((char *)testobj + (N_SLOT_TESTOBJ/2) * sizeof(mps_word_t)); + + /* Ensure that this is an interior pointer, and not the base pointer */ + Insist(in > testobj); + + /* Do Test */ + res = mps_addr_object(&out, arena, in); + Insist(out == testobj); + Insist(res == MPS_RES_OK); + + + /* TEST 2: Test using the base pointer itself as an input*/ + + in = testobj; + + /* Do Test */ + res = mps_addr_object(&out, arena, in); + Insist(out == testobj); + Insist(res == MPS_RES_OK); + + + /* TEST 3: Test using a pointer one-off-the-end of the object*/ + + in = (mps_addr_t)((char *)testobj + (N_SLOT_TESTOBJ + 2) * sizeof(mps_word_t)); + + /* Do Test */ + res = mps_addr_object(&out, arena, in); + Insist(res == MPS_RES_FAIL); + + /* Clean up from above tests */ + mps_root_destroy(testobj_root); + mps_ap_destroy(obj_ap); + mps_pool_destroy(amcz_pool); + mps_fmt_destroy(obj_fmt); + + + /* TEST 4: Test using a pointer in unmanaged memory */ + + /* Use malloc to allocate non-mps-managed memory on the heap */ + in = malloc(sizeof(mps_word_t)); + Insist(NULL != in); + + /* Do the test */ + res = mps_addr_object(&out, arena, in); + + /* Expect MPS to fail to find a base pointer for addresses not in managed memory */ + Insist(res == MPS_RES_FAIL); + + /* clean up from this test */ + if (NULL != in) + free(in); + + + /* TEST 5: Test using a pointer in a pool which currently doesn't implement mps_addr_object */ + + /* Create mvff pool for which mps_addr_object is not implemented */ + die(mps_pool_create_k(&mvff_pool, arena, mps_class_mvff(), mps_args_none), "mps_pool_create_k mvff"); + + /* allocate an object (just some memory) in this pool */ + die(mps_alloc(&in, mvff_pool, sizeof(mps_word_t)), "mps_alloc"); + + /* Do the test */ + res = mps_addr_object(&out, arena, in); + + /* We should insist MPS_RES_UNIMPL here but the Trivial Method for mps_addr_object is only reached for pools that contain objects in Segs. + Since mvff does not, and the input address is not inside a Seg, the call is rejected by the Arena with MPS_RES_FAIL */ + Insist(res == MPS_RES_FAIL); + + /* Final clean up */ + mps_free(mvff_pool, in, sizeof(mps_word_t)); + mps_pool_destroy(mvff_pool); + mps_arena_destroy(arena); +} + +int main(int argc, char *argv[]) +{ + testlib_init(argc, argv); + + test_main(); + + 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. + */ From 4bab56b498fc0fb85f867e547136d53146e0f16f Mon Sep 17 00:00:00 2001 From: Jonathan Holburn Date: Wed, 17 May 2023 20:39:58 +0100 Subject: [PATCH 04/17] add addrobj.c to the testcases --- code/comm.gmk | 4 ++++ code/commpost.nmk | 3 +++ code/commpre.nmk | 1 + tool/testcases.txt | 1 + 4 files changed, 9 insertions(+) diff --git a/code/comm.gmk b/code/comm.gmk index a2cd68baab..ea532e4914 100644 --- a/code/comm.gmk +++ b/code/comm.gmk @@ -253,6 +253,7 @@ LIB_TARGETS=mps.a mpsplan.a TEST_TARGETS=\ abqtest \ + addrobj \ airtest \ amcss \ amcsshe \ @@ -445,6 +446,9 @@ ifdef VARIETY $(PFM)/$(VARIETY)/abqtest: $(PFM)/$(VARIETY)/abqtest.o \ $(TESTLIBOBJ) $(PFM)/$(VARIETY)/mps.a +$(PFM)/$(VARIETY)/adrobj: $(PFM)/$(VARIETY)/addrobj.o \ + $(FMTDYTSTOBJ) $(TESTLIBOBJ) $(PFM)/$(VARIETY)/mps.a + $(PFM)/$(VARIETY)/airtest: $(PFM)/$(VARIETY)/airtest.o \ $(FMTSCMOBJ) $(TESTLIBOBJ) $(PFM)/$(VARIETY)/mps.a diff --git a/code/commpost.nmk b/code/commpost.nmk index 470db2519b..c7e0b0ff4b 100644 --- a/code/commpost.nmk +++ b/code/commpost.nmk @@ -177,6 +177,9 @@ $(PFM)\cool\mps.lib: $(MPMOBJ) $(PFM)\$(VARIETY)\abqtest.exe: $(PFM)\$(VARIETY)\abqtest.obj \ $(PFM)\$(VARIETY)\mps.lib $(TESTLIBOBJ) +$(PFM)\$(VARIETY)\addrobj.exe: $(PFM)\$(VARIETY)\addrobj.obj \ + $(PFM)\$(VARIETY)\mps.lib $(FMTTESTOBJ) $(TESTLIBOBJ) + $(PFM)\$(VARIETY)\airtest.exe: $(PFM)\$(VARIETY)\airtest.obj \ $(PFM)\$(VARIETY)\mps.lib $(FMTSCHEMEOBJ) $(TESTLIBOBJ) diff --git a/code/commpre.nmk b/code/commpre.nmk index 236e08ad2e..7fc4719b8d 100644 --- a/code/commpre.nmk +++ b/code/commpre.nmk @@ -59,6 +59,7 @@ LIB_TARGETS=mps.lib TEST_TARGETS=\ abqtest.exe \ + addrObj.exe \ airtest.exe \ amcss.exe \ amcsshe.exe \ diff --git a/tool/testcases.txt b/tool/testcases.txt index b19de92d12..a08d27e259 100644 --- a/tool/testcases.txt +++ b/tool/testcases.txt @@ -2,6 +2,7 @@ Test case Flags Notes ============= ================ ========================================== abqtest +addrobj =W TODO: Enable when we can update Xcode project. See GitHub issue #217 airtest amcss =P amcsshe =P From 9ed69039ee528efbbbe3403226b126bee20ac64b Mon Sep 17 00:00:00 2001 From: Jonathan Holburn Date: Wed, 17 May 2023 20:41:09 +0100 Subject: [PATCH 05/17] add documentation for mps_addr_object --- manual/source/release.rst | 6 ++++++ manual/source/topic/arena.rst | 34 ++++++++++++++++++++++++++++++++++ 2 files changed, 40 insertions(+) diff --git a/manual/source/release.rst b/manual/source/release.rst index cba65bcc3a..f6df09940e 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. +#. The new function :c:func:`mps_addr_object` allows clients to + discover the base pointer of an object from a pointer to anywhere + inside the object. This is intended to support stack tracing and + debugging for client programs that allocate their code on the + heap. + Interface changes ................. diff --git a/manual/source/topic/arena.rst b/manual/source/topic/arena.rst index 520c8179d1..b7360e07b2 100644 --- a/manual/source/topic/arena.rst +++ b/manual/source/topic/arena.rst @@ -1040,3 +1040,37 @@ 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`. + + +.. c:function:: mps_res_t mps_addr_object(mps_addr_t *p_o, mps_arena_t arena, mps_addr_t addr) + + Return the :term:`base pointer` of an :term:`object` from an + :term:`interior pointer` to the object, or the object's base pointer. + + ``p_o`` points to the :term:`address` to which the object's base pointer + should be returned. + + ``arena`` is an arena. + + ``addr`` is an address that might be an interior or base pointer. + + Returns MPS_RES_OK if a base pointer to an object into which ``addr`` + points was successfully returned. Returns MPS_RES_FAIL if ``addr`` + points to memory not managed by the ``arena`` or if ``addr`` points + to the interior of an object which has been moved by a :term:`moving memory manager`. + If ``addr`` is found to be managed by a :term:`pool` which does not support this feature, + MPS_RES_FAIL or MPS_RES_UNIMPL may be returned, depending on the :term:`pool class`. + + :c:func:`mps_addr_object` allows a client program that allocates + code on the heap to implement debugging and stack tracing. + + This feature is supported by AMC and AMCZ pools only. + + .. note:: + + This function is intended to assist with debugging fatal + errors in the :term:`client program`. It is not expected to be + needed in normal use. If you find yourself wanting to use this + function other than in the use case described, there may + be a better way to meet your requirements: please + :ref:`contact us `. From ab22eb9c241d4a5992e5d454cee0dda36c5715cb Mon Sep 17 00:00:00 2001 From: Jonathan Holburn Date: Wed, 17 May 2023 20:48:33 +0100 Subject: [PATCH 06/17] correct capitalisation on target name --- code/commpre.nmk | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/code/commpre.nmk b/code/commpre.nmk index 7fc4719b8d..edfcdf8973 100644 --- a/code/commpre.nmk +++ b/code/commpre.nmk @@ -59,7 +59,7 @@ LIB_TARGETS=mps.lib TEST_TARGETS=\ abqtest.exe \ - addrObj.exe \ + addrobj.exe \ airtest.exe \ amcss.exe \ amcsshe.exe \ From 11e31a61bd7420523e073634758e20043d0f7323 Mon Sep 17 00:00:00 2001 From: Jonathan Holburn Date: Wed, 17 May 2023 20:52:31 +0100 Subject: [PATCH 07/17] correct spelling of target --- code/comm.gmk | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/code/comm.gmk b/code/comm.gmk index ea532e4914..cf5cf8c851 100644 --- a/code/comm.gmk +++ b/code/comm.gmk @@ -446,7 +446,7 @@ ifdef VARIETY $(PFM)/$(VARIETY)/abqtest: $(PFM)/$(VARIETY)/abqtest.o \ $(TESTLIBOBJ) $(PFM)/$(VARIETY)/mps.a -$(PFM)/$(VARIETY)/adrobj: $(PFM)/$(VARIETY)/addrobj.o \ +$(PFM)/$(VARIETY)/addrobj: $(PFM)/$(VARIETY)/addrobj.o \ $(FMTDYTSTOBJ) $(TESTLIBOBJ) $(PFM)/$(VARIETY)/mps.a $(PFM)/$(VARIETY)/airtest: $(PFM)/$(VARIETY)/airtest.o \ From e99035e3c019e4435c7b4aa9547d0d797c19028f Mon Sep 17 00:00:00 2001 From: Jonathan Holburn Date: Wed, 17 May 2023 21:30:07 +0100 Subject: [PATCH 08/17] avoid type punning while using dylan test utilities --- code/addrobj.c | 18 +++++++++++++++++- 1 file changed, 17 insertions(+), 1 deletion(-) diff --git a/code/addrobj.c b/code/addrobj.c index 43d1cd14b9..6c34e83351 100644 --- a/code/addrobj.c +++ b/code/addrobj.c @@ -66,7 +66,23 @@ static void test_main(void) /* Make an arbitrary sized object, size = (N_SLOT_TESTOBJ+2) * sizeof(mps_word_t). (See fmtdytst.c for size calculation) */ - die(make_dylan_vector((mps_word_t *)&testobj, obj_ap, N_SLOT_TESTOBJ), "make_dylan_vector"); + + /* Because make_dylan_vector returns its pointer-to-object as an mps_word_t rather than an + mps_addr_t, and commits the object, we need to somehow safely allocate our object without + type punning and without risking that our object be destroyed. + Rather than redefine our reference table with type mps_word_t, which hides the intention of the table, + park the arena to disable garbage collection. Allocate our dylan object on the (unregistered) stack + storing its address in an mps_word_t. Then store this mps_word_t as an mps_addr_t in our reference + table, and release the arena since our object is now safely pinned. + */ + { + mps_word_t p_word; + mps_arena_park(arena); + die(make_dylan_vector(&p_word, obj_ap, N_SLOT_TESTOBJ), "make_dylan_vector"); + /* If we hadn't parked the arena, our vector might have been GC'd here */ + testobj = (mps_addr_t)p_word; + mps_arena_release(arena); + } /* Construct a pointer to roughly halfway inside the object */ in = (mps_addr_t)((char *)testobj + (N_SLOT_TESTOBJ/2) * sizeof(mps_word_t)); From 2eadb7ed41a95f9d95582db6a8e8ccb95b9a1295 Mon Sep 17 00:00:00 2001 From: Jonathan Holburn Date: Wed, 17 May 2023 21:51:12 +0100 Subject: [PATCH 09/17] reorganise comments for readability in addrobj.c --- code/addrobj.c | 17 ++++++++--------- 1 file changed, 8 insertions(+), 9 deletions(-) diff --git a/code/addrobj.c b/code/addrobj.c index 6c34e83351..1e4bcbff66 100644 --- a/code/addrobj.c +++ b/code/addrobj.c @@ -66,16 +66,15 @@ static void test_main(void) /* Make an arbitrary sized object, size = (N_SLOT_TESTOBJ+2) * sizeof(mps_word_t). (See fmtdytst.c for size calculation) */ - - /* Because make_dylan_vector returns its pointer-to-object as an mps_word_t rather than an - mps_addr_t, and commits the object, we need to somehow safely allocate our object without - type punning and without risking that our object be destroyed. - Rather than redefine our reference table with type mps_word_t, which hides the intention of the table, - park the arena to disable garbage collection. Allocate our dylan object on the (unregistered) stack - storing its address in an mps_word_t. Then store this mps_word_t as an mps_addr_t in our reference - table, and release the arena since our object is now safely pinned. - */ { + /* Because make_dylan_vector returns its pointer-to-object as an mps_word_t rather than an + mps_addr_t, and commits the object, we need to somehow safely allocate our object without + type punning and without risking that our object be destroyed. + Rather than redefine our reference table with type mps_word_t, which hides the intention of the table, + park the arena to disable garbage collection. Allocate our dylan object on the (unregistered) stack + storing its address in an mps_word_t. Then store this mps_word_t as an mps_addr_t in our reference + table, and release the arena since our object is now safely pinned. + */ mps_word_t p_word; mps_arena_park(arena); die(make_dylan_vector(&p_word, obj_ap, N_SLOT_TESTOBJ), "make_dylan_vector"); From c7049ec06d67bcc088fd4d64cad402b84f742d26 Mon Sep 17 00:00:00 2001 From: Richard Brooksby Date: Wed, 7 Jun 2023 14:57:55 +0100 Subject: [PATCH 10/17] Generalising mps_addr_object, ArenaAddrObject, PoolAddrObject for pools that do not use segments. --- code/addrobj.c | 4 +--- code/arena.c | 12 +++++------- code/mpm.h | 4 ++-- code/mpmtypes.h | 2 +- code/pool.c | 18 +++++++++++------- code/poolabs.c | 6 +++--- code/poolamc.c | 12 ++++++------ 7 files changed, 29 insertions(+), 29 deletions(-) diff --git a/code/addrobj.c b/code/addrobj.c index 1e4bcbff66..eda49fc1ff 100644 --- a/code/addrobj.c +++ b/code/addrobj.c @@ -148,9 +148,7 @@ static void test_main(void) /* Do the test */ res = mps_addr_object(&out, arena, in); - /* We should insist MPS_RES_UNIMPL here but the Trivial Method for mps_addr_object is only reached for pools that contain objects in Segs. - Since mvff does not, and the input address is not inside a Seg, the call is rejected by the Arena with MPS_RES_FAIL */ - Insist(res == MPS_RES_FAIL); + Insist(res == MPS_RES_UNIMPL); /* Final clean up */ mps_free(mvff_pool, in, sizeof(mps_word_t)); diff --git a/code/arena.c b/code/arena.c index 9b5fc8f7a3..7139ff4bc8 100644 --- a/code/arena.c +++ b/code/arena.c @@ -1367,19 +1367,17 @@ Bool ArenaHasAddr(Arena arena, Addr addr) /* ArenaAddrObject -- return base pointer of managed object */ Res ArenaAddrObject(Addr *pReturn, Arena arena, Addr addr) { - Seg seg; - Pool pool; + Tract tract; AVER(pReturn != NULL); AVERT(Arena, arena); - /* We currently assume that managed objects are inside Segments - and that pools where this feature is needed have Segments */ - if (!SegOfAddr(&seg, arena, addr)) + if (!TractOfAddr(&tract, arena, addr)) { + /* address does not belong to the arena */ return ResFAIL; + } - pool = SegPool(seg); - return PoolAddrObject(pReturn, pool, seg, addr); + return PoolAddrObject(pReturn, TractPool(tract), addr); } diff --git a/code/mpm.h b/code/mpm.h index d896fe369f..e072a7a1ea 100644 --- a/code/mpm.h +++ b/code/mpm.h @@ -236,7 +236,7 @@ extern Res PoolTraceBegin(Pool pool, Trace trace); extern void PoolFreeWalk(Pool pool, FreeBlockVisitor f, void *p); extern Size PoolTotalSize(Pool pool); extern Size PoolFreeSize(Pool pool); -extern Res PoolAddrObject(Addr *pReturn, Pool pool, Seg seg, Addr addr); +extern Res PoolAddrObject(Addr *pReturn, Pool pool, Addr addr); extern Res PoolAbsInit(Pool pool, Arena arena, PoolClass klass, ArgList arg); extern void PoolAbsFinish(Inst inst); @@ -268,7 +268,7 @@ extern void PoolTrivFreeWalk(Pool pool, FreeBlockVisitor f, void *p); extern PoolDebugMixin PoolNoDebugMixin(Pool pool); extern BufferClass PoolNoBufferClass(void); extern Size PoolNoSize(Pool pool); -extern Res PoolTrivAddr(Addr *pReturn, Pool pool, Seg seg, Addr addr); +extern Res PoolTrivAddrObject(Addr *pReturn, Pool pool, Addr addr); /* See .critical.macros. */ #define PoolFreeMacro(pool, old, size) Method(Pool, pool, free)(pool, old, size) diff --git a/code/mpmtypes.h b/code/mpmtypes.h index d68134a657..c219e908e0 100644 --- a/code/mpmtypes.h +++ b/code/mpmtypes.h @@ -214,7 +214,7 @@ typedef Res (*PoolFramePushMethod)(AllocFrame *frameReturn, Pool pool, Buffer buf); typedef Res (*PoolFramePopMethod)(Pool pool, Buffer buf, AllocFrame frame); -typedef Res (*PoolAddrObjectMethod)(Addr *pReturn, Pool pool, Seg seg, Addr addr); +typedef Res (*PoolAddrObjectMethod)(Addr *pReturn, Pool pool, Addr addr); typedef void (*PoolFreeWalkMethod)(Pool pool, FreeBlockVisitor f, void *p); typedef BufferClass (*PoolBufferClassMethod)(void); typedef PoolDebugMixin (*PoolDebugMixinMethod)(Pool pool); diff --git a/code/pool.c b/code/pool.c index 0257359e36..fc04563175 100644 --- a/code/pool.c +++ b/code/pool.c @@ -303,16 +303,20 @@ Size PoolFreeSize(Pool pool) return Method(Pool, pool, freeSize)(pool); } -/* PoolAddrObject -- return base pointer from interior pointer */ -Res PoolAddrObject(Addr *pReturn, Pool pool, Seg seg, Addr addr) + +/* PoolAddrObject -- return base pointer from interior pointer + * + * Note: addr is not necessarily inside the pool, even though + * mps_addr_object dispatches via the tract table. This allows this + * function to be used more generally internally. The pool should + * check (it has to anyway). + */ + +Res PoolAddrObject(Addr *pReturn, Pool pool, Addr addr) { AVER(pReturn != NULL); AVERT(Pool, pool); - AVERT(Seg, seg); - AVER(pool == SegPool(seg)); - AVER(SegBase(seg) <= addr); - AVER(addr < SegLimit(seg)); - return Method(Pool, pool, addrObject)(pReturn, pool, seg, addr); + return Method(Pool, pool, addrObject)(pReturn, pool, addr); } /* PoolDescribe -- describe a pool */ diff --git a/code/poolabs.c b/code/poolabs.c index eb469e967d..d6eae2dd04 100644 --- a/code/poolabs.c +++ b/code/poolabs.c @@ -173,7 +173,7 @@ DEFINE_CLASS(Pool, AbstractPool, klass) klass->debugMixin = PoolNoDebugMixin; klass->totalSize = PoolNoSize; klass->freeSize = PoolNoSize; - klass->addrObject = PoolTrivAddr; + klass->addrObject = PoolTrivAddrObject; klass->sig = PoolClassSig; AVERT(PoolClass, klass); } @@ -476,10 +476,10 @@ Size PoolNoSize(Pool pool) return UNUSED_SIZE; } -Res PoolTrivAddr(Addr *pReturn, Pool pool, Seg seg, Addr addr) + +Res PoolTrivAddrObject(Addr *pReturn, Pool pool, Addr addr) { AVERT(Pool, pool); - AVERT(Seg, seg); AVER(pReturn != NULL); UNUSED(addr); diff --git a/code/poolamc.c b/code/poolamc.c index f62413b640..4455bb9e85 100644 --- a/code/poolamc.c +++ b/code/poolamc.c @@ -1926,24 +1926,24 @@ static Res amcAddrObjectSearch(Addr *pReturn, Pool pool, Addr objBase, return ResFAIL; } + /* AMCAddrObject -- return base pointer from interior pointer */ -static Res AMCAddrObject(Addr *pReturn, Pool pool, Seg seg, Addr addr) +static Res AMCAddrObject(Addr *pReturn, Pool pool, Addr addr) { Res res; Arena arena; Addr base, limit; Buffer buffer; - + Seg seg; AVER(pReturn != NULL); AVERT(Pool, pool); - AVERT(Seg, seg); - AVER(SegPool(seg) == pool); - AVER(SegBase(seg) <= addr); - AVER(addr < SegLimit(seg)); arena = PoolArena(pool); + if (!SegOfAddr(&seg, arena, addr) || SegPool(seg) != pool) + return ResFAIL; + base = SegBase(seg); if (SegBuffer(&buffer, seg)) limit = BufferGetInit(buffer); From 7e0289fa591a2970a9b88e5ef5bb00187bebf099 Mon Sep 17 00:00:00 2001 From: Richard Brooksby Date: Wed, 7 Jun 2023 16:12:36 +0100 Subject: [PATCH 11/17] Adding addrobj test case to Xcode project and enabling it as part of testci target on all platforms. --- code/mps.xcodeproj/project.pbxproj | 227 +++++++++--------- .../xcshareddata/xcschemes/addrobj.xcscheme | 78 ++++++ .../xcshareddata/xcschemes/finaltest.xcscheme | 78 ++++++ tool/testcases.txt | 2 +- 4 files changed, 272 insertions(+), 113 deletions(-) create mode 100644 code/mps.xcodeproj/xcshareddata/xcschemes/addrobj.xcscheme create mode 100644 code/mps.xcodeproj/xcshareddata/xcschemes/finaltest.xcscheme diff --git a/code/mps.xcodeproj/project.pbxproj b/code/mps.xcodeproj/project.pbxproj index 42f60ccb58..909dbfbd60 100644 --- a/code/mps.xcodeproj/project.pbxproj +++ b/code/mps.xcodeproj/project.pbxproj @@ -73,6 +73,7 @@ buildPhases = ( ); dependencies = ( + 319F7A192A30D2F000E5B418 /* PBXTargetDependency */, 3104AFF6156D37BC000A585A /* PBXTargetDependency */, 3114A644156E94FB001E0AA3 /* PBXTargetDependency */, 22FACEF1188809B5000FDBC1 /* PBXTargetDependency */, @@ -89,8 +90,6 @@ 3114A677156E961C001E0AA3 /* PBXTargetDependency */, 3114A612156E943B001E0AA3 /* PBXTargetDependency */, 22B2BC3D18B643B300C33E63 /* PBXTargetDependency */, - 2291A5E6175CB207001D4920 /* PBXTargetDependency */, - 2291A5E8175CB20E001D4920 /* PBXTargetDependency */, 3114A5CC156E932C001E0AA3 /* PBXTargetDependency */, 3114A5EA156E93C4001E0AA3 /* PBXTargetDependency */, 22EA3F4820D2B23F0065F5B6 /* PBXTargetDependency */, @@ -158,16 +157,6 @@ 2291A5B5175CAB2F001D4920 /* testlib.c in Sources */ = {isa = PBXBuildFile; fileRef = 31EEAC9E156AB73400714D05 /* testlib.c */; }; 2291A5B7175CAB2F001D4920 /* libmps.a in Frameworks */ = {isa = PBXBuildFile; fileRef = 31EEABFB156AAF9D00714D05 /* libmps.a */; }; 2291A5BE175CAB4E001D4920 /* awlutth.c in Sources */ = {isa = PBXBuildFile; fileRef = 2291A5A9175CAA9B001D4920 /* awlutth.c */; }; - 2291A5C5175CAFCA001D4920 /* fmtdy.c in Sources */ = {isa = PBXBuildFile; fileRef = 3124CAC6156BE48D00753214 /* fmtdy.c */; }; - 2291A5C6175CAFCA001D4920 /* fmtdytst.c in Sources */ = {isa = PBXBuildFile; fileRef = 3124CAC7156BE48D00753214 /* fmtdytst.c */; }; - 2291A5C7175CAFCA001D4920 /* fmtno.c in Sources */ = {isa = PBXBuildFile; fileRef = 3124CACC156BE4C200753214 /* fmtno.c */; }; - 2291A5C8175CAFCA001D4920 /* testlib.c in Sources */ = {isa = PBXBuildFile; fileRef = 31EEAC9E156AB73400714D05 /* testlib.c */; }; - 2291A5CB175CAFCA001D4920 /* libmps.a in Frameworks */ = {isa = PBXBuildFile; fileRef = 31EEABFB156AAF9D00714D05 /* libmps.a */; }; - 2291A5D8175CB05F001D4920 /* fmtdy.c in Sources */ = {isa = PBXBuildFile; fileRef = 3124CAC6156BE48D00753214 /* fmtdy.c */; }; - 2291A5D9175CB05F001D4920 /* fmtdytst.c in Sources */ = {isa = PBXBuildFile; fileRef = 3124CAC7156BE48D00753214 /* fmtdytst.c */; }; - 2291A5DA175CB05F001D4920 /* fmtno.c in Sources */ = {isa = PBXBuildFile; fileRef = 3124CACC156BE4C200753214 /* fmtno.c */; }; - 2291A5DB175CB05F001D4920 /* testlib.c in Sources */ = {isa = PBXBuildFile; fileRef = 31EEAC9E156AB73400714D05 /* testlib.c */; }; - 2291A5DD175CB05F001D4920 /* libmps.a in Frameworks */ = {isa = PBXBuildFile; fileRef = 31EEABFB156AAF9D00714D05 /* libmps.a */; }; 2291A5ED175CB5E2001D4920 /* landtest.c in Sources */ = {isa = PBXBuildFile; fileRef = 2291A5E9175CB4EC001D4920 /* landtest.c */; }; 22B2BC2E18B6434F00C33E63 /* mps.c in Sources */ = {isa = PBXBuildFile; fileRef = 31A47BA3156C1E130039B1C2 /* mps.c */; }; 22B2BC3718B6437C00C33E63 /* scheme-advanced.c in Sources */ = {isa = PBXBuildFile; fileRef = 22B2BC2B18B6434000C33E63 /* scheme-advanced.c */; }; @@ -301,6 +290,12 @@ 3124CAFC156BE82900753214 /* libmps.a in Frameworks */ = {isa = PBXBuildFile; fileRef = 31EEABFB156AAF9D00714D05 /* libmps.a */; }; 3150AE53156ABA2500A6E22A /* libmps.a in Frameworks */ = {isa = PBXBuildFile; fileRef = 31EEABFB156AAF9D00714D05 /* libmps.a */; }; 318DA8D31892B27E0089718C /* testlib.c in Sources */ = {isa = PBXBuildFile; fileRef = 31EEAC9E156AB73400714D05 /* testlib.c */; }; + 319F7A082A30D08500E5B418 /* testlib.c in Sources */ = {isa = PBXBuildFile; fileRef = 31EEAC9E156AB73400714D05 /* testlib.c */; }; + 319F7A0A2A30D08500E5B418 /* fmtdy.c in Sources */ = {isa = PBXBuildFile; fileRef = 3124CAC6156BE48D00753214 /* fmtdy.c */; }; + 319F7A0B2A30D08500E5B418 /* fmtdytst.c in Sources */ = {isa = PBXBuildFile; fileRef = 3124CAC7156BE48D00753214 /* fmtdytst.c */; }; + 319F7A0C2A30D08500E5B418 /* fmtno.c in Sources */ = {isa = PBXBuildFile; fileRef = 3124CACC156BE4C200753214 /* fmtno.c */; }; + 319F7A0E2A30D08500E5B418 /* libmps.a in Frameworks */ = {isa = PBXBuildFile; fileRef = 31EEABFB156AAF9D00714D05 /* libmps.a */; }; + 319F7A172A30D11400E5B418 /* addrobj.c in Sources */ = {isa = PBXBuildFile; fileRef = 319F7A152A30D11400E5B418 /* addrobj.c */; }; 31A47BA4156C1E130039B1C2 /* mps.c in Sources */ = {isa = PBXBuildFile; fileRef = 31A47BA3156C1E130039B1C2 /* mps.c */; }; 31D60007156D3C6200337B26 /* segsmss.c in Sources */ = {isa = PBXBuildFile; fileRef = 31D60006156D3C5F00337B26 /* segsmss.c */; }; 31D60008156D3C7400337B26 /* libmps.a in Frameworks */ = {isa = PBXBuildFile; fileRef = 31EEABFB156AAF9D00714D05 /* libmps.a */; }; @@ -468,20 +463,6 @@ remoteGlobalIDString = 2291A5AC175CAB2F001D4920; remoteInfo = awlutth; }; - 2291A5C3175CAFCA001D4920 /* PBXContainerItemProxy */ = { - isa = PBXContainerItemProxy; - containerPortal = 31EEABDA156AAE9E00714D05 /* Project object */; - proxyType = 1; - remoteGlobalIDString = 31EEABFA156AAF9D00714D05; - remoteInfo = mps; - }; - 2291A5D5175CB05F001D4920 /* PBXContainerItemProxy */ = { - isa = PBXContainerItemProxy; - containerPortal = 31EEABDA156AAE9E00714D05 /* Project object */; - proxyType = 1; - remoteGlobalIDString = 31EEABFA156AAF9D00714D05; - remoteInfo = mps; - }; 229E228719EAB10D00E21417 /* PBXContainerItemProxy */ = { isa = PBXContainerItemProxy; containerPortal = 31EEABDA156AAE9E00714D05 /* Project object */; @@ -923,6 +904,20 @@ remoteGlobalIDString = 31108A3A1C6B90E900E728EA; remoteInfo = tagtest; }; + 319F7A062A30D08500E5B418 /* PBXContainerItemProxy */ = { + isa = PBXContainerItemProxy; + containerPortal = 31EEABDA156AAE9E00714D05 /* Project object */; + proxyType = 1; + remoteGlobalIDString = 31EEABFA156AAF9D00714D05; + remoteInfo = mps; + }; + 319F7A182A30D2F000E5B418 /* PBXContainerItemProxy */ = { + isa = PBXContainerItemProxy; + containerPortal = 31EEABDA156AAE9E00714D05 /* Project object */; + proxyType = 1; + remoteGlobalIDString = 319F7A042A30D08500E5B418; + remoteInfo = addrobj; + }; 31A47BA9156C210D0039B1C2 /* PBXContainerItemProxy */ = { isa = PBXContainerItemProxy; containerPortal = 31EEABDA156AAE9E00714D05 /* Project object */; @@ -1092,24 +1087,6 @@ ); runOnlyForDeploymentPostprocessing = 1; }; - 2291A5CC175CAFCA001D4920 /* CopyFiles */ = { - isa = PBXCopyFilesBuildPhase; - buildActionMask = 2147483647; - dstPath = /usr/share/man/man1/; - dstSubfolderSpec = 0; - files = ( - ); - runOnlyForDeploymentPostprocessing = 1; - }; - 2291A5DE175CB05F001D4920 /* CopyFiles */ = { - isa = PBXCopyFilesBuildPhase; - buildActionMask = 2147483647; - dstPath = /usr/share/man/man1/; - dstSubfolderSpec = 0; - files = ( - ); - runOnlyForDeploymentPostprocessing = 1; - }; 22B2BC3118B6434F00C33E63 /* CopyFiles */ = { isa = PBXCopyFilesBuildPhase; buildActionMask = 2147483647; @@ -1407,6 +1384,15 @@ ); runOnlyForDeploymentPostprocessing = 1; }; + 319F7A0F2A30D08500E5B418 /* CopyFiles */ = { + isa = PBXCopyFilesBuildPhase; + buildActionMask = 2147483647; + dstPath = /usr/share/man/man1/; + dstSubfolderSpec = 0; + files = ( + ); + runOnlyForDeploymentPostprocessing = 1; + }; 31D6000B156D3CB200337B26 /* CopyFiles */ = { isa = PBXCopyFilesBuildPhase; buildActionMask = 2147483647; @@ -1729,6 +1715,8 @@ 31942AA91C8EC446001AAF32 /* sp.txt */ = {isa = PBXFileReference; lastKnownFileType = text; name = sp.txt; path = ../design/sp.txt; sourceTree = ""; }; 31942AAB1C8EC446001AAF32 /* stack-scan.txt */ = {isa = PBXFileReference; lastKnownFileType = text; name = "stack-scan.txt"; path = "../design/stack-scan.txt"; sourceTree = ""; }; 31942AB01C8EC446001AAF32 /* testthr.txt */ = {isa = PBXFileReference; lastKnownFileType = text; name = testthr.txt; path = ../design/testthr.txt; sourceTree = ""; }; + 319F7A142A30D08500E5B418 /* addrobj */ = {isa = PBXFileReference; explicitFileType = "compiled.mach-o.executable"; includeInIndex = 0; path = addrobj; sourceTree = BUILT_PRODUCTS_DIR; }; + 319F7A152A30D11400E5B418 /* addrobj.c */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.c.c; path = addrobj.c; sourceTree = ""; }; 31A47BA3156C1E130039B1C2 /* mps.c */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.c.c; path = mps.c; sourceTree = ""; }; 31C83ADD1786281C0031A0DB /* protxc.h */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.c.h; path = protxc.h; sourceTree = ""; }; 31CD33BB173A9F1500524741 /* mpscams.h */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.c.h; path = mpscams.h; sourceTree = ""; }; @@ -1856,22 +1844,6 @@ ); runOnlyForDeploymentPostprocessing = 0; }; - 2291A5CA175CAFCA001D4920 /* Frameworks */ = { - isa = PBXFrameworksBuildPhase; - buildActionMask = 2147483647; - files = ( - 2291A5CB175CAFCA001D4920 /* libmps.a in Frameworks */, - ); - runOnlyForDeploymentPostprocessing = 0; - }; - 2291A5DC175CB05F001D4920 /* Frameworks */ = { - isa = PBXFrameworksBuildPhase; - buildActionMask = 2147483647; - files = ( - 2291A5DD175CB05F001D4920 /* libmps.a in Frameworks */, - ); - runOnlyForDeploymentPostprocessing = 0; - }; 22B2BC3018B6434F00C33E63 /* Frameworks */ = { isa = PBXFrameworksBuildPhase; buildActionMask = 2147483647; @@ -2134,6 +2106,14 @@ ); runOnlyForDeploymentPostprocessing = 0; }; + 319F7A0D2A30D08500E5B418 /* Frameworks */ = { + isa = PBXFrameworksBuildPhase; + buildActionMask = 2147483647; + files = ( + 319F7A0E2A30D08500E5B418 /* libmps.a in Frameworks */, + ); + runOnlyForDeploymentPostprocessing = 0; + }; 31D6000A156D3CB200337B26 /* Frameworks */ = { isa = PBXFrameworksBuildPhase; buildActionMask = 2147483647; @@ -2340,6 +2320,7 @@ 3124CAB3156BE1B700753214 /* Tests */ = { isa = PBXGroup; children = ( + 319F7A152A30D11400E5B418 /* addrobj.c */, 3114A63D156E94EA001E0AA3 /* abqtest.c */, 22FACED1188807FF000FDBC1 /* airtest.c */, 3124CAF5156BE81100753214 /* amcss.c */, @@ -2485,6 +2466,7 @@ 223E796519EAB00B00DC26A6 /* sncss */, 22EA3F4520D2B0D90065F5B6 /* forktest */, 2265D71D20E53F9C003019E8 /* mpseventpy */, + 319F7A142A30D08500E5B418 /* addrobj */, ); name = Products; sourceTree = ""; @@ -3365,6 +3347,24 @@ productReference = 318DA8CD1892B0F30089718C /* djbench */; productType = "com.apple.product-type.tool"; }; + 319F7A042A30D08500E5B418 /* addrobj */ = { + isa = PBXNativeTarget; + buildConfigurationList = 319F7A102A30D08500E5B418 /* Build configuration list for PBXNativeTarget "addrobj" */; + buildPhases = ( + 319F7A072A30D08500E5B418 /* Sources */, + 319F7A0D2A30D08500E5B418 /* Frameworks */, + 319F7A0F2A30D08500E5B418 /* CopyFiles */, + ); + buildRules = ( + ); + dependencies = ( + 319F7A052A30D08500E5B418 /* PBXTargetDependency */, + ); + name = addrobj; + productName = finalcv; + productReference = 319F7A142A30D08500E5B418 /* addrobj */; + productType = "com.apple.product-type.tool"; + }; 31D6000C156D3CB200337B26 /* awluthe */ = { isa = PBXNativeTarget; buildConfigurationList = 31D60014156D3CB200337B26 /* Build configuration list for PBXNativeTarget "awluthe" */; @@ -3555,6 +3555,7 @@ developmentRegion = English; hasScannedForEncodings = 0; knownRegions = ( + English, en, ); mainGroup = 31EEABD8156AAE9E00714D05; @@ -3617,6 +3618,7 @@ 31FCAE0917692403008C034C /* scheme */, 22B2BC2C18B6434F00C33E63 /* scheme-advanced */, 31108A3A1C6B90E900E728EA /* tagtest */, + 319F7A042A30D08500E5B418 /* addrobj */, ); }; /* End PBXProject section */ @@ -4090,6 +4092,18 @@ ); runOnlyForDeploymentPostprocessing = 0; }; + 319F7A072A30D08500E5B418 /* Sources */ = { + isa = PBXSourcesBuildPhase; + buildActionMask = 2147483647; + files = ( + 319F7A172A30D11400E5B418 /* addrobj.c in Sources */, + 319F7A082A30D08500E5B418 /* testlib.c in Sources */, + 319F7A0A2A30D08500E5B418 /* fmtdy.c in Sources */, + 319F7A0B2A30D08500E5B418 /* fmtdytst.c in Sources */, + 319F7A0C2A30D08500E5B418 /* fmtno.c in Sources */, + ); + runOnlyForDeploymentPostprocessing = 0; + }; 31D60009156D3CB200337B26 /* Sources */ = { isa = PBXSourcesBuildPhase; buildActionMask = 2147483647; @@ -4287,16 +4301,6 @@ target = 2291A5AC175CAB2F001D4920 /* awlutth */; targetProxy = 2291A5BF175CAB5F001D4920 /* PBXContainerItemProxy */; }; - 2291A5C2175CAFCA001D4920 /* PBXTargetDependency */ = { - isa = PBXTargetDependency; - target = 31EEABFA156AAF9D00714D05 /* mps */; - targetProxy = 2291A5C3175CAFCA001D4920 /* PBXContainerItemProxy */; - }; - 2291A5D4175CB05F001D4920 /* PBXTargetDependency */ = { - isa = PBXTargetDependency; - target = 31EEABFA156AAF9D00714D05 /* mps */; - targetProxy = 2291A5D5175CB05F001D4920 /* PBXContainerItemProxy */; - }; 229E228819EAB10D00E21417 /* PBXTargetDependency */ = { isa = PBXTargetDependency; target = 223E795819EAB00B00DC26A6 /* sncss */; @@ -4612,6 +4616,16 @@ target = 31108A3A1C6B90E900E728EA /* tagtest */; targetProxy = 314CB6EA1C6D272A0073CA42 /* PBXContainerItemProxy */; }; + 319F7A052A30D08500E5B418 /* PBXTargetDependency */ = { + isa = PBXTargetDependency; + target = 31EEABFA156AAF9D00714D05 /* mps */; + targetProxy = 319F7A062A30D08500E5B418 /* PBXContainerItemProxy */; + }; + 319F7A192A30D2F000E5B418 /* PBXTargetDependency */ = { + isa = PBXTargetDependency; + target = 319F7A042A30D08500E5B418 /* addrobj */; + targetProxy = 319F7A182A30D2F000E5B418 /* PBXContainerItemProxy */; + }; 31A47BAA156C210D0039B1C2 /* PBXTargetDependency */ = { isa = PBXTargetDependency; target = 31EEABFA156AAF9D00714D05 /* mps */; @@ -4891,34 +4905,6 @@ }; name = Release; }; - 2291A5CE175CAFCA001D4920 /* Debug */ = { - isa = XCBuildConfiguration; - buildSettings = { - PRODUCT_NAME = "$(TARGET_NAME)"; - }; - name = Debug; - }; - 2291A5CF175CAFCA001D4920 /* Release */ = { - isa = XCBuildConfiguration; - buildSettings = { - PRODUCT_NAME = "$(TARGET_NAME)"; - }; - name = Release; - }; - 2291A5E0175CB05F001D4920 /* Debug */ = { - isa = XCBuildConfiguration; - buildSettings = { - PRODUCT_NAME = "$(TARGET_NAME)"; - }; - name = Debug; - }; - 2291A5E1175CB05F001D4920 /* Release */ = { - isa = XCBuildConfiguration; - buildSettings = { - PRODUCT_NAME = "$(TARGET_NAME)"; - }; - name = Release; - }; 22B2BC3318B6434F00C33E63 /* Debug */ = { isa = XCBuildConfiguration; buildSettings = { @@ -5633,20 +5619,6 @@ }; name = RASH; }; - 318DA8E51892C0D00089718C /* RASH */ = { - isa = XCBuildConfiguration; - buildSettings = { - PRODUCT_NAME = "$(TARGET_NAME)"; - }; - name = RASH; - }; - 318DA8E61892C0D00089718C /* RASH */ = { - isa = XCBuildConfiguration; - buildSettings = { - PRODUCT_NAME = "$(TARGET_NAME)"; - }; - name = RASH; - }; 318DA8E71892C0D00089718C /* RASH */ = { isa = XCBuildConfiguration; buildSettings = { @@ -5816,6 +5788,27 @@ }; name = RASH; }; + 319F7A112A30D08500E5B418 /* Debug */ = { + isa = XCBuildConfiguration; + buildSettings = { + PRODUCT_NAME = "$(TARGET_NAME)"; + }; + name = Debug; + }; + 319F7A122A30D08500E5B418 /* Release */ = { + isa = XCBuildConfiguration; + buildSettings = { + PRODUCT_NAME = "$(TARGET_NAME)"; + }; + name = Release; + }; + 319F7A132A30D08500E5B418 /* RASH */ = { + isa = XCBuildConfiguration; + buildSettings = { + PRODUCT_NAME = "$(TARGET_NAME)"; + }; + name = RASH; + }; 31D60015156D3CB200337B26 /* Debug */ = { isa = XCBuildConfiguration; buildSettings = { @@ -6553,6 +6546,16 @@ defaultConfigurationIsVisible = 0; defaultConfigurationName = Release; }; + 319F7A102A30D08500E5B418 /* Build configuration list for PBXNativeTarget "addrobj" */ = { + isa = XCConfigurationList; + buildConfigurations = ( + 319F7A112A30D08500E5B418 /* Debug */, + 319F7A122A30D08500E5B418 /* Release */, + 319F7A132A30D08500E5B418 /* RASH */, + ); + defaultConfigurationIsVisible = 0; + defaultConfigurationName = Release; + }; 31D60014156D3CB200337B26 /* Build configuration list for PBXNativeTarget "awluthe" */ = { isa = XCConfigurationList; buildConfigurations = ( diff --git a/code/mps.xcodeproj/xcshareddata/xcschemes/addrobj.xcscheme b/code/mps.xcodeproj/xcshareddata/xcschemes/addrobj.xcscheme new file mode 100644 index 0000000000..fc6990f251 --- /dev/null +++ b/code/mps.xcodeproj/xcshareddata/xcschemes/addrobj.xcscheme @@ -0,0 +1,78 @@ + + + + + + + + + + + + + + + + + + + + + + + + + + + + + diff --git a/code/mps.xcodeproj/xcshareddata/xcschemes/finaltest.xcscheme b/code/mps.xcodeproj/xcshareddata/xcschemes/finaltest.xcscheme new file mode 100644 index 0000000000..67ef98c430 --- /dev/null +++ b/code/mps.xcodeproj/xcshareddata/xcschemes/finaltest.xcscheme @@ -0,0 +1,78 @@ + + + + + + + + + + + + + + + + + + + + + + + + + + + + + diff --git a/tool/testcases.txt b/tool/testcases.txt index a08d27e259..acdaade286 100644 --- a/tool/testcases.txt +++ b/tool/testcases.txt @@ -2,7 +2,7 @@ Test case Flags Notes ============= ================ ========================================== abqtest -addrobj =W TODO: Enable when we can update Xcode project. See GitHub issue #217 +addrobj airtest amcss =P amcsshe =P From 24915dfc5f98096fcea12bd0f97e3291485993a8 Mon Sep 17 00:00:00 2001 From: Jonathan Holburn Date: Mon, 12 Jun 2023 23:40:10 +0100 Subject: [PATCH 12/17] make various edits from required after proc.review --- code/addrobj.c | 49 ++++++++++++++++++++++++++++++----- code/poolamc.c | 7 +++-- manual/source/topic/arena.rst | 38 ++++++++++++++++++--------- 3 files changed, 73 insertions(+), 21 deletions(-) diff --git a/code/addrobj.c b/code/addrobj.c index eda49fc1ff..8d18432bba 100644 --- a/code/addrobj.c +++ b/code/addrobj.c @@ -11,8 +11,11 @@ * * .limitations Objects that have been moved should cause the function to * fail with MPS_RES_FAIL, however this is not tested. It could be tested if - * there is a way to reliably force the MPS to move a specific object. This - * test only examines behaviour in AMCZ and MVFF pools. + * a testbench deliberately created a forwarding object, however this might + * confuse a pool that does automatic garbage collection such as AMC or AMCZ, + * so any such test would need to be designed to handle that. + * This test only examines behaviour in AMCZ and MVFF pools, i.e. A pool (AMCZ) + * which currently implements mps_addr_object() and one (MVFF) that doesn't. */ #include "mps.h" @@ -24,7 +27,11 @@ #include "mpscmvff.h" #include -/* Define an arbitrarily large object size to allocate */ +/* Define an object size to allocate. The size chosen doesn't matter much, except that this testbench assumes + that the object is large enough that a pointer could point to the interior of the object, without also + pointing to the base pointer of the object at the same time. For char pointers, this is probably 2 bytes. + Since we are using the Dylan library, we define the size of the object in terms of Dylan slots. See + fmtdytst.c for details of the Dylan object structure.*/ #define N_SLOT_TESTOBJ 100 static void test_main(void) @@ -35,6 +42,10 @@ static void test_main(void) mps_fmt_t obj_fmt; mps_root_t testobj_root; mps_res_t res; + /* In another testbench (extcon.c) 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_addr_t testobj; mps_addr_t out, in; @@ -42,12 +53,23 @@ static void test_main(void) die(mps_arena_create_k(&arena, mps_arena_class_vm(), mps_args_none), "mps_arena_create_k"); - /* TEST 1: Test using an interior pointer in an object in an AMCZ pool */ + /* INTRO TO TESTS: There are several tests. They test the expected "normal" operation of the + function, using an interior pointer, also corner cases where the interior pointer equals the + base pointer, where it equals the limit pointer. We also test asking about an address in unmanaged + memory, and about an address in a pool which currently does not support mps_addr_object. If you write + more tests, describe them here.*/ + + + /* TEST 1: Test using an interior pointer in an object in an AMCZ pool. + At the time of writing this test, the AMCZ pool is the only pool where + there exists a requirement to provide base addresses from interior pointers. + Currently, the AMCZ pool (and by extension, the AMC pool which shares the same + module as AMCZ) is the only pool for which mps_addr_object is implemented */ /* Use the dylan format for convenience */ die(dylan_fmt(&obj_fmt, arena), "dylan_fmt"); - /* Create amcz pool */ + /* Create the pool */ MPS_ARGS_BEGIN(args) { MPS_ARGS_ADD(args, MPS_KEY_FORMAT, obj_fmt); die(mps_pool_create_k(&amcz_pool, arena, mps_class_amcz(), args), "mps_pool_create_k amcz"); @@ -64,7 +86,7 @@ static void test_main(void) /* Create the allocation point */ die(mps_ap_create_k(&obj_ap, amcz_pool, mps_args_none), "mps_ap_create_k"); - /* Make an arbitrary sized object, size = (N_SLOT_TESTOBJ+2) * sizeof(mps_word_t). + /* Make a Dylan object, size = (N_SLOT_TESTOBJ+2) * sizeof(mps_word_t). (See fmtdytst.c for size calculation) */ { /* Because make_dylan_vector returns its pointer-to-object as an mps_word_t rather than an @@ -74,6 +96,8 @@ static void test_main(void) park the arena to disable garbage collection. Allocate our dylan object on the (unregistered) stack storing its address in an mps_word_t. Then store this mps_word_t as an mps_addr_t in our reference table, and release the arena since our object is now safely pinned. + Another approach would be to create another static registered root for ambiguous references of type + mps_word_t and then copy to the mps_addr_t root, which would avoid needing to park the arena. */ mps_word_t p_word; mps_arena_park(arena); @@ -86,7 +110,15 @@ static void test_main(void) /* Construct a pointer to roughly halfway inside the object */ in = (mps_addr_t)((char *)testobj + (N_SLOT_TESTOBJ/2) * sizeof(mps_word_t)); - /* Ensure that this is an interior pointer, and not the base pointer */ + /* Ensure that this is an interior pointer, and not the base pointer, + since we want to make sure we are testing with a true interior pointer and not + one that also happens to be the base pointer. This Insist is intended to protect + against the testbench losing its ability to test "true" interior pointers (i.e. ones + which don't match the base pointer) if the test object sizes were changed to be very + small. Note that we don't currently consider the "limit" of the object as a corner case + (so we don't Insist(in != limit) ) but we do consider limit+1, i.e. the pointer to the + next object to be a corner case. This test could be updated to consider in == limit as a + corner case. */ Insist(in > testobj); /* Do Test */ @@ -150,6 +182,9 @@ static void test_main(void) Insist(res == MPS_RES_UNIMPL); + + /* If more tests are added here, briefly describe them above under "INTRO TO TESTS" comment */ + /* Final clean up */ mps_free(mvff_pool, in, sizeof(mps_word_t)); mps_pool_destroy(mvff_pool); diff --git a/code/poolamc.c b/code/poolamc.c index 4455bb9e85..366cc44bbf 100644 --- a/code/poolamc.c +++ b/code/poolamc.c @@ -1893,8 +1893,11 @@ static void amcWalkAll(Pool pool, FormattedObjectsVisitor f, void *p, size_t s) } } -static Res amcAddrObjectSearch(Addr *pReturn, Pool pool, Addr objBase, - Addr searchLimit, Addr addr) +static Res amcAddrObjectSearch(Addr *pReturn, + Pool pool, + Addr objBase, + Addr searchLimit, + Addr addr) { Format format; Size hdrSize; diff --git a/manual/source/topic/arena.rst b/manual/source/topic/arena.rst index b7360e07b2..765a85a7a4 100644 --- a/manual/source/topic/arena.rst +++ b/manual/source/topic/arena.rst @@ -1044,8 +1044,9 @@ Arena introspection and debugging .. c:function:: mps_res_t mps_addr_object(mps_addr_t *p_o, mps_arena_t arena, mps_addr_t addr) - Return the :term:`base pointer` of an :term:`object` from an - :term:`interior pointer` to the object, or the object's base pointer. + Return the :term:`base pointer` of an :term:`object` if provided with an + :term:`interior pointer` to that object, or the object's base pointer, + provided the object exists in a pool that supports this feature. ``p_o`` points to the :term:`address` to which the object's base pointer should be returned. @@ -1054,23 +1055,36 @@ Arena introspection and debugging ``addr`` is an address that might be an interior or base pointer. + Returns MPS_RES_OK if a base pointer to an object into which ``addr`` - points was successfully returned. Returns MPS_RES_FAIL if ``addr`` - points to memory not managed by the ``arena`` or if ``addr`` points - to the interior of an object which has been moved by a :term:`moving memory manager`. - If ``addr`` is found to be managed by a :term:`pool` which does not support this feature, - MPS_RES_FAIL or MPS_RES_UNIMPL may be returned, depending on the :term:`pool class`. + points was successfully returned. + + Returns MPS_RES_FAIL if ``addr`` points to memory not managed by the + ``arena`` or if ``addr`` points to the interior of an object which has + been moved by a :term:`moving memory manager`. + + Returns MPS_RES_UNIMPL if ``addr`` is found to be managed by a :term:`pool` + which does not currently implement this feature. - :c:func:`mps_addr_object` allows a client program that allocates - code on the heap to implement debugging and stack tracing. + :c:func:`mps_addr_object` allows client programs that allocate + code on the heap to implement debugging and stack tracing, in that it provides + a way to unwind a client program's stack by finding the block of code to which the + program counter or function return addresses currently point. It can be called + multiple times as needed to build a complete trace of the client program's stack. - This feature is supported by AMC and AMCZ pools only. + This function does not support debugging in situations where the arena + itself has encountered a runtime error. For cases where the MPS encounters + runtime errors, see :c:func:`mps_arena_postmortem`. .. note:: This function is intended to assist with debugging fatal errors in the :term:`client program`. It is not expected to be - needed in normal use. If you find yourself wanting to use this - function other than in the use case described, there may + needed in normal use, i.e. as part of the regular operation of code in + production, since it is not optimized for performance. If you find yourself + wanting to use this function other than in the use case described, there may be a better way to meet your requirements: please :ref:`contact us `. + + If you would like this function to work in a pool in which it's currently + unimplemented, please :ref:`contact us `. From 932fa9f755bc39a397a0f5d565234db67c328a25 Mon Sep 17 00:00:00 2001 From: Richard Brooksby Date: Thu, 15 Jun 2023 08:55:00 +0100 Subject: [PATCH 13/17] Removing redundant user documentation in response to review --- code/mpsi.c | 13 +++---------- 1 file changed, 3 insertions(+), 10 deletions(-) diff --git a/code/mpsi.c b/code/mpsi.c index 72f65d941a..a3e7dce377 100644 --- a/code/mpsi.c +++ b/code/mpsi.c @@ -447,16 +447,9 @@ mps_bool_t mps_addr_pool(mps_pool_t *mps_pool_o, return b; } -/* mps_addr_object -- find base pointer of a managed object - * - * Sets *p_o to the base address of an object given a pointer to the - * interior of that object. - * - * Will return resFAIL if the object is not in managed memory - * or the object has been moved. - * - * Will return resUNIMPL if the pool doesn't support this feature - */ + +/* mps_addr_object -- find base pointer of a managed object */ + mps_res_t mps_addr_object(mps_addr_t *p_o, mps_arena_t arena, mps_addr_t addr) { Res res; From 160c87da35bce9c6e7f6c83817cae5ab29456027 Mon Sep 17 00:00:00 2001 From: Richard Brooksby Date: Thu, 15 Jun 2023 09:17:44 +0100 Subject: [PATCH 14/17] Removing ambiguous use of "return" to mean passing back a value through an out argument, in response to review . --- manual/source/topic/arena.rst | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/manual/source/topic/arena.rst b/manual/source/topic/arena.rst index 765a85a7a4..77c24f26da 100644 --- a/manual/source/topic/arena.rst +++ b/manual/source/topic/arena.rst @@ -1044,18 +1044,16 @@ Arena introspection and debugging .. c:function:: mps_res_t mps_addr_object(mps_addr_t *p_o, mps_arena_t arena, mps_addr_t addr) - Return the :term:`base pointer` of an :term:`object` if provided with an + Find the :term:`base pointer` of an :term:`object` if provided with an :term:`interior pointer` to that object, or the object's base pointer, provided the object exists in a pool that supports this feature. - ``p_o`` points to the :term:`address` to which the object's base pointer - should be returned. + ``p_o`` points to a location that will hold the object's base pointer. ``arena`` is an arena. ``addr`` is an address that might be an interior or base pointer. - Returns MPS_RES_OK if a base pointer to an object into which ``addr`` points was successfully returned. From c4213cb12099811f7f963cc6636086cc6ba54e98 Mon Sep 17 00:00:00 2001 From: Richard Brooksby Date: Thu, 15 Jun 2023 09:21:55 +0100 Subject: [PATCH 15/17] Improving comments on AMCAddrObject in response to review . --- code/poolamc.c | 13 ++++++++++--- 1 file changed, 10 insertions(+), 3 deletions(-) diff --git a/code/poolamc.c b/code/poolamc.c index 366cc44bbf..4e6267f773 100644 --- a/code/poolamc.c +++ b/code/poolamc.c @@ -1893,6 +1893,16 @@ static void amcWalkAll(Pool pool, FormattedObjectsVisitor f, void *p, size_t s) } } +/* AMCAddrObject -- return base pointer from interior pointer + * + * amcAddrObjectSearch implements the scan for an object containing + * the interior pointer by skipping using format methods. + * + * AMCAddrObject locates the segment containing the interior pointer + * and wraps amcAddrObjectSearch in the necessary shield operations to + * give it access. + */ + static Res amcAddrObjectSearch(Addr *pReturn, Pool pool, Addr objBase, @@ -1929,9 +1939,6 @@ static Res amcAddrObjectSearch(Addr *pReturn, return ResFAIL; } - -/* AMCAddrObject -- return base pointer from interior pointer */ - static Res AMCAddrObject(Addr *pReturn, Pool pool, Addr addr) { Res res; From 4f3e8db94724c1d999a4cbc7c56a7bd765d15184 Mon Sep 17 00:00:00 2001 From: Jonathan Holburn Date: Thu, 15 Jun 2023 11:51:17 +0100 Subject: [PATCH 16/17] reintroduce comments that went missing during the original work to port this functionality from Configura's mps-preview --- code/mpsi.c | 12 +++++++++++- code/poolamc.c | 11 +++++++++++ 2 files changed, 22 insertions(+), 1 deletion(-) diff --git a/code/mpsi.c b/code/mpsi.c index a3e7dce377..a2fe146159 100644 --- a/code/mpsi.c +++ b/code/mpsi.c @@ -457,11 +457,21 @@ mps_res_t mps_addr_object(mps_addr_t *p_o, mps_arena_t arena, mps_addr_t addr) AVER(p_o != NULL); + /* This function cannot be called while walking the heap, unlike + * mps_arena_has_addr(). This is because it is designed to be called + * with an active mutator, so takes the arena lock. This is in order + * that it sees a consistent view of MPS structures and the heap, + * and can peek behind the barrier. + */ ArenaEnter(arena); AVERT(Arena, arena); res = ArenaAddrObject(&p, arena, (Addr)addr); ArenaLeave(arena); - + /* We require the object to be ambiguously referenced (hence pinned) + * so that p doesn't become invalid before it is written to *p_o. + * (We can't simply put this write before the ArenaLeave(), because + * p_o could point to MPS-managed memory that is behind a barrier.) + */ if (res == ResOK) *p_o = (mps_addr_t)p; diff --git a/code/poolamc.c b/code/poolamc.c index 4e6267f773..1db13147fe 100644 --- a/code/poolamc.c +++ b/code/poolamc.c @@ -1956,6 +1956,17 @@ static Res AMCAddrObject(Addr *pReturn, Pool pool, Addr addr) base = SegBase(seg); if (SegBuffer(&buffer, seg)) + /* We use BufferGetInit here (and not BufferScanLimit) because we + * want to be able to find objects that have been allocated and + * committed since the last flip. These objects lie between the + * addresses returned by BufferScanLimit (which returns the value + * of init at the last flip) and BufferGetInit. + * + * Strictly speaking we only need a limit that is at least the + * maximum of the objects on the segments. This is because addr + * *must* point inside a live object and we stop skipping once we + * have found it. The init pointer serves this purpose. + */ limit = BufferGetInit(buffer); else limit = SegLimit(seg); From 49568611210421816ddd09744fc321b91b841dcd Mon Sep 17 00:00:00 2001 From: Jonathan Holburn Date: Thu, 15 Jun 2023 12:06:18 +0100 Subject: [PATCH 17/17] make addrobj.c more verbose --- code/addrobj.c | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/code/addrobj.c b/code/addrobj.c index 8d18432bba..f4fe524771 100644 --- a/code/addrobj.c +++ b/code/addrobj.c @@ -25,6 +25,7 @@ #include "mpsavm.h" #include "mpscamc.h" #include "mpscmvff.h" +#include "stdio.h" #include /* Define an object size to allocate. The size chosen doesn't matter much, except that this testbench assumes @@ -125,6 +126,7 @@ static void test_main(void) res = mps_addr_object(&out, arena, in); Insist(out == testobj); Insist(res == MPS_RES_OK); + printf("Interior pointer input: passed\n"); /* TEST 2: Test using the base pointer itself as an input*/ @@ -135,6 +137,8 @@ static void test_main(void) res = mps_addr_object(&out, arena, in); Insist(out == testobj); Insist(res == MPS_RES_OK); + printf("Base pointer input: passed\n"); + /* TEST 3: Test using a pointer one-off-the-end of the object*/ @@ -144,6 +148,8 @@ static void test_main(void) /* Do Test */ res = mps_addr_object(&out, arena, in); Insist(res == MPS_RES_FAIL); + printf("Pointer to next object input: passed\n"); + /* Clean up from above tests */ mps_root_destroy(testobj_root); @@ -163,6 +169,7 @@ static void test_main(void) /* Expect MPS to fail to find a base pointer for addresses not in managed memory */ Insist(res == MPS_RES_FAIL); + printf("Pointer to unmanaged memory input: passed\n"); /* clean up from this test */ if (NULL != in) @@ -181,6 +188,7 @@ static void test_main(void) res = mps_addr_object(&out, arena, in); Insist(res == MPS_RES_UNIMPL); + printf("Pointer to object in pool where mps_addr_object not implemented: passed\n"); /* If more tests are added here, briefly describe them above under "INTRO TO TESTS" comment */ @@ -197,6 +205,8 @@ int main(int argc, char *argv[]) test_main(); + printf("%s: Conculsion, failed to find any defects.\n", argv[0]); + return 0; }