RE: Re[3]: setup-2.243 Crash / Install from Local Directory / WinXP

RE: Re[3]: setup-2.243 Crash / Install from Local Directory / WinXP

[Robert Collins]

Thanks Pavel. I've no time to test this tonight, but:

==
Index: IniParseFindVisitor.cc
===================================================================
RCS file: /cvs/cygwin-apps/setup/IniParseFindVisitor.cc,v
retrieving revision 2.2
diff -u -p -r2.2 IniParseFindVisitor.cc
--- IniParseFindVisitor.cc	19 May 2002 12:54:16 -0000	2.2
+++ IniParseFindVisitor.cc	22 May 2002 14:56:13 -0000
@@ -62,7 +62,11 @@ IniParseFindVisitor::visitFile(String co
   /* Copy leading part of path to temporary buffer and unescape it */
   
   String prefix (&basePath.cstr_oneuse()[baseLength + 1]);
-  String mirror = rfc1738_unescape_part (prefix.substr(0,prefix.size()
- 1));
+  String mirror;
+  if (prefix.size())
+    mirror = rfc1738_unescape_part (prefix.substr(0,prefix.size() -
1));
+  else
+    mirror = String ();
   _Builder.parse_mirror = mirror;
   ini_init (ini_file, &_Builder);
   ==
may help.

Rob

> -----Original Message-----
> From: Pavel Tsekov [mailto:ptsekov@syntrex.com] 
> Sent: Thursday, May 23, 2002 12:32 AM
> To: Pavel Tsekov
> Cc: Robert Collins; Pavel Tsekov; Max Bowsher
> Subject: Re[3]: setup-2.243 Crash / Install from Local 
> Directory / WinXP
> 
> 
> Wednesday, May 22, 2002, 2:34:47 PM, you wrote:
> 
> PT> I can confirm this - here is the output shown from Dr. Mingw JIT:
> 
> I'm not sure this is the same crash but the crash I'm 
> reporting is easy reproducable and I think it is not the 
> expected behaviour :) Here is more info on it..
> 
> The crash is caused by this code from IniParseFindVisitor.cc, line 62:
> 
>   /* Copy leading part of path to temporary buffer and unescape it */
> 
>   String prefix (&basePath.cstr_oneuse()[baseLength + 1]);
>   String mirror = rfc1738_unescape_part 
> (prefix.substr(0,prefix.size() - 1));
> 
>   baseLenght - the length of the fullpath to the local package dir
>   basePath - the full path of a directory we are currently scanning
>              with FindFirstFile and friends.
> 
> When scannin the local package directory itself prefix would 
> be with zero length. Now prefix.size() will result in 0 thus this call
> prefix.substr(0,prefix.size() - 1) will look like this 
> prefix.substr(0, -1) i.e. give me the whole string. Ok 
> fine... Now take a look at String::substr:
> 
> String
> String::substr(size_t start, size_t len) const
> {
>   // Adapt the C++ string class
>   return string(cstr_oneuse()).substr(start, len);
> }
> 
> Ok cstr_oneuse() would return 0. This is where the crash 
> happens because the string class does strlen() when this 
> constructor is used i.e. const char *. I suggest to change this to:
> 
> string tmpStr;
> tmpStr.assign (cstr_oneuse(), theData->length); // this 
> construct is safe return tmpStr.substr (start, len);
> 
> You can also check theData->length and return empty string :)
> 
> Here is a backtrace from the crash:
> 
> #0  0x0043a1a5 in String::substr (this=0x152fa94, start=0, 
> len=4294967295)
>     at /usr/include/g++-3/std/straits.h:125
> 125         { return strlen (s); }
> (gdb) bt
> #0  0x0043a1a5 in String::substr (this=0x152fa94, start=0, 
> len=4294967295)
>     at /usr/include/g++-3/std/straits.h:125
> #1  0x0041897c in IniParseFindVisitor::visitFile (this=0x152fe14,
>     basePath=@0x152fc14, theFile=0x152fc44) at 
> IniParseFindVisitor.cc:65 #2  0x0040e89f in Find::accept 
> (this=0x152fe34, aVisitor=@0x152fe14)
>     at find.cc:71
> #3  0x00410f03 in do_local_ini (owner=0x1f0272) at ini.cc:76
> #4  0x00411a3b in do_ini_thread (h=0x400000, owner=0x1f0272) 
> at ini.cc:155 #5  0x00411f58 in do_ini_thread_reflector 
> (p=0x4723a8) at ini.cc:222 #6  0x77e602ed in _libwsock32_a_iname ()
> (gdb)
> 
> 

