Merge lp:~thomas-voss/process-cpp/more_verbose_output_from_fork_and_run into lp:process-cpp

[Thomas Voß]

Print issues with the forked child to std::cerr.

Print issues with the forked child to std::cerr.

[PS Jenkins bot (ps-jenkins)]

FAILED: Continuous integration, rev:34
http://jenkins.qa.ubuntu.com/job/process-cpp-ci/28/
Executed test runs:
    FAILURE: http://jenkins.qa.ubuntu.com/job/process-cpp-trusty-amd64-ci/28/console
    SUCCESS: http://jenkins.qa.ubuntu.com/job/process-cpp-trusty-armhf-ci/28
        deb: http://jenkins.qa.ubuntu.com/job/process-cpp-trusty-armhf-ci/28/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/process-cpp-trusty-i386-ci/24

Click here to trigger a rebuild:
http://s-jenkins.ubuntu-ci:8080/job/process-cpp-ci/28/rebuild

review: Needs Fixing (continuous-integration)

[PS Jenkins bot (ps-jenkins)]

PASSED: Continuous integration, rev:35
http://jenkins.qa.ubuntu.com/job/process-cpp-ci/29/
Executed test runs:
    SUCCESS: http://jenkins.qa.ubuntu.com/job/process-cpp-trusty-amd64-ci/29
    SUCCESS: http://jenkins.qa.ubuntu.com/job/process-cpp-trusty-armhf-ci/29
        deb: http://jenkins.qa.ubuntu.com/job/process-cpp-trusty-armhf-ci/29/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/process-cpp-trusty-i386-ci/25

Click here to trigger a rebuild:
http://s-jenkins.ubuntu-ci:8080/job/process-cpp-ci/29/rebuild

review: Approve (continuous-integration)

[Martin Pitt (pitti)]

Nice!

review: Approve

[PS Jenkins bot (ps-jenkins)]

FAILED: Autolanding.
More details in the following jenkins job:
http://jenkins.qa.ubuntu.com/job/process-cpp-autolanding/21/
Executed test runs:
    FAILURE: http://jenkins.qa.ubuntu.com/job/process-cpp-trusty-amd64-autolanding/21/console
    SUCCESS: http://jenkins.qa.ubuntu.com/job/process-cpp-trusty-armhf-autolanding/22
        deb: http://jenkins.qa.ubuntu.com/job/process-cpp-trusty-armhf-autolanding/22/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/process-cpp-trusty-i386-autolanding/21

review: Needs Fixing (continuous-integration)

[PS Jenkins bot (ps-jenkins)]

PASSED: Continuous integration, rev:36
http://jenkins.qa.ubuntu.com/job/process-cpp-ci/31/
Executed test runs:
    SUCCESS: http://jenkins.qa.ubuntu.com/job/process-cpp-trusty-amd64-ci/31
    SUCCESS: http://jenkins.qa.ubuntu.com/job/process-cpp-trusty-armhf-ci/31
        deb: http://jenkins.qa.ubuntu.com/job/process-cpp-trusty-armhf-ci/31/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/process-cpp-trusty-i386-ci/27

Click here to trigger a rebuild:
http://s-jenkins.ubuntu-ci:8080/job/process-cpp-ci/31/rebuild

review: Approve (continuous-integration)

[lp:~thomas-voss/process-cpp/more_verbose_output_from_fork_and_run]

37. By Thomas Voß on 2014-01-24

Factor out backtrace generation to make it testable.

[Martin Pitt (pitti)]

In debian/libprocess-cpp0.symbols, does something automatically replace the "0replaceme" version when autolanding? Otherwise this needs to be set to something sensible. That said, I'm not that happy about exposing internal symbols publicly; at least in a libtool world, you could statically link the library to the test, so that you can access its internal symbols. Is that possible with cmake at all?

If static linking doesn't work, how about dropping the .h etc. and have the test #include backtrace.cpp directly?

