Skip to content

Conversation

@michaelrj-google
Copy link
Contributor

This bug is caused by the BigInt implementation failing to initialize
from errno. Explanation below, but the fix is a static cast to int.

The bug only shows up on risc-v 32 because of a chain of type-oddities:

  1. Errno is provided by a struct with an implicit cast to int.
  2. The printf parser uses an int128 to store the value of a conversion
    on systems with long double greater than double.
  3. On systems without native int128 support we use our own BigInt instead.

These combine such that if both long double and int128 exist (e.g. on
x86) there's no issue, errno is implicitly cast to int, which is
extended to int128. If long double is double (e.g. on arm32) then int64
is used in the printf parser, the implicit cast works, and there's no
issue. The only way this would come up is if the target has a proper
long double type, but not int128, which is the case for at least the
current risc-v 32 bot.

This bug is caused by the BigInt implementation failing to initialize
from errno. Explanation below, but the fix is a static cast to int.

The bug only shows up on risc-v 32 because of a chain of type-oddities:
1) Errno is provided by a struct with an implicit cast to int.
2) The printf parser uses an int128 to store the value of a conversion
   on systems with long double greater than double.
3) On systems without native int128 support we use our own BigInt instead.

These combine such that if both long double and int128 exist (e.g. on
x86) there's no issue, errno is implicitly cast to int, which is
extended to int128. If long double is double (e.g. on arm32) then int64
is used in the printf parser, the implicit cast works, and there's no
issue. The only way this would come up is if the target has a proper
long double type, but not int128, which is the case for at least the
current risc-v 32 bot.
@llvmbot llvmbot added the libc label Sep 25, 2024
@llvmbot
Copy link
Member

llvmbot commented Sep 25, 2024

@llvm/pr-subscribers-libc

Author: Michael Jones (michaelrj-google)

Changes

This bug is caused by the BigInt implementation failing to initialize
from errno. Explanation below, but the fix is a static cast to int.

The bug only shows up on risc-v 32 because of a chain of type-oddities:

  1. Errno is provided by a struct with an implicit cast to int.
  2. The printf parser uses an int128 to store the value of a conversion
    on systems with long double greater than double.
  3. On systems without native int128 support we use our own BigInt instead.

These combine such that if both long double and int128 exist (e.g. on
x86) there's no issue, errno is implicitly cast to int, which is
extended to int128. If long double is double (e.g. on arm32) then int64
is used in the printf parser, the implicit cast works, and there's no
issue. The only way this would come up is if the target has a proper
long double type, but not int128, which is the case for at least the
current risc-v 32 bot.


Full diff: https://github.com/llvm/llvm-project/pull/110053.diff

1 Files Affected:

  • (modified) libc/src/stdio/printf_core/parser.h (+1-1)
diff --git a/libc/src/stdio/printf_core/parser.h b/libc/src/stdio/printf_core/parser.h
index e2cb734b5be715..acbbaa25b1c9da 100644
--- a/libc/src/stdio/printf_core/parser.h
+++ b/libc/src/stdio/printf_core/parser.h
@@ -265,7 +265,7 @@ template <typename ArgProvider> class Parser {
       case ('m'):
         // %m is an odd conversion in that it doesn't consume an argument, it
         // just takes the current value of errno as its argument.
-        section.conv_val_raw = libc_errno;
+        section.conv_val_raw = static_cast<int>(libc_errno);
         break;
 #endif // LIBC_COPT_PRINTF_DISABLE_STRERROR
 #ifndef LIBC_COPT_PRINTF_DISABLE_WRITE_INT

@michaelrj-google michaelrj-google merged commit eab63b5 into llvm:main Sep 25, 2024
6 checks passed
@michaelrj-google michaelrj-google deleted the libcStrerrorCastFix branch September 25, 2024 22:30
Sterling-Augustine pushed a commit to Sterling-Augustine/llvm-project that referenced this pull request Sep 27, 2024
This bug is caused by the BigInt implementation failing to initialize
from errno. Explanation below, but the fix is a static cast to int.

The bug only shows up on risc-v 32 because of a chain of type-oddities:
1) Errno is provided by a struct with an implicit cast to int.
2) The printf parser uses an int128 to store the value of a conversion
   on systems with long double greater than double.
3) On systems without native int128 support we use our own BigInt
instead.

These combine such that if both long double and int128 exist (e.g. on
x86) there's no issue, errno is implicitly cast to int, which is
extended to int128. If long double is double (e.g. on arm32) then int64
is used in the printf parser, the implicit cast works, and there's no
issue. The only way this would come up is if the target has a proper
long double type, but not int128, which is the case for at least the
current risc-v 32 bot.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants