swiftlang / swift

The Swift Programming Language
https://swift.org
Apache License 2.0
67.18k stars 10.32k forks source link

[SR-4955] SuperChars benchmark fails to run with illegal instruction:4 #47532

Closed 56ed3025-6cad-4bf8-b972-9c308205a0af closed 7 years ago

56ed3025-6cad-4bf8-b972-9c308205a0af commented 7 years ago
Previous ID SR-4955
Radar rdar://problem/32316519
Original Reporter @palimondo
Type Bug
Status Closed
Resolution Done
Environment Swift version 4.0-dev (LLVM e23c4f1c07, Clang b4252460af, Swift 8fbd6ace17) Target: x86_64-apple-macosx10.9
Additional Detail from JIRA | | | |------------------|-----------------| |Votes | 0 | |Component/s | Compiler | |Labels | Bug | |Assignee | @dabrahams | |Priority | Medium | md5: 126f996d3a2623f6d0e405c686efec89

Issue Description:

Since about 18 of May 2017, the SuperChars benchmark fails to run:

$ ./Benchmark_O SuperChars
#,TEST,SAMPLES,MIN(us),MAX(us),MEAN(us),SD(us),MEDIAN(us)
fatal error: file /Users/mondo/Developer/swift-source/swift/stdlib/public/core/Character.swift, line 206
Current stack trace:
0    libswiftCore.dylib                 0x00000001032c3750 _swift_stdlib_reportFatalErrorInFile + 100
1    libswiftCore.dylib                 0x0000000102f66e60 closure #​1 in closure #​1 in closure #​1 in _assertionFailure(_:_:file:line:flags:) + 320
2    libswiftCore.dylib                 0x0000000102f66e30 closure #​1 in closure #​1 in closure #​1 in _fatalErrorMessage(_:_:file:line:flags:) + 35
3    libswiftCore.dylib                 0x0000000103256ce0 partial apply for closure #​1 in closure #​1 in closure #​1 in _fatalErrorMessage(_:_:file:line:flags:) + 97
4    libswiftCore.dylib                 0x0000000102f662e0 specialized StaticString.withUTF8Buffer<A>(_:) + 216
5    libswiftCore.dylib                 0x0000000103256c20 partial apply for closure #&#8203;1 in closure #&#8203;1 in _assertionFailure(_:_:file:line:flags:) + 146
6    libswiftCore.dylib                 0x0000000103256c00 partial apply for closure #&#8203;1 in closure #&#8203;1 in _fatalErrorMessage(_:_:file:line:flags:) + 23
7    libswiftCore.dylib                 0x0000000102f662e0 specialized StaticString.withUTF8Buffer<A>(_:) + 216
8    libswiftCore.dylib                 0x00000001031abf80 partial apply for closure #&#8203;1 in _fatalErrorMessage(_:_:file:line:flags:) + 159
9    libswiftCore.dylib                 0x00000001031abf20 partial apply for closure #&#8203;1 in _fatalErrorMessage(_:_:file:line:flags:) + 23
10   libswiftCore.dylib                 0x0000000102f662e0 specialized StaticString.withUTF8Buffer<A>(_:) + 216
11   libswiftCore.dylib                 0x0000000102f661e0 _fatalErrorMessage(_:_:file:line:flags:) + 126
12   libswiftCore.dylib                 0x00000001031312d0 specialized Character.init(_largeRepresentationString:) + 671
13   Benchmark_O                        0x0000000102e53480 run_SuperChars(_:) + 894
14   Benchmark_O                        0x0000000102e63aa0 specialized thunk for @callee_owned (@unowned Int) -> () + 12
15   Benchmark_O                        0x0000000102d1cad0 partial apply for thunk for @callee_owned (@in Int) -> (@out ()) + 56
16   Benchmark_O                        0x0000000102d187e0 specialized runBench(_:_:_:) + 1327
17   Benchmark_O                        0x0000000102d1a850 specialized runBenchmarks(_:) + 3001
18   Benchmark_O                        0x0000000102d0adc0 main() + 1604
19   Benchmark_O                        0x0000000102e5cca0 main + 23747
20   libdyld.dylib                      0x00007fffcff28234 start + 1
Illegal instruction: 4
56ed3025-6cad-4bf8-b972-9c308205a0af commented 7 years ago

Same issue in Onone build:

