Skip to content
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

[libc++][format][chrono] Problems with chrono-specs with short lifetime #62074

Closed
JMazurkiewicz opened this issue Apr 11, 2023 · 3 comments
Closed
Assignees
Labels
libc++ libc++ C++ Standard Library. Not GNU libstdc++. Not libc++abi.

Comments

@JMazurkiewicz
Copy link
Contributor

JMazurkiewicz commented Apr 11, 2023

Repro:

#include <chrono>
#include <format>
#include <iostream>

struct Midnight {};

template <> struct std::formatter<Midnight> {
private:
  std::formatter<std::chrono::hours> underlying;

public:
  constexpr auto parse(auto &ctx) {
    const auto it = ctx.begin();
    if (it != ctx.end() && *it != '}') { // Let's require `midnight-spec` to be empty.
      throw std::format_error{"Format spec for Midnight should be empty"};
    } else {
      char spec[] = "%T %p"; // Variable with automatic storage duration, not available after leaving this function.
      format_parse_context new_ctx{spec};
      underlying.parse(new_ctx); // Underlying keeps `string_view` pointing to `spec` automatic variable!
      return it;
    }
  }

  auto format(Midnight, auto &ctx) const {
    return underlying.format(std::chrono::hours{0}, ctx); // Sheesh!
  }
};

int main() { std::cout << std::format("{}\n", Midnight{}); }

Build:

PS D:\libcxx-playground> clang -std=c++2b -fexperimental-library -Iinclude\c++\v1 -llib\c++ "-llib\windows\clang_rt.builtins-x86_64" .\test.cpp
PS D:\libcxx-playground> .\a.exe
[garbage]

Currently libc++ stores chrono-specs as basic_string_view, which means that it might become dangling in some pathological cases (like the one presented here).

Other builds:

Clang/libc++/ASAN/Windows
PS D:\libcxx-playground> clang -std=c++2b -fexperimental-library -fsanitize=address -g -O2 -Iinclude\c++\v1 -llib\c++ "-llib\windows\clang_rt.builtins-x86_64" .\test.cpp
   Creating library a.lib and object a.exp
