Fmt: Inconsistent type detection of custom string types

Created on 3 May 2020  路  22Comments  路  Source: fmtlib/fmt

I have a custom string type (seastar::basic_sstring<> below)that has an operator std::basic_string_view defined (the type parameter is char).

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

      string = {
        data = 0x2266c0 "test/boost/castas_fcts_test.cc",
        size = 193731415
      },
      custom = {
        value = 0x2266c0,
        format = 0xb8c1b57 <fmt::v6::internal::value<fmt::v6::basic_format_context<std::back_insert_iterator<fmt::v6::internal::buffer<char> >, char> >::format_custom_arg<seastar::basic_sstring<char, unsigned int, 15u, true>, fmt::v6::internal::fallback_formatter<seastar::basic_sstring<char, unsigned int, 15u, true>, char, void> >(void const*, fmt::v6::basic_format_parse_context<char, fmt::v6::internal::error_handler>&, fmt::v6::basic_format_context<std::back_insert_iterator<fmt::v6::internal::buffer<char> >, char>&)>
      },

Indicating it was stored as a custom type. As a result, asan complains when copy_str tries to copy a string with too-large size.

#9  0x000000000b8dfbee in fmt::v6::internal::copy_str<char, char const*, char*, 0> (begin=0x604000103a50 "\300\315\025", end=0x60400b9c55a7 "", it=0x7ffeca8af880 '\276' <repeats 200 times>...) at /usr/include/fmt/format.h:537
537   return std::copy(begin, end, it);
(gdb) p end - begin
$50 = 193731415

All 22 comments

fmt 6.2.0 + e99809f29da1002e8b9246e9278084ad231174e5

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());

(fragment is the variable giving me trouble)

and got

type: 14
typeof: N3fmt2v617basic_string_viewIcEE

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:

docker run -it --rm --cap-add=SYS_PTRACE docker.io/avikivity/fmt-1662 bash
cd scylla
build/debug/test/boost/castas_fcts_test_g -- --unsafe-bypass-fsync 1

to build:

ninja -j3 build/debug/test/boost/castas_fcts_test_g

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 fmt/format.h to fmt/ostream.h in test/lib/exception_utils.cc fix the issue? (I wasn't able to check myself because the linker fails.)

A simple fix is to not use operator<< if there is an (implicit) conversion to string_view but I'll see if it can be fixed in a nicer way.

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?

I can't drop operator<< support because there may be (misguided) users using sstring with iostreams.

I meant that {fmt} can resolve this problematic case by always preferring implicit conversion to operator<<, not suggesting that you should remove it.

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?

That's an option too.

I can't drop operator<< support because there may be (misguided) users using sstring with iostreams.

I meant that {fmt} can resolve this problematic case by always preferring implicit conversion to operator<<, not suggesting that you should remove it.

For me it is fine, but for the general case, they may yield different results.

Fixed in https://github.com/fmtlib/fmt/commit/080e44d0bf9581b27ac655e5ab747e5bb8ab4ccf. 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.

https://github.com/fmtlib/fmt/releases/tag/6.2.1

Commits from this tag does not belongs to any branches. Cannot be fetched via Git:

fatal: invalid upstream '19bd751020a1f3c3363b2eb67a039852f139a8d3'
git clone https://github.com/fmtlib/fmt.git
cd fmt
git checkout 19bd751020a1f3c3363b2eb67a039852f139a8d3

git checkout 19bd751020a1f3c3363b2eb67a039852f139a8d3

git fetch --tags fixed issue for me. Thanks.

Was this page helpful?
0 / 5 - 0 ratings