sal/osl/unx/backtraceapi.cxx |  195 +++++++++++++++++++++++++++++++++++++++----
 1 file changed, 180 insertions(+), 15 deletions(-)

New commits:
commit 9aa2fcb408334f48b9ba3a7525b4f20f8e0f19e9
Author:     Luboš Luňák <l.lu...@collabora.com>
AuthorDate: Fri Sep 3 11:58:47 2021 +0200
Commit:     Luboš Luňák <l.lu...@collabora.com>
CommitDate: Mon Sep 6 14:40:35 2021 +0200

    group addr2line calls per binary for better performance
    
    Change-Id: Ifc655e4d5e2f3eb934b407e146ee564e3db0146b
    Reviewed-on: https://gerrit.libreoffice.org/c/core/+/121584
    Tested-by: Jenkins
    Reviewed-by: Luboš Luňák <l.lu...@collabora.com>

diff --git a/sal/osl/unx/backtraceapi.cxx b/sal/osl/unx/backtraceapi.cxx
index aee29991b632..e24a9150c91d 100644
--- a/sal/osl/unx/backtraceapi.cxx
+++ b/sal/osl/unx/backtraceapi.cxx
@@ -49,35 +49,57 @@ std::unique_ptr<sal::BacktraceState> 
sal::backtrace_get(sal_uInt32 maxDepth)
 // (e.g. the calls could be grouped by the binary).
 #include <dlfcn.h>
 #include <unistd.h>
+#include <vector>
 #include <osl/process.h>
 #include <rtl/strbuf.hxx>
 #include "file_url.hxx"
 
