Date: Sat, 19 Aug 2000 21:38:10 +0200
From: "Eli Zaretskii" <eliz AT is DOT elta DOT co DOT il>
Sender: halo1 AT zahav DOT net DOT il
To: news AT jgreen4 DOT fsnet DOT co DOT uk
Message-Id: <1438-Sat19Aug2000213809+0300-eliz@is.elta.co.il>
X-Mailer: Emacs 20.6 (via feedmail 8.2.emacs20_6 I) and Blat ver 1.8.5b
CC: djgpp AT delorie DOT com
In-reply-to: <m91rps83eoo8thnrmjumrbg1i9fhdlbf8a@4ax.com> (message from Jason
	Green on Fri, 18 Aug 2000 19:57:04 +0100)
Subject: Re: Symify crash
References: <3 DOT 0 DOT 5 DOT 32 DOT 20000813122244 DOT 007c0c00 AT pop DOT mail DOT yahoo DOT com> <3405-Sun13Aug2000163046+0300-eliz AT is DOT elta DOT co DOT il> <3996E3F4 DOT C9B37F8B AT softhome DOT net> <2593-Sun13Aug2000214525+0300-eliz AT is DOT elta DOT co DOT il> <fadqpscd8dj2q2la9jbojdvuou9a9283vl AT 4ax DOT com> <7704-Fri18Aug2000203741+0300-eliz AT is DOT elta DOT co DOT il> <m91rps83eoo8thnrmjumrbg1i9fhdlbf8a AT 4ax DOT com>
Reply-To: djgpp AT delorie DOT com
Errors-To: nobody AT delorie DOT com
X-Mailing-List: djgpp AT delorie DOT com
X-Unsubscribes-To: listserv AT delorie DOT com
Precedence: bulk

> From: Jason Green <news AT jgreen4 DOT fsnet DOT co DOT uk>
> Newsgroups: comp.os.msdos.djgpp
> Date: Fri, 18 Aug 2000 19:57:04 +0100
> > > 
> > >   syms = (SymNode *)malloc(num_syms * sizeof(SymNode));
> > >   memset(syms, num_syms * sizeof(SymNode), 0);
> > >   files = (FileNode *)malloc(num_files * sizeof(FileNode));
> > >   memset(files, num_files * sizeof(FileNode), 0);
> > 
> > I must be missing something, because I don't see what's not
> > initialized here.  Care to point it out?
> 
> void *memset(void *buffer, int ch, size_t num);
> 
> The 2nd & 3rd arguments to memset are reversed.

Thanks for catching this, this is indeed a bug.

However, it cannot possibly explain the problem at hand, since this
code is in the function process_aout, which is used for a.out
executables.  DJGPP switched to COFF many years ago, so this function
is almost never used nowadays.  (In the v1.x days, it worked because
malloc returned zeroed-out storage.)

The real bug was much more elusive and subtle, but I finally hunted it
down today.  (Actually, there were two bugs.)  The source-level patch
below should solve the crashes in SYMIFY; if not, please post the
details.  This patch includes the fix for reversed arguments in memset.

> Also, ISTR that generally accepted best practice is *not* to cast the
> return from malloc() so the code should read:
> 
> syms = malloc(num_syms * sizeof(SymNode));

Yes, but that's not really a bug.  Casting malloc doesn't produce
invalid code, it just obscures possible problems from not including
stdlib.h, which in this case *is* included.

--- src/debug/common/syms.c~0	Tue Dec 14 06:52:54 1999
+++ src/debug/common/syms.c	Sat Aug 19 20:50:42 2000
@@ -157,6 +157,7 @@
   int l_pending;
   unsigned long strsize;
   char *name;
+  int i2_max;
 
   fseek(fd, ofs, 0);
   fread(&f_fh, 1, FILHSZ, fd);
@@ -219,7 +220,7 @@
 
   syms = (SymNode *)malloc(num_syms * sizeof(SymNode));
 
-  f = s = f_pending = l_pending = 0;
+  f = s = f_pending = l_pending = i2_max = 0;
   for (i=0; i<f_fh.f_nsyms; i++)
   {
     switch (f_symtab[i].e_sclass)
@@ -252,9 +253,19 @@
         if (ISFCN(f_symtab[i].e_type))
         {
           int scn = f_symtab[i].e_scnum - 1;
-          l = f_lnno[scn] + ((f_aux[i+1].x_sym.x_fcnary.x_fcn.x_lnnoptr - f_sh[scn].s_lnnoptr)/LINESZ);
-          l_pending = 1;
-          l->l_addr.l_paddr = f_symtab[i].e_value;
+
+          /* For some weird reason, sometimes x_lnnoptr is less than
+             s_lnnoptr.  We just punt for such cases, rather than
+             crash.  */
+          if (f_aux[i+1].x_sym.x_fcnary.x_fcn.x_lnnoptr >= f_sh[scn].s_lnnoptr)
+          {
+            l = f_lnno[scn]
+              + ((f_aux[i+1].x_sym.x_fcnary.x_fcn.x_lnnoptr
+                  - f_sh[scn].s_lnnoptr)/LINESZ);
+            l_pending = 1;
+            i2_max = f_sh[scn].s_nlnno - (l - f_lnno[scn]);
+            l->l_addr.l_paddr = f_symtab[i].e_value;
+          }
         }
 
         if (!valid_symbol(i))
@@ -297,7 +308,7 @@
           int i2;
           l->l_lnno = lbase;
           l++;
-          for (i2=0; l[i2].l_lnno; i2++)
+          for (i2 = 0; i2 < i2_max && l[i2].l_lnno; i2++)
             l[i2].l_lnno += lbase;
           l_pending = 0;
         }
@@ -360,9 +371,9 @@
   }
   
   syms = (SymNode *)malloc(num_syms * sizeof(SymNode));
-  memset(syms, num_syms * sizeof(SymNode), 0);
+  memset(syms, 0, num_syms * sizeof(SymNode));
   files = (FileNode *)malloc(num_files * sizeof(FileNode));
-  memset(files, num_files * sizeof(FileNode), 0);
+  memset(files, 0, num_files * sizeof(FileNode));
 
   f = s = 0;
   for (i=0; i<nsyms; i++)