Skip to content

Debugger hook/breakpoint for issue reporting from the Swift runtime #10293

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 5 commits into from
Jun 19, 2017
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
42 changes: 42 additions & 0 deletions include/swift/Runtime/Debug.h
Original file line number Diff line number Diff line change
Expand Up @@ -134,6 +134,48 @@ void dumpStackTraceEntry(unsigned index, void *framePC,
LLVM_ATTRIBUTE_NOINLINE
void printCurrentBacktrace(unsigned framesToSkip = 1);

/// Debugger breakpoint ABI. This structure is passed to the debugger (and needs
/// to be stable) and describes extra information about a fatal error or a
/// non-fatal warning, which should be logged as a runtime issue. Please keep
/// all integer values pointer-sized.
struct RuntimeErrorDetails {
// ABI version, needs to be "1" currently.
uintptr_t version;

// A short hyphenated string describing the type of the issue, e.g.
// "precondition-failed" or "exclusivity-violation".
const char *errorType;

// Description of the current thread's stack position.
const char *currentStackDescription;
Copy link
Member

Choose a reason for hiding this comment

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

# of frames to skip?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

That's a good suggestion, I'll add it. It may get tricky to know the right value, though, because we need to know what is inlined and what is not. But it seems to be controllable enough in the two current use cases (implicit objc entrypoints and exclusivity violations).


// Number of frames in the current stack that should be ignored when reporting
// the issue (exluding the reportToDebugger/_swift_runtime_on_report frame).
// The remaining top frame should point to user's code where the bug is.
uintptr_t framesToSkip;

// Address of some associated object (if there's any).
void *memoryAddress;

// A structure describing an extra thread (and its stack) that is related.
struct Thread {
const char *description;
uint64_t threadID;
uintptr_t numFrames;
void **frames;
};
Copy link
Member

Choose a reason for hiding this comment

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

Do you want to describe the # of frames to skip here as well?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I don't think it makes sense here: Since the runtime needs to provide the frames itself, it should already strip away any uninteresting frames.

Copy link
Member

Choose a reason for hiding this comment

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

Good point!


// Number of extra threads (excluding the current thread) that are related,
// and the pointer to the array of extra threads.
uintptr_t numExtraThreads;
Thread *threads;
};

/// Debugger hook. Calling this stops the debugger with a message and details
/// about the issues.
void reportToDebugger(bool isFatal, const char *message,
RuntimeErrorDetails *details = nullptr);

// namespace swift
}

Expand Down
14 changes: 14 additions & 0 deletions stdlib/public/runtime/Errors.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -243,6 +243,20 @@ reportNow(uint32_t flags, const char *message)
#endif
}

LLVM_ATTRIBUTE_NOINLINE SWIFT_RUNTIME_EXPORT
void _swift_runtime_on_report(bool isFatal, const char *message,
RuntimeErrorDetails *details) {
// Do nothing. This function is meant to be used by the debugger.

// The following is necessary to avoid calls from being optimized out.
asm volatile("" ::: "memory");
}

void swift::reportToDebugger(bool isFatal, const char *message,
RuntimeErrorDetails *details) {
_swift_runtime_on_report(isFatal, message, details);
}

/// Report a fatal error to system console, stderr, and crash logs.
/// Does not crash by itself.
void swift::swift_reportError(uint32_t flags,
Expand Down
70 changes: 46 additions & 24 deletions stdlib/public/runtime/Exclusivity.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -64,23 +64,6 @@ static const char *getAccessName(ExclusivityFlags flags) {
}
}

LLVM_ATTRIBUTE_ALWAYS_INLINE
static void printConflictDetails(const char *oldAccessName, void *oldPC,
const char *newAccessName, void *newPC) {
fprintf(stderr, "Previous access (a %s) started at ", oldAccessName);
if (oldPC) {
dumpStackTraceEntry(0, oldPC, /*shortOutput=*/true);
fprintf(stderr, " (0x%lx).\n", (uintptr_t)oldPC);
} else {
fprintf(stderr, "<unknown>.\n");
}

fprintf(stderr, "Current access (a %s) started at:\n", newAccessName);
// The top frame is in swift_beginAccess, don't print it.
constexpr unsigned framesToSkip = 2;
printCurrentBacktrace(framesToSkip);
}