PS D:\libcxx-playground> .\a.exe
=================================================================
==13728==ERROR: AddressSanitizer: stack-use-after-scope on address 0x00901cfeeab0 at pc 0x7ff795fd48b4 bp 0x00901cfedfa0 sp 0x00901cfedfe8
READ of size 1 at 0x00901cfeeab0 thread T0
    #0 0x7ff795fd48b3 in std::__1::__formatter::__format_chrono_using_chrono_specs<char, class std::__1::chrono::duration<long, class std::__1::ratio<3600, 1>>>(class std::__1::chrono::duration<long, class std::__1::ratio<3600, 1>> const &, class std::__1::basic_stringstream<char, struct std::__1::char_traits<char>, class std::__1::allocator<char>> &, class std::__1::basic_string_view<char, struct std::__1::char_traits<char>>) D:\libcxx-playground\include\c++\v1\__chrono\formatter.h:179
    #1 0x7ff795fd1275 in std::__1::__formatter::__format_chrono<char, class std::__1::chrono::duration<long, class std::__1::ratio<3600, 1>>, class std::__1::basic_format_context<class std::__1::back_insert_iterator<class std::__1::__format::__output_buffer<char>>, char>>(class std::__1::chrono::duration<long, class std::__1::ratio<3600, 1>> const &, class std::__1::basic_format_context<class std::__1::back_insert_iterator<class std::__1::__format::__output_buffer<char>>, char> &, struct std::__1::__format_spec::__parsed_specifications<char>, class std::__1::basic_string_view<char, struct std::__1::char_traits<char>>) D:\libcxx-playground\include\c++\v1\__chrono\formatter.h:510
    #2 0x7ff795fcfd7f in std::__1::__formatter_chrono<char>::format D:\libcxx-playground\include\c++\v1\__chrono\formatter.h:576
    #3 0x7ff795fcfd7f in std::__1::formatter<Midnight,char>::format D:\libcxx-playground\test.cpp:25
    #4 0x7ff795fcfd7f in `public: __cdecl std::__1::__basic_format_arg_value<class std::__1::basic_format_context<class std::__1::back_insert_iterator<class std::__1::__format::__output_buffer<char>>, char>>::__handle::__handle<struct Midnight &>(struct Midnight &)'::`1'::<lambda_1>::operator()(class std::__1::basic_format_parse_context<char> &, class std::__1::basic_format_context<class std::__1::back_insert_iterator<class std::__1::__format::__output_buffer<char>>, char> &, void const *) const D:\libcxx-playground\include\c++\v1\__format\format_arg.h:172
    #5 0x7ff795fcf9cd in `public: __cdecl std::__1::__basic_format_arg_value<class std::__1::basic_format_context<class std::__1::back_insert_iterator<class std::__1::__format::__output_buffer<char>>, char>>::__handle::__handle<struct Midnight &>(struct Midnight &)'::`1'::<lambda_1>::__invoke(class std::__1::basic_format_parse_context<char> &, class std::__1::basic_format_context<class std::__1::back_insert_iterator<class std::__1::__format::__output_buffer<char>>, char> &, void const *) D:\libcxx-playground\include\c++\v1\__format\format_arg.h:161
    #6 0x7ff795fa5d3a in std::__1::basic_format_arg<std::__1::basic_format_context<std::__1::back_insert_iterator<std::__1::__format::__output_buffer<char> >,char> >::handle::format D:\libcxx-playground\include\c++\v1\__format\format_arg.h:267
    #7 0x7ff795fa5d3a in std::__1::__format::__handle_replacement_field<const char *,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_1>::operator() D:\libcxx-playground\include\c++\v1\__format\format_functions.h:279
    #8 0x7ff795fa5d3a in std::__1::__invoke D:\libcxx-playground\include\c++\v1\__functional\invoke.h:394
    #9 0x7ff795fa5d3a in std::__1::invoke D:\libcxx-playground\include\c++\v1\__functional\invoke.h:539
    #10 0x7ff795fa5d3a in std::__1::__visit_format_arg<class `char const * __cdecl std::__1::__format::__handle_replacement_field<char const *, class std::__1::basic_format_parse_context<char>, class std::__1::basic_format_context<class std::__1::back_insert_iterator<class std::__1::__format::__output_buffer<char>>, char>>(char const *, char const *, class std::__1::basic_format_parse_context<char> &, class std::__1::basic_format_context<class std::__1::back_insert_iterator<class std::__1::__format::__output_buffer<char>>, char> &)'::`1'::<lambda_1>, class std::__1::basic_format_context<class std::__1::back_insert_iterator<class std::__1::__format::__output_buffer<char>>, char>>(class `char const * __cdecl std::__1::__format::__handle_replacement_field<char const *, class std::__1::basic_format_parse_context<char>, class std::__1::basic_format_context<class std::__1::back_insert_iterator<class std::__1::__format::__output_buffer<char>>, char>>(char const *, char const *, class std::__1::basic_format_parse_context<char> &, class std::__1::basic_format_context<class std::__1::back_insert_iterator<class std::__1::__format::__output_buffer<char>>, char> &)'::`1'::<lambda_1> &&, class std::__1::basic_format_arg<class std::__1::basic_format_context<class std::__1::back_insert_iterator<class std::__1::__format::__output_buffer<char>>, char>>) D:\libcxx-playground\include\c++\v1\__format\format_arg.h:140
    #11 0x7ff795fa4ced in std::__1::__format::__handle_replacement_field<char const *, class std::__1::basic_format_parse_context<char>, class std::__1::basic_format_context<class std::__1::back_insert_iterator<class std::__1::__format::__output_buffer<char>>, char>>(char const *, char const *, class std::__1::basic_format_parse_context<char> &, class std::__1::basic_format_context<class std::__1::back_insert_iterator<class std::__1::__format::__output_buffer<char>>, char> &) D:\libcxx-playground\include\c++\v1\__format\format_functions.h:274
    #12 0x7ff795fa2080 in std::__1::__format::__vformat_to<class std::__1::basic_format_parse_context<char>, class std::__1::basic_format_context<class std::__1::back_insert_iterator<class std::__1::__format::__output_buffer<char>>, char>>(class std::__1::basic_format_parse_context<char> &&, class std::__1::basic_format_context<class std::__1::back_insert_iterator<class std::__1::__format::__output_buffer<char>>, char> &&) D:\libcxx-playground\include\c++\v1\__format\format_functions.h:314
    #13 0x7ff795fa197d in std::__1::__vformat_to<class std::__1::back_insert_iterator<class std::__1::basic_string<char, struct std::__1::char_traits<char>, class std::__1::allocator<char>>>, char, class std::__1::back_insert_iterator<class std::__1::__format::__output_buffer<char>>>(class std::__1::back_insert_iterator<class std::__1::basic_string<char, struct std::__1::char_traits<char>, class std::__1::allocator<char>>>, class std::__1::basic_string_view<char, struct std::__1::char_traits<char>>, class std::__1::basic_format_args<class std::__1::basic_format_context<class std::__1::back_insert_iterator<class std::__1::__format::__output_buffer<char>>, char>>) D:\libcxx-playground\include\c++\v1\__format\format_functions.h:405
    #14 0x7ff795fa125e in std::__1::vformat_to D:\libcxx-playground\include\c++\v1\__format\format_functions.h:417
    #15 0x7ff795fa125e in std::__1::vformat D:\libcxx-playground\include\c++\v1\__format\format_functions.h:450
    #16 0x7ff795fa125e in std::__1::format D:\libcxx-playground\include\c++\v1\__format\format_functions.h:469
    #17 0x7ff795fa125e in main D:\libcxx-playground\test.cpp:29
    #18 0x7ff796027f1b in invoke_main D:\a\_work\1\s\src\vctools\crt\vcstartup\src\startup\exe_common.inl:78
    #19 0x7ff796027f1b in __scrt_common_main_seh D:\a\_work\1\s\src\vctools\crt\vcstartup\src\startup\exe_common.inl:288
    #20 0x7ffea8fb7603  (C:\WINDOWS\System32\KERNEL32.DLL+0x180017603)
    #21 0x7ffea9a226a0  (C:\WINDOWS\SYSTEM32\ntdll.dll+0x1800526a0)

