Skip to content

Commit 53722a3

Browse files
carlpetocarlpeto
authored
Merge pull request #85483 from carlpeto/fix-backtrace-stderr-color-bug
Backtrace incorrectly picks up colour from stdout instead of stderr
2 parents 9e216ed + 4d57a80 commit 53722a3

File tree

5 files changed

+304
-4
lines changed

5 files changed

+304
-4
lines changed

‎stdlib/public/runtime/Backtrace.cpp‎

Lines changed: 20 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -221,6 +221,16 @@ char swiftBacktraceOutputPath[SWIFT_BACKTRACE_OUTPUT_PATH_SIZE] __attribute__((s
221221

222222
void _swift_backtraceSetupEnvironment();
223223

224+
bool isStderrATty()
225+
{
226+
#ifndef _WIN32
227+
return isatty(STDERR_FILENO);
228+
#else
229+
DWORD dwMode;
230+
return GetConsoleMode(GetStdHandle(STD_ERROR_HANDLE), &dwMode);
231+
#endif
232+
}
233+
224234
bool isStdoutATty()
225235
{
226236
#ifndef _WIN32
@@ -412,10 +422,6 @@ BacktraceInitializer::BacktraceInitializer() {
412422
(isStdoutATty() && isStdinATty()) ? OnOffTty::On : OnOffTty::Off;
413423
}
414424

415-
if (_swift_backtraceSettings.color == OnOffTty::TTY)
416-
_swift_backtraceSettings.color =
417-
isStdoutATty() ? OnOffTty::On : OnOffTty::Off;
418-
419425
if (_swift_backtraceSettings.preset == Preset::Auto) {
420426
if (_swift_backtraceSettings.interactive == OnOffTty::On)
421427
_swift_backtraceSettings.preset = Preset::Friendly;
@@ -454,6 +460,16 @@ BacktraceInitializer::BacktraceInitializer() {
454460
_swift_backtraceSettings.outputTo = OutputTo::Stderr;
455461
}
456462

463+
if (_swift_backtraceSettings.color == OnOffTty::TTY) {
464+
bool outputToIsTty;
465+
if (_swift_backtraceSettings.outputTo == OutputTo::Stderr)
466+
outputToIsTty = isStderrATty();
467+
else
468+
outputToIsTty = isStdoutATty();
469+
_swift_backtraceSettings.color =
470+
outputToIsTty ? OnOffTty::On : OnOffTty::Off;
471+
}
472+
457473
if (_swift_backtraceSettings.enabled == OnOffTty::On) {
458474
// Copy the path to swift-backtrace into swiftBacktracePath, then write
459475
// protect it so that it can't be overwritten easily at runtime. We do

‎test/Backtracing/Inputs/CrashSubprocess.swift‎

Lines changed: 38 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,38 @@
1+
#if os(macOS)
2+
internal import Darwin
3+
#elseif canImport(Glibc)
4+
internal import Glibc
5+
#elseif canImport(Musl)
6+
internal import Musl
7+
#endif
8+
9+
func level1() {
10+
level2()
11+
}
12+
13+
func level2() {
14+
level3()
15+
}
16+
17+
func level3() {
18+
level4()
19+
}
20+
21+
func level4() {
22+
level5()
23+
}
24+
25+
func level5() {
26+
print("About to crash")
27+
let ptr = UnsafeMutablePointer<Int>(bitPattern: 4)!
28+
ptr.pointee = 42
29+
}
30+
31+
@main
32+
struct Crash {
33+
static func main() {
34+
print("subprocess stdout is a tty: \(isatty(1))")
35+
print("subprocess stderr is a tty: \(isatty(2))")
36+
level1()
37+
}
38+
}

‎test/Backtracing/TTYDetection.swift‎

Lines changed: 73 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,73 @@
1+
// RUN: %empty-directory(%t)
2+
// RUN: %target-build-swift %s -parse-as-library -Onone -g -o %t/TTYDetection
3+
// RUN: %target-codesign %t/TTYDetection
4+
5+
// RUN: (env SWIFT_BACKTRACE=enable=yes,cache=no %target-run %t/TTYDetection 2> %t/default-output || true) && cat %t/default-output | %FileCheck %s
6+
// RUN: (env SWIFT_BACKTRACE=enable=yes,cache=no,output-to=stderr %target-run %t/TTYDetection 2> %t/stderr-output || true) && cat %t/stderr-output | %FileCheck %s --check-prefix STDERR
7+
// RUN: (env SWIFT_BACKTRACE=enable=yes,cache=no,output-to=stdout %target-run %t/TTYDetection > %t/stdout-output || true) && cat %t/stdout-output | %FileCheck %s --check-prefix STDOUT
8+
9+
10+
// UNSUPPORTED: use_os_stdlib
11+
// UNSUPPORTED: back_deployment_runtime
12+
// UNSUPPORTED: asan
13+
// REQUIRES: executable_test
14+
// REQUIRES: backtracing
15+
// REQUIRES: OS=macosx || OS=linux-gnu
16+
// COM: we should be able to add Windows to this test
17+
18+
func level1() {
19+
level2()
20+
}
21+
22+
func level2() {
23+
level3()
24+
}
25+
26+
func level3() {
27+
level4()
28+
}
29+
30+
func level4() {
31+
level5()
32+
}
33+
34+
func level5() {
35+
print("About to crash")
36+
let ptr = UnsafeMutablePointer<Int>(bitPattern: 4)!
37+
ptr.pointee = 42
38+
}
39+
40+
@main
41+
struct TTYDetection {
42+
static func main() {
43+
level1()
44+
}
45+
}
46+
47+
// CHECK: *** Program crashed: Bad pointer dereference at 0x{{0+}}4 ***
48+
49+
// CHECK: Thread 0 {{(".*" )?}}crashed:
50+
51+
// CHECK: 0 0x{{[0-9a-f]+}} level5() + {{[0-9]+}} in TTYDetection
52+
// CHECK-NEXT: 1 [ra] 0x{{[0-9a-f]+}} level4() + {{[0-9]+}} in TTYDetection
53+
// CHECK-NEXT: 2 [ra] 0x{{[0-9a-f]+}} level3() + {{[0-9]+}} in TTYDetection
54+
// CHECK-NEXT: 3 [ra] 0x{{[0-9a-f]+}} level2() + {{[0-9]+}} in TTYDetection
55+
56+
57+
// STDERR: *** Program crashed: Bad pointer dereference at 0x{{0+}}4 ***
58+
59+
// STDERR: Thread 0 {{(".*" )?}}crashed:
60+
61+
// STDERR: 0 0x{{[0-9a-f]+}} level5() + {{[0-9]+}} in TTYDetection
62+
// STDERR-NEXT: 1 [ra] 0x{{[0-9a-f]+}} level4() + {{[0-9]+}} in TTYDetection
63+
// STDERR-NEXT: 2 [ra] 0x{{[0-9a-f]+}} level3() + {{[0-9]+}} in TTYDetection
64+
// STDERR-NEXT: 3 [ra] 0x{{[0-9a-f]+}} level2() + {{[0-9]+}} in TTYDetection
65+
66+
// STDOUT: *** Program crashed: Bad pointer dereference at 0x{{0+}}4 ***
67+
68+
// STDOUT: Thread 0 {{(".*" )?}}crashed:
69+
70+
// STDOUT: 0 0x{{[0-9a-f]+}} level5() + {{[0-9]+}} in TTYDetection
71+
// STDOUT-NEXT: 1 [ra] 0x{{[0-9a-f]+}} level4() + {{[0-9]+}} in TTYDetection
72+
// STDOUT-NEXT: 2 [ra] 0x{{[0-9a-f]+}} level3() + {{[0-9]+}} in TTYDetection
73+
// STDOUT-NEXT: 3 [ra] 0x{{[0-9a-f]+}} level2() + {{[0-9]+}} in TTYDetection

‎test/Backtracing/TTYDetectionColourLinux.swift‎

Lines changed: 127 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,127 @@
1+
// RUN: %empty-directory(%t)
2+
// RUN: %target-build-swift %S/Inputs/CrashSubprocess.swift -parse-as-library -Onone -g -o %t/crash
3+
// RUN: %target-codesign %t/crash
4+
5+
// RUN: %target-build-swift %s -o %t/crash-host
6+
7+
// RUN: (env SWIFT_BACKTRACE=enable=yes,cache=no,interactive=no %target-run %t/crash-host %t/crash)| %FileCheck %s
8+
9+
// UNSUPPORTED: use_os_stdlib
10+
// UNSUPPORTED: back_deployment_runtime
11+
// UNSUPPORTED: asan
12+
13+
// REQUIRES: executable_test
14+
// REQUIRES: backtracing
15+
// REQUIRES: OS=linux-gnu || OS=macosx
16+
17+
// COM: this test would need significant rework on Windows/WinSDK, but is not critical function so probably not worth porting the test to that platform
18+
19+
#if os(macOS)
20+
internal import Darwin
21+
#elseif canImport(Glibc)
22+
internal import Glibc
23+
#elseif canImport(Musl)
24+
internal import Musl
25+
#endif
26+
27+
guard CommandLine.arguments.count > 1 else {
28+
fputs("Usage: \(CommandLine.arguments[0]) <path-to-crash>\n", stderr)
29+
exit(1)
30+
}
31+
32+
let crashPath = CommandLine.arguments[1]
33+
34+
// CHECK: host stdout is a tty: 0
35+
print("host stdout is a tty: \(isatty(1))")
36+
// (piped output on linux is fully buffered by default)
37+
fflush(stdout)
38+
39+
// CHECK: host stderr is a tty: 0
40+
print("host stderr is a tty: \(isatty(2))")
41+
fflush(stdout)
42+
43+
var masterFD: Int32 = 0
44+
var slaveFD: Int32 = 0
45+
46+
let slaveName = UnsafeMutablePointer<CChar>.allocate(capacity: 1024)
47+
48+
guard openpty(&masterFD, &slaveFD, slaveName, nil, nil) == 0 else {
49+
perror("openpty")
50+
exit(1)
51+
}
52+
53+
// CHECK: slave is: /dev/{{.+}}
54+
print("slave is: \(String(cString: slaveName))")
55+
slaveName.deallocate()
56+
fflush(stdout)
57+
58+
// try to avoid using Foundation, use Posix primitives instead
59+
var childPid: pid_t = 0
60+
61+
#if os(macOS)
62+
var childFileActions: posix_spawn_file_actions_t?
63+
posix_spawn_file_actions_init(&childFileActions)
64+
posix_spawn_file_actions_adddup2(&childFileActions, slaveFD, STDOUT_FILENO)
65+
posix_spawn_file_actions_adddup2(&childFileActions, slaveFD, STDERR_FILENO)
66+
67+
guard posix_spawn(&childPid, Array<CChar>(crashPath.utf8CString), &childFileActions, nil, nil, environ) == 0 else {
68+
perror("unable to spawn child")
69+
exit(1)
70+
}
71+
72+
posix_spawn_file_actions_destroy(&childFileActions)
73+
74+
#elseif os(Linux)
75+
var childFileActions = posix_spawn_file_actions_t()
76+
posix_spawn_file_actions_init(&childFileActions)
77+
posix_spawn_file_actions_adddup2(&childFileActions, slaveFD, STDOUT_FILENO)
78+
posix_spawn_file_actions_adddup2(&childFileActions, slaveFD, STDERR_FILENO)
79+
80+
var argv: [UnsafeMutablePointer<CChar>?] = [nil]
81+
82+
guard posix_spawn(&childPid, Array<CChar>(crashPath.utf8CString), &childFileActions, nil, argv, environ) == 0 else {
83+
perror("unable to spawn child")
84+
exit(1)
85+
}
86+
87+
posix_spawn_file_actions_destroy(&childFileActions)
88+
89+
#endif
90+
91+
// CHECK: child pid is {{[0-9]+}}
92+
print("child pid is \(childPid)")
93+
fflush(stdout)
94+
95+
// the parent process should now close this FD as it is used by the child for stdout/stderr
96+
close(slaveFD)
97+
98+
// CHECK: subprocess stdout is a tty: 1
99+
// CHECK: subprocess stderr is a tty: 1
100+
101+
// CHECK: About to crash
102+
103+
// CHECK: 💣{{.*}}Program crashed: Bad pointer dereference at 0x{{0+}}4
104+
// CHECK: Thread 0 {{(".*" )?}}crashed:
105+
// CHECK: Backtrace took {{[0-9.]+}}s
106+
107+
// allow the child process time to start, and crash...
108+
sleep(1)
109+
110+
// now read the standard output and error from our master pty
111+
// and write it to our own standard output for scanning by FileCheck
112+
let bufferSize = 4096
113+
let buffer = UnsafeMutablePointer<UInt8>.allocate(capacity: bufferSize)
114+
var bytesRead = read(masterFD, buffer, bufferSize)
115+
116+
while bytesRead > 0 {
117+
write(1, buffer, bytesRead)
118+
bytesRead = read(masterFD, buffer, bufferSize)
119+
}
120+
121+
close(masterFD)
122+
buffer.deallocate()
123+
124+
// CHECK-NOT: child exited with status: 0
125+
var childExitStatus: Int32 = 0
126+
waitpid(childPid, &childExitStatus, 0)
127+
print("child exited with status: \(childExitStatus)")

‎test/Backtracing/TTYDetectionColourMacOS.swift‎

Lines changed: 46 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,46 @@
1+
// RUN: %empty-directory(%t)
2+
// RUN: %target-build-swift %s -parse-as-library -Onone -g -o %t/TTYDetection
3+
// RUN: %target-codesign %t/TTYDetection
4+
5+
// RUN: (env SWIFT_BACKTRACE=enable=yes,cache=no,interactive=no %target-run script -reF %t/script-output %t/TTYDetection || true) && cat %t/script-output | %FileCheck %s
6+
7+
8+
// UNSUPPORTED: use_os_stdlib
9+
// UNSUPPORTED: back_deployment_runtime
10+
// UNSUPPORTED: asan
11+
// REQUIRES: executable_test
12+
// REQUIRES: backtracing
13+
// REQUIRES: OS=macosx
14+
// COM: this won't work on Windows, but is not critical function
15+
16+
func level1() {
17+
level2()
18+
}
19+
20+
func level2() {
21+
level3()
22+
}
23+
24+
func level3() {
25+
level4()
26+
}
27+
28+
func level4() {
29+
level5()
30+
}
31+
32+
func level5() {
33+
print("About to crash")
34+
let ptr = UnsafeMutablePointer<Int>(bitPattern: 4)!
35+
ptr.pointee = 42
36+
}
37+
38+
@main
39+
struct TTYDetection {
40+
static func main() {
41+
level1()
42+
}
43+
}
44+
45+
// CHECK: 💣{{.*}}Program crashed: Bad pointer dereference at 0x{{0+}}4
46+
// CHECK: Thread 0 {{(".*" )?}}crashed:

0 commit comments

Comments
 (0)