Improving support of dotted unit names. Strange error.
lazarus / 9-Editor

----------------------------------------------------------------------
JuhaManninen · 2024-02-02 11:27

This is about the commit 593bf2d5415 based on the work of @WooBean007.
 IDE,Codetools: Improve support of dotted unit names. Issue #40596
Some unit tests fail but it also triggers another weird problem which
I explain here. The issue report will become too long if I copy long
listings there.

The IDE built for LCL-QT5 crashes on startup when LazBuild is the
active project. LazBuild uses NoGUI widgetset, I don't know if it
affects.
It does not happen with IDE built for LCL-GTK2.
It does not happen with any other project open than LazBuild.
It does not happen if I add DebugLn() in
TLazPackage.SubstitutePkgMacros function. First I listed all entries
in SubstitutePkgMacros, then only those that really got macros
substituted. No errors in neither case.
I can open the IDE with another project active and then open LazBuild.
No errors. Only when I close the IDE and then restart it, the error
comes.
It is 100% reproducible at least in my current Linux session. I can
jump to a commit prior to 593bf2d5415, build Lazarus and the error is
gone. Then I jump to 593bf2d5415, build again and the error is back.

The Call trace listing says: was released at and freed again at
It hints to some weird timing issue especially because a single
DebugLn prevents it. It may be triggered by the commit but not
directly caused by it.
The Circle in Options: pcosOutputDir may be a wrong note caused by the
other errors.
It is very possible I cannot reproduce this after rebooting my
computer. Can anybody else reproduce?

Code: [Select]LoadComponentDependencyHidden
/home/juha/SW/lazarus/packager/packageeditor.pas
AComponentClassName=TBasePackageEditor AComponentClass=nil
AncestorClass=TForm LFMFilename=
InitOpenedProjectFile select form in designer:
PackageEditorForm:TPackageEditorForm TDesigner
Marked memory at $00007F7B27DD3600 released
Call trace for block at $00007F7B27DD3600 size 42
  $0000000000C13FB2  SubstitutePkgMacros,  line 2245 of
../packager/packagedefs.pas
  $0000000000BB62EE  DoParseOption,  line 3903 of compileroptions.pp
  $0000000000BB5DCE  GetParsedValue,  line 3812 of compileroptions.pp
  $0000000000C1D7DB  GetOutputDirectory,  line 3846 of
../packager/packagedefs.pas
  $0000000000C21B04  UpdateOutputDirectory,  line 4517 of
../packager/packagedefs.pas
  $00000000013B0FB0  OutputDirectoryChanged,  line 206 of
../packager/projpackcommon.pas
  $00000000013B0EAB  IDChanged,  line 185 of
../packager/projpackcommon.pas
  $0000000000C21279  AllChanged,  line 4426 of
../packager/packagedefs.pas
 was released at
  $0000000000D55ED2  RebuildDefineTemplates,  line 1319 of
../packager/packagesystem.pas
  $0000000000D86019  DoOpenPackage,  line 3479 of
../packager/pkgmanager.pas
  $0000000000D86BF9  DoOpenPackageFile,  line 3628 of
../packager/pkgmanager.pas
  $00000000004C1F9B  SetupStartProject,  line 2424 of main.pp
  $00000000004BDAFF  StartIDE,  line 1695 of main.pp
  $00000000004257D2  $main,  line 148 of lazarus.pp
  $00000000004257D2  $main,  line 148 of lazarus.pp
  $0000000000D838DB  LoadInstalledPackages,  line 3029 of
../packager/pkgmanager.pas
freed again at
  $0000000000BB62EE  DoParseOption,  line 3903 of compileroptions.pp
  $0000000000BB5DCE  GetParsedValue,  line 3812 of compileroptions.pp
  $0000000000C1D7DB  GetOutputDirectory,  line 3846 of
../packager/packagedefs.pas
  $0000000000C21B04  UpdateOutputDirectory,  line 4517 of
../packager/packagedefs.pas
  $00000000013B0FB0  OutputDirectoryChanged,  line 206 of
