Просмотр исходного кода

Fix emit_ansistr_const: its input is not guaranteed to be #0-terminated

Also cleaned up all memory leaks where pchars were allocated, but never freed.
Before the change to dynamic arrays, these pchars were kept in the tai_string,
but now they got copied. Changed the tai_string constructor to support adding
a terminating #0, so we don't need to create intermediates just for that.
Jonas Maebe 4 месяцев назад
Родитель
Сommit
547fa426c7
7 измененных файлов с 56 добавлено и 86 удалено
  1. 15 31
      compiler/aasmcnst.pas
  2. 14 5
      compiler/aasmtai.pas
  3. 2 2
      compiler/ncal.pas
  4. 4 14
      compiler/ncgcon.pas
  5. 2 7
      compiler/ncgvmt.pas
  6. 18 21
      compiler/ngtcon.pas
  7. 1 6
      compiler/objcgutl.pas

+ 15 - 31
compiler/aasmcnst.pas

@@ -349,7 +349,8 @@ type
      class function get_dynstring_rec_name(typ: tstringtype; winlike: boolean; len: asizeint): TSymStr;
      class function get_dynstring_rec(typ: tstringtype; winlike: boolean; len: asizeint): trecorddef;
      { the datalist parameter specifies where the data for the string constant
-       will be emitted (via an internal data builder) }
+       will be emitted (via an internal data builder)
+       Note: data does not have to be #0-terminated (len specifies the length of the valid data) }
      function emit_ansistring_const(datalist: TAsmList; data: pchar; len: asizeint; encoding: tstringencoding): tasmlabofs;
      function emit_unicodestring_const(datalist: TAsmList; data: tcompilerwidestring; encoding: tstringencoding; winlike: boolean):tasmlabofs;
      { emits a tasmlabofs as returned by emit_*string_const }
@@ -364,8 +365,9 @@ type
 
      { emit a shortstring constant, and return its def }
      function emit_shortstring_const(const str: shortstring): tdef;