Address 0x00901cfeeab0 is located in stack of thread T0 at offset 176 in frame
    #0 0x7ff795fcfa1f in `public: __cdecl std::__1::__basic_format_arg_value<class std::__1::basic_format_context<class std::__1::back_insert_iterator<class std::__1::__format::__output_buffer<char>>, char>>::__handle::__handle<struct Midnight &>(struct Midnight &)'::`1'::<lambda_1>::operator()(class std::__1::basic_format_parse_context<char> &, class std::__1::basic_format_context<class std::__1::back_insert_iterator<class std::__1::__format::__output_buffer<char>>, char> &, void const *) const D:\libcxx-playground\include\c++\v1\__format\format_arg.h:161

  This frame has 8 object(s):
    [32, 48) 'agg.tmp.i.i'
    [64, 80) 'agg.tmp2.i.i'
    [96, 100) 'ref.tmp.i'
    [112, 136) 'tmp.i'
    [176, 182) 'spec.i' <== Memory access at offset 176 is inside this variable
    [208, 248) 'new_ctx.i'
    [288, 320) '__f' (line 170)
    [352, 360) 'agg.tmp'
HINT: this may be a false positive if your program uses some custom stack unwind mechanism, swapcontext or vfork
      (longjmp, SEH and C++ exceptions *are* supported)
