This is an archive of the discontinued LLVM Phabricator instance.

[libc++][print] Adds FILE functions.
ClosedPublic

Authored by Mordante on May 6 2023, 9:53 AM.

Details

Reviewers
ldionne
Group Reviewers
Restricted Project
Commits
rG3f65f718332c: [libc++][print] Adds FILE functions.
Summary

Drive-by fix to make sure the __retarget_buffer works correctly whan
using a hint of 1. This was discovered in one of the new tests.

Drive-by fixes __retarget_buffer when initialized with size 1.

Implements parts of

  • P2093R14 Formatted output
  • P2539R4 Should the output of std::print to a terminal be synchronized with the underlying stream?

Diff Detail

Event Timeline

There are a very large number of changes, so older changes are hidden. Show Older Changes
Mordante updated this revision to Diff 521989.May 14 2023, 4:18 AM

Several CI fixes.

Mordante updated this revision to Diff 521996.May 14 2023, 5:10 AM

CI fixes.

Mordante updated this revision to Diff 528101.Jun 3 2023, 6:09 AM

Fixes a part of the CI issues.

Mordante updated this revision to Diff 528133.Jun 3 2023, 11:02 AM

Rebased and adjusted to upstream changes.

Mordante updated this revision to Diff 532213.Jun 16 2023, 10:20 AM

Rebased and updated modules to test with CI.

Mordante updated this revision to Diff 532251.Jun 16 2023, 11:36 AM

Rebased to fix CI.

Mordante updated this revision to Diff 532375.Jun 17 2023, 3:06 AM

CI fixes.

Mordante updated this revision to Diff 532381.Jun 17 2023, 4:08 AM

Update ABI list.

Mordante updated this revision to Diff 532461.Jun 18 2023, 3:20 AM

Fixes issues and test with reduced CI run.

Mordante updated this revision to Diff 532478.Jun 18 2023, 9:05 AM

Test CI and remove unwanted forwards.

Mordante added inline comments.Jun 18 2023, 9:13 AM
libcxx/include/print
327

Remove this comment.
I spoke with Victor about this in the context of P2905 R0 Runtime format strings and the std::forward should indeed be removed

Mordante updated this revision to Diff 532707.Jun 19 2023, 10:35 AM

Attempts to fix the CI.

Mordante updated this revision to Diff 532886.Jun 20 2023, 6:19 AM

Try to fix the CI.

Mordante updated this revision to Diff 533336.Jun 21 2023, 10:55 AM

Attempts to fix the CI.

Mordante updated this revision to Diff 533348.Jun 21 2023, 11:18 AM

Rebased and CI fixes.

Mordante updated this revision to Diff 534196.Jun 24 2023, 4:56 AM

CI fixes.

Mordante updated this revision to Diff 534199.Jun 24 2023, 5:29 AM

CI fixes.

Mordante updated this revision to Diff 534204.Jun 24 2023, 6:32 AM

CI fixes and update the lit flags.

Mordante updated this revision to Diff 534207.Jun 24 2023, 7:13 AM

CI fixes.

Mordante updated this revision to Diff 534214.Jun 24 2023, 7:39 AM

CI fixes.

Mordante updated this revision to Diff 534236.Jun 24 2023, 10:48 AM

Rebased, minimal improvement, and reanable full CI again.

Mordante updated this revision to Diff 534247.Jun 24 2023, 11:28 AM

Removes flush testing; its to unreliable.

Mordante updated this revision to Diff 534330.Jun 25 2023, 4:41 AM

CI fixes.

Mordante updated this revision to Diff 534349.Jun 25 2023, 8:36 AM

Trigger CI.

Mordante edited the summary of this revision. (Show Details)Jun 26 2023, 8:24 AM
Mordante updated this revision to Diff 534574.Jun 26 2023, 8:39 AM

Did a final review before publishing.

Mordante updated this revision to Diff 535060.Jun 27 2023, 10:58 AM

Fixes a typo that broke the Windows build.

Mordante published this revision for review.Jun 27 2023, 11:09 PM
Herald added a project: Restricted Project. · View Herald TranscriptJun 27 2023, 11:09 PM
Herald added a reviewer: Restricted Project. · View Herald Transcript

It's really cool to see this coming together slowly. I didn't do any in-depth review since I'm not familiar with most of the code, but I have a few comments.

libcxx/docs/Status/FormatPaper.csv
50–51

You probably meant LLVM?

libcxx/include/print
146

Would it make sense to use TODO PRINT or something like that instead? the actual output and formatting seem like two separate things to me.

194

Is this some sort of POSIX function? If yes, maybe we should check that we are on a POSIX system and error out otherwise?

302