--
Unsubscribe info:      http://cygwin.com/ml/#unsubscribe-simple
Bug reporting:         http://cygwin.com/bugs.html
Documentation:         http://cygwin.com/docs.html
FAQ:                   http://cygwin.com/faq/

Re[5]: setup-2.243 Crash / Install from Local Directory / WinXP

[Pavel Tsekov]

Hello Robert,

Well, this will fix the problem, but String::substr() will still
cause a crash if for some reason a NULL pointer is passed to it!
Btw have you read my mail till the end ?

Wednesday, May 22, 2002, 4:57:13 PM, you wrote:

RC> Thanks Pavel. I've no time to test this tonight, but:

[snip pathc and other stuff]


--
Unsubscribe info:      http://cygwin.com/ml/#unsubscribe-simple
Bug reporting:         http://cygwin.com/bugs.html
Documentation:         http://cygwin.com/docs.html
FAQ:                   http://cygwin.com/faq/

Re[3]: setup-2.243 Crash / Install from Local Directory / WinXP

[Pavel Tsekov]

Wednesday, May 22, 2002, 2:34:47 PM, you wrote:

PT> I can confirm this - here is the output shown from Dr. Mingw JIT:

I'm not sure this is the same crash but the crash I'm reporting is
easy reproducable and I think it is not the expected behaviour :)
Here is more info on it..

The crash is caused by this code from IniParseFindVisitor.cc, line 62:

  /* Copy leading part of path to temporary buffer and unescape it */

  String prefix (&basePath.cstr_oneuse()[baseLength + 1]);
  String mirror = rfc1738_unescape_part (prefix.substr(0,prefix.size() - 1));

  baseLenght - the length of the fullpath to the local package dir
  basePath - the full path of a directory we are currently scanning
             with FindFirstFile and friends.

When scannin the local package directory itself prefix would be with
zero length. Now prefix.size() will result in 0 thus this call
prefix.substr(0,prefix.size() - 1) will look like this
prefix.substr(0, -1) i.e. give me the whole string. Ok fine...
Now take a look at String::substr:

String
String::substr(size_t start, size_t len) const
{
  // Adapt the C++ string class
  return string(cstr_oneuse()).substr(start, len);
}

Ok cstr_oneuse() would return 0. This is where the crash happens
because the string class does strlen() when this constructor is used
i.e. const char *. I suggest to change this to:

string tmpStr;
tmpStr.assign (cstr_oneuse(), theData->length); // this construct is safe
return tmpStr.substr (start, len);

You can also check theData->length and return empty string :)

Here is a backtrace from the crash:

#0  0x0043a1a5 in String::substr (this=0x152fa94, start=0, len=4294967295)
    at /usr/include/g++-3/std/straits.h:125
125         { return strlen (s); }
(gdb) bt
#0  0x0043a1a5 in String::substr (this=0x152fa94, start=0, len=4294967295)
    at /usr/include/g++-3/std/straits.h:125
#1  0x0041897c in IniParseFindVisitor::visitFile (this=0x152fe14,
    basePath=@0x152fc14, theFile=0x152fc44) at IniParseFindVisitor.cc:65
#2  0x0040e89f in Find::accept (this=0x152fe34, aVisitor=@0x152fe14)
    at find.cc:71
#3  0x00410f03 in do_local_ini (owner=0x1f0272) at ini.cc:76
#4  0x00411a3b in do_ini_thread (h=0x400000, owner=0x1f0272) at ini.cc:155
#5  0x00411f58 in do_ini_thread_reflector (p=0x4723a8) at ini.cc:222
#6  0x77e602ed in _libwsock32_a_iname ()
(gdb)


--
Unsubscribe info:      http://cygwin.com/ml/#unsubscribe-simple
Bug reporting:         http://cygwin.com/bugs.html
Documentation:         http://cygwin.com/docs.html
FAQ:                   http://cygwin.com/faq/

Re[3]: setup-2.243 Crash / Install from Local Directory / WinXP

[Pavel Tsekov]

Wednesday, May 22, 2002, 2:34:47 PM, you wrote:

PT> I can confirm this - here is the output shown from Dr. Mingw JIT:

I'm not sure this is the same crash but the crash I'm reporting is
easy reproducable and I think it is not the expected behaviour :)
Here is more info on it..

