Skip to content

Commit

Permalink
linux-user: Fix brk() to release pages
Browse files Browse the repository at this point in the history
The current brk() implementation does not de-allocate pages if a lower
address is given compared to earlier brk() calls.
But according to the manpage, brk() shall deallocate memory in this case
and currently it breaks a real-world application, specifically building
the debian gcl package in qemu-user.

Fix this issue by reworking the qemu brk() implementation.

Tested with the C-code testcase included in qemu commit 4d1de87, and
by building debian package of gcl in a hppa-linux guest on a x86-64
host.

Signed-off-by: Helge Deller <deller@gmx.de>
Message-Id: <Y6gId80ek49TK1xB@p100>
Signed-off-by: Laurent Vivier <laurent@vivier.eu>
  • Loading branch information
hdeller authored and vivier committed Mar 10, 2023
1 parent 25bb27c commit 86f0473
Showing 1 changed file with 35 additions and 34 deletions.
69 changes: 35 additions & 34 deletions linux-user/syscall.c
Original file line number Diff line number Diff line change
Expand Up @@ -795,49 +795,52 @@ static inline int host_to_target_sock_type(int host_type)
}

static abi_ulong target_brk;
static abi_ulong target_original_brk;
static abi_ulong brk_page;

void target_set_brk(abi_ulong new_brk)
{
target_original_brk = target_brk = HOST_PAGE_ALIGN(new_brk);
target_brk = new_brk;
brk_page = HOST_PAGE_ALIGN(target_brk);
}

//#define DEBUGF_BRK(message, args...) do { fprintf(stderr, (message), ## args); } while (0)
#define DEBUGF_BRK(message, args...)

/* do_brk() must return target values and target errnos. */
abi_long do_brk(abi_ulong new_brk)
abi_long do_brk(abi_ulong brk_val)
{
abi_long mapped_addr;
abi_ulong new_alloc_size;
abi_ulong new_brk, new_host_brk_page;

/* brk pointers are always untagged */

DEBUGF_BRK("do_brk(" TARGET_ABI_FMT_lx ") -> ", new_brk);

if (!new_brk) {
DEBUGF_BRK(TARGET_ABI_FMT_lx " (!new_brk)\n", target_brk);
/* return old brk value if brk_val unchanged or zero */
if (!brk_val || brk_val == target_brk) {
return target_brk;
}
if (new_brk < target_original_brk) {
DEBUGF_BRK(TARGET_ABI_FMT_lx " (new_brk < target_original_brk)\n",
target_brk);

new_brk = TARGET_PAGE_ALIGN(brk_val);
new_host_brk_page = HOST_PAGE_ALIGN(brk_val);

/* brk_val and old target_brk might be on the same page */
if (new_brk == TARGET_PAGE_ALIGN(target_brk)) {
if (brk_val > target_brk) {
/* empty remaining bytes in (possibly larger) host page */
memset(g2h_untagged(target_brk), 0, new_host_brk_page - target_brk);
}
target_brk = brk_val;
return target_brk;
}

/* If the new brk is less than the highest page reserved to the
* target heap allocation, set it and we're almost done... */
if (new_brk <= brk_page) {
/* Heap contents are initialized to zero, as for anonymous
* mapped pages. */
if (new_brk > target_brk) {
memset(g2h_untagged(target_brk), 0, new_brk - target_brk);
}
target_brk = new_brk;
DEBUGF_BRK(TARGET_ABI_FMT_lx " (new_brk <= brk_page)\n", target_brk);
return target_brk;
/* Release heap if necesary */
if (new_brk < target_brk) {
/* empty remaining bytes in (possibly larger) host page */
memset(g2h_untagged(brk_val), 0, new_host_brk_page - brk_val);

/* free unused host pages and set new brk_page */
target_munmap(new_host_brk_page, brk_page - new_host_brk_page);
brk_page = new_host_brk_page;

target_brk = brk_val;
return target_brk;
}

/* We need to allocate more memory after the brk... Note that
Expand All @@ -846,10 +849,14 @@ abi_long do_brk(abi_ulong new_brk)
* itself); instead we treat "mapped but at wrong address" as
* a failure and unmap again.
*/
new_alloc_size = HOST_PAGE_ALIGN(new_brk - brk_page);
mapped_addr = get_errno(target_mmap(brk_page, new_alloc_size,
new_alloc_size = new_host_brk_page - brk_page;
if (new_alloc_size) {
mapped_addr = get_errno(target_mmap(brk_page, new_alloc_size,
PROT_READ|PROT_WRITE,
MAP_ANON|MAP_PRIVATE, 0, 0));
} else {
mapped_addr = brk_page;
}

if (mapped_addr == brk_page) {
/* Heap contents are initialized to zero, as for anonymous
Expand All @@ -861,21 +868,15 @@ abi_long do_brk(abi_ulong new_brk)
* then shrunken). */
memset(g2h_untagged(target_brk), 0, brk_page - target_brk);

target_brk = new_brk;
brk_page = HOST_PAGE_ALIGN(target_brk);
DEBUGF_BRK(TARGET_ABI_FMT_lx " (mapped_addr == brk_page)\n",
target_brk);
target_brk = brk_val;
brk_page = new_host_brk_page;
return target_brk;
} else if (mapped_addr != -1) {
/* Mapped but at wrong address, meaning there wasn't actually
* enough space for this brk.
*/
target_munmap(mapped_addr, new_alloc_size);
mapped_addr = -1;
DEBUGF_BRK(TARGET_ABI_FMT_lx " (mapped_addr != -1)\n", target_brk);
}
else {
DEBUGF_BRK(TARGET_ABI_FMT_lx " (otherwise)\n", target_brk);
}

#if defined(TARGET_ALPHA)
Expand Down

0 comments on commit 86f0473

Please sign in to comment.