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

Port the fix from #377 to emmmalloc. #378

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

sunfishcode
Copy link
Member

Avoid using sbrk(0) in emmalloc too.

Avoid using sbrk(0) in emmalloc too.
@jedisct1
Copy link
Member

jedisct1 commented Jan 9, 2023

Unfortunately this is causing applications to trap with an out of bounds access.

@sunfishcode
Copy link
Member Author

Ah, that's unfortunate. Any help anyone might be able to provide in debugging this would be appreciated!

@sbc100
Copy link
Member

sbc100 commented Jan 9, 2023

Unfortunately this is causing applications to trap with an out of bounds access.

Is that with or without the __heap_base symbol? i.e. which llvm version are you using?

@jedisct1
Copy link
Member

jedisct1 commented Jan 9, 2023

This is with LLVM 15.0.6.

@jedisct1
Copy link
Member

jedisct1 commented Jan 9, 2023

__heap_base is defined but not __heap_end.

@cuviper
Copy link
Contributor

cuviper commented Jun 15, 2023

(irrelevant nit: the commit message and title of this PR have too many 'm's)

@@ -56,6 +56,7 @@

// Defind by the linker to have the address of the start of the heap.
extern unsigned char __heap_base;
extern unsigned char __heap_end __attribute__((__weak__));
Copy link
Contributor

Choose a reason for hiding this comment

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

Since #394, this should no longer be weak.

In fact, building with MALLOC_IMPL=emmalloc is currently broken due to missing this symbol.

unsigned char *heap_end = sbrk(0);
unsigned char *heap_end = &__heap_end;
if (heap_end == NULL)
heap_end = (unsigned char*) ((size_t) &__heap_base + (PAGE_SIZE-1) & -PAGE_SIZE);
Copy link
Contributor

Choose a reason for hiding this comment

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

Once non-weak, the NULL fallback is not needed, but the dlmalloc code is trapping on end < base.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants