From Adrien Destugues <pulkomandy@xxxxxxxxx>:
Adrien Destugues has uploaded this change for review. (
https://review.haiku-os.org/c/haiku/+/2371 ;)
Change subject: bootloader: reduce stack usage
......................................................................
bootloader: reduce stack usage
On Sparc Openboot, we get allocated a stack of only 8 kilobytes, and
each called function costs at least 176 bytes for the stack frame.
This means we need to be more careful than usual about stack usage. Move
some large-ish allocations off the stack by either making them static, or
allocated dynamically.
Add a compiler flag to error on functions which use too much stack. The
threshold is at 1023 bytes, because that's what allowed me to find the
two functions that were causing a stack overflow (open_from and
_ParseActivatedPackagesFile)
---
M build/jam/ArchitectureRules
M src/system/boot/loader/file_systems/tarfs/tarfs.cpp
M src/system/boot/loader/package_support.cpp
M src/system/boot/loader/vfs.cpp
M src/system/boot/platform/generic/text_menu.cpp
M src/system/boot/platform/openfirmware/arch/sparc/mmu.cpp
M src/system/boot/platform/openfirmware/start.cpp
7 files changed, 71 insertions(+), 31 deletions(-)
git pull ssh://git.haiku-os.org:22/haiku refs/changes/71/2371/1
diff --git a/build/jam/ArchitectureRules b/build/jam/ArchitectureRules
index 3208f2e..4ecc6c6 100644
--- a/build/jam/ArchitectureRules
+++ b/build/jam/ArchitectureRules
@@ -527,6 +527,9 @@
HAIKU_BOOT_$(bootTarget:U)_CCFLAGS +=
-Wno-error=main -m32 ;
HAIKU_BOOT_$(bootTarget:U)_C++FLAGS +=
-Wno-error=main -m32 ;
}
+ case openfirmware :
+ HAIKU_BOOT_$(bootTarget:U)_CCFLAGS += -fno-pic
-Wno-error=main -Wstack-usage=1023 ;
+ HAIKU_BOOT_$(bootTarget:U)_C++FLAGS += -fno-pic
-Wno-error=main -Wstack-usage=1023 ;
case * :
# all other bootloaders are non-PIC
HAIKU_BOOT_$(bootTarget:U)_CCFLAGS += -fno-pic
-Wno-error=main ;
diff --git a/src/system/boot/loader/file_systems/tarfs/tarfs.cpp
b/src/system/boot/loader/file_systems/tarfs/tarfs.cpp
index 552d800..3969938 100644
--- a/src/system/boot/loader/file_systems/tarfs/tarfs.cpp
+++ b/src/system/boot/loader/file_systems/tarfs/tarfs.cpp
@@ -753,10 +753,10 @@
TarFS::Volume::_Inflate(boot::Partition* partition, void* cookie, off_t offset,
RegionDeleter& regionDeleter, size_t* inflatedBytes)
{
- char in[2048];
+ char* in = (char*)malloc(2048);
z_stream zStream = {
(Bytef*)in, // next in
- sizeof(in), // avail in
+ 2048, // avail in
0, // total in
NULL, // next out
0, // avail out
@@ -776,7 +776,7 @@
bool headerRead = false;
do {
- ssize_t bytesRead = partition->ReadAt(cookie, offset, in,
sizeof(in));
+ ssize_t bytesRead = partition->ReadAt(cookie, offset, in, 2048);
if (bytesRead != (ssize_t)sizeof(in)) {
if (bytesRead <= 0) {
status = Z_STREAM_ERROR;
@@ -819,6 +819,7 @@
} while (status == Z_OK);
inflateEnd(&zStream);
+ free(in);
if (status != Z_STREAM_END) {
TRACE(("tarfs: inflating failed: %d!\n", status));
diff --git a/src/system/boot/loader/package_support.cpp
b/src/system/boot/loader/package_support.cpp
index 12ab7c7..9c92bd5 100644
--- a/src/system/boot/loader/package_support.cpp
+++ b/src/system/boot/loader/package_support.cpp
@@ -274,16 +274,16 @@
PackageVolumeState* state)
{
// find the system package
- char systemPackageName[B_FILE_NAME_LENGTH];
+ char* systemPackageName = (char*)malloc(B_FILE_NAME_LENGTH);
status_t error = _ParseActivatedPackagesFile(packagesDirectory, state,
- systemPackageName, sizeof(systemPackageName));
+ systemPackageName, B_FILE_NAME_LENGTH);
if (error == B_OK) {
// check, if package exists
+ char* packagePath = (char*)malloc(B_PATH_NAME_LENGTH);
for (PackageVolumeState* otherState = state; otherState != NULL;
otherState = fStates.GetPrevious(otherState)) {
- char packagePath[B_PATH_NAME_LENGTH];
otherState->GetPackagePath(systemPackageName,
packagePath,
- sizeof(packagePath));
+ B_PATH_NAME_LENGTH);
struct stat st;
if (get_stat(packagesDirectory, packagePath, st) == B_OK
&& S_ISREG(st.st_mode)) {
@@ -291,6 +291,7 @@
break;
}
}
+ free(packagePath);
} else {
TRACE("PackageVolumeInfo::_InitState(): failed to parse "
"activated-packages: %s\n", strerror(error));
@@ -310,6 +311,7 @@
}
}
+ free(systemPackageName);
if (state->SystemPackage() == NULL)
return B_ENTRY_NOT_FOUND;
@@ -322,28 +324,37 @@
PackageVolumeState* state, char* packageName, size_t packageNameSize)
{
// open the activated-packages file
- char path[3 * B_FILE_NAME_LENGTH + 2];
- snprintf(path, sizeof(path), "%s/%s/%s",
+ static const size_t kBufferSize = 3 * B_FILE_NAME_LENGTH + 2;
+ char* path = (char*)malloc(kBufferSize);
+ snprintf(path, kBufferSize, "%s/%s/%s",
kAdministrativeDirectory, state->Name() != NULL ? state->Name()
: "",
kActivatedPackagesFile);
int fd = open_from(packagesDirectory, path, O_RDONLY);
- if (fd < 0)
+ if (fd < 0) {
+ free(path);
return fd;
+ }
FileDescriptorCloser fdCloser(fd);
struct stat st;
- if (fstat(fd, &st) != 0)
+ if (fstat(fd, &st) != 0) {
+ free(path);
return errno;
- if (!S_ISREG(st.st_mode))
+ }
+ if (!S_ISREG(st.st_mode)) {
+ free(path);
return B_ENTRY_NOT_FOUND;
+ }
// read the file until we find the system package line
size_t remainingBytes = 0;
for (;;) {
ssize_t bytesRead = read(fd, path + remainingBytes,
- sizeof(path) - remainingBytes - 1);
- if (bytesRead <= 0)
+ kBufferSize - remainingBytes - 1);
+ if (bytesRead <= 0) {
+ free(path);
return B_ENTRY_NOT_FOUND;
+ }
remainingBytes += bytesRead;
path[remainingBytes] = '\0';
@@ -352,9 +363,11 @@
while (char* lineEnd = strchr(line, '\n')) {
*lineEnd = '\0';
if (is_system_package(line)) {
- return strlcpy(packageName, line,
packageNameSize)
+ status_t result = strlcpy(packageName, line,
packageNameSize)
< packageNameSize
? B_OK : B_NAME_TOO_LONG;
+ free(path);
+ return result;
}
line = lineEnd + 1;
@@ -369,5 +382,6 @@
remainingBytes = 0;
}
+ free(path);
return B_ENTRY_NOT_FOUND;
}
diff --git a/src/system/boot/loader/vfs.cpp b/src/system/boot/loader/vfs.cpp
index 2a57686..91b12ce 100644
--- a/src/system/boot/loader/vfs.cpp
+++ b/src/system/boot/loader/vfs.cpp
@@ -250,20 +250,25 @@
return node;
// the node is a symbolic link, so we have to resolve the path
- char linkPath[B_PATH_NAME_LENGTH];
- status_t error = node->ReadLink(linkPath, sizeof(linkPath));
+ char* linkPath = (char*)malloc(B_PATH_NAME_LENGTH);
+ status_t error = node->ReadLink(linkPath, B_PATH_NAME_LENGTH);
node->Release();
// we don't need this one anymore
- if (error != B_OK)
+ if (error != B_OK) {
+ free(linkPath);
return NULL;
+ }
// let open_from() do the real work
int fd = open_from(this, linkPath, O_RDONLY);
- if (fd < 0)
+ if (fd < 0) {
+ free(linkPath);
return NULL;
+ }
+ free(linkPath);
node = get_node_from(fd);
if (node != NULL)
node->Acquire();
@@ -1031,34 +1036,45 @@
name++;
}
- char path[B_PATH_NAME_LENGTH];
- if (strlcpy(path, name, sizeof(path)) >= sizeof(path))
+ char* path = (char*)malloc(B_PATH_NAME_LENGTH);
+ if (strlcpy(path, name, B_PATH_NAME_LENGTH) >= B_PATH_NAME_LENGTH) {
+ free(path);
return B_NAME_TOO_LONG;
+ }
Node *node;
status_t error = get_node_for_path(directory, path, &node);
if (error != B_OK) {
- if (error != B_ENTRY_NOT_FOUND)
+ if (error != B_ENTRY_NOT_FOUND) {
+ free(path);
return error;
+ }
- if ((mode & O_CREAT) == 0)
+ if ((mode & O_CREAT) == 0) {
+ free(path);
return B_ENTRY_NOT_FOUND;
+ }
// try to resolve the parent directory
- strlcpy(path, name, sizeof(path));
+ strlcpy(path, name, B_PATH_NAME_LENGTH);
if (char* lastSlash = strrchr(path, '/')) {
- if (lastSlash[1] == '\0')
+ if (lastSlash[1] == '\0') {
+ free(path);
return B_ENTRY_NOT_FOUND;
+ }
*lastSlash = '\0';
name = lastSlash + 1;
// resolve the directory
- if (get_node_for_path(directory, path, &node) != B_OK)
+ if (get_node_for_path(directory, path, &node) != B_OK) {
+ free(path);
return B_ENTRY_NOT_FOUND;
+ }
if (node->Type() != S_IFDIR) {
node->Release();
+ free(path);
return B_NOT_A_DIRECTORY;
}
@@ -1070,16 +1086,20 @@
error = directory->CreateFile(name, permissions, &node);
directory->Release();
- if (error != B_OK)
+ if (error != B_OK) {
+ free(path);
return error;
+ }
} else if ((mode & O_EXCL) != 0) {
node->Release();
+ free(path);
return B_FILE_EXISTS;
}
int fd = open_node(node, mode);
node->Release();
+ free(path);
return fd;
}
diff --git a/src/system/boot/platform/generic/text_menu.cpp
b/src/system/boot/platform/generic/text_menu.cpp
index 611268c..f4cdc6e 100644
--- a/src/system/boot/platform/generic/text_menu.cpp
+++ b/src/system/boot/platform/generic/text_menu.cpp
@@ -159,7 +159,7 @@
if (length > width * 2)
width += 2 * kOffsetX - 1;
- char buffer[width + 1];
+ char* buffer = (char*)malloc(width + 1);
buffer[width] = '\0';
// make sure the buffer is always terminated
@@ -195,6 +195,8 @@
print_centered(console_height() - kHelpLines + row,
buffer);
row++;
}
+
+ free(buffer);
}
}
diff --git a/src/system/boot/platform/openfirmware/arch/sparc/mmu.cpp
b/src/system/boot/platform/openfirmware/arch/sparc/mmu.cpp
index 000f149..d00f814 100644
--- a/src/system/boot/platform/openfirmware/arch/sparc/mmu.cpp
+++ b/src/system/boot/platform/openfirmware/arch/sparc/mmu.cpp
@@ -100,7 +100,7 @@
return B_ERROR;
}
- struct of_region<uint64, uint64> regions[64];
+ static struct of_region<uint64, uint64> regions[64];
int count = of_getprop(package, "reg", regions, sizeof(regions));
if (count == OF_FAILED)
count = of_getprop(sMemoryInstance, "reg", regions,
sizeof(regions));
@@ -185,7 +185,7 @@
// we have proper driver support for the target hardware).
intptr_t mmu = of_instance_to_package(sMmuInstance);
- struct translation_map {
+ static struct translation_map {
void *PhysicalAddress() {
int64_t p = data;
#if 0
diff --git a/src/system/boot/platform/openfirmware/start.cpp
b/src/system/boot/platform/openfirmware/start.cpp
index fb267ef..5cecbc5 100644
--- a/src/system/boot/platform/openfirmware/start.cpp
+++ b/src/system/boot/platform/openfirmware/start.cpp
@@ -85,7 +85,7 @@
extern "C" void
start(void *openFirmwareEntry)
{
- char bootargs[512];
+ static char bootargs[512];
// stage2 args - might be set via the command line one day
stage2_args args;
--
To view, visit https://review.haiku-os.org/c/haiku/+/2371
To unsubscribe, or for help writing mail filters, visit
https://review.haiku-os.org/settings
Gerrit-Project: haiku
Gerrit-Branch: master
Gerrit-Change-Id: Ia0d13a9247e1a3fff4ce654bdffd6edb16e7cbc7
Gerrit-Change-Number: 2371
Gerrit-PatchSet: 1
Gerrit-Owner: Adrien Destugues <pulkomandy@xxxxxxxxx>
Gerrit-MessageType: newchange
