[haiku-commits] haiku: hrev49872 - in src: system/libroot/posix/unistd tests/system/libroot/posix system

  • From: jerome.duval@xxxxxxxxx
  • To: haiku-commits@xxxxxxxxxxxxx
  • Date: Sun, 22 Nov 2015 17:59:33 +0100 (CET)

hrev49872 adds 1 changeset to branch 'master'
old head: dc0347f13e7c578deb6bd4195f1cb0e32d652e38
new head: dbf060a3f7d42cc5a6f79a48749f1a07161179d1
overview:
http://cgit.haiku-os.org/haiku/log/?qt=range&q=dbf060a3f7d4+%5Edc0347f13e7c

----------------------------------------------------------------------------

dbf060a3f7d4: libroot: Add brk() and sbrk().

This commit replaces the placeholder implementation of sbrk(), which
operated on a process' heap, with real implementations of brk() and
sbrk() that adjust a process' program break.

* unistd.h: Add standard definitions of brk() and sbrk(); include
stdint.h for intptr_t.
* thread.cpp: Recognize RLIMIT_AS and RLIMIT_DATA resource limits
(both currently unlimited); order limit identifiers alphabetically.
* arch-specific.cpp: Remove sbrk_hook().
* malloc_debug_api.cpp: Remove sbrk_hook().
* unistd/Jamfile: Build brk.c instead of sbrk.c.
* unistd/brk.c: Add.
* unistd/sbrk.c: Delete (placeholder implementation).
* libroot_stubs.c: Remove sbrk_hook().
* libroot_stubs_legacy.c: Remove sbrk_hook().
* src/tests/.../posix/Jamfile: Build brk_test.c.
* brk_test.c: Add (simple unit test that demonstrates behaviour of
sbrk()).

Signed-off-by: Jérôme Duval <jerome.duval@xxxxxxxxx>

[ Simon South <ssouth@xxxxxxxxxxxxxx> ]

----------------------------------------------------------------------------

Revision: hrev49872
Commit: dbf060a3f7d42cc5a6f79a48749f1a07161179d1
URL: http://cgit.haiku-os.org/haiku/commit/?id=dbf060a3f7d4
Author: Simon South <ssouth@xxxxxxxxxxxxxx>
Date: Thu Oct 29 17:31:28 2015 UTC
Committer: Jérôme Duval <jerome.duval@xxxxxxxxx>
Commit-Date: Sun Nov 22 16:59:22 2015 UTC

----------------------------------------------------------------------------

11 files changed, 429 insertions(+), 51 deletions(-)
headers/posix/unistd.h | 5 +-
src/system/kernel/thread.cpp | 24 +-
.../libroot/posix/malloc/arch-specific.cpp | 3 -
.../posix/malloc_debug/malloc_debug_api.cpp | 9 -
src/system/libroot/posix/unistd/Jamfile | 2 +-
src/system/libroot/posix/unistd/brk.c | 268 +++++++++++++++++++
src/system/libroot/posix/unistd/sbrk.c | 28 --
src/system/libroot/stubbed/libroot_stubs.c | 1 -
.../libroot/stubbed/libroot_stubs_legacy.c | 1 -
src/tests/system/libroot/posix/Jamfile | 1 +
src/tests/system/libroot/posix/brk_test.c | 138 ++++++++++

----------------------------------------------------------------------------

diff --git a/headers/posix/unistd.h b/headers/posix/unistd.h
index 42e4584..9c8fc77 100644
--- a/headers/posix/unistd.h
+++ b/headers/posix/unistd.h
@@ -7,6 +7,7 @@


#include <null.h>
+#include <stdint.h>
#include <sys/types.h>


@@ -221,7 +222,9 @@ extern void _exit(int status) __attribute__
((noreturn));

extern pid_t tcgetpgrp(int fd);
extern int tcsetpgrp(int fd, pid_t pgrpid);
-extern void *sbrk(long incr);
+
+extern int brk(void *addr);
+extern void *sbrk(intptr_t increment);