./Benchmark_Onone SuperChars
#,TEST,SAMPLES,MIN(us),MAX(us),MEAN(us),SD(us),MEDIAN(us)
fatal error: file /Users/mondo/Developer/swift-source/swift/stdlib/public/core/Character.swift, line 206
Current stack trace:
0    libswiftCore.dylib                 0x000000010c290750 _swift_stdlib_reportFatalErrorInFile + 100
1    libswiftCore.dylib                 0x000000010bf33e60 closure #&#8203;1 in closure #&#8203;1 in closure #&#8203;1 in _assertionFailure(_:_:file:line:flags:) + 320
2    libswiftCore.dylib                 0x000000010bf33e30 closure #&#8203;1 in closure #&#8203;1 in closure #&#8203;1 in _fatalErrorMessage(_:_:file:line:flags:) + 35
3    libswiftCore.dylib                 0x000000010c223ce0 partial apply for closure #&#8203;1 in closure #&#8203;1 in closure #&#8203;1 in _fatalErrorMessage(_:_:file:line:flags:) + 97
4    libswiftCore.dylib                 0x000000010bf332e0 specialized StaticString.withUTF8Buffer<A>(_:) + 216
5    libswiftCore.dylib                 0x000000010c223c20 partial apply for closure #&#8203;1 in closure #&#8203;1 in _assertionFailure(_:_:file:line:flags:) + 146
6    libswiftCore.dylib                 0x000000010c223c00 partial apply for closure #&#8203;1 in closure #&#8203;1 in _fatalErrorMessage(_:_:file:line:flags:) + 23
7    libswiftCore.dylib                 0x000000010bf332e0 specialized StaticString.withUTF8Buffer<A>(_:) + 216
8    libswiftCore.dylib                 0x000000010c178f80 partial apply for closure #&#8203;1 in _fatalErrorMessage(_:_:file:line:flags:) + 159
9    libswiftCore.dylib                 0x000000010c178f20 partial apply for closure #&#8203;1 in _fatalErrorMessage(_:_:file:line:flags:) + 23
10   libswiftCore.dylib                 0x000000010bf332e0 specialized StaticString.withUTF8Buffer<A>(_:) + 216
11   libswiftCore.dylib                 0x000000010bf331e0 _fatalErrorMessage(_:_:file:line:flags:) + 126
12   libswiftCore.dylib                 0x000000010c0fe2d0 specialized Character.init(_largeRepresentationString:) + 671
13   libswiftCore.dylib                 0x000000010bf40920 Character.init(_builtinExtendedGraphemeClusterLiteral:utf8CodeUnitCount:isASCII:) + 161
14   Benchmark_Onone                    0x000000010be30ac0 run_SuperChars(_:) + 2155
15   Benchmark_Onone                    0x000000010bdcdd70 thunk for @callee_owned (@unowned Int) -> () + 15
16   Benchmark_Onone                    0x000000010be3e9d0 partial apply for thunk for @callee_owned (@unowned Int) -> () + 74
17   Benchmark_Onone                    0x000000010bd4d0d0 partial apply for thunk for @callee_owned (@in Int) -> (@out ()) + 56
18   Benchmark_Onone                    0x000000010bd48de0 specialized runBench(_:_:_:) + 1327
19   Benchmark_Onone                    0x000000010bd4ae50 specialized runBenchmarks(_:) + 3001
20   Benchmark_Onone                    0x000000010bd3b3c0 main() + 1604
21   Benchmark_Onone                    0x000000010be36bf0 main + 31777
22   libdyld.dylib                      0x00007fffcff28234 start + 1
56ed3025-6cad-4bf8-b972-9c308205a0af commented 7 years ago

CI is able to build and run the benchmark. I did a clean build of swift and it didn't help... So I guess I need a clean rebuild of llvm and clang, too?

56ed3025-6cad-4bf8-b972-9c308205a0af commented 7 years ago

I’m still having this issue after clean build with ToT:
Swift version 4.0-dev (LLVM f61cc3ca1f, Clang b4252460af, Swift d1bd12f7cb)
Reopening.

@dabrahams Can you help? (I see you’re up...)

dabrahams commented 7 years ago

So, you must be running an interesting configuration for benchmarks that the bots don't test. A _sanityCheck is firing. _sanityCheck is basically for the standard library's internal assertions and is not supposed to be compiled into any shipping builds. Any configuration that leaves _sanityCheck's enabled would be terrible for benchmarking! That said, you have found a problem that is not detected by any of our regular tests. The _sanityCheck is trying to check that the Character in question contains only one grapheme. Now, it's actually not too surprising that it should fire given the current state of our grapheme breaking, which is using a mixture of Unicode 8 and Unicode 9-compatible algorithms. But it's not clear that's what is causing it to fire.

Would you please:

  1. Include full instructions for reproducing the problem, including the commands used to build swift and launch the benchmark?

  2. Try to reduce the test a little? Ideally we'd end up with something we can put in the standard library test suite.

Thanks!

tkremenek commented 7 years ago

Dave pointed out to me that this is possibly related to:

https://github.com/apple/swift/pull/9784

This has been temporarily reverted to see if it is related to this issue.

tkremenek commented 7 years ago

Actually, PR 9784 landed after this failure started.

tkremenek commented 7 years ago

I re-applied the change in PR 9784.

tkremenek commented 7 years ago

@swift-ci create

dabrahams commented 7 years ago

OK, so I think there's no risk here. The small reproducer is

let x: Character = "\u{1f1ef}\u{1f1f5}\u{1f1ef}\u{1f1f5}" // two Japanese flags

which will only fail in a build made with --swift-stdlib-assertions.

The compiler shouldn't even let us write that code… if we're compiling for a target with Unicode 9-compatible grapheme breaking. The compiler is clearly using a Unicode-8-compatible grapheme boundary detector. Hooboy, we can't make the compiler behavior dependent on the ICU installed on the target system, can we? The whole notion of an extended grapheme cluster literal may be broken from that perspective.

I'll take out the sanity check. What we do about Character literals is a separate question.

dabrahams commented 7 years ago

Fixed in https://github.com/apple/swift/pull/9815

56ed3025-6cad-4bf8-b972-9c308205a0af commented 7 years ago

Above mentioned fix did resolve the issue for me. Closing as Done.

56ed3025-6cad-4bf8-b972-9c308205a0af commented 7 years ago

Re: @dabrahams

Would you please:

  1. Include full instructions for reproducing the problem, including the commands used to build swift and launch the benchmark?

  2. Try to reduce the test a little? Ideally we'd end up with something we can put in the standard library test suite.

  1. I have been running normal build compiled with build-script -R. The crash manifested itself during Benchmark_Driver run. Only thing outside of normal is that I'm running this on an ancient MacBook Pro (15-inch, Late 2008) with 2,4 GHz Intel Core 2 Duo. I don't understand how that could be a factor, but since CI ran fine, that's only thing that comes to mind...

  2. You did come up with small reproducer above, right? (Haven't seen that added into validation-test as apart of PR #9815, BTW)