-
Notifications
You must be signed in to change notification settings - Fork 864
Attach DebuggableAttribute to FSI multi-emit manifests under --debug+ #19921
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
base: main
Are you sure you want to change the base?
Changes from all commits
037e067
d8d871b
549a8a4
518a5c7
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -252,3 +252,91 @@ match x with | |
| """ | ||
| |> eval | ||
| |> shouldSucceed | ||
|
|
||
| // https://github.com/dotnet/fsharp/issues/14572 | ||
| // Verify that the per-submission dynamic assembly emitted by FSI in --multiemit+ mode | ||
| // carries a manifest-level DebuggableAttribute when --debug+ is in effect, so that the | ||
| // CLR's JIT does not optimize away locals (which would empty Locals/Autos/Watch in VS). | ||
| module DebuggableAttributeManifest = | ||
|
|
||
| let private reflectionHelperScript = | ||
| """ | ||
| let asm = System.Reflection.Assembly.GetExecutingAssembly() | ||
| asm.GetCustomAttributes(typeof<System.Diagnostics.DebuggableAttribute>, false) | ||
| |> Array.map (fun a -> int (a :?> System.Diagnostics.DebuggableAttribute).DebuggingFlags) | ||
| """ | ||
|
|
||
| let private evalDebuggableFlags (session: FSharpScript) : int[] = | ||
| let result, errors = session.Eval(reflectionHelperScript) | ||
| Assert.Empty(errors) | ||
|
|
||
| match result with | ||
| | Result.Ok(Some v) -> v.ReflectionValue :?> int[] | ||
| | Result.Ok None -> failwith "Expected a value from reflection helper script" | ||
| | Result.Error ex -> raise ex | ||
|
|
||
| let private disableOptimizationsBit = | ||
| int System.Diagnostics.DebuggableAttribute.DebuggingModes.DisableOptimizations | ||
|
|
||
| [<Fact>] | ||
| let ``multi-emit submission with --debug+ has DebuggableAttribute with DisableOptimizations`` () = | ||
| let args: string array = [| "--multiemit+"; "--debug+" |] | ||
| use session = new FSharpScript(additionalArgs = args) | ||
| let flags = evalDebuggableFlags session | ||
|
|
||
| Assert.NotEmpty(flags) | ||
|
|
||
| Assert.True( | ||
| flags |> Array.exists (fun f -> f &&& disableOptimizationsBit <> 0), | ||
| $"Expected at least one DebuggableAttribute with DisableOptimizations bit set on the FSI submission's manifest, but got DebuggingFlags = %A{flags}" | ||
| ) | ||
|
|
||
| [<Fact>] | ||
| let ``multi-emit submission with --debug- has no DebuggableAttribute`` () = | ||
| let args: string array = [| "--multiemit+"; "--debug-" |] | ||
| use session = new FSharpScript(additionalArgs = args) | ||
| let flags = evalDebuggableFlags session | ||
|
|
||
| Assert.Empty(flags) | ||
|
|
||
| [<Fact>] | ||
| let ``single-emit submission with --debug+ keeps DebuggableAttribute (regression)`` () = | ||
| // ilreflect.fs's mkDynamicAssemblyAndModule attaches DebuggableAttribute only when | ||
| // local optimizations are disabled. --optimize- gates that codepath, so include it | ||
| // here to make this a faithful regression test of the existing single-emit behavior. | ||
| let args: string array = [| "--multiemit-"; "--debug+"; "--optimize-" |] | ||
| use session = new FSharpScript(additionalArgs = args) | ||
| let flags = evalDebuggableFlags session | ||
|
|
||
| Assert.NotEmpty(flags) | ||
|
|
||
| Assert.True( | ||
| flags |> Array.exists (fun f -> f &&& disableOptimizationsBit <> 0), | ||
| $"Expected at least one DebuggableAttribute with DisableOptimizations bit set on the single-emit FSI submission's manifest, but got DebuggingFlags = %A{flags}" | ||
| ) | ||
|
|
||
| [<Fact>] | ||
| let ``multi-emit + --debug+ does not duplicate user-declared DebuggableAttribute`` () = | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. In |
||
| let args: string array = [| "--multiemit+"; "--debug+" |] | ||
| use session = new FSharpScript(additionalArgs = args) | ||
|
|
||
| // User declares the attribute themselves in a prior submission. The fix must | ||
| // still emit exactly one DebuggableAttribute on subsequent submissions' manifests | ||
| // (i.e. the auto-attach must not introduce a duplicate when one is already present). | ||
| let userDecl, errors = | ||
| session.Eval( | ||
| """ | ||
| [<assembly: System.Diagnostics.DebuggableAttribute(System.Diagnostics.DebuggableAttribute.DebuggingModes.Default)>] | ||
| do () | ||
| """ | ||
| ) | ||
|
|
||
| Assert.Empty(errors) | ||
|
|
||
| match userDecl with | ||
| | Result.Ok _ -> () | ||
| | Result.Error ex -> raise ex | ||
|
|
||
| let flags = evalDebuggableFlags session | ||
|
|
||
| Assert.Equal(1, flags.Length) | ||
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.
This gates on
--debugand always passesjitOptimizerDisabled = true, which doesn''t match either path cited as the reference. Single-emit (ilreflect.fs) gates onnot LocalOptimizationsEnabled, and the batch compiler (CreateILModule.fs) passesnot JitOptimizationsEnabledrather than a hardcodedtrue. As a result--multiemit+ --debug+ --optimize+disables JIT optimizations here but not in the batch compiler, and--multiemit+ --optimize- --debug-attaches nothing here but does in single-emit. Is the debug-based gating with unconditionalDisableOptimizationsthe intended semantics?