38 + * Copyright © 2013 Canonical Ltd.

Should be ++year by now :-)

review: Needs Information

[PS Jenkins bot (ps-jenkins)]

PASSED: Continuous integration, rev:37
http://jenkins.qa.ubuntu.com/job/process-cpp-ci/32/
Executed test runs:
    SUCCESS: http://jenkins.qa.ubuntu.com/job/process-cpp-trusty-amd64-ci/32
    SUCCESS: http://jenkins.qa.ubuntu.com/job/process-cpp-trusty-armhf-ci/32
        deb: http://jenkins.qa.ubuntu.com/job/process-cpp-trusty-armhf-ci/32/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/process-cpp-trusty-i386-ci/28

Click here to trigger a rebuild:
http://s-jenkins.ubuntu-ci:8080/job/process-cpp-ci/32/rebuild

review: Approve (continuous-integration)

[lp:~thomas-voss/process-cpp/more_verbose_output_from_fork_and_run]

38. By Thomas Voß on 2014-01-24

Do not expose implementation details via the libraries API & ABI.

39. By Thomas Voß on 2014-01-24

Fix copyright year.

[Thomas Voß (thomas-voss)]

> In debian/libprocess-cpp0.symbols, does something automatically replace the
> "0replaceme" version when autolanding? Otherwise this needs to be set to
> something sensible. That said, I'm not that happy about exposing internal
> symbols publicly; at least in a libtool world, you could statically link the
> library to the test, so that you can access its internal symbols. Is that
> possible with cmake at all?
>

It would be possible with cmake, but your suggestion of just compiling in the source file is actually better :)

> If static linking doesn't work, how about dropping the .h etc. and have the
> test #include backtrace.cpp directly?
>

See before.

> 38 + * Copyright © 2013 Canonical Ltd.
>
> Should be ++year by now :-)

Fixed :) Good catch.

[PS Jenkins bot (ps-jenkins)]

PASSED: Continuous integration, rev:39
http://jenkins.qa.ubuntu.com/job/process-cpp-ci/33/
Executed test runs:
    SUCCESS: http://jenkins.qa.ubuntu.com/job/process-cpp-trusty-amd64-ci/33
    SUCCESS: http://jenkins.qa.ubuntu.com/job/process-cpp-trusty-armhf-ci/33
        deb: http://jenkins.qa.ubuntu.com/job/process-cpp-trusty-armhf-ci/33/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/process-cpp-trusty-i386-ci/29

Click here to trigger a rebuild:
http://s-jenkins.ubuntu-ci:8080/job/process-cpp-ci/33/rebuild

review: Approve (continuous-integration)

[Martin Pitt (pitti)]

Hah, very nice. Thanks!

review: Approve

