View Issue Details
|ID||Project||Category||View Status||Date Submitted||Last Update|
|0030289||FPC||Compiler||public||2016-06-16 23:09||2020-08-07 02:09|
|Reporter||Emelyanov Roman||Assigned To||Jonas Maebe|
|Status||resolved||Resolution||no change required|
|Summary||0030289: Compiler crash when try reuse compiled ppu`s|
|Description||I work in Lazarus. When i use my own RTL in my project (key thing in that: my RTL dont use precompiler files and every time compile with app) first build runs normaly, but when i try make rebuild (compiler try use already builded ppu`s) compiler crashs. So i make little investigation.|
First i build compiler with debug symbols (look down). Then i find than when compiler try load PPU it ref to something with too high ID. Next i find that this problem start from r33492. After it i localized problem in tppumodule.buildderefunitimportsyms (fppu.pas). Continue seaching show me that problem in tderef.build (symtype.pas). Looks like this function is serialize something to store in PPU. But it have side effect in "if not tsym(s).registered then tsym(s).register_sym;". I dont look deeper, but looks like error somewhere is in this place.
|Steps To Reproduce||Compile and compile again any project that compile all units than it use.|
|Additional Information||Warning: (2081) PIC directive or switch ignored|
(1002) Target OS: Win64 for x64
(3104) Compiling Bug.lpr
D:\Work\CatFramework\Source\CRTL\Win\system.pas(3,1) Error: (1026) Compilation raised exception internally
Fatal: (1018) Compilation aborted
An unhandled exception occurred at $0000000100024477:
EListError: List index exceeds bounds (1880)
$0000000100024477 RAISEINDEXERROR, line 716 of cclasses.pas
$0000000100024502 PUT, line 729 of cclasses.pas
$00000001000252F1 SETITEM, line 1071 of cclasses.pas
$000000010006B6E6 PPULOAD, line 563 of symsym.pas
$0000000100071807 PPULOAD, line 2596 of symsym.pas
$000000010004295B LOADSYMS, line 557 of symtable.pas
$0000000100042334 PPULOAD, line 477 of symtable.pas
$0000000100046D30 PPULOAD, line 2443 of symtable.pas
$00000001001A43E7 LOAD_USEDUNITS, line 1605 of fppu.pas
$00000001001A4DEF LOADPPU, line 1892 of fppu.pas
$00000001001AFDD2 ADDUNIT, line 184 of pmodules.pas
$00000001001B01EF LOADDEFAULTUNITS, line 301 of pmodules.pas
$00000001001B436C PROC_PROGRAM, line 1963 of pmodules.pas
$00000001000421DA COMPILE, line 391 of parser.pas
$0000000100019A48 COMPILE, line 272 of compiler.pas
$00000001000018E2 main, line 227 of pp.pas
Error: C:\fpc\3.1.1\bin\x86_64-win64\ppcx64.exe returned an error exitcode
|Tags||No tags attached.|
|Fixed in Revision|
||It is impossible to fix issues if we don't have the source code to reproduce it. Additionally, the system unit and compiler and closely interwoven. Always recompiling the system unit as part of a complete project is not a scenario that we test or support.|
>It is impossible to fix issues if we don't have the source code to reproduce it.
I make more investegation on problem to make minimal example for you. Bug not reproduced just with nonstandard RTL that recompile every time. Its depend on module references/symbols (you just need some badluck to see it). As i wrote problem is some sideeffect of serialization of symbol info. I just say that i found problem case with my RTL (looks like it make just "good" symbol links to show the problems).
I cleary understood that you need way to reproduce it. If you provide me your mail and promise to delete my code after you fix bug i can send you minimal example with my RTL and information.
Or possible you can give me more info about code "if not tsym(s).registered then tsym(s).register_sym;" to i can find problem sideeffect by myself and sinteticaly make code that will have it on standard RTL.
||In additional. I have simple patch than fix it, but because i don`t know what sideeffect do, it can`t be safety apply.|
PPUReuseFixFor33492.patch (1,667 bytes)
Index: compiler/fppu.pas =================================================================== --- compiler/fppu.pas (revision 33492) +++ compiler/fppu.pas (working copy) @@ -605,7 +605,7 @@ for i:=0 to unitimportsyms.count-1 do begin new(deref); - deref^.build(unitimportsyms[i]); + deref^.build(unitimportsyms[i], True); unitimportsymsderefs.add(deref); end; end; Index: compiler/symtype.pas =================================================================== --- compiler/symtype.pas (revision 33492) +++ compiler/symtype.pas (working copy) @@ -146,7 +146,7 @@ tderef = object dataidx : longint; procedure reset; - procedure build(s:TObject); + procedure build(s:TObject; SideEffectWorkaround: Boolean = false); function resolve:TObject; end; pderef = ^tderef; @@ -728,7 +728,7 @@ end; - procedure tderef.build(s:TObject); + procedure tderef.build(s:TObject; SideEffectWorkaround: Boolean); var len : byte; st : TSymtable; @@ -747,6 +747,7 @@ this symbol shouldn't be written to a ppu } if tsym(s).SymId=symid_registered_nost then Internalerror(2015102504); + if not SideEffectWorkaround then if not tsym(s).registered then tsym(s).register_sym; st:=FindUnitSymtable(tsym(s).owner) @@ -803,7 +804,6 @@ current_module.derefdata.write(data,len); end; - function tderef.resolve:TObject; var pm : tmodule;
PPUReuseFixFor33492.patch (1,667 bytes)
I have a similar problems (without own RTL, only compile project and reuse project ppu`s) http://bugs.freepascal.org/view.php?id=28814
Patch not help me.
||I just got: xquery__functions.pas(3046,72) Error: Internal error 200310221|
tw30289.patch (341 bytes)
diff --git a/compiler/fppu.pas b/compiler/fppu.pas index fd68399..b3b8a74 100644 --- a/compiler/fppu.pas +++ b/compiler/fppu.pas @@ -173,6 +173,8 @@ var ppufile.free; ppufile:=nil; end; + freederefunitimportsyms; + unitimportsymsderefs:=tfplist.create; inherited reset; end;
tw30289.patch (341 bytes)
||@Emelyanov Roman: could you try with the attached patch?|
||I've committed the patch, since it definitely fixes an error. I'm resolving this bug, since your error was related to the class of symbols affected by the patch.|
@Jonas Maebe, i try your patch, but it didn`t work. So i continue investigation and try localized probled from different angle. I check system.ppu of my RTL with PPUdump and find that all types has normaly SymID, but some not. I dont declare this symbols... In this moment i understand the real source of a bug.
Now i try too describe it. Compilation of system unit have one different (possible not only in this, but bug creating with this) from normal unit - calling psystem.create_intern_types. This proc register some types (looks like it work like type definition it unit code), but this types are not registred. My system.pas have used some other units in implementation section, so size of current_module.symlist grow when system.pas compiled. In end, when time to write ppu compiler check for unregistred syms and assign ID to them (tppumodule.buildderefunitimportsyms) ID getting too big. My english is not so good, so possible i describe it no so clear. As a proof-of-concept i add to
psystem.create_intern_types.addtype line "tstoredsym(result).register_sym;" and it fix problem with too big ID.
In summary, to fix problem we need register syms added by psystem.create_intern_types somewhere early than tppumodule.buildderefunitimportsyms. I continue work on this tomorrow.
||Using another unit in the system unit is not supported, and will never be supported. You will have to restructure your RTL and either include the functionality in your system unit via include files, or use a manager-style approach (like with our heap manager and widestring manager).|
>Using another unit in the system unit is not supported, and will never be supported.
Why? May you explain it more in detail? I have a number of thoughts why it is necessary and below I will state them. Perhaps it will help to change your opinion.
>You will have to restructure your RTL and either include the functionality in your system unit via include files
As a example i will explain what units and why i use in system.pas on Windows. First unit is CFWLib__WinAPI - headers to WinAPI. We need this in system.pas to implement think like Write(ln)/Read(ln), command line arguments, expections and etc. Yes i can make copy need part of this unit and move it to system.pas. But we got 2 copy of code that is bad (because all changes and fixes must be done in 2 diffenet locations).
Second used unit is CFWMath_FPUControl, that constain functions and flags to control FPU/SSE math mode. And again, i can just copy this code, but its make 2 version of code. Third unit is CFW_Mem that provide functions like CopyMem, CompareMem and etc. If you ask why i put this functions in external units at all - i answer, this allow to write code that can use with standard RTL and with special RTL. Its very usefull and comfortable. I have 2 main protects with special RTL. One of that is 3D-engine. It use standard RTL + LCL for tools and special (speed optimized) RTL for main applications. 95% of code can compile with both RTL without any changes.
>or use a manager-style approach (like with our heap manager and widestring manager)
But its make performance impact, dependent memory read. First of all for threadvars reads/writes. I dont say that this can not be done, but i think we dont need make programs slowly for nothing.
I clearly understand that units that used in system.pas implementation sections have strong restrictions on functionality, but its ok for tasks they used for. It is possible to use include files to one code into units and system.pas, but when its compile its produce double code. This make performance impact (first of all in I-cache using). Possible i misunderstand somethink about why you dont want allow this scenario in FPC (as i think the problem solution is very simple).
> Why? May you explain it more in detail?
The system unit is part of the Pascal language, and every unit automatically depends on it. Having the system unit depend on other units, even only in the implementation, goes completely against this principle.
The system unit and the compiler are tightly interwoven. The compiler assumes that unless it is compiling the system unit, that the system unit is completely available. There is less code specific to the system unit in the compiler these days than there used to be, but there is still some (such as the code you mentioned in psystem.pas).
This scenario is not tested, because our RTL by design never does this (nor will it ever do this). Even if you would fix the compiler now for your current case, it would not be tested in the future either. This means that you are using something unsupported and untested, which could break at any point. Additionally, such bug reports require a lot of time to figure out, especially since you did not even mention in the beginning that you were using other units in the implementation of your system unit.
> But its make performance impact, dependent memory read. First of all for threadvars reads/writes.
You don't have to make your managers threadvars.
> I dont say that this can not be done, but i think we dont need make programs slowly for nothing.
It's not for nothing. It is to keep a proper modularisation at the source code level. Design and maintainability are at least as important as speed, and in a big project with multi-platform support like FPC, these are even much more important. Speed is useless if it becomes very hard to extend the project because every change you make can easily break something on a platform you don't have yourself.
If you absolutely want to avoid both source code and binary code duplication of that code, and also function pointers, you can work with aliases. In the system unit:
procedure copymem(source, dest: pointer; size: sizeint); [public, alias: 'CFWLIB_copymem'];
In your other unit:
procedure copymem(source, dest: pointer; size: sizeint); external name 'CFWLIB_copymem';
That is ugly too, but this is guaranteed to work today and it will keep working in the future.
Thanks you for answer. Now i more clear understand your position.
> Additionally, such bug reports require a lot of time to figure out, especially since you did not even mention in the beginning that you were using other units in the implementation of your system unit.
I am not FPC developer, so i can dont know that this information needed. I try understand working of FPC compiler deaper when i try fix something, but not always understand it correct. So please excuse me for wrong problem localization.
> If you absolutely want to avoid both source code and binary code duplication of that code, and also function pointers, you can work with aliases:
Oh, its greate idea. Thanks you very much for it.
||Ok, then I will resolve this issue. Thanks for explaining the reasons for taking your approach.|
|2016-06-16 23:09||Emelyanov Roman||New Issue|
|2016-06-16 23:14||Jonas Maebe||Note Added: 0093253|
|2016-06-16 23:14||Jonas Maebe||Status||new => resolved|
|2016-06-16 23:14||Jonas Maebe||Resolution||open => unable to reproduce|
|2016-06-16 23:14||Jonas Maebe||Assigned To||=> Jonas Maebe|
|2016-06-17 01:03||Emelyanov Roman||Note Added: 0093258|
|2016-06-17 01:03||Emelyanov Roman||Status||resolved => feedback|
|2016-06-17 01:03||Emelyanov Roman||Resolution||unable to reproduce => reopened|
|2016-06-17 01:22||Emelyanov Roman||Note Added: 0093260|
|2016-06-17 01:22||Emelyanov Roman||Status||feedback => assigned|
|2016-06-17 01:22||Emelyanov Roman||File Added: PPUReuseFixFor33492.patch|
|2016-06-17 18:11||Andrey Zubarev||Note Added: 0093268|
|2016-06-17 19:41||Andrey Zubarev||Note Edited: 0093268||View Revisions|
|2016-10-09 14:58||Jonas Maebe||Relationship added||has duplicate 0030652|
|2016-10-23 14:05||Benito van der Zander||Note Added: 0095274|
|2016-11-01 23:36||Jonas Maebe||File Added: tw30289.patch|
|2016-11-01 23:37||Jonas Maebe||Note Added: 0095469|
|2016-11-06 20:00||Jonas Maebe||Status||assigned => feedback|
|2016-11-13 17:43||Jonas Maebe||Fixed in Revision||=> 34894|
|2016-11-13 17:43||Jonas Maebe||Note Added: 0095833|
|2016-11-13 17:43||Jonas Maebe||Status||feedback => resolved|
|2016-11-13 17:43||Jonas Maebe||Fixed in Version||=> 3.1.1|
|2016-11-13 17:43||Jonas Maebe||Resolution||reopened => fixed|
|2016-11-16 00:32||Emelyanov Roman||Note Added: 0095903|
|2016-11-16 00:32||Emelyanov Roman||Status||resolved => feedback|
|2016-11-16 00:32||Emelyanov Roman||Resolution||fixed => reopened|
|2016-11-16 13:34||Jonas Maebe||Note Added: 0095911|
|2016-11-16 19:25||Emelyanov Roman||Note Added: 0095924|
|2016-11-16 19:25||Emelyanov Roman||Status||feedback => assigned|
|2016-11-16 20:08||Jonas Maebe||Note Added: 0095925|
|2016-11-16 20:20||Jonas Maebe||Note Edited: 0095925||View Revisions|
|2016-11-16 20:20||Jonas Maebe||Note Edited: 0095925||View Revisions|
|2016-11-16 20:24||Emelyanov Roman||Note Added: 0095926|
|2016-11-16 20:34||Jonas Maebe||Fixed in Revision||34894 =>|
|2016-11-16 20:34||Jonas Maebe||Note Added: 0095928|
|2016-11-16 20:34||Jonas Maebe||Status||assigned => resolved|
|2016-11-16 20:34||Jonas Maebe||Fixed in Version||3.1.1 =>|
|2016-11-16 20:34||Jonas Maebe||Resolution||reopened => no change required|
|2016-11-16 20:35||Jonas Maebe||Relationship deleted||has duplicate 0030652|