diff --git a/Delegate/MatchConstantDelegate.cs b/Delegate/MatchConstantDelegate.cs index 3361433..dbe17cf 100644 --- a/Delegate/MatchConstantDelegate.cs +++ b/Delegate/MatchConstantDelegate.cs @@ -7,7 +7,7 @@ public delegate bool MatchConstantDelegate ulong address , LinearDisassemblyLine line ); - + internal static partial class NativeDelegates { // bool (*matchCallback)(void* matchCtxt, uint64_t addr, BNLinearDisassemblyLine* line) @@ -17,19 +17,40 @@ public delegate bool MatchConstantDelegate( IntPtr line ); } - + internal static partial class UnsafeUtils { + // Adapts a public MatchConstantDelegate into the native match-callback shape. The BN core + // hands the callback an OWNED BNLinearDisassemblyLine* per match (the official Python + // binding frees it via _LinearDisassemblyLine_convertor -> BNFreeLinearDisassemblyLines). + // The line is fully copied into managed objects by MustFromNativePointer, so freeing the + // native line afterward is safe; the earlier form never freed it and leaked one line per + // match. + internal sealed class MatchConstantContext + { + private readonly MatchConstantDelegate m_callback; + + internal MatchConstantContext(MatchConstantDelegate callback) + { + this.m_callback = callback; + } + + internal bool OnMatch(IntPtr matchCtxt, ulong address, IntPtr line) + { + LinearDisassemblyLine copy = LinearDisassemblyLine.MustFromNativePointer(line); + + NativeMethods.BNFreeLinearDisassemblyLines(line, 1); + + return this.m_callback(address, copy); + } + } + internal static NativeDelegates.MatchConstantDelegate WrapMatchConstantDelegate( MatchConstantDelegate callback) { - return bool (matchCtxt , address , line) => - { - return callback ( - address , - LinearDisassemblyLine.MustFromNativePointer(line) - ); - }; + MatchConstantContext context = new MatchConstantContext(callback); + + return new NativeDelegates.MatchConstantDelegate(context.OnMatch); } } } diff --git a/Delegate/MatchDataDelegate.cs b/Delegate/MatchDataDelegate.cs index 1db93c2..abf12ee 100644 --- a/Delegate/MatchDataDelegate.cs +++ b/Delegate/MatchDataDelegate.cs @@ -7,7 +7,7 @@ public delegate bool MatchDataDelegate ulong address , byte[] data ); - + internal static partial class NativeDelegates { // bool (*matchCallback)(void* matchCtxt, uint64_t addr, BNDataBuffer* match) @@ -17,19 +17,38 @@ public delegate bool MatchDataDelegate( IntPtr match ); } - + internal static partial class UnsafeUtils { + // Adapts a public MatchDataDelegate into the native match-callback shape. The BN core hands + // the callback an OWNED BNDataBuffer per match (the official Python binding adopts it as + // DataBuffer(handle=match) and frees it in __del__ via BNFreeDataBuffer). The buffer is + // therefore taken (owned) and freed deterministically with `using`, never borrowed; the + // earlier borrow form leaked one buffer per match. + internal sealed class MatchDataContext + { + private readonly MatchDataDelegate m_callback; + + internal MatchDataContext(MatchDataDelegate callback) + { + this.m_callback = callback; + } + + internal bool OnMatch(IntPtr matchCtxt, ulong address, IntPtr match) + { + using (DataBuffer owned = DataBuffer.MustTakeHandle(match)) + { + return this.m_callback(address, owned.Contents); + } + } + } + internal static NativeDelegates.MatchDataDelegate WrapMatchDataDelegate( MatchDataDelegate callback) { - return bool (matchCtxt , address , buffer) => - { - return callback( - address , - DataBuffer.MustBorrowHandle(buffer).Contents - ); - }; + MatchDataContext context = new MatchDataContext(callback); + + return new NativeDelegates.MatchDataDelegate(context.OnMatch); } } } diff --git a/Delegate/MatchTextDelegate.cs b/Delegate/MatchTextDelegate.cs index 2883718..907166a 100644 --- a/Delegate/MatchTextDelegate.cs +++ b/Delegate/MatchTextDelegate.cs @@ -8,7 +8,7 @@ public delegate bool MatchTextDelegate string text, LinearDisassemblyLine line ); - + internal static partial class NativeDelegates { // bool (*matchCallback)(void* matchCtxt, uint64_t addr, const char* match, BNLinearDisassemblyLine* line) @@ -19,20 +19,44 @@ public delegate bool MatchTextDelegate( IntPtr line ); } - + internal static partial class UnsafeUtils { - internal static NativeDelegates.MatchTextDelegate WrapMatchTextDelegate( - MatchTextDelegate callback) + // Adapts a public MatchTextDelegate into the native match-callback shape. The BN core hands + // the callback an OWNED BNLinearDisassemblyLine* per match (the official Python binding + // frees it via _LinearDisassemblyLine_convertor -> BNFreeLinearDisassemblyLines). The line + // is fully copied into managed objects by MustFromNativePointer, so freeing the native line + // afterward is safe; the earlier form never freed it and leaked one line per match. The + // matched text is a borrowed const char* (core-owned) and is only read, never freed. + internal sealed class MatchTextContext { - return bool (matchCtxt , address , text , line) => + private readonly MatchTextDelegate m_callback; + + internal MatchTextContext(MatchTextDelegate callback) + { + this.m_callback = callback; + } + + internal bool OnMatch(IntPtr matchCtxt, ulong address, IntPtr text, IntPtr line) { - return callback ( - address , + LinearDisassemblyLine copy = LinearDisassemblyLine.MustFromNativePointer(line); + + NativeMethods.BNFreeLinearDisassemblyLines(line, 1); + + return this.m_callback( + address , UnsafeUtils.ReadUtf8String(text), - LinearDisassemblyLine.MustFromNativePointer(line) + copy ); - }; + } + } + + internal static NativeDelegates.MatchTextDelegate WrapMatchTextDelegate( + MatchTextDelegate callback) + { + MatchTextContext context = new MatchTextContext(callback); + + return new NativeDelegates.MatchTextDelegate(context.OnMatch); } } } diff --git a/Function/BNCreateFunctionGraph.cs b/Function/BNCreateFunctionGraph.cs index 5c3ff0a..869943e 100644 --- a/Function/BNCreateFunctionGraph.cs +++ b/Function/BNCreateFunctionGraph.cs @@ -21,7 +21,7 @@ internal static extern IntPtr BNCreateFunctionGraph( IntPtr func , // BNFunctionViewType type - in BNFunctionViewType type , + BNFunctionViewType type , // BNDisassemblySettings* settings IntPtr settings diff --git a/Function/BNCreateImmediateFunctionGraph.cs b/Function/BNCreateImmediateFunctionGraph.cs index c0ffa57..b35cdac 100644 --- a/Function/BNCreateImmediateFunctionGraph.cs +++ b/Function/BNCreateImmediateFunctionGraph.cs @@ -21,7 +21,7 @@ internal static extern IntPtr BNCreateImmediateFunctionGraph( IntPtr func , // BNFunctionViewType type - in BNFunctionViewType type , + BNFunctionViewType type , // BNDisassemblySettings* settings IntPtr settings diff --git a/Function/BNFindAllConstantWithProgress.cs b/Function/BNFindAllConstantWithProgress.cs index 4fa63f1..600488d 100644 --- a/Function/BNFindAllConstantWithProgress.cs +++ b/Function/BNFindAllConstantWithProgress.cs @@ -34,7 +34,7 @@ internal static extern bool BNFindAllConstantWithProgress( IntPtr settings , // BNFunctionViewType viewType - in BNFunctionViewType viewType , + BNFunctionViewType viewType , // void* ctxt IntPtr ctxt , diff --git a/Function/BNFindAllTextWithProgress.cs b/Function/BNFindAllTextWithProgress.cs index 7c02fb9..9fff5ec 100644 --- a/Function/BNFindAllTextWithProgress.cs +++ b/Function/BNFindAllTextWithProgress.cs @@ -38,7 +38,7 @@ internal static extern bool BNFindAllTextWithProgress( FindFlag flags , // BNFunctionViewType viewType - in BNFunctionViewType viewType , + BNFunctionViewType viewType , // void* ctxt IntPtr ctxt , diff --git a/Function/BNFindNextConstant.cs b/Function/BNFindNextConstant.cs index d076d17..c4a3c7f 100644 --- a/Function/BNFindNextConstant.cs +++ b/Function/BNFindNextConstant.cs @@ -34,7 +34,7 @@ internal static extern bool BNFindNextConstant( IntPtr settings , // BNFunctionViewType viewType - in BNFunctionViewType viewType + BNFunctionViewType viewType ); } } \ No newline at end of file diff --git a/Function/BNFindNextConstantWithProgress.cs b/Function/BNFindNextConstantWithProgress.cs index ad3cca3..9b3d138 100644 --- a/Function/BNFindNextConstantWithProgress.cs +++ b/Function/BNFindNextConstantWithProgress.cs @@ -37,7 +37,7 @@ internal static extern bool BNFindNextConstantWithProgress( IntPtr settings , // BNFunctionViewType viewType - in BNFunctionViewType viewType , + BNFunctionViewType viewType , // void* ctxt IntPtr ctxt , diff --git a/Function/BNFindNextText.cs b/Function/BNFindNextText.cs index f09ae41..44829ff 100644 --- a/Function/BNFindNextText.cs +++ b/Function/BNFindNextText.cs @@ -38,7 +38,7 @@ internal static extern bool BNFindNextText( FindFlag flags , // BNFunctionViewType viewType - in BNFunctionViewType viewType + BNFunctionViewType viewType ); } } \ No newline at end of file diff --git a/Function/BNFindNextTextWithProgress.cs b/Function/BNFindNextTextWithProgress.cs index 49caa2a..05a1709 100644 --- a/Function/BNFindNextTextWithProgress.cs +++ b/Function/BNFindNextTextWithProgress.cs @@ -41,7 +41,7 @@ internal static extern bool BNFindNextTextWithProgress( FindFlag flags , // BNFunctionViewType viewType - in BNFunctionViewType viewType , + BNFunctionViewType viewType , // void* ctxt IntPtr ctxt , diff --git a/Handle/BNBinaryView.cs b/Handle/BNBinaryView.cs index 5631006..6476982 100644 --- a/Handle/BNBinaryView.cs +++ b/Handle/BNBinaryView.cs @@ -4765,6 +4765,12 @@ public bool FindAllData( NativeDelegates.BNProgressFunction? progressWrapper = null == progress ? null : UnsafeUtils.WrapProgressDelegate(progress); + // The match thunk is handed to the core as a raw function pointer and must survive the + // whole synchronous scan; without KeepAlive a GC during the call could free it and + // crash the native callback. The thunk takes (owns) each matched BNDataBuffer. + NativeDelegates.MatchDataDelegate matchWrapper = + UnsafeUtils.WrapMatchDataDelegate(match); + bool result = NativeMethods.BNFindAllDataWithProgress( this.handle , start , @@ -4776,12 +4782,11 @@ public bool FindAllData( ? IntPtr.Zero : Marshal.GetFunctionPointerForDelegate(progressWrapper) , IntPtr.Zero , - Marshal.GetFunctionPointerForDelegate( - UnsafeUtils.WrapMatchDataDelegate(match) - ) + Marshal.GetFunctionPointerForDelegate(matchWrapper) ); GC.KeepAlive(progressWrapper); + GC.KeepAlive(matchWrapper); return result; } @@ -4791,39 +4796,67 @@ public bool FindAllText( string data , ulong start , ulong end , - MatchDataDelegate match , + MatchTextDelegate match , FunctionViewType viewType , ProgressDelegate? progress = null , FindFlag flags = FindFlag.FindCaseSensitive , DisassemblySettings? settings = null ) { - using (ScopedAllocator allocator = new ScopedAllocator()) - { - NativeDelegates.BNProgressFunction? progressWrapper = - null == progress ? null : UnsafeUtils.WrapProgressDelegate(progress); + // The core dereferences the settings pointer while rendering text, so a null settings + // crashes the process. The official Python binding builds a default DisassemblySettings + // when none is supplied; do the same and dispose only the one allocated here, never a + // caller-owned instance. + bool ownsSettings = null == settings; + DisassemblySettings effectiveSettings = + ownsSettings ? new DisassemblySettings() : settings!; - bool result = NativeMethods.BNFindAllTextWithProgress( - this.handle , - start , - end , - data , - null == settings ? IntPtr.Zero : settings.DangerousGetHandle() , - flags , - viewType.ToNativeEx(allocator) , - IntPtr.Zero , - null == progressWrapper - ? IntPtr.Zero - : Marshal.GetFunctionPointerForDelegate(progressWrapper) , - IntPtr.Zero , - Marshal.GetFunctionPointerForDelegate( - UnsafeUtils.WrapMatchDataDelegate(match) - ) - ); + try + { + using (ScopedAllocator allocator = new ScopedAllocator()) + { + NativeDelegates.BNProgressFunction? progressWrapper = + null == progress ? null : UnsafeUtils.WrapProgressDelegate(progress); + + // BNFindAllTextWithProgress invokes a 4-argument match callback + // (ctxt, addr, const char* text, BNLinearDisassemblyLine*); the public + // MatchTextDelegate mirrors that shape. Passing MatchDataDelegate here + // reinterpreted the text pointer as a BNDataBuffer handle and crashed on the + // first match. + NativeDelegates.MatchTextDelegate matchWrapper = + UnsafeUtils.WrapMatchTextDelegate(match); + + bool result = NativeMethods.BNFindAllTextWithProgress( + this.handle , + start , + end , + data , + effectiveSettings.DangerousGetHandle() , + flags , + viewType.ToNativeEx(allocator) , + IntPtr.Zero , + null == progressWrapper + ? IntPtr.Zero + : Marshal.GetFunctionPointerForDelegate(progressWrapper) , + IntPtr.Zero , + Marshal.GetFunctionPointerForDelegate(matchWrapper) + ); - GC.KeepAlive(progressWrapper); + // The match thunk is handed to the core as a raw function pointer and must + // survive the whole synchronous scan; without KeepAlive a GC during the call + // could free it and crash the native callback. + GC.KeepAlive(progressWrapper); + GC.KeepAlive(matchWrapper); - return result; + return result; + } + } + finally + { + if (ownsSettings) + { + effectiveSettings.Dispose(); + } } } @@ -4831,37 +4864,64 @@ public bool FindAllConstant( ulong data , ulong start , ulong end , - MatchDataDelegate match , + MatchConstantDelegate match , FunctionViewType viewType , ProgressDelegate? progress = null , DisassemblySettings? settings = null ) { - using (ScopedAllocator allocator = new ScopedAllocator()) + // The core dereferences the settings pointer while rendering instructions, so a null + // settings crashes the process. The official Python binding builds a default + // DisassemblySettings when none is supplied; do the same and dispose only the one + // allocated here, never a caller-owned instance. + bool ownsSettings = null == settings; + DisassemblySettings effectiveSettings = + ownsSettings ? new DisassemblySettings() : settings!; + + try { - NativeDelegates.BNProgressFunction? progressWrapper = - null == progress ? null : UnsafeUtils.WrapProgressDelegate(progress); + using (ScopedAllocator allocator = new ScopedAllocator()) + { + NativeDelegates.BNProgressFunction? progressWrapper = + null == progress ? null : UnsafeUtils.WrapProgressDelegate(progress); - bool result = NativeMethods.BNFindAllConstantWithProgress( - this.handle , - start , - end , - data , - null == settings ? IntPtr.Zero : settings.DangerousGetHandle() , - viewType.ToNativeEx(allocator) , - IntPtr.Zero , - null == progressWrapper - ? IntPtr.Zero - : Marshal.GetFunctionPointerForDelegate(progressWrapper) , - IntPtr.Zero , - Marshal.GetFunctionPointerForDelegate( - UnsafeUtils.WrapMatchDataDelegate(match) - ) - ); + // BNFindAllConstantWithProgress invokes a 3-argument match callback + // (ctxt, addr, BNLinearDisassemblyLine*); the public MatchConstantDelegate + // mirrors that shape. Passing MatchDataDelegate here fed the line pointer to + // the data-buffer thunk and crashed on the first match. + NativeDelegates.MatchConstantDelegate matchWrapper = + UnsafeUtils.WrapMatchConstantDelegate(match); - GC.KeepAlive(progressWrapper); + bool result = NativeMethods.BNFindAllConstantWithProgress( + this.handle , + start , + end , + data , + effectiveSettings.DangerousGetHandle() , + viewType.ToNativeEx(allocator) , + IntPtr.Zero , + null == progressWrapper + ? IntPtr.Zero + : Marshal.GetFunctionPointerForDelegate(progressWrapper) , + IntPtr.Zero , + Marshal.GetFunctionPointerForDelegate(matchWrapper) + ); - return result; + // The match thunk is handed to the core as a raw function pointer and must + // survive the whole synchronous scan; without KeepAlive a GC during the call + // could free it and crash the native callback. + GC.KeepAlive(progressWrapper); + GC.KeepAlive(matchWrapper); + + return result; + } + } + finally + { + if (ownsSettings) + { + effectiveSettings.Dispose(); + } } } @@ -4876,6 +4936,12 @@ public bool Search( NativeDelegates.BNProgressFunction? progressWrapper = null == progress ? null : UnsafeUtils.WrapProgressDelegate(progress); + // The match thunk is handed to the core as a raw function pointer and must survive + // the whole synchronous search; without KeepAlive a GC during the call could free it + // and crash the native callback. The thunk takes (owns) each matched BNDataBuffer. + NativeDelegates.MatchDataDelegate matchWrapper = + UnsafeUtils.WrapMatchDataDelegate(match); + bool result = NativeMethods.BNSearch( this.handle , query , @@ -4884,12 +4950,11 @@ public bool Search( ? IntPtr.Zero : Marshal.GetFunctionPointerForDelegate(progressWrapper) , IntPtr.Zero , - Marshal.GetFunctionPointerForDelegate( - UnsafeUtils.WrapMatchDataDelegate(match) - ) + Marshal.GetFunctionPointerForDelegate(matchWrapper) ); GC.KeepAlive(progressWrapper); + GC.KeepAlive(matchWrapper); return result; }