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

release9.2/bugs/rtheap_mmap_only #76

Merged
merged 4 commits into from
Jun 9, 2016

Conversation

wrwilliams
Copy link
Member

DYNINSTos_malloc in the RTlib would malloc its heap data structures and string processing data for /proc/maps. Both of these could lead to deadlocks with newer glibc versions. Fixed as follows:

  1. mmaps are no longer trying to be smart, but instead asking the OS for something valid and bounds-checking it against the request
  2. heap data structures are no longer malloced, but instead tacked onto the end of each heap segment

…ations such that:

1) Usable size >= requested size
2) Heap data structure at end, not at beginning, of heap (so that user heaps start aligned, and so that we aren't grabbing a whole previous page regardless of requested size).
@wrwilliams
Copy link
Member Author

Fixes #50; posted for review.

#ifdef DEBUG
fprintf(stderr, "MALLOC'd area fails range constraints\n");
#endif
return NULL;
}

/* define new heap */
node = heap + size;
Copy link
Contributor

Choose a reason for hiding this comment

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

I think it should be node = ret_heap + size, but in practice malloc probably aligns sufficiently anyway.

@cuviper
Copy link
Contributor

cuviper commented Jun 9, 2016

Small malloc nit, otherwise looks good to me!

The node data structure should, in the case where the actual heap and the returned heap differ, go at the end of the returned heap so that the returned heap's size is correct.
@wrwilliams wrwilliams merged commit ce99292 into master Jun 9, 2016
@wrwilliams wrwilliams deleted the release9.2/bugs/rtheap_mmap_only branch June 9, 2016 15:32
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.

2 participants