-static OString symbol_addr2line_info(const char* file, ptrdiff_t offset)
+namespace
+{
+struct FrameData
+{
+    const char* file = nullptr;
+    ptrdiff_t offset;
+    OString info;
+    bool handled = false;
+};
+
+void process_file_addr2line( const char* file, std::vector<FrameData>& 
frameData )
 {
     OUString binary("addr2line");
     OUString dummy;
-    if(!osl::detail::find_in_PATH(binary, dummy) || access( file, R_OK ) != 0)
-        return OString(); // Will not work, avoid warnings from osl process 
code.
     OUString arg1("-Cfe");
     OUString arg2 = OUString::fromUtf8(file);
-    OUString arg3 = "0x" + OUString::number(offset, 16);
-    rtl_uString* args[] = { arg1.pData, arg2.pData, arg3.pData };
-    sal_Int32 nArgs = 3;
+    std::vector<OUString> addrs;
+    std::vector<rtl_uString*> args;
+    args.reserve(frameData.size() + 2);
+    args.push_back( arg1.pData );
+    args.push_back( arg2.pData );
+    for( FrameData& frame : frameData )
+    {
+        if( strcmp( file, frame.file ) == 0 )
+        {
+            addrs.push_back("0x" + OUString::number(frame.offset, 16));
+            args.push_back(addrs.back().pData);
+            frame.handled = true;
+        }
+    }
 
+    if(!osl::detail::find_in_PATH(binary, dummy) || access( file, R_OK ) != 0)
+        return; // Will not work, avoid warnings from osl process code.
     oslProcess aProcess;
     oslFileHandle pOut = nullptr;
     oslFileHandle pErr = nullptr;
     oslSecurity pSecurity = osl_getCurrentSecurity();
     oslProcessError eErr = osl_executeProcess_WithRedirectedIO(
-        binary.pData, args, nArgs, osl_Process_SEARCHPATH | 
osl_Process_HIDDEN, pSecurity, nullptr,
+        binary.pData, args.data(), args.size(), osl_Process_SEARCHPATH | 
osl_Process_HIDDEN, pSecurity, nullptr,
         nullptr, 0, &aProcess, nullptr, &pOut, &pErr);
     osl_freeSecurityHandle(pSecurity);
 
     if (eErr != osl_Process_E_None)
-        return OString();
+        return;
 
-    OStringBuffer result;
+    OStringBuffer outputBuffer;
     if (pOut)
     {
         const sal_uInt64 BUF_SIZE = 1024;
@@ -91,7 +113,7 @@ static OString symbol_addr2line_info(const char* file, 
ptrdiff_t offset)
             oslFileError err = osl_readFile(pOut, buffer, BUF_SIZE, 
&bytesRead);
             if(bytesRead == 0 && err == osl_File_E_None)
                 break;
-            result.append(buffer, bytesRead);
+            outputBuffer.append(buffer, bytesRead);
             if (err != osl_File_E_None && err != osl_File_E_AGAIN)
                 break;
         }
@@ -101,42 +123,71 @@ static OString symbol_addr2line_info(const char* file, 
ptrdiff_t offset)
         osl_closeFile(pErr);
     eErr = osl_joinProcess(aProcess);
     osl_freeProcessHandle(aProcess);
-    return result.makeStringAndClear();
-}
 
-static OString symbol_addr2line(void* addr)
-{
-    Dl_info dli;
-    ptrdiff_t offset;
-    if (dladdr(addr, &dli) != 0)
+    OString output = outputBuffer.makeStringAndClear();
+    std::vector<OString> lines;
+    sal_Int32 outputPos = 0;
+    while(outputPos < output.getLength())
     {
-        if (dli.dli_fname && dli.dli_fbase)
+        sal_Int32 end1 = output.indexOf('\n', outputPos);
+        if(end1 < 0)
+            break;
+        sal_Int32 end2 = output.indexOf('\n', end1 + 1);
+        if(end2 < 0)
+            end2 = output.getLength();
+        lines.push_back(output.copy( outputPos, end1 - outputPos ));
+        lines.push_back(output.copy( end1 + 1, end2 - end1 - 1 ));
+        outputPos = end2 + 1;
+    }
+    if(lines.size() != addrs.size() * 2)
+        return; // addr2line problem?
+    size_t linesPos = 0;
+    for( FrameData& frame : frameData )
+    {
+        if( strcmp( file, frame.file ) == 0 )
         {
-            offset = reinterpret_cast<ptrdiff_t>(addr) - 
reinterpret_cast<ptrdiff_t>(dli.dli_fbase);
-            OString info = symbol_addr2line_info(dli.dli_fname, offset);
             // There should be two lines, first function name and second 
source file information.
             // If each of them starts with ??, it is invalid/unknown.
-            if(!info.isEmpty() && !info.startsWith("??"))
+            OString function = lines[linesPos];
+            OString source = lines[linesPos+1];
+            linesPos += 2;
+            if(!function.isEmpty() && !function.startsWith("??"))
             {
-                sal_Int32 end1 = info.indexOf('\n');
-                if(end1 > 0)
-                {
-                    OString function = info.copy( 0, end1 );
-                    sal_Int32 end2 = info.indexOf('\n', end1 + 1);
-                    OString source = info.copy( end1 + 1, end2 > 0 ? end2 - 
end1 - 1 : info.getLength() - end1 );
-                    if( source.startsWith("??"))
-                        return function + " in " + dli.dli_fname;
-                    else
-                        return function + " at " + source;
-                }
+                if( source.startsWith("??"))
+                    frame.info = function + " in " + file;
+                else
+                    frame.info = function + " at " + source;
             }
         }
     }
-    return OString();
 }
 
+} // namespace
+
 OUString sal::backtrace_to_string(BacktraceState* backtraceState)
 {
+    // Collect frames for each binary and process each binary in one addr2line
+    // call for better performance.
+    std::vector< FrameData > frameData;
+    frameData.resize(backtraceState->nDepth);
+    for (int i = 0; i != backtraceState->nDepth; ++i)
+    {
+        Dl_info dli;
+        void* addr = backtraceState->buffer[i];
+        if (dladdr(addr, &dli) != 0)
+        {
+            if (dli.dli_fname && dli.dli_fbase)
+            {
+                frameData[ i ].file = dli.dli_fname;
+                frameData[ i ].offset = reinterpret_cast<ptrdiff_t>(addr) - 
reinterpret_cast<ptrdiff_t>(dli.dli_fbase);
+            }
+        }
+    }
+    for (int i = 0; i != backtraceState->nDepth; ++i)
+    {
+        if(frameData[ i ].file != nullptr && !frameData[ i ].handled)
+            process_file_addr2line( frameData[ i ].file, frameData );
+    }
     OUStringBuffer b3;
     std::unique_ptr<char*, decltype(free)*> b2{ nullptr, free };
     bool fallbackInitDone = false;
@@ -144,9 +195,8 @@ OUString sal::backtrace_to_string(BacktraceState* 
backtraceState)
     {
         if (i != 0)
             b3.append("\n");
-        OString info = symbol_addr2line(backtraceState->buffer[i]);
-        if(!info.isEmpty())
-            b3.append(o3tl::runtimeToOUString(info.getStr()));
+        if(!frameData[i].info.isEmpty())
+            b3.append(o3tl::runtimeToOUString(frameData[i].info.getStr()));
         else
         {
             if(!fallbackInitDone)
commit 665bd649088e6dccf4d53251e10b182950259b46
Author:     Luboš Luňák <l.lu...@collabora.com>
AuthorDate: Thu Sep 2 23:18:54 2021 +0200
Commit:     Luboš Luňák <l.lu...@collabora.com>
CommitDate: Mon Sep 6 14:40:21 2021 +0200

    improve sal::backtrace_get() by using addr2line in debug builds
    
    The backtrace_symbols() function provides backtraces that often
    miss many function names, try harder to resolve them, using addr2line
    is the best (only?) working solution I've found.
    
    Change-Id: Ieda06fc52735596e499fb7f9443cae13d134da5c
    Reviewed-on: https://gerrit.libreoffice.org/c/core/+/121539
    Tested-by: Jenkins
    Reviewed-by: Luboš Luňák <l.lu...@collabora.com>

diff --git a/sal/osl/unx/backtraceapi.cxx b/sal/osl/unx/backtraceapi.cxx
index d11128353d47..aee29991b632 100644
--- a/sal/osl/unx/backtraceapi.cxx
+++ b/sal/osl/unx/backtraceapi.cxx
@@ -23,18 +23,6 @@
 #include "backtrace.h"
 #include <backtraceasstring.hxx>
 
-namespace {
-
-struct FreeGuard {
-    FreeGuard(char ** theBuffer): buffer(theBuffer) {}
-
-    ~FreeGuard() { std::free(buffer); }
-
-    char ** buffer;
-};
-
-}
-
 OUString osl::detail::backtraceAsString(sal_uInt32 maxDepth) {
     std::unique_ptr<sal::BacktraceState> backtrace = sal::backtrace_get( 
maxDepth );
     return sal::backtrace_to_string( backtrace.get());
@@ -53,10 +41,135 @@ std::unique_ptr<sal::BacktraceState> 
sal::backtrace_get(sal_uInt32 maxDepth)
     return std::unique_ptr<BacktraceState>(new BacktraceState{ b1, n });
 }
 
+#if OSL_DEBUG_LEVEL > 0 && (defined LINUX || defined MACOSX || defined FREEBSD 
|| defined NETBSD || defined OPENBSD || defined(DRAGONFLY))
+// The backtrace_symbols() function is unreliable, it requires -rdynamic and 
even then it cannot resolve names
+// of many functions, such as those with hidden ELF visibility. Libunwind 
doesn't resolve names for me either,
+// boost::stacktrace doesn't work properly, the best result I've found is 
addr2line. Using addr2line is relatively
+// slow, but I don't find that to be a big problem for printing of backtraces. 
Feel free to improve if needed
+// (e.g. the calls could be grouped by the binary).
+#include <dlfcn.h>
+#include <unistd.h>
+#include <osl/process.h>
+#include <rtl/strbuf.hxx>
+#include "file_url.hxx"
+
+static OString symbol_addr2line_info(const char* file, ptrdiff_t offset)
+{
+    OUString binary("addr2line");
+    OUString dummy;
+    if(!osl::detail::find_in_PATH(binary, dummy) || access( file, R_OK ) != 0)
+        return OString(); // Will not work, avoid warnings from osl process 
code.
+    OUString arg1("-Cfe");
+    OUString arg2 = OUString::fromUtf8(file);
+    OUString arg3 = "0x" + OUString::number(offset, 16);
+    rtl_uString* args[] = { arg1.pData, arg2.pData, arg3.pData };
+    sal_Int32 nArgs = 3;
+
+    oslProcess aProcess;
+    oslFileHandle pOut = nullptr;
+    oslFileHandle pErr = nullptr;
+    oslSecurity pSecurity = osl_getCurrentSecurity();
+    oslProcessError eErr = osl_executeProcess_WithRedirectedIO(
+        binary.pData, args, nArgs, osl_Process_SEARCHPATH | 
osl_Process_HIDDEN, pSecurity, nullptr,
+        nullptr, 0, &aProcess, nullptr, &pOut, &pErr);
+    osl_freeSecurityHandle(pSecurity);
+
+    if (eErr != osl_Process_E_None)
+        return OString();
+
+    OStringBuffer result;
+    if (pOut)
+    {
+        const sal_uInt64 BUF_SIZE = 1024;
+        char buffer[BUF_SIZE];
+        while (true)
+        {
+            sal_uInt64 bytesRead = 0;
+            while(osl_readFile(pErr, buffer, BUF_SIZE, &bytesRead) == 
osl_File_E_None
+                && bytesRead != 0)
+                ; // discard possible stderr output
+            oslFileError err = osl_readFile(pOut, buffer, BUF_SIZE, 
&bytesRead);
+            if(bytesRead == 0 && err == osl_File_E_None)
+                break;
+            result.append(buffer, bytesRead);
+            if (err != osl_File_E_None && err != osl_File_E_AGAIN)
+                break;
+        }
+        osl_closeFile(pOut);
+    }
+    if(pErr)
+        osl_closeFile(pErr);
+    eErr = osl_joinProcess(aProcess);
+    osl_freeProcessHandle(aProcess);
+    return result.makeStringAndClear();
+}
+
+static OString symbol_addr2line(void* addr)
+{
+    Dl_info dli;
+    ptrdiff_t offset;
+    if (dladdr(addr, &dli) != 0)
+    {
+        if (dli.dli_fname && dli.dli_fbase)
+        {
+            offset = reinterpret_cast<ptrdiff_t>(addr) - 
reinterpret_cast<ptrdiff_t>(dli.dli_fbase);
+            OString info = symbol_addr2line_info(dli.dli_fname, offset);
+            // There should be two lines, first function name and second 
source file information.
+            // If each of them starts with ??, it is invalid/unknown.
+            if(!info.isEmpty() && !info.startsWith("??"))
+            {
+                sal_Int32 end1 = info.indexOf('\n');
+                if(end1 > 0)
+                {
+                    OString function = info.copy( 0, end1 );
+                    sal_Int32 end2 = info.indexOf('\n', end1 + 1);
+                    OString source = info.copy( end1 + 1, end2 > 0 ? end2 - 
end1 - 1 : info.getLength() - end1 );
+                    if( source.startsWith("??"))
+                        return function + " in " + dli.dli_fname;
+                    else
+                        return function + " at " + source;
+                }
+            }
+        }
+    }
+    return OString();
+}
+
 OUString sal::backtrace_to_string(BacktraceState* backtraceState)
 {
-    FreeGuard b2(backtrace_symbols(backtraceState->buffer, 
backtraceState->nDepth));
-    if (b2.buffer == nullptr) {
+    OUStringBuffer b3;
+    std::unique_ptr<char*, decltype(free)*> b2{ nullptr, free };
+    bool fallbackInitDone = false;
+    for (int i = 0; i != backtraceState->nDepth; ++i)
+    {
+        if (i != 0)
+            b3.append("\n");
+        OString info = symbol_addr2line(backtraceState->buffer[i]);
+        if(!info.isEmpty())
+            b3.append(o3tl::runtimeToOUString(info.getStr()));
+        else
+        {
+            if(!fallbackInitDone)
+            {
+                b2 = std::unique_ptr<char*, decltype(free)*>
+                    {backtrace_symbols(backtraceState->buffer, 
backtraceState->nDepth), free};
+                fallbackInitDone = true;
+            }
+            if(b2)
+                b3.append(o3tl::runtimeToOUString(b2.get()[i]));
+            else
+                b3.append("??");
+        }
+    }
+    return b3.makeStringAndClear();
+}
+
+#else
+
+OUString sal::backtrace_to_string(BacktraceState* backtraceState)
+{
+    std::unique_ptr<char*, decltype(free)*> 
b2{backtrace_symbols(backtraceState->buffer, backtraceState->nDepth), free};
+    if (b2.get() == nullptr) {
         return OUString();
     }
     OUStringBuffer b3;
@@ -64,9 +177,11 @@ OUString sal::backtrace_to_string(BacktraceState* 
backtraceState)
         if (i != 0) {
             b3.append("\n");
         }
-        b3.append(o3tl::runtimeToOUString(b2.buffer[i]));
+        b3.append(o3tl::runtimeToOUString(b2.get()[i]));
     }
     return b3.makeStringAndClear();
 }
 
+#endif
+
 /* vim:set shiftwidth=4 softtabstop=4 expandtab: */

Reply via email to