Using C++ interface, prematurely cleans up arrays.

[comeradealexi]

Ran into the following issue below in our big project which I've replicated below.

I've replicated the issue below on x64 Windows & Xbox One. On Windows it's the latest 2015 compiler.

From what I can tell, it appears that the call to new[] allocates 8 extra bytes than actually requested, which I assume is the OS but the GC seems to be aware of the memory location before the OS offsets it so the pointer value stored is ultimately not the one the GC is aware of so it gets cleaned up prematurely.

Running the code also highlights another issue where by only a single destructor is ever called, despite 1024 instances being created.

Is this expected behaviour? It's a bit misleading, especially the fact the new'd array is cleaned up straight away.

Thanks.

#include <gc_cpp.h>
#include <stdio.h>

struct CleanupClass : public gc_cleanup
{
    CleanupClass()
    {
        printf("CONST\n");
    }
    ~CleanupClass()
    {
        printf("DEST\n");
    }
};

void* pData = nullptr;
void ALloc()
{
    auto asdasd = new CleanupClass[1024];
    pData = asdasd;
}

int main()
{
    ALloc();

    for (size_t i = 0; i < 20; i++)
    {
        GC_gcollect();
    }

    printf("Nothing should have been cleaned up\n");
}

[hboehm]

GC_init in GC_misc.c probably needs a call GC_register_displacement_inner() call on this platform.

The GC_register_displacement_inner calls in dbg_mlc.c also probably need a second corresponding variant that adds this offset.

On Mon, May 8, 2017 at 5:46 AM, Alex Houghton notifications@github.com wrote:

Ran into the following issue below in our big project which I've replicated below.

I've replicated the issue below on x64 Windows & Xbox One. On Windows it's the latest 2015 compiler.

From what I can tell, it appears that the call to new[] allocates 8 extra bytes than actually requested, which I assume is the OS but the GC seems to be aware of the memory location before the OS offsets it so the pointer value stored is ultimately not the one the GC is aware of so it gets cleaned up prematurely.

Running the code also highlights another issue where by only a single destructor is ever called, despite 1024 instances being created.

Is this expected behaviour? It's a bit misleading, especially the fact the new'd array is cleaned up straight away.

Thanks.

`#include

include

struct CleanupClass : public gc_cleanup { CleanupClass() { printf("CONST\n"); } ~CleanupClass() { printf("DEST\n"); } };

void* pData = nullptr; void ALloc() { auto asdasd = new CleanupClass[1024]; pData = asdasd; }

int main() { ALloc();

for (size_t i = 0; i < 20; i++) { GC_gcollect(); }

printf("Nothing should have been cleaned up\n");

}`

— You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub https://github.com/ivmai/bdwgc/issues/160, or mute the thread https://github.com/notifications/unsubscribe-auth/AA9DqvKa8RJuy_qZgaN-FFKgsuug8OpXks5r3w6cgaJpZM4NT5dz .

[comeradealexi]

Hi Hans, thanks for the reply!

I've modified the register displacement inner function to the following which fixes the issue for arrays but now breaks it for any other type of allocation because the offset is only needed for array allocations...

This is what I did to get it working for arrays with and without GC_DEBUG defined:

GC_INNER void GC_register_displacement_inner(size_t offset)
{
#ifdef _WIN64
    offset += 8;
#elif defined(_WIN32)
    offset += 4;
#else
#error Add Support For This Platform: Array Offset
#endif

    if (offset >= VALID_OFFSET_SZ) {
        ABORT("Bad argument to GC_register_displacement");
    }
    if (!GC_valid_offsets[offset]) {
      GC_valid_offsets[offset] = TRUE;
      GC_modws_valid_offsets[offset % sizeof(word)] = TRUE;
    }
}

So in debug this adds to the value set in dbg_mlc.c and in non-debug it just adds to the 0 passed through in gc_init which is already there.

Is there a way to specify an offset just for array types?

I've modified my test code to test array and non-array allocations:

#include <gc_cpp.h>
#include <stdio.h>

struct CleanupClass : public gc_cleanup
{
    ~CleanupClass()
    {
        printf("DEST: %s\n", pType);
    }
    const char* pType = "Array";
};

void* pData = nullptr;
CleanupClass* pCleanup = nullptr;
void ALloc()
{
    auto asdasd = new CleanupClass[1024];
    pData = asdasd;
    pCleanup = new CleanupClass();
    pCleanup->pType = "Single";
}

