Skip to content
This repository was archived by the owner on Jan 23, 2023. It is now read-only.

Add work around a bug in lldb on a core dump #1327

Merged
merged 1 commit into from
Aug 4, 2015
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion src/ToolBox/SOS/Strike/strike.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -14019,4 +14019,4 @@ Help(PDEBUG_CLIENT Client, PCSTR Args)
return S_OK;
}

#endif // !FEATURE_PAL
#endif // FEATURE_PAL
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not necessarily something to address in this PR, but it looks like there's some hairy #ifdef nesting going on in this file. It took me a while to find the matching #ifndef FEATURE_PAL (I think it's the one at line 12660).

The other issue with it is that there is some inconsistency with what the comments next to the endif's say when they're closing an #ifndef. If there's #ifndef FOO, sometimes the endif is #endif // FOO and elsewhere it's #endif // !FOO. This makes those comments almost useless since you end up having to scroll up to figure out what's going on anyway.

Again, this doesn't need to be fixed in this PR, but I think it would be good to standardize that and also detangle some of the nesting in this file.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, it is a big mess. I’ll try to clean this up in the future.

From: Aditya Mandaleeka [mailto:notifications@github.com]
Sent: Monday, August 3, 2015 12:15 PM
To: dotnet/coreclr coreclr@noreply.github.com
Cc: Mike McLaughlin mikem@microsoft.com
Subject: Re: [coreclr] Add work around a bug in lldb on a core dump (#1327)

In src/ToolBox/SOS/Strike/strike.cpphttps://github.com//pull/1327#discussion_r36120010:

@@ -14019,4 +14019,4 @@ Help(PDEBUG_CLIENT Client, PCSTR Args)

 return S_OK;

}

-#endif // !FEATURE_PAL

+#endif // FEATURE_PAL

Not necessarily something to address in this PR, but it looks like there's some hairy #ifdef nesting going on in this file. It took me a while to find the matching #ifndef FEATURE_PAL (I think it's the one at line 12660).

The other issue with it is that there is some inconsistency with what the comments next to the endif's say when they're closing an #ifndef. If there's #ifndef FOO, sometimes the endif is #endif // FOO and elsewhere it's #endif // !FOO. This makes those comments almost useless since you end up having to scroll up to figure out what's going on anyway.

Again, this doesn't need to be fixed in this PR, but I think it would be good to standardize that and also detangle some of the nesting in this file.


Reply to this email directly or view it on GitHubhttps://github.com//pull/1327/files#r36120010.

4 changes: 3 additions & 1 deletion src/ToolBox/SOS/lldbplugin/CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@ if(NOT ENABLE_LLDBPLUGIN)
endif()

# Check for LLDB library
find_library(LLDB NAMES lldb-3.6 lldb-3.5 LLDB lldb PATHS "${WITH_LLDB_LIBS}" PATH_SUFFIXES llvm)
find_library(LLDB NAMES LLDB lldb lldb-3.6 lldb-3.5 PATHS "${WITH_LLDB_LIBS}" PATH_SUFFIXES llvm)
if(LLDB STREQUAL LLDB-NOTFOUND)
if(REQUIRE_LLDBPLUGIN)
message(FATAL_ERROR "Cannot find lldb-3.5 or lldb-3.6. Try installing lldb-3.6-dev (or the appropriate package for your platform)")
Expand Down Expand Up @@ -70,6 +70,8 @@ include_directories(${CLR_DIR}/src/coreclr/hosts/unixcoreruncommon)
set(SOURCES
sosplugin.cpp
soscommand.cpp
setclrpathcommand.cpp
setsostidcommand.cpp
coreruncommand.cpp
debugclient.cpp
${CLR_DIR}/src/coreclr/hosts/unixcorerun/corerun.cpp
Expand Down
56 changes: 47 additions & 9 deletions src/ToolBox/SOS/lldbplugin/debugclient.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -10,10 +10,13 @@
#include <dbgtargetcontext.h>
#include <string>

DebugClient::DebugClient(lldb::SBDebugger &debugger, lldb::SBCommandReturnObject &returnObject, char *coreclrDirectory) :
ULONG g_currentThreadIndex = -1;
ULONG g_currentThreadSystemId = -1;
char *g_coreclrDirectory;

DebugClient::DebugClient(lldb::SBDebugger &debugger, lldb::SBCommandReturnObject &returnObject) :
m_debugger(debugger),
m_returnObject(returnObject),
m_coreclrDirectory(coreclrDirectory)
m_returnObject(returnObject)
{
returnObject.SetStatus(lldb::eReturnStatusSuccessFinishResult);
}
Expand Down Expand Up @@ -741,6 +744,14 @@ DebugClient::GetCurrentThreadId(
return E_FAIL;
}

// This is allow the a valid current TID to be returned to
// workaround a bug in lldb on core dumps.
if (g_currentThreadIndex != -1)
{
*id = g_currentThreadIndex;
return S_OK;
}

*id = thread.GetIndexID();
return S_OK;
}
Expand Down Expand Up @@ -774,6 +785,14 @@ DebugClient::GetCurrentThreadSystemId(
return E_FAIL;
}

