Skip to content

Commit

Permalink
Misc createdump fixes (#73426)
Browse files Browse the repository at this point in the history
Add --crashreportonly command line option that doesn't generated a dump.

Add matching DOTNET_EnableCrashReportOnly env var.

Add LoadModule error logging for MacOS.

Make DAC validate EEClass and MethodDesc functions more robust on Linux/MacOS so SOS's eestack and dumpstack commands don't segfault.

Update createdump doc.
  • Loading branch information
mikem8361 authored Aug 5, 2022
1 parent 4b7f748 commit 543bcc5
Show file tree
Hide file tree
Showing 10 changed files with 81 additions and 35 deletions.
10 changes: 9 additions & 1 deletion docs/design/coreclr/botr/xplat-minidump-generation.md
Original file line number Diff line number Diff line change
Expand Up @@ -56,13 +56,18 @@ NOTE: Core dump generation in docker containers require the ptrace capability (-

Any configuration or policy is set with environment variables which are passed as options to the _createdump_ utility.

Starting with .NET 7.0 or greater the below environment variables can be prefixed with DOTNET_ or COMPlus_.

Environment variables supported:

- `COMPlus_DbgEnableMiniDump`: if set to "1", enables this core dump generation. The default is NOT to generate a dump.
- `COMPlus_DbgMiniDumpType`: See below. Default is "2" MiniDumpWithPrivateReadWriteMemory.
- `COMPlus_DbgMiniDumpName`: if set, use as the template to create the dump path and file name. See "Dump name formatting" for how the dump name can be formatted. The default is _/tmp/coredump.%p_.
- `COMPlus_CreateDumpDiagnostics`: if set to "1", enables the _createdump_ utilities diagnostic messages (TRACE macro).
- `COMPlus_CreateDumpVerboseDiagnostics`: if set to "1", enables the _createdump_ utilities verbose diagnostic messages (TRACE_VERBOSE macro).
- `COMPlus_CreateDumpLogToFile`: if set, it is the path of the file to write the _createdump_ diagnostic messages.
- `COMPlus_EnableCrashReport`: In .NET 6.0 or greater, if set to "1", createdump also generates a json formatted crash report which includes information about the threads and stack frames of the crashing application. The crash report name is the dump path/name with _.crashreport.json_ appended.
- `COMPlus_EnableCrashReportOnly`: In .NET 7.0 or greater, same as COMPlus_EnableCrashReport except the core dump is not generated.

COMPlus_DbgMiniDumpType values:

Expand Down Expand Up @@ -93,9 +98,12 @@ createdump [options] pid
-u, --full - create full core dump.
-d, --diag - enable diagnostic messages.
-v, --verbose - enable verbose diagnostic messages.
--crashreport - write crash report file.
-l, --logtofile - file path and name to log diagnostic messages.
--crashreport - write crash report file (dump file path + .crashreport.json).
--crashreportonly - write crash report file only (no dump).
--crashthread <id> - the thread id of the crashing thread.
--signal <code> - the signal code of the crash.
--singlefile - enable single-file app check.
```

**Dump name formatting**
Expand Down
12 changes: 12 additions & 0 deletions src/coreclr/debug/createdump/crashinfomac.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -463,6 +463,10 @@ ModuleInfo::LoadModule()
{
g_image_infos = (const struct dyld_all_image_infos*)dyld_info.all_image_info_addr;
}
else
{
TRACE("LoadModule: task_info(self) FAILED %x %s\n", result, mach_error_string(result));
}
}
if (g_image_infos != nullptr)
{
Expand All @@ -475,7 +479,15 @@ ModuleInfo::LoadModule()
break;
}
}
if (m_localBaseAddress == 0)
{
TRACE("LoadModule: local base address not found for %s\n", m_moduleName.c_str());
}
}
}
else
{
TRACE("LoadModule: dlopen(%s) FAILED %d %s\n", m_moduleName.c_str(), errno, strerror(errno));
}
}
}
2 changes: 1 addition & 1 deletion src/coreclr/debug/createdump/createdump.h
Original file line number Diff line number Diff line change
Expand Up @@ -107,7 +107,7 @@ typedef int T_CONTEXT;
#endif

extern bool FormatDumpName(std::string& name, const char* pattern, const char* exename, int pid);
extern bool CreateDump(const char* dumpPathTemplate, int pid, const char* dumpType, MINIDUMP_TYPE minidumpType, bool crashReport, int crashThread, int signal);
extern bool CreateDump(const char* dumpPathTemplate, int pid, const char* dumpType, MINIDUMP_TYPE minidumpType, bool createDump, bool crashReport, int crashThread, int signal);

extern std::string GetLastErrorString();
extern void printf_status(const char* format, ...);
Expand Down
43 changes: 23 additions & 20 deletions src/coreclr/debug/createdump/createdumpunix.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ long g_pageSize = 0;
// The Linux/MacOS create dump code
//
bool
CreateDump(const char* dumpPathTemplate, int pid, const char* dumpType, MINIDUMP_TYPE minidumpType, bool crashReport, int crashThread, int signal)
CreateDump(const char* dumpPathTemplate, int pid, const char* dumpType, MINIDUMP_TYPE minidumpType, bool createDump, bool crashReport, int crashThread, int signal)
{
ReleaseHolder<CrashInfo> crashInfo = new CrashInfo(pid, crashReport, crashThread, signal);
DumpWriter dumpWriter(*crashInfo);
Expand Down Expand Up @@ -57,28 +57,31 @@ CreateDump(const char* dumpPathTemplate, int pid, const char* dumpType, MINIDUMP
CrashReportWriter crashReportWriter(*crashInfo);
crashReportWriter.WriteCrashReport(dumpPath);
}
// Gather all the useful memory regions from the DAC
if (!crashInfo->EnumerateMemoryRegionsWithDAC(minidumpType))
if (createDump)
{
goto exit;
}
// Join all adjacent memory regions
crashInfo->CombineMemoryRegions();

printf_status("Writing %s to file %s\n", dumpType, dumpPath.c_str());
// Gather all the useful memory regions from the DAC
if (!crashInfo->EnumerateMemoryRegionsWithDAC(minidumpType))
{
goto exit;
}
// Join all adjacent memory regions
crashInfo->CombineMemoryRegions();

printf_status("Writing %s to file %s\n", dumpType, dumpPath.c_str());

// Write the actual dump file
if (!dumpWriter.OpenDump(dumpPath.c_str()))
{
goto exit;
}
if (!dumpWriter.WriteDump())
{
printf_error("Writing dump FAILED\n");
// Write the actual dump file
if (!dumpWriter.OpenDump(dumpPath.c_str()))
{
goto exit;
}
if (!dumpWriter.WriteDump())
{
printf_error("Writing dump FAILED\n");

// Delete the partial dump file on error
remove(dumpPath.c_str());
goto exit;
// Delete the partial dump file on error
remove(dumpPath.c_str());
goto exit;
}
}
result = true;
exit:
Expand Down
5 changes: 4 additions & 1 deletion src/coreclr/debug/createdump/createdumpwindows.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -18,12 +18,15 @@ typedef struct _PROCESS_BASIC_INFORMATION_ {
// The Windows create dump code
//
bool
CreateDump(const char* dumpPathTemplate, int pid, const char* dumpType, MINIDUMP_TYPE minidumpType, bool crashReport, int crashThread, int signal)
CreateDump(const char* dumpPathTemplate, int pid, const char* dumpType, MINIDUMP_TYPE minidumpType, bool createDump, bool crashReport, int crashThread, int signal)
{
HANDLE hFile = INVALID_HANDLE_VALUE;
HANDLE hProcess = NULL;
bool result = false;

_ASSERTE(createDump);
_ASSERTE(!crashReport);

ArrayHolder<char> pszName = new char[MAX_LONGPATH + 1];
std::string dumpPath;

Expand Down
9 changes: 8 additions & 1 deletion src/coreclr/debug/createdump/main.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,7 @@ const char* g_help = "createdump [options]\n"
"-l, --logtofile - file path and name to log diagnostic messages.\n"
#ifdef HOST_UNIX
"--crashreport - write crash report file (dump file path + .crashreport.json).\n"
"--crashreportonly - write crash report file only (no dump).\n"
"--crashthread <id> - the thread id of the crashing thread.\n"
"--signal <code> - the signal code of the crash.\n"
"--singlefile - enable single-file app check.\n"
Expand Down Expand Up @@ -64,6 +65,7 @@ int __cdecl main(const int argc, const char* argv[])
const char* dumpType = "minidump with heap";
const char* dumpPathTemplate = nullptr;
bool crashReport = false;
bool createDump = true;
bool help = false;
int signal = 0;
int crashThread = 0;
Expand Down Expand Up @@ -136,6 +138,11 @@ int __cdecl main(const int argc, const char* argv[])
{
crashReport = true;
}
else if (strcmp(*argv, "--crashreportonly") == 0)
{
crashReport = true;
createDump = false;
}
else if (strcmp(*argv, "--crashthread") == 0)
{
crashThread = atoi(*++argv);
Expand Down Expand Up @@ -221,7 +228,7 @@ int __cdecl main(const int argc, const char* argv[])
dumpPathTemplate = tmpPath;
}

if (CreateDump(dumpPathTemplate, pid, dumpType, minidumpType, crashReport, crashThread, signal))
if (CreateDump(dumpPathTemplate, pid, dumpType, minidumpType, createDump, crashReport, crashThread, signal))
{
printf_status("Dump successfully written in %llums\n", GetTimeStamp() - g_startTime);
}
Expand Down
14 changes: 7 additions & 7 deletions src/coreclr/debug/daccess/request.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -106,15 +106,15 @@ PTR_SyncBlock DACGetSyncBlockFromObjectPointer(TADDR objAddr, ICorDebugDataTarge
return ste->m_SyncBlock;
}

BOOL DacValidateEEClass(EEClass *pEEClass)
BOOL DacValidateEEClass(PTR_EEClass pEEClass)
{
// Verify things are right.
// The EEClass method table pointer should match the method table.
// TODO: Microsoft, need another test for validity, this one isn't always true anymore.
BOOL retval = TRUE;
EX_TRY
{
MethodTable *pMethodTable = pEEClass->GetMethodTable();
PTR_MethodTable pMethodTable = pEEClass->GetMethodTable();
if (!pMethodTable)
{
// PREfix.
Expand Down Expand Up @@ -181,7 +181,7 @@ BadMethodTable: ;

}

BOOL DacValidateMD(MethodDesc * pMD)
BOOL DacValidateMD(PTR_MethodDesc pMD)
{
if (pMD == NULL)
{
Expand All @@ -192,7 +192,7 @@ BOOL DacValidateMD(MethodDesc * pMD)
BOOL retval = TRUE;
EX_TRY
{
MethodTable *pMethodTable = pMD->GetMethodTable();
PTR_MethodTable pMethodTable = pMD->GetMethodTable();

// Standard fast check
if (!pMethodTable->ValidateWithPossibleAV())
Expand Down Expand Up @@ -245,7 +245,7 @@ BOOL DacValidateMD(MethodDesc * pMD)

BOOL DacValidateMD(LPCVOID pMD)
{
return DacValidateMD((MethodDesc *)pMD);
return DacValidateMD(dac_cast<PTR_MethodDesc>(pMD));
}

VOID GetJITMethodInfo (EECodeInfo * pCodeInfo, JITTypes *pJITType, CLRDATA_ADDRESS *pGCInfo)
Expand Down Expand Up @@ -1227,7 +1227,7 @@ ClrDataAccess::GetMethodDescTransparencyData(CLRDATA_ADDRESS methodDesc, struct

SOSDacEnter();

MethodDesc *pMD = PTR_MethodDesc(TO_TADDR(methodDesc));
PTR_MethodDesc pMD = PTR_MethodDesc(TO_TADDR(methodDesc));
if (!DacValidateMD(pMD))
{
hr = E_INVALIDARG;
Expand Down Expand Up @@ -2016,7 +2016,7 @@ ClrDataAccess::GetMethodTableForEEClass(CLRDATA_ADDRESS eeClass, CLRDATA_ADDRESS

SOSDacEnter();

EEClass * pClass = PTR_EEClass(TO_TADDR(eeClass));
PTR_EEClass pClass = PTR_EEClass(TO_TADDR(eeClass));
if (!DacValidateEEClass(pClass))
{
hr = E_INVALIDARG;
Expand Down
3 changes: 2 additions & 1 deletion src/coreclr/pal/inc/pal.h
Original file line number Diff line number Diff line change
Expand Up @@ -452,7 +452,8 @@ enum
GenerateDumpFlagsNone = 0x00,
GenerateDumpFlagsLoggingEnabled = 0x01,
GenerateDumpFlagsVerboseLoggingEnabled = 0x02,
GenerateDumpFlagsCrashReportEnabled = 0x04
GenerateDumpFlagsCrashReportEnabled = 0x04,
GenerateDumpFlagsCrashReportOnlyEnabled = 0x08
};

PALIMPORT
Expand Down
15 changes: 13 additions & 2 deletions src/coreclr/pal/src/thread/process.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -2344,6 +2344,11 @@ PROCBuildCreateDumpCommandLine(
argv.push_back("--crashreport");
}

if (flags & GenerateDumpFlagsCrashReportOnlyEnabled)
{
argv.push_back("--crashreportonly");
}

if (g_running_in_exe)
{
argv.push_back("--singlefile");
Expand Down Expand Up @@ -2519,12 +2524,18 @@ PROCAbortInitialize()
{
flags |= GenerateDumpFlagsVerboseLoggingEnabled;
}
CLRConfigNoCache enabldReportCfg = CLRConfigNoCache::Get("EnableCrashReport", /*noprefix*/ false, &getenv);
CLRConfigNoCache enabledReportCfg = CLRConfigNoCache::Get("EnableCrashReport", /*noprefix*/ false, &getenv);
val = 0;
if (enabldReportCfg.IsSet() && enabldReportCfg.TryAsInteger(10, val) && val == 1)
if (enabledReportCfg.IsSet() && enabledReportCfg.TryAsInteger(10, val) && val == 1)
{
flags |= GenerateDumpFlagsCrashReportEnabled;
}
CLRConfigNoCache enabledReportOnlyCfg = CLRConfigNoCache::Get("EnableCrashReportOnly", /*noprefix*/ false, &getenv);
val = 0;
if (enabledReportOnlyCfg.IsSet() && enabledReportOnlyCfg.TryAsInteger(10, val) && val == 1)
{
flags |= GenerateDumpFlagsCrashReportOnlyEnabled;
}

char* program = nullptr;
char* pidarg = nullptr;
Expand Down
3 changes: 2 additions & 1 deletion src/coreclr/vm/excep.h
Original file line number Diff line number Diff line change
Expand Up @@ -201,7 +201,8 @@ enum
GenerateDumpFlagsNone = 0x00,
GenerateDumpFlagsLoggingEnabled = 0x01,
GenerateDumpFlagsVerboseLoggingEnabled = 0x02,
GenerateDumpFlagsCrashReportEnabled = 0x04
GenerateDumpFlagsCrashReportEnabled = 0x04,
GenerateDumpFlagsCrashReportOnlyEnabled = 0x08
};

void InitializeCrashDump();
Expand Down

0 comments on commit 543bcc5

Please sign in to comment.