int main()
{
    //GC_INIT();
    ALloc();

    for (size_t i = 0; i < 20; i++)
    {
        GC_gcollect();
    }

    printf("Nothing should have been cleaned up\n");
}

So with the modification made the above code prints:

DEST: Single
Nothing should have been cleaned up

whereas previously it would have printed:

DEST: Array
Nothing should have been cleaned up

Alex

[hboehm]

Sorry. I didn't mean to suggest modifying GC_register_displacement_inner itself. Just add additional calls to this function from GC_init to register the additional object offsets you need recognized. It's fine to call this function repeatedly with multiple offsets to allow for pointers at differing offsets into the object.

On Mon, May 8, 2017 at 3:43 PM, Alex Houghton notifications@github.com wrote:

Hi Hans, thanks for the reply!

I've modified the register displacement inner function to the following which fixes the issue for arrays but now breaks it for any other type of allocation because the offset is only needed for array allocations...

This is what I did to get it working for arrays with and without GC_DEBUG defined:

GC_INNER void GC_register_displacement_inner(size_t offset) {

ifdef _WIN64

offset += 8;

elif defined(_WIN32)

offset += 4;

else

error Add Support For This Platform: Array Offset

endif

if (offset >= VALID_OFFSET_SZ) {
    ABORT("Bad argument to GC_register_displacement");
}
if (!GC_valid_offsets[offset]) {
  GC_valid_offsets[offset] = TRUE;
  GC_modws_valid_offsets[offset % sizeof(word)] = TRUE;
}

}

So in debug this adds to the value set in dbg_mlc.c and in non-debug it just adds to the 0 passed through in gc_init which is already there.

Is there a way to specify an offset just for array types?

I've modified my test code to test array and non-array allocations:

include

include

struct CleanupClass : public gc_cleanup { ~CleanupClass() { printf("DEST: %s\n", pType); } const char* pType = "Array"; };

void pData = nullptr; CleanupClass pCleanup = nullptr; void ALloc() { auto asdasd = new CleanupClass[1024]; pData = asdasd; pCleanup = new CleanupClass(); pCleanup->pType = "Single"; }

int main() { //GC_INIT(); ALloc();

for (size_t i = 0; i < 20; i++) { GC_gcollect(); }

printf("Nothing should have been cleaned up\n");

}

Alex

— You are receiving this because you commented. Reply to this email directly, view it on GitHub https://github.com/ivmai/bdwgc/issues/160#issuecomment-300011529, or mute the thread https://github.com/notifications/unsubscribe-auth/AA9DqqTTTLBVc9w_TZOdESgecddp9Itmks5r35qOgaJpZM4NT5dz .

[comeradealexi]

Ah I see what you mean now, that's done the trick, so I've got the following code now which works in GC_init.

    GC_register_displacement_inner(0L);
#ifdef _WIN64
#ifdef GC_DEBUG
    GC_register_displacement_inner(40L);
#else
    GC_register_displacement_inner(8L);
#endif
#elif defined(_WIN32)
#ifdef GC_DEBUG
    GC_register_displacement_inner(20L);
#else
    GC_register_displacement_inner(4L);
#endif
#endif

Thanks for your help, I've just got a couple follow up questions:

Is this something which should be added to the project since by default the GC won't be cleaning up arrays properly (at least on Windows).

Also, regarding the other point I mentioned about the destructors, even though a full array of objects is new'd and constructed, the GC will only ever call the destructor for the first element in the array, even though the entire memory is freed. Is this expected behaviour?

Alex

[comeradealexi]

Delved further into the documentation for gc_cleanup and found the answer to the array destructor issue:

3. The destructors of collectible arrays of objects derived from
"gc_cleanup" will not be invoked properly.  For example:

    class A: public gc_cleanup {...};
    A* a = new (GC) A[ 10 ];    // destructors not invoked correctly

Typically, only the destructor for the first element of the array will
be invoked when the array is garbage-collected.  To get all the
destructors of any array executed, you must supply an explicit
clean-up function:

    A* a = new (GC, MyCleanUp) A[ 10 ];

(Implementing clean-up of arrays correctly, portably, and in a way
that preserves the correct exception semantics requires a language
extension, e.g. the "gc" keyword.)

Thanks for all the assistance @hboehm .

Alex