../packager/projpackcommon.pas
  $00000000013B0EAB  IDChanged,  line 185 of
../packager/projpackcommon.pas
  $0000000000C21279  AllChanged,  line 4426 of
../packager/packagedefs.pas
  $0000000000D55ED2  RebuildDefineTemplates,  line 1319 of
../packager/packagesystem.pas
  $0000000000D86019  DoOpenPackage,  line 3479 of
../packager/pkgmanager.pas
  $0000000000D86BF9  DoOpenPackageFile,  line 3628 of
../packager/pkgmanager.pas
  $00000000004C1F9B  SetupStartProject,  line 2424 of main.pp
  $00000000004BDAFF  StartIDE,  line 1695 of main.pp
  $00000000004257D2  $main,  line 148 of lazarus.pp
FreeFormEditor: FormEditor1=TFormEditor
[TJITComponentList.DestroyJITComponent] ERROR destroying component
Error: Object reference is Nil
  Stack trace:
  $0000000000C852A2  SetCustomForm,  line 1167 of
../designer/controlselection.pp
  $0000000000C8A4B1  Delete,  line 2285 of
../designer/controlselection.pp
  $0000000000C8A3CF  Remove,  line 2266 of
../designer/controlselection.pp
  $0000000000C8697B  Notification,  line 1455 of
../designer/controlselection.pp
  $000000000053D517
  $0000000000709D21  Destroy,  line 5226 of include/control.inc
  $00000000006F09C0  Destroy,  line 6633 of include/wincontrol.inc
  $000000000070D1C4  Destroy,  line 40 of include/customcontrol.inc
  $0000000000489FF4  Destroy,  line 354 of
include/scrollingwincontrol.inc
  $000000000048BF21  Destroy,  line 138 of include/customform.inc
  $00000000004372BB
  $0000000000C480FA  Destroy,  line 741 of ../designer/jitforms.pp
  $00000000004372BB
  $00000000004372BB
  $00000000004BDF99  Destroy,  line 1761 of main.pp
  $00000000004372BB
  $0000000000485923  BeforeFinalization,  line 2068 of forms.pp
