Repository navigation
[cDAC] Add stackwalk DAC flags - #11
anurag6569201 wants to merge 1 commit into
Conversation
Source PR: dotnet#131901 Source head: 714df81
⛔ Shipwright · BlockedRecommendation: do not merge PR #11 · Tier
Findings (4)
Fireworks usage: 7,284 input · 555 output · 7,839 total tokens · $0.0020 · 10s · 0 fix iteration(s) Open the Shipwright check for full evidence and the audit bundle. Use |
![shipwright-agent[bot]](https://avatars.githubusercontent.com/in/4490927?s=60&v=4){kind=small}
| } | ||
|
|
||
| internal static bool IsLegacyVisible(IStackDataFrameHandle frame) | ||
| internal bool IsLegacyVisible(IStackDataFrameHandle frame) |
There was a problem hiding this comment.
Shipwright · CRITICAL
The IsLegacyVisible logic now hides Frame and SkippedFrame states unless CLRDATA_SIMPFRAME_RUNTIME_UNMANAGED_CODE is set.
Impact: The IsLegacyVisible logic now hides Frame and SkippedFrame states unless CLRDATA_SIMPFRAME_RUNTIME_UNMANAGED_CODE is set. Any existing caller that invokes CreateStackWalk with flags=0 (the common default) will silently lose managed frames that were previously visible, producing truncated or empty stack walks in production diagnostics. This is a behavioral regression, not an additive flag.
Suggested fix: Review the cited evidence, fix the risk if confirmed, and rerun Shipwright.
| } | ||
|
|
||
| internal static bool IsLegacyVisible(IStackDataFrameHandle frame) | ||
| internal bool IsLegacyVisible(IStackDataFrameHandle frame) |
There was a problem hiding this comment.
Shipwright · HIGH
The 'IsLegacyVisible' logic was changed from returning true for 'Frameless', 'Frame', or 'SkippedFrame' states to only returning true for 'Frameless' unless the 'CLRDATA_SIMPFRAME_
Impact: The 'IsLegacyVisible' logic was changed from returning true for 'Frameless', 'Frame', or 'SkippedFrame' states to only returning true for 'Frameless' unless the 'CLRDATA_SIMPFRAME_RUNTIME_UNMANAGED_CODE' flag is set. This means frames in 'Frame' or 'SkippedFrame' states are now hidden by default, which is a behavioral regression for stack walks that do not set this flag. The previous behavior treated these states as…
Suggested fix: Fix the review finding before release.
| } | ||
|
|
||
| internal static bool IsLegacyVisible(IStackDataFrameHandle frame) | ||
| internal bool IsLegacyVisible(IStackDataFrameHandle frame) |
There was a problem hiding this comment.
Shipwright · HIGH
The flag name CLRDATA_SIMPFRAME_RUNTIME_UNMANAGED_CODE is used to gate visibility of Frame and SkippedFrame states, but the name suggests it controls unmanaged runtime frames.
Impact: The flag name CLRDATA_SIMPFRAME_RUNTIME_UNMANAGED_CODE is used to gate visibility of Frame and SkippedFrame states, but the name suggests it controls unmanaged runtime frames. The relationship between the flag and the visibility behavior is non-obvious and undocumented, making future maintenance error-prone.
Suggested fix: Review the cited evidence, fix the risk if confirmed, and rerun Shipwright.
| CLRDATA_STACK_WALK_REQUEST_SET_FIRST_FRAME = 0xe1000000, | ||
| } | ||
|
|
||
| [Flags] |
There was a problem hiding this comment.
Shipwright · HIGH
The new CLRDataStackWalkFlag enum is declared with [Flags] but only defines value 0x8.
Impact: The new CLRDataStackWalkFlag enum is declared with [Flags] but only defines value 0x8. Callers passing arbitrary uint values cast to this enum can set undefined bits; IsLegacyVisible only checks one bit, so unknown flag combinations are silently ignored. This weakens validation at the interop boundary and can mask caller errors.
Suggested fix: Review the cited evidence, fix the risk if confirmed, and rerun Shipwright.
1 participant
Fixes dotnet#112366
Source merge-base:
7ddca6ea6099f3983e9b6d63d52acd9337bfcb7aSource head:
714df81a197342a8e4562ed7a3e9807b57846df6