I don't expect disabled wide characters and windows to be a common combination, so maybe just adding a compile time error would be the right thing to do here? That would probably be a lot easier to diagnose, and in the unlikely case that people complain we can still change it.

libcxx/src/print.cpp
18

Is lower or upper case the right thing here? I can never remember...

libcxx/test/libcxx/input.output/iostream.format/print.fun/vprint_unicode_windows.pass.cpp
42

This should probably not be here, since this test is currently disabled for GCC.

Mordante marked 5 inline comments as done.Jul 1 2023, 4:07 AM

Thanks for the review!

libcxx/docs/Status/FormatPaper.csv
50–51

This is what we did in the past, I agree it's not entirely correct. For now I keep it as is. In the end this page is intended to be removed once format is complete.

libcxx/include/print
146

Good point, mainly a force of habit to type FMT in format related places.
(Since print is quite small I prefer to track the status on the format page.)

Mordante added inline comments.Jul 1 2023, 4:07 AM
libcxx/include/print
194

Yes it's indeed POSIX and provided by unistd. I followed the method of src/filesystem/operations.cpp where we include this when not on Windows. Other places use #if __has_include(<unistd.h>), so I switch to that method.

302

Good point!

libcxx/src/print.cpp
18

I checked, we use lower case everywhere.

libcxx/test/libcxx/input.output/iostream.format/print.fun/vprint_unicode_windows.pass.cpp
42

I prefer to keep it since this was needed for GCC. I still hope we can enable format/print with GCC in the near future.

Mordante updated this revision to Diff 536525.Jul 1 2023, 4:28 AM
Mordante marked 5 inline comments as done.

Addresses review comments.

ldionne requested changes to this revision.Jul 11 2023, 10:01 AM

Nice, thanks! There's a lot of details in this patch, but it means that users would have a really hard time doing this right if they did it themselves!

libcxx/docs/Status/FormatPaper.csv
50–51

I think this should be 17.0, and this patch needs to be rebased.

libcxx/include/print
67
97–102

I don't think this hunk should be in this patch.

177

We could reorder like this instead, I think it would be easier to follow:

#ifdef _LIBCPP_HAS_NO_UNICODE
inline constexpr bool __use_unicode = false;
#elif defined(_MSVC_EXECUTION_CHARACTER_SET)
inline constexpr bool __use_unicode = _MSVC_EXECUTION_CHARACTER_SET == 65001;
#else
inline constexpr bool __use_unicode = true;
#endif

WDYT? This removes one layer.

179–180
200

Can you link to http://llvm.org/PR61563? Same thing below.

203

Let's use _LIBCPP_UNCATEGORIZED_ASSERT everywhere for now.

221
247
304
337–340

Suggestions:

  1. Don't define this function when unicode has been disabled. It's less surprising to have a compilation error when using this function on a system where unicode has been disabled than to call it and get non-unicode output.
  2. Don't define __vprint_unicode (above) when unicode is disabled.
  3. Switch the if constexpr (__print::__use_unicode) above in println and print to #if <USE UNICODE>
libcxx/src/print.cpp
44

I think this comment is outdated.

46

I don't think this function is needed outside of Windows anymore, since you now use a macro for testing. This would also get rid of the diff in the ABI list.

55

__throw_system_error? This way you don't need ifndef _LIBCPP_HAS_NO_EXCEPTIONS.

libcxx/test/libcxx/input.output/iostream.format/print.fun/transcoding.pass.cpp
39–42

I don't think this should be specific to Glibc?

libcxx/test/libcxx/input.output/iostream.format/print.fun/vprint_unicode_posix.pass.cpp
45
49

Maybe we can use ranges::all_of here too?

libcxx/test/std/input.output/iostream.format/print.fun/print.file.pass.cpp
68

We're not using _POSIX_C_SOURCE anywhere in the code base right now, and it looks like at least macOS doesn't define it properly. I generally try to steer away from checking specific macros in the test suite if I can because it's easy to disable tests without intending it.

Instead, I would suggest moving this test to another .pass.cpp file and we can mark it as XFAIL or UNSUPPORTED on the relevant platforms.

71
libcxx/test/std/input.output/iostream.format/print.fun/print_tests.h
68

Can you add some unicode test cases here? It should just write the data as-is IIUC.

libcxx/utils/libcxx/test/header_information.py
43

Can you keep this list sorted?

This revision now requires changes to proceed.Jul 11 2023, 10:01 AM
Mordante updated this revision to Diff 539392.Jul 11 2023, 11:07 PM

Addresses review comments.

Mordante marked 23 inline comments as done.Jul 11 2023, 11:14 PM

Thanks for the review!

libcxx/include/print
177

Yes this looks better.

200

I slightly changed the message to include the report, I prefer to keep it at this location instead of before the template. Some functions have useful comment which I don't want to clutter with this todo.