TCustomFormEditor.JITListException List.CurReadStreamClass=nil nil
---ComponentIsInvisible: Adding SynGutterMarks1:TSynGutterMarks to
InvisibleClasses.---
---ComponentIsInvisible: Adding
SynGutterLineNumber1:TSynGutterLineNumber to InvisibleClasses.---
---ComponentIsInvisible: Adding SynGutterChanges1:TSynGutterChanges to
InvisibleClasses.---
---ComponentIsInvisible: Adding
SynGutterSeparator1:TSynGutterSeparator to InvisibleClasses.---
---ComponentIsInvisible: Adding
SynGutterCodeFolding1:TSynGutterCodeFolding to InvisibleClasses.---
---ComponentIsInvisible: Adding
SynLeftGutterPartList1:TSynGutterPartList to InvisibleClasses.---
TParsedCompilerOptions.GetParsedValue Circle in Options: pcosOutputDir
Unparsed="lib/$(TargetCPU)-$(TargetOS)/$(LCLWidgetType)"
TParsedCompilerOptions.GetParsedValue Circle in Options: pcosOutputDir
Unparsed="lib/$(TargetCPU)-$(TargetOS)/$(LCLWidgetType)"
TParsedCompilerOptions.GetParsedValue Circle in Options: pcosOutputDir
Unparsed="lib/$(TargetCPU)-$(TargetOS)/$(LCLWidgetType)"
TParsedCompilerOptions.GetParsedValue Circle in Options: pcosOutputDir
Unparsed="lib/$(TargetCPU)-$(TargetOS)/$(LCLWidgetType)"
TParsedCompilerOptions.GetParsedValue Circle in Options: pcosOutputDir
Unparsed="lib/$(TargetCPU)-$(TargetOS)/$(LCLWidgetType)"
TParsedCompilerOptions.GetParsedValue Circle in Options: pcosOutputDir
Unparsed="lib/$(TargetCPU)-$(TargetOS)/$(LCLWidgetType)"
TParsedCompilerOptions.GetParsedValue Circle in Options: pcosOutputDir
Unparsed="lib/$(TargetCPU)-$(TargetOS)/$(LCLWidgetType)"
TParsedCompilerOptions.GetParsedValue Circle in Options: pcosOutputDir
Unparsed="lib/$(TargetCPU)-$(TargetOS)/$(LCLWidgetType)"
TParsedCompilerOptions.GetParsedValue Circle in Options: pcosOutputDir
Unparsed="lib/$(TargetCPU)-$(TargetOS)/$(LCLWidgetType)"
TParsedCompilerOptions.GetParsedValue Circle in Options: pcosOutputDir
Unparsed="lib/$(TargetCPU)-$(TargetOS)/$(LCLWidgetType)"
TParsedCompilerOptions.GetParsedValue Circle in Options: pcosOutputDir
Unparsed="lib/$(TargetCPU)-$(TargetOS)/$(LCLWidgetType)"
TParsedCompilerOptions.GetParsedValue Circle in Options: pcosOutputDir
Unparsed="lib/$(TargetCPU)-$(TargetOS)/$(LCLWidgetType)"
TParsedCompilerOptions.GetParsedValue Circle in Options: pcosOutputDir
Unparsed="lib/$(TargetCPU)-$(TargetOS)/$(LCLWidgetType)"
TParsedCompilerOptions.GetParsedValue Circle in Options: pcosOutputDir
Unparsed="lib/$(TargetCPU)-$(TargetOS)/$(LCLWidgetType)"
TParsedCompilerOptions.GetParsedValue Circle in Options: pcosOutputDir
Unparsed="lib/$(TargetCPU)-$(TargetOS)/$(LCLWidgetType)"
TParsedCompilerOptions.GetParsedValue Circle in Options: pcosOutputDir
Unparsed="lib/$(TargetCPU)-$(TargetOS)/$(LCLWidgetType)"
TParsedCompilerOptions.GetParsedValue Circle in Options: pcosOutputDir
Unparsed="lib/$(TargetCPU)-$(TargetOS)/$(LCLWidgetType)"
TParsedCompilerOptions.GetParsedValue Circle in Options: pcosOutputDir
Unparsed="lib/$(TargetCPU)-$(TargetOS)/$(LCLWidgetType)"
TParsedCompilerOptions.GetParsedValue Circle in Options: pcosOutputDir
Unparsed="lib/$(TargetCPU)-$(TargetOS)/$(LCLWidgetType)"
TParsedCompilerOptions.GetParsedValue Circle in Options: pcosOutputDir
Unparsed="lib/$(TargetCPU)-$(TargetOS)/$(LCLWidgetType)"
TParsedCompilerOptions.GetParsedValue Circle in Options: pcosOutputDir
Unparsed="lib/$(TargetCPU)-$(TargetOS)/$(LCLWidgetType)"
TParsedCompilerOptions.GetParsedValue Circle in Options: pcosOutputDir
Unparsed="lib/$(TargetCPU)-$(TargetOS)/$(LCLWidgetType)"
TParsedCompilerOptions.GetParsedValue Circle in Options: pcosOutputDir
Unparsed="lib/$(TargetCPU)-$(TargetOS)/$(LCLWidgetType)"
TParsedCompilerOptions.GetParsedValue Circle in Options: pcosOutputDir
Unparsed="lib/$(TargetCPU)-$(TargetOS)/$(LCLWidgetType)"
TParsedCompilerOptions.GetParsedValue Circle in Options: pcosOutputDir
Unparsed="lib/$(TargetCPU)-$(TargetOS)/$(LCLWidgetType)"
TApplication.HandleException: EAccessViolation
Access violation
  Stack trace:
  $000000000053D511
  $00000000004372BB
  $00000000004372BB
  $00000000004BDF99  Destroy,  line 1761 of main.pp
  $00000000004372BB
  $0000000000485923  BeforeFinalization,  line 2068 of forms.pp
  $000000000043C53B
  $0000000000C13FB2  SubstitutePkgMacros,  line 2245 of
