Skip to content

Commit fb613a5

Browse files
author
Mike McLaughlin
authored
Remove createdump's DAC dependency for the PAL for NativeAOT (#88802)
* 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
1 parent 4f933e1 commit fb613a5

14 files changed

+401
-71
lines changed

src/coreclr/debug/createdump/CMakeLists.txt

Lines changed: 10 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -37,6 +37,12 @@ if(CLR_CMAKE_HOST_WIN32)
3737

3838
else(CLR_CMAKE_HOST_WIN32)
3939

40+
if(NOT DEFINED ENV{ROOTFS_DIR})
41+
include_directories(SYSTEM /usr/local/include)
42+
elseif (CLR_CMAKE_TARGET_FREEBSD)
43+
include_directories(SYSTEM $ENV{ROOTFS_DIR}/usr/local/include)
44+
endif()
45+
4046
include(configure.cmake)
4147

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

6976
if(CLR_CMAKE_HOST_OSX)
@@ -74,9 +81,6 @@ if(CLR_CMAKE_HOST_OSX)
7481
dumpwritermacho.cpp
7582
${CREATEDUMP_SOURCES}
7683
)
77-
add_executable_clr(createdump
78-
main.cpp
79-
)
8084
else()
8185
add_library_clr(createdump_static
8286
STATIC
@@ -85,25 +89,21 @@ else()
8589
dumpwriterelf.cpp
8690
${CREATEDUMP_SOURCES}
8791
)
92+
endif(CLR_CMAKE_HOST_OSX)
93+
8894
add_executable_clr(createdump
8995
main.cpp
90-
${PAL_REDEFINES_FILE}
96+
createdumppal.cpp
9197
)
92-
add_dependencies(createdump pal_redefines_file)
93-
endif(CLR_CMAKE_HOST_OSX)
9498

9599
target_link_libraries(createdump
96100
PRIVATE
97101
createdump_static
98102
corguids
99103
dbgutil
100-
# share the PAL in the dac module
101-
mscordaccore
102104
dl
103105
)
104106

105-
add_dependencies(createdump mscordaccore)
106-
107107
endif(CLR_CMAKE_HOST_WIN32)
108108

109109
install_clr(TARGETS createdump DESTINATIONS . sharedFramework COMPONENT runtime)

src/coreclr/debug/createdump/config.h.in

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4,3 +4,5 @@
44
#pragma once
55

66
#cmakedefine HAVE_PROCESS_VM_READV
7+
#cmakedefine01 HAVE_CLOCK_GETTIME_NSEC_NP
8+
#cmakedefine01 HAVE_CLOCK_MONOTONIC
Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,22 @@
11
check_function_exists(process_vm_readv HAVE_PROCESS_VM_READV)
22

