Skip to content

[do-not-upstream] GNU toolchain Picolibc fixes (arc-2025.09) #24

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

Open
wants to merge 9 commits into
base: arc-2025.09
Choose a base branch
from

Conversation

mostafa-salmaan
Copy link
Collaborator

@mostafa-salmaan mostafa-salmaan commented Aug 13, 2025

This PR is based on branch arc-2025.09 and contains a set of targeted fixes required specifically for the GNU toolchain. Tsome of these changes are not meant to be merged upstream, as they address toolchain-specific behaviors that aren't relevant to the official Picolibc repository.

Includes the following commits

  1. tinystdio: Handled invalid sequence of characters/numbers – tighten error handling in tinystdio to reject invalid input sequences
  2. complex: Modify casin/casinh functions for better accuracy – adjust casin() and casinh() functions to correct accuracy spikes
  3. complex: Avoid cancelation errors in clog() – improve numerical stability in clog() for edge-case inputs
  4. complex: Fix cacoshf() function for pure real inputs – fix for cacoshf() when dealing with real-only inputs
  5. complex: Fix catan() functions with pure imaginary inputs – patch catan() math function for purely imaginary inputs under GNU
  6. open sys call doesn't handle "rb+" mode – add GNU-specific handling for binary read/write (rb+) mode in semihosted file operations
  7. Revert "semihost: ARM semihosting 'open' doesn't support stdin" – undo a commit that broke open for GNU's semihosting driver

Note:
These commits should remain isolated from upstream synchronization to avoid merge conflicts and unintended code changes in the upstream branch.

…umbers

Handled invalid hexadecimal sequence passed to the `vfscanf` function by using `isxdigit`
to check for the full hex range and detect invalid entry.

Handled if the exponent in the floating point is a valid entry by using `isdigit`

Signed-off-by: Mostafa Salman <[email protected]>
…ccuracy

re-implement `casin/casinh` functions to match the standard relationship between these functions which is
`casin(z) = -i * casinh(i*z)` this is numerically more stable with less cancelation errors avoiding the direct `sqrt(1 - z^2)`
problematic calculation leading to more accurate results.

Signed-off-by: Mostafa Salman <[email protected]>
for `rr ~= 1.0`, compute `log(rr)` as `0.5*log1p(rr^2 - 1)`, where `r^2 - 1` is calculated accurately to avoid cancellation errors.
because direct `log(rr)` loses precision when `rr ~= 1.0`

Signed-off-by: Mostafa Salman <[email protected]>
Special handling for purely real inputs where `x >= 1` to use the real `acoshf()` function which is more accurate for these cases
and use the complex formula for other cases

Signed-off-by: Mostafa Salman <[email protected]>
…inputs

`catan()` functions needed special case handling for pure imaginary inputs instead of triggering the overflow case when it shouldn't

Signed-off-by: Mostafa Salman <[email protected]>
According to the ISO/IEC_9899_1999, section:7.19.5.3 Opening file with `r+` mode just open text file for update (reading and writing) but dosen't create it. if the file isn't exist `open()` returns NULL.

So, to create and open a file to update (reading and writing), we should open with "a+" or "w+" modes.

Signed-off-by: Mostafa Salman <[email protected]>
…doesn't support stdin"

This reverts commit [81e6fa2](picolibc@81e6fa2)
As This commit introduced a workaround specifically for ARM that is no longer necessary or desirable.

Signed-off-by: Mostafa Salman <[email protected]>
…ve values of width

before this change, if width is zero code get stuck in an infite loop through
negative values of width as it is defined as a signed int.

Signed-off-by: Mostafa Salman <[email protected]>
- Extends the handling of the `%n` specifier to support different integer types,
  including `%ln` (long), `%lln` (long long), `%hn` (short) and `%hhn` (char).
  This ensures correct and consistent behavior for various specifier types.
- Fix in vfprintf with %n when buffer has limited size
  before this change, the %n specifier returns the number of characters
  that would have been written if the buffer was large enough,
  instead of the actual number of characters written.
  Also, Added a gaurd if %n format specifier is enabled to protect printf
  functions from %n security issues

Signed-off-by: Mostafa Salman <[email protected]>
@@ -1249,7 +1266,9 @@ int vfprintf (FILE * stream, const CHAR *fmt, va_list ap_orig)
base = 2;
#endif
} else {
my_putc('%', stream);
while (--width > 0) {
Copy link
Collaborator

Choose a reason for hiding this comment

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

This change seems unrelated to the rest of the patch?

Copy link
Collaborator

@keith-packard keith-packard left a comment

Choose a reason for hiding this comment

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

I read through the POSIX spec and your interpretation seems reasonable for this. However, I then created a testcase for this and discovered that glibc does exactly what the upstream picolibc code does -- it writes the number of characters that were generated by the operation, even if some of those are not actually written to the output because of the buffer size limit.

I'm struggling to understand how to read the POSIX spec in a way that makes glibc compliant, and yet glibc compatibility is at least as important as POSIX compliance for picolibc.

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