Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

WIP/RFC: stack scrubbing #12317

Merged
merged 2 commits into from
Aug 12, 2015
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
51 changes: 51 additions & 0 deletions src/gc-debug.c
Original file line number Diff line number Diff line change
@@ -1,5 +1,30 @@
// This file is a part of Julia. License is MIT: http://julialang.org/license

// Find the memory block in the pool that owns the byte pointed to by p.
// For end of object pointer (which is always the case for pointer to a
// singleton object), this usually returns the same pointer which points to
// the next object but it can also return NULL if the pointer is pointing to
// the end of the page.
DLLEXPORT jl_taggedvalue_t *jl_gc_find_taggedvalue_pool(char *p,
size_t *osize_p)
{
region_t *r = find_region(p, 1);
if (!r)
return NULL;
char *page_begin = GC_PAGE_DATA(p) + GC_PAGE_OFFSET;
if (p < page_begin)
return NULL;
size_t ofs = p - page_begin;
int pg_idx = PAGE_INDEX(r, p);
gcpage_t *pagemeta = &r->meta[pg_idx];
int osize = pagemeta->osize;
if (osize == 0)
return NULL;
if (osize_p)
*osize_p = osize;
return (jl_taggedvalue_t*)((char*)p - (ofs % osize));
}

#ifdef GC_DEBUG_ENV
#include <inttypes.h>
#include <stdio.h>
Expand Down Expand Up @@ -262,8 +287,10 @@ static int gc_debug_alloc_check(jl_alloc_num_t *num)
return ((num->num - num->min) % num->interv) == 0;
}

static char *gc_stack_lo;
static void gc_debug_init()
{
gc_stack_lo = (char*)gc_get_stack_ptr();
char *env = getenv("JL_GC_NO_GENERATIONAL");
if (env && strcmp(env, "0") != 0) {
gc_debug_env.sweep_mask = GC_MARKED;
Expand Down Expand Up @@ -301,6 +328,25 @@ static inline void gc_debug_print()
gc_debug_print_status();
}

static void gc_scrub_range(char *stack_lo, char *stack_hi)
{
stack_lo = (char*)((uintptr_t)stack_lo & ~(uintptr_t)15);
for (char **stack_p = (char**)stack_lo;
stack_p > (char**)stack_hi;stack_p--) {
char *p = *stack_p;
size_t osize;
jl_taggedvalue_t *tag = jl_gc_find_taggedvalue_pool(p, &osize);
if (!tag || gc_marked(tag) || osize <= sizeof_jl_taggedvalue_t)
continue;
memset(tag, 0xff, osize);
}
}

static void gc_scrub(char *stack_hi)
{
gc_scrub_range(gc_stack_lo, stack_hi);
}

#else

static inline int gc_debug_check_other()
Expand All @@ -321,4 +367,9 @@ static inline void gc_debug_init()
{
}

static void gc_scrub(char *stack_hi)
{
(void)stack_hi;
}

#endif
28 changes: 23 additions & 5 deletions src/gc.c
Original file line number Diff line number Diff line change
Expand Up @@ -259,6 +259,20 @@ static int check_timeout = 0;
static gcpage_t *page_metadata(void *data);
static void pre_mark(void);
static void post_mark(arraylist_t *list, int dryrun);
static region_t *find_region(void *ptr, int maybe);
jl_taggedvalue_t *jl_gc_find_taggedvalue_pool(char *p, size_t *osize_p);
Copy link
Contributor

Choose a reason for hiding this comment

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

For MSVC, you need DLLEXPORT to be consistent between the prototype and the implementation. It should either be on both or neither. Which should this be?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Export for debugging purpose THX.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I'm not by my computer now, will fix it later. Feel free to do it if you need to.

Copy link
Contributor

Choose a reason for hiding this comment

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

It's okay, just came up in #12056. We don't do the MSVC build on CI right now, so just trying to remind people who touch the C code of some of its constraints so hopefully it breaks a little less often.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@tkelman Should be fixed by 24f6647 . I'll push to master after the branch travis passes.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@tkelman Merged. Please let me know if there's still an issue.


#define PAGE_INDEX(region, data) \
((GC_PAGE_DATA((data) - GC_PAGE_OFFSET) - \
&(region)->pages[0][0])/GC_PAGE_SZ)

NOINLINE static uintptr_t gc_get_stack_ptr()
{
void *dummy = NULL;
// The mask is to suppress the compiler warning about returning
// address of local variable
return (uintptr_t)&dummy & ~(uintptr_t)15;
}

#include "gc-debug.c"

Expand Down Expand Up @@ -391,21 +405,23 @@ void jl_finalize(jl_value_t *o)
(void)finalize_object(o);
}

#define PAGE_INDEX(region, data) ((GC_PAGE_DATA((data) - GC_PAGE_OFFSET) - &(region)->pages[0][0])/GC_PAGE_SZ)
static region_t *find_region(void *ptr)
static region_t *find_region(void *ptr, int maybe)
{
// on 64bit systems we could probably use a single region and remove this loop
for (int i = 0; i < REGION_COUNT && regions[i]; i++) {
if ((char*)ptr >= (char*)regions[i] && (char*)ptr <= (char*)regions[i] + sizeof(region_t))
char *begin = &regions[i]->pages[0][0];
char *end = begin + sizeof(regions[i]->pages);
if ((char*)ptr >= begin && (char*)ptr <= end)
return regions[i];
}
assert(0 && "find_region failed");
(void)maybe;
assert(maybe && "find_region failed");
return NULL;
}

static gcpage_t *page_metadata(void *data)
{
region_t *r = find_region(data);
region_t *r = find_region(data, 0);
int pg_idx = PAGE_INDEX(r, (char*)data);
return &r->meta[pg_idx];
}
Expand Down Expand Up @@ -1969,6 +1985,7 @@ void jl_gc_collect(int full)
{
if (!is_gc_enabled) return;
if (jl_in_gc) return;
char *stack_hi = (char*)gc_get_stack_ptr();
gc_debug_print();
JL_SIGATOMIC_BEGIN();
jl_in_gc = 1;
Expand Down Expand Up @@ -2070,6 +2087,7 @@ void jl_gc_collect(int full)
#endif
estimate_freed = live_bytes - scanned_bytes - perm_scanned_bytes + actual_allocd;

gc_scrub(stack_hi);
gc_verify();

#if defined(MEMPROFILE)
Expand Down