All Projects FPC fpcprojects fpprofilerfpGUI Lazarus Packages PatchesLazarus CCR Mantis pas2js

View Issue Details

Jump to NotesJump to History
IDProjectCategoryView StatusDate SubmittedLast Update
0035590FPCCompilerpublic2019-05-16 12:212020-02-08 02:45
Reporternanobit Assigned To 
PrioritynormalSeveritymajorReproducibilityalways
Status acknowledgedResolutionopen 
Platformwin32OSWindows 
Product Version3.2.0 
Fixed in Version3.3.1 
Summary0035590: Internal error 200405231 with inline
DescriptionI run into 3rd party code (with provoking coding style) which worked in fpc 3.0.4,
but gives Internal error 200405231 in fpc 3.2.0 beta (x86).
Steps To Reproduceprogram project1;

  function sLow: integer; inline;
  begin result := 1; end;

  function sHigh( const s: string): integer; inline;
  begin result := Length(s); end;

  procedure insert2( const substr: string; var s: string; index: integer);
  begin insert( substr, s, index); end;

  function replaceChars(const s, subStr: string): string;
  var i: integer;
  begin
    result := s;
    // ok with sHigh(s) or with non-inlined sHigh(result)
    for i := sHigh(result) downto sLow() do begin
      delete( result, i, 1);
      insert2( subStr, result, i); // ok with (unwrapped) insert( subStr, result, i)
    end;
  end; // Error: Internal error 200405231

  procedure test1;
  var s, newChar, r: ansistring;
  begin
    s := 'old'; newChar := 'Replace';
    r := replaceChars( s, newChar);
  end;

begin
test1;
end.
TagsNo tags attached.
Fixed in Revision44119
FPCOldBugId
FPCTarget3.2.0
Attached Files

Activities

J. Gareth Moreton

2019-05-16 21:52

developer   ~0116223

Last edited: 2019-05-16 21:58

View 2 revisions

 Confirmed "Internal error 200405231" on x86_64-win64 when compiling through Lazarus.

Running "fpc -MobjFPC -O1 -B -g inline_crash.pas" from the command prompt does not generate the internal error - now researching which compiler option is causing the internal error to be triggered.

J. Gareth Moreton

2019-05-16 22:08

developer   ~0116224

Last edited: 2019-05-16 22:11

View 2 revisions

 Okay, minor breakthrough... the internal error only occurs if the "-Sh" option is specified, which instructs the compiler to use reference-counted ansistrings. In Lazarus, clearing the option "Use ansistrings" under "Compiler Options -> Parsing" allows the code to be built successfully (with the hint "inline_crash.pas(24,19) Note: Local variable "r" is assigned but never used").

The internal error is raised because it seems the implicit finally block isn't being generated properly. This is a case where "inline" should be disabled, because you're not even allowed to use explicit try..finally blocks in an inlined routine (although the compiler calls it a nested procedure).

(ADDENDUM: The implicit finally block is required to clean up the reference-counted strings)

J. Gareth Moreton

2019-05-18 21:38

developer  

i35590.patch (784 bytes)    
Index: compiler/hlcgobj.pas
===================================================================
--- compiler/hlcgobj.pas	(revision 42086)
+++ compiler/hlcgobj.pas	(working copy)
@@ -4697,7 +4697,10 @@
             assigned(hp^.def) and
             is_managed_type(hp^.def) then
           begin
-            include(current_procinfo.flags,pi_needs_implicit_finally);
+            { If it needs an implicit finally block, the relevant flag should
+              have been set in the first pass.  Note that we can't set it here
+              because "add_entry_exit_code" has already been called, and
+              setting the flag now will raise Internal Error 200405231. [Kit] }
             tg.temp_to_ref(hp,href);
             g_finalize(list,hp^.def,href);
           end;
i35590.patch (784 bytes)   

J. Gareth Moreton

2019-05-18 21:40

developer   ~0116252

Last edited: 2019-05-18 21:40

View 2 revisions

 I've fixed the crash and the sample program runs without issue now (find attached a new patch file). The regression tests pass without incident, although I admit that I'm not sure if memory leaks occur or not.

nanobit

2019-05-19 11:45

reporter   ~0116259

I'm not a compiler dev, but fpc 3.0.4 finds an inlined solution, still with the source which your patch removes.
Does fpc 3.0.4 have an alternative solution, possible for fpc3.2?

J. Gareth Moreton

2019-05-19 12:04

developer   ~0116260

Last edited: 2019-05-19 13:20

View 2 revisions

 Aah, I forgot to clarify. Though I removed a line of code, everything is still inlined as requested. I was mistaken about the implicit finally block not being allowed... that's simply moved to the caller if it's needed.

I'm not sure what 3.0.4 did that 3.2 does differently, but the internal error was caused because the compiler set the "needs implicit finally" flag after the point where such a finally block is inserted (and the flag wasn't set at that point), so it crashed at a sanity check later.

