Merge pull request #105 from jayrm/mangling

Name mangling on win target

#890: Fix Bad @n stdcall suffix for non-trivial byval type parameters with -gen gcc

Changes in this pull request are based on, and related to:
#828 32bit Integer/Long mangling reversed

By default, fbc's Long/ULong data type maps to c's int type, and since Integer/Uinteger are meant to be (consistently) 64bits in fbc-64bit, there is no other type that (by default) that can map to c's long int, which is 32bit even on Win64.

So, in Win64, to allow calling an external library that needs a 'long int' parameter there is the following, non-portable mangling modifier. It is ignored for all targets except Win64.

map fbc's 32bit 'LONG' type to Win64's 32bit 'long' type
type cxxlongint as long alias "long"

Author: jayrm

changelog.txt

@@ -1,6 +1,7 @@
 Version 1.06.0
 
 [changed]
+- Name mangling of Integer/Long parameters was reversed on 32bit to match 64bit, so the same FB and C++ code will be compatible on both 32bit and 64bit. Integer is mangled as C++ long (except on Win64), Long is mangled as C++ int.
 - Adjusted warning text for "mixed bool/nonbool operands" warning
 - test-suite uses libfbcunit for unit testing framework
 - SELECT CASE AS CONST respects data type and will show overflow warnings on out-of-range constants
@@ -9,6 +10,7 @@ Version 1.06.0
 - WSTRING can be a return type, but only for prototypes (DECLARE) and function pointers, this allows getting PROCPTR() of all fb built-in run time functions
 
 [added]
+- Name mangling of Long parameters on Win64 are mangled to C++ int by default and to C++ long with a modifier in the form 'as [u]long alias "long"'.  The data type size is still 32bit but allows calling external C++ library expecting a C++ long.
 - -noobjinfo option to disable the writing/reading of compile-time library and other linking options from/to .o and .a files. This also disables the use of fbextra.x (the supplemental linker script) for discarding the .fbctinf sections, which is useful when using the gold linker that doesn't support this kind of linker script.
 - Linux console Inkey() now recognizes F11 and F12
 - 2D render through OpenGL on Windows and Linux via screencontrol (angros47)
@@ -69,6 +71,7 @@ Version 1.06.0
 - #822, #814, #842: Compiler crash when initializing static/shared reference (DIM SHARED/STATIC BYREF) with non-constant initializer (e.g. another reference, or dynamic array element)
 - ASM backend: Fix bad code generated for comparisons such as IF @globalvar = 0 THEN
 - #884: FORMATing dates can deadlock under Unix
+- #890: Fix Bad @N stdcall suffix for non-trivial byval type parameters with -gen gcc
 - #642, #886: CASTs involving CONST qualifiers solved out too early allowing invalid statements to be compiled
 - #801: *@(expr) solved to (expr) did not cleanly remove null ptr checks allowing invalid datatype assignment with -exx.  *PTRCHK(@expr) solves to (expr).
 - #880: Overload binary operators now support covariant arguments, overloaded procedure resolution changed especially with respect to CONST and non-CONST parameters

src/compiler/ir-hlc.bas

@@ -223,8 +223,8 @@ declare sub _emitDBG _
 	( _
 		byval op as integer, _
 		byval proc as FBSYMBOL ptr, _
-		byval lnum as integer, _
-		ByVal filename As zstring ptr = 0 _
+		byval lnum as integer, _
+		ByVal filename As zstring ptr = 0 _
 	)
 
 declare sub exprFreeNode( byval n as EXPRNODE ptr )
@@ -505,11 +505,13 @@ private function hNeedAlias( byval proc as FBSYMBOL ptr ) as integer
 		function = TRUE
 
 	'' For stdcall with @N suffix, if the function has a hidden UDT result