// This is allow the a valid current TID to be returned to
// workaround a bug in lldb on core dumps.
if (g_currentThreadSystemId != -1)
{
*sysId = g_currentThreadSystemId;
return S_OK;
}

*sysId = thread.GetThreadID();
return S_OK;
}
Expand All @@ -795,13 +814,22 @@ DebugClient::GetThreadIdBySystemId(
goto exit;
}

thread = process.GetThreadByID(sysId);
if (!thread.IsValid())
// If we have a "fake" thread OS (system) id and a fake thread index,
// we need to return fake thread index.
if (g_currentThreadSystemId == sysId && g_currentThreadIndex != -1)
{
goto exit;
id = g_currentThreadIndex;
}
else
{
thread = process.GetThreadByID(sysId);
if (!thread.IsValid())
{
goto exit;
}

id = thread.GetIndexID();
id = thread.GetIndexID();
}
hr = S_OK;

exit:
Expand Down Expand Up @@ -834,7 +862,17 @@ DebugClient::GetThreadContextById(
goto exit;
}

thread = process.GetThreadByID(threadID);
// If we have a "fake" thread OS (system) id and a fake thread index,
// use the fake thread index to get the context.
if (g_currentThreadSystemId == threadID && g_currentThreadIndex != -1)
{
thread = process.GetThreadByIndexID(g_currentThreadIndex);
}
else
{
thread = process.GetThreadByID(threadID);
}

if (!thread.IsValid())
{
goto exit;
Expand Down Expand Up @@ -1001,7 +1039,7 @@ DebugClient::GetFrameOffset(
PCSTR
DebugClient::GetCoreClrDirectory()
{
return m_coreclrDirectory;
return g_coreclrDirectory;
}

DWORD_PTR
Expand Down
9 changes: 4 additions & 5 deletions src/ToolBox/SOS/lldbplugin/debugclient.h
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,6 @@ class DebugClient : public IDebugClient
private:
lldb::SBDebugger &m_debugger;
lldb::SBCommandReturnObject &m_returnObject;
char *m_coreclrDirectory;

void OutputString(ULONG mask, PCSTR str);
lldb::SBProcess GetCurrentProcess();
Expand All @@ -19,7 +18,7 @@ class DebugClient : public IDebugClient
DWORD_PTR GetRegister(lldb::SBFrame frame, const char *name);

public:
DebugClient(lldb::SBDebugger &debugger, lldb::SBCommandReturnObject &returnObject, char *coreclrDirectory);
DebugClient(lldb::SBDebugger &debugger, lldb::SBCommandReturnObject &returnObject);
~DebugClient();

//----------------------------------------------------------------------------
Expand Down Expand Up @@ -139,6 +138,9 @@ class DebugClient : public IDebugClient
ULONG loadedImageNameBufferSize,
PULONG loadedImageNameSize);

PCSTR GetModuleDirectory(
PCSTR name);

//----------------------------------------------------------------------------
// IDebugSystemObjects
//----------------------------------------------------------------------------
Expand Down Expand Up @@ -187,7 +189,4 @@ class DebugClient : public IDebugClient

DWORD_PTR GetExpression(
PCSTR exp);

PCSTR GetModuleDirectory(
PCSTR name);
};
52 changes: 52 additions & 0 deletions src/ToolBox/SOS/lldbplugin/setclrpathcommand.cpp
Original file line number Diff line number Diff line change
@@ -0,0 +1,52 @@
//
// Copyright (c) Microsoft. All rights reserved.
// Licensed under the MIT license. See LICENSE file in the project root for full license information.
//

#include "sosplugin.h"
#include <dlfcn.h>
#include <string.h>
#include <string>

class setclrpathCommand : public lldb::SBCommandPluginInterface
{
public:
setclrpathCommand()
{
}

virtual bool
DoExecute (lldb::SBDebugger debugger,
char** arguments,
lldb::SBCommandReturnObject &result)
{
if (arguments[0] == NULL)
{
result.Printf("setclrpath error - no path\n");
return false;
}

if (g_coreclrDirectory != NULL)
{
free(g_coreclrDirectory);
}

std::string path(arguments[0]);
if (path[path.length() - 1] != '/')
{
path.append("/");
}

g_coreclrDirectory = strdup(path.c_str());
result.Printf("Set load path for sos/dac/dbi to %s\n", g_coreclrDirectory);
return result.Succeeded();
}
};

bool
setclrpathCommandInitialize(lldb::SBDebugger debugger)
{
lldb::SBCommandInterpreter interpreter = debugger.GetCommandInterpreter();
lldb::SBCommand command = interpreter.AddCommand("setclrpath", new setclrpathCommand(), "Set the path to load coreclr sos/dac/dbi files. setclrpath <path>");
return true;
}
53 changes: 53 additions & 0 deletions src/ToolBox/SOS/lldbplugin/setsostidcommand.cpp
Original file line number Diff line number Diff line change
@@ -0,0 +1,53 @@
//
// Copyright (c) Microsoft. All rights reserved.
// Licensed under the MIT license. See LICENSE file in the project root for full license information.
//

