Skip to content

Improve dotnet-pinvoke skill: stop signals, routing, inline anti-patterns #72

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.

Sign up for GitHub

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

Merged
lewing merged 2 commits into from
Mar 4, 2026
Merged

Improve dotnet-pinvoke skill: stop signals, routing, inline anti-patterns #72

Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
c94318d
Improve dotnet-pinvoke skill: add stop signals, When to Use, inline a…
lewing Feb 20, 2026
662a0d1
fix: use YAML block scalar for description to fix frontmatter parsing
lewing Feb 21, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
35 changes: 34 additions & 1 deletion plugins/dotnet/skills/dotnet-pinvoke/SKILL.md
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,14 @@
---
name: dotnet-pinvoke
description: Correctly call native (C/C++) libraries from .NET using P/Invoke and LibraryImport. Covers function signatures, string marshalling, memory lifetime, SafeHandle, and cross-platform patterns. Use when (1) writing new P/Invoke or LibraryImport declarations, (2) reviewing or debugging existing native interop code, (3) wrapping a C or C++ library for use in .NET, or (4) diagnosing crashes, memory leaks, or corruption at the managed/native boundary. Do not use for COM interop, C++/CLI mixed-mode assemblies, or pure managed code with no native dependencies.
description: >
Correctly call native (C/C++) libraries from .NET using P/Invoke and LibraryImport.
Covers function signatures, string marshalling, memory lifetime, SafeHandle, and
cross-platform patterns.
USE FOR: writing new P/Invoke or LibraryImport declarations, reviewing or debugging
existing native interop code, wrapping a C or C++ library for use in .NET, diagnosing
crashes, memory leaks, or corruption at the managed/native boundary.
DO NOT USE FOR: COM interop, C++/CLI mixed-mode assemblies, or pure managed code with
no native dependencies.
---

# .NET P/Invoke
Expand All @@ -9,6 +17,23 @@ Calling native code from .NET is powerful but unforgiving. Incorrect signatures,

This skill covers both `DllImport` (available since .NET Framework 1.0) and `LibraryImport` (source-generated, .NET 7+). When targeting .NET Framework, always use `DllImport`. When targeting .NET 7+, prefer `LibraryImport` for new code. When native AOT is a requirement, `LibraryImport` is the only option.

## When to Use This Skill

