Skip to content

Commit

Permalink
Remove createdump's DAC dependency for the PAL for NativeAOT (#88802)
Browse files Browse the repository at this point in the history
* Remove createdump's DAC dependency for the PAL for NativeAOT

* Code review feedback - use utf16 mini pal functions

* Fix OSX build break

* Code review feedback
  • Loading branch information
mikem8361 committed Jul 18, 2023
1 parent 4f933e1 commit fb613a5
Show file tree
Hide file tree
Showing 14 changed files with 401 additions and 71 deletions.
20 changes: 10 additions & 10 deletions src/coreclr/debug/createdump/CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,12 @@ if(CLR_CMAKE_HOST_WIN32)

else(CLR_CMAKE_HOST_WIN32)

if(NOT DEFINED ENV{ROOTFS_DIR})
include_directories(SYSTEM /usr/local/include)
elseif (CLR_CMAKE_TARGET_FREEBSD)
include_directories(SYSTEM $ENV{ROOTFS_DIR}/usr/local/include)
endif()

include(configure.cmake)

# Set the RPATH of createdump so that it can find dependencies without needing to set LD_LIBRARY_PATH
Expand Down Expand Up @@ -64,6 +70,7 @@ else(CLR_CMAKE_HOST_WIN32)
datatarget.cpp
dumpwriter.cpp
crashreportwriter.cpp
${CLR_SRC_NATIVE_DIR}/minipal/utf8.c
)

if(CLR_CMAKE_HOST_OSX)
Expand All @@ -74,9 +81,6 @@ if(CLR_CMAKE_HOST_OSX)
dumpwritermacho.cpp
${CREATEDUMP_SOURCES}
)
add_executable_clr(createdump
main.cpp
)
else()
add_library_clr(createdump_static
STATIC
Expand All @@ -85,25 +89,21 @@ else()
dumpwriterelf.cpp
${CREATEDUMP_SOURCES}
)
endif(CLR_CMAKE_HOST_OSX)

add_executable_clr(createdump
main.cpp
${PAL_REDEFINES_FILE}
createdumppal.cpp
)
add_dependencies(createdump pal_redefines_file)
endif(CLR_CMAKE_HOST_OSX)

target_link_libraries(createdump
PRIVATE
createdump_static
corguids
dbgutil
# share the PAL in the dac module
mscordaccore
dl
)

add_dependencies(createdump mscordaccore)

endif(CLR_CMAKE_HOST_WIN32)

install_clr(TARGETS createdump DESTINATIONS . sharedFramework COMPONENT runtime)
2 changes: 2 additions & 0 deletions src/coreclr/debug/createdump/config.h.in
Original file line number Diff line number Diff line change
Expand Up @@ -4,3 +4,5 @@
#pragma once

#cmakedefine HAVE_PROCESS_VM_READV
#cmakedefine01 HAVE_CLOCK_GETTIME_NSEC_NP
#cmakedefine01 HAVE_CLOCK_MONOTONIC
19 changes: 19 additions & 0 deletions src/coreclr/debug/createdump/configure.cmake
Original file line number Diff line number Diff line change
@@ -1,3 +1,22 @@
check_function_exists(process_vm_readv HAVE_PROCESS_VM_READV)

check_symbol_exists(
clock_gettime_nsec_np
time.h
HAVE_CLOCK_GETTIME_NSEC_NP)

