View Issue Details
ID | Project | Category | View Status | Date Submitted | Last Update |
---|---|---|---|---|---|
0035590 | FPC | Compiler | public | 2019-05-16 12:21 | 2020-10-22 21:37 |
Reporter | nanobit | Assigned To | |||
Priority | normal | Severity | major | Reproducibility | always |
Status | acknowledged | Resolution | open | ||
Platform | win32 | OS | Windows | ||
Product Version | 3.2.0 | ||||
Fixed in Version | 3.3.1 | ||||
Summary | 0035590: Internal error 200405231 with inline | ||||
Description | I 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 Reproduce | program 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. | ||||
Tags | No tags attached. | ||||
Fixed in Revision | 44119 | ||||
FPCOldBugId | |||||
FPCTarget | 3.2.0 | ||||
Attached Files |
|
|
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. |
|
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) |
|
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; |
|
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. |
|
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? |
|
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. |
|
Thanks, committed. |
|
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. |
|
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. |
|
> 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. |
|
I think Jonas is right, I misunderstood the code. |
|
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. |
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 |
2020-10-22 21:36 | Florian | Relationship added | duplicate of 0036840 |
2020-10-22 21:37 | Florian | Relationship deleted | 0036840 |
2020-10-22 21:37 | Florian | Relationship added | related to 0036840 |