-     { emit a pchar string constant (the characters, not a pointer to them), and return its def }
-     function emit_pchar_const(str: pchar; len: pint; copypchar: boolean): tdef;
+     { emit a pchar string constant (the characters, not a pointer to them), and return its def;
+       len does not include the terminating #0 (will be added) }
+     function emit_pchar_const(str: pchar; len: pint): tdef;
      { emit a guid constant }
      procedure emit_guid_const(const guid: tguid);
      { emit a procdef constant }
@@ -735,8 +737,8 @@ implementation
          ait_string:
            begin
              // lengths without terminating 0
-             len1:=length(strtai.str)-1;
-             len2:=length(lother_string.str)-1;
+             len1:=strtai.len;
+             len2:=lother_string.len;
              lent:=len1+len2;
              SetLength(strtai.str,lent+1);
              { also copy null terminator }
@@ -842,7 +844,7 @@ implementation
            if fvalues.count<>1 then
              internalerror(2014070105);
            lString:=tai_string(tai_simpletypedconst(fvalues[0]).val);
-           len:=length(lString.str)-1;
+           len:=lString.len;
            tai_simpletypedconst(fvalues[0]).fdef:=carraydef.getreusable(cansichartype,len);
          end;
      end;
@@ -1704,7 +1706,6 @@ implementation
 
    function ttai_typedconstbuilder.emit_ansistring_const(datalist: TAsmList; data: pchar; len: asizeint; encoding: tstringencoding): tasmlabofs;
      var
-       s: PChar;
        startlab: tasmlabel;
        ansistrrecdef: trecorddef;
        datadef: tdef;
@@ -1714,10 +1715,10 @@ implementation
        start_internal_data_builder(datalist,sec_rodata_norel,'',datatcb,startlab);
        result:=datatcb.emit_string_const_common(st_ansistring,len,encoding,startlab);
 
-       { terminating zero included }
+       { add room for the #0-terminator }
        datadef:=carraydef.getreusable(cansichartype,len+1);
        datatcb.maybe_begin_aggregate(datadef);
-       ts:=tai_string.create_pchar(data,len+1); // +1 to include terminating 0
+       ts:=tai_string.Create_Data(data,len,true);
        datatcb.emit_tai(ts,datadef);
        datatcb.maybe_end_aggregate(datadef);
        ansistrrecdef:=datatcb.end_anonymous_record;
@@ -1850,26 +1851,14 @@ implementation
      end;
 
 
-   function ttai_typedconstbuilder.emit_pchar_const(str: pchar; len: pint; copypchar: boolean): tdef;
-     var
-       newstr: pchar;
+   function ttai_typedconstbuilder.emit_pchar_const(str: pchar; len: pint): tdef;
      begin
        result:=carraydef.getreusable(cansichartype,len+1);
        maybe_begin_aggregate(result);
-       if (len=0) and
-          (not assigned(str) or
-           copypchar) then
+       if len=0 then
          emit_tai(Tai_const.Create_8bit(0),cansichartype)
        else
-         begin
-           if copypchar then
-             begin
-               getmem(newstr,len+1);
-               move(str^,newstr^,len+1);
-               str:=newstr;
-             end;
-           emit_tai(Tai_string.Create_pchar(str,len+1),result);
-         end;
+         emit_tai(Tai_string.Create_Data(str,len,true),result);
        maybe_end_aggregate(result);
      end;
 
@@ -1926,7 +1915,6 @@ implementation
        entry : phashsetitem;
        strlab : tasmlabel;
        l : longint;
-       pc : pansichar;
        datadef : tdef;
        strtcb : ttai_typedconstbuilder;
      begin
@@ -1941,11 +1929,6 @@ implementation
 
            { include length and terminating zero for quick conversion to pchar }
            l:=length(str);
-           getmem(pc,l+2);
-           move(str[1],pc[1],l);
-           pc[0]:=chr(l);
-           pc[l+1]:=#0;
-
            datadef:=carraydef.getreusable(cansichartype,l+2);
 
            { we start a new constbuilder as we don't know whether we're called
@@ -1953,7 +1936,8 @@ implementation
            strtcb:=ctai_typedconstbuilder.create([tcalo_is_lab,tcalo_make_dead_strippable,tcalo_apply_constalign]);
 
            strtcb.maybe_begin_aggregate(datadef);
-           strtcb.emit_tai(Tai_string.Create_pchar(pc,l+2),datadef);
+           { l+1: include length byte; true: add terminating #0 }
+           strtcb.emit_tai(Tai_string.Create_Data(@str[0],l+1,true),datadef);
            strtcb.maybe_end_aggregate(datadef);
 
            current_asmdata.asmlists[al_typedconsts].concatList(

+ 14 - 5
compiler/aasmtai.pas

@@ -611,7 +611,11 @@ interface
           str : TAnsiCharDynArray;
           constructor Create(const _str : string);
           constructor Create(const _str : ansistring);
-          constructor Create_pchar(_str : pchar;length : longint);
+          { data: not guaranteed to #0-terminated
+            length: length of the data without #0 terminator (unless the #0
+              terminator itself must be included)
+            add0: add a terminating zero as part of the data after data  }
+          constructor Create_Data(data : pchar;length : longint; add0: boolean);
           destructor Destroy;override;
           constructor ppuload(t:taitype;ppufile:tcompilerppufile);override;
           procedure ppuwrite(ppufile:tcompilerppufile);override;
@@ -2445,14 +2449,19 @@ implementation
        end;
 
 
-    constructor tai_string.Create_pchar(_str : pchar;length : longint);
+    constructor tai_string.Create_Data(data : pchar;length : longint; add0: boolean);
        begin
           inherited Create;
           typ:=ait_string;
-          setlength(str,length+1);
-          str[length]:=#0;
+          setlength(str,length+ord(add0)+1);
           if length>0 then
-            move(_str^,str[0],length);
+            move(data^,str[0],length);
+          if add0 then
+            begin
+              str[length]:=#0;
+              inc(length);
+            end;
+          str[length]:=#0;
        end;
 
 

+ 2 - 2
compiler/ncal.pas

@@ -592,11 +592,11 @@ implementation
 
         if variantdispatch then
           begin
-            tcb.emit_pchar_const(pchar(methodname),length(methodname),true);
+            tcb.emit_pchar_const(pchar(methodname),length(methodname));
             if names<>'' then
               { length-1 because we added a null terminator to the string itself
                 already }
-              tcb.emit_pchar_const(pchar(names),length(names)-1,true);
+              tcb.emit_pchar_const(pchar(names),length(names)-1);
           end;
 
         { may be referred from other units in case of inlining -> global

+ 4 - 14
compiler/ncgcon.pas

@@ -220,7 +220,6 @@ implementation
     procedure tcgstringconstnode.pass_generate_code;
       var
          lastlabel: tasmlabofs;
-         pc: pchar;
          l: longint;
          pool: THashSet;
          entry: PHashSetItem;
@@ -332,15 +331,12 @@ implementation
                            l:=255
                           else
                            l:=len;
+
                           { include length and terminating zero for quick conversion to pchar }
-                          getmem(pc,l+2);
-                          if l>0 then
-                            move(asconstpchar^,pc[1],l);
-                          pc[0]:=chr(l);
-                          pc[l+1]:=#0;
                           datadef:=carraydef.getreusable(cansichartype,l+2);
                           datatcb.maybe_begin_aggregate(datadef);
-                          t:=Tai_string.Create_pchar(pc,l+2);
+                          datatcb.emit_tai(Tai_const.Create_8bit(l),cansichartype);
+                          t:=Tai_string.Create_Data(asconstpchar,l,true);
                           datatcb.emit_tai(t,datadef);
                           datatcb.maybe_end_aggregate(datadef);
                           current_asmdata.asmlists[al_typedconsts].concatList(
@@ -351,20 +347,14 @@ implementation
                         begin
                           current_asmdata.getlocaldatalabel(lastlabel.lab);
 
-                          { include terminating zero }
-                          getmem(pc,len+1);
-                          if len>0 then
-                            move(asconstpchar^,pc[0],len);
-                          pc[len]:=#0;
                           { the data includes the terminating #0 because this
                             string can be used for pchar assignments (but it's
                             also used for array-of-char assignments, in which
                             case the terminating #0 is not part of the data) }
                           datadef:=carraydef.getreusable(cansichartype,len+1);
                           datatcb.maybe_begin_aggregate(datadef);
-                          t:=Tai_string.Create_pchar(pc,len+1);
+                          t:=Tai_string.Create_Data(asconstpchar,len,true);
                           datatcb.emit_tai(t,datadef);
-                          freemem(pc);
                           datatcb.maybe_end_aggregate(datadef);
                           current_asmdata.asmlists[al_typedconsts].concatList(
                             datatcb.get_final_asmlist(lastlabel.lab,datadef,sec_rodata_norel,lastlabel.lab.name,const_align(sizeof(pint)))

+ 2 - 7
compiler/ncgvmt.pas

@@ -227,20 +227,15 @@ implementation
 
     procedure TVMTWriter.writenames(tcb: ttai_typedconstbuilder; p: pprocdeftree);
       var
-        ca : pchar;
-        len : byte;
         datatcb : ttai_typedconstbuilder;
+        len : byte;
       begin
          if assigned(p^.l) then
            writenames(tcb,p^.l);
          tcb.start_internal_data_builder(current_asmdata.AsmLists[al_const],sec_rodata,_class.vmt_mangledname,datatcb,p^.nl);
          len:=length(p^.data.messageinf.str^);
          datatcb.maybe_begin_aggregate(carraydef.getreusable(cansichartype,len+1));
-         datatcb.emit_tai(tai_const.create_8bit(len),cansichartype);
-         getmem(ca,len+1);
-         move(p^.data.messageinf.str^[1],ca^,len);
-         ca[len]:=#0;
-         datatcb.emit_tai(Tai_string.Create_pchar(ca,len),carraydef.getreusable(cansichartype,len));
+         datatcb.emit_tai(Tai_string.Create_Data(@p^.data.messageinf.str^[0],len+1,false),carraydef.getreusable(cansichartype,len+1));
          datatcb.maybe_end_aggregate(carraydef.getreusable(cansichartype,len+1));
          tcb.finish_internal_data_builder(datatcb,p^.nl,carraydef.getreusable(cansichartype,len+1),sizeof(pint));
          if assigned(p^.r) then

+ 18 - 21
compiler/ngtcon.pas

@@ -486,12 +486,13 @@ function get_next_varsym(def: tabstractrecorddef; const SymList:TFPHashObjectLis
 
     procedure tasmlisttypedconstbuilder.tc_emit_stringdef(def: tstringdef; var node: tnode);
       var
-        strlength : {$ifdef CPU8BITALU}smallint{$else}aint{$endif};
+        strlength,
+        defsize   : {$ifdef CPU8BITALU}smallint{$else}aint{$endif};
         strval    : pchar;
         ll        : tasmlabofs;
-        ca        : pchar;
         winlike   : boolean;
         hsym      : tconstsym;
+        paddedstrdata   : shortstring;
       begin
         strval:='';
         { load strval and strlength of the constant tree }
@@ -562,20 +563,18 @@ function get_next_varsym(def: tabstractrecorddef; const SymList:TFPHashObjectLis
               st_shortstring:
                 begin
                   ftcb.maybe_begin_aggregate(def);
-                  if strlength>=def.size then
+                  defsize:=def.size;
+                  if strlength>=defsize then
                    begin
-                     message2(parser_w_string_too_long,strpas(strval),tostr(def.size-1));
-                     strlength:=def.size-1;
+                     message2(parser_w_string_too_long,strval,tostr(defsize-1));
+                     strlength:=defsize-1;
                    end;
-                  ftcb.emit_tai(Tai_const.Create_8bit(strlength),cansichartype);
-                  { room for the string data + terminating #0 }
-                  getmem(ca,def.size);
-                  move(strval^,ca^,strlength);
-                  { zero-terminate and fill with spaces if size is shorter }
-                  fillchar(ca[strlength],def.size-strlength-1,' ');
-                  ca[strlength]:=#0;
-                  ca[def.size-1]:=#0;
-                  ftcb.emit_tai(Tai_string.Create_pchar(ca,def.size-1),carraydef.getreusable(cansichartype,def.size-1));
+                  paddedstrdata[0]:=chr(strlength);
+                  move(strval^,paddedstrdata[1],strlength);
+                  { fill with spaces if size is shorter }
+                  fillchar(paddedstrdata[strlength+1],defsize-strlength-1,' ');
+                  paddedstrdata[strlength+1]:=#0;
+                  ftcb.emit_tai(Tai_string.Create_Data(@paddedstrdata[0],defsize,false),carraydef.getreusable(cansichartype,defsize+1));
                   ftcb.maybe_end_aggregate(def);
                 end;
               st_ansistring:
@@ -783,7 +782,6 @@ function get_next_varsym(def: tabstractrecorddef; const SymList:TFPHashObjectLis
         hp        : tnode;
         srsym     : tsym;
         pd        : tprocdef;
-        ca        : pchar;
         pw        : tcompilerwidestring;
         i,len     : longint;
         ll        : tasmlabel;
@@ -876,14 +874,13 @@ function get_next_varsym(def: tabstractrecorddef; const SymList:TFPHashObjectLis
                   { For tp7 the maximum lentgh can be 255 }
                   if (m_tp7 in current_settings.modeswitches) and
                      (len>255) then
-                   len:=255;
-                  getmem(ca,len+1);
-                  ca[len]:=#0;
-                  if len>0 then
-                    move(tstringconstnode(node).valueas[0],ca^,len);
+                    len:=255;
                   datadef:=carraydef.getreusable(cansichartype,len+1);
                   datatcb.maybe_begin_aggregate(datadef);
-                  datatcb.emit_tai(Tai_string.Create_pchar(ca,len+1),datadef);
+                  if len>0 then
+                    datatcb.emit_tai(Tai_string.Create_Data(@tstringconstnode(node).valueas[0],len,true),datadef)
+                  else
+                    datatcb.emit_tai(Tai_string.Create_Data(nil,0,true),datadef);
                   datatcb.maybe_end_aggregate(datadef);
                 end
               else if is_constcharnode(node) then

+ 1 - 6
compiler/objcgutl.pas

@@ -151,7 +151,6 @@ procedure objcreatestringpoolentryintern(p: pchar; len: longint; pooltype: tcons
   var
     entry  : PHashSetItem;
     strlab : tasmlabel;
-    pc     : pchar;
     pool   : THashSet;
     tcb    : ttai_typedconstbuilder;
   begin
@@ -167,13 +166,9 @@ procedure objcreatestringpoolentryintern(p: pchar; len: longint; pooltype: tcons
         { Make sure strlab has a reference }
         strlab.increfs;
 
-        getmem(pc,entry^.keylength+1);
-        move(entry^.key^,pc^,entry^.keylength);
-        pc[entry^.keylength]:=#0;
-
         { add the string to the approriate section }
         tcb:=ctai_typedconstbuilder.create([tcalo_is_lab,tcalo_new_section]);
-        def:=tcb.emit_pchar_const(pc,entry^.keylength,false);
+        def:=tcb.emit_pchar_const(pchar(entry^.key),entry^.keylength);
         current_asmdata.asmlists[al_objc_pools].concatList(
           tcb.get_final_asmlist(strlab,def,stringsec,strlab.name,1)
         );