=== modified file 'src/CMakeLists.txt'
--- src/CMakeLists.txt	2014-01-15 15:18:53 +0000
+++ src/CMakeLists.txt	2014-01-24 19:28:21 +0000
@@ -17,6 +17,9 @@
 add_library(
   process-cpp SHARED
   
+  core/posix/backtrace.h
+  core/posix/backtrace.cpp
+
   core/posix/child_process.cpp
   core/posix/exec.cpp
   core/posix/fork.cpp

=== added file 'src/core/posix/backtrace.cpp'
--- src/core/posix/backtrace.cpp	1970-01-01 00:00:00 +0000
+++ src/core/posix/backtrace.cpp	2014-01-24 19:28:21 +0000
@@ -0,0 +1,153 @@
+/*
+ * Copyright © 2014 Canonical Ltd.
+ *
+ * This program is free software: you can redistribute it and/or modify it
+ * under the terms of the GNU Lesser General Public License version 3,
+ * as published by the Free Software Foundation.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU Lesser General Public License for more details.
+ *
+ * You should have received a copy of the GNU Lesser General Public License
+ * along with this program.  If not, see <http://www.gnu.org/licenses/>.
+ *
+ * Authored by: Thomas Voß <[e-mail]>
+ */
+
+#include "backtrace.h"
+
+#include <cxxabi.h>
+
+#include <execinfo.h>
+
+namespace bt = core::posix::backtrace;
+
+namespace impl
+{
+std::tuple<std::string, bool> demangle(const std::string& symbol)
+{
+    int status = 1;
+    auto result = abi::__cxa_demangle(symbol.c_str(),
+                                      nullptr,
+                                      nullptr,
+                                      &status);
+
+    if (!result || status != 0)
+    {
+        return std::make_tuple(std::string(), false);
+    }
+
+    std::string s{result};
+    ::free(result);
+
+    return std::make_tuple(s, true);
+}
+
+struct Frame : public bt::Frame
+{
+    struct Symbol : public bt::Frame::Symbol
+    {
+        Symbol(const char* symbol) : raw_(symbol)
+        {
+            auto first = raw_.find_first_of("(");
+            auto last = raw_.find_last_of(")");
+
+            if (first != std::string::npos && last != std::string::npos)
+            {
+                auto mangled_symbol = raw_.substr(first+1,
+                                                  (last-1) - (first+1));
+
+                auto plus = mangled_symbol.find_first_of("+");
+                if (plus != std::string::npos)
+                    mangled_symbol.erase(plus);
+
+                std::tie(demangled_, is_cxx_) = demangle(mangled_symbol);
+                if (!is_cxx_)
+                    demangled_ = raw_;
+            }
+        }
+
+        bool is_cxx() const
+        {
+            return is_cxx_;
+        }
+
+        std::string demangled() const
+        {
+            return demangled_;
+        }
+
+        std::string raw() const
+        {
+            return raw_;
+        }
+
+        std::string raw_;
+        std::string demangled_;
+        bool is_cxx_ = false;
+    };
+
+    std::size_t depth_;
+    void* frame_pointer_;
+    Symbol symbol_;
+
+    Frame(std::size_t depth, void* frame_pointer, const char* symbol)
+        : depth_(depth),
+          frame_pointer_(frame_pointer),
+          symbol_(symbol)
+    {
+    }
+
+    std::size_t depth() const
+    {
+        return depth_;
+    }
+
+    virtual void* frame_pointer() const
+    {
+        return frame_pointer_;
+    }
+
+    const Symbol& symbol() const
+    {
+        return symbol_;
+    }
+};
+}
+
+std::shared_ptr<bt::Frame::Symbol> bt::Frame::Symbol::for_testing_from_raw_symbol(const char* symbol)
+{
+    return std::shared_ptr<bt::Frame::Symbol>(new impl::Frame::Symbol(symbol));
+}
+
+void bt::visit_with_handler(const bt::FrameHandler& handler)
+{
+    static const unsigned int max_frames=64;
+    void *frames[max_frames];
+
+    auto frame_count = ::backtrace(frames, max_frames);
+    auto symbols = ::backtrace_symbols(frames, frame_count);
+
+    struct Scope
+    {
+        Scope(char** symbols) : symbols(symbols)
+        {
+        }
+
+        ~Scope()
+        {
+            ::free(symbols);
+        }
+
+        char** symbols = nullptr;
+    } scope{symbols};
+
+    for (int i = 0; i < frame_count; i++)
+    {
+        impl::Frame frame(i, frames[i], symbols[i]);
+        if (!handler(frame))
+            return;
+    }
+}

=== added file 'src/core/posix/backtrace.h'
--- src/core/posix/backtrace.h	1970-01-01 00:00:00 +0000
+++ src/core/posix/backtrace.h	2014-01-24 19:28:21 +0000
@@ -0,0 +1,122 @@
+/*
+ * Copyright © 2014 Canonical Ltd.
+ *
+ * This program is free software: you can redistribute it and/or modify it
+ * under the terms of the GNU Lesser General Public License version 3,
+ * as published by the Free Software Foundation.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU Lesser General Public License for more details.
+ *
+ * You should have received a copy of the GNU Lesser General Public License
+ * along with this program.  If not, see <http://www.gnu.org/licenses/>.
+ *
+ * Authored by: Thomas Voß <[e-mail]>
+ */
+
+#ifndef CORE_POSIX_BACKTRACE_H_
+#define CORE_POSIX_BACKTRACE_H_
+
+#include <core/posix/visibility.h>
+
+#include <functional>
+#include <memory>
+#include <string>
+
+namespace core
+{
+namespace posix
+{
+namespace backtrace
+{
+/**
+ * @brief The Frame class models an individual frame of a backtrace.
+ */
+class Frame
+{
+public:
+    /**
+     * @brief The Symbol class models the symbolic representation of a frame pointer.
+     */
+    class Symbol
+    {
+    public:
+
+        static std::shared_ptr<Symbol> for_testing_from_raw_symbol(const char* symbol);
+
+        Symbol(const Symbol&) = delete;
+        virtual ~Symbol() = default;
+
+        Symbol& operator=(const Symbol&) = delete;
+
+        /**
+         * @brief is_cxx checks whether the symbol refers to a mangled C++ symbol.
+         * @return true iff the symbol refers to a mangled C++ symbol.
+         */
+        virtual bool is_cxx() const = 0;
+
+        /**
+         * @brief demangled returns the demangled C++ symbol name or raw.
+         */
+        virtual std::string demangled() const = 0;
+
+        /**
+         * @brief raw The raw symbolic representation of a frame pointer.
+         * @return
+         */
+        virtual std::string raw() const = 0;
+
+    protected:
+        Symbol() = default;
+    };
+
+    Frame(const Frame&) = delete;
+    virtual ~Frame() = default;
+
+    Frame& operator=(const Frame&) = delete;
+
+    /**
+     * @brief depth returns the depth of this frame in the overall backtrace.
+     */
+    virtual std::size_t depth() const = 0;
+
+    /**
+     * @brief frame_pointer returns the the raw frame pointer of this frame.
+     * @return
+     */
+    virtual void* frame_pointer() const = 0;
+
+    /**
+     * @brief symbol returns the symbolic representation of this frame.
+     */
+    virtual const Symbol& symbol() const = 0;
+
+protected:
+    Frame() = default;
+};
+
+/**
+ * @brief FrameHandler is the functor invoked for every frame of a backtrace.
+ *
+ * A FrameHandler should return true if it wants to continue walking the stack
+ * or false otherwise.
+ */
+typedef std::function<bool(const Frame& frame)> FrameHandler;
+
+/**
+ *@brief visit_with_handler iterates the backtrace of the calling program,
+ *invoking the handler for every frame.
+ *
+ * A FrameHandler should return true if it wants to continue walking the stack
+ * or false otherwise
+ *
+ * @param handler The handler invoked for every frame.
+ */
+void visit_with_handler(const FrameHandler& handler);
+}
+}
+}
+
+#endif // CORE_POSIX_BACKTRACE_H_

=== modified file 'src/core/posix/fork.cpp'
--- src/core/posix/fork.cpp	2013-11-29 07:38:07 +0000
+++ src/core/posix/fork.cpp	2014-01-24 19:28:21 +0000
@@ -19,6 +19,9 @@
 #include <core/posix/exit.h>
 #include <core/posix/fork.h>
 
+#include "backtrace.h"
+
+#include <iomanip>
 #include <iostream>
 #include <system_error>
 
@@ -32,6 +35,16 @@
     if (rc == -1)
         throw std::system_error(errno, std::system_category());
 }
