Inconsistent type detection of custom string types #1662
Comments
|
fmt 6.2.0 + e99809f |
|
Could you give a reproducible example? I tried to repro myself (https://godbolt.org/z/27P3by) but to no avail. |
|
I'll try to minimize. |
|
I added these prints: using fmt_context = fmt::v6::basic_format_context<std::back_insert_iterator<fmt::v6::internal::buffer<char> >, char>;
fmt::print("type: {}\n", fmt::internal::mapped_type_constant<decltype(fragment), fmt_context>::value);
fmt::internal::arg_mapper<fmt_context> arg_mapper;
fmt::print("typeof: {}\n", (typeid(arg_mapper.map(fragment))).name());( and got Which is self-consistent, but not consistent with what I got. I suspect a compiler bug (using gcc 10 here). I'll try with gcc 9. |
|
I reproduced with gcc 9 and fmt-6.2.0 (plus that patch) |
|
I packaged a non-minimal reproducer into a convenient 3GB container, docker.io/avikivity/fmt-1662. It is prebuilt so you don't have to recompile the world, just whatever you touch. fmt is in a system package, installed at /usr/include/fmt. sstring is at /scylla/seastar/include/sestar/code/sstring.hh. To reproduce: to build: |
|
oh, and if you install gdb and break in __sanitizer::Die, you can inspect the inconsistent state where the type doesn't match the value. |
|
Something I suspected and ruled out: ODR violation. Note this is debug mode with -O0, so if some translation unit built an arg with just a forward declaration of basic_sstring, then the various enable_ifs would get different results. The linker is free to use the non-constexpr-evaluated map() from a TU that did not have all the definitions. I ruled it out because we don't forward-declare basic_sstring. But maybe a similar mistake is triggering this. |
|
Thanks for the repro, I'm able to debug the issue now. |
|
I think it's due to ODR violation with ostream operator<< being picked in some TUs but not the other. Does changing |
|
A simple fix is to not use |
|
Adding fmt/ostream.h works. I can't drop operator<< support because there may be (misguided) users using sstring with iostreams. Perhaps just enough forward declarations can be added to format.h so that it can detect the custom transformation, even if it can't compile it? |
fmt, the formatting library we use, detects types with conversion to std::string_view (and formats them as strings) and types that support operator<<(std::ostream, const T&) (and performs custom formatting on them). However, if <fmt/ostream.h>, the latter is not done. The problem happens with seastar::sstring, which implements both, and debug mode, which disables inlining. Some translation units do include <fmt/ostream.h>, and so generate code to do custom formatting. exception_utils.cc doesn't, and so generates code to format via string_view conversion. At link time, the compiler picks one of the generated functions and includes it in the final binary; it happened to pick one generated outside exception_utils.cc, using custom formatting. However, there is also code in fmt to encode which path fmt chose - string_view or custom. This code is constexpr and so is evaluated in exception_utils.cc. The result is that the function to perform formatting of seastar::sstring uses custom formatting, while the descriptor containing the method used says it is formatting via string_view. This is enough to cause a crash. The problem is limited to debug mode, since in other modes all this code is inlined, and so is consistent within the translation unit. We need a more general fix (hopefully in fmt), but for now a simple fix is to add the missing include. Ref fmtlib/fmt#1662
fmt, the formatting library we use, detects types with conversion to std::string_view (and formats them as strings) and types that support operator<<(std::ostream, const T&) (and performs custom formatting on them). However, if <fmt/ostream.h>, the latter is not done. The problem happens with seastar::sstring, which implements both, and debug mode, which disables inlining. Some translation units do include <fmt/ostream.h>, and so generate code to do custom formatting. exception_utils.cc doesn't, and so generates code to format via string_view conversion. At link time, the compiler picks one of the generated functions and includes it in the final binary; it happened to pick one generated outside exception_utils.cc, using custom formatting. However, there is also code in fmt to encode which path fmt chose - string_view or custom. This code is constexpr and so is evaluated in exception_utils.cc. The result is that the function to perform formatting of seastar::sstring uses custom formatting, while the descriptor containing the method used says it is formatting via string_view. This is enough to cause a crash. The problem is limited to debug mode, since in other modes all this code is inlined, and so is consistent within the translation unit. We need a more general fix (hopefully in fmt), but for now a simple fix is to add the missing include. Ref fmtlib/fmt#1662
I meant that {fmt} can resolve this problematic case by always preferring implicit conversion to
That's an option too. |
For me it is fine, but for the general case, they may yield different results. |
|
Fixed in 080e44d. Thanks a lot for reporting and particularly for the repro. That was very useful. |
|
Confirmed in my project. Thanks for the prompt fix. |
|
@vitaut Can you create a 6.2.1 release will all bug fixes? GNU/Linux maintainers will appreciate for this. |
|
@xvitaly, I'll look into releasing 6.2.1 with important fixes (including this one) in the next few weeks. |
fmt, the formatting library we use, detects types with conversion to std::string_view (and formats them as strings) and types that support operator<<(std::ostream, const T&) (and performs custom formatting on them). However, if <fmt/ostream.h>, the latter is not done. The problem happens with seastar::sstring, which implements both, and debug mode, which disables inlining. Some translation units do include <fmt/ostream.h>, and so generate code to do custom formatting. exception_utils.cc doesn't, and so generates code to format via string_view conversion. At link time, the compiler picks one of the generated functions and includes it in the final binary; it happened to pick one generated outside exception_utils.cc, using custom formatting. However, there is also code in fmt to encode which path fmt chose - string_view or custom. This code is constexpr and so is evaluated in exception_utils.cc. The result is that the function to perform formatting of seastar::sstring uses custom formatting, while the descriptor containing the method used says it is formatting via string_view. This is enough to cause a crash. The problem is limited to debug mode, since in other modes all this code is inlined, and so is consistent within the translation unit. We need a more general fix (hopefully in fmt), but for now a simple fix is to add the missing include. Ref fmtlib/fmt#1662
|
Commits from this tag does not belongs to any branches. Cannot be fetched via Git: |
|
|
I have a custom string type (
seastar::basic_sstring<>below)that has anoperator std::basic_string_viewdefined (the type parameter ischar).The
types_field is 112077, which I decoded as 13 14 13 3, or cstring string cstring uint, which matches the call site.However, args_[1] is
Indicating it was stored as a custom type. As a result, asan complains when
copy_strtries to copy a string with too-large size.The text was updated successfully, but these errors were encountered: