- Notifications
You must be signed in to change notification settings - Fork7.7k
Fix debug tracing error with magic extents#25726
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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.
Already on GitHub?Sign in to your account
Uh oh!
There was an error while loading.Please reload this page.
Conversation
Fixes the error when PowerShell attempts to print the debug trace linefor functions generated with an invalid extent. For example PowerShellcreates an extent with a column and line of 0 for things like thedefault class constructors. This is an issue as the trace debug linealways subtracts by 1 to make the value start at 0 rather than 1.It also adds the DebuggerHidden attribute to the class defaultconstructor to avoid confusing end users from seeing a trace line forsomething they did not write.
| { | ||
| Set-PSDebug -Trace 1 | ||
| [ClassWithDefaultCtor]::new() | ||
| } | Should -Not -Throw |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
Does the test really throw before the fix?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
Run it yourself and see :)
| if (_context.ShouldTraceStatement && | ||
| !_callStack.Last().IsFrameHidden && | ||
| !functionContext._debuggerStepThrough && | ||
| functionContext.CurrentPosition.StartScriptPosition.ColumnNumber > 0) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
BothStartScriptPosition andInternalScriptPosition.ColumnNumber are calculated in the getter, so while tracing isn't exactly perf focused hopefully something like this will work:
| functionContext.CurrentPosition.StartScriptPosition.ColumnNumber>0) | |
| functionContext.CurrentPositionis notEmptyScriptExtent&& | |
| (functionContext.CurrentPositionisInternalScriptExtent|| | |
| functionContext.CurrentPosition.StartColumnNumber>0)) |
Thoughts?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
Just tested it locally and looks like it works for this scenario. I wasn't sure whether the fix should have been done here or just a check in theBriefMessage to just treat it as the start column or the change in this PR.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
I see EmptyScriptExtent is used only in scenario where an error is in script file. So it makes no sense to check the both types.
SeeminglyScienceJul 15, 2025 • edited
Loading Uh oh!
There was an error while loading.Please reload this page.
edited
Uh oh!
There was an error while loading.Please reload this page.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
I see EmptyScriptExtent is used only in scenario where an error is in script file. So it makes no sense to check the both types.
I think you may have followed the wrong reference, it's used in a ton of places including this one. It's typically accessed viaPositionUtilities.EmptyExtent so maybe you were just looking for the type itself.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
Just tested it locally and looks like it works for this scenario. I wasn't sure whether the fix should have been done here or just a check in the
BriefMessageto just treat it as the start column or the change in this PR.
Yeah kind of a toss up, realistically any code that accessesIScriptExtent should be way more careful. It tends to assume we're talking aboutInternalScriptExtent but it could be external code.
Co-authored-by: Patrick Meinecke <SeeminglyScience@users.noreply.github.com>
/azp run PowerShell-CI-linux-packaging, PowerShell-Windows-Packaging-CI |
| Azure Pipelines could not run because the pipeline triggers exclude this branch/path. |
ce76ae1 intoPowerShell:masterUh oh!
There was an error while loading.Please reload this page.
Any chance for a backport with this? Seems like a simple fix to include. |
We don't typically backport unless it's a regression or a new high impact scenario is discovered. Is it a blocker for you in something? Note that I know it's a simple code change, but when doing a servicing release even an unexpected false positive on an analyzer could cost a day of release time. That can be especially impactful for the servicing releases that are security related. |
Not a blocker, just didn’t seem like a big change and would have been nice to see in earlier releases. |
Uh oh!
There was an error while loading.Please reload this page.
PR Summary
Fixes the error when PowerShell attempts to print the debug trace line for functions generated with an invalid extent. For example PowerShell creates an extent with a column and line of 0 for things like the default class constructors. This is an issue as the trace debug line always subtracts by 1 to make the value start at 0 rather than 1.
It also adds the DebuggerHidden attribute to the class default constructor to avoid confusing end users from seeing a trace line for something they did not write.Edit: Removed this due to build issues.
PR Context
Fixes:#16874
I can remove the added attribute on the scriptblock and the fix will still be present but I thought it best to keep it there anyway. It doesn't hurt to be present and have that particular scenario skip the tracing altogether.Edit: Was removed due to build issues.
Also Pester code coverage and Profiler uses this tracing so it's commonly seen even if people don't enable the tracing themselves.
PR Checklist
.h,.cpp,.cs,.ps1and.psm1files have the correct copyright header