Florian

2020-02-05 21:35

administrator   ~0120894

Thanks, committed.

Jonas Maebe

2020-02-06 19:43

manager   ~0120901

That does not seem like a fix, but like hiding an error. The added comment even says that the flag should have been set already at that place, so if anything, an internalerror should be added there in case it's not yet set.

J. Gareth Moreton

2020-02-06 20:01

developer   ~0120902

Not quite. You see, the flag was set earlier for the calling procedure, the implicit finally handler created and then the flag is cleared, it seems. If it's set again (which is the line removed), then the internal error is raised later because the flag exists to create an implicit finally block, but one already exists.

Some extra testing may need to be done to make sure things are still inlined, but it was mostly a logic flow issue.

Jonas Maebe

2020-02-06 21:07

manager   ~0120904

Last edited: 2020-02-06 21:07

View 2 revisions

 > the internal error is raised later because the flag exists to create an implicit finally block, but one already exists. the flag was set earlier for the calling procedure, the implicit finally handler created and then the flag is cleared,

I don't see any place in the compiler where pi_needs_implicit_finally gets cleared.

> If it's set again (which is the line removed), then the internal error is raised later because the flag exists to create an implicit finally block, but one already exists.

The internalerror triggers if the pi_needs_implicit_finally is set, but _no_ implicit finally block has been generated.

Perhaps it is just a matter of bookkeeping in the parent routine regarding what has been done for the inline routines, but this change really seems wrong.

Florian

2020-02-06 21:19

administrator   ~0120905

I think Jonas is right, I misunderstood the code.

J. Gareth Moreton

2020-02-06 23:26

developer   ~0120911

Fair enough. Maybe it does cover up something deeper. It works for the time being. Sorry I got some details mixed up. All I know is that at the point where I removed that line of code, add_entry_exit_code had already been called and I believe the correct implicit finally block had been created on account of the inlined nodes containing reference-counted strings.

Issue History

Date Modified Username Field Change
2019-05-16 12:21 nanobit New Issue
2019-05-16 21:52 J. Gareth Moreton Status new => confirmed
2019-05-16 21:52 J. Gareth Moreton FPCTarget => -
2019-05-16 21:52 J. Gareth Moreton Note Added: 0116223
2019-05-16 21:58 J. Gareth Moreton Note Edited: 0116223 View Revisions
2019-05-16 22:08 J. Gareth Moreton Note Added: 0116224
2019-05-16 22:11 J. Gareth Moreton Note Edited: 0116224 View Revisions
2019-05-17 22:12 J. Gareth Moreton Assigned To => J. Gareth Moreton
2019-05-17 22:12 J. Gareth Moreton Status confirmed => assigned
2019-05-17 22:13 J. Gareth Moreton Severity minor => major
2019-05-18 21:38 J. Gareth Moreton File Added: i35590.patch
2019-05-18 21:40 J. Gareth Moreton Assigned To J. Gareth Moreton => Florian
2019-05-18 21:40 J. Gareth Moreton Status assigned => feedback
2019-05-18 21:40 J. Gareth Moreton FPCTarget - => 3.2.0
2019-05-18 21:40 J. Gareth Moreton Note Added: 0116252
2019-05-18 21:40 J. Gareth Moreton Note Edited: 0116252 View Revisions
2019-05-19 11:45 nanobit Note Added: 0116259
2019-05-19 11:45 nanobit Status feedback => assigned
2019-05-19 12:04 J. Gareth Moreton Note Added: 0116260
2019-05-19 13:20 J. Gareth Moreton Note Edited: 0116260 View Revisions
2020-02-05 21:35 Florian Status assigned => resolved
2020-02-05 21:35 Florian Resolution open => fixed
2020-02-05 21:35 Florian Fixed in Revision => 44119
2020-02-05 21:35 Florian Note Added: 0120894
2020-02-05 21:35 Florian Fixed in Version => 3.3.1
2020-02-06 19:43 Jonas Maebe Note Added: 0120901
2020-02-06 20:01 J. Gareth Moreton Status resolved => feedback
2020-02-06 20:01 J. Gareth Moreton Resolution fixed => reopened
2020-02-06 20:01 J. Gareth Moreton Note Added: 0120902
2020-02-06 20:02 J. Gareth Moreton Status feedback => resolved
2020-02-06 20:02 J. Gareth Moreton Resolution reopened => fixed
2020-02-06 21:07 Jonas Maebe Note Added: 0120904
2020-02-06 21:07 Jonas Maebe Note Edited: 0120904 View Revisions
2020-02-06 21:19 Florian Note Added: 0120905
2020-02-06 21:19 Florian Assigned To Florian =>
2020-02-06 21:19 Florian Status resolved => acknowledged
2020-02-06 23:26 J. Gareth Moreton Note Added: 0120911
2020-02-08 02:45 J. Gareth Moreton Resolution fixed => open