Jump to content

Mahdi Safsafi

Members
 View Profile  See their activity

  • Content Count

    383

  • Joined

  • Last visited

  • Days Won

    10

Posts posted by Mahdi Safsafi

  1. Posted

    Excellent work ... Thanks !

    I was using the tool with some map file and I got an AV error. I started investigating and I found a bug in the map parser :  

    // debug.info.reader.map.pas (line 349):
var SegmentID: Cardinal := HexToInt16(Reader.LineBuffer, n);

    The SegmentID should be read as a DECIMAL value and not as a HEX ! if the map file contains more than 9 segment, the next segment is emitted like (0010:xxxxx) but you're reading it like a hex (so its ID becomes 16 !) and then there is this line  

    FSegments[ListIndex] := Result; // FSegments[10 .. ListIndex - 1] = nil => AV

     A simple fix :  

      function DecToInt32(const s: string; var Offset: integer): integer;
  var
    P: PChar;
  begin
    Result := 0;
    P := PChar(s);
    while Ord(P^) in [$30 .. $39] do
    begin
      Result := (Result * 10) + (Ord(P^) - $30);
      Inc(P);
      Inc(Offset);
    end;
  end;
// ...
var SegmentID: Cardinal := DecToInt32(reader.LineBuffer, n);

     

    • Thanks 3

  4. Posted · Edited by Mahdi Safsafi

    @Kas Ob. No ! its more complicated than what you're describing ! 

    Its not about the ability of the compiler to generate more than 2GB ! its about the virtual memory ! On x64, the OS can load DLL at the highest address and your hook can be located at the lowest address ... you see ?

    Also what happens if the variable is created at runtime (through a call to MM)!

  7. Posted · Edited by Mahdi Safsafi

    When you ask for memory, MM(FastMM) asks the OS for a large chunks and then it splits them and gives you a piece (based on the size you need). When the object is destroyed (free), the memory is returned to the MM. Now, based on the returned size, the MM may either choose to recycle the object location (if its small) or return the memory to the OS (a real-free-op).

    What you're doing in MultiplyStr is not just wasteful but extremely harmful ! For each iteration you're reallocating memory. Allocating a new block and copying the old block to the new one. It's very important to know that small block are implemented as a segregated list. i.e if you ask for a 32 bytes, MM on reality allocates an entire table i.e 32x32=1024 bytes and yields first block. In your example you said you used 1GB ! this is extremely bad because you're not economizing resources and you'll quickly run out of memory i.e another thread that asks for a large chunk.

    It's indeed a good practice to pre-allocate memory : 

    function MultiplyStr(aStr: string; aMultiplier: integer): string;
var i: Integer;
begin
  SetLength(Result, length(aStr) * aMultiplier);
  for i := 1 to aMultiplier do
    Result := Result + aStr;