libcxx/test/libcxx/input.output/iostream.format/print.fun/transcoding.pass.cpp
39–42

Since this is not a part of this patch, I'll do the all_of change separately.

Mordante updated this revision to Diff 539634.Jul 12 2023, 10:48 AM
Mordante marked 2 inline comments as done.

CI fixes.

Mordante updated this revision to Diff 539661.Jul 12 2023, 11:44 AM

Retrigger CI.

Mordante edited the summary of this revision. (Show Details)Jul 12 2023, 12:10 PM
Mordante updated this revision to Diff 539672.Jul 12 2023, 12:13 PM

Rebased and removed parent dependencies. Hopefully this lets the CI ruun.

Mordante updated this revision to Diff 540080.Jul 13 2023, 9:35 AM

Fixes building on Windows.

Mordante added inline comments.Jul 13 2023, 9:39 AM
libcxx/src/print.cpp
52–53

This changes back to the original version. The function __throw_system_error is in the dylib but has no error code overload. I don't think we need to add an overload for one usage.

ldionne accepted this revision.Jul 18 2023, 9:25 AM
ldionne added inline comments.
libcxx/src/print.cpp
52–53

We could add an overload in another patch to address https://reviews.llvm.org/D154995#inline-1500381 at the same time.

This revision is now accepted and ready to land.Jul 18 2023, 9:25 AM
Mordante marked an inline comment as done.Jul 18 2023, 11:06 AM
Mordante added inline comments.
libcxx/src/print.cpp
52–53

As discussed I'll do that in a follow-up patch.

This revision was automatically updated to reflect the committed changes.
Mordante marked an inline comment as done.
hctim added a subscriber: hctim.EditedJul 19 2023, 3:24 AM

Edit: May have misattributed this due to changes in vprint files, but I'm bisecting now. There were ~4 other libcxx changes that I'm not 100% confident this is the culprit.

Hi folks,

Looks like this broke sanitizer buildbots, in https://lab.llvm.org/buildbot/#/builders/237/builds/3578:

llvm-libc++-shared.cfg.in :: std/input.output/iostream.format/print.fun/vprint_nonunicode.file.pass.cpp
llvm-libc++-shared.cfg.in :: std/input.output/iostream.format/print.fun/vprint_unicode.file.pass.cpp

(the other failure has since gone away)