SUMMARY: AddressSanitizer: stack-use-after-scope D:\libcxx-playground\include\c++\v1\__chrono\formatter.h:179 in std::__1::__formatter::__format_chrono_using_chrono_specs<char, class std::__1::chrono::duration<long, class std::__1::ratio<3600, 1>>>(class std::__1::chrono::duration<long, class std::__1::ratio<3600, 1>> const &, class std::__1::basic_stringstream<char, struct std::__1::char_traits<char>, class std::__1::allocator<char>> &, class std::__1::basic_string_view<char, struct std::__1::char_traits<char>>)
Shadow bytes around the buggy address:
  0x00901cfee800: f2 f2 f8 f2 f2 f2 00 f2 f2 f2 00 00 f2 f2 04 f2
  0x00901cfee880: f8 f8 f8 f2 f2 f2 f2 f2 00 00 f2 f2 00 00 f3 f3
  0x00901cfee900: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
  0x00901cfee980: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
  0x00901cfeea00: f1 f1 f1 f1 00 00 f2 f2 00 00 f2 f2 04 f2 f8 f8
=>0x00901cfeea80: f8 f2 f2 f2 f2 f2[f8]f2 f2 f2 f8 f8 f8 f8 f8 f2
  0x00901cfeeb00: f2 f2 f2 f2 00 00 00 00 f2 f2 f2 f2 00 f3 f3 f3
  0x00901cfeeb80: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
  0x00901cfeec00: 00 00 00 00 00 00 00 00 f1 f1 f1 f1 01 f3 f3 f3
  0x00901cfeec80: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
  0x00901cfeed00: 00 00 00 00 f1 f1 f1 f1 f8 f8 f2 f2 f8 f8 f2 f2
Shadow byte legend (one shadow byte represents 8 application bytes):
  Addressable:           00
  Partially addressable: 01 02 03 04 05 06 07
  Heap left redzone:       fa
  Freed heap region:       fd
  Stack left redzone:      f1
  Stack mid redzone:       f2
  Stack right redzone:     f3
  Stack after return:      f5
  Stack use after scope:   f8
  Global redzone:          f9
  Global init order:       f6
  Poisoned by user:        f7
  Container overflow:      fc
  Array cookie:            ac
  Intra object redzone:    bb
  ASan internal:           fe
  Left alloca redzone:     ca
  Right alloca redzone:    cb
==13728==ABORTING
Clang/STL/ASAN (correct behaviour, STL stores *`chrono-specs`* in `vector`)
PS D:\libcxx-playground> clang -std=c++2b -fsanitize=address -g -O2 .\test.cpp
   Creating library a.lib and object a.exp
PS D:\libcxx-playground> .\a.exe
00:00:00 AM

Compiler explorer: https://godbolt.org/z/hGK6456bY (this bug is present in libstdc++ too)

CC: @mordante

@EugeneZelenko EugeneZelenko added libc++ libc++ C++ Standard Library. Not GNU libstdc++. Not libc++abi. and removed new issue labels Apr 11, 2023
@mordante mordante self-assigned this Apr 12, 2023
@mordante
Copy link
Member

This indeed is a pathological case which is unlikely to happen in practice. Copying would solve the issue, but it has overhead. Before fixing this I probably want some feedback of other format specialists. There have been plans in the past to make delimiters of ranges configurable. Currently these are stored in a basic_string_view. Which would fail in a similar fashion.

Note that by storing the specs in a static variable would solve the issue.

@mordante
Copy link
Member

mordante commented May 7, 2023

I've discussed this on the reflector. The general feedback is that we want to clarify the life-time requirements and disallowing this use case. This direction matches the original design intend.
I will close this issue after I've filed the LWG issue to resolve this, probably next weekend.

@mordante
Copy link
Member

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
libc++ libc++ C++ Standard Library. Not GNU libstdc++. Not libc++abi.
Projects
None yet
Development

No branches or pull requests

3 participants