Skip to content

Make TryGetSymbol cross-os - #135734

Merged
rcj1 merged 5 commits into
dotnet:mainfrom
rcj1:alternate
Oct 19, 2026
Merged

Make TryGetSymbol cross-os#135734
rcj1 merged 5 commits into
dotnet:mainfrom
rcj1:alternate

Conversation

@rcj1

@rcj1 rcj1 commented Oct 14, 2026 •
edited
Loading

Copy link
Copy Markdown
Contributor

Ensure that TryGetSymbol works for all architectures and OSes regardless of the host architecture and OS.

rcj1 and others added 2 commits October 13, 2026 16:43
@rcj1
Dispatch symbol lookup using data target information
ae0068b 
Compile PE, ELF, and Mach-O readers together and select the existing decoder in a separate TryGetSymbol multiplexer. Add the matching ICorDebugDataTarget5 interface used by diagnostics.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Copilot-Session: 48661cdb-9269-437f-86fa-da4ae6de5b8e
@rcj1
use byte sniffing instead
c1c0fb1
@azure-pipelines

azure-pipelines Bot commented Oct 14, 2026

Copy link
Copy Markdown
Azure Pipelines:
Successfully started running 4 pipeline(s).
12 pipeline(s) were filtered out due to trigger conditions.
There may be pipelines that require an authorized user to comment /azp run to run.

@rcj1 rcj1 changed the title Alternate Make TryGetSymbol cross-os Oct 14, 2026
@dotnet-policy-service

dotnet-policy-service Bot commented Oct 14, 2026

Copy link
Copy Markdown
Contributor

Tagging subscribers to this area: @steveisok, @tommcdon, @dotnet/dotnet-diag
See info in area-owners.md if you want to be subscribed.

rcj1 and others added 3 commits October 13, 2026 17:12
@rcj1
Fix formatting in cordebug.h
ad30b0a
@rcj1
Fix conditional check for CLR_CMAKE_HOST_OSX
2b5cbda
@rcj1
Limit cross-platform symbol lookup to supported hosts
9b19433 
Guard the image readers for Windows, Linux, and macOS, excluding WASM. Return false for symbol lookup on other hosts while retaining shared dbgutil helpers.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Copilot-Session: 48661cdb-9269-437f-86fa-da4ae6de5b8e
@rcj1
rcj1 requested a review from hoyosjs October 14, 2026 03:47
@rcj1
rcj1 marked this pull request as ready for review October 14, 2026 04:20
This was referenced Oct 14, 2026
Closed
Closed
Closed
Closed
@steveisok
steveisok self-requested a review October 19, 2026 17:05
@jkotas

jkotas commented Oct 19, 2026

Copy link
Copy Markdown
Member

What is this change needed for?

@rcj1

rcj1 commented Oct 19, 2026

Copy link
Copy Markdown
Contributor Author

What is this change needed for?

This is needed in the case that we enter here inspecting a dump, and the target and host OS are different.

@rcj1
rcj1 merged commit 5aa0039 into dotnet:main Oct 19, 2026
101 of 103 checks passed
@rcj1
rcj1 deleted the alternate branch October 19, 2026 21:47
@dotnet-maestro dotnet-maestro Bot mentioned this pull request Oct 20, 2026
Merged
@dotnet-milestone-bot dotnet-milestone-bot Bot added this to the 12.0-preview1 milestone Oct 20, 2026
@hoyosjs

hoyosjs commented Oct 20, 2026

Copy link
Copy Markdown
Member

@rcj1 sorry I didn't get to this before merging. A few things:

  • Failed reads during bias detection are treated as "no additional bias needed." Could we distinguish detection failure from a definite no-bias result? Lookup might fail later, but it proceeds with an invalid pointer interpretation.
  • It doesn't guard for ill-formed/non-terminated dynamic table in HasMuslDependency. It should use PT_DYNAMIC extent as upper bound instead of trusting DT_NULL is always there. Could we bound the walk and process dependencies without retaining all their offsets? That might need 2 walks but might be better to guard.
  • There are also existing hardening gaps worth addressing as these readers become available in cross-host scenarios:
    • unchecked baseAddress + e_phoff, dynamic-pointer-plus-bias, and symbol-index multiplication.
    • The read adapters accept successful short reads, so ELF records can be consumed without all their fields having been initialized.