********************
Testing:  0.. 10.. 20.. 30.. 40.. 50
FAIL: llvm-libc++-shared.cfg.in :: std/input.output/iostream.format/print.fun/vprint_unicode.file.pass.cpp (5373 of 10181)
******************** TEST 'llvm-libc++-shared.cfg.in :: std/input.output/iostream.format/print.fun/vprint_unicode.file.pass.cpp' FAILED ********************
Script:
--
: 'COMPILED WITH';  /b/sanitizer-aarch64-linux-bootstrap-msan/build/llvm_build0/bin/clang++ /b/sanitizer-aarch64-linux-bootstrap-msan/build/llvm-project/libcxx/test/std/input.output/iostream.format/print.fun/vprint_unicode.file.pass.cpp -pthread --target=aarch64-unknown-linux-gnu -g -fno-omit-frame-pointer -fsanitize=memory -nostdinc++ -I /b/sanitizer-aarch64-linux-bootstrap-msan/build/libcxx_build_msan/include/c++/v1 -I /b/sanitizer-aarch64-linux-bootstrap-msan/build/libcxx_build_msan/include/c++/v1 -I /b/sanitizer-aarch64-linux-bootstrap-msan/build/llvm-project/libcxx/test/support -std=c++26 -Werror -Wall -Wctad-maybe-unsupported -Wextra -Wshadow -Wundef -Wunused-template -Wno-unused-command-line-argument -Wno-attributes -Wno-pessimizing-move -Wno-noexcept-type -Wno-atomic-alignment -Wno-reserved-module-identifier -Wno-user-defined-literals -Wno-tautological-compare -Wsign-compare -Wunused-variable -Wunused-parameter -Wunreachable-code -Wno-unused-local-typedef -Wno-local-type-template-args -Wno-c++11-extensions -D_LIBCPP_HAS_NO_PRAGMA_SYSTEM_HEADER -D_LIBCPP_ENABLE_EXPERIMENTAL -Werror=thread-safety -Wuser-defined-warnings  -lc++experimental -nostdlib++ -L /b/sanitizer-aarch64-linux-bootstrap-msan/build/libcxx_build_msan/lib -Wl,-rpath,/b/sanitizer-aarch64-linux-bootstrap-msan/build/libcxx_build_msan/lib -lc++ -o /b/sanitizer-aarch64-linux-bootstrap-msan/build/libcxx_build_msan/test/std/input.output/iostream.format/print.fun/Output/vprint_unicode.file.pass.cpp.dir/t.tmp.exe
: 'EXECUTED AS';  "/usr/bin/python3.10" /b/sanitizer-aarch64-linux-bootstrap-msan/build/llvm-project/libcxx/test/../utils/run.py --execdir /b/sanitizer-aarch64-linux-bootstrap-msan/build/libcxx_build_msan/test/std/input.output/iostream.format/print.fun/Output/vprint_unicode.file.pass.cpp.dir --  /b/sanitizer-aarch64-linux-bootstrap-msan/build/libcxx_build_msan/test/std/input.output/iostream.format/print.fun/Output/vprint_unicode.file.pass.cpp.dir/t.tmp.exe
--
Exit Code: 250
Command Output (stdout):
--
$ ":" "COMPILED WITH"
note: command had no output on stdout or stderr
$ "/b/sanitizer-aarch64-linux-bootstrap-msan/build/llvm_build0/bin/clang++" "/b/sanitizer-aarch64-linux-bootstrap-msan/build/llvm-project/libcxx/test/std/input.output/iostream.format/print.fun/vprint_unicode.file.pass.cpp" "-pthread" "--target=aarch64-unknown-linux-gnu" "-g" "-fno-omit-frame-pointer" "-fsanitize=memory" "-nostdinc++" "-I" "/b/sanitizer-aarch64-linux-bootstrap-msan/build/libcxx_build_msan/include/c++/v1" "-I" "/b/sanitizer-aarch64-linux-bootstrap-msan/build/libcxx_build_msan/include/c++/v1" "-I" "/b/sanitizer-aarch64-linux-bootstrap-msan/build/llvm-project/libcxx/test/support" "-std=c++26" "-Werror" "-Wall" "-Wctad-maybe-unsupported" "-Wextra" "-Wshadow" "-Wundef" "-Wunused-template" "-Wno-unused-command-line-argument" "-Wno-attributes" "-Wno-pessimizing-move" "-Wno-noexcept-type" "-Wno-atomic-alignment" "-Wno-reserved-module-identifier" "-Wno-user-defined-literals" "-Wno-tautological-compare" "-Wsign-compare" "-Wunused-variable" "-Wunused-parameter" "-Wunreachable-code" "-Wno-unused-local-typedef" "-Wno-local-type-template-args" "-Wno-c++11-extensions" "-D_LIBCPP_HAS_NO_PRAGMA_SYSTEM_HEADER" "-D_LIBCPP_ENABLE_EXPERIMENTAL" "-Werror=thread-safety" "-Wuser-defined-warnings" "-lc++experimental" "-nostdlib++" "-L" "/b/sanitizer-aarch64-linux-bootstrap-msan/build/libcxx_build_msan/lib" "-Wl,-rpath,/b/sanitizer-aarch64-linux-bootstrap-msan/build/libcxx_build_msan/lib" "-lc++" "-o" "/b/sanitizer-aarch64-linux-bootstrap-msan/build/libcxx_build_msan/test/std/input.output/iostream.format/print.fun/Output/vprint_unicode.file.pass.cpp.dir/t.tmp.exe"
note: command had no output on stdout or stderr
$ ":" "EXECUTED AS"
note: command had no output on stdout or stderr
$ "/usr/bin/python3.10" "/b/sanitizer-aarch64-linux-bootstrap-msan/build/llvm-project/libcxx/test/../utils/run.py" "--execdir" "/b/sanitizer-aarch64-linux-bootstrap-msan/build/libcxx_build_msan/test/std/input.output/iostream.format/print.fun/Output/vprint_unicode.file.pass.cpp.dir" "--" "/b/sanitizer-aarch64-linux-bootstrap-msan/build/libcxx_build_msan/test/std/input.output/iostream.format/print.fun/Output/vprint_unicode.file.pass.cpp.dir/t.tmp.exe"
# command stderr:
libc++abi: terminating due to uncaught exception of type std::__1::format_error: Argument index out of bounds
error: command failed with exit status: 250

You can reproduce the exact bot with https://github.com/google/sanitizers/wiki/SanitizerBotReproduceBuild (buildbot_bootstrap_msan.sh is that particular bot), but I suspect a libcxxabi-libcxx bootstrap build should be sufficient to reproduce.

Edit: May have misattributed this due to changes in vprint files, but I'm bisecting now. There were ~4 other libcxx changes that I'm not 100% confident this is the culprit.

This indeed looks like the culprit.

You can reproduce the exact bot with https://github.com/google/sanitizers/wiki/SanitizerBotReproduceBuild (buildbot_bootstrap_msan.sh is that particular bot), but I suspect a libcxxabi-libcxx bootstrap build should be sufficient to reproduce.

