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

Fix crash in GC_scratch_alloc under low-memory conditions #36

Closed
wants to merge 1 commit into from

Conversation

wingo
Copy link
Contributor

@wingo wingo commented Mar 22, 2014

  • headers.c (GC_scratch_alloc): Always round bytes_to_get up to a
    multiple of the page size, even if USE_MMAP is not defined. This is
    necessary because GC_unix_get_mem, which is one of the possible
    GET_MEM implementations, may use mmap even if USE_MMAP isn't defined.
    See os_dep.c for a definition of GC_unix_get_mem which tries sbrk
    first, but then falls back to mmap. Calling GC_unix_mmap_get_mem will
    abort if the allocation is not a multiple of the page size.

Here's an example stack trace of the error without this patch:

GC Warning: Out of memory - trying to allocate less
Bad GET_MEM arg

Program received signal SIGABRT, Aborted.
0x00007ffff6fd83a9 in __GI_raise (sig=sig@entry=6) at ../nptl/sysdeps/unix/sysv/linux/raise.c:56
56 ../nptl/sysdeps/unix/sysv/linux/raise.c: No such file or directory.
(gdb) bt
#0 0x00007ffff6fd83a9 in __GI_raise (sig=sig@entry=6) at ../nptl/sysdeps/unix/sysv/linux/raise.c:56
#1 0x00007ffff6fdb4c8 in __GI_abort () at abort.c:89
#2 0x00007ffff6d50009 in GC_unix_mmap_get_mem (bytes=bytes@entry=336) at os_dep.c:2065
#3 0x00007ffff6d500e0 in GC_unix_get_mem (bytes=bytes@entry=336) at os_dep.c:2152
#4 0x00007ffff6d483ef in GC_scratch_alloc (bytes=336, bytes@entry=328) at headers.c:154
#5 0x00007ffff6d4863f in alloc_hdr () at headers.c:173
#6 GC_install_header (h=h@entry=0x7ffff2413000) at headers.c:266
#7 0x00007ffff6d41ae0 in GC_get_first_part (h=h@entry=0x7ffff2412000, hhdr=hhdr@entry=0x7ffff1dbcea0, bytes=bytes@entry=4096, index=index@entry=60)

at allchblk.c:500

#8 0x00007ffff6d41e0a in GC_allochblk_nth (sz=sz@entry=16, kind=kind@entry=1, flags=flags@entry=0, n=n@entry=60, may_split=may_split@entry=1)

at allchblk.c:783

#9 0x00007ffff6d421c6 in GC_allochblk (sz=sz@entry=16, kind=kind@entry=1, flags=flags@entry=0) at allchblk.c:624
#10 0x00007ffff6d49d85 in GC_generic_malloc_many (lb=16, k=1, result=0x68ff00) at mallocx.c:404

* headers.c (GC_scratch_alloc): Always round bytes_to_get up to a
  multiple of the page size, even if USE_MMAP is not defined.  This is
  necessary because GC_unix_get_mem, which is one of the possible
  GET_MEM implementations, may use mmap even if USE_MMAP isn't defined.
  See os_dep.c for a definition of GC_unix_get_mem which tries sbrk
  first, but then falls back to mmap.  Calling GC_unix_mmap_get_mem will
  abort if the allocation is not a multiple of the page size.
@ivmai
Copy link
Owner

ivmai commented Mar 22, 2014

Thanks. I discovered this bug 2 days ago and decided to develop a fix on Sunday but you did it faster.

But I think your solution should be improved - GC_unix_mmap_get_mem is defined only if MMAP_SUPPORTED (thus there is no need for alignment otherwise).

Plus, there are other uses of GET_MEM (in backgraph and misc.c) - they should be handled in the similar way, I suppose.

@ivmai
Copy link
Owner

ivmai commented Mar 30, 2014

I did the fix in other (more general) way - see commit 180b56c (it also fixes absence of rounding in backgraph.c). Please test master branch. After that, I'll cherry-pick this commit to release-7_2/7_4 branches.

@ivmai ivmai closed this Mar 30, 2014
@wingo
Copy link
Contributor Author

wingo commented Apr 2, 2014

Makes sense to me. The test case was hard for me to hit, so I can't reproduce right now. That said, I have had reports of this happening to other people; I'll be sure to report again if something goes wrong.

Thanks for the fix!

ivmai added a commit that referenced this pull request Feb 1, 2017
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.

None yet

2 participants