Linux64 support #62
Linux64 support #62
Conversation
| interface | ||
|
|
||
| uses | ||
| {$IFDEF MSWINDOWS} |
There was a problem hiding this comment.
perhaps the following pattern would be nicer:
{$IF DEFINED(MSWINDOWS)}
{$ELSEIF DEFINED(POSIX)}
{$MESSAGE ERROR 'Unsupported platform'}
{$ENDIF}
| {External POSIX function declarations} | ||
| function __write(fd: Integer; const buf; count: Integer): Integer; cdecl; external 'libc.so.6' name 'write'; | ||
| function __close(fd: Integer): Integer; cdecl; external 'libc.so.6' name 'close'; | ||
| function mmap(addr: Pointer; len: NativeInt; prot: Integer; flags: Integer; fd: Integer; offset: Integer): Pointer; cdecl; external 'libc.so.6' name 'mmap'; |
There was a problem hiding this comment.
If it is possible to use the Apis as defined in Delphi RTL, as it makes it brittle declaring it here. Using the RTL units should provide correct API binding for each posix platform. This appears to be linux specific.
mmap for example can be used with " Posix.SysMman, Posix.Unistd,", so I'm not sure why these external declarations have been redefined. Is there a good reason?
| {Shows a message box if the program is not showing one already.} | ||
| procedure OS_ShowMessageBox(APText, APCaption: PWideChar); | ||
| begin | ||
| {$IFDEF MSWINDOWS} |
There was a problem hiding this comment.
Is there a reason you want message box events to be output to the console instead? Console events already get output if configured in var FastMM_OutputDebugStringEvents set. My concern is that it is not doing what the procedure says in its title (its using the console instead of a message box).
| {The default name of debug support library.} | ||
| CFastMM_DefaultDebugSupportLibraryName = {$ifndef 64Bit}'FastMM_FullDebugMode.dll'{$else}'FastMM_FullDebugMode64.dll'{$endif}; | ||
|
|
||
| {$IFDEF POSIX} |
There was a problem hiding this comment.
are constants these required if the appropriate RTL Posix units are used?
|
|
||
| type | ||
|
|
||
| {$IFDEF POSIX} |
There was a problem hiding this comment.
Posix.SysTypes declares size_t. Is it needed to be redeclared (along with the rest of these)?
| DebugLibrary_GetRawStackTrace := GetProcAddress(DebugSupportLibraryHandle, PAnsiChar('GetRawStackTrace')); | ||
| DebugLibrary_GetFrameBasedStackTrace := GetProcAddress(DebugSupportLibraryHandle, PAnsiChar('GetFrameBasedStackTrace')); | ||
| DebugLibrary_LogStackTrace_Legacy := GetProcAddress(DebugSupportLibraryHandle, PAnsiChar('LogStackTrace')); | ||
| DebugLibrary_GetRawStackTrace := GetProcAddress(DebugSupportLibraryHandle, ('GetRawStackTrace')); |
There was a problem hiding this comment.
Unlike on Windows which defines LoadLibrary/GetProcAddress in Winapi.Windows, these are defined for posix in System.Sysutils. I suspect this means that these units get initialized before you can install the memory manager, and the default memory manager ends up being effective instead.
Not sure of the impact of dropping PAnsiChar() cast for Windows. (possibly it would cause managed strings to be used which would require a memory manager). (Having said that it does seem to install the memory manager.)
I expect these should be implemented directly.
| {$ELSE} {$IFDEF POSIX} | ||
| System.SysUtils, | ||
| System.Math, | ||
| Posix.Base, |
There was a problem hiding this comment.
System.SysUtils and System.Math should not be used as they will likely cause initialization of the default memory manager.
Posix units probably should be in the implementation section.
There was a problem hiding this comment.
However, it doesn't seem to have prevented installation of the memory manager. Mind you, the debug library hasn't been successfully loaded, as I haven't compiled it.
|
Thanks, I'll review these comments and see what I can update. |
|
I can send you some patches if you're interested, that address some of the comments, that I did at the time, but got side tracked if you're interested. Let me know. |
|
FastMM_FullDebugMode.pas needs work. Externally linked version of assembler could potentially be linked to substitute the inline assembler for POSIX since DCC does not support inline asm on LLVM based compilers. Assembler source file should be provided. procedure GetStackRange(var AStackBaseAddress, ACurrentStackPointer: NativeUInt); implementation required for GetFrameBasedStackTrace |
|
@deadserious I've sent you a pull request for my patches I referred to above, that address the review comments I supplied. (On your fork) |
2 participants
Experimental support for POSIX operating systems