Skip to content

Inproc crashreport ios#2

Draft
mdh1418 wants to merge 2 commits intoinproc_crashreport_minimalfrom
inproc_crashreport_ios
Draft

Inproc crashreport ios#2
mdh1418 wants to merge 2 commits intoinproc_crashreport_minimalfrom
inproc_crashreport_ios

Conversation

@mdh1418
Copy link
Copy Markdown
Owner

@mdh1418 mdh1418 commented Apr 22, 2026

No description provided.

mdh1418 pushed a commit that referenced this pull request Apr 28, 2026
…otnet#124642)

## Summary

Fixes dotnet#123621

When a constant-folded operand appears **after** a non-constant operand
in a short-circuit `&&` expression (e.g., `v == 2 && Environment.NewLine
!= "\r\n"`), callee inlining can leave dead local stores in the return
block. The `isReturnBool` lambda in `fgFoldCondToReturnBlock` required
`hasSingleStmt()`, which caused the optimization to bail out when these
dead stores were present, resulting in suboptimal branching codegen.

### Changes

- **`src/coreclr/jit/optimizebools.cpp`**: Relax the `hasSingleStmt()`
constraint in `isReturnBool` to allow preceding statements as long as
they have no globally visible side effects
(`GTF_GLOBALLY_VISIBLE_SIDE_EFFECTS`). This enables
`fgFoldCondToReturnBlock` to fold the conditional into a branchless
return even when dead local stores from inlining remain in the block.

### Before (ARM64, `Inline_After`)
```asm
      cmp     w0, #2
      bne     G_M4495_IG04
      mov     w0, #1
      ret
G_M4495_IG04:
      mov     w0, #0
      ret
```

### After (ARM64, `Inline_After`)
```asm
      cmp     w0, #2
      cset    x0, eq
      ret
```

## Test plan

- [x] Added regression test `Runtime_123621` covering the original issue
pattern
- [x] Verified `Hoisted`, `Inline_Before`, and `Inline_After` all
produce identical branchless codegen (`cset` on ARM64)
- [x] Verified existing `DevDiv_168744` regression test still passes
- [x] Verified side-effect-ful blocks are correctly excluded from the
optimization
mdh1418 and others added 2 commits April 30, 2026 11:07
Broaden the Android-only opt-in for the in-proc crash reporter to cover
the other mobile Apple platforms: iOS, tvOS, and MacCatalyst. None of
these ship with createdump, so the in-proc reporter is the only
crash-report path available and the Android integration pattern applies
directly. FEATURE_INPROC_CRASHREPORT in clrfeatures.cmake is extended
from CLR_CMAKE_TARGET_ANDROID to also cover CLR_CMAKE_TARGET_IOS,
CLR_CMAKE_TARGET_TVOS, and CLR_CMAKE_TARGET_MACCATALYST; no other build
plumbing is needed because the existing PAL callback registration and
DOTNET_EnableCrashReport / DOTNET_EnableCrashReportOnly knobs are
already platform-agnostic.

inproccrashreporter.cpp gains __APPLE__ branches for the instruction,
stack, and frame pointer accessors. Darwin's ucontext_t::uc_mcontext is
a pointer to __darwin_mcontext64, so the Apple branches dereference
through ->__ss. On arm64 the arm_thread_state64_get_pc/sp/fp(__ss)
accessor macros are used so the registers are stripped of pointer
authentication codes on arm64e (identity on arm64).

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Replace the placeholder empty strings for OSVersion and SystemModel in
the in-proc crash report with values read via sysctlbyname, matching
what createdump's CrashReportWriter::WriteSysctl does:

  OSVersion      <- kern.osproductversion  (e.g. '15.1')
  SystemModel    <- hw.model               (e.g. 'MacBookPro18,3', 'iPhone13,2')

A small WriteSysctlString helper uses a CRASHREPORT_STRING_BUFFER_SIZE
stack buffer (ample for both sysctl values) and writes an empty string
on failure so the JSON schema remains stable for downstream consumers.
sysctlbyname is async-signal-safe and avoids heap allocation, matching
the createdump behavior. SystemManufacturer stays hardcoded to 'apple'.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
@mdh1418 mdh1418 force-pushed the inproc_crashreport_ios branch from 126ac32 to 2cb34d2 Compare April 30, 2026 16:33
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.

1 participant