Unfortunately I can't reproduce it locally (Linux x86_64). I see https://lab.llvm.org/buildbot/#/builders/74 the x86_64 msan buildbot is happy but the aarch64. The exception in the logs is thrown in the test, but it seems it's not caught. Is it possible to see where the exception is thrown? Or can I get access to the buildbot to run some tests?

Unfortunately I can't reproduce it locally (Linux x86_64). I see https://lab.llvm.org/buildbot/#/builders/74 the x86_64 msan buildbot is happy but the aarch64. The exception in the logs is thrown in the test, but it seems it's not caught. Is it possible to see where the exception is thrown? Or can I get access to the buildbot to run some tests?

Let me see if I can get you a stack trace (it might take me until ~tomorrow). Is it possible to revert/skip these tests on aarch64 in the meantime?

Unfortunately I can't reproduce it locally (Linux x86_64). I see https://lab.llvm.org/buildbot/#/builders/74 the x86_64 msan buildbot is happy but the aarch64. The exception in the logs is thrown in the test, but it seems it's not caught. Is it possible to see where the exception is thrown? Or can I get access to the buildbot to run some tests?

Let me see if I can get you a stack trace (it might take me until ~tomorrow). Is it possible to revert/skip these tests on aarch64 in the meantime?

Thanks! It's a bit tricky to find the proper flags to only disable it on aarch64 with msan. Instead I'll disable it for all msan builds.

hctim added a comment.Jul 20 2023, 7:54 AM

Thanks! It's a bit tricky to find the proper flags to only disable it on aarch64 with msan. Instead I'll disable it for all msan builds.

