add realpath() and fix setenv() to store the reallocated environ array - #15
Merged
Merged
Conversation
codewiz
changed the title
There was a problem hiding this comment.
🟡 Changes recommended
The setenv() insert path still reallocates the environment pointer array with an off-by-one size, which can overflow by one pointer and corrupt the heap.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Pull request overview
This PR addresses AmigaOS-hosted toolchain startup crashes by fixing setenv()’s environment-array reallocation handling, and adds an AmigaOS implementation of realpath() needed by gcc/libiberty to canonicalize filenames.
Changes:
- Fix
setenv()to persist the (potentially moved)realloc()’denviron_ptr__datapointer and to return-1witherrnoset on allocation failure. - Add
sources/nix20/extra/realpath.cimplementingrealpath()viaLock()+NameFromLock(), allocating a buffer whenresolved == NULL.
File summaries
| File | Description |
|---|---|
| sources/nix20/stdlib/putenv.c | Adjusts setenv() failure semantics and stores the reallocated environ_ptr__data pointer. |
| sources/nix20/extra/realpath.c | Introduces AmigaOS realpath() implementation using DOS locking/name resolution APIs. |
Review details
- Files reviewed: 2/2 changed files
- Comments generated: 1
- Review effort level: Lite
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
The insert path grew environ_ptr__data with realloc but kept the old pointer, so the next setenv reallocated a freed block and corrupted the heap. The new size was also one pointer short of the entry plus the terminator. Return -1 with errno set instead of ENOMEM on failure.
Resolve the name with Lock and NameFromLock into the caller buffer, or a malloc(MAXPATHLEN) one when it is NULL. Programs such as gcc use it to compare file names.
codewiz
force-pushed
the
fix/setenv-environ
branch
from
September 8, 2026 01:39
c429343 to
06d5c0d
Compare
MBeijer
approved these changes
Sep 8, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
The AmigaOS-hosted gcc and cpp from the 16.2 packages crash on startup. The driver inserts several environment variables with
setenv, and libnix's insert path growsenviron_ptr__datawithreallocbut never stores the new pointer. The second insert reallocs the already freed block, reads a garbage size from its header and hands exec a bogusFreeMem. Under vamos that logsdeallocate: block outside of mem header!and then spins on the corrupted chunk list; on AmigaOS it corrupts the system memory list.setenv: store the reallocated array, and return -1 witherrnoset instead ofENOMEMon failure.realpath: new, in nix20 since it needsNameFromLock. Lock plus NameFromLock into the caller's buffer, or amalloc(MAXPATHLEN)one when it is NULL. gcc's libiberty uses it to compare file names.With these two the hosted driver runs and compiles under vamos; what remains is a binutils bug in cc1's exit path: AmigaPorts/binutils-gdb#21. The libiberty fallback it used to hit without realpath is AmigaPorts/gcc#37.