check_cxx_source_runs("
#include <stdlib.h>
#include <time.h>
#include <sys/time.h>
int main()
{
int ret;
struct timespec ts;
ret = clock_gettime(CLOCK_MONOTONIC, &ts);
exit(ret);
}" HAVE_CLOCK_MONOTONIC)

configure_file(${CMAKE_CURRENT_SOURCE_DIR}/config.h.in ${CMAKE_CURRENT_BINARY_DIR}/config.h)
49 changes: 28 additions & 21 deletions src/coreclr/debug/createdump/crashinfo.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -280,10 +280,11 @@ GetHResultString(HRESULT hr)
bool
CrashInfo::InitializeDAC(DumpType dumpType)
{
// Don't attempt to load the DAC if the app model doesn't support it by default. The default for single-file is a
// full dump, but if the dump type requested is a mini, triage or heap and the DAC is side-by-side to the single-file
// application the core dump will be generated.
if (dumpType == DumpType::Full && (m_appModel == AppModelType::SingleFile || m_appModel == AppModelType::NativeAOT))
// Don't attempt to load the DAC if the app model doesn't support it by default. The default for single-file is
// a full dump, but if the dump type requested is a mini, triage or heap and the DAC is next to the single-file
// application the core dump will be generated. For NativeAOT, there is currently no DAC available so never
// attempt to load it.
if ((dumpType == DumpType::Full && m_appModel == AppModelType::SingleFile) || m_appModel == AppModelType::NativeAOT)
{
return true;
}
Expand Down Expand Up @@ -483,19 +484,24 @@ CrashInfo::EnumerateManagedModules()
bool
CrashInfo::UnwindAllThreads()
{
TRACE("UnwindAllThreads: STARTED (%d)\n", m_dataTargetPagesAdded);
ReleaseHolder<ISOSDacInterface> pSos = nullptr;
if (m_pClrDataProcess != nullptr) {
m_pClrDataProcess->QueryInterface(__uuidof(ISOSDacInterface), (void**)&pSos);
}
// For each native and managed thread
for (ThreadInfo* thread : m_threads)
// Don't unwind any threads if Native AOT since there isn't a DAC to get the remote
// unwinder support and they are full dumps.
if (m_appModel != AppModelType::NativeAOT)
{
if (!thread->UnwindThread(m_pClrDataProcess, pSos)) {
return false;
TRACE("UnwindAllThreads: STARTED (%d)\n", m_dataTargetPagesAdded);
ReleaseHolder<ISOSDacInterface> pSos = nullptr;
if (m_pClrDataProcess != nullptr) {
m_pClrDataProcess->QueryInterface(__uuidof(ISOSDacInterface), (void**)&pSos);
}
// For each native and managed thread
for (ThreadInfo* thread : m_threads)
{
if (!thread->UnwindThread(m_pClrDataProcess, pSos)) {
return false;
}
}
TRACE("UnwindAllThreads: FINISHED (%d)\n", m_dataTargetPagesAdded);
}
TRACE("UnwindAllThreads: FINISHED (%d)\n", m_dataTargetPagesAdded);
return true;
}

Expand Down Expand Up @@ -959,9 +965,9 @@ FormatString(const char* format, ...)
ArrayHolder<char> buffer = new char[MAX_LONGPATH + 1];
va_list args;
va_start(args, format);
int result = vsprintf_s(buffer, MAX_LONGPATH, format, args);
int result = vsnprintf(buffer, MAX_LONGPATH, format, args);
va_end(args);
return result > 0 ? std::string(buffer) : std::string();
return result > 0 && result < MAX_LONGPATH ? std::string(buffer) : std::string();
}

//
Expand All @@ -971,15 +977,16 @@ std::string
ConvertString(const WCHAR* str)
{
if (str == nullptr)
return{};
return { };

int len = WideCharToMultiByte(CP_UTF8, 0, str, -1, nullptr, 0, nullptr, nullptr);
size_t cch = u16_strlen(str) + 1;
int len = minipal_get_length_utf16_to_utf8((CHAR16_T*)str, cch, 0);
if (len == 0)
return{};
return { };

ArrayHolder<char> buffer = new char[len + 1];
WideCharToMultiByte(CP_UTF8, 0, str, -1, buffer, len + 1, nullptr, nullptr);
return std::string{ buffer };
minipal_convert_utf16_to_utf8((CHAR16_T*)str, cch, buffer, len + 1, 0);
return std::string { buffer };
}

//
Expand Down
4 changes: 2 additions & 2 deletions src/coreclr/debug/createdump/crashinfomac.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -288,11 +288,11 @@ void CrashInfo::VisitSegment(MachOModule& module, const segment_command_64& segm

// Round to page boundary
start = start & PAGE_MASK;
_ASSERTE(start > 0);
assert(start > 0);

// Round up to page boundary
end = (end + (PAGE_SIZE - 1)) & PAGE_MASK;
_ASSERTE(end > 0);
assert(end > 0);

// Add module memory region if not already on the list
MemoryRegion newModule(regionFlags, start, end, offset, module.Name());
Expand Down
68 changes: 49 additions & 19 deletions src/coreclr/debug/createdump/crashinfounix.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,12 @@ bool
CrashInfo::Initialize()
{
char memPath[128];
_snprintf_s(memPath, sizeof(memPath), sizeof(memPath), "/proc/%u/mem", m_pid);
int chars = snprintf(memPath, sizeof(memPath), "/proc/%u/mem", m_pid);
if (chars <= 0 || (size_t)chars >= sizeof(memPath))
{
printf_error("snprintf failed building /proc/<pid>/mem name\n");
return false;
}

m_fdMem = open(memPath, O_RDONLY);
if (m_fdMem == -1)
Expand All @@ -42,7 +47,12 @@ CrashInfo::Initialize()
{
TRACE("DbgDisablePagemapUse detected - pagemap file checking is enabled\n");
char pagemapPath[128];
_snprintf_s(pagemapPath, sizeof(pagemapPath), sizeof(pagemapPath), "/proc/%u/pagemap", m_pid);
chars = snprintf(pagemapPath, sizeof(pagemapPath), "/proc/%u/pagemap", m_pid);
if (chars <= 0 || (size_t)chars >= sizeof(pagemapPath))
{
printf_error("snprintf failed building /proc/<pid>/pagemap name\n");
return false;
}
m_fdPagemap = open(pagemapPath, O_RDONLY);
if (m_fdPagemap == -1)
{
Expand Down Expand Up @@ -95,7 +105,12 @@ bool
CrashInfo::EnumerateAndSuspendThreads()
{
char taskPath[128];
_snprintf_s(taskPath, sizeof(taskPath), sizeof(taskPath), "/proc/%u/task", m_pid);
int chars = snprintf(taskPath, sizeof(taskPath), "/proc/%u/task", m_pid);
if (chars <= 0 || (size_t)chars >= sizeof(taskPath))
{
printf_error("snprintf failed building /proc/<pid>/task\n");
return false;
}

DIR* taskDir = opendir(taskPath);
if (taskDir == nullptr)
Expand Down Expand Up @@ -139,8 +154,12 @@ bool
CrashInfo::GetAuxvEntries()
{
char auxvPath[128];
_snprintf_s(auxvPath, sizeof(auxvPath), sizeof(auxvPath), "/proc/%u/auxv", m_pid);

int chars = snprintf(auxvPath, sizeof(auxvPath), "/proc/%u/auxv", m_pid);
if (chars <= 0 || (size_t)chars >= sizeof(auxvPath))
{
printf_error("snprintf failed building /proc/<pid>/auxv\n");
return false;
}
int fd = open(auxvPath, O_RDONLY, 0);
if (fd == -1)
{
Expand Down Expand Up @@ -195,9 +214,12 @@ CrashInfo::EnumerateMemoryRegions()

// Making something like: /proc/123/maps
char mapPath[128];
int chars = _snprintf_s(mapPath, sizeof(mapPath), sizeof(mapPath), "/proc/%u/maps", m_pid);
assert(chars > 0 && (size_t)chars <= sizeof(mapPath));

int chars = snprintf(mapPath, sizeof(mapPath), "/proc/%u/maps", m_pid);
if (chars <= 0 || (size_t)chars >= sizeof(mapPath))
{
printf_error("snprintf failed building /proc/<pid>/maps\n");
return false;
}
FILE* mapsFile = fopen(mapPath, "r");
if (mapsFile == nullptr)
{
Expand Down Expand Up @@ -401,19 +423,22 @@ CrashInfo::VisitProgramHeader(uint64_t loadbias, uint64_t baseAddress, Phdr* phd
TRACE("VisitProgramHeader: ehFrameHdrStart %016llx ehFrameHdrSize %08llx\n", ehFrameHdrStart, ehFrameHdrSize);
InsertMemoryRegion(ehFrameHdrStart, ehFrameHdrSize);

ULONG64 ehFrameStart;
ULONG64 ehFrameSize;
if (PAL_GetUnwindInfoSize(baseAddress, ehFrameHdrStart, ReadMemoryAdapter, &ehFrameStart, &ehFrameSize))
if (m_appModel != AppModelType::NativeAOT)
{
TRACE("VisitProgramHeader: ehFrameStart %016llx ehFrameSize %08llx\n", ehFrameStart, ehFrameSize);
if (ehFrameStart != 0 && ehFrameSize != 0)
ULONG64 ehFrameStart;
ULONG64 ehFrameSize;
if (PAL_GetUnwindInfoSize(baseAddress, ehFrameHdrStart, ReadMemoryAdapter, &ehFrameStart, &ehFrameSize))
{
InsertMemoryRegion(ehFrameStart, ehFrameSize);
TRACE("VisitProgramHeader: ehFrameStart %016llx ehFrameSize %08llx\n", ehFrameStart, ehFrameSize);
if (ehFrameStart != 0 && ehFrameSize != 0)
{
InsertMemoryRegion(ehFrameStart, ehFrameSize);
}
}
else
{
TRACE("VisitProgramHeader: PAL_GetUnwindInfoSize FAILED\n");
}
}
else
{
TRACE("VisitProgramHeader: PAL_GetUnwindInfoSize FAILED\n");
}
}
break;
Expand Down Expand Up @@ -489,7 +514,12 @@ bool
GetStatus(pid_t pid, pid_t* ppid, pid_t* tgid, std::string* name)
{
char statusPath[128];
_snprintf_s(statusPath, sizeof(statusPath), sizeof(statusPath), "/proc/%d/status", pid);
int chars = snprintf(statusPath, sizeof(statusPath), "/proc/%d/status", pid);
if (chars <= 0 || (size_t)chars >= sizeof(statusPath))
{
printf_error("snprintf failed building /proc/<pid>/status\n");
return false;
}

FILE *statusFile = fopen(statusPath, "r");
if (statusFile == nullptr)
Expand Down
4 changes: 2 additions & 2 deletions src/coreclr/debug/createdump/crashreportwriter.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -376,15 +376,15 @@ void
CrashReportWriter::WriteValue32(const char* key, uint32_t value)
{
char buffer[16];
_snprintf_s(buffer, sizeof(buffer), sizeof(buffer), "0x%x", value);
snprintf(buffer, sizeof(buffer), "0x%x", value);
WriteValue(key, buffer);
}

void
CrashReportWriter::WriteValue64(const char* key, uint64_t value)
{
char buffer[32];
_snprintf_s(buffer, sizeof(buffer), sizeof(buffer), "0x%" PRIx64, value);
snprintf(buffer, sizeof(buffer), "0x%" PRIx64, value);
WriteValue(key, buffer);
}

Expand Down
2 changes: 2 additions & 0 deletions src/coreclr/debug/createdump/createdump.h
Original file line number Diff line number Diff line change
Expand Up @@ -53,6 +53,8 @@ typedef int T_CONTEXT;
#include <arrayholder.h>
#include <releaseholder.h>
#ifdef HOST_UNIX
#include <minipal/utf8.h>
#include <dn-u16.h>
#include <dumpcommon.h>
#include <clrconfignocache.h>
#include <unistd.h>
Expand Down
10 changes: 7 additions & 3 deletions src/coreclr/debug/createdump/createdumpmain.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,8 @@ const char* g_help = "createdump [options]\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"
"--singlefile - single-file app model.\n"
"--nativeaot - native AOT app model.\n"
#endif
;

Expand Down Expand Up @@ -126,6 +127,10 @@ int createdump_main(const int argc, const char* argv[])
{
options.AppModel = AppModelType::SingleFile;
}
else if (strcmp(*argv, "--nativeaot") == 0)
{
options.AppModel = AppModelType::NativeAOT;
}
else if (strcmp(*argv, "--code") == 0)
{
options.SignalCode = atoi(*++argv);
Expand Down Expand Up @@ -199,9 +204,8 @@ int createdump_main(const int argc, const char* argv[])
{
if (::GetTempPathA(MAX_LONGPATH, tmpPath) == 0)
{
//printf_error("GetTempPath failed %s", GetLastErrorString().c_str());
printf_error("GetTempPath failed\n");
return ::GetLastError();
return -1;
}
exitCode = strcat_s(tmpPath, MAX_LONGPATH, DEFAULT_DUMP_TEMPLATE);
if (exitCode != 0)
Expand Down

0 comments on commit fb613a5

Please sign in to comment.