Unfortunately that doesn't quite do the trick, as we're also seeing the bug on the aarch64 asan and aarch64 hwasan bots as well :(.

Thanks! It's a bit tricky to find the proper flags to only disable it on aarch64 with msan. Instead I'll disable it for all msan builds.

Unfortunately that doesn't quite do the trick, as we're also seeing the bug on the aarch64 asan and aarch64 hwasan bots as well :(.

I indeed only turned of msan, doing it for asan and hwsan is trivial. Are the other sanitizers that fail?

hctim added a comment.Jul 20 2023, 9:09 AM

I indeed only turned of msan, doing it for asan and hwsan is trivial. Are the other sanitizers that fail?

The aarch64-ubsan build bot is fine, so it's just aarch64_{asan,msan,hwasan}. Still looking into getting you a stack trace or some other debugging info.

I indeed only turned of msan, doing it for asan and hwsan is trivial. Are the other sanitizers that fail?

The aarch64-ubsan build bot is fine, so it's just aarch64_{asan,msan,hwasan}. Still looking into getting you a stack trace or some other debugging info.

I've committed a fix for these two sanitizers too. Thanks for the info.

$ lldb /home/mitchp/build/libcxx_build_msan/test/std/input.output/iostream.format/print.fun/Output/vprint_nonunicode.file.pass.cpp.dir/t.tmp.exe
(lldb) target create "/home/mitchp/build/libcxx_build_msan/test/std/input.output/iostream.format/print.fun/Output/vprint_nonunicode.file.pass.cpp.dir/t.tmp.exe"                                            Current executable set to '/home/mitchp/build/libcxx_build_msan/test/std/input.output/iostream.format/print.fun/Output/vprint_nonunicode.file.pass.cpp.dir/t.tmp.exe' (aarch64).                            (lldb) r
Process 737940 launched: '/home/mitchp/build/libcxx_build_msan/test/std/input.output/iostream.format/print.fun/Output/vprint_nonunicode.file.pass.cpp.dir/t.tmp.exe' (aarch64)
Process 737940 stopped and restarted: thread 1 received signal: SIGCHLD
libc++abi: terminating due to uncaught exception of type std::__1::format_error: Argument index out of bounds
Process 737940 stopped
* thread #1, name = 't.tmp.exe', stop reason = signal SIGABRT
    frame #0: 0x0000fffff7ac7eac libc.so.6`__GI_raise(sig=6) at raise.c:51:1
(lldb) bt
* thread #1, name = 't.tmp.exe', stop reason = signal SIGABRT
  * frame #0: 0x0000fffff7ac7eac libc.so.6`__GI_raise(sig=6) at raise.c:51:1
    frame #1: 0x0000fffff7ab4aa0 libc.so.6`__GI_abort at abort.c:79:7
    frame #2: 0x0000fffff7dd4948 libc++abi.so.1`abort_message at abort_message.cpp:78:5
    frame #3: 0x0000fffff7d8a1f0 libc++abi.so.1`::demangling_terminate_handler() at cxa_default_handlers.cpp:72:9
    frame #4: 0x0000fffff7dd3138 libc++abi.so.1`std::__terminate(void (*)()) at cxa_handlers.cpp:59:9
    frame #5: 0x0000fffff7dda23c libc++abi.so.1`__cxxabiv1::failed_throw(__cxxabiv1::__cxa_exception*) at cxa_exception.cpp:152:5
    frame #6: 0x0000fffff7dda168 libc++abi.so.1`::__cxa_throw() at cxa_exception.cpp:283:5
    frame #7: 0x0000aaaaaabdbfac t.tmp.exe`std::__1::__throw_format_error[abi:v170000](__s="Argument index out of bounds") at format_error.h:41:3
    frame #8: 0x0000aaaaaabdf058 t.tmp.exe`auto char const* std::__1::__format::__handle_replacement_field[abi:v170000]<char const*, std::__1::basic_format_parse_context<char>, std::__1::basic_format_cont
ext<std::__1::back_insert_iterator<std::__1::__format::__output_buffer<char>>, char>>(this=0x0000ffffffffe0c0, __arg=monostate @ 0x0000ffffffffdeaf)::'lambda'(char const*)::operator()<std::__1::monostate>
(char const*) const at format_functions.h:274:13
    frame #9: 0x0000aaaaaabdefe8 t.tmp.exe`decltype(std::declval<char const*>()(std::declval<std::__1::monostate&>())) std::__1::__invoke[abi:v170000]<char const* std::__1::__format::__handle_replacement_
field[abi:v170000]<char const*, std::__1::basic_format_parse_context<char>, std::__1::basic_format_context<std::__1::back_insert_iterator<std::__1::__format::__output_buffer<char>>, char>>(char const*, ch
ar const*, std::__1::basic_format_parse_context<char>&, std::__1::basic_format_context<std::__1::back_insert_iterator<std::__1::__format::__output_buffer<char>>, char>&)::'lambda'(char const*), std::__1::
monostate&>(__f=0x0000ffffffffe0c0, __args=0x0000ffffffffe0a0) at invoke.h:340:25
    frame #10: 0x0000aaaaaabde430 t.tmp.exe`std::__1::invoke_result<char const*, std::__1::monostate&>::type std::__1::invoke[abi:v170000]<char const* std::__1::__format::__handle_replacement_field[abi:v1
70000]<char const*, std::__1::basic_format_parse_context<char>, std::__1::basic_format_context<std::__1::back_insert_iterator<std::__1::__format::__output_buffer<char>>, char>>(char const*, char const*, s
td::__1::basic_format_parse_context<char>&, std::__1::basic_format_context<std::__1::back_insert_iterator<std::__1::__format::__output_buffer<char>>, char>&)::'lambda'(char const*), std::__1::monostate&>(
__f=0x0000ffffffffe0c0, __args=0x0000ffffffffe0a0) at invoke.h:30:12
    frame #11: 0x0000aaaaaabdcfa0 t.tmp.exe`decltype(auto) std::__1::__visit_format_arg[abi:v170000]<char const* std::__1::__format::__handle_replacement_field[abi:v170000]<char const*, std::__1::basic_fo
rmat_parse_context<char>, std::__1::basic_format_context<std::__1::back_insert_iterator<std::__1::__format::__output_buffer<char>>, char>>(char const*, char const*, std::__1::basic_format_parse_context<ch
ar>&, std::__1::basic_format_context<std::__1::back_insert_iterator<std::__1::__format::__output_buffer<char>>, char>&)::'lambda'(char const*), std::__1::basic_format_context<std::__1::back_insert_iterato
r<std::__1::__format::__output_buffer<char>>, char>>(__vis=0x0000ffffffffe0c0, __arg=basic_format_arg<std::__1::basic_format_context<std::__1::back_insert_iterator<std::__1::__format::__output_buffer<char
> >, char> > @ 0x0000ffffffffe0a0) at format_arg.h:103:12
    frame #12: 0x0000aaaaaabdc404 t.tmp.exe`char const* std::__1::__format::__handle_replacement_field[abi:v170000]<char const*, std::__1::basic_format_parse_context<char>, std::__1::basic_format_context<
std::__1::back_insert_iterator<std::__1::__format::__output_buffer<char>>, char>>(__begin="}", __end="", __parse_ctx=0x0000ffffffffe408, __ctx=0x0000ffffffffe3c8) at format_functions.h:271:5
    frame #13: 0x0000aaaaaabd7ddc t.tmp.exe`std::__1::basic_format_context<std::__1::back_insert_iterator<std::__1::__format::__output_buffer<char>>, char>::iterator std::__1::__format::__vformat_to[abi:v
170000]<std::__1::basic_format_parse_context<char>, std::__1::basic_format_context<std::__1::back_insert_iterator<std::__1::__format::__output_buffer<char>>, char>>(__parse_ctx=0x0000ffffffffe408, __ctx=0
x0000ffffffffe3c8) at format_functions.h:312:13
    frame #14: 0x0000aaaaaabd7738 t.tmp.exe`std::__1::back_insert_iterator<std::__1::basic_string<char, std::__1::char_traits<char>, std::__1::allocator<char>>> std::__1::__vformat_to[abi:v170000]<std::__1::back_insert_iterator<std::__1::basic_string<char, std::__1::char_traits<char>, std::__1::allocator<char>>>, char, std::__1::back_insert_iterator<std::__1::__format::__output_buffer<char>>>(__out_it=(container = ""), __fmt="hello {}", __args=basic_format_args<std::__1::basic_format_context<std::__1::back_insert_iterator<std::__1::__format::__output_buffer<char> >, char> > @ 0x0000ffffffffe808) at format_functions.h:387:5
    frame #15: 0x0000aaaaaabd6ee4 t.tmp.exe`void std::__1::__print::__vprint_nonunicode[abi:v170000]<void>(_IO_FILE*, std::__1::basic_string_view<char, std::__1::char_traits<char>>, std::__1::basic_format_args<std::__1::basic_format_context<std::__1::back_insert_iterator<std::__1::__format::__output_buffer<char>>, char>>, bool) [inlined] std::__1::back_insert_iterator<std::__1::basic_string<char, std::__1::char_traits<char>, std::__1::allocator<char>>> std::__1::vformat_to[abi:v170000]<std::__1::back_insert_iterator<std::__1::basic_string<char, std::__1::char_traits<char>, std::__1::allocator<char>>>>(__out_it=(container = ""), __fmt="hello {}", __args=std::__1::format_args @ 0x0000ffffffffe7a0) at format_functions.h:399:10
    frame #16: 0x0000aaaaaabd6e5c t.tmp.exe`void std::__1::__print::__vprint_nonunicode[abi:v170000]<void>(_IO_FILE*, std::__1::basic_string_view<char, std::__1::char_traits<char>>, std::__1::basic_format_args<std::__1::basic_format_context<std::__1::back_insert_iterator<std::__1::__format::__output_buffer<char>>, char>>, bool) [inlined] std::__1::basic_string<char, std::__1::char_traits<char>, std::__1::allocator<char>> std::__1::vformat[abi:v170000]<void>(__fmt="hello {}", __args=std::__1::format_args @ 0x0000ffffffffe730) at format_functions.h:432:3
    frame #17: 0x0000aaaaaabd6d3c t.tmp.exe`void std::__1::__print::__vprint_nonunicode[abi:v170000]<void>(__stream=0x0000e14000000000, __fmt="hello {}", __args=std::__1::format_args @ 0x0000ffffffffe8c8, __write_nl=false) at print:208:18
    frame #18: 0x0000aaaaaabd6850 t.tmp.exe`void std::__1::vprint_nonunicode[abi:v170000]<void>(__stream=0x0000e14000000000, __fmt="hello {}", __args=std::__1::format_args @ 0x0000ffffffffea38) at print:355:3
    frame #19: 0x0000aaaaaabc1510 t.tmp.exe`auto $_1::operator()<>(this=0x0000fffffffff2f6, what="Argument index out of bounds", fmt="hello {}") const at vprint_nonunicode.file.pass.cpp:57:3
    frame #20: 0x0000aaaaaabbb3f0 t.tmp.exe`void print_tests<$_0, $_1>(check=(unnamed class) @ 0x0000fffffffff2f7, check_exception=(unnamed class) @ 0x0000fffffffff2f6) at print_tests.h:78:3
    frame #21: 0x0000aaaaaabb9d9c t.tmp.exe`main((null)=1, (null)=0x0000fffffffff4e8) at vprint_nonunicode.file.pass.cpp:134:3
    frame #22: 0x0000fffff7ab4e18 libc.so.6`__libc_start_main(main=(t.tmp.exe`main at vprint_nonunicode.file.pass.cpp:133), argc=1, argv=0x0000fffffffff4e8, init=<unavailable>, fini=<unavailable>, rtld_fini=<unavailable>, stack_end=<unavailable>) at libc-start.c:308:16
    frame #23: 0x0000aaaaaab31df4 t.tmp.exe`_start + 52
