Skip to content
This repository has been archived by the owner on Oct 12, 2022. It is now read-only.

GC debug fixes #1197

Merged
merged 4 commits into from Apr 17, 2015
Merged

Conversation

CyberShadow
Copy link
Member

No description provided.

@CyberShadow
Copy link
Member Author

Not sure what's wrong with the rt.lifetime unittests. Ping @rainers @Orvid

@CyberShadow CyberShadow force-pushed the pull-20150323-233811-gc-debug branch 2 times, most recently from a3b73f4 to f901b5f Compare March 24, 2015 00:58
// Note: Some of these need to be set globally
// (for the entire Druntime) in the Makefile
// instead of uncommenting the lines here.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

AFAICT it's only SENTINEL that needs to be set globally to disable some unittests.

@rainers
Copy link
Member

rainers commented Mar 24, 2015

Not sure what's wrong with the rt.lifetime unittests.

I don't see something obvious. Do you have more info on where the tests are failing?

@CyberShadow
Copy link
Member Author

The ones I disabled in 03ab3d6.

@CyberShadow CyberShadow force-pushed the pull-20150323-233811-gc-debug branch from f901b5f to c73e6b7 Compare March 24, 2015 11:48
@MartinNowak
Copy link
Member

  • We should restructure those debug tools as wrappers instead of polluting the GC code.
  • The code in rt.lifetime is too specifically written for our current GC, maybe we need to move some functionality inside the GC.
  • It's possible to use valgrind with gcstub to debug memory corruptions.
  • Don't know why debug STOMP doesn't work with struct finalizers.

@CyberShadow
Copy link
Member Author

It's possible to use valgrind with gcstub to debug memory corruptions.

Is there more information about this? I'm currently debugging memory corruption and was thinking about trying Valgrind.

However gcstub won't help find use-after-free bugs in cases where the GC incorrectly collected something.

Don't know why debug STOMP doesn't work with struct finalizers.

The problem is with SENTINEL, not MEMSTOMP, so probably an alignment issue somewhere.

@CyberShadow
Copy link
Member Author

I'm currently debugging memory corruption and was thinking about trying Valgrind.

I've got something working here:

CyberShadow/druntime@pull-20150323-233811-gc-debug...valgrind

@MartinNowak
Copy link
Member

Pretty interesting.

@MartinNowak
Copy link
Member

However gcstub won't help find use-after-free bugs in cases where the GC incorrectly collected something.

Works, because gcstub never collects, that's a problem on it's own though.

@CyberShadow
Copy link
Member Author

No, that's not the problem I'm talking about.

Say you allocated an object X, and stored the only pointer somewhere the GC can't see it. The GC collects it and allocates a new object at the same address. The code that then attempts to refer to X causes memory corruption.

This is a nontrivial problem even with Valgrind integration. What I'd need to do is make the GC collect objects (and stomp on freed memory), but instead of reusing the memory, keep allocating new memory, so that dangling pointers cause Valgrind errors. Any advice on the easiest way to do this?

@MartinNowak
Copy link
Member

Just comment out the cleaning of the free bits.

@rainers
Copy link
Member

rainers commented Mar 27, 2015

Not sure what's wrong with the rt.lifetime unittests.

The runFinalizer functions pass the wrong size to the rt_hasFinalizerInSegment and rt_finalizeFromGC functions. It should be something like this:

            debug(SENTINEL)
                immutable qsize = *sentinel_size(p);
            else
                immutable qsize = size;
            if(!rt_hasFinalizerInSegment(p, qsize, attr, segment))
                continue;

            rt_finalizeFromGC(p, qsize, attr);

(replace p with q for the small object pool).

@@ -23,6 +23,8 @@ module gc.gc;
//debug = LOGGING; // log allocations / frees
//debug = MEMSTOMP; // stomp on memory
//debug = SENTINEL; // add underrun/overrrun protection
// note: this needs to be enabled globally in the makefiles
// (-debug=SENTINEL) instead of uncommented here.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think you should restrict the comment to building the unittests, e.g. ".. when building druntime unittests".

@MartinNowak MartinNowak merged commit c73e6b7 into dlang:master Apr 17, 2015
MartinNowak added a commit that referenced this pull request Apr 17, 2015
@MartinNowak
Copy link
Member

Please fixup the finalizer if you find time @CyberShadow.

@CyberShadow
Copy link
Member Author

Sorry, forgot about this pull. Will do.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
3 participants