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.

Sign up for GitHub

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

Jump to bottom

Use __heap_base by dlmalloc #114

Merged
merged 10 commits into from
Nov 5, 2019
Merged

Use __heap_base by dlmalloc #114

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
10 commits
Select commit Hold shift + click to select a range
aa698a1
Merge pull request #1 from CraneStation/master
mikevoronov Oct 18, 2019
93476e4
add explicit allocator initialization
mikevoronov Oct 19, 2019
5b72609
move init to a better place
mikevoronov Oct 19, 2019
72a6db1
fix warnings
mikevoronov Oct 19, 2019
7d1e5ab
add __wasilibc_try_init_allocator
mikevoronov Oct 19, 2019
8dbe4e2
move initialization to dlmalloc()
mikevoronov Oct 20, 2019
403d4a9
fix typos
mikevoronov Oct 20, 2019
f858b48
fix identations
mikevoronov Oct 20, 2019
2e4eee3
follow style guide
mikevoronov Oct 21, 2019
5e70f07
review changes
mikevoronov Oct 24, 2019
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
48 changes: 48 additions & 0 deletions dlmalloc/src/malloc.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4560,6 +4560,11 @@ static void* tmalloc_small(mstate m, size_t nb) {

#if !ONLY_MSPACES

#if __wasilibc_unmodified_upstream // Forward declaration of try_init_allocator.
#else
static void try_init_allocator(void);
#endif

void* dlmalloc(size_t bytes) {
/*
Basic algorithm:
Expand Down Expand Up @@ -4588,6 +4593,13 @@ void* dlmalloc(size_t bytes) {
ensure_initialization(); /* initialize in sys_alloc if not using locks */
#endif

#if __wasilibc_unmodified_upstream // Try to initialize the allocator.
#else
if (!is_initialized(gm)) {
try_init_allocator();
}
#endif
Copy link
Member

Choose a reason for hiding this comment

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

Would it work to call this from within init_mparams instead? That way it'd get called for all entrypoints, not just malloc.

In that case, it shouldn't do ensure_initialization itself, but just do its work after init_mparams has done its work.

Copy link
Contributor Author

@mikevoronov mikevoronov Oct 24, 2019 •
edited
Loading

Choose a reason for hiding this comment

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

Hmmm, I've though about it, but there are some clues:

  1. ensure_initialization is used for mspace also (mspace is like arena in ptmalloc for different continuous space of allocations). At now, mspace is disabled, but if someone want to enable it, there will some unobvious problems. And it is logically incorrect.
  2. Currently, ensure_initialization is disabled in dlmalloc, because it is inside #if USE_LOCKS that is also disabled. And it is called into sys_alloc. But for our purpose it is important to have it initialized before main logic of chunk choosing in dlmalloc being called. => in case of ensure_initialization we will need two augmentation (into ensure_initialization and dlmalloc) instead of one in current case.
  3. On the other side this initialization into dlmalloc seems correct, because other entry points (realloc, calloc) in all important CFG paths will call it before any chunk manipulation. The only exception here is free, but it is UB to call free without any previous malloc.

So, yes, it possible to move top chunk initialization into ensure_initialization, but it seems that it is better leave it as now.

What do you think?

Copy link
Member

Choose a reason for hiding this comment

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

Thanks for looking into that! I think what you have right now looks like a reasonable approach then.


if (!PREACTION(gm)) {
void* mem;
size_t nb;
Expand Down Expand Up @@ -5197,6 +5209,42 @@ static void internal_inspect_all(mstate m,
}
#endif /* MALLOC_INSPECT_ALL */

#ifdef __wasilibc_unmodified_upstream // Define a function that initializes the initial state of dlmalloc
#else
mikevoronov marked this conversation as resolved.
Show resolved Hide resolved
/* ------------------ Exported try_init_allocator -------------------- */

/* Symbol marking the end of data, bss and explicit stack, provided by wasm-ld. */
extern unsigned char __heap_base;

/* Initialize the initial state of dlmalloc to be able to use free memory between __heap_base and initial. */
static void try_init_allocator(void) {
/* Check that it is a first-time initialization. */
assert(!is_initialized(gm));

char *base = (char *)&__heap_base;
/* Calls sbrk(0) that returns the initial memory position. */
char *init = (char *)CALL_MORECORE(0);
int initial_heap_size = init - base;

/* Check that initial heap is long enough to serve a minimal allocation request. */
if (initial_heap_size <= MIN_CHUNK_SIZE + TOP_FOOT_SIZE + MALLOC_ALIGNMENT) {
return;
}

/* Initialize mstate. */
ensure_initialization();

/* Initialize the dlmalloc internal state. */
gm->least_addr = base;
gm->seg.base = base;
gm->seg.size = initial_heap_size;
gm->magic = mparams.magic;
gm->release_checks = MAX_RELEASE_CHECK_RATE;
init_bins(gm);
init_top(gm, (mchunkptr)base, initial_heap_size - TOP_FOOT_SIZE);
}
#endif

/* ------------------ Exported realloc, memalign, etc -------------------- */

#if !ONLY_MSPACES
Expand Down
1 change: 1 addition & 0 deletions expected/wasm32-wasi/undefined-symbols.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ __floatsitf
__floatunsitf
__getf2
__gttf2
__heap_base
__letf2
__lttf2
__netf2
Expand Down