open-telemetry / opentelemetry-cpp-contrib

https://opentelemetry.io/
Apache License 2.0
121 stars 128 forks source link

[BUILD] boost_log compilation error (possibly dangling reference) #439

Closed bergjohan closed 1 month ago

bergjohan commented 1 month ago

Describe your environment Ubuntu 24.04 LTS g++ (Ubuntu 13.2.0-23ubuntu4) 13.2.0 Boost 1.83.0

Steps to reproduce

git clone https://github.com/open-telemetry/opentelemetry-cpp-contrib.git && \
cd opentelemetry-cpp-contrib && \
mkdir build && cd build && cmake -DWITH_EXAMPLES=ON ../instrumentation/boost_log && \
make

What is the expected behavior? Successful compilation

What is the actual behavior?

/opentelemetry-cpp-contrib/instrumentation/boost_log/src/sink.cc: In function 'bool opentelemetry::instrumentation::boost_log::ToTimestampDefault(const boost::log::v2_mt_posix::record_view&, std::chrono::_V2::system_clock::time_point&)':
/opentelemetry-cpp-contrib/instrumentation/boost_log/src/sink.cc:29:15: error: possibly dangling reference to a temporary [-Werror=dangling-reference]
   29 |   const auto &timestamp =
      |               ^~~~~~~~~
/opentelemetry-cpp-contrib/instrumentation/boost_log/src/sink.cc:30:63: note: the temporary was destroyed at the end of the full expression 'boost::log::v2_mt_posix::extract_or_default<boost::posix_time::ptime>(boost::log::v2_mt_posix::record_view::operator[](boost::log::v2_mt_posix::attribute_value_set::key_type) const(boost::log::v2_mt_posix::attribute_name(((const char*)"TimeStamp"))), kInvalid)'
   30 |       boost::log::extract_or_default<boost::posix_time::ptime>(record["TimeStamp"], kInvalid);
      |       ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
/opentelemetry-cpp-contrib/instrumentation/boost_log/src/sink.cc: In function 'bool opentelemetry::instrumentation::boost_log::ToThreadIdDefault(const boost::log::v2_mt_posix::record_view&, std::string&)':
/opentelemetry-cpp-contrib/instrumentation/boost_log/src/sink.cc:39:15: error: possibly dangling reference to a temporary [-Werror=dangling-reference]
   39 |   const auto &thread_id =
      |               ^~~~~~~~~
/opentelemetry-cpp-contrib/instrumentation/boost_log/src/sink.cc:40:66: note: the temporary was destroyed at the end of the full expression 'boost::log::v2_mt_posix::extract_or_default<aux::id<aux::thread> >(boost::log::v2_mt_posix::record_view::operator[](boost::log::v2_mt_posix::attribute_value_set::key_type) const(boost::log::v2_mt_posix::attribute_name(((const char*)"ThreadID"))), kInvalid)'
   40 |       boost::log::extract_or_default<boost::log::aux::thread::id>(record["ThreadID"], kInvalid);
      |       ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
cc1plus: all warnings being treated as errors
make[2]: *** [CMakeFiles/opentelemetry_boost_log_sink.dir/build.make:76: CMakeFiles/opentelemetry_boost_log_sink.dir/src/sink.cc.o] Error 1
make[1]: *** [CMakeFiles/Makefile2:85: CMakeFiles/opentelemetry_boost_log_sink.dir/all] Error 2
make: *** [Makefile:91: all] Error 2

Additional context Note from https://www.boost.org/doc/libs/1_83_0/libs/log/doc/html/boost/log/extract_or_defaul_idm26363.html

Caution must be exercised if the default value is a temporary object. Because the function returns a reference, if the temporary object is destroyed, the reference may become dangling.

But afaict the default value is not a temporary object: https://github.com/open-telemetry/opentelemetry-cpp-contrib/blob/main/instrumentation/boost_log/src/sink.cc#L27

Is GCC wrong here?

marcalff commented 1 month ago

Thanks for the report.

I don't have the proper setup locally, could you try this simple change:

  auto timestamp =
      boost::log::extract_or_default<boost::posix_time::ptime>(record["TimeStamp"], kInvalid);

and report if this fixes the build error ?

Thanks.

bergjohan commented 1 month ago

Yes, if I save both timestamp and thread_id by value it compiles successfully.