@AaronRobinsonMSFT AaronRobinsonMSFT Feb 20, 2026

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ugh. I have no idea why this was removed. I think when I asked copilot to make the skill less than 500 lines it removed this. This should have been there. Apologizes @danmoseley I didn't realize you were calling out this wasn't present at all. That is on me :(


- Writing a new `[DllImport]` or `[LibraryImport]` declaration from a C/C++ header
- Reviewing P/Invoke signatures for correctness (type sizes, calling conventions, string encoding)
- Wrapping an entire C library for use from .NET
- Debugging `AccessViolationException`, `DllNotFoundException`, or silent data corruption at the native boundary
- Migrating `DllImport` declarations to `LibraryImport` for AOT/trimming compatibility
- Diagnosing memory leaks or heap corruption involving native handles or buffers

## Stop Signals

- **Single function?** Map the signature (Steps 1-3), handle strings/memory only if relevant, skip tooling and migration sections.
- **Don't migrate** existing `DllImport` to `LibraryImport` unless the user asks or AOT/trimming is an explicit requirement.
- **Don't recommend CsWin32** unless the target is specifically Win32 APIs.
- **Don't generate callbacks** (Step 8) unless the native API requires function pointers.
- **Review request?** Use the validation checklist — don't rewrite working code.

## Inputs

| Input | Required | Description |
Expand Down Expand Up @@ -50,6 +75,10 @@ The most dangerous mappings — these cause the majority of bugs:

**For the complete type mapping table, struct layout, and blittable type rules**, see [references/type-mapping.md](references/type-mapping.md).

> ❌ **NEVER** use `int` or `long` for C `long` — it's 32-bit on Windows, 64-bit on Unix. Always use `CLong`.

Copilot AI Feb 28, 2026

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The guidance about C long size is inaccurate/misleading: it’s 32-bit on Windows and typically 64-bit only on 64-bit Unix (LP64); on 32-bit Unix it’s also 32-bit. Also, saying to “always use CLong” conflicts with the earlier note that this skill covers .NET Framework (which doesn’t have CLong). Consider rewording to reflect platform/TMF constraints (e.g., use CLong/CULong when available, otherwise use conditional compilation/custom typedef wrappers).

Copilot uses AI. Check for mistakes.

@AaronRobinsonMSFT AaronRobinsonMSFT Mar 2, 2026

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This feedback is correct in the sense the it is implying long is always 32-bit. Which of course isn't the case and copilot is correct. The CLong type though will always do the right thing for whenever C long is used. I'm fine adding clarity here or ignoring this feedback.

> ❌ **NEVER** use `ulong` for `size_t` — causes stack corruption on 32-bit. Use `nuint` or `UIntPtr`.
> ❌ **NEVER** use `bool` without `MarshalAs` — the default marshal size is wrong.
Comment on lines +78 to +80

@AaronRobinsonMSFT AaronRobinsonMSFT Feb 20, 2026

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

All of these are captured in the type-mapping.md file. I don't think they should be here.

@lewing lewing Feb 21, 2026 •
edited
Loading

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

it is in the assessment https://gist.github.com/lewing/859ab56b0c37601804c03a5c601cfd8d
image
and the pr description. It's a pattern that works but isn't a hard rule.

@AaronRobinsonMSFT AaronRobinsonMSFT Feb 22, 2026

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I get that it is in the assessment, but what is the source of that guidance from the assessment? How do we know this is good guidance? I've been following the guidelines on anthropic and I've not seen this called out.

@lewing lewing Feb 23, 2026

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

dotnet/runtime#124734

@lewing lewing Feb 23, 2026 •
edited
Loading

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I get that it is in the assessment, but what is the source of that guidance from the assessment? How do we know this is good guidance? I've been following the guidelines on anthropic and I've not seen this called out.

Did you read the links I shared previously? https://github.com/lewing/agent-plugins/blob/main/plugins/skill-trainer/skills/skill-trainer-knowledge/references/skill-builder-knowledge.md#burying-critical-rules-as-numbered-workflow-steps I've found it to work multiple times in testing some of the weaker models. It's a style suggestion not a requirement. Do as you please.

dotnet/runtime#124734

Copilot AI Feb 28, 2026

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

“NEVER use bool without MarshalAs — the default marshal size is wrong” is too absolute and can be incorrect: the default bool marshalling matches Win32 BOOL (4 bytes), while it’s C99 _Bool/bool that requires [MarshalAs(UnmanagedType.U1)]. Suggest rephrasing to distinguish Win32 BOOL vs C99 bool, and to emphasize explicitly confirming the native boolean representation rather than banning bool outright.

Suggested change
> ❌ **NEVER** use `bool` without `MarshalAs` — the default marshal size is wrong.
> ⚠️ Do not assume the default `bool` marshalling matches native — confirm the native boolean type. For Win32 `BOOL`, the default is correct; for C99 `bool` / `_Bool`, use `[MarshalAs(UnmanagedType.U1)] bool`.

Copilot uses AI. Check for mistakes.

@AaronRobinsonMSFT AaronRobinsonMSFT Mar 2, 2026

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

while it’s C99 _Bool/bool that requires [MarshalAs(UnmanagedType.U1)]

Minor pedantic push back. The C standard says nothing about the size of _Bool or bool. It is always 1 byte as far as I've found, but it doesn't need to be. I think the suggest feedback by copilot is fine, but the point of the NEVER was to imply the nuance the suggested comment is expressing.


### Step 3: Write the Declaration

Given a C header:
Expand Down Expand Up @@ -109,6 +138,8 @@ internal static partial int ProcessRecords(
4. **Specify encoding explicitly.** Never rely on `CharSet.Auto`.
5. **Never introduce `StringBuilder` for output buffers.**

> ❌ **NEVER** rely on `CharSet.Auto` or omit string encoding — there is no safe default.

@AaronRobinsonMSFT AaronRobinsonMSFT Feb 20, 2026

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This should have been captured in the type-mapping.md file. I'm really confused on this right now.


```csharp
// DllImport — Windows API (UTF-16)
[DllImport("kernel32.dll", CharSet = CharSet.Unicode, SetLastError = true)]
Expand Down Expand Up @@ -137,6 +168,8 @@ internal static partial int SetName(string name);

When memory crosses the boundary, exactly one side must own it — and both sides must agree.

> ❌ **NEVER** free with a mismatched allocator — `Marshal.FreeHGlobal` on `malloc`'d memory is heap corruption.

@AaronRobinsonMSFT AaronRobinsonMSFT Feb 20, 2026

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ugh. This was another item that was in the original proposal. I'm very confused on what happend here.


**Model 1 — Caller allocates, caller frees (safest):**

```csharp
Expand Down