@steveisok steveisok mentioned this pull request Oct 23, 2026
Merged
steveisok added a commit that referenced this pull request Oct 23, 2026
@steveisok
Fix TryGetSymbol on iOS, tvOS and Mac Catalyst (#136361)
856a1e3 
## Summary

#135734 ("Make TryGetSymbol cross-os") moved `TryGetSymbol` into
`src/coreclr/debug/dbgutil/symbol.cpp`. The new dispatcher and the
Mach-O and ELF
readers are compiled only under

```c
#if !defined(HOST_WASM) && (defined(HOST_WINDOWS) || defined(HOST_OSX) || defined(__linux__))
```

and `dbgutil/CMakeLists.txt` adds the in-tree `inc/llvm` include
directory
(for `ELF.h`) only for `CLR_CMAKE_HOST_WIN32 OR CLR_CMAKE_HOST_OSX`.

Apple mobile hosts (iOS, tvOS, Mac Catalyst) define `HOST_APPLE`, plus
`HOST_IOS`, `HOST_TVOS` or `HOST_MACCATALYST`, but **not** `HOST_OSX`
(`eng/native/configurecompiler.cmake`). On those hosts `TryGetSymbol`
compiles
to `return false`. Before #135734, `machoreader.cpp` (with
`TryGetSymbol`) was
built for every `CLR_CMAKE_TARGET_APPLE`.

Consequences for in-app debugging on Apple mobile, where the DBI and DAC
run in
the app process:
- The DAC's `GetDacTableAddress` can't find the DAC table export.
  `CordbProcess::Init` → `CreateDacDbiInterface` fails with
  `CORDBG_E_MISSING_DEBUGGER_EXPORTS` (`0x80131C4F`), so
  `DebugActiveProcess` fails and no debugger events are ever raised.
`Debugger.Break()` is a no-op, and the debugger UI shows nothing. The
HRESULT
  isn't surfaced to the user by the IDE.
- The DBI and DAC lookups of `DotNetRuntimeContractDescriptor`
(`di/process.cpp`, `daccess.cpp`, `enummem.cpp`) also fail, so the cDAC
can't
  be located either.

## Change

Use `HOST_APPLE` instead of `HOST_OSX`:
- in the reader guards: `symbol.cpp` (×2), `machoreader.cpp`,
`elfreader.cpp`;
- for the `inc/llvm` include directory in `dbgutil/CMakeLists.txt`.
Without
it, `elfreader.h`'s `#include <ELF.h>` (already chosen for `HOST_APPLE`)
  fails to compile once the guards are widened.

5 lines across 4 files; no behavior change for Windows, Linux or macOS
hosts.

**Alternative for reviewers:** a broader guard such as
`!defined(HOST_WASM) && (defined(HOST_WINDOWS) || defined(HOST_UNIX))`
would also
cover non-Linux Unix hosts (for example FreeBSD and Haiku), which today
also hit
the `return false` fallback for the contract-descriptor lookup. That
would need
the include directory and `elfreader.h`'s `<ELF.h>` vs `<elf.h>`
selection
checked for those hosts. This PR keeps the minimal Apple-only change.

## Validation

- iOS Simulator (iossimulator-arm64, Xcode 27):
- built with `./build.sh clr+clr.runtime+libs+packs -os iossimulator
-arch arm64 -cross -c Release`,
with `-cmakeargs "-DCMAKE_CXX_FLAGS=-Wno-error=#warnings"`. That
argument is
an unrelated local workaround for Xcode 27's libc++ `#warning` about the
    13.0 minimum under main's `-Werror`.
  - The DAC now contains the Mach-O reader (`TryGetMachOSymbol`).
- With the locally built runtime pack, the in-app CoreCLR debugger (VS
Code,
  .NET MAUI F5 flow, remote debugger host and target) **stops at
  `Debugger.Break` and steps again**. Before the fix, the attach failed
silently, with `0x80131C4F` from `CordbProcess::Init` confirmed by lldb
on
  the in-app DBI.
- The end-to-end debugger validation ran on a build whose source tree is
identical to this commit; this exact commit was also rebuilt for
iossimulator-arm64 to confirm it builds.
- No unit test covers this native, host-configuration-dependent path.

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@steveisok steveisok steveisok approved these changes
@hoyosjs hoyosjs Awaiting requested review from hoyosjs

Assignees

@rcj1 rcj1

Labels

area-Diagnostics-coreclr

Projects

None yet

Milestone

12.0-preview1

Development

Successfully merging this pull request may close these issues.

4 participants

@rcj1 @jkotas @hoyosjs @steveisok