../packager/packagedefs.pas
  $0000000000BB62EE  DoParseOption,  line 3903 of compileroptions.pp
  $0000000000BB5DCE  GetParsedValue,  line 3812 of compileroptions.pp
  $0000000000C1D7DB  GetOutputDirectory,  line 3846 of
../packager/packagedefs.pas
  $0000000000C21B04  UpdateOutputDirectory,  line 4517 of
../packager/packagedefs.pas
  $00000000013B0FB0  OutputDirectoryChanged,  line 206 of
../packager/projpackcommon.pas
  $00000000013B0EAB  IDChanged,  line 185 of
../packager/projpackcommon.pas
  $0000000000C21279  AllChanged,  line 4426 of
../packager/packagedefs.pas
  $0000000000D55ED2  RebuildDefineTemplates,  line 1319 of
../packager/packagesystem.pas
  $0000000000D86019  DoOpenPackage,  line 3479 of
../packager/pkgmanager.pas

----------------------------------------------------------------------
zeljko · 2024-02-02 13:16

It works perfect here. Opened lazbuild.lpi - nothing bad happens,
closed lazarus, started lazarus with lazbuild.lpi as default -
everything ok. LinuxMint 20.2, 64bit,
Lazarus 3.99 (rev main_3_99-1344-g45f4cd65d9) FPC 3.2.2 x86_64-linux-
qt5 .
Maybe you have some wierd components installed ?

----------------------------------------------------------------------
JuhaManninen · 2024-02-02 15:32

I had KAS Components and GifAnim installed. I uninstalled them and the
error was gone. Then I reinstalled them and the error was still gone.
I cannot reproduce it any more.
A bug manifested itself with a certain memory layout in this computer
with certain settings. There is a dangling pointer or similar real bug
but it will be difficult to find.

Anyway, the commit for codetools did not cause it. It only triggered
it in this particular computer.

[Edit] I figured it must be related to reference counted string
handling. Methods in the Call trace do not allocate much memory and do
not free any memory, but they manipulate strings.
The innocent looking "const" modifier can cause memory corruption
issues which are difficult to spot.
Martin has explained pitfalls of "const" in some mails / posts. With
"const" a compiler does not promise that a string remains unmodified.
It is a promise from a programmer to a compiler that he will not
modify the string.
Often it is difficult to see how a string is modified by the
underlying functions. In this case I didn't find the exact spot either
but there is PChar manipulation involved.

I removed some "const" modifiers. See commit 39fb484747.

----------------------------------------------------------------------
zeljko · 2024-02-02 19:05

Why CI does not work since you commited that patch about codetools ?

----------------------------------------------------------------------
JuhaManninen · 2024-02-03 07:49

Quote from: zeljko on February 02, 2024, 08:05:51 pmWhy CI does not
work since you commited that patch about codetools ?
Unit tests failed, yes. I reverted the commit. Now the code can be
developed further without hurry.

----------------------------------------------------------------------
n7800 · 2024-02-03 09:34

Everything is fine on Windows too. It seems like it's just a problem
with an "incomplete" rebuild. The fact that after removing the
components "KAS Components" and "GifAnim" the error stopped appearing
only confirms this. Did you use any cleaning before?

By the way, it will be more convenient if you leave the URL to the
commits, and not just their hash.

And I wonder what kind of "KAS Components" it is? I didn't find it on
the Internet.

----------------------------------------------------------------------
JuhaManninen · 2024-02-03 23:11

Quote from: n7800 on February 03, 2024, 10:34:34 amEverything is fine
on Windows too. It seems like it's just a problem with an "incomplete"
rebuild. The fact that after removing the components "KAS Components"
and "GifAnim" the error stopped appearing only confirms this. Did you
use any cleaning before?
No! I made a clean build many times.
Removing the components changed the memory layout.
Bugs caused by a "const" modifier are very real. For example :
 https://gitlab.com/freepascal.org/lazarus/lazarus/-/issues/40300