$ lldb /home/mitchp/build/libcxx_build_msan/test/std/input.output/iostream.format/print.fun/Output/vprint_nonunicode.file.pass.cpp.dir/t.tmp.exe
(lldb) target create "/home/mitchp/build/libcxx_build_msan/test/std/input.output/iostream.format/print.fun/Output/vprint_nonunicode.file.pass.cpp.dir/t.tmp.exe"                                            Current executable set to '/home/mitchp/build/libcxx_build_msan/test/std/input.output/iostream.format/print.fun/Output/vprint_nonunicode.file.pass.cpp.dir/t.tmp.exe' (aarch64).                            (lldb) r
Process 737940 launched: '/home/mitchp/build/libcxx_build_msan/test/std/input.output/iostream.format/print.fun/Output/vprint_nonunicode.file.pass.cpp.dir/t.tmp.exe' (aarch64)
Process 737940 stopped and restarted: thread 1 received signal: SIGCHLD
libc++abi: terminating due to uncaught exception of type std::__1::format_error: Argument index out of bounds
Process 737940 stopped
* thread #1, name = 't.tmp.exe', stop reason = signal SIGABRT
    frame #0: 0x0000fffff7ac7eac libc.so.6`__GI_raise(sig=6) at raise.c:51:1
(lldb) bt
* thread #1, name = 't.tmp.exe', stop reason = signal SIGABRT
  * frame #0: 0x0000fffff7ac7eac libc.so.6`__GI_raise(sig=6) at raise.c:51:1
    frame #1: 0x0000fffff7ab4aa0 libc.so.6`__GI_abort at abort.c:79:7
    frame #2: 0x0000fffff7dd4948 libc++abi.so.1`abort_message at abort_message.cpp:78:5
    frame #3: 0x0000fffff7d8a1f0 libc++abi.so.1`::demangling_terminate_handler() at cxa_default_handlers.cpp:72:9
    frame #4: 0x0000fffff7dd3138 libc++abi.so.1`std::__terminate(void (*)()) at cxa_handlers.cpp:59:9
    frame #5: 0x0000fffff7dda23c libc++abi.so.1`__cxxabiv1::failed_throw(__cxxabiv1::__cxa_exception*) at cxa_exception.cpp:152:5
    frame #6: 0x0000fffff7dda168 libc++abi.so.1`::__cxa_throw() at cxa_exception.cpp:283:5
    frame #7: 0x0000aaaaaabdbfac t.tmp.exe`std::__1::__throw_format_error[abi:v170000](__s="Argument index out of bounds") at format_error.h:41:3