-	'' pointer parameter, we need the alias to get the correct @N suffix.
-	'' (gcc would calculate the parameter into the @N suffix, since it
-	'' doesn't know that the parameter is the special result parameter)
+	'' pointer parameter, or UDT's passed byval, we need the alias to get
+	'' the correct @N suffix. (gcc could calculate the wrong value into the
+	'' @N suffix, since it doesn't known about the special result parameter
+	'' or byval UDTs).  It should be safe to always generate the alias
+	'' ourselves since we already must control for the special cases.
 	case FB_FUNCMODE_STDCALL
-		function = symbProcReturnsOnStack( proc )
+		function = TRUE
 	end select
 end function
 
@@ -3159,15 +3161,15 @@ private sub _emitDBG _
 	( _
 		byval op as integer, _
 		byval proc as FBSYMBOL ptr, _
-		byval lnum as integer, _
-		ByVal filename As zstring ptr _
+		byval lnum as integer, _
+		ByVal filename As zstring ptr _
 	)
 
 	if( op = AST_OP_DBG_LINEINI ) then
-		ctx.linenum = lnum
+		ctx.linenum = lnum
 		if( filename <> NULL ) then
 			hUpdateCurrentFileName( filename )
-		end if
+		end if
 	end if
 
 end sub

src/compiler/parser-decl-symbtype.bas

@@ -393,6 +393,48 @@ function hIntegerTypeFromBitSize _
 
 end function
 
+private function cMangleModifier _
+	( _
+	) as integer
+
+	function = FALSE
+
+	'' currently, cMangleModifier() returns true/false, because
+	'' we are checking for one case only.  And the FB_DATATYPE
+	'' stored in the upper bits of the dtype in FB_DT_MANGLEMASK
+	'' was selected for convenience.  In future, it might be
+	'' better to have a new FB_MANGLE_DATATYPE enum for tracking
+	'' the desired mapping stored in the upper bits of the
+	'' dtype, especially if there are some mappings that can't
+	'' be found in existing FB_DATATYPE.  To support additional
+	'' checks and mappings, should probably pass a dtype
+	'' parameter in to this procedure to be updated or returned.
+
+	'' ALIAS?
+	if( lexGetToken( ) = FB_TK_ALIAS ) then
+		lexSkipToken( )
+
+		'' "long"?
+		if( lexGetClass( ) = FB_TKCLASS_STRLITERAL ) then
+			select case lcase( *lexGetText( ) )
+			case "long"
+				'' only Win64 is affected by ths modifer
+				function = fbIs64bit( ) and ((env.target.options and FB_TARGETOPT_UNIX) = 0)
+			case ""
+				errReport( FB_ERRMSG_EMPTYALIASSTRING )
+			case else
+				errReport( FB_ERRMSG_SYNTAXERROR )	
+			end select
+
+			lexSkipToken( )
+		else
+			errReport( FB_ERRMSG_SYNTAXERROR )
+		end if
+
+	end if
+
+end function
+
 '':::::
 ''SymbolType      =   CONST? UNSIGNED? (
 ''				      ANY
@@ -528,10 +570,16 @@ function cSymbolType _
 		case FB_TK_LONG
 			lexSkipToken( )
 			dtype = FB_DATATYPE_LONG
+			if( cMangleModifier() ) then
+				dtype = typeSetMangleDt( dtype, FB_DATATYPE_INTEGER )
+			end if
 
 		case FB_TK_ULONG
 			lexSkipToken( )
 			dtype = FB_DATATYPE_ULONG
+			if( cMangleModifier() ) then
+				dtype = typeSetMangleDt( dtype, FB_DATATYPE_UINT )
+			end if
 
 		case FB_TK_LONGINT
 			lexSkipToken( )
@@ -668,7 +716,11 @@ function cSymbolType _
 				dtype = FB_DATATYPE_UINT
 
 			case FB_DATATYPE_LONG
-				dtype = FB_DATATYPE_ULONG
+				if( typeHasMangleDt( dtype ) ) then
+					dtype = typeSetMangleDt( FB_DATATYPE_ULONG, FB_DATATYPE_UINT )
+				else
+					dtype = FB_DATATYPE_ULONG
+				end if
 
 			case FB_DATATYPE_LONGINT
 				dtype = FB_DATATYPE_ULONGINT

src/compiler/symb-mangling.bas

@@ -319,7 +319,7 @@ function hMangleBuiltInType _
 		byref add_abbrev as integer _
 	) as zstring ptr
 
-	assert( dtype = typeGetDtOnly( dtype ) )
+	assert( dtype = (typeGetDtOnly( dtype ) or (dtype and FB_DT_MANGLEMASK)) )
 
 	''
 	'' Plain unqualified C++ built-in types are not considered for abbreviation.
@@ -336,74 +336,81 @@ function hMangleBuiltInType _
 		return @"8FBSTRING"
 	end if
 
-	if( fbIs64bit( ) ) then
-		'' By default on x86 we mangle INTEGER to "int", but on 64bit
-		'' our INTEGER becomes 64bit, while int stays 32bit, so we
-		'' really shouldn't use the same mangling in that case.
-		''
-		'' Mangling the 64bit INTEGER as "long long" would conflict
-		'' with the LONGINT mangling though (we cannot allow separate
-		'' INTEGER/LONGINT overloads in code but then generate the same
-		'' mangled id for them, the assembler/linker would complain).
-		''
-		'' Besides that, our LONG stays 32bit always, but "long" on
-		'' 64bit Linux changes to 64bit, so we shouldn't mangle LONG
-		'' to "long" in that case. It would still be possible on 64bit
-		'' Windows, because there "long" stays 32bit, but it seems best
-		'' to mangle LONG to "int" on 64bit consistently, since "int"
-		'' stays 32bit on both Linux and Windows.
-		''
-		'' That allows 64bit INTEGER to be mangled as 64bit long on
-		'' Linux & co, making GCC compatibility easier, it's only Win64
-		'' where we need a custom mangling.
-		''
-		'' Itanium C++ ABI compatible mangling of non-C++ built-in
-		'' types (vendor extended types):
-		''    u <length-of-id> <id>
-
-		if( env.target.options and FB_TARGETOPT_UNIX ) then
+	''
+	'' Integer/Long mangling:
+	''
+	''           32bit        64bit
+	'' Integer   long         long (Unix) or INTEGER (Windows)
+	'' Long      int          int
+	'' LongInt   long long    long long
+	''
+	''  - Fundamental problem: FB and C++ types are different, an exact mapping
+	''    is impossible
+	''
+	''  - mangling should match the C/C++ binding recommendations, i.e. use Long
+	''    for int and Integer for things like ssize_t. On linux-x86_64 Integer
+	''    can be mangled as long, but on win64 the only 64bit integer type is
+	''    long long and that's already used for LongInt. So it seems that we have
+	''    to use a custom mangling for that case (INTEGER).
+	''
+	''  - 32bit fbc used to mangle Integer as int and Long as long, but to match
+	''    64bit fbc it was reversed, allowing the same FB and C++ code to work
+	''    together on both 32bit and 64bit.
+	''
+	''  - as special expection for windows 64bit, to get a 32bit type that will
+	''    mangle to C++ long, allow 'as [u]long alias "[u]long"' declarations.
+	''    The size of LONG/ULONG does not change, it's 32bit, only the mangling,
+	''    so fbc programs can call C++ code requiring 'long int' arguments.
+
+	if( fbIs64bit( ) and ((env.target.options and FB_TARGETOPT_UNIX) = 0) ) then
+		'' Windows 64bit
+
+		'' check for remapping of dtype mangling
+		if( typeHasMangleDt( dtype ) ) then
+			dtype = typeGetMangleDt( dtype )
+			'' Windows 64bit
 			select case( dtype )
 			case FB_DATATYPE_INTEGER : return @"l"  '' long
 			case FB_DATATYPE_UINT    : return @"m"  '' unsigned long
 			end select
 		else
+			'' Itanium C++ ABI compatible mangling of non-C++ built-in types (vendor extended types):
+			''    u <length-of-id> <id>
 			select case( dtype )
 			case FB_DATATYPE_INTEGER : add_abbrev = TRUE : return @"u7INTEGER"  '' seems like a good choice
 			case FB_DATATYPE_UINT    : add_abbrev = TRUE : return @"u8UINTEGER"
 			end select
 		end if
 
-		select case( dtype )
-		case FB_DATATYPE_LONG    : return @"i"  '' int
-		case FB_DATATYPE_ULONG   : return @"j"  '' unsigned int
-		end select
 	else
+		'' 32bit, Unix 64bit
 		select case( dtype )
-		case FB_DATATYPE_INTEGER : return @"i"  '' int
-		case FB_DATATYPE_UINT    : return @"j"  '' unsigned int
-		case FB_DATATYPE_LONG    : return @"l"  '' long
-		case FB_DATATYPE_ULONG   : return @"m"  '' unsigned long
+		case FB_DATATYPE_INTEGER : return @"l"  '' long
+		case FB_DATATYPE_UINT    : return @"m"  '' unsigned long
 		end select
 	end if
 
+	'' dtype should be a FB_DATATYPE by now
+	assert( dtype = typeGetDtOnly( dtype ) )
+
 	static as zstring ptr typecodes(0 to FB_DATATYPES-1) => _
 	{ _
 		@"v", _ '' void
 		@"b", _ '' boolean
-		@"a", _ '' byte (signed char)
-		@"h", _ '' ubyte (unsigned char)
+		@"a", _ '' Byte: signed char
+		@"h", _ '' UByte: unsigned char
 		@"c", _ '' char
 		@"s", _ '' short
 		@"t", _ '' ushort
 		@"w", _ '' wchar
-		NULL, _ '' integer
-		NULL, _ '' uinteger
+		NULL, _ '' Integer
+		NULL, _ '' UInteger
 		NULL, _ '' enum
-		NULL, _ '' long
-		NULL, _ '' ulong
-		@"x", _ '' longint (long long)
-		@"y", _ '' ulongint (unsigned long long)
-		@"f", _ '' single
+		@"i", _ '' Long: int
+		@"j", _ '' ULong: unsigned int
+		@"x", _ '' LongInt: long long
+		@"y", _ '' ULongInt: unsigned long long
+		@"f", _ '' Single: float
 		@"d", _ '' double
 		NULL, _ '' var-len string
 		NULL, _ '' fix-len string
@@ -493,7 +500,7 @@ sub symbMangleType _
 	''
 	'' Plain type without reference/pointer/const bits
 	''
-	assert( dtype = typeGetDtOnly( dtype ) )
+	assert( dtype = (typeGetDtOnly( dtype ) or (dtype and FB_DT_MANGLEMASK)) )
 
 	select case( dtype )
 	case FB_DATATYPE_STRUCT, FB_DATATYPE_ENUM

src/compiler/symb.bi

@@ -57,13 +57,16 @@ const FB_DT_TYPEMASK 		= &b00000000000000000000000000011111 '' max 32 built-in d
 const FB_DT_PTRMASK  		= &b00000000000000000000000111100000 '' level of pointer indirection
 const FB_DT_CONSTMASK		= &b00000000000000111111111000000000 '' PTRLEVELS + 1 bit-masks
 const FB_DATATYPE_REFERENCE	= &b00000000000010000000000000000000 '' used for mangling BYREF parameters
+const FB_DT_MANGLEMASK		= &b00000001111100000000000000000000 '' used to modify mangling for builtin types
 const FB_DATATYPE_INVALID	= &b10000000000000000000000000000000
 
 const FB_DT_PTRLEVELS		= 8					'' max levels of pointer indirection
 
 const FB_DT_PTRPOS			= 5
 const FB_DT_CONSTPOS		= FB_DT_PTRPOS + 4
 
+const FB_DT_MANGLEPOS		= 20
+
 enum FB_OVLPROC_MATCH_SCORE
 	FB_OVLPROC_NO_MATCH = 0
 	FB_OVLPROC_LOWEST_MATCH = 1
@@ -2492,6 +2495,10 @@ declare sub symbProcRecalcRealType( byval proc as FBSYMBOL ptr )
 #define	typeSetIsRef( dt ) (dt or FB_DATATYPE_REFERENCE)
 #define	typeUnsetIsRef( dt ) (dt and not FB_DATATYPE_REFERENCE)
 
+#define	typeHasMangleDt( dt ) (((dt and FB_DT_MANGLEMASK) <> 0))
+#define typeGetMangleDt( dt ) ((dt and FB_DT_MANGLEMASK) shr FB_DT_MANGLEPOS)
+#define typeSetMangleDt( dt, mangle_dt ) ((dt and not FB_DT_MANGLEMASK) or ((mangle_dt shl FB_DT_MANGLEPOS) and FB_DT_MANGLEMASK ))
+
 declare sub symbForEachGlobal _
 	( _
 		byval symclass as integer, _