IssueTracker

I have been trying to reproduce the "garbage" output on godbolt, but I failed. Are you doing something special that produces this output? It is not shown if you simply follow the link you have provided.

In the link I shared it results in a segfault. This is an example where it outputs a garbage string: https://godbolt.org/z/f841678fW

I can reproduce the output and it triggers valgrind and address sanitizer diagnostics.
==234974==ERROR: AddressSanitizer: stack-buffer-overflow on address 0x7f77bad00438 at pc 0x55b43b424449 bp 0x7fff81a59f20 sp 0x7fff81a59f18
READ of size 1 at 0x7f77bad00438 thread T0
...
  This frame has 13 object(s):
    [32, 56) '__args.byval' <== Memory access at offset 56 overflows this variable
    [96, 104) 'retval'
    [128, 136) '__out_it'
    [160, 176) '__fmt'
    [192, 496) '__buffer' (line 398)
    [560, 568) 'agg.tmp'
    [592, 632) 'ref.tmp' (line 399)
    [672, 688) 'agg.tmp2'
    [704, 752) 'ref.tmp3' (line 399)
    [784, 792) 'agg.tmp4'
    [816, 840) 'agg.tmp7'
    [880, 896) 'ref.tmp8' (line 399)
    [912, 920) 'coerce'
...

And Valgrind
==235260== Syscall param write(buf) points to uninitialised byte(s)
==235260==    at 0x4BE9077: write (write.c:26)
==235260==    by 0x4B69E8C: _IO_file_write@@GLIBC_2.2.5 (fileops.c:1181)
==235260==    by 0x4B6B950: new_do_write (fileops.c:449)
==235260==    by 0x4B6B950: _IO_new_do_write (fileops.c:426)
==235260==    by 0x4B6B950: _IO_do_write@@GLIBC_2.2.5 (fileops.c:423)
==235260==    by 0x4B69477: _IO_file_sync@@GLIBC_2.2.5 (fileops.c:799)
==235260==    by 0x4B5D3C5: fflush (iofflush.c:40)
==235260==    by 0x48C92F3: ??? (in /usr/lib/llvm-10/lib/libc++.so.1.0)
==235260==    by 0x48BE5F2: std::__1::basic_ostream<char, std::__1::char_traits<char> >::flush() (in /usr/lib/llvm-10/lib/libc++.so.1.0)
==235260==    by 0x48C89DC: ??? (in /usr/lib/llvm-10/lib/libc++.so.1.0)
==235260==    by 0x4B21FDD: __cxa_finalize (cxa_finalize.c:83)
==235260==    by 0x48AFF86: ??? (in /usr/lib/llvm-10/lib/libc++.so.1.0)
==235260==    by 0x4011F6A: _dl_fini (dl-fini.c:138)
==235260==    by 0x4B218A6: __run_exit_handlers (exit.c:108)
==235260==  Address 0x4cff305 is 21 bytes inside a block of size 1,024 alloc'd
==235260==    at 0x483B7F3: malloc (in /usr/lib/x86_64-linux-gnu/valgrind/vgpreload_memcheck-amd64-linux.so)
==235260==    by 0x4B5CD03: _IO_file_doallocate (filedoalloc.c:101)
==235260==    by 0x4B6CECF: _IO_doallocbuf (genops.c:347)
==235260==    by 0x4B6BF2F: _IO_file_overflow@@GLIBC_2.2.5 (fileops.c:745)
==235260==    by 0x4B6A6B4: _IO_new_file_xsputn (fileops.c:1244)
==235260==    by 0x4B6A6B4: _IO_file_xsputn@@GLIBC_2.2.5 (fileops.c:1197)
==235260==    by 0x4B5E3C0: fwrite (iofwrite.c:39)

With the current relase I did have to use -fexperimental-library to get access to libfmt, at least in LLVM 16. The libc++ documentation on that flag says https://libcxx.llvm.org/UsingLibcxx.html#enabling-experimental-c-library-features that it is for features that aren't implementation complete or are not ABI stable. It doesn't warn that users should expect more bugs, although that might be implicit in the use of experimental.

Regardless I think we'll need some input from the libc++ developers on what the scope of the problem is and how they want to proceed.

It is still marked experimental in Clang 16, so might not be as big a security issue.
It is marked non experimental in main, so should be fixed before the Clang 17 release:

    [libc++][format] Removes the experimental status.
Committer: Mark de Wever <koraq@xs4all.nl>  2023-05-24 17:16:22


I did some debugging and found the issue:

The __copy function fills the buffer (of size 256) until the very end and does not call flush: https://github.com/llvm/llvm-project/blob/10417b1359c2ba7cde2c40e61530f7de30b6d199/libcxx/include/__format/buffer.h#L111

Then push_back appends to the buffer: https://github.com/llvm/llvm-project/blob/10417b1359c2ba7cde2c40e61530f7de30b6d199/libcxx/include/__format/buffer.h#L82
And then it checks if _size == __capacity_  but that will never be true, as size is already 257 and capacity still 256

Putting Mark de Wever on CC for feedback.

RC1 for LLVM 17 is currently planned for July 27th (according to https://discourse.llvm.org/t/llvm-17-0-0-release-planning-and-update/71762), some fix is definitely needed until then as it's not marked experimental there anymore.

Mark de Wever doesn't have a Google account and therefore does not have access to this issue.
I shared the comments above with him and he said they are working on a fix.

https://reviews.llvm.org/D155397 has just landed on main and should fix this issue.

[Empty comment from Monorail migration]

This issue was migrated from crbug.com/llvm/48?no_tracker_redirect=1

[Auto-CCs applied]