This looks suspicious.
The code throws an exception which I verified is intended.
Then the libc++abi calls failed_throw which causes termination.
Is there a way to get more information about what happens in frame #6? Here it seems the exception is not able to find the proper handler for the exception and thus calling terminate.

Mordante added inline comments.Jul 22 2023, 9:26 AM
libcxx/src/print.cpp
55

D156019 is the followup.

rprichard added inline comments.
libcxx/include/print
209

I'm wondering about this usage of std::ferror. The first argument to std::__throw_system_error appears to be a Unix errno value, whereas ferror appears to returns non-zero if the stream's error indicator flag is set.

e.g. The (print.file.pass.cpp, test_read_only) test opens a read-only file and tries to print to it. fwrite to such a stream fails with EBADF(9). glibc sets the stream's error indicator, so ferror returns 1, which is interpreted as EPERM(1). With Bionic, ferror instead returns 0, so this test is failing on Bionic/Android, because there is no OS error string suffixed to the what() result.

uabelho added inline comments.
libcxx/test/libcxx/input.output/iostream.format/print.fun/vprint_unicode_posix.pass.cpp
41

When I run this testcase the assert on line 48 below fails for me, and it seems to be because the "fmemopen" call here writes '\0' at the first byte in buffer.

I really doen't know anything about this and if I read at
https://man7.org/linux/man-pages/man3/fmemopen.3.html
it says

w      The stream is opened for writing.

and also

w+     Open the stream for reading and writing.  The buffer
       contents are truncated (i.e., '\0' is placed in the first
       byte of the buffer).

So it seems like when I run this in my environment "w" partly does what "w+" should do.

Does this problem sound familiar to anyone?

tahonermann added inline comments.Aug 24 2023, 6:46 AM
libcxx/test/libcxx/input.output/iostream.format/print.fun/vprint_unicode_posix.pass.cpp
41

Are you by chance using an old glibc version from 2015 or earlier? Early implementations of fmemopen() unconditionally truncated the buffer when opened for write. This was changed in this commit:

Alternatively, the "old" implementation replaced by the commit above is still present in current glibc releases under the name __old_fmemopen. Perhaps your builds are somehow calling that implementation?

uabelho added inline comments.Aug 24 2023, 10:13 PM
libcxx/test/libcxx/input.output/iostream.format/print.fun/vprint_unicode_posix.pass.cpp
41

Yes, seems like we're using ancient glibc versions on our servers.

That explains our problem then. Thank you!

ldionne added inline comments.Sep 12 2023, 10:29 AM
libcxx/include/print
209