Recent

Author Topic: [SOLVED] Improvement of rtl/linux/system.pp procedure InitTLS;  (Read 2604 times)

lagprogramming

  • Sr. Member
  • ****
  • Posts: 404
Looking at the procedure I've noticed that variable "i" is declared as integer. When I moved the mouse over it, Lazarus wrote that integer is an alias for smallint.
Later the code contains "for i:=1 to phnum do", where variable "phnum" is of type dword. This means that some valid iterations might not be executed inside the loop.

The modified code removes variable "i" and replaces the line "for i:=1 to phnum do" with "for phnum:=phnum downto 1 do".
You might also be interested in reading the forum post Miscellaneous>Suggestions>Optimization of for loops in the near zero range

Existing code
Code: Pascal  [Select][+][-]
  1. procedure InitTLS; [public,alias:'FPC_INITTLS'];
  2.   const
  3.     PT_TLS = 7;
  4.     PT_DYNAMIC = 2;
  5.  
  6.   type
  7. {$ifdef CPU64}
  8.     tphdr = record
  9.       p_type,
  10.       p_flags : dword;
  11.       p_offset,
  12.       p_vaddr,
  13.       p_paddr,
  14.       p_filesz,
  15.       p_memsz,
  16.       p_align : qword;
  17.     end;
  18. {$else CPU64}
  19.     tphdr = record
  20.       p_type,
  21.       p_offset,
  22.       p_vaddr,
  23.       p_paddr,
  24.       p_filesz,
  25.       p_memsz,
  26.       p_flags,
  27.       p_align : dword;
  28.     end;
  29. {$endif CPU64}
  30.     pphdr = ^tphdr;
  31.  
  32.   var
  33.     phdr : pphdr;
  34.     phnum : dword;
  35.     i   : integer;
  36.     tls : pointer;
  37.     auxp : ppointer;
  38.     found : boolean;
  39.     size : SizeUInt;
  40.   begin
  41.     auxp:=ppointer(envp);
  42.     { skip environment }
  43.     while assigned(auxp^) do
  44.       inc(auxp);
  45.     inc(auxp);
  46.     phdr:=nil;
  47.     phnum:=0;
  48.     { now we are at the auxillary vector }
  49.     while assigned(auxp^) do
  50.       begin
  51.         case plongint(auxp)^ of
  52.           3:
  53.             phdr:=pphdr(ppointer(auxp+1)^);
  54.           5:
  55.             phnum:=pdword(auxp+1)^;
  56.         end;
  57.         inc(auxp,2);
  58.       end;
  59.     found:=false;
  60.     size:=0;
  61.     for i:=1 to phnum do
  62.       begin
  63.         case phdr^.p_type of
  64.           PT_TLS:
  65.             begin
  66.               found:=true;
  67.               inc(size,phdr^.p_memsz);
  68.               size:=Align(size,phdr^.p_align);
  69.             end;
  70.           PT_DYNAMIC:
  71.             { if the program header contains a dynamic section, the program
  72.               is linked dynamically so the dynamic linker takes care of the
  73.               allocation of the TLS segment.
  74.  
  75.               We cannot allocate it by ourself anyways as PT_TLS provides only
  76.               the size of TLS data of the executable itself
  77.              }
  78.             exit;
  79.         end;
  80.         inc(phdr);
  81.       end;
  82.     if found then
  83.       begin
  84. {$ifdef CPUI386}
  85.         { threadvars should start at a page boundary,
  86.           add extra space for the TCB }
  87.         size:=Align(size,4096)+sizeof(Pointer);
  88. {$endif CPUI386}
  89. {$ifdef CPUX86_64}
  90.         { threadvars should start at a page boundary,
  91.           add extra space for the TCB }
  92.         size:=Align(size,4096)+sizeof(Pointer);
  93. {$endif CPUX86_64}
  94.         tls:=Fpmmap(nil,size,3,MAP_PRIVATE+MAP_ANONYMOUS,-1,0);
  95.         fpset_tls(tls,size);
  96.       end;
  97.   end;
  98. {$endif INITTLS}