end;

     Please run the above and notice the memory and performance !!!

    Also a small remark ! aStr should be const !!!

    • Thanks 1

  9. Posted

    2 minutes ago, Kas Ob. said:

    The packer will not see or handle the original file, it will fed the patched one, the process of building usually like this in sequenced event or batch command

    1) Build, this include compile/link/EL etc..

    2) Protect/Pack the generated EXE

    3) Sign

    4) Build the setup/installer and sing it again

     

    With the proposed Patching

    1) Build, this include compile/link/EL etc..

    2) Run one time and generate a patched version

    3) Protect/Pack the patched file

    4) Sign

    5) Build the setup/installer and sing it again

    Ah I see ... I thought you were referring to :

    - Build

    - Pack

    - Run one time and generate a patched version

    - ...

     

    Thanks ! I'll investigate further on your idea and if all works great, I'll release it as an IDE plugin 🙂 

  10. Posted

    Thanks I got clearly your idea ... but I have some hard time to digest this statement : 

    Quote

    In such we don't have any sort of limitation on any tools to be used from simple packing/compressing ( UPX..) to protecting ( WinLicense, ASProtect ..)

    Technically this won't work ! The packer unpacks the exe in memory, then DDetours applies patch ... right ? But now we ended-up with an unpacked exe ... You can't simply dump this on disk !

  11. Posted

    2 hours ago, Kas Ob. said:

    Most likely no, as i always prefer security over speed, see with DDetours i will lose DEP or the ability to use an EXE packers/protector, so weighing my options i will not with this, not now at least, but.. for this exact function in other applications it can have huge performance impact specially on DB operations ...

    Why ? the packer encrypts/compresses the exe in disk but soon the exe is loaded in memory, packer had to decrypt/decompress the exe and DDetours applies hook at runtime (in memory). So I don't think that using DDetours prevents anyone from using a Packer. Please if you encountered such a case send me some details 🙂 

    Quote

    There is many more important functions do have more speed impact on application written by Delphi, two of them are disguised and looks innocents but i hate them the most as they are essential everywhere even more then SetLength, they are outdated and will have impact on the IDE as well any almost any application from DB (operation specially), HTTP, HTML, logging .... they are everywhere.

    IntToStr and StrToInt these also affect the IDE from scrolling to disassembler view, now to Mahdi question and suggestion

    Indeed ! Personally I spotted many place where core RTL functions were doing crappy things. 

    Quote

    How about a spin-off or a new feature to DDetours, where a hooks will be applied to the file as a patch then remove itself, lets say PatchSelf is procedure will be called from initialization and it will apply any number of hooks/patchs for a copy of the EXE then patch PatchSelf itself to make it clean, this way we can safely patch the RTL/VCL without losing the DEP or the protector, i think it is doable and very useful and will not break RTL compatibility, yet any enhancement/replacement of these functions will be useful for everyone ! (the impact on debug and log error will be minimum but known and predictable), the tricky one will be handling DLLs, away from that i suggest to make this cold patching form itself to shorten the process, although a tool that look for specific signature for a constant record to replace one by one also doable, but patching itself at first run or with a command line to self is cool and less prone to bugs,

    so what do you think ?

    Here is what I understand from your idea (please correct me if I'm wrong): I'm I right to think that you want DDetours to hook functions on the first run and then generates a patched exe file that eventually will be saved on disk ? 

    If so, then this can't be done because DDetours can't pack/compress exe ... In other word, it will fail the day you use a packer.

     

    Thanks for your ideas 🙂  

  14. Posted

    10 minutes ago, Fr0sT.Brutal said:

    Fantastic! This function is undocumented and couldn't be found in System for some reason... I added it to my TEnum<T> implementation

    It's an intrinsic and has no implementation ... the function is evaluated on the fly (no code will be generated). BTW, Spring4D makes extensively use of it.

  15. Posted

    3 minutes ago, Mike Torrettinni said:

    I need to keep some of my own personal quirks even in coding... if that means my prefixes are trivial and waste of time... if that means I will never be Delphi expert... if that means I annoy someone on the internet... too bad, I like them! 😉

    No ! I'm sure you'll become a good developer and BTW I don't find your topics annoying. I just don't want you to spend your valuable time on something that doesn't really make difference. You said you're working alone ... so literally you can pick whatever naming you like. In other word, your topic is just a color and you know that there is no color better than other 🙂 

    • Like 1

  18. Posted

    24 minutes ago, David Heffernan said:

    I'm just saying that it's very unlikely that there will be real world code that suffers. That said, I don't know about weak refs so that could be significant. 

    Maybe but its not just related to jagged array or weak refs. Here is a simple example where a copy happens too: 

    procedure TForm1.Button1Click(Sender: TObject);
var
  // array of simple type
  LArray, LArray2: array of Integer;
begin
  SetLength(LArray, 100);
  LArray2 := LArray;
  SetLength(LArray2, 100); // copy happens here too
end;

    With all that being said, I think that a fix at the RTL level is necessary.

  21. Posted

    2 minutes ago, David Heffernan said:

    It was you that referred to jagged arrays. Once you start using them, for rectangular data, you've given up caring about performance.

    All the points I demonstrated are related to the use of SetLength without a check.

    Quote

    I didn't see any copying when I looked at this. I don't see any evidence that performance is a significant issue here. 

    A copy happens when array contains weak-references: 

    // DynArraySetLength:
if SysHasWeakRef(PTypeInfo(ElTypeInfo)) then
begin
...
	GetMem(pp, neededSize);
    FillChar((PByte(pp) + SizeOf(TDynArrayRec))^, minLength * elSize, 0);
    MoveArray(PByte(pp) + SizeOf(TDynArrayRec), PByte(p) + SizeOf(TDynArrayRec), ElTypeInfo, minLength);
end

     

  23. Posted

    1 hour ago, David Heffernan said:

    It's just a realloc of a block the same size, which is a null op and nothing happens. Unless you have a pathologically insane memory manager. But no memory manager I know of would do anything other than null op for this realloc.

    In fact it can be a serious bottleneck !!! 

    1 - A copy operation may happen even if the block wasn't resized (same size) ... weak-ref : 

    procedure DynArraySetLength(var a: Pointer; typeInfo: Pointer; dimCnt: NativeInt; lengthVec: PNativeint);
//....
if SysHasWeakRef(PTypeInfo(ElTypeInfo)) then
      begin
        if newLength < oldLength then
          minLength := newLength
        else
          minLength := oldLength;
        GetMem(pp, neededSize);
        FillChar((PByte(pp) + SizeOf(TDynArrayRec))^, minLength * elSize, 0);
        if p <> nil then
        begin
         // ---> here <---
          MoveArray(PByte(pp) + SizeOf(TDynArrayRec),
                     PByte(p) + SizeOf(TDynArrayRec), ElTypeInfo, minLength);
          if newLength < oldLength then
            FinalizeArray(PByte(p) + SizeOf(TDynArrayRec) + newLength*elSize, ElTypeInfo, oldLength - newLength);
          FreeMem(p);
        end;
      end

    2 - The operation executes on O(n) when the array is multidimensional : 

     // Take care of the inner dimensions, if any
  if dimCnt > 1 then
  begin
    Inc(lengthVec);
    Dec(dimCnt);
    i := 0;
    try
      while i < newLength do
        begin
          DynArraySetLength(PPointerArray(p)[i], ElTypeInfo, dimCnt, lengthVec);
          Inc(i);
        end;
    except
      // Free arrays on exception
      for j := 0 to i  do
        _DynArrayClear(PPointerArray(p)[j], ElTypeInfo);
      _DynArrayClear(p, typeInfo);
      raise;
    end;
  end;
 //---------------------------------
 var
  LArray: array of array of Integer;
begin
  SetLength(LArray, 100,10);
  SetLength(LArray, 100,10); // DynArraySetLength x100
end;

    3 - The function can ruin the cache ! because it dereferences some rtti data and it relies on MM to check for the block size. If the check was implemented inside the function than we won't dereference any unnecessary data.

    4 - Adding a simple check to compare old vs new length will be much better and avoids all the issues above.

    1 hour ago, Mike Torrettinni said:

    I would file it if I had a case where the changes would have meaningful effect, but I don't have one. Maybe it's better not to waste their time on this.

     

    I think that you should fire a case.

    • Like 1
×