ConfigMetadata ctor: code may be uninitialized

[malt3]

EDIT: This only happens using gcc 12. When using gcc 13, the error disappears.

While trying to compile envoy v1.30.0 (with Bazel), I see an error that a variable may be uninitialized.

dd-trace-cpp/src/datadog/config.h

Line 50 in e576445

Optional<Error> err = nullopt)

datadog::tracing::Error::code' may be used uninitialized [-Werror=maybe-uninitialized].

The BUILD file sets strict compiler flags, leading to an error:

dd-trace-cpp/BUILD.bazel

Lines 120 to 124 in e576445

	copts = [
	"-Wall",
	"-Wextra",
	"-Werror",
	"-pedantic",

Details:

In file included from external/com_github_datadog_dd_trace_cpp/src/datadog/config.h:3,
                 from external/com_github_datadog_dd_trace_cpp/src/datadog/trace_sampler_config.h:13,
                 from external/com_github_datadog_dd_trace_cpp/src/datadog/config_update.h:6,
                 from external/com_github_datadog_dd_trace_cpp/src/datadog/config_manager.h:11,
                 from external/com_github_datadog_dd_trace_cpp/src/datadog/config_manager.cpp:1:
In constructor 'datadog::tracing::Error::Error(datadog::tracing::Error&&)',
    inlined from 'void std::_Construct(_Tp*, _Args&& ...) [with _Tp = datadog::tracing::Error; _Args = {datadog::tracing::Error}]' at /nix/store/dqa6kvs8h9nxlrh0a0lgqrdkpx62791d-gcc-12.3.0/include/c++/12.3.0/bits/stl_construct.h:119:7,
    inlined from 'constexpr void std::_Optional_payload_base<_Tp>::_M_construct(_Args&& ...) [with _Args = {datadog::tracing::Error}; _Tp = datadog::tracing::Error]' at /nix/store/dqa6kvs8h9nxlrh0a0lgqrdkpx62791d-gcc-12.3.0/include/c++/12.3.0/optional:278:19,
    inlined from 'constexpr std::_Optional_payload_base<_Tp>::_Optional_payload_base(bool, std::_Optional_payload_base<_Tp>&&) [with _Tp = datadog::tracing::Error]' at /nix/store/dqa6kvs8h9nxlrh0a0lgqrdkpx62791d-gcc-12.3.0/include/c++/12.3.0/optional:155:22,
    inlined from 'constexpr std::_Optional_payload_base<_Tp>::_Optional_payload_base(bool, std::_Optional_payload_base<_Tp>&&) [with _Tp = datadog::tracing::Error]' at /nix/store/dqa6kvs8h9nxlrh0a0lgqrdkpx62791d-gcc-12.3.0/include/c++/12.3.0/optional:151:7,
    inlined from 'constexpr std::_Optional_payload<datadog::tracing::Error, true, false, false>::_Optional_payload(bool, std::_Optional_payload_base<datadog::tracing::Error>&&) [inherited from std::_Optional_payload_base<datadog::tracing::Error>]' at /nix/store/dqa6kvs8h9nxlrh0a0lgqrdkpx62791d-gcc-12.3.0/include/c++/12.3.0/optional:397:42,
    inlined from 'constexpr std::_Optional_payload<datadog::tracing::Error, false, false, false>::_Optional_payload(bool, std::_Optional_payload_base<datadog::tracing::Error>&&) [inherited from std::_Optional_payload_base<datadog::tracing::Error>]' at /nix/store/dqa6kvs8h9nxlrh0a0lgqrdkpx62791d-gcc-12.3.0/include/c++/12.3.0/optional:431:57,
    inlined from 'constexpr std::_Optional_base<_Tp, <anonymous>, <anonymous> >::_Optional_base(std::_Optional_base<_Tp, <anonymous>, <anonymous> >&&) [with _Tp = datadog::tracing::Error; bool <anonymous> = false; bool <anonymous> = false]' at /nix/store/dqa6kvs8h9nxlrh0a0lgqrdkpx62791d-gcc-12.3.0/include/c++/12.3.0/optional:544:9,
    inlined from 'constexpr std::optional<datadog::tracing::Error>::optional(std::optional<datadog::tracing::Error>&&)' at /nix/store/dqa6kvs8h9nxlrh0a0lgqrdkpx62791d-gcc-12.3.0/include/c++/12.3.0/optional:705:11,
    inlined from 'datadog::tracing::ConfigMetadata::ConfigMetadata(datadog::tracing::ConfigName, std::string, Origin, datadog::tracing::Optional<datadog::tracing::Error>)' at external/com_github_datadog_dd_trace_cpp/src/datadog/config.h:51:53,
    inlined from 'void std::__new_allocator<_Tp>::construct(_Up*, _Args&& ...) [with _Up = datadog::tracing::ConfigMetadata; _Args = {datadog::tracing::ConfigName, std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> >, datadog::tracing::ConfigMetadata::Origin}; _Tp = datadog::tracing::ConfigMetadata]' at /nix/store/dqa6kvs8h9nxlrh0a0lgqrdkpx62791d-gcc-12.3.0/include/c++/12.3.0/bits/new_allocator.h:175:4,
    inlined from 'static void std::allocator_traits<std::allocator<_CharT> >::construct(allocator_type&, _Up*, _Args&& ...) [with _Up = datadog::tracing::ConfigMetadata; _Args = {datadog::tracing::ConfigName, std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> >, datadog::tracing::ConfigMetadata::Origin}; _Tp = datadog::tracing::ConfigMetadata]' at /nix/store/dqa6kvs8h9nxlrh0a0lgqrdkpx62791d-gcc-12.3.0/include/c++/12.3.0/bits/alloc_traits.h:516:17,
    inlined from 'std::vector<_Tp, _Alloc>::reference std::vector<_Tp, _Alloc>::emplace_back(_Args&& ...) [with _Args = {datadog::tracing::ConfigName, std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> >, datadog::tracing::ConfigMetadata::Origin}; _Tp = datadog::tracing::ConfigMetadata; _Alloc = std::allocator<datadog::tracing::ConfigMetadata>]' at /nix/store/dqa6kvs8h9nxlrh0a0lgqrdkpx62791d-gcc-12.3.0/include/c++/12.3.0/bits/vector.tcc:117:30,
    inlined from 'std::vector<datadog::tracing::ConfigMetadata> datadog::tracing::ConfigManager::update(const datadog::tracing::ConfigUpdate&)' at external/com_github_datadog_dd_trace_cpp/src/datadog/config_manager.cpp:90:28:
external/com_github_datadog_dd_trace_cpp/src/datadog/error.h:21:8: error: '*(datadog::tracing::Error*)((char*)&<unnamed> + offsetof(std::Optional<datadog::tracing::Error>,std::optional<datadog::tracing::Error>::<unnamed>.std::_Optional_base<datadog::tracing::Error, false, false>::<unnamed>)).datadog::tracing::Error::code' may be used uninitialized [-Werror=maybe-uninitialized]
   21 | struct Error {
      |        ^~~~~
external/com_github_datadog_dd_trace_cpp/src/datadog/config.h: In member function 'std::vector<datadog::tracing::ConfigMetadata> datadog::tracing::ConfigManager::update(const datadog::tracing::ConfigUpdate&)':
external/com_github_datadog_dd_trace_cpp/src/datadog/config.h:50:40: note: '<anonymous>' declared here
   50 |                  Optional<Error> err = nullopt)
      |                                        ^~~~~~~
cc1plus: all warnings being treated as errors

[dgoffredo]

I think that this is spurious. I don't say that lightly, since last time I thought this warning was spurious, it turned out that I had made a nasty bug.

Here's how I figure, though:

• The warning is about Error's Code code data member, which indeed is left uninitialized if you don't give it a value.
• The warning is triggered by vector::emplace_back here, which will call this flavor of ConfigMetadata's constructor.
• This will set ConfigMetadata's Optional<Error> error data member to nullopt, meaning that no Error object is constructed.

It's worth looking at more closely, and with different compilers, to be sure that this is spurious and not a symptom of a real issue somewhere (else) in the code.

Maybe somewhere, operator*() or similar is being called on a null-valued ConfigMetadata::error. That is just a guess, and I can't find an example.

[malt3]

Thank you for taking a look. I am definitely not an expert on this. In case it is indeed spurious, would it be correct to add -Wno-error=maybe-uninitialized, or is there a better fix?

[dgoffredo]

EDIT: This only happens using gcc 12. When using gcc 13, the error disappears.

This makes me think that it's a spurious error.

I wonder if there's ever a place in the code where an Error is default constructed (that is, with an undefined code) and then later filled out, e.g.

{
    Error error;
    error.code = DATADOG_AGENT_NULL_HTTP_CLIENT;
    error.message = "uh oh!";
}

Or, more likely:

{
    Error error;
    // stuff...
    if (something(&error)) {
        // use `error`
    }
    // ...
}

I don't think that there's any code in the library written this way. If that's the case, then you might try deleteing the default constructor of Error. Maybe that will sidestep whatever compiler bug is producing the spurious warning.

Even if that works, I'm not sure that it is worth doing. See what Damien thinks.

My goal with Error was to create a C-style plain old data wrapper for an Error::Code and a string. The type is not meant to have any default behavior, value, or semantics.

Error does not default value initialize its code member, and the natural default value (zero) is not even a valid Error::Code value. This is all on purpose.