My error output mentioned GetParsedValue and pcosOutputDir. Macro
substitution involves PChar manipulation. Somewhere a string is
modified although a "const" modifier promised otherwise.
Unfortunately we have such bugs lurking around. People experience
mysterious crashes or malfunctions which nobody can reproduce. The
reason is very difficult to find even if a bug can be reproduced. The
"const" can be somewhere up in the call stack.

QuoteAnd I wonder what kind of "KAS Components" it is? I didn't find
it on the Internet.
It can be found in Online Package Manager with name KASToolBar. It was
needed by some application. I forgot the details already.

----------------------------------------------------------------------
Martin_fr · 2024-02-04 00:58

If it as a "const" issue, or anything of that sort, does valgrind
report anything?

----------------------------------------------------------------------
n7800 · 2024-02-04 04:24

Quote from: JuhaManninen on February 02, 2024, 04:32:48 pmI removed
some "const" modifiers. See commit 39fb484747.

And removing "const" helped? Or was it after the error stopped
reproducing?

Quote from: JuhaManninen on February 02, 2024, 04:32:48 pmI had KAS
Components and GifAnim installed. I uninstalled them and the error was
gone. Then I reinstalled them and the error was still gone. I cannot
reproduce it any more.

Maybe you installed the wrong version?

----------------------------------------------------------------------
JuhaManninen · 2024-02-04 10:05

I have a problem with valgrind --tool=memcheck now. It says :

Code: [Select]valgrind:  Fatal error at startup: a function
redirection
valgrind:  which is mandatory for this platform-tool combination
valgrind:  cannot be set up.  Details of the redirection are:
valgrind: 
valgrind:  A must-be-redirected function
valgrind:  whose name matches the pattern:      strlen
valgrind:  in an object with soname matching:   ld-linux-x86-64.so.2
valgrind:  was not found whilst processing
valgrind:  symbols from the object with soname: ld-linux-x86-64.so.2
valgrind: 
valgrind:  Possible fixes: (1, short term): install glibc's debuginfo
valgrind:  package on this machine.  (2, longer term): ask the
packagers
valgrind:  for your Linux distribution to please in future ship a non-
valgrind:  stripped ld.so (or whatever the dynamic linker .so is
called)
valgrind:  that exports the above-named function using the standard
valgrind:  calling conventions for this platform.  The package you
need
valgrind:  to install for fix (1) is called
valgrind: 
valgrind:    On Debian, Ubuntu:                 libc6-dbg
valgrind:    On SuSE, openSuSE, Fedora, RHEL:   glibc-debuginfo
valgrind: 
valgrind:  Note that if you are debugging a 32 bit process on a
valgrind:  64 bit system, you will need a corresponding 32 bit
debuginfo
valgrind:  package (e.g. libc6-dbg:i386).
valgrind: 
valgrind:  Cannot continue -- exiting now.  Sorry.

I didn't find how to fix it in Manjaro yet.

Removing "const" modifiers was an educated guess. I could not
reproduce the error at that point.
KASToolBar or other components do not cause such errors. You can test
it yourself.

----------------------------------------------------------------------
Martin_fr · 2024-02-04 11:46

Don't know, but did you check the suggestion from the message " (1,
short term): install glibc's debuginfo package on this machine"?

I guess some package with "glibc" and "dev" in the name... But again,
don't know.

----------------------------------------------------------------------
JuhaManninen · 2024-02-04 12:41

Quote from: Martin_fr on February 04, 2024, 12:46:21 pmDon't know, but
did you check the suggestion from the message " (1, short term):
install glibc's debuginfo package on this machine"?
I guess some package with "glibc" and "dev" in the name... But again,
don't know.
No such packages in Manjaro system. There is "debuginfod" which is
already installed.
I try to solve it later somehow.

----------------------------------------------------------------------
WooBean · 2024-02-04 13:45

Maybe it somehow related to reported error.

I indeed modified some declarations of existing parameter lists by
adding default const parameters of type Boolean or Integer like:
Code: Pascal  [Select][+][-]window.onload = function(){var x1 =
document.getElementById("main_content_section"); if (x1) { var x =
document.getElementsByClassName("geshi");for (var i = 0; i function
GetIdentifier(Identifier: PChar; const aSkipAmp: Boolean = True;   
const IsDottedIdent: Boolean = False): string; 
The last parameter IsDottedIdent was "mine".

I considered it a safe way of modifing very large application without
any risk that a new code will be executed  unintentionally.

----------------------------------------------------------------------
Martin_fr · 2024-02-04 13:53

Quote from: WooBean on February 04, 2024, 02:45:35 pmI indeed modified
some declarations of existing parameter lists by adding default const
parameters of type Boolean or Integer like:
Code: Pascal  [Select][+][-]window.onload = function(){var x1 =
document.getElementById("main_content_section"); if (x1) { var x =
document.getElementsByClassName("geshi");for (var i = 0; i function
GetIdentifier(Identifier: PChar; const aSkipAmp: Boolean = True;   
const IsDottedIdent: Boolean = False): string; 
The last parameter IsDottedIdent was "mine".

I considered it a safe way of modifing very large application without
any risk that a new code will be executed  unintentionally.

It is no problem to add  new params with default "AFoo: boolean =
false" is fine.


The problem is about adding "const param" => any "CONST foo:
sometype".
Especially if sometype is a managed param: string, array, interface,
...


"somefunc( CONST foo: ansistring)" is a very dangerous it of code.
There are countless posts in the forum already, explaining why.

----------------------------------------------------------------------
JuhaManninen · 2024-02-05 05:34

Quote from: WooBean on February 04, 2024, 02:45:35 pmMaybe it somehow
related to reported error.
I indeed modified some declarations of existing parameter lists by
adding default const parameters of type Boolean or Integer like:
...
With boolean and integer the "const" modifier does no harm, but
usually has no benefits either. It only prevents you treating the
parameter as a normal variable and modifying it.
With a reference counted string parameter the compiler can do some
optimization when it is marked as "const". That's why it is used so
much.
Earlier I thought it is safe and the compiler can spot attempts to
modify such a parameter and throw an error. No, it is not safe, the
compiler spots only the obvious cases.
Now I understand "const" should be used only when a string parameter
is not passed to any other function and there are no pointers
involved.
The speed benefit of "const" with a string is very small, it is more
like a habit to put it everywhere. With atomic types like boolean and
integer it makes no sense at all. It only adds useless noise to the
code.
It would be interesting to have a tool that removes all "const"
modifiers from parameters in Lazarus sources. Then we could measure
the differences both in speed and in stability.

----------------------------------------------------------------------
dbannon · 2024-02-05 09:37

Quote from: JuhaManninen on February 05, 2024, 06:34:18 am...
Now I understand "const" should be used only when a string parameter
is not passed to any other function and there are no pointers
involved.
Thanks for clarifing that Juha, I (superficially) scanned all 137
forum posts from Martin that mention 'const' and did not find what I
was looking for. So, had a play.

I think you were a bit harsh on the compiler, it applies the same
constriction about not changing a const parameter to subsequent
functions that, themselves get passed the parameter. And it seems
using pointers is the only way to hack your way into trouble. And,
honestly, I think when you use a pointer, you are generally saying,
"Hey, trust me, I know what I am doing". Even when reading my own
code, when I come across a pointer, I go back and read a bit more
carefully.

Looking at the forum discussions, I saw several code examples that
really demonstrated why global var are a bad thing, here I demonstrate
that (with out pointers or similar techniques) you cannot fool the
compiler !

Code: Pascal  [Select][+][-]window.onload = function(){var x1 =
document.getElementById("main_content_section"); if (x1) { var x =
document.getElementsByClassName("geshi");for (var i = 0; i Program
abc;(* {$mode objfpc}{$H+} *)var    SomeString : string =
'abc'; procedure WithVar(var AString : string);   // Will not
compilebegin    AString := 'def';        // Works without the "var"  
AString[1] := 'x';       // Works without the "var"end; procedure
Foo(const MyString : string);begin    WithVar(MyString);   
writeln(MyString);end; begin  Foo(SomeString);end.
The procedure WithVar(var AString : string) will not compile while
that modifier "var" is there. Replace it with "const" and compiler
objects to either my attempts to alter AString. With no modifier, it
compiles nicely but SomeString remains unchanged. Pretty much what you
would expect !

Davo

----------------------------------------------------------------------
Martin_fr · 2024-02-05 10:09

Code: Pascal  [Select][+][-]window.onload = function(){var x1 =
document.getElementById("main_content_section"); if (x1) { var x =
document.getElementsByClassName("geshi");for (var i = 0; i program a;
uses Sysutils;var s: ansistring;procedure foo(const a:
ansistring);begin  s:=copy(inttostr(random(999))+'x',1,3);   // this
modifies the string in a, but a must not be modified 
writeln(a);end; begin  s := copy(inttostr(random(999))+'x',1,3); //
make sure it is not a constant, but a string created at runtime 
foo(s);end.
Look at the docs "the programmer promises to the compiler the value
will not change"

Essentially "const foo string" triggers an optimization, whereby the
compiler does not increase the refcount. (Even though there is an
extra var referring to the string).

In other words, you are passing a pchar (with included length). And if
you pass any data by pointer (like the chars in a pchar) then you must
make sure that the data is not removed while you still use the
pointer.

But when you reassign the caller's "s" then you remove the data.

In practice it does not happen all that easily, because the string in
the caller often already has a higher refcount, giving it a bit more
protection (in the example I made sure it had a refcnt of 1). But it
does happen in real life. I've seen it.

About "nested calls" => it's not the nested call itself. It's the fact
that eventually you may reach the code that destroys the original
data.
E.g. in the LCL, if any method accepts "const string" and that method
can be called by a user, and that method (or any method called by it)
triggers an event to user code => then it is broken. Because the event
could destroy the data. Mind "it could", it is not known, but it can
not be proven that it does not. Because the user can put anycode into
that event.


The above code is the same as
Code: Pascal  [Select][+][-]window.onload = function(){var x1 =
document.getElementById("main_content_section"); if (x1) { var x =
document.getElementsByClassName("geshi");for (var i = 0; i program a;
uses Sysutils;var s: TMyObject;procedure foo(a: TMyObject);begin 
s.free  writeln(a.bar());end; begin  s := TMyObject.Create; 
foo(s);end.
Only, in that example you know that you shouldn't free s, while it is
in use. With strings you believe the refcount is protecting you. Only
no such luck, if you told the compiler the refcount wasn't needed.


And you get the same with "const foo: MyDynArray".

You also (though different) have a problem with Code: Pascal
 [Select][+][-]window.onload = function(){var x1 =
document.getElementById("main_content_section"); if (x1) { var x =
document.getElementsByClassName("geshi");for (var i = 0; i procedure
bar(const a: array of integer);This is an open array, it is not
refcounted. But using "const" means the data is not copied (and that
is an optimization you may really want). But without copy it's just a
pointer to the callers data. => So again, if the callers data is
removed....

----------------------------------------------------------------------
Martin_fr · 2024-02-05 10:44

And in theory also "constref boolean" or "var boolean"

Because those are passed by pointer. In an extreme case, you might
pass a bool field of some object. Then you free the object, and the
memory manager release the entire memory page. => that means access to
that address becomes an access violation.

Of course, the above is simple a glorified syntax for "bar: PBoolean"
and using "bar^". And using pointers you would know that you must make
sure the pointer remains valid.

----------------------------------------------------------------------
dbannon · 2024-02-05 23:21

Martin, I have to disagree.

In your first example, the procedure Foo is directly modifying a
global variable. That is completely avoiding any protection offered
when it is passed as a const to Foo. Foo is modifying 'S', not 'a'
(although they are the same thing). It would work exactly the same if
you don't bother to pass 'a' over there at all, you make no use of 'a'
in the procedure.

This is not an issue related to the const modifier, its an issue that
relates to global variables. Misuse of globals is a very different
matter.

Move the 'S' declaration down to below Foo's declaration and, as
expected, it does not compile (I did not try it but I am sure it
wont).

Davo

----------------------------------------------------------------------
Martin_fr · 2024-02-05 23:45

Quote from: dbannon on February 06, 2024, 12:21:05 amMartin, I have to
disagree.

In your first example, the procedure Foo is directly modifying a
global variable. That is completely avoiding any protection offered
when it is passed as a const to Foo. Foo is modifying 'S', not 'a'
(although they are the same thing). It would work exactly the same if
you don't bother to pass 'a' over there at all, you make no use of 'a'
in the procedure.
I would say
Code: Pascal  [Select][+][-]window.onload = function(){var x1 =
document.getElementById("main_content_section"); if (x1) { var x =
document.getElementsByClassName("geshi");for (var i = 0; i
 writeln(a);is "making use of 'a'".

And, at least on my systems that code prints either nothing, or trash.
Yet, if it would work it should print "x" followed by a digit.

If it works on your system, then that is purely being lucky. And I can
guaranty you, other similar code will fail to work.

The behaviour of this code is undefined (and have had that directly
confirmed by members of the compiler team).
What will happen, depends on what exactly the memory manager does,
when it re-uses mem, gets new mem, ....


QuoteThis is not an issue related to the const modifier, its an issue
that relates to global variables. Misuse of globals is a very
different matter.
You can get the same effect without globals.

Make the procedure a method on an object, and s a field on the object.
It is an example. So I kept it simple.

This has happened in real live code (without global vars involved).

Besides, even if global variables aren't good coding style, they are
valid code.
The invalid code is accessing the data of "a" via the indirection of
"s" while being in a procedure where "a" is marked as const.

This isn't just something that I observed and concluded. As stated
above, I have spoken about this directly with some of the members of
the FPC team, and it has been confirmed, that such access it a
violation of the way "const param" are meant to be used.
The remainder of the examples (non-string) examples are added by me.

----------------------------------------------------------------------
Martin_fr · 2024-02-05 23:51

Quote from: dbannon on February 06, 2024, 12:21:05 amIn your first
example, the procedure Foo is directly modifying a global variable.
That is completely avoiding any protection offered when it is passed
as a const to Foo. Foo is modifying 'S', not 'a' (although they are
the same thing). It would work exactly the same if you don't bother to
pass 'a' over there at all, you make no use of 'a' in the procedure.

Just to clarify. This isn't about the compiler "not giving an error
for modifying s" => not expected.

This is, that the author of the code is responsible for not doing so
(and the author of the code has taken that responsibility by using
"const" for the parameter, and by passing "s" to the function). 

https://www.freepascal.org/docs-html/ref/refsu67.html
QuoteIt is the programmer who tells the compiler that the contents of
the const parameter will not be changed when the routine is executed,
it is not the compiler who tells the programmer that the parameter
will not be changed.
Changing "s" in that code changes "a" and the contract is violated.
(And yes again, confirmed by the fpc team).

And again, global var is an example. In the real world this will
happen 10 nested calls down in a callback, that changes some string
value in some object.

Mind that "10 nested calls down in a callback" is within "when the
routine is executed". After all, the routine has not finished, not
returned. Any work done in calls by the routine happen "when the
routine is executed".

----------------------------------------------------------------------
JuhaManninen · 2024-02-06 06:12

Quote from: dbannon on February 05, 2024, 10:37:39 amLooking at the
forum discussions, I saw several code examples that really
demonstrated why global var are a bad thing, here I demonstrate that
(with out pointers or similar techniques) you cannot fool the compiler
!
No, global variables are not special in this regard. The errors caused
by "const" found in Lazarus sources happened to class member
variables.
Your example code is an obvious case and the compiler can spot the
error.