#include "sosplugin.h"
#include <dlfcn.h>
#include <string.h>
#include <string>

class setsostidCommand : public lldb::SBCommandPluginInterface
{
public:
setsostidCommand()
{
}

virtual bool
DoExecute(lldb::SBDebugger debugger,
char** arguments,
lldb::SBCommandReturnObject &result)
{
if (arguments[0] == NULL)
{
result.Printf("Clearing sos thread os id/index\n");
g_currentThreadIndex = -1;
g_currentThreadSystemId = -1;
}
else if (arguments[1] == NULL)
{
result.Printf("Need thread index parameter that maps to the os id\n");
}
else
{
ULONG tid = strtoul(arguments[0], NULL, 16);
g_currentThreadSystemId = tid;

ULONG index = strtoul(arguments[1], NULL, 16);
g_currentThreadIndex = index;

result.Printf("Set sos thread os id to 0x%x which maps to lldb thread index %d\n", tid, index);
}
return result.Succeeded();
}
};

bool
setsostidCommandInitialize(lldb::SBDebugger debugger)
{
lldb::SBCommandInterpreter interpreter = debugger.GetCommandInterpreter();
lldb::SBCommand command = interpreter.AddCommand("setsostid", new setsostidCommand(), "Set the current os tid/thread index instead of using the one lldb provides. setsostid <tid> <index>");
return true;
}
27 changes: 17 additions & 10 deletions src/ToolBox/SOS/lldbplugin/soscommand.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,6 @@
class sosCommand : public lldb::SBCommandPluginInterface
{
void *m_sosHandle;
char m_coreclrDirectory[MAX_PATH];

public:
sosCommand()
Expand All @@ -24,7 +23,7 @@ class sosCommand : public lldb::SBCommandPluginInterface
char** arguments,
lldb::SBCommandReturnObject &result)
{
DebugClient* client = new DebugClient(debugger, result, m_coreclrDirectory);
DebugClient* client = new DebugClient(debugger, result);
if (arguments)
{
LoadSos(client);
Expand Down Expand Up @@ -65,16 +64,24 @@ class sosCommand : public lldb::SBCommandPluginInterface
{
if (m_sosHandle == NULL)
{
const char *coreclrModule = MAKEDLLNAME_A("coreclr");
const char *directory = client->GetModuleDirectory(coreclrModule);
if (directory == NULL)
if (g_coreclrDirectory == NULL)
{
client->Output(DEBUG_OUTPUT_WARNING, "The %s module is not loaded yet in the target process\n", coreclrModule);
const char *coreclrModule = MAKEDLLNAME_A("coreclr");
const char *directory = client->GetModuleDirectory(coreclrModule);
if (directory != NULL)
{
std::string path(directory);
path.append("/");
g_coreclrDirectory = strdup(path.c_str());
}
else
{
client->Output(DEBUG_OUTPUT_WARNING, "The %s module is not loaded yet in the target process\n", coreclrModule);
}
}
else

if (g_coreclrDirectory != NULL)
{
strcpy(m_coreclrDirectory, directory);
strcat(m_coreclrDirectory, "/");

// Load the DAC module first explicitly because SOS and DBI
// have implicit references to the DAC's PAL.
Expand All @@ -88,7 +95,7 @@ class sosCommand : public lldb::SBCommandPluginInterface
void *
LoadModule(DebugClient *client, const char *moduleName)
{
std::string modulePath(m_coreclrDirectory);
std::string modulePath(g_coreclrDirectory);
modulePath.append(moduleName);

void *moduleHandle = dlopen(modulePath.c_str(), RTLD_NOW);
Expand Down
2 changes: 2 additions & 0 deletions src/ToolBox/SOS/lldbplugin/sosplugin.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -14,5 +14,7 @@ lldb::PluginInitialize (lldb::SBDebugger debugger)
{
corerunCommandInitialize(debugger);
sosCommandInitialize(debugger);
setclrpathCommandInitialize(debugger);
setsostidCommandInitialize(debugger);
return true;
}
12 changes: 11 additions & 1 deletion src/ToolBox/SOS/lldbplugin/sosplugin.h
Original file line number Diff line number Diff line change
Expand Up @@ -11,8 +11,18 @@

typedef HRESULT (*CommandFunc)(PDEBUG_CLIENT client, const char *args);

extern char *g_coreclrDirectory;
extern ULONG g_currentThreadIndex;
extern ULONG g_currentThreadSystemId;

bool
sosCommandInitialize(lldb::SBDebugger debugger);

bool
corerunCommandInitialize(lldb::SBDebugger debugger);
setsostidCommandInitialize(lldb::SBDebugger debugger);

bool
setclrpathCommandInitialize(lldb::SBDebugger debugger);

bool
corerunCommandInitialize(lldb::SBDebugger debugger);