swift-docc: `convert` and `preview` commands crash on certain Markdown pages
Description
When rendering Swift compiler documentation the diagnostics formatting code crashes on certain Markdown pages.
Checklist
- If possible, I’ve reproduced the issue using the
main
branch of this package. - This issue hasn’t been addressed in an existing GitHub issue.
Expected Behavior
No crashes.
Actual behavior
Swift/StringCharacterView.swift:158: Fatal error: String index is out of bounds
Steps To Reproduce
- Clone https://github.com/apple/swift.git
- Install a recent development snapshot of Swift, I’m using
swift-DEVELOPMENT-SNAPSHOT-2023-07-23-a
. - Run DocC from this snapshot this way (assuming
swift
is the clone from step 1):
docc preview --allow-arbitrary-catalog-directories swift/docs
Swift-DocC Version Information
swift-DEVELOPMENT-SNAPSHOT-2023-07-23-a
Swift Compiler Version Information
Apple Swift version 5.9-dev (LLVM 9b562f55c38e378, Swift b4ee68bd37c4f7d)
Target: arm64-apple-macosx14.0
About this issue
- Original URL
- State: closed
- Created a year ago
- Comments: 15 (10 by maintainers)
I’m also frequently encountered this locally. This should definitely be a block issue for release/5.10 since it may crash a lot of downstream user.
Maybe we can discuss it in next DocC workgroup. cc @franklinsch @QuietMisdreavus
IMO, We should either fix it before 5.10 release or revert relevant PR until we fix the issue.
For the problem you found, I’ve open a PR to fix it. https://github.com/apple/swift-markdown/pull/151 cc @arthurcro
And there is another crash cause, I’ve opened a new issue on swift-docc to track it. #729
Hi! 🙂I have identified the issue and I have found a proper fix. The crashes were caused by poor UTF-8 encoding handling while highlighting a source line using the diagnostic source range. I will open a pull request with the fix and several tests covering the issue shortly!
Thanks again for reporting this! @MaxDesiatov cc @franklinsch
Both bugfix PR is merged into main. The cherry-pick to release/5.10 is still in progress.
If you are still encountering such crash on the latest toolchain, feel free to reopen the issue. @MaxDesiatov
Thanks for pointing it out. Maybe you can help file up a issue with some minimum reproducible input to Swift-Markdown so that I can help to look at it on the Markdown module side.
QuietMisdreavus should have some better insights on such problem. But it looks like she is busy now and will back September 18.
Also to know that there are several other issues about SourceRange parsing like https://github.com/apple/swift-markdown/issues/71. I have tried to fix it, but the CodeReview process is pending for some unknown reason.
Hi! 👋 After some investigation, I’ve identified what seems to be the underlying cause of the problem.
Interestingly, the issue doesn’t seem to be directly caused by the improved default diagnostic formatter, even though the formatter does highlight it.
While I might not have captured all the details accurately, here’s my understanding so far:
When using:
DocC will see this as a symbol link.
For this specific crash, when using:
DocC will try to resolve this symbol link as a
TopicReference
but it will eventually fail.https://github.com/apple/swift-docc/blob/f70b3ea281773121a468d9c0966ab8a34bba58d2/Sources/SwiftDocC/Semantics/MarkupReferenceResolver.swift#L161
To resolve the symbol link and eventually emit a diagnostic for this resolution error, DocC uses the symbol link
SourceRange
provided by MarkupData.swift fromswift-markdown
.This source range is used as the diagnostic’s source range in case of resolution error. It’s then used by the default diagnostic formatter to highlight the unresolved reference. If the source range does not match the actual content of the source file, it crashes.
If we look at your crash, we get a
SourceRange
253:56-253:97
while if we look at the source file content, we should be getting253:56-254:5
. It seems we are getting an incorrect source range for some symbol links.I hope that made sense. @d-ronnqvist I’m not sure how to proceed to fix this issue. Could you advice possible next steps? 🙇🏻
Great! I can reproduce with this commit, thank you! 🙇🏻
Reproduced again with
main
DocC, checkout this commit with the docs: https://github.com/apple/swift/commit/1de261bb28b99c6d63a325fef84c854415ab7a32I didn’t, potentially more docs were published in the
docs
directory in question. But this crash looked similar if it wasn’t the same as the original issue, so I felt that reopening this one would work better than creating a new one.@arthurcro this is still reproducible on
main
with f70b3ea, I’m reopening the issueThe code that crashes is in the new diagnostics formatter (#535). cc @arthurcro if you have any ideas 🙂