View Issue Details

IDProjectCategoryView StatusLast Update
0030289FPCCompilerpublic2020-08-07 02:09
ReporterEmelyanov Roman Assigned ToJonas Maebe  
PrioritynormalSeverityminorReproducibilityalways
Status resolvedResolutionno change required 
Platformx86_64OSWindows 
Product Version3.1.1 
Summary0030289: Compiler crash when try reuse compiled ppu`s
DescriptionI 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 ReproduceCompile and compile again any project that compile all units than it use.
Additional InformationWarning: (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
  $0000000100001906

Error: C:\fpc\3.1.1\bin\x86_64-win64\ppcx64.exe returned an error exitcode
TagsNo tags attached.
Fixed in Revision
FPCOldBugId
FPCTarget
Attached Files

Activities

Jonas Maebe

2016-06-16 23:14

manager   ~0093253

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.

Emelyanov Roman

2016-06-17 01:03

reporter   ~0093258

>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.

Emelyanov Roman

2016-06-17 01:22

reporter   ~0093260

In additional. I have simple patch than fix it, but because i don`t know what sideeffect do, it can`t be safety apply.

Emelyanov Roman

2016-06-17 01:22

reporter  

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)   

Andrey Zubarev

2016-06-17 18:11

reporter   ~0093268

Last edited: 2016-06-17 19:41

View 2 revisions

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.

Benito van der Zander

2016-10-23 14:05

reporter   ~0095274

I just got: xquery__functions.pas(3046,72) Error: Internal error 200310221

Jonas Maebe

2016-11-01 23:36

manager  

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)   

Jonas Maebe

2016-11-01 23:37

manager   ~0095469

@Emelyanov Roman: could you try with the attached patch?

Jonas Maebe

2016-11-13 17:43

manager   ~0095833

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.

Emelyanov Roman

2016-11-16 00:32

reporter   ~0095903

@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.

Jonas Maebe

2016-11-16 13:34

manager   ~0095911

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).

Emelyanov Roman

2016-11-16 19:25

reporter   ~0095924

>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).

Jonas Maebe

2016-11-16 20:08

manager   ~0095925

Last edited: 2016-11-16 20:20

View 3 revisions

> Why? May you explain it more in detail?

1) Semantically

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.

2) Practically

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).

3) Work

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'];
begin
...
end;

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.

Emelyanov Roman

2016-11-16 20:24

reporter   ~0095926

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.

Jonas Maebe

2016-11-16 20:34

manager   ~0095928

Ok, then I will resolve this issue. Thanks for explaining the reasons for taking your approach.

Issue History

Date Modified Username Field Change
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