The crash is caused by this code from IniParseFindVisitor.cc, line 62:

  /* Copy leading part of path to temporary buffer and unescape it */

  String prefix (&basePath.cstr_oneuse()[baseLength + 1]);
  String mirror = rfc1738_unescape_part (prefix.substr(0,prefix.size() - 1));

  baseLenght - the length of the fullpath to the local package dir
  basePath - the full path of a directory we are currently scanning
             with FindFirstFile and friends.

When scannin the local package directory itself prefix would be with
zero length. Now prefix.size() will result in 0 thus this call
prefix.substr(0,prefix.size() - 1) will look like this
prefix.substr(0, -1) i.e. give me the whole string. Ok fine...
Now take a look at String::substr:

String
String::substr(size_t start, size_t len) const
{
  // Adapt the C++ string class
  return string(cstr_oneuse()).substr(start, len);
}

Ok cstr_oneuse() would return 0. This is where the crash happens
because the string class does strlen() when this constructor is used
i.e. const char *. I suggest to change this to:

string tmpStr;
tmpStr.assign (cstr_oneuse(), theData->length); // this construct is safe
return tmpStr.substr (start, len);

You can also check theData->length and return empty string :)

Here is a backtrace from the crash:

#0  0x0043a1a5 in String::substr (this=0x152fa94, start=0, len=4294967295)
    at /usr/include/g++-3/std/straits.h:125
125         { return strlen (s); }
(gdb) bt
#0  0x0043a1a5 in String::substr (this=0x152fa94, start=0, len=4294967295)
    at /usr/include/g++-3/std/straits.h:125
#1  0x0041897c in IniParseFindVisitor::visitFile (this=0x152fe14,
    basePath=@0x152fc14, theFile=0x152fc44) at IniParseFindVisitor.cc:65
#2  0x0040e89f in Find::accept (this=0x152fe34, aVisitor=@0x152fe14)
    at find.cc:71
#3  0x00410f03 in do_local_ini (owner=0x1f0272) at ini.cc:76
#4  0x00411a3b in do_ini_thread (h=0x400000, owner=0x1f0272) at ini.cc:155
#5  0x00411f58 in do_ini_thread_reflector (p=0x4723a8) at ini.cc:222
#6  0x77e602ed in _libwsock32_a_iname ()
(gdb)


--
Unsubscribe info:      http://cygwin.com/ml/#unsubscribe-simple
Bug reporting:         http://cygwin.com/bugs.html
Documentation:         http://cygwin.com/docs.html
FAQ:                   http://cygwin.com/faq/

Re[3]: setup-2.243 Crash / Install from Local Directory / WinXP

[Pavel Tsekov]

Hello Pavel,

Wednesday, May 22, 2002, 2:34:47 PM, you wrote:

PT> Hello Robert,

PT> Wednesday, May 22, 2002, 2:20:20 PM, you wrote:

RC>> C'mon Max, you *know* I need more detail than that.

RC>> Install from Local works fine here. Even though it's only a snapshot, I
RC>> wasn't aware of any -bugs- in the release.

PT> I can confirm this - here is the output shown from Dr. Mingw JIT:

PT> setup-2.243.exe caused an Access Violation at location 0043a1a5 in module setup-2.243.exe Reading from location 00000000.

PT> Registers:
PT> eax=00000000 ebx=00000000 ecx=015ef944 edx=00000000 esi=004839ac edi=015ef864
PT> eip=0043a1a5 esp=015ef810 ebp=015ef884 iopl=0         nv up ei pl zr na po nc
PT> cs=001b  ss=0023  ds=0023  es=0023  fs=0038  gs=0000             efl=00000246

PT> Call stack:
PT> 0043A1A5  setup-2.243.exe:0043A1A5
PT> 0041897C  setup-2.243.exe:0041897C
PT> 0040E89F  setup-2.243.exe:0040E89F
PT> 00410F03  setup-2.243.exe:00410F03
PT> 00411A3B  setup-2.243.exe:00411A3B
PT> 00411F58  setup-2.243.exe:00411F58
PT> 77E602ED  kernel32.dll:77E602ED  OpenConsoleW


PT> I'll try to debug this at home this night since I cannot access CVS
PT> now. Btw I forced it to crash by doing 'Instal from local directory'
PT> and choose a mirror named directory inside my root package directory.
                                                           |
                                          download package +
PT> It doesn't crash for me if a choose the root package directory I use
                                            the same here
PT> normally.

PT> If someone can send me a debuggable setup.exe I'll debug it ASAP :)


--
Unsubscribe info:      http://cygwin.com/ml/#unsubscribe-simple
Bug reporting:         http://cygwin.com/bugs.html
Documentation:         http://cygwin.com/docs.html
FAQ:                   http://cygwin.com/faq/