+
+void print_backtrace(std::ostream& out, const std::string& line_prefix)
+{
+    core::posix::backtrace::visit_with_handler([&out, line_prefix](const core::posix::backtrace::Frame& frame)
+    {
+        out << line_prefix << std::dec << std::setw(2) << frame.depth() << "@" << std::hex << std::setw(14) << frame.frame_pointer() << ": "
+            << (frame.symbol().is_cxx() ? frame.symbol().demangled() : frame.symbol().raw()) << std::endl;
+        return true;
+    });
+}
 }
 
 namespace core
@@ -78,8 +91,15 @@
                 redirect_stream_to_fd(stderr_pipe.write_fd(), STDERR_FILENO);
 
             result = main();
+        } catch(const std::exception& e)
+        {
+            std::cerr << "core::posix::fork(): An unhandled std::exception occured in the child process:" << std::endl
+                      << "  what(): " << e.what() << std::endl;
+            print_backtrace(std::cerr, "  ");
         } catch(...)
         {
+            std::cerr << "core::posix::fork(): An unhandled exception occured in the child process." << std::endl;
+            print_backtrace(std::cerr, "  ");
         }
 
         // We have to ensure that we exit here. Otherwise, we run into
@@ -128,8 +148,15 @@
                 redirect_stream_to_fd(stderr_pipe.write_fd(), STDERR_FILENO);
 
             result = main();
