Skip to content
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

[Backtracing][Linux] Add support for Linux in the _Backtracing module. #66335

Merged
merged 14 commits into from Jun 7, 2023

Conversation

al45tair
Copy link
Contributor

@al45tair al45tair commented Jun 5, 2023

Rearrange things to avoid using SwiftShims, because that's grotty and means we have to carry all kinds of definitions from the C library. Instead, turn off implicit modules for the _Backtracing build and add some explicit modules that we can use as we please.

Also add the code to handle ELF and DWARF for Linux, including symbol lookup, demangling, line number/source file extraction, Linux mcontext_t support et al.

Tests for all of this will be in a separate PR that also enables everything.

rdar://110261712

This also adds a function to demangle a symbol, and a way for the
backtracing code to report warning messages to the same place as
the main runtime.

I'd like to rename the _swift_isThunkFunction() SPI also, but we
can't do that until we've made the changes to the _Backtracing
library, so we'll do that there instead.

rdar://110261430
@al45tair
Copy link
Contributor Author

al45tair commented Jun 5, 2023

This depends on #66333 and #66334.

This was added to a later PR, but not to this one, though we need
it here.

rdar://110261430
@al45tair
Copy link
Contributor Author

al45tair commented Jun 5, 2023

@swift-ci Please smoke test

This should have been disabled until apple#66338.

rdar://110261430
@al45tair
Copy link
Contributor Author

al45tair commented Jun 5, 2023

@swift-ci Please smoke test

@al45tair
Copy link
Contributor Author

al45tair commented Jun 6, 2023

@swift-ci Please smoke test Linux platform

@al45tair al45tair requested review from mikeash and etcwilde June 6, 2023 10:08
@al45tair
Copy link
Contributor Author

al45tair commented Jun 6, 2023

When reviewing this, you can start at 7e3df6e; the changes prior to that are in #66333 and #66334 and will be reviewed there instead.

@al45tair
Copy link
Contributor Author

al45tair commented Jun 6, 2023

If you're wondering where the tests are, I left them until the last PR (#66338) so everything was enabled and they'd actually work.

Copy link
Contributor

@mikeash mikeash left a comment

Choose a reason for hiding this comment

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

LGTM. I'm trusting you and your tests for large chunks of the DWARF/ELF/compression stuff.

stdlib/public/Backtracing/Context.swift Show resolved Hide resolved
stdlib/public/Backtracing/MemoryReader.swift Outdated Show resolved Hide resolved
Mike and Max made various helpful suggestions, so I've added and updated
various comments and amended the code to cope with partial reads and
writes.

rdar://110261430
Moved the comment for `_swift_backtrace_demangle` into the header file
instead of it being in the implementation.

rdar://110261430
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
Copy link
Contributor Author

al45tair commented Jun 6, 2023

@swift-ci Please smoke test

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

rdar://110261712
@al45tair
Copy link
Contributor Author

al45tair commented Jun 6, 2023

@swift-ci Please smoke test

Copy link
Member

@etcwilde etcwilde left a comment

Choose a reason for hiding this comment

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

Okay, some comments. Maybe could use a little more documenting. The code mostly looks fine though.

include/swift/Runtime/Backtrace.h Show resolved Hide resolved
stdlib/public/runtime/Backtrace.cpp Show resolved Hide resolved
stdlib/public/runtime/Backtrace.cpp Outdated Show resolved Hide resolved
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
Copy link
Contributor Author

al45tair commented Jun 7, 2023

@swift-ci Please smoke test

…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
@al45tair
Copy link
Contributor Author

al45tair commented Jun 7, 2023

@swift-ci Please test

@al45tair al45tair requested a review from etcwilde June 7, 2023 08:23
@al45tair al45tair merged commit d673544 into apple:main Jun 7, 2023
5 checks passed
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.

None yet

3 participants