extern unsigned int alarm(unsigned int seconds);
extern useconds_t ualarm(useconds_t microSeconds, useconds_t interval);
diff --git a/src/system/kernel/thread.cpp b/src/system/kernel/thread.cpp
index de4adb5..12203f0 100644
--- a/src/system/kernel/thread.cpp
+++ b/src/system/kernel/thread.cpp
@@ -1307,15 +1307,25 @@ common_getrlimit(int resource, struct rlimit * rlp)
return B_BAD_ADDRESS;

switch (resource) {
- case RLIMIT_NOFILE:
- case RLIMIT_NOVMON:
- return vfs_getrlimit(resource, rlp);
+ case RLIMIT_AS:
+ rlp->rlim_cur = __HAIKU_ADDR_MAX;
+ rlp->rlim_max = __HAIKU_ADDR_MAX;
+ return B_OK;

case RLIMIT_CORE:
rlp->rlim_cur = 0;
rlp->rlim_max = 0;
return B_OK;

+ case RLIMIT_DATA:
+ rlp->rlim_cur = RLIM_INFINITY;
+ rlp->rlim_max = RLIM_INFINITY;
+ return B_OK;
+
+ case RLIMIT_NOFILE:
+ case RLIMIT_NOVMON:
+ return vfs_getrlimit(resource, rlp);
+
case RLIMIT_STACK:
{
rlp->rlim_cur = USER_MAIN_THREAD_STACK_SIZE;
@@ -1338,16 +1348,16 @@ common_setrlimit(int resource, const struct rlimit *
rlp)
return B_BAD_ADDRESS;

switch (resource) {
- case RLIMIT_NOFILE:
- case RLIMIT_NOVMON:
- return vfs_setrlimit(resource, rlp);
-
case RLIMIT_CORE:
// We don't support core file, so allow settings to 0/0
only.
if (rlp->rlim_cur != 0 || rlp->rlim_max != 0)
return EINVAL;
return B_OK;

+ case RLIMIT_NOFILE:
+ case RLIMIT_NOVMON:
+ return vfs_setrlimit(resource, rlp);
+
default:
return EINVAL;
}
diff --git a/src/system/libroot/posix/malloc/arch-specific.cpp
b/src/system/libroot/posix/malloc/arch-specific.cpp
index 872867c..4167f04 100644
--- a/src/system/libroot/posix/malloc/arch-specific.cpp
+++ b/src/system/libroot/posix/malloc/arch-specific.cpp
@@ -36,9 +36,6 @@
# define CTRACE(x) ;
#endif

-extern "C" void *(*sbrk_hook)(long);
-void *(*sbrk_hook)(long) = &BPrivate::hoardSbrk;
-
using namespace BPrivate;

struct free_chunk {
diff --git a/src/system/libroot/posix/malloc_debug/malloc_debug_api.cpp
b/src/system/libroot/posix/malloc_debug/malloc_debug_api.cpp
index 45d3c1c..3d3b062 100644
--- a/src/system/libroot/posix/malloc_debug/malloc_debug_api.cpp
+++ b/src/system/libroot/posix/malloc_debug/malloc_debug_api.cpp
@@ -206,15 +206,6 @@ __heap_terminate_after()


extern "C" void*
-sbrk_hook(long)
-{
- debug_printf("sbrk not supported on malloc debug\n");
- debugger("sbrk not supported on malloc debug");
- return NULL;
-}
-
-
-extern "C" void*
memalign(size_t alignment, size_t size)
{
return sCurrentHeap->memalign(alignment, size);
diff --git a/src/system/libroot/posix/unistd/Jamfile
b/src/system/libroot/posix/unistd/Jamfile
index c57c52f..f973a9c 100644
--- a/src/system/libroot/posix/unistd/Jamfile
+++ b/src/system/libroot/posix/unistd/Jamfile
@@ -12,6 +12,7 @@ for architectureObject in [ MultiArchSubDirSetup ] {
MergeObject <$(architecture)>posix_unistd.o :
access.c
alarm.c
+ brk.c
chown.c
chroot.cpp
close.c
@@ -33,7 +34,6 @@ for architectureObject in [ MultiArchSubDirSetup ] {
pipe.c
process.c
read.c
- sbrk.c
sleep.c
sync.c
system.cpp
diff --git a/src/system/libroot/posix/unistd/brk.c
b/src/system/libroot/posix/unistd/brk.c
new file mode 100644
index 0000000..abc6e0c
--- /dev/null
+++ b/src/system/libroot/posix/unistd/brk.c
@@ -0,0 +1,268 @@
+/*
+ * Copyright 2015 Simon South, ssouth@xxxxxxxxxxxxxx
+ * All rights reserved. Distributed under the terms of the MIT License.
+ */
+
+
+/*!
+ * \file brk.c
+ *
+ * This module provides \c brk and \c sbrk, functions that adjust the program
+ * break of their calling process.
+ *
+ * A process' program break is the first memory location past the end of its
+ * data segment. Moving the program break has the effect of increasing or
+ * decreasing the amount of memory allocated to the process for its data and
+ * can be used to achieve a simple form of memory management.
+ *
+ * An ELF executable may contain multiple writable segments that make up a
+ * program's data segment, each of which may be placed by Haiku's \c
+ * runtime_loader in either one or two areas in memory. Additionally, nothing
+ * stops a program from adding onto its data segment directly using \c
+ * create_area. For these reasons this implementation avoids making
+ * assumptions about the process' data segment except that it is contiguous.
+ * \c brk and \c sbrk will function correctly regardless of the number of
+ * (adjacent) areas used to hold the process' data.
+ *
+ * Note this implementation never creates new areas; its operation is limited
+ * to expanding, contracting or deleting areas as necessary.
+ */
+
+
+#include <errno.h>
+#include <stdint.h>
+#include <sys/resource.h>
+#include <unistd.h>
+
+#include <image.h>
+#include <OS.h>
+
+#include <errno_private.h>
+
+
+/*!
+ * Locates and returns information about the base (lowest in memory) area of
+ * the contiguous set of areas that hold the calling process' data segment.
+ *
+ * \param base_area_info A pointer to an \c area_info structure to receive
+ * information about the area.
+ * \return \c B_OK on success; \c B_BAD_VALUE otherwise.
+ */
+static status_t
+get_data_segment_base_area_info(area_info *base_area_info)
+{
+ status_t result = B_ERROR;
+ bool app_image_found = false;
+
+ image_info i_info;
+ int32 image_cookie = 0;
+
+ /* Locate our app image and output information for the area that holds
the
+ * start of its data segment */
+ while (!app_image_found
+ && get_next_image_info(0, &image_cookie, &i_info) == B_OK) {
+ if (i_info.type == B_APP_IMAGE) {
+ app_image_found = true;
+
+ result = get_area_info(area_for(i_info.data),
base_area_info);
+ }
+ }
+
+ return result;
+}
+
+
+/*!
+ * Checks whether a proposed new program break would be valid given the
+ * resource limits imposed on the calling process.
+ *
+ * \param program_break The proposed new program break.
+ * \param base_address The base (start) address of the process' data segment.
+ * \return \c true if the program break would be valid given this process'
+ * resource limits; \c false otherwise.
+ */
+static bool
+program_break_within_resource_limits(void *program_break, void *base_address)
+{
+ bool result = false;
+
+ struct rlimit rlim;
+
+ if (getrlimit(RLIMIT_AS, &rlim) == 0
+ && (rlim.rlim_cur == RLIM_INFINITY
+ || program_break < (void *)rlim.rlim_cur)
+ && getrlimit(RLIMIT_DATA, &rlim) == 0
+ && (rlim.rlim_cur == RLIM_INFINITY
+ || ((rlim_t)((uint8_t *)program_break - (uint8_t
*)(base_address))
+ < rlim.rlim_cur)))
+ result = true;
+
+ return result;
+}
+
+
+/*!
+ * Resizes an area, up or down as necessary, so it ends at the specified
+ * address (rounded up to the nearest page boundary).
+ *
+ * \param a_info An \c area_info structure corresponding to the area on which
+ * to operate.
+ * \param address The new address at which the area should end. This address
+ * will be rounded up to the nearest page boundary.
+ * \return \c B_OK on success; \c B_BAD_VALUE, \c B_NO_MEMORY or \c B_ERROR
+ * otherwise (refer to the documentation for \c resize_area).
+ */
+static status_t
+resize_area_to_address(area_info *a_info, void *address)
+{
+ size_t new_size = (uint8_t *)address - (uint8_t *)(a_info->address);
+ size_t new_size_aligned = (new_size + B_PAGE_SIZE - 1) & ~(B_PAGE_SIZE
- 1);
+
+ return resize_area(a_info->area, new_size_aligned);
+}
+
+
+/*!
+ * An internal method that sets the program break of the calling process to the
+ * specified address.
+ *
+ * \param addr The requested new program break. This address will be rounded up
+ * as necessary to align the program break on a page boundary.
+ * \param base_area_info An \c area_info structure corresponding to the base
+ * (lowest in memory) area of the contiguous set of areas
+ * that hold the calling process' data segment.
+ * \return 0 on success; -1 otherwise.
+ */
+static int
+brk_internal(void *addr, area_info *base_area_info)
+{
+ int result = 0;
+
+ void *next_area_address;
+ area_id next_area;
+ area_info next_area_info;
+
+ /* First, recursively process the next (higher) adjacent area, if any */
+ next_area_address = (uint8_t *)base_area_info->address
+ + base_area_info->size;
+ next_area = area_for(next_area_address);
+ if (next_area != B_ERROR) {
+ result = get_area_info(next_area, &next_area_info) == B_OK ?
+ brk_internal(addr, &next_area_info) : -1;
+ } else {
+ /* This is the highest ("last") area in the data segment, so any
+ * increase in size needs to occur on this area */
+ if (addr > next_area_address) {
+ result = resize_area_to_address(base_area_info, addr)
== B_OK ? 0
+ : -1;
+ }
+ }
+
+ if (result == 0) {
+ if (addr <= base_area_info->address) {
+ /* This area starts at or above the program break the
caller has
+ * requested, so the entire area can be deleted */
+ result = delete_area(base_area_info->area) == B_OK ? 0
: -1;
+ } else if (addr < next_area_address) {
+ /* The requested program break lies within this area,
so this area
+ * must be contracted */
+ result = resize_area_to_address(base_area_info, addr)
== B_OK ? 0
+ : -1;
+ }
+ }
+
+ return result;
+}
+
+
+/*!
+ * Sets the calling process' program break to the specified address, if valid
+ * and within the limits of available resources, expanding or contracting the
+ * process' data segment as necessary.
+ *
+ * \param addr The requested new program break.
+ * \return 0 on success; -1 on error (in which case \c errno will be set to
+ * \c ENOMEM).
+ */
+int
+brk(void *addr)
+{
+ int result = -1;
+
+ area_info base_area_info;
+
+ if (get_data_segment_base_area_info(&base_area_info) == B_OK
+ && addr > base_area_info.address
+ && program_break_within_resource_limits(addr,
base_area_info.address))
+ result = brk_internal(addr, &base_area_info);
+
+ if (result == -1)
+ __set_errno(ENOMEM);
+
+ return result;
+}
+
+
+/*!
+ * Adjusts the calling process' program break up or down by the requested
+ * number of bytes, if valid and within the limits of available resources,
+ * expanding or contracting the process' data segment as necessary.
+ *
+ * \param increment The amount, positive or negative, in bytes by which to
+ * adjust the program break. This value will be rounded up as
+ * necessary to align the program break on a page boundary.
+ * \return The previous program break on success; (void *)-1 on error (in which
+ * case \c errno will be set to \c ENOMEM).
+ */
+void *
+sbrk(intptr_t increment)
+{
+ void *result = NULL;
+
+ area_info base_area_info;
+
+ area_id next_area;
+ area_info next_area_info;
+
+ uint8_t *program_break;
+ uint8_t *new_program_break;
+
+ if (get_data_segment_base_area_info(&base_area_info) == B_OK) {
+ /* Find the current program break, which will be the memory
address
+ * just past the end of the highest ("last") of the areas that
hold the
+ * data segment */
+ next_area = base_area_info.area;
+ do {
+ if (get_area_info(next_area, &next_area_info) == B_OK) {
+ next_area = area_for((uint8_t
*)(next_area_info.address)
+ + next_area_info.size);
+ } else {
+ result = (void *)-1;
+ }
+ } while (next_area != B_ERROR && result == NULL);
+
+ if (result == NULL) {
+ program_break = (uint8_t *)(next_area_info.address)
+ + next_area_info.size;
+ new_program_break = program_break + increment;
+
+ /* If the requested increment is zero, just return the
address of
+ * the current program break; otherwise, set a new
program break
+ * and return the address of the old one */
+ if (increment == 0
+ ||
(program_break_within_resource_limits(new_program_break,
+ base_area_info.address)
+ && brk_internal(new_program_break,
&base_area_info) == 0))
+ result = program_break;
+ else
+ result = (void *)-1;
+ }
+ } else {
+ result = (void *)-1;
+ }
+
+ if (result == (void *)-1)
+ __set_errno(ENOMEM);
+
+ return result;
+}
diff --git a/src/system/libroot/posix/unistd/sbrk.c
b/src/system/libroot/posix/unistd/sbrk.c
deleted file mode 100644
index 42a9838..0000000
--- a/src/system/libroot/posix/unistd/sbrk.c
+++ /dev/null
@@ -1,28 +0,0 @@
-/*
- * Copyright 2004-2006, Axel Dörfler, axeld@xxxxxxxxxxxxxxxx. All rights
reserved.
- * Distributed under the terms of the MIT License.
- */
-
-
-#include <unistd.h>
-#include <errno.h>
-
-#include <errno_private.h>
-
-
-/* in hoard wrapper */
-extern void *(*sbrk_hook)(long);
-
-
-void *
-sbrk(long increment)
-{
- // TODO: we only support extending the heap for now using this method
- if (increment <= 0)
- return NULL;
-
- if (sbrk_hook)
- return (*sbrk_hook)(increment);
-
- return NULL;
-}
diff --git a/src/system/libroot/stubbed/libroot_stubs.c
b/src/system/libroot/stubbed/libroot_stubs.c
index cd8ff77..76cff4e 100644
--- a/src/system/libroot/stubbed/libroot_stubs.c
+++ b/src/system/libroot/stubbed/libroot_stubs.c
@@ -79,7 +79,6 @@ int opterr;
int optind;
int optopt;
int re_syntax_options;
-int sbrk_hook;
int signgam;
int stderr;
int stdin;
diff --git a/src/system/libroot/stubbed/libroot_stubs_legacy.c
b/src/system/libroot/stubbed/libroot_stubs_legacy.c
index 855c3bb..d53e8a0 100644
--- a/src/system/libroot/stubbed/libroot_stubs_legacy.c
+++ b/src/system/libroot/stubbed/libroot_stubs_legacy.c
@@ -103,7 +103,6 @@ int opterr;
int optind;
int optopt;
int re_syntax_options;
-int sbrk_hook;
int signgam;
int stderr;
int stdin;
diff --git a/src/tests/system/libroot/posix/Jamfile
b/src/tests/system/libroot/posix/Jamfile
index 92b09df..131972c 100644
--- a/src/tests/system/libroot/posix/Jamfile
+++ b/src/tests/system/libroot/posix/Jamfile
@@ -7,6 +7,7 @@ TARGET_WARNING_C++FLAGS_$(TARGET_PACKAGING_ARCH)
# POSIX/libc tests
SimpleTest abort_test : abort_test.cpp ;
SimpleTest SyslogTest : SyslogTest.cpp ;
+SimpleTest brk_test : brk_test.c ;
SimpleTest clearenv : clearenv.cpp ;
SimpleTest dirent_test : dirent_test.cpp ;
SimpleTest flock_test : flock_test.cpp ;
diff --git a/src/tests/system/libroot/posix/brk_test.c
b/src/tests/system/libroot/posix/brk_test.c
new file mode 100644
index 0000000..fd8d6d0
--- /dev/null
+++ b/src/tests/system/libroot/posix/brk_test.c
@@ -0,0 +1,138 @@
+/*
+ * Copyright 2015 Simon South, ssouth@xxxxxxxxxxxxxx
+ * All rights reserved. Distributed under the terms of the MIT License.
+ */
+
+
+#include <errno.h>
+#include <stdint.h>
+#include <stdio.h>
+#include <string.h>
+#include <unistd.h>
+
+#include <image.h>
+#include <OS.h>
+
+
+struct sbrk_test {
+ char *name;
+ char *sbrk_arg_text;
+ intptr_t sbrk_arg;
+};
+
+
+static void
+output_area_info(void *base_address)
+{
+ uint8_t *next_area_address;
+ area_id next_area;
+ area_info next_area_info;
+
+ next_area = area_for(base_address);
+ while (next_area != B_ERROR) {
+ if (get_area_info(next_area, &next_area_info) == B_OK) {
+ next_area_address = (uint8_t *)(next_area_info.address)
+ + next_area_info.size;
+
+ printf("Area ID 0x%x: Addr: %p - %p Size: 0x%04zx
%s\n",
+ next_area_info.area, next_area_info.address,
+ next_area_address - 1, next_area_info.size,
+ next_area_info.name);
+
+ next_area = area_for(next_area_address);
+ } else {
+ fprintf(stderr, "PROBLEM: Couldn't get area info");
+ }
+ }
+}
+
+
+static void
+output_data_segment_info(void *address)
+{
+ puts("Current data segment layout:");
+ output_area_info(address);
+ puts("");
+}
+
+
+static void
+output_sbrk_result(intptr_t sbrk_arg, char *sbrk_arg_text)
+{
+ printf("\tsbrk(%s) returns %p\n", sbrk_arg_text, sbrk(sbrk_arg));
+ if (errno != 0) {
+ printf("\tError: %s\n", strerror(errno));
+ errno = 0;
+ }
+}
+
+
+int
+main(int argc, char **argv)
+{
+ static const uint NUM_TESTS = 7;
+ static struct sbrk_test sbrk_tests[] = {
+ { "Get current program break", "0", 0
},
+
+ { "Expand data segment (less than one page)", "0x7ff", 0x7ff
},
+ { "Expand data segment (more than one page)", "0x57ff", 0x57ff
},
+ {
+ "Expand data segment (unreasonable value)",
+ "INTPTR_MAX",
+ INTPTR_MAX
+ },
+
+ { "Shrink data segment (less than one page)", "-0x7ff",
-0x7ff },
+ { "Shrink data segment (more than one page)", "-0x27ff",
-0x27ff },
+ {
+ "Shrink data segment (unreasonable value)",
+ "INTPTR_MIN",
+ INTPTR_MIN
+ }
+ };
+
+ int result = -1;
+
+ bool app_image_found = false;
+ image_info i_info;
+ int32 image_cookie = 0;
+
+ void *data_segment_address = NULL;
+
+ uint test_index;
+ struct sbrk_test *next_sbrk_test;
+
+ /* Find the address of our data segment */
+ while (!app_image_found
+ && get_next_image_info(0, &image_cookie, &i_info) == B_OK) {
+ if (i_info.type == B_APP_IMAGE) {
+ app_image_found = true;
+
+ data_segment_address = i_info.data;
+ }
+ }
+
+ if (data_segment_address != NULL) {
+ /* Run our tests */
+ test_index = 0;
+ while (test_index < NUM_TESTS) {
+ next_sbrk_test = &sbrk_tests[test_index];
+
+ output_data_segment_info(data_segment_address);
+ printf("%s:\n", next_sbrk_test->name);
+ output_sbrk_result(next_sbrk_test->sbrk_arg,
+ next_sbrk_test->sbrk_arg_text);
+ printf("\n");
+
+ test_index += 1;
+ }
+
+ output_data_segment_info(data_segment_address);
+
+ result = 0;
+ } else {
+ fprintf(stderr, "PROBLEM: Couldn't locate data segment\n");
+ }
+
+ return result;
+}


Other related posts:

  • » [haiku-commits] haiku: hrev49872 - in src: system/libroot/posix/unistd tests/system/libroot/posix system - jerome . duval