LLVM_ATTRIBUTE_ALWAYS_INLINE
static void reportExclusivityConflict(ExclusivityFlags oldAction, void *oldPC,
ExclusivityFlags newFlags, void *newPC,
Expand All @@ -94,14 +77,53 @@ static void reportExclusivityConflict(ExclusivityFlags oldAction, void *oldPC,
return;
}

fprintf(stderr,
"Simultaneous accesses to 0x%lx, but modification requires exclusive "
"access.\n",
(uintptr_t)pointer);
printConflictDetails(getAccessName(oldAction), oldPC,
getAccessName(getAccessAction(newFlags)), newPC);
constexpr unsigned maxMessageLength = 100;
constexpr unsigned maxAccessDescriptionLength = 50;
char message[maxMessageLength];
snprintf(message, sizeof(message),
"Simultaneous accesses to 0x%lx, but modification requires "
"exclusive access",
(uintptr_t)pointer);
fprintf(stderr, "%s.\n", message);

char oldAccess[maxAccessDescriptionLength];
snprintf(oldAccess, sizeof(oldAccess),
"Previous access (a %s) started at", getAccessName(oldAction));
fprintf(stderr, "%s ", oldAccess);
if (oldPC) {
dumpStackTraceEntry(0, oldPC, /*shortOutput=*/true);
fprintf(stderr, " (0x%lx).\n", (uintptr_t)oldPC);
Copy link
Member

Choose a reason for hiding this comment

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

Shouldn't this line-ending fprintf come before the dumpStackTraceEntry call?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I'm not trying to change the text output with this patch, so I think this is okay. Notice the /*shortOutput=*/true, so the whole line looks like:

Previous access (a modification) started at module`your_function+0xaa (0x1000a0cd).

Copy link
Member

Choose a reason for hiding this comment

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

Ah, ok.

} else {
fprintf(stderr, "<unknown>.\n");
}

char newAccess[maxAccessDescriptionLength];
snprintf(newAccess, sizeof(newAccess), "Current access (a %s) started at",
getAccessName(getAccessAction(newFlags)));
fprintf(stderr, "%s:\n", newAccess);
// The top frame is in swift_beginAccess, don't print it.
constexpr unsigned framesToSkip = 1;
printCurrentBacktrace(framesToSkip);

bool keepGoing = isWarningOnly(newFlags);

RuntimeErrorDetails::Thread secondaryThread = {
.description = oldAccess,
.numFrames = 1,
.frames = &oldPC
};
RuntimeErrorDetails details = {
.version = 1,
.errorType = "exclusivity-violation",
.currentStackDescription = newAccess,
.framesToSkip = framesToSkip,
.memoryAddress = pointer,
.numExtraThreads = 1,
.threads = &secondaryThread
};
reportToDebugger(!keepGoing, message, &details);

if (isWarningOnly(newFlags)) {
if (keepGoing) {
return;
}

Expand Down
36 changes: 25 additions & 11 deletions stdlib/public/runtime/SwiftObject.mm
Original file line number Diff line number Diff line change
Expand Up @@ -1425,17 +1425,31 @@ void swift_objc_swift3ImplicitObjCEntrypoint(id self, SEL selector,

if (filenameLength > INT_MAX)
filenameLength = INT_MAX;

reporter(
flags,
"*** %*s:%zu:%zu: implicit Objective-C entrypoint %c[%s %s] "
"is deprecated and will be removed in Swift 4; "
"add explicit '@objc' to the declaration to emit the Objective-C "
"entrypoint in Swift 4 and suppress this message\n",
(int)filenameLength, filename, line, column,
isInstanceMethod ? '-' : '+',
class_getName([self class]),
sel_getName(selector));

char *message, *nullTerminatedFilename;
asprintf(&message,
"implicit Objective-C entrypoint %c[%s %s] is deprecated and will "
"be removed in Swift 4",
isInstanceMethod ? '-' : '+',
class_getName([self class]),
sel_getName(selector));
asprintf(&nullTerminatedFilename, "%*s", (int)filenameLength, filename);

RuntimeErrorDetails details = {
.version = 1,
.errorType = "implicit-objc-entrypoint",
.framesToSkip = 1
};
bool isFatal = reporter == swift::fatalError;
reportToDebugger(isFatal, message, &details);

reporter(flags,
"*** %s:%zu:%zu: %s; add explicit '@objc' to the declaration to "
"emit the Objective-C entrypoint in Swift 4 and suppress this "
"message\n",
nullTerminatedFilename, line, column, message);
free(message);
free(nullTerminatedFilename);
}

#endif
Expand Down