3+
check_symbol_exists(
4+
clock_gettime_nsec_np
5+
time.h
6+
HAVE_CLOCK_GETTIME_NSEC_NP)
7+
8+
check_cxx_source_runs("
9+
#include <stdlib.h>
10+
#include <time.h>
11+
#include <sys/time.h>
12+
13+
int main()
14+
{
15+
int ret;
16+
struct timespec ts;
17+
ret = clock_gettime(CLOCK_MONOTONIC, &ts);
18+
19+
exit(ret);
20+
}" HAVE_CLOCK_MONOTONIC)
21+
322
configure_file(${CMAKE_CURRENT_SOURCE_DIR}/config.h.in ${CMAKE_CURRENT_BINARY_DIR}/config.h)

src/coreclr/debug/createdump/crashinfo.cpp

Lines changed: 28 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -280,10 +280,11 @@ GetHResultString(HRESULT hr)
280280
bool
281281
CrashInfo::InitializeDAC(DumpType dumpType)
282282
{
283-
// Don't attempt to load the DAC if the app model doesn't support it by default. The default for single-file is a
284-
// 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
285-
// application the core dump will be generated.
286-
if (dumpType == DumpType::Full && (m_appModel == AppModelType::SingleFile || m_appModel == AppModelType::NativeAOT))
283+
// Don't attempt to load the DAC if the app model doesn't support it by default. The default for single-file is
284+
// a full dump, but if the dump type requested is a mini, triage or heap and the DAC is next to the single-file
285+
// application the core dump will be generated. For NativeAOT, there is currently no DAC available so never
286+
// attempt to load it.
287+
if ((dumpType == DumpType::Full && m_appModel == AppModelType::SingleFile) || m_appModel == AppModelType::NativeAOT)
287288
{
288289
return true;
289290
}
@@ -483,19 +484,24 @@ CrashInfo::EnumerateManagedModules()
483484
bool
484485
CrashInfo::UnwindAllThreads()
485486
{
486-
TRACE("UnwindAllThreads: STARTED (%d)\n", m_dataTargetPagesAdded);
487-
ReleaseHolder<ISOSDacInterface> pSos = nullptr;
488-
if (m_pClrDataProcess != nullptr) {
489-
m_pClrDataProcess->QueryInterface(__uuidof(ISOSDacInterface), (void**)&pSos);
490-
}
491-
// For each native and managed thread
492-
for (ThreadInfo* thread : m_threads)
487+
// Don't unwind any threads if Native AOT since there isn't a DAC to get the remote
488+
// unwinder support and they are full dumps.
489+
if (m_appModel != AppModelType::NativeAOT)
493490
{
494-
if (!thread->UnwindThread(m_pClrDataProcess, pSos)) {
495-
return false;
491+
TRACE("UnwindAllThreads: STARTED (%d)\n", m_dataTargetPagesAdded);
492+
ReleaseHolder<ISOSDacInterface> pSos = nullptr;
493+
if (m_pClrDataProcess != nullptr) {
494+
m_pClrDataProcess->QueryInterface(__uuidof(ISOSDacInterface), (void**)&pSos);
495+
}
496+
// For each native and managed thread
497+
for (ThreadInfo* thread : m_threads)
498+
{
499+
if (!thread->UnwindThread(m_pClrDataProcess, pSos)) {
500+
return false;
501+
}
496502
}
503+
TRACE("UnwindAllThreads: FINISHED (%d)\n", m_dataTargetPagesAdded);
497504
}
498-
TRACE("UnwindAllThreads: FINISHED (%d)\n", m_dataTargetPagesAdded);
499505
return true;
500506
}
501507

@@ -959,9 +965,9 @@ FormatString(const char* format, ...)
959965
ArrayHolder<char> buffer = new char[MAX_LONGPATH + 1];
960966
va_list args;
961967
va_start(args, format);
962-
int result = vsprintf_s(buffer, MAX_LONGPATH, format, args);
968+
int result = vsnprintf(buffer, MAX_LONGPATH, format, args);
963969
va_end(args);
964-
return result > 0 ? std::string(buffer) : std::string();
970+
return result > 0 && result < MAX_LONGPATH ? std::string(buffer) : std::string();
965971
}
966972

967973
//
@@ -971,15 +977,16 @@ std::string
971977
ConvertString(const WCHAR* str)
972978
{
973979
if (str == nullptr)
974-
return{};
980+
return { };
975981

976-
int len = WideCharToMultiByte(CP_UTF8, 0, str, -1, nullptr, 0, nullptr, nullptr);
982+
size_t cch = u16_strlen(str) + 1;
983+
int len = minipal_get_length_utf16_to_utf8((CHAR16_T*)str, cch, 0);
977984
if (len == 0)
978-
return{};
985+
return { };
979986

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

985992
//

src/coreclr/debug/createdump/crashinfomac.cpp

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -288,11 +288,11 @@ void CrashInfo::VisitSegment(MachOModule& module, const segment_command_64& segm
288288

289289
// Round to page boundary
290290
start = start & PAGE_MASK;
291-
_ASSERTE(start > 0);
291+
assert(start > 0);
292292

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

297297
// Add module memory region if not already on the list
298298
MemoryRegion newModule(regionFlags, start, end, offset, module.Name());

src/coreclr/debug/createdump/crashinfounix.cpp

Lines changed: 49 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -17,7 +17,12 @@ bool
1717
CrashInfo::Initialize()
1818
{
1919
char memPath[128];
20-
_snprintf_s(memPath, sizeof(memPath), sizeof(memPath), "/proc/%u/mem", m_pid);
20+
int chars = snprintf(memPath, sizeof(memPath), "/proc/%u/mem", m_pid);
21+
if (chars <= 0 || (size_t)chars >= sizeof(memPath))
22+
{
23+
printf_error("snprintf failed building /proc/<pid>/mem name\n");
24+
return false;
25+
}
2126

2227
m_fdMem = open(memPath, O_RDONLY);
2328
if (m_fdMem == -1)
@@ -42,7 +47,12 @@ CrashInfo::Initialize()
4247
{
4348
TRACE("DbgDisablePagemapUse detected - pagemap file checking is enabled\n");
4449
char pagemapPath[128];
45-
_snprintf_s(pagemapPath, sizeof(pagemapPath), sizeof(pagemapPath), "/proc/%u/pagemap", m_pid);
50+
chars = snprintf(pagemapPath, sizeof(pagemapPath), "/proc/%u/pagemap", m_pid);
51+
if (chars <= 0 || (size_t)chars >= sizeof(pagemapPath))
52+
{
53+
printf_error("snprintf failed building /proc/<pid>/pagemap name\n");
54+
return false;
55+
}
4656
m_fdPagemap = open(pagemapPath, O_RDONLY);
4757
if (m_fdPagemap == -1)
4858
{
@@ -95,7 +105,12 @@ bool
95105
CrashInfo::EnumerateAndSuspendThreads()
96106
{
97107
char taskPath[128];
98-
_snprintf_s(taskPath, sizeof(taskPath), sizeof(taskPath), "/proc/%u/task", m_pid);
108+
int chars = snprintf(taskPath, sizeof(taskPath), "/proc/%u/task", m_pid);
109+
if (chars <= 0 || (size_t)chars >= sizeof(taskPath))
110+
{
111+
printf_error("snprintf failed building /proc/<pid>/task\n");
112+
return false;
113+
}
99114

100115
DIR* taskDir = opendir(taskPath);
101116
if (taskDir == nullptr)
@@ -139,8 +154,12 @@ bool
139154
CrashInfo::GetAuxvEntries()
140155
{
141156
char auxvPath[128];
142-
_snprintf_s(auxvPath, sizeof(auxvPath), sizeof(auxvPath), "/proc/%u/auxv", m_pid);
143-
157+
int chars = snprintf(auxvPath, sizeof(auxvPath), "/proc/%u/auxv", m_pid);
158+
if (chars <= 0 || (size_t)chars >= sizeof(auxvPath))
159+
{
160+
printf_error("snprintf failed building /proc/<pid>/auxv\n");
161+
return false;
162+
}
144163
int fd = open(auxvPath, O_RDONLY, 0);
145164
if (fd == -1)
146165
{
@@ -195,9 +214,12 @@ CrashInfo::EnumerateMemoryRegions()
195214

196215
// Making something like: /proc/123/maps
197216
char mapPath[128];
198-
int chars = _snprintf_s(mapPath, sizeof(mapPath), sizeof(mapPath), "/proc/%u/maps", m_pid);
199-
assert(chars > 0 && (size_t)chars <= sizeof(mapPath));
200-
217+
int chars = snprintf(mapPath, sizeof(mapPath), "/proc/%u/maps", m_pid);
218+
if (chars <= 0 || (size_t)chars >= sizeof(mapPath))
219+
{
220+
printf_error("snprintf failed building /proc/<pid>/maps\n");
221+
return false;
222+
}
201223
FILE* mapsFile = fopen(mapPath, "r");
202224
if (mapsFile == nullptr)
203225
{
@@ -401,19 +423,22 @@ CrashInfo::VisitProgramHeader(uint64_t loadbias, uint64_t baseAddress, Phdr* phd
401423
TRACE("VisitProgramHeader: ehFrameHdrStart %016llx ehFrameHdrSize %08llx\n", ehFrameHdrStart, ehFrameHdrSize);
402424
InsertMemoryRegion(ehFrameHdrStart, ehFrameHdrSize);
403425

404-
ULONG64 ehFrameStart;
405-
ULONG64 ehFrameSize;
406-
if (PAL_GetUnwindInfoSize(baseAddress, ehFrameHdrStart, ReadMemoryAdapter, &ehFrameStart, &ehFrameSize))
426+
if (m_appModel != AppModelType::NativeAOT)
407427
{
408-
TRACE("VisitProgramHeader: ehFrameStart %016llx ehFrameSize %08llx\n", ehFrameStart, ehFrameSize);
409-
if (ehFrameStart != 0 && ehFrameSize != 0)
428+
ULONG64 ehFrameStart;
429+
ULONG64 ehFrameSize;
430+
if (PAL_GetUnwindInfoSize(baseAddress, ehFrameHdrStart, ReadMemoryAdapter, &ehFrameStart, &ehFrameSize))
410431
{
411-
InsertMemoryRegion(ehFrameStart, ehFrameSize);
432+
TRACE("VisitProgramHeader: ehFrameStart %016llx ehFrameSize %08llx\n", ehFrameStart, ehFrameSize);
433+
if (ehFrameStart != 0 && ehFrameSize != 0)
434+
{
435+
InsertMemoryRegion(ehFrameStart, ehFrameSize);
436+
}
437+
}
438+
else
439+
{
440+
TRACE("VisitProgramHeader: PAL_GetUnwindInfoSize FAILED\n");
412441
}
413-
}
414-
else
415-
{
416-
TRACE("VisitProgramHeader: PAL_GetUnwindInfoSize FAILED\n");
417442
}
418443
}
419444
break;
@@ -489,7 +514,12 @@ bool
489514
GetStatus(pid_t pid, pid_t* ppid, pid_t* tgid, std::string* name)
490515
{
491516
char statusPath[128];
492-
_snprintf_s(statusPath, sizeof(statusPath), sizeof(statusPath), "/proc/%d/status", pid);
517+
int chars = snprintf(statusPath, sizeof(statusPath), "/proc/%d/status", pid);
518+
if (chars <= 0 || (size_t)chars >= sizeof(statusPath))
519+
{
520+
printf_error("snprintf failed building /proc/<pid>/status\n");
521+
return false;
522+
}
493523

494524
FILE *statusFile = fopen(statusPath, "r");
495525
if (statusFile == nullptr)

src/coreclr/debug/createdump/crashreportwriter.cpp

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -376,15 +376,15 @@ void
376376
CrashReportWriter::WriteValue32(const char* key, uint32_t value)
377377
{
378378
char buffer[16];
379-
_snprintf_s(buffer, sizeof(buffer), sizeof(buffer), "0x%x", value);
379+
snprintf(buffer, sizeof(buffer), "0x%x", value);
380380
WriteValue(key, buffer);
381381
}
382382

383383
void
384384
CrashReportWriter::WriteValue64(const char* key, uint64_t value)
385385
{
386386
char buffer[32];
387-
_snprintf_s(buffer, sizeof(buffer), sizeof(buffer), "0x%" PRIx64, value);
387+
snprintf(buffer, sizeof(buffer), "0x%" PRIx64, value);
388388
WriteValue(key, buffer);
389389
}
390390

src/coreclr/debug/createdump/createdump.h

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -53,6 +53,8 @@ typedef int T_CONTEXT;
5353
#include <arrayholder.h>
5454
#include <releaseholder.h>
5555
#ifdef HOST_UNIX
56+
#include <minipal/utf8.h>
57+
#include <dn-u16.h>
5658
#include <dumpcommon.h>
5759
#include <clrconfignocache.h>
5860
#include <unistd.h>

src/coreclr/debug/createdump/createdumpmain.cpp

Lines changed: 7 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -33,7 +33,8 @@ const char* g_help = "createdump [options]\n"
3333
"--crashreportonly - write crash report file only (no dump).\n"
3434
"--crashthread <id> - the thread id of the crashing thread.\n"
3535
"--signal <code> - the signal code of the crash.\n"
36-
"--singlefile - enable single-file app check.\n"
36+
"--singlefile - single-file app model.\n"
37+
"--nativeaot - native AOT app model.\n"
3738
#endif
3839
;
3940

@@ -126,6 +127,10 @@ int createdump_main(const int argc, const char* argv[])
126127
{
127128
options.AppModel = AppModelType::SingleFile;
128129
}
130+
else if (strcmp(*argv, "--nativeaot") == 0)
131+
{
132+
options.AppModel = AppModelType::NativeAOT;
133+
}
129134
else if (strcmp(*argv, "--code") == 0)
130135
{
131136
options.SignalCode = atoi(*++argv);
@@ -199,9 +204,8 @@ int createdump_main(const int argc, const char* argv[])
199204
{
200205
if (::GetTempPathA(MAX_LONGPATH, tmpPath) == 0)
201206
{
202-
//printf_error("GetTempPath failed %s", GetLastErrorString().c_str());
203207
printf_error("GetTempPath failed\n");
204-
return ::GetLastError();
208+
return -1;
205209
}
206210
exitCode = strcat_s(tmpPath, MAX_LONGPATH, DEFAULT_DUMP_TEMPLATE);
207211
if (exitCode != 0)

0 commit comments

Comments
 (0)