+        } catch(const std::exception& e)
+        {
+            std::cerr << "core::posix::fork(): An unhandled std::exception occured in the child process:" << std::endl
+                      << "  what(): " << e.what() << std::endl;
+            print_backtrace(std::cerr, "  ");
         } catch(...)
         {
+            std::cerr << "core::posix::fork(): An unhandled exception occured in the child process." << std::endl;
+            print_backtrace(std::cerr, "  ");
         }
 
         // We have to ensure that we exit here. Otherwise, we run into

=== modified file 'tests/CMakeLists.txt'
--- tests/CMakeLists.txt	2014-01-15 15:18:53 +0000
+++ tests/CMakeLists.txt	2014-01-24 19:28:21 +0000
@@ -21,7 +21,10 @@
 include_directories(${GMOCK_INCLUDE_DIR} ${GTEST_INCLUDE_DIR})
 set (CMAKE_CXX_FLAGS ${OLD_CMAKE_CXX_FLAGS})
 
-include_directories(${GTEST_INCLUDE_DIR})
+include_directories(
+  ${CMAKE_SOURCE_DIR}/src
+  ${GTEST_INCLUDE_DIR}
+)
 
 add_executable(
   posix_process_test
@@ -36,6 +39,10 @@
 add_executable(
   fork_and_run_test
   fork_and_run_test.cpp
+
+  # We include an external source file to prevent from leaking
+  # symbols to the outside world
+  ${CMAKE_SOURCE_DIR}/src/core/posix/backtrace.cpp
 )
 
 add_executable(

=== modified file 'tests/fork_and_run_test.cpp'
--- tests/fork_and_run_test.cpp	2013-12-06 12:50:07 +0000
+++ tests/fork_and_run_test.cpp	2014-01-24 19:28:21 +0000
@@ -145,3 +145,16 @@
     {
         return core::posix::exit::Status::failure;
     })
+
+#include <core/posix/backtrace.h>
+
+TEST(BacktraceSymbolDemangling, demangling_a_cpp_symbol_works)
+{
+    const char* ref = "tests/fork_and_run_test(_ZN7testing8internal35HandleExceptionsInMethodIfSupportedINS0_12UnitTestImplEbEET0_PT_MS4_FS3_vEPKc+0x4b) [0x4591f8]";
+    const char* ref_demangled = "bool testing::internal::HandleExceptionsInMethodIfSupported<testing::internal::UnitTestImpl, bool>(testing::internal::UnitTestImpl*, bool (testing::internal::UnitTestImpl::*)(), char const*)";
+    auto symbol = core::posix::backtrace::Frame::Symbol::for_testing_from_raw_symbol(ref);
+
+    EXPECT_TRUE(symbol->is_cxx());
+    EXPECT_EQ(ref, symbol->raw());
+    EXPECT_EQ(ref_demangled, symbol->demangled());
+}