From 0023dbddc7449ffc56ddeec17372f778edf50f72 Mon Sep 17 00:00:00 2001 From: Alex Soto Date: Wed, 11 Mar 2026 23:54:44 -0400 Subject: [PATCH 1/2] [sharpie] Fix --scope path matching and improve error reporting for large translation units MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit When using `sharpie bind --header --scope `, the --scope argument was stored verbatim without path normalization. Since Clang always reports declaration source locations as absolute paths, a relative --scope value (e.g. `MyFramework.framework/Headers`) would never match the absolute filename from `presumedLoc.FileName`, causing IsInScope() to filter out every declaration and produce zero output files — even though parsing succeeded. Additionally, the StartsWith comparison in IsInScope() could produce false positive matches when one directory name was a prefix of another (e.g. scope `/tmp/scope` would incorrectly match files in `/tmp/scopeextra/`). Finally, when binding a very large number of Objective-C headers (200+), ClangSharp can throw an ArgumentOutOfRangeException with parameter name "handle" during AST traversal, due to cursor/type handle misclassification in its managed wrapper layer. Sharpie caught this exception but reported the raw, opaque message ("Specified argument was out of the range of valid values"), giving users no guidance on how to work around the issue. Changes: 1. Tools.cs: Normalize --scope paths to absolute via Path.GetFullPath() when parsing CLI arguments, matching the behavior already used by --framework mode (which calls Path.GetFullPath on SourceFramework in ResolveFramework()). 2. ObjectiveCBinder.cs (IsInScope): Append a trailing directory separator to scope directory paths before the StartsWith check, so that `/tmp/scope/` does not falsely match `/tmp/scopeextra/`. 3. BindingResult.cs (ReportUnexpectedError): Detect the specific ArgumentOutOfRangeException("handle") pattern from ClangSharp and report an actionable error message that explains the root cause (translation unit too complex) and suggests workarounds (bind fewer headers, use --scope). 4. Tests: Added 5 new test cases: - Scope_RelativePath: verifies relative --scope paths produce correct output after normalization - Scope_FiltersOutOfScopeDeclarations: verifies that only declarations from in-scope headers are bound - Scope_PrefixDoesNotFalseMatch: verifies that scope "/foo/bar" does not match "/foo/barbaz/header.h" - HandleCrash_ReportsUsefulError: verifies the improved error message for ClangSharp handle exceptions - HandleCrash_OtherExceptionsUnchanged: verifies that other exception types still report their original message Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- .../Sharpie.Bind.Tests/ObjectiveCClass.cs | 201 ++++++++++++++++++ tools/sharpie/Sharpie.Bind/BindingResult.cs | 9 +- .../sharpie/Sharpie.Bind/ObjectiveCBinder.cs | 7 +- tools/sharpie/Sharpie.Bind/Tools.cs | 2 +- 4 files changed, 216 insertions(+), 3 deletions(-) diff --git a/tests/sharpie/Sharpie.Bind.Tests/ObjectiveCClass.cs b/tests/sharpie/Sharpie.Bind.Tests/ObjectiveCClass.cs index 3ef19a810e1b..38be3c2d45ae 100644 --- a/tests/sharpie/Sharpie.Bind.Tests/ObjectiveCClass.cs +++ b/tests/sharpie/Sharpie.Bind.Tests/ObjectiveCClass.cs @@ -456,4 +456,205 @@ public struct MyStruct { Assert.That (bindings.AdditionalFiles ["ApiDefinition.cs"].Trim (), Is.EqualTo (expectedApiDefinitionBindings.Trim ()), "Api definition"); Assert.That (bindings.AdditionalFiles ["StructsAndEnums.cs"].Trim (), Is.EqualTo (expectedStructAndEnumsBindings.Trim ()), "Struct and enums"); } + + [Test] + public void Scope_RelativePath () + { + // Verify that --scope with a relative path works correctly + // (the relative path should be resolved to absolute before matching). + var binder = new BindTool (); + var tmpdir = Cache.CreateTemporaryDirectory (); + var subdir = Path.Combine (tmpdir, "headers"); + Directory.CreateDirectory (subdir); + + // Create a header in the scoped directory + var scopedHeader = Path.Combine (subdir, "InScope.h"); + File.WriteAllText (scopedHeader, + """ + @interface InScopeClass { + } + @property int Value; + @end + """); + + // Create an umbrella header that includes both + var mainHeader = Path.Combine (tmpdir, "main.h"); + File.WriteAllText (mainHeader, $"#import \"{scopedHeader}\"\n"); + + binder.SplitDocuments = false; + binder.SourceFile = mainHeader; + binder.OutputDirectory = tmpdir; + + // Use a relative scope path (the bug was that this produced empty output) + var oldCwd = Environment.CurrentDirectory; + try { + Environment.CurrentDirectory = tmpdir; + binder.DirectoriesInScope.Add (Path.GetFullPath ("headers")); + } finally { + Environment.CurrentDirectory = oldCwd; + } + + Configuration.IgnoreIfIgnoredPlatform (binder.Platform); + binder.PlatformAssembly = Extensions.GetPlatformAssemblyPath (binder.Platform); + binder.ClangResourceDirectory = Extensions.GetClangResourceDirectory (); + var bindings = binder.BindInOrOut (); + var expectedBindings = +""" +using Foundation; + +// @interface InScopeClass +interface InScopeClass { + // @property int Value; + [Export ("Value")] + int Value { get; set; } +} + +"""; + bindings.AssertSuccess (expectedBindings); + } + + [Test] + public void Scope_FiltersOutOfScopeDeclarations () + { + // Verify that declarations from headers outside the scope directory are not bound. + var binder = new BindTool (); + var tmpdir = Cache.CreateTemporaryDirectory (); + var scopedDir = Path.Combine (tmpdir, "scoped"); + var unscopedDir = Path.Combine (tmpdir, "unscoped"); + Directory.CreateDirectory (scopedDir); + Directory.CreateDirectory (unscopedDir); + + var scopedHeader = Path.Combine (scopedDir, "Scoped.h"); + File.WriteAllText (scopedHeader, + """ + @interface ScopedClass { + } + @property int A; + @end + """); + + var unscopedHeader = Path.Combine (unscopedDir, "Unscoped.h"); + File.WriteAllText (unscopedHeader, + """ + @interface UnscopedClass { + } + @property int B; + @end + """); + + var mainHeader = Path.Combine (tmpdir, "main.h"); + File.WriteAllText (mainHeader, $"#import \"{scopedHeader}\"\n#import \"{unscopedHeader}\"\n"); + + binder.SplitDocuments = false; + binder.SourceFile = mainHeader; + binder.OutputDirectory = tmpdir; + binder.DirectoriesInScope.Add (scopedDir); + Configuration.IgnoreIfIgnoredPlatform (binder.Platform); + binder.PlatformAssembly = Extensions.GetPlatformAssemblyPath (binder.Platform); + binder.ClangResourceDirectory = Extensions.GetClangResourceDirectory (); + var bindings = binder.BindInOrOut (); + + // Only ScopedClass should be in the output, not UnscopedClass + var expectedBindings = +""" +using Foundation; + +// @interface ScopedClass +interface ScopedClass { + // @property int A; + [Export ("A")] + int A { get; set; } +} + +"""; + bindings.AssertSuccess (expectedBindings); + } + + [Test] + public void Scope_PrefixDoesNotFalseMatch () + { + // Verify that a scope of "/foo/bar" does not match "/foo/barbaz/header.h" + // (the scope must be a proper directory prefix with separator). + var binder = new BindTool (); + var tmpdir = Cache.CreateTemporaryDirectory (); + var scopedDir = Path.Combine (tmpdir, "scope"); + var falseMatchDir = Path.Combine (tmpdir, "scopeextra"); + Directory.CreateDirectory (scopedDir); + Directory.CreateDirectory (falseMatchDir); + + var scopedHeader = Path.Combine (scopedDir, "Good.h"); + File.WriteAllText (scopedHeader, + """ + @interface GoodClass { + } + @property int X; + @end + """); + + var falseMatchHeader = Path.Combine (falseMatchDir, "Bad.h"); + File.WriteAllText (falseMatchHeader, + """ + @interface BadClass { + } + @property int Y; + @end + """); + + var mainHeader = Path.Combine (tmpdir, "main.h"); + File.WriteAllText (mainHeader, $"#import \"{scopedHeader}\"\n#import \"{falseMatchHeader}\"\n"); + + binder.SplitDocuments = false; + binder.SourceFile = mainHeader; + binder.OutputDirectory = tmpdir; + binder.DirectoriesInScope.Add (scopedDir); + Configuration.IgnoreIfIgnoredPlatform (binder.Platform); + binder.PlatformAssembly = Extensions.GetPlatformAssemblyPath (binder.Platform); + binder.ClangResourceDirectory = Extensions.GetClangResourceDirectory (); + var bindings = binder.BindInOrOut (); + + // Only GoodClass should appear (from "scope/"), not BadClass (from "scopeextra/") + var expectedBindings = +""" +using Foundation; + +// @interface GoodClass +interface GoodClass { + // @property int X; + [Export ("X")] + int X { get; set; } +} + +"""; + bindings.AssertSuccess (expectedBindings); + } + + [Test] + public void HandleCrash_ReportsUsefulError () + { + // Verify that when ClangSharp throws an ArgumentOutOfRangeException for a handle, + // the error message is actionable rather than opaque. + var result = new BindingResult (); + var exception = new ArgumentOutOfRangeException ("handle", "some internal value"); + result.ReportUnexpectedError (exception); + + Assert.That (result.Errors.Count, Is.EqualTo (1), "Expected one error"); + var error = result.Errors [0]; + Assert.That (error.Code, Is.EqualTo (0), "Error code"); + Assert.That (error.Message, Does.Contain ("too complex"), "Error should mention complexity"); + Assert.That (error.Message, Does.Contain ("--scope"), "Error should suggest --scope"); + Assert.That (error.Message, Does.Not.Contain ("handle"), "Error should not expose raw parameter name"); + } + + [Test] + public void HandleCrash_OtherExceptionsUnchanged () + { + // Verify that non-handle ArgumentOutOfRangeExceptions still report the original message. + var result = new BindingResult (); + var exception = new InvalidOperationException ("Something went wrong."); + result.ReportUnexpectedError (exception); + + Assert.That (result.Errors.Count, Is.EqualTo (1), "Expected one error"); + var error = result.Errors [0]; + Assert.That (error.Message, Does.Contain ("Something went wrong"), "Should contain original message"); + } } diff --git a/tools/sharpie/Sharpie.Bind/BindingResult.cs b/tools/sharpie/Sharpie.Bind/BindingResult.cs index c4cc7a4153e4..bf9e33f8ee0a 100644 --- a/tools/sharpie/Sharpie.Bind/BindingResult.cs +++ b/tools/sharpie/Sharpie.Bind/BindingResult.cs @@ -66,7 +66,14 @@ public void ReportUnsupportedConstruct (CXSourceLocation? location, string messa public void ReportUnexpectedError (Exception exception) { - ReportError (0 /* An unexpected error occurred: {0}. Please fill a bug report at https://github.com/dotnet/macios/issues. */, exception.Message.TrimEnd ('.')); + if (exception is ArgumentOutOfRangeException aoore && string.Equals (aoore.ParamName, "handle", StringComparison.Ordinal)) { + ReportError (0 /* An unexpected error occurred: {0}. Please fill a bug report at https://github.com/dotnet/macios/issues. */, + "The translation unit is too complex for ClangSharp to process. " + + "This is typically caused by binding a very large number of headers at once. " + + "Try binding fewer headers at a time, or use the --scope option to limit which declarations are bound"); + } else { + ReportError (0 /* An unexpected error occurred: {0}. Please fill a bug report at https://github.com/dotnet/macios/issues. */, exception.Message.TrimEnd ('.')); + } } public void ReportInternalError (CXSourceLocation? location, string message) diff --git a/tools/sharpie/Sharpie.Bind/ObjectiveCBinder.cs b/tools/sharpie/Sharpie.Bind/ObjectiveCBinder.cs index 8406602d1bcf..7e76452e4acd 100644 --- a/tools/sharpie/Sharpie.Bind/ObjectiveCBinder.cs +++ b/tools/sharpie/Sharpie.Bind/ObjectiveCBinder.cs @@ -154,7 +154,12 @@ bool IsInScope (Decl? decl) if (string.IsNullOrEmpty (fn)) return true; foreach (var dir in DirectoriesInScope) { - if (fn.StartsWith (dir, StringComparison.Ordinal)) + // Ensure the scope directory ends with a directory separator so that + // a scope of "/foo/bar" doesn't falsely match "/foo/barbaz/header.h". + var normalizedDir = dir.EndsWith (Path.DirectorySeparatorChar) || dir.EndsWith (Path.AltDirectorySeparatorChar) + ? dir + : dir + Path.DirectorySeparatorChar; + if (fn.StartsWith (normalizedDir, StringComparison.Ordinal)) return true; } diff --git a/tools/sharpie/Sharpie.Bind/Tools.cs b/tools/sharpie/Sharpie.Bind/Tools.cs index 1fd06094bfb4..3381d62a0962 100644 --- a/tools/sharpie/Sharpie.Bind/Tools.cs +++ b/tools/sharpie/Sharpie.Bind/Tools.cs @@ -33,7 +33,7 @@ public static int Bind (string [] arguments) { "s|sdk=", "Target SDK.", v => binder.Sdk = v }, { "f|framework=", "The input framework to bind. Implies setting the scope (--scope) to the framework, setting the namespace (--namespace) to the name of the framework, and no other sources/headers can be specified. If the framework provides an 'Info.plist' with SDK information (DTSDKName), the '-sdk' option will be implied as well (if not manually specified).", v => binder.SourceFramework = v }, { "header=", "The input header file to bind. This can also be a .framework directory.", v => binder.SourceFile = v }, - { "scope=", "Restrict following #include and #import directives declared in header files to within the specified DIR directory.", v => binder.DirectoriesInScope.Add (v) }, + { "scope=", "Restrict following #include and #import directives declared in header files to within the specified DIR directory.", v => binder.DirectoriesInScope.Add (Path.GetFullPath (v)) }, { "c|clang", "All arguments after this argument are not processed by Objective Sharpie and are proxied directly to Clang.", v => { } }, { "clang-resource-dir=", "Specify the Clang resource directory.", v => binder.ClangResourceDirectory = v }, { "platform-assembly=", "Specify the platform assembly to use for binding.", v => binder.PlatformAssembly = v }, From c7143620365d5434d9798a3a3e37a56b5f1415b1 Mon Sep 17 00:00:00 2001 From: Alex Soto Date: Thu, 12 Mar 2026 01:56:53 -0400 Subject: [PATCH 2/2] =?UTF-8?q?Remove=20handle=20crash=20error=20message?= =?UTF-8?q?=20=E2=80=94=20root=20cause=20is=20in=20ClangSharp?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Ref: dotnet/ClangSharp#690 --- .../Sharpie.Bind.Tests/ObjectiveCClass.cs | 30 ------------------- tools/sharpie/Sharpie.Bind/BindingResult.cs | 9 +----- 2 files changed, 1 insertion(+), 38 deletions(-) diff --git a/tests/sharpie/Sharpie.Bind.Tests/ObjectiveCClass.cs b/tests/sharpie/Sharpie.Bind.Tests/ObjectiveCClass.cs index 38be3c2d45ae..06946f8eee59 100644 --- a/tests/sharpie/Sharpie.Bind.Tests/ObjectiveCClass.cs +++ b/tests/sharpie/Sharpie.Bind.Tests/ObjectiveCClass.cs @@ -627,34 +627,4 @@ interface GoodClass { """; bindings.AssertSuccess (expectedBindings); } - - [Test] - public void HandleCrash_ReportsUsefulError () - { - // Verify that when ClangSharp throws an ArgumentOutOfRangeException for a handle, - // the error message is actionable rather than opaque. - var result = new BindingResult (); - var exception = new ArgumentOutOfRangeException ("handle", "some internal value"); - result.ReportUnexpectedError (exception); - - Assert.That (result.Errors.Count, Is.EqualTo (1), "Expected one error"); - var error = result.Errors [0]; - Assert.That (error.Code, Is.EqualTo (0), "Error code"); - Assert.That (error.Message, Does.Contain ("too complex"), "Error should mention complexity"); - Assert.That (error.Message, Does.Contain ("--scope"), "Error should suggest --scope"); - Assert.That (error.Message, Does.Not.Contain ("handle"), "Error should not expose raw parameter name"); - } - - [Test] - public void HandleCrash_OtherExceptionsUnchanged () - { - // Verify that non-handle ArgumentOutOfRangeExceptions still report the original message. - var result = new BindingResult (); - var exception = new InvalidOperationException ("Something went wrong."); - result.ReportUnexpectedError (exception); - - Assert.That (result.Errors.Count, Is.EqualTo (1), "Expected one error"); - var error = result.Errors [0]; - Assert.That (error.Message, Does.Contain ("Something went wrong"), "Should contain original message"); - } } diff --git a/tools/sharpie/Sharpie.Bind/BindingResult.cs b/tools/sharpie/Sharpie.Bind/BindingResult.cs index bf9e33f8ee0a..c4cc7a4153e4 100644 --- a/tools/sharpie/Sharpie.Bind/BindingResult.cs +++ b/tools/sharpie/Sharpie.Bind/BindingResult.cs @@ -66,14 +66,7 @@ public void ReportUnsupportedConstruct (CXSourceLocation? location, string messa public void ReportUnexpectedError (Exception exception) { - if (exception is ArgumentOutOfRangeException aoore && string.Equals (aoore.ParamName, "handle", StringComparison.Ordinal)) { - ReportError (0 /* An unexpected error occurred: {0}. Please fill a bug report at https://github.com/dotnet/macios/issues. */, - "The translation unit is too complex for ClangSharp to process. " + - "This is typically caused by binding a very large number of headers at once. " + - "Try binding fewer headers at a time, or use the --scope option to limit which declarations are bound"); - } else { - ReportError (0 /* An unexpected error occurred: {0}. Please fill a bug report at https://github.com/dotnet/macios/issues. */, exception.Message.TrimEnd ('.')); - } + ReportError (0 /* An unexpected error occurred: {0}. Please fill a bug report at https://github.com/dotnet/macios/issues. */, exception.Message.TrimEnd ('.')); } public void ReportInternalError (CXSourceLocation? location, string message)