New code
Code: Pascal  [Select][+][-]
  1. procedure InitTLS; [public,alias:'FPC_INITTLS'];
  2.   const
  3.     PT_TLS = 7;
  4.     PT_DYNAMIC = 2;
  5.  
  6.   type
  7. {$ifdef CPU64}
  8.     tphdr = record
  9.       p_type,
  10.       p_flags : dword;
  11.       p_offset,
  12.       p_vaddr,
  13.       p_paddr,
  14.       p_filesz,
  15.       p_memsz,
  16.       p_align : qword;
  17.     end;
  18. {$else CPU64}
  19.     tphdr = record
  20.       p_type,
  21.       p_offset,
  22.       p_vaddr,
  23.       p_paddr,
  24.       p_filesz,
  25.       p_memsz,
  26.       p_flags,
  27.       p_align : dword;
  28.     end;
  29. {$endif CPU64}
  30.     pphdr = ^tphdr;
  31.  
  32.   var
  33.     phdr : pphdr;
  34.     phnum : dword;
  35.     tls : pointer;
  36.     auxp : ppointer;
  37.     found : boolean;
  38.     size : SizeUInt;
  39.   begin
  40.     auxp:=ppointer(envp);
  41.     { skip environment }
  42.     while assigned(auxp^) do
  43.       inc(auxp);
  44.     inc(auxp);
  45.     phdr:=nil;
  46.     phnum:=0;
  47.     { now we are at the auxillary vector }
  48.     while assigned(auxp^) do
  49.       begin
  50.         case plongint(auxp)^ of
  51.           3:
  52.             phdr:=pphdr(ppointer(auxp+1)^);
  53.           5:
  54.             phnum:=pdword(auxp+1)^;
  55.         end;
  56.         inc(auxp,2);
  57.       end;
  58.     found:=false;
  59.     size:=0;
  60.     for phnum:=phnum downto 1 do
  61.       begin
  62.         case phdr^.p_type of
  63.           PT_TLS:
  64.             begin
  65.               found:=true;
  66.               inc(size,phdr^.p_memsz);
  67.               size:=Align(size,phdr^.p_align);
  68.             end;
  69.           PT_DYNAMIC:
  70.             { if the program header contains a dynamic section, the program
  71.               is linked dynamically so the dynamic linker takes care of the
  72.               allocation of the TLS segment.
  73.  
  74.               We cannot allocate it by ourself anyways as PT_TLS provides only
  75.               the size of TLS data of the executable itself
  76.              }
  77.             exit;
  78.         end;
  79.         inc(phdr);
  80.       end;
  81.     if found then
  82.       begin
  83. {$ifdef CPUI386}
  84.         { threadvars should start at a page boundary,
  85.           add extra space for the TCB }
  86.         size:=Align(size,4096)+sizeof(Pointer);
  87. {$endif CPUI386}
  88. {$ifdef CPUX86_64}
  89.         { threadvars should start at a page boundary,
  90.           add extra space for the TCB }
  91.         size:=Align(size,4096)+sizeof(Pointer);
  92. {$endif CPUX86_64}
  93.         tls:=Fpmmap(nil,size,3,MAP_PRIVATE+MAP_ANONYMOUS,-1,0);
  94.         fpset_tls(tls,size);
  95.       end;
  96.   end;
  97. {$endif INITTLS}
« Last Edit: August 22, 2023, 10:56:55 am by lagprogramming »

Blaazen

  • Hero Member
  • *****
  • Posts: 3237
  • POKE 54296,15
    • Eye-Candy Controls
Re: Improvement of rtl/linux/system.pp procedure InitTLS;
« Reply #1 on: March 24, 2023, 03:56:17 pm »
Is it really SmallInt? Sometimes hovering variables at design-time gives wrong info because there can be some conditions, i.e. this:
Code: Pascal  [Select][+][-]
  1. const
  2.    TBITS_SHIFT =
  3. {$if sizeof(TBitsBase) = sizeof(word)}
  4.             4
  5. {$elseif sizeof(TBitsBase) = sizeof(dword)}
  6.             5
  7. {$elseif sizeof(TBitsBase) = sizeof(qword)}
  8.             6
  9. {$else}
  10. {$error unknown TBitsBase}
  11. {$endif}
  12.             ;
  13.    TBITS_MASK = 1 shl TBITS_SHIFT - 1;
Hovering TBITS_SHIFT shows 4 at design-time while it is 6 at runtime (64-bit Linux, file: bits.inc).
Lazarus 2.3.0 (rev main-2_3-2863...) FPC 3.3.1 x86_64-linux-qt Chakra, Qt 4.8.7/5.13.2, Plasma 5.17.3
Lazarus 1.8.2 r57369 FPC 3.0.4 i386-win32-win32/win64 Wine 3.21

Try Eye-Candy Controls: https://sourceforge.net/projects/eccontrols/files/

lagprogramming

  • Sr. Member
  • ****
  • Posts: 404
Re: Improvement of rtl/linux/system.pp procedure InitTLS;
« Reply #2 on: March 26, 2023, 10:16:19 am »
Is it really SmallInt? Sometimes hovering variables at design-time gives wrong info because there can be some conditions, i.e. this:
I don't think it matters much.
According to Free Pascal Language Reference Guide Types>Base types>Ordinal types: "The integer type is an alias to the smallint type in the default Free Pascal mode. It is an alias for the longint type in either Delphi or ObjFPC mode."
So, the maximum value of an integer type is longint, which is smaller than the maximum value of a dword(longword). Notice that variable "phnum" is dword type. This means that iterations between high(integer) and high(dword) won't run.
The proposed code removes the variable "i" which will get rid of any error regarding the compatibility between integer and dword variable types.

marcov

  • Administrator
  • Hero Member
  • *
  • Posts: 11351
  • FPC developer.
Re: Improvement of rtl/linux/system.pp procedure InitTLS;
« Reply #3 on: March 26, 2023, 01:10:29 pm »
Linux'  system.pp unit includes sysunixh.inc which includes systemh.inc which contains $mode objfpc

PascalDragon

  • Hero Member
  • *****
  • Posts: 5444
  • Compiler Developer
Re: Improvement of rtl/linux/system.pp procedure InitTLS;
« Reply #4 on: March 27, 2023, 11:20:35 pm »
Linux'  system.pp unit includes sysunixh.inc which includes systemh.inc which contains $mode objfpc

Which will not change the declaration of Integer inside the System unit, because the Integer = LongInt alias is declared in unit ObjPas which is not included inside the System unit.

lagprogramming

  • Sr. Member
  • ****
  • Posts: 404
Re: Improvement of rtl/linux/system.pp procedure InitTLS;
« Reply #5 on: May 04, 2023, 10:52:36 am »
The following patch removes the variable i by replacing the line
for i:=1 to phnum do
with
while phnum>0 do
If the patch is accepted, all loop iterations should run without errors.
Code: Pascal  [Select][+][-]
  1. diff --git a/rtl/linux/system.pp b/rtl/linux/system.pp
  2. index 96c6344672..438e535ca9 100644
  3. --- a/rtl/linux/system.pp
  4. +++ b/rtl/linux/system.pp
  5. @@ -243,7 +243,6 @@     tphdr = record
  6.    var
  7.      phdr : pphdr;
  8.      phnum : dword;
  9. -    i   : integer;
  10.      tls : pointer;
  11.      auxp : ppointer;
  12.      found : boolean;
  13. @@ -269,7 +268,7 @@     tphdr = record
  14.        end;
  15.      found:=false;
  16.      size:=0;
  17. -    for i:=1 to phnum do
  18. +    while phnum>0 do
  19.        begin
  20.          case phdr^.p_type of
  21.            PT_TLS:
  22. @@ -289,6 +288,7 @@     tphdr = record
  23.              exit;
  24.          end;
  25.          inc(phdr);
  26. +        dec(phnum);
  27.        end;
  28.      if found then
  29.        begin
  30.  
« Last Edit: August 21, 2023, 01:39:36 pm by lagprogramming »

Thaddy

  • Hero Member
  • *****
  • Posts: 14163
  • Probably until I exterminate Putin.
Re: Improvement of rtl/linux/system.pp procedure InitTLS;
« Reply #6 on: May 04, 2023, 11:01:48 am »
Code: Pascal  [Select][+][-]
  1. -    for i:=1 to phnum do
  2. +    while phnum>0 do
Risks an overflow or even endless loop. Try to make it for in do. Then I would concur.
( and start from zero )
« Last Edit: May 04, 2023, 11:04:20 am by Thaddy »
Specialize a type, not a var.

lagprogramming

  • Sr. Member
  • ****
  • Posts: 404
Re: Improvement of rtl/linux/system.pp procedure InitTLS;
« Reply #7 on: May 04, 2023, 01:46:03 pm »
Code: Pascal  [Select][+][-]
  1. -    for i:=1 to phnum do
  2. +    while phnum>0 do
Risks an overflow or even endless loop. Try to make it for in do. Then I would concur.
( and start from zero )
The patch is fine because later it also has
Code: Pascal  [Select][+][-]
  1. +        dec(phnum);

You might also be interested in the patch at
https://forum.lazarus.freepascal.org/index.php/topic,62757.msg474774.html#msg474774

PascalDragon

  • Hero Member
  • *****
  • Posts: 5444
  • Compiler Developer
Re: Improvement of rtl/linux/system.pp procedure InitTLS;
« Reply #8 on: May 05, 2023, 09:24:52 pm »
Code: Pascal  [Select][+][-]
  1. -    for i:=1 to phnum do
  2. +    while phnum>0 do
Risks an overflow or even endless loop. Try to make it for in do. Then I would concur.
( and start from zero )

You can't use forindo if there isn't an enumerable type for the second expression and the RTL code in question doesn't contain a suitable enumerable.


 

TinyPortal © 2005-2018