From 50040a2cab9975aff6974bf462a0e2635d9b84ca Mon Sep 17 00:00:00 2001 From: Jonas Maebe Date: Tue, 15 Aug 2023 21:08:58 +0200 Subject: [PATCH] default values: store as staticvarsyms in staticsymtable Previously, they were stored as localvarsyms in either the localsymtable (for procedures/functions) or as localvarsyms in the staticsymtable (for init/fini code of units/main programs). The latter was a hack (staticsymtables normally cannot contain localvarsyms) and caused the temp allocator to also allocate them as a local in fini code even if the default was only in the init code. The new approach ensures at most one copy gets allocated per unit, it doesn't require explicit initialisation (since staticvarsyms are in bss -> zeroed by default), gets rid of the localvarsyms in staticsymtables, and as a bonus solves an issue with inconsistent LLVM debug information for the localvarsym in init/fini code (since the staticsymtable is shared between the init and fini code, so was the local, and therefore we generated debug info stating it was defined in the fini code but within the scope of the init code). Resolves #40395 --- compiler/ninl.pas | 17 ++++++++++++----- compiler/psub.pas | 39 --------------------------------------- tests/webtbs/tw40395.pp | 20 ++++++++++++++++++++ tests/webtbs/tw40395a.pp | 18 ++++++++++++++++++ 4 files changed, 50 insertions(+), 44 deletions(-) create mode 100644 tests/webtbs/tw40395.pp create mode 100644 tests/webtbs/tw40395a.pp diff --git a/compiler/ninl.pas b/compiler/ninl.pas index cd47ee2d74..27a3c6c9e5 100644 --- a/compiler/ninl.pas +++ b/compiler/ninl.pas @@ -138,7 +138,7 @@ implementation symconst,symdef,symsym,symcpu,symtable,paramgr,defcmp,defutil,symbase, cpuinfo,cpubase, pass_1, - ncal,ncon,ncnv,nadd,nld,nbas,nflw,nmem,nmat,nutils, + ncal,ncon,ncnv,nadd,nld,nbas,nflw,nmem,nmat,nutils,ngenutil, nobjc,objcdef, cgbase,procinfo; @@ -445,6 +445,13 @@ implementation not (def.typ in [arraydef,recorddef,variantdef,objectdef,procvardef]) or ((def.typ=objectdef) and not is_object(def)) then internalerror(201202101); + + if df_generic in current_procinfo.procdef.defoptions then + begin + result:=cpointerconstnode.create(0,def); + exit; + end; + { extra '$' prefix because on darwin the result of makemangledname is prefixed by '_' and hence adding a '$' at the start of the prefix passed to makemangledname doesn't help (the whole point of @@ -462,20 +469,20 @@ implementation if assigned(current_procinfo) then begin { the default sym is always part of the current procedure/function } - srsymtable:=current_procinfo.procdef.localst; + srsymtable:=current_module.localsymtable; srsym:=tsym(srsymtable.findwithhash(hashedid)); if not assigned(srsym) then begin { no valid default variable found, so create it } - srsym:=clocalvarsym.create(defaultname,vs_const,def,[]); + srsym:=cstaticvarsym.create(defaultname,vs_const,def,[]); srsymtable.insertsym(srsym); + cnodeutils.insertbssdata(tstaticvarsym(srsym)); + { mark the staticvarsym as typedconst } include(tabstractvarsym(srsym).varoptions,vo_is_typed_const); include(tabstractvarsym(srsym).varoptions,vo_is_default_var); { The variable has a value assigned } tabstractvarsym(srsym).varstate:=vs_initialised; - { the variable can't be placed in a register } - tabstractvarsym(srsym).varregable:=vr_none; end; result:=cloadnode.create(srsym,srsymtable); end diff --git a/compiler/psub.pas b/compiler/psub.pas index 1a430a3cab..dfbf5a6d25 100644 --- a/compiler/psub.pas +++ b/compiler/psub.pas @@ -275,34 +275,6 @@ implementation PROCEDURE/FUNCTION BODY PARSING ****************************************************************************} - procedure initializedefaultvars(p:TObject;arg:pointer); - var - b : tblocknode; - begin - if tsym(p).typ<>localvarsym then - exit; - with tabstractnormalvarsym(p) do - begin - if (vo_is_default_var in varoptions) and (vardef.size>0) then - begin - b:=tblocknode(arg); - b.left:=cstatementnode.create( - ccallnode.createintern('fpc_zeromem', - ccallparanode.create( - cordconstnode.create(vardef.size,sizeuinttype,false), - ccallparanode.create( - caddrnode.create_internal( - cloadnode.create(tsym(p),tsym(p).owner)), - nil - ) - ) - ), - b.left); - end; - end; - end; - - procedure initializevars(p:TObject;arg:pointer); var b : tblocknode; @@ -320,8 +292,6 @@ implementation cloadnode.create(defaultconstsym,defaultconstsym.owner)), b.left); end - else - initializedefaultvars(p,arg); end; end; @@ -365,15 +335,6 @@ implementation current_filepos:=current_procinfo.entrypos; current_procinfo.procdef.localst.SymList.ForEachCall(@initializevars,block); current_filepos:=oldfilepos; - end - else if current_procinfo.procdef.localst.symtabletype=staticsymtable then - begin - { for program and unit initialization code we also need to - initialize the local variables used of Default() } - oldfilepos:=current_filepos; - current_filepos:=current_procinfo.entrypos; - current_procinfo.procdef.localst.SymList.ForEachCall(@initializedefaultvars,block); - current_filepos:=oldfilepos; end; if assigned(current_procinfo.procdef.parentfpstruct) then diff --git a/tests/webtbs/tw40395.pp b/tests/webtbs/tw40395.pp new file mode 100644 index 0000000000..fb9186292c --- /dev/null +++ b/tests/webtbs/tw40395.pp @@ -0,0 +1,20 @@ +{ %norun } + +{$mode objfpc} + +unit tw40395; + +interface + +implementation + +var x: TRTLCriticalSection; + +initialization +x := default(TRTLCriticalSection); +InitCriticalSection(x); + +finalization +DoneCriticalsection(x); + +end. diff --git a/tests/webtbs/tw40395a.pp b/tests/webtbs/tw40395a.pp new file mode 100644 index 0000000000..a79eb0610c --- /dev/null +++ b/tests/webtbs/tw40395a.pp @@ -0,0 +1,18 @@ +{ %norun } + +{$mode objfpc} + +unit tw40395a; + +interface + +implementation + +var x: TRTLCriticalSection; + +finalization +x := default(TRTLCriticalSection); +InitCriticalSection(x); +DoneCriticalsection(x); + +end.