Skip to content

[Backtracing][Linux] Enable Linux backtracing, add tests. #66338

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 14 commits into from
Jun 7, 2023

Conversation

al45tair
Copy link
Contributor

@al45tair al45tair commented Jun 5, 2023

Turn on the Linux backtracing implementation and update the tests.

rdar://101623265

@al45tair
Copy link
Contributor Author

al45tair commented Jun 5, 2023

This needs #66337, which needs #66335 (which in turn requires #66334 and #66333).

@al45tair
Copy link
Contributor Author

al45tair commented Jun 5, 2023

@swift-ci Please test

al45tair added a commit to al45tair/swift that referenced this pull request Jun 5, 2023
This should have been disabled until swiftlang#66338.

rdar://110261430
@al45tair al45tair force-pushed the eng/PR-101623265 branch from c4f92c5 to e3425aa Compare June 5, 2023 21:42
@al45tair
Copy link
Contributor Author

al45tair commented Jun 5, 2023

@swift-ci Please test

1 similar comment
@al45tair
Copy link
Contributor Author

al45tair commented Jun 6, 2023

@swift-ci Please test

@al45tair
Copy link
Contributor Author

al45tair commented Jun 6, 2023

When reviewing, you can start at c9992ad; previous change sets are from #66333, #66334, #66335 and #66337 and will be reviewed there.


import Swift

@_implementationOnly import OS.Libc
Copy link
Contributor

Choose a reason for hiding this comment

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

I wasn't aware of this module, do we finally have a replacement for all the numerous platform-specific libc modules?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

No. The _Backtracing module has its own thing that does precisely that, but it isn't available outside of the build of libswift_Backtracing. Why do that? Because we actually can't use Glibc, Darwin et al from here for build system related reasons, so the alternative was a horrible header in SwiftShims that had to redeclare all kinds of things from each of the C libraries. That made me very sad, so instead I turned off implicit modules and added a modules directory in the Backtracing directory that contains a handful of special modules that are only used by _Backtracing, only with the C/C++ importer, and only ever @_implementationOnly at that.

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 is relevant here because of 1c8362b, which was needed because I did that (and because the test does slightly the wrong thing — it treats the module it's testing as the main module, which causes the deserialiser to behave differently).

@al45tair
Copy link
Contributor Author

al45tair commented Jun 6, 2023

Just to clarify, the commits that need reviewing in this PR are c9992ad, e3425aa and 1c8362b. The others aren't relevant to this one, and I don't want to get everything all mixed up here if we can help it.

@al45tair al45tair requested a review from MaxDesiatov June 6, 2023 12:19
@al45tair
Copy link
Contributor Author

al45tair commented Jun 6, 2023

swiftlang/swift-corelibs-xctest#445

@swift-ci Please clean test Linux platform

al45tair added 4 commits June 6, 2023 16:16
The Swift backtracer's frame pointer unwinder cannot work on Linux
without this change, because the compiler omits the frame pointer from
the function in libSwift_Backtracing that actually captures the stack.

rdar://110260855
Using `SwiftShims` is undesirable - it creates all kinds of build issues,
and means shipping the `_SwiftBacktracing.h` header in the SDK, which is
not necessary.

While we're doing this, add the necessary definitions for reading ELF
and DWARF information.

rdar://110261712
Use the new module structure rather the old SwiftShims header.  This
is much cleaner and lets us include operating system headers to get
the relevant definitions where possible.

Add code to support ELF and DWARF, including decompression using
zlib, zstd and liblzma if those turn out to be required and available.

rdar://110261712
This is for compatibility, so that I can split up the PRs.
We'll remove it in the next PR.

rdar://110261712
@al45tair al45tair force-pushed the eng/PR-101623265 branch from 1c8362b to bc738e7 Compare June 6, 2023 16:47
@al45tair
Copy link
Contributor Author

al45tair commented Jun 6, 2023

swiftlang/swift-corelibs-xctest#445

@swift-ci Please clean test Linux platform

There's a chance that pipes might perform a partial read; we should
handle that case.

rdar://110261712
@al45tair al45tair force-pushed the eng/PR-101623265 branch from bc738e7 to 65803a6 Compare June 6, 2023 17:28
@al45tair
Copy link
Contributor Author

al45tair commented Jun 6, 2023

swiftlang/swift-corelibs-xctest#445

@swift-ci Please clean test Linux platform

al45tair added 2 commits June 7, 2023 08:19
The `status` argument to the `_swift_backtrace_demangle()` function
isn't especially useful, won't match behaviour on Windows, and we
actually don't use it in the Swift code that calls this SPI.

Remove it.

rdar://110261712
`__cxa_demangle()` is a rather unusual API; one of its "features" is that
the pointer you pass in must either be `nullptr`, in which case it
will call `malloc()` itself, _or_ it has to be a pointer to a block
of memory allocated with `malloc()`, because `__cxa_demangle()` may
`realloc()` it for you.

This seems to me to be something of a non-obvious footgun, so we never
pass the caller's pointer through to `__cxa_demangle()`, which lets them
decide how they want to allocate space.

rdar://110261712
@al45tair al45tair force-pushed the eng/PR-101623265 branch from 65803a6 to 47f47f2 Compare June 7, 2023 07:29
@al45tair
Copy link
Contributor Author

al45tair commented Jun 7, 2023

swiftlang/swift-corelibs-xctest#445

@swift-ci Please clean test

al45tair added a commit to al45tair/swift that referenced this pull request Jun 7, 2023
This should have been disabled until swiftlang#66338.

rdar://110261430
@al45tair
Copy link
Contributor Author

al45tair commented Jun 7, 2023

Ah, the swift-corelibs-xctest PR is merged now, which will upset that test run.

@al45tair
Copy link
Contributor Author

al45tair commented Jun 7, 2023

@swift-ci Please test

al45tair added 7 commits June 7, 2023 09:02
…ule.

There's a separate declaration here because we can't include the `Backtrace.h`
header from inside `modules/Runtime/Runtime.h`.

rdar://110261712
The `_Backtracing` module has a number of private implementation only
imports that aren't used outside of the module and that don't require
any additional libraries (hence they aren't relevant to the outside
world).  `verify_all_overlays.py` needs to know about these when it
does its test, because it loadas the module as the main module, which
results in implementation only imports being required instead of
ignored.

rdar://110261712
We need a Linux specific `Target` implementation, and a couple of minor
tweaks to make things build everywhere.

rdar://110262673
We don't need these here.  They were a leftover from previous code.

rdar://110262673
Turn on the Linux backtracing implementation and update the tests.

rdar://101623265
@al45tair al45tair force-pushed the eng/PR-101623265 branch from 47f47f2 to 3042cd7 Compare June 7, 2023 08:08
@al45tair
Copy link
Contributor Author

al45tair commented Jun 7, 2023

@swift-ci Please test

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants