Re: Patch to allow trailing dots on managed mounts
Reini Urban wrote: Thinking some more about this, there are really some inconsistencies with the current and proposed behavior that I don't like. [...] I have no strong opinion in these issues (yet), but please look also at the related ending-colon ':extension' problem on NTFS. Such files are also not listed, but probably should be. Why are you hijacking this thread for something unrelated? The alternate streams are not seperate files, they are just additional file data. If the need arises then standalone tools should be made to access them, just like getfacl and friends. They should not be treated as seperate files because they're not. Brian
stopping floppy seeks (Was: available for test: findutils-20041219-1)
Brian Dessent wrote:
Let me say it again. This is not new behavior:
2003-08-05 Pavel Tsekov ptsekov AT gmx.net
* path.cc (cygdrive_getmntent): Do not skip over drives of type
DRIVE_REMOVABLE.
Perhaps you should be discussing this with Pavel.
Okay, I misunderstood. I thought that you were saying someone had
posted a patch that would prevent checking floppy drives in that section
of the code. I now see that it used to be the case that this was done,
and the above patch removed that functionality.
I have no idea what Pavel's intentions were with his change. I can only
guess it was to support /cygdrive use with some form of removable media,
perhaps floppy, perhaps otherwise. However at the time it was
committed, there was no mount checking code in find, and so there were
no spurious floppy seeks for opening a login shell and many other
activities. I will CC him on this email to see if he wants to clarify.
It seems to me that making this behavior settable through a CYGWIN env
option would satisy everyone, but I'm also quite sure that no patch I
submit to implement this would be accepted, mainly due to not having a
copyright assignment on file.
Here is a patch. If $CYGWIN does not contain removable (or contains
noremovable) then /cygdrive's where GetDriveType() returns
DRIVE_REMOVABLE are skipped, avoiding the annoying floppy seeks.
CYGWIN=removable works the same as current code.
Note: I don't know if this would be considered trivial or not. Nor do I
know if it satisfies Pavel's needs. Just thought I'd post it anyway.
2004-12-24 Brian Dessent [EMAIL PROTECTED]
* environ.cc: Add extern decl for `cygdrive_removable'.
(struct parse_thing): Add entry for `[no]removable'.
* path.cc (cygdrive_getmntent): Ignore drive letters of
removable drives, unless `cygdrive_removable' set.Index: src/winsup/cygwin/environ.cc
===
RCS file: /cvs/src/src/winsup/cygwin/environ.cc,v
retrieving revision 1.105
diff -u -p -r1.105 environ.cc
--- src/winsup/cygwin/environ.cc3 Dec 2004 23:49:06 - 1.105
+++ src/winsup/cygwin/environ.cc24 Dec 2004 09:12:45 -
@@ -31,6 +31,7 @@ extern bool ignore_case_with_glob;
extern bool allow_ntea;
extern bool allow_smbntsec;
extern bool allow_winsymlinks;
+extern bool cygdrive_removable;
extern bool strip_title_path;
extern int pcheck_case;
extern int subauth_id;
@@ -537,6 +538,7 @@ static struct parse_thing
{ntea, {func: set_ntea}, isfunc, NULL, {{0}, {s: yes}}},
{ntsec, {func: set_ntsec}, isfunc, NULL, {{0}, {s: yes}}},
{smbntsec, {func: set_smbntsec}, isfunc, NULL, {{0}, {s: yes}}},
+ {removable, {cygdrive_removable}, justset, NULL, {{false}, {true}}},
{reset_com, {reset_com}, justset, NULL, {{false}, {true}}},
{strip_title, {strip_title_path}, justset, NULL, {{false}, {true}}},
{subauth_id, {func: subauth_id_init}, isfunc, NULL, {{0}, {0}}},
Index: src/winsup/cygwin/path.cc
===
RCS file: /cvs/src/src/winsup/cygwin/path.cc,v
retrieving revision 1.335
diff -u -p -r1.335 path.cc
--- src/winsup/cygwin/path.cc 23 Dec 2004 21:37:43 - 1.335
+++ src/winsup/cygwin/path.cc 24 Dec 2004 09:12:57 -
@@ -2301,6 +2301,9 @@ mount_item::getmntent ()
return fillout_mntent (native_path, posix_path, flags);
}
+/* If true, removable /cygdrive's should be returned by getmntent() */
+bool cygdrive_removable;
+
static struct mntent *
cygdrive_getmntent ()
{
@@ -2316,7 +2319,8 @@ cygdrive_getmntent ()
break;
__small_sprintf (native_path, %c:\\, drive);
- if (GetFileAttributes (native_path) == INVALID_FILE_ATTRIBUTES)
+ if ((!cygdrive_removable GetDriveType (native_path) ==
DRIVE_REMOVABLE) ||
+ GetFileAttributes (native_path) == INVALID_FILE_ATTRIBUTES)
{
_my_tls.locals.available_drives = ~mask;
continue;
[Patch] Fix buffer overflow in kill utility
In kill.cc there exists the possibility to overflow the char buf[80]
array by supplying malformed command line arguments.
An attacker could use this to overwrite the return value on the stack
and execute arbitrary code, but the amount of space available on the
stack for shellcode is approx 108 bytes so you'd have to be mighty
creative to do anything significant with it. A far-fetched scenario
might be some kind of perl or other CGI script running under Apache that
somehow allows a user-specified signal name to reach the command line of
/bin/kill. Emphasis on the far-fetched part though.
Example:
$ /bin/kill -s `perl -e 'print Ax200'`
Segmentation fault (core dumped)
As far as I can tell from CVS history this has existed in kill.cc since
its first version (~5 years.) Trivial patch below.
2005-02-26 Brian Dessent [EMAIL PROTECTED]
* kill.cc (getsig): Use snprintf to prevent overflowing `buf'.Index: winsup/utils/kill.cc
===
RCS file: /cvs/src/src/winsup/utils/kill.cc,v
retrieving revision 1.25
diff -u -p -r1.25 kill.cc
--- winsup/utils/kill.cc13 Nov 2004 16:30:19 - 1.25
+++ winsup/utils/kill.cc27 Feb 2005 02:29:40 -
@@ -87,7 +87,7 @@ getsig (const char *in_sig)
sig = in_sig;
else
{
- sprintf (buf, SIG%s, in_sig);
+ snprintf (buf, sizeof(buf), SIG%s, in_sig);
sig = buf;
}
intsig = strtosigno (sig) ?: atoi (in_sig);
[patch] fix for cygcheck -s if run from /usr/bin
Currently, if you run cygcheck -s with the current directory as /usr/bin
you get every cyg*.dll found twice, once with .\ prefix and the second
time with \cygwin\bin\ prefix. The user gets a spurious Multiple
Cygwin DLLs found warning even if there is only one present.
The following patch tries to correct this. In init_paths(), the
paths[1] value is populated by GetCurrentDirectory() instead of just
.. This causes the existing duplicate checking code in add_path() to
reject a later attempt to add a directory from $PATH that is the same as
CWD.
However, this also means that if . is in $PATH it will no longer be
rejected by that same duplicate checking code, so init_paths() is also
modified to not add . since we already have the CWD added explicitly.
Finally, in dump_sysinfo() the loop is changed to check starting with
paths[1] instead of paths[0], since paths[0] is a special placeholder
value that is initialized to .. paths[1] contains the CWD anyway so
there's no need to examine paths[0].
Brian
===
2005-03-24 Brian Dessent [EMAIL PROTECTED]
* cygcheck.cc (init_paths): Use full path instead of . for the
current directory. Do not add . if present in $PATH.
(dump_sysinfo): Skip placeholder first value of paths[].Index: cygcheck.cc
===
RCS file: /cvs/src/src/winsup/utils/cygcheck.cc,v
retrieving revision 1.64
diff -u -p -r1.64 cygcheck.cc
--- cygcheck.cc 18 Nov 2004 05:20:23 - 1.64
+++ cygcheck.cc 24 Mar 2005 09:41:40 -
@@ -158,7 +158,12 @@ init_paths ()
{
char tmp[4000], *sl;
add_path ((char *) ., 1); /* to be replaced later */
- add_path ((char *) ., 1); /* the current directory */
+
+ if (GetCurrentDirectory (4000, tmp))
+add_path (tmp, strlen (tmp));
+ else
+display_error (init_paths: GetCurrentDirectory());
+
if (GetSystemDirectory (tmp, 4000))
add_path (tmp, strlen (tmp));
else
@@ -180,7 +185,8 @@ init_paths ()
while (1)
{
for (e = b; *e *e != ';'; e++);
- add_path (b, e - b);
+ if (strncmp(b, ., 1) strncmp(b, .\\, 2))
+ add_path (b, e - b);
if (!*e)
break;
b = e + 1;
@@ -1237,7 +1243,7 @@ dump_sysinfo ()
if (givehelp)
printf (Looking for various Cygnus DLLs... (-v gives version info)\n);
int cygwin_dll_count = 0;
- for (i = 0; i num_paths; i++)
+ for (i = 1; i num_paths; i++)
{
WIN32_FIND_DATA ffinfo;
sprintf (tmp, %s/*.*, paths[i]);
[patch] dup_ent does not set dst when src is NULL
In net.cc, there are several cases where dup_ent() is used as follows:
dup_ent (servent_buf, getservbyname (name, proto), t_servent);
syscall_printf (%p = getservbyname (%s, %s),
_my_tls.locals.servent_buf, name, proto);
return _my_tls.locals.servent_buf;
This presents a problem if getservbyname() returns NULL, because
dup_ent just returns NULL, it does not modify 'dst'. This results in
the function returning the previous successful value if the
get_foo_by_bar() function returned NULL. This seems to be applicable to
getservbyname(), getservbyport(), gethostbyaddr(), and gethostbyname().
In the case of gethostbyname() there's also another bug in that there
will be a spurious debug_printf() about dup_ent failing if the address
simply didn't resolve. That should probably be fixed too but I wanted
to be sure the patch stayed trivial.
A simple testcase that demonstrates the problem:
#include stdio.h
#include string.h
#include netdb.h
#include sys/socket.h
#include netinet/in.h
void mygetservbyname(char *serv, char *proto)
{
struct servent *p;
if((p = getservbyname(serv, proto)))
printf(getservbyname(\%s\, \%s\) success, port = %u\n,
serv, proto, (unsigned int)ntohs (p-s_port));
else
printf(getservbyname(\%s\, \%s\) returns NULL\n, serv, proto);
}
int main(int argc, char **argv)
{
mygetservbyname(25, tcp);
mygetservbyname(auth, tcp);
mygetservbyname(25, tcp);
return 0;
}
$ ./getservbyname
getservbyname(25, tcp) returns NULL
getservbyname(auth, tcp) success, port = 113
getservbyname(25, tcp) success, port = 113
Brian
===
2005-04-05 Brian Dessent [EMAIL PROTECTED]
* net.cc (__dup_ent): Make dst point to NULL if src is NULL.
Free dst if it was previously allocated to not leak memory.Index: net.cc
===
RCS file: /cvs/src/src/winsup/cygwin/net.cc,v
retrieving revision 1.186
diff -u -p -r1.186 net.cc
--- net.cc 24 Mar 2005 14:04:06 - 1.186
+++ net.cc 6 Apr 2005 05:17:50 -
@@ -387,6 +387,9 @@ __dup_ent (unionent *dst, unionent *src
if (!src)
{
set_winsock_errno ();
+ if(dst)
+free(dst);
+ dst = NULL;
return NULL;
}
Re: [patch] dup_ent does not set dst when src is NULL
Christopher Faylor wrote: Thanks for the patch, but I went out of my way to avoid freeing the buffer when I maded changes to dup_ent a couple of weeks ago. I don't want to revert to doing that again, so I've just used the return value in all cases. Thanks for taking care of that. My original fix did more or less what you have done, by checking the return value, but I submitted the other way because it was much shorter and I didn't want to send anything non-trivial. Hmm, maybe if my printer had some ink in it I could print out that copyright assignment form... Brian
[patch] gcc4 fixes
This is just a trivial change of argument to execl() testcases, which
supresses the warning 'missing sentinel in function call' in gcc4 that
causes the tests to fail.
winsup/testsuite
2005-05-17 Brian Dessent [EMAIL PROTECTED]
* winsup.api/signal-into-win32-api.c (main): Use 'NULL' instead
of '0' in argument list to avoid compiler warning with gcc4.
* winsup.api/ltp/execle01.c (main): Ditto.
* winsup.api/ltp/execlp01.c (main): Ditto.
* winsup.api/ltp/fcntl07.c (do_exec): Ditto.
* winsup.api/ltp/fcntl07B.c (do_exec): Ditto.
This fixes the problem of mmap() not working with gcc4.
winsup/cygwin
2005-05-17 Brian Dessent [EMAIL PROTECTED]
* mmap.cc (mmap64): Move 'granularity' into file scope so that
it will be initialized.Index: winsup.api/signal-into-win32-api.c
===
RCS file: /cvs/src/src/winsup/testsuite/winsup.api/signal-into-win32-api.c,v
retrieving revision 1.3
diff -u -p -r1.3 signal-into-win32-api.c
--- winsup.api/signal-into-win32-api.c 23 Jan 2003 21:21:28 - 1.3
+++ winsup.api/signal-into-win32-api.c 17 May 2005 20:12:57 -
@@ -37,7 +37,7 @@ main (int argc, char** argv)
return 2;
}
else if (pid == 0)
-execl ( argv[0], argv[0], child, 0 );
+execl ( argv[0], argv[0], child, (char *)NULL );
else
{
sleep_stage = 0;
Index: winsup.api/ltp/execle01.c
===
RCS file: /cvs/src/src/winsup/testsuite/winsup.api/ltp/execle01.c,v
retrieving revision 1.3
diff -u -p -r1.3 execle01.c
--- winsup.api/ltp/execle01.c 24 Jan 2003 01:09:39 - 1.3
+++ winsup.api/ltp/execle01.c 17 May 2005 20:12:57 -
@@ -172,7 +172,7 @@ main(int ac, char **av)
*/
switch(pid=fork()) {
case 0: /* CHILD - Call execle(2) */
- execle(test, test, 0, environ);
+ execle(test, test, (char *)NULL, environ);
/* should not get here!! if we do, the parent will fail the Test
Case */
exit(errno);
case -1:/* ERROR!!! exit now!!*/
Index: winsup.api/ltp/execlp01.c
===
RCS file: /cvs/src/src/winsup/testsuite/winsup.api/ltp/execlp01.c,v
retrieving revision 1.3
diff -u -p -r1.3 execlp01.c
--- winsup.api/ltp/execlp01.c 24 Jan 2003 01:09:39 - 1.3
+++ winsup.api/ltp/execlp01.c 17 May 2005 20:12:57 -
@@ -171,7 +171,7 @@ main(int ac, char **av)
*/
switch(pid=fork()) {
case 0: /* CHILD - Call execlp(2) */
- execlp(/usr/bin/test, /usr/bin/test, 0);
+ execlp(/usr/bin/test, /usr/bin/test, (char *)NULL);
/* should not get here!! if we do, the parent will fail the Test
Case */
exit(errno);
case -1:/* ERROR!!! exit now!!*/
Index: winsup.api/ltp/fcntl07.c
===
RCS file: /cvs/src/src/winsup/testsuite/winsup.api/ltp/fcntl07.c,v
retrieving revision 1.3
diff -u -p -r1.3 fcntl07.c
--- winsup.api/ltp/fcntl07.c24 Jan 2003 01:09:39 - 1.3
+++ winsup.api/ltp/fcntl07.c17 May 2005 20:12:57 -
@@ -375,7 +375,7 @@ do_exec(const char *prog, int fd, const
case -1:
return(-1);
case 0:/* child */
- execlp(prog, openck, -T, pidname, 0);
+ execlp(prog, openck, -T, pidname, (char *)NULL);
/* the ONLY reason to do this is to get the errno printed out */
fprintf(stderr, exec(%s, %s, -T, %s) failed. Errno %s [%d]\n,
Index: winsup.api/ltp/fcntl07B.c
===
RCS file: /cvs/src/src/winsup/testsuite/winsup.api/ltp/fcntl07B.c,v
retrieving revision 1.3
diff -u -p -r1.3 fcntl07B.c
--- winsup.api/ltp/fcntl07B.c 24 Jan 2003 01:09:39 - 1.3
+++ winsup.api/ltp/fcntl07B.c 17 May 2005 20:12:57 -
@@ -374,7 +374,7 @@ do_exec(const char *prog, int fd, const
case -1:
return(-1);
case 0:/* child */
- execlp(prog, openck, -T, pidname, 0);
+ execlp(prog, openck, -T, pidname, (char *)NULL);
/* the ONLY reason to do this is to get the errno printed out */
fprintf(stderr, exec(%s, %s, -T, %s) failed. Errno %s [%d]\n,
Index: mmap.cc
===
RCS file: /cvs/src/src/winsup/cygwin/mmap.cc,v
retrieving revision 1.109
diff -u -r1.109 mmap.cc
--- mmap.cc 2 May 2005 03:50:07 - 1.109
+++ mmap.cc 17 May 2005 22:40:14 -
@@ -500,14 +500,14 @@
}
}
+static DWORD granularity = getshmlba ();
+
extern C void *
mmap64 (void *addr, size_t len, int prot, int flags, int fd, _off64_t off)
{
syscall_printf (addr %x, len %u, prot %x, flags %x, fd %d, off %D,
addr, len, prot, flags
Re: [patch] gcc4 fixes
Christopher Faylor wrote: Go ahead and check these in but please use GNU formatting conventions, i.e., it's (char *) NULL, not (char *)NULL. Actually, isn't just NULL sufficient? I must have had C++ on the mind, thinking that the cast was necessary. Sorry but no. This is a workaround. We need to fix the actual problem. Certainly. I fully admit I have no real idea what the 'actual' problem is yet. Brian
[patch] update documentation Was: cygwin-host-setup does not install sshd
admin wrote:
Thanks so much that worked like a charm.
umount -A to remove all mounts, and then delete the cygwin install
directory. Rummaging around in the registry is not recommended.
http://www.cygwin.com/faq/faq_2.html#SEC20 --- when removing the two
registry values suggested there didnt work, i just removed anything, like i
do when we get malware :). I figured that would get it.
It sounds like you still have a sshd service installed that is
referencing a nonexistent path. Type cygrunsrv -Q sshd to see if
there is such a service, and if so cygrunsrv -R ssh and then rerun
ssh-host-config.
i did, and it was, and that worked. Some values in the registry could not
be deleted i didnt really pay attention to which, i guess one was the
running ssh server.
I have to admit, the documentation could be a little more clear about
the fact that you need to remove services. I've also often read that
people encounter problems when trying to delete the Cygwin tree because
they encounter files with permissions that don't allow the file to be
deleted (e.g. files created by SYSTEM.)
I therefore propose the following rewrite of the How do I uninstall all
of Cygwin entry in the FAQ. This version is much more wordy, I admit.
But since it seems to come up every so often I feel a little
hand-holding in the FAQ can't hurt. Rather than saying basically to
delete the folder and the registry key and you're on your own for the
other stuff, this gives a list of steps that should cover everything.
2005-05-17 Brian Dessent [EMAIL PROTECTED]
* install.texinfo (How do I uninstall...): Rewrite to cover
removing services, dealing with permissions, and other common
tasks for removing Cygwin completely.Index: install.texinfo
===
RCS file: /cvs/src/src/winsup/doc/install.texinfo,v
retrieving revision 1.52
diff -u -r1.52 install.texinfo
--- install.texinfo 29 Jan 2005 22:35:17 - 1.52
+++ install.texinfo 18 May 2005 03:15:57 -
@@ -252,29 +252,60 @@
@subsection How do I uninstall @strong{all} of Cygwin?
-Setup has no automatic uninstall facility. Just delete everything
-manually:
+Setup has no automatic uninstall facility. The recommended method to remove
all
+of Cygwin is as follows:
[EMAIL PROTECTED] @bullet
[EMAIL PROTECTED] Cygwin shortcuts on the Desktop and Start Menu
-
[EMAIL PROTECTED] The registry tree @samp{Software\Cygnus Solutions} under
[EMAIL PROTECTED] and/or @code{HKEY_CURRENT_USER}.
-
[EMAIL PROTECTED] Anything under the Cygwin root folder, @samp{C:\cygwin} by
-default.
[EMAIL PROTECTED]
[EMAIL PROTECTED] Anything created by setup in its temporary working directory.
[EMAIL PROTECTED] Remove all Cygwin services. If a service is currently
running, it must
+first be stopped with @samp{cygrunsrv -E name}, where @samp{name}
+is the name of the service. Then use @samp{cygrunsrv -R name} to uninstall
the
+service from the registry. Repeat this for all services that you installed.
+Common services that might have been installed are @code{sshd}, @code{cron},
[EMAIL PROTECTED], @code{inetd}, @code{apache}, and so on.
+
[EMAIL PROTECTED] Remove all mount information with @samp{umount -A}. If you
want to
+save your mount points for a later reinstall, first save the output of
[EMAIL PROTECTED] -m} as described at
[EMAIL PROTECTED]://cygwin.com/cygwin-ug-net/using-utils.html#mount}.
[EMAIL PROTECTED] itemize
[EMAIL PROTECTED] Close all Cygwin command prompts, xterms, etc. and stop the
X11 server if
+it is running.
-It's up to you to deal with other changes you made to your system, such
-as installing the inetd service, altering system paths, etc. Setup
-would not have done any of these things for you.
[EMAIL PROTECTED] Delete the Cygwin root folder and all subfolders. If you get
an error
+that an object is in use, then ensure that you've stopped all services and
+closed all Cygwin programs. If you get a 'Permission Denied' error then you
+will need to modify the permissions and/or ownership of the files or folders
+that are causing the error. For example, sometimes files used by system
+services end up owned by the SYSTEM account and not writable by regular users.
+
+The quickest way to delete the entire tree if you run into this problem is to
+change the ownership of all files and folders to your account. To do this in
+Windows Explorer, right click on the root Cygwin folder, choose Properties,
then
+the Security tab. Select Advanced, then go to the Owner tab and make sure
your
+account is listed as the owner. Select the 'Replace owner on subcontainers
and
+objects' checkbox and press Ok. After Explorer applies the changes you should
+be able to delete the entire tree in one operation. Note that you can also
+achieve this in Cygwin by typing @samp{chown -R user /} or by using other
tools
+such as CACLS.EXE.
+
[EMAIL PROTECTED
Re: [patch] update documentation Was: cygwin-host-setup does not install sshd
Corinna Vinschen wrote:
2005-05-17 Brian Dessent [EMAIL PROTECTED]
http://cygwin.com/acronyms#PCYMTNQREAIYR ;-)
Yeah, I know. Spammers have had my address for some time, I don't feel
like hiding. Me heart SpamAssassin. :)
Close all Cygwin command prompts, xterms, etc. and stop the X11 server [...]
one item up and then to begin the next item with
Open a single Cygwin command promt, remove all mount information with
@samp{umount -A} [...]
Ah, right. I guess I was trying to avoid saying close down everything
followed by open a command prompt and... I combined the two steps
into one, hopefully less confusing.
2005-05-18 Brian Dessent [EMAIL PROTECTED]
* install.texinfo (How do I uninstall...): Rewrite to cover
removing services, dealing with permissions, and other common
tasks for removing Cygwin completely.Index: install.texinfo
===
RCS file: /cvs/src/src/winsup/doc/install.texinfo,v
retrieving revision 1.52
diff -u -r1.52 install.texinfo
--- install.texinfo 29 Jan 2005 22:35:17 - 1.52
+++ install.texinfo 18 May 2005 10:33:20 -
@@ -252,29 +252,59 @@
@subsection How do I uninstall @strong{all} of Cygwin?
-Setup has no automatic uninstall facility. Just delete everything
-manually:
+Setup has no automatic uninstall facility. The recommended method to remove
all
+of Cygwin is as follows:
[EMAIL PROTECTED] @bullet
[EMAIL PROTECTED] Cygwin shortcuts on the Desktop and Start Menu
-
[EMAIL PROTECTED] The registry tree @samp{Software\Cygnus Solutions} under
[EMAIL PROTECTED] and/or @code{HKEY_CURRENT_USER}.
-
[EMAIL PROTECTED] Anything under the Cygwin root folder, @samp{C:\cygwin} by
-default.
-
[EMAIL PROTECTED] Anything created by setup in its temporary working directory.
[EMAIL PROTECTED]
[EMAIL PROTECTED] itemize
[EMAIL PROTECTED] Remove all Cygwin services. If a service is currently
running, it must
+first be stopped with @samp{cygrunsrv -E name}, where @samp{name}
+is the name of the service. Then use @samp{cygrunsrv -R name} to uninstall
the
+service from the registry. Repeat this for all services that you installed.
+Common services that might have been installed are @code{sshd}, @code{cron},
[EMAIL PROTECTED], @code{inetd}, @code{apache}, and so on.
+
[EMAIL PROTECTED] Stop the X11 server if it is running, and terminate any
Cygwin programs
+that might be running in the background. Remove all mount information by
typing
[EMAIL PROTECTED] -A} and then exit the command prompt and ensure that no
Cygwin
+processes remain. Note: If you want to save your mount points for a later
+reinstall, first save the output of @samp{mount -m} as described at
[EMAIL PROTECTED]://cygwin.com/cygwin-ug-net/using-utils.html#mount}.
-It's up to you to deal with other changes you made to your system, such
-as installing the inetd service, altering system paths, etc. Setup
-would not have done any of these things for you.
[EMAIL PROTECTED] Delete the Cygwin root folder and all subfolders. If you get
an error
+that an object is in use, then ensure that you've stopped all services and
+closed all Cygwin programs. If you get a 'Permission Denied' error then you
+will need to modify the permissions and/or ownership of the files or folders
+that are causing the error. For example, sometimes files used by system
+services end up owned by the SYSTEM account and not writable by regular users.
+
+The quickest way to delete the entire tree if you run into this problem is to
+change the ownership of all files and folders to your account. To do this in
+Windows Explorer, right click on the root Cygwin folder, choose Properties,
then
+the Security tab. Select Advanced, then go to the Owner tab and make sure
your
+account is listed as the owner. Select the 'Replace owner on subcontainers
and
+objects' checkbox and press Ok. After Explorer applies the changes you should
+be able to delete the entire tree in one operation. Note that you can also
+achieve this in Cygwin by typing @samp{chown -R user /} or by using other
tools
+such as CACLS.EXE.
+
[EMAIL PROTECTED] Delete the Cygwin shortcuts on the Desktop and Start Menu,
and anything
+left by setup.exe in the download directory. However, if you plan to
reinstall
+Cygwin it's a good idea to keep your setup.exe download directory since you
can
+reinstall the packages left in its cache without redownloading them.
+
[EMAIL PROTECTED] If you added Cygwin to your system path, you should remove it
unless you
+plan to reinstall Cygwin to the same location. Similarly, if you set your
+CYGWIN environment variable system-wide and don't plan to reinstall, you
should
+remove it.
+
[EMAIL PROTECTED] Finally, if you want to be thorough you can delete the
registry tree
[EMAIL PROTECTED] Solutions} under @code{HKEY_LOCAL_MACHINE} and/or
[EMAIL PROTECTED] However, if you followed the directions
Re: [patch] update documentation Was: cygwin-host-setup does not install sshd
Corinna Vinschen wrote: Alright. I am not sure how to push out the new version to the web site, so someone else will have to do that (or tell me what to do - check in the .html files into the website CVS or something?) Yep. cvs -d :ext:cygwin.com:/cvs/cygwin co htdocs Got it, thanks. Brian
gcc4 and local statics
Corinna Vinschen wrote: While this might help to avoid... something, I'm seriously wondering what's wrong with this expression. Why does each new version of gcc add new incompatibilities? I think I've figured this out. PR/13684 added thread safety to initialization of local statics.[1] It does this by wrapping the call to the contructor around __cxa_guard_acquire and __cxa_guard_release, which are supposed to prevent two threads from both calling the constructor at the same time. The problem is that in Cygwin, these functions are defined as no-ops in cxx.cc. That means that GCC calls the function and expects a nonzero return value if it was able to acquire the mutex, but in our case the function always returns zero (or rather, it does nothing and eax contained zero before the function call) and so gcc never tries to call the constructor. There seem to be several possible ways to go here: 1. Compile with -fno-threadsafe-statics. 2. Implement an actual muto in __cxa_guard_*. 3. Remove Cygwin's no-op __cxa_guard_* and rely on the libstdc++ provided ones. 4. Move the variable to file/global scope. This recently came up on an Apple list[2], apparently in the context of a vendor trying to compile their kernel driver against Tiger using gcc4. It looks like they're going with either #4 or #1. I tested #1 and it indeed cures the failing mmap testsuites. For Cygwin's purposes it seems that we need to decide if two threads could ever potentially call this function at the same time. If so, then #1 is out. Correct me if I'm wrong but Cygwin does not use anything from libstdc++ so #3 is out as well. In this particular case of 'granularity' it seems rather trivial to spend much time implementing actual locking. But then you have to determine if there are any other local statics that will be suffering from the same fate, and if so then #2 starts to become reasonable, otherwise I'd say #4. Brian [1] http://gcc.gnu.org/bugzilla/show_bug.cgi?id=13684 [2] http://lists.apple.com/archives/darwin-drivers/2005/May/msg00066.html
[patch] several new features for cygrunsrv
, but I also took the liberty of doing
some minor code cleanups here and there while implementing this.
Brian
2005-05-19 Brian Dessent [EMAIL PROTECTED]
* cygrunsrv.cc (longopts): Add '--list' and '--verbose' options.
(opts): Add '-L' and '-V' options; keep order consistent with above.
(action_t): Add 'List'.
(err_out_set_error): Define version of 'err_out' macro that allows for
convenient setting the error code.
(get_description): New function.
(install_service): Use 'err_out_set_error' instead throughout.
(start_service): Ditto.
(stop_service): Ditto.
(ServiceType_desc): Add. Use structs to map DWORD fields onto strings.
(StartType_desc): Ditto.
(CurrentState_desc): Ditto.
(ErrorControl_desc): Ditto.
(ControlsAccepted_desc): Ditto.
(make_desc): Add new function that generalizes the task of creating
a textual field from a binary DWORD.
(serviceTypeToString): Remove.
(serviceStateToString): Ditto.
(controlsToString): Ditto.
(parsedoublenull): Add new helper function for parsing lists of
strings, which is used below when printing the 'lpDependencies' value.
(print_service): Add new function that is responsible for generating
the formatted output for --list and --query commands.
(QSC_BUF_SIZE): Add.
(query_service): Add verbosity parameter. Remove printf output from
here, call 'print_service' instead. Call QueryServiceConfig to
retrieve more detail on the service.
(list_services): Add new function that implements -L,--list command.
Call EnumServicesStatus to get names of all services, and then
determine which ones are cygrunsrv services. List their names, or
call print_service() if verbosity was requested.
(main): Declare new variable 'verbosity'. Support new command line
switches. Pass on verbosity information to query_service and
list_services.
* utils.cc (reason_list): Update error text.
(usage): Document new switches in the help text.
* utils.h (reason_t): Add new symbolic name for error text.--- /tmp/cygrunsrv-1.02-1/cygrunsrv.cc 2005-05-16 07:55:41.0 -0700
+++ ./cygrunsrv.cc 2005-05-19 11:27:42.609375000 -0700
@@ -51,6 +51,7 @@ struct option longopts[] = {
{ start, required_argument, NULL, 'S' },
{ stop, required_argument, NULL, 'E' },
{ query, required_argument, NULL, 'Q' },
+ { list, no_argument, NULL, 'L' },
{ path, required_argument, NULL, 'p' },
{ args, required_argument, NULL, 'a' },
{ chdir, required_argument, NULL, 'c' },
@@ -69,6 +70,7 @@ struct option longopts[] = {
{ shutdown, no_argument, NULL, 'o' },
{ interactive, no_argument, NULL, 'i' },
{ nohide, no_argument, NULL, 'j' },
+ { verbose, no_argument, NULL, 'V' },
{ help, no_argument, NULL, 'h' },
{ version, no_argument, NULL, 'v' },
{ 0, no_argument, NULL, 0 }
@@ -77,8 +79,9 @@ struct option longopts[] = {
char *opts = I:
R:
S:
- Q:
E:
+ Q:
+ L
p:
a:
c:
@@ -97,6 +100,7 @@ char *opts = I:
n
i
j
+ V
h
v;
@@ -118,7 +122,8 @@ enum action_t {
Remove,
Start,
Stop,
- Query
+ Query,
+ List
};
enum type_t {
@@ -156,6 +161,8 @@ eval_wait_time (register DWORD wait)
}
#define err_out(name) {err_func = #name; err = GetLastError (); goto out;}
+#define err_out_set_error(name, error) \
+{err_func = #name; err = error; SetLastError (error); goto out;}
/* Installs the subkeys of the service registry entry so that cygrunsrv
can determine what application to start on service startup. */
@@ -463,6 +470,36 @@ out:
return ret;
}
+/* Retrieves the description of the service. Note: it would be so much
+ cleaner to do this by a simple call to QueryServiceConfig2(), but alas this
+ does not exist in NT4. *sigh* */
+int
+get_description (const char *name, char *descr)
+{
+ HKEY desc_key = NULL;
+ char desc_key_path[MAX_PATH];
+ int ret;
+
+ strcat (strcpy (desc_key_path, SRV_KEY), name);
+ if ((ret = RegOpenKeyEx (HKEY_LOCAL_MACHINE, desc_key_path, 0,
+ KEY_READ, desc_key)) != ERROR_SUCCESS)
+goto out;
+
+ if ((ret = get_opt_string_entry (desc_key, DESC, descr)))
+goto out;
+
+ ret = 0;
+
+out:
+ if (ret)
+descr = NULL;
+ if (desc_key)
+RegCloseKey (desc_key);
+ return ret;
+}
+
+
+
int
add_env_var (char *envstr, env_t *env)
{
@@ -575,10 +612,7 @@ install_service (const char *name, const
GetLastError() != ERROR_SERVICE_DOES_NOT_EXIST)
err_out (OpenService);
if (sh)
-{
- SetLastError (ERROR_SERVICE_EXISTS);
- err_out (OpenService);
-}
+err_out_set_error (OpenService, ERROR_SERVICE_EXISTS);
/* Set optional dependencies. */
if (deps)
{
@@ -587,10 +621,7 @@ install_service (const char *name, const
concat_length
Re: [patch] several new features for cygrunsrv
Brian Dessent wrote: -controlsToString(DWORD controls) + char *base, *end; + static char buf[34]; + int used = 0, dsiz = strlen (delim); Crap, that is a mistake. buf[34] should be something more generous like 128 or 256. I had it set small to test to make sure it couldn't overflow and forgot to put it back. Brian
[patch] dump service info in cygcheck
(Okay, this time I hope this is the correct mailing list since this lives in
winsup/utils.)
Here is a first stab at the aforementioned patch to dump service information for
cygcheck -s. If you do not provide -v then you get the condensed output
(i.e. cygrunsrv -Q service for each service), otherwise you get the full
output of cygrunsrv -LV. This has logic to deal with the following conditions:
- Not running NT
- cygrunsrv package not installed or can't execute
- cygrunsrv is older than v1.10
- no services installed
It also acts in the same way as the other cygcheck -s checks, in that if -h is
given it is slightly more verbose.
I used existing code that calls '/bin/id.exe' as a model, so that it looks up
the location in the mounts rather than searching the path.
I also changed 'Cygnus' to 'Cygwin' in an unrelated printf that I happened to
notice. Not sure if that's desired or not but I would imagine most direct
references to Cygnus are probably anachronisms.
Brian
2005-05-22 Brian Dessent [EMAIL PROTECTED]
* cygcheck.cc (dump_sysinfo_services): Add new function that uses
new cygrunsrv options to dump service info.
(dump_sysinfo): Call dump_sysinfo_services if running under NT.
Change 'Cygnus' to 'Cygwin' in output.Index: cygcheck.cc
===
RCS file: /cvs/src/src/winsup/utils/cygcheck.cc,v
retrieving revision 1.71
diff -u -p -r1.71 cygcheck.cc
--- cygcheck.cc 20 May 2005 16:50:39 - 1.71
+++ cygcheck.cc 22 May 2005 19:08:48 -
@@ -870,6 +870,94 @@ pretty_id (const char *s, char *cygwin,
}
}
+/* This dumps information about each installed cygwin service, if cygrunsrv
+ is available. */
+void
+dump_sysinfo_services ()
+{
+ char buf[1024];
+ char buf2[1024];
+ FILE *f;
+
+ if (givehelp)
+printf (\nChecking for any Cygwin services... %s\n\n,
+ verbose ? : (use -v for more detail));
+ else
+fputc ('\n', stdout);
+
+ /* find the location of cygrunsrv.exe */
+ char *cygrunsrv = cygpath (/bin/cygrunsrv.exe, NULL);
+ for (char *p = cygrunsrv; (p = strchr (p, '/')); p++)
+*p = '\\';
+
+ if (access (cygrunsrv, X_OK))
+{
+ puts (Can't find the cygrunsrv utility, skipping services check.\n);
+ return;
+}
+
+ /* check for a recent cygrunsrv */
+ snprintf (buf, sizeof (buf), %s --version, cygrunsrv);
+ if ((f = popen (buf, rt)) == NULL)
+{
+ printf (Failed to execute '%s', skipping services check.\n, buf);
+ return;
+}
+ int maj, min;
+ int ret = fscanf (f, cygrunsrv V%u.%u, maj, min);
+ if (ferror (f) || feof (f) || ret == EOF || maj 1 || min 10)
+{
+ puts (The version of cygrunsrv installed is too old to dump service
info.\n);
+ return;
+}
+ fclose (f);
+
+ /* run cygrunsrv --list */
+ snprintf (buf, sizeof (buf), %s --list, cygrunsrv);
+ if ((f = popen (buf, rt)) == NULL)
+{
+ printf (Failed to execute '%s', skipping services check.\n, buf);
+ return;
+}
+ size_t nchars = fread ((void *) buf, 1, sizeof (buf), f);
+ pclose (f);
+
+ /* were any services found? */
+ if (nchars 1)
+{
+ puts (No Cygwin services found.\n);
+ return;
+}
+
+
+ /* In verbose mode, just run 'cygrunsrv --list --verbose' and copy the
+ entire output. Otherwise run 'cygrunsrv --query' for each service. */
+ for (char *srv = strtok (buf, \n); srv; srv = strtok (NULL, \n))
+{
+ if (verbose)
+snprintf (buf2, sizeof (buf2), %s --list --verbose, cygrunsrv);
+ else
+snprintf (buf2, sizeof (buf2), %s --query %s, cygrunsrv, srv);
+ if ((f = popen (buf2, rt)) == NULL)
+{
+ printf (Failed to execute '%s', skipping services check.\n, buf2);
+ return;
+}
+
+ /* copy output to stdout */
+ do
+{
+ nchars = fread ((void *)buf2, 1, sizeof (buf2), f);
+ fwrite ((void *)buf2, 1, nchars, stdout);
+}
+ while (!feof (f) !ferror (f));
+ pclose (f);
+
+ if (verbose)
+break;
+}
+}
+
static void
dump_sysinfo ()
{
@@ -877,6 +965,7 @@ dump_sysinfo ()
char tmp[4000];
time_t now;
char *found_cygwin_dll;
+ bool is_nt = false;
printf (\nCygwin Configuration Diagnostics\n);
time (now);
@@ -916,6 +1005,7 @@ dump_sysinfo ()
}
break;
case VER_PLATFORM_WIN32_NT:
+ is_nt = true;
if (osversion.dwMajorVersion == 5)
{
BOOL more_info = FALSE;
@@ -1248,7 +1338,7 @@ dump_sysinfo ()
printf (\n);
if (givehelp)
-printf (Looking for various Cygnus DLLs... (-v gives version info)\n);
+printf (Looking for various Cygwin DLLs... (-v gives version info)\n);
int cygwin_dll_count = 0;
for (i = 1; i num_paths; i++)
{
@@ -1288,6 +1378,9 @@ dump_sysinfo ()
puts (Warning: There are multiple cygwin1.dlls on your path);
if (!cygwin_dll_count
[patch] add -p option to cygcheck to query website package search
Here is a patch that implements the -p option to cygcheck that was mentioned on the list previously. It uses the WinInet API to hit the package-grep.cgi URL on cygwin.com with the search regexp supplied by the user. Rather than trying to parse the html output or requiring cygcheck to depend on awk or something, I instead modified the cgi script to recognise an additional parameter named 'plain'. If present in the request, the script replies with a text/plain version of the results that cygcheck just copies to stdout. Included in the patch is an update of the utils.sgml documentation for the new parameter. As I understand it the man pages are created from this part of the user's guide, so that should kill two birds with one stone. I also document the new switch in the --help output. I took the liberty of rewording it considerably, because the way that it described the options seemed rather unintuitive -- there are certain combinations of allowed and unallowed parameters, and rather than trying to explain for each switch which others it is incompatible with, instead it gives a list of the acceptible forms of calling the program. Here is the output after this patch: $ cygcheck -h Usage: cygcheck PROGRAM [ -v ] [ -h ] cygcheck -c [ PACKAGE ] [ -d ] cygcheck -s [ -r ] [ -v ] [ -h ] cygcheck -k cygcheck -f FILE [ FILE ... ] cygcheck -l [ PACKAGE ] [ PACKAGE ... ] cygcheck -p REGEXP List system information, check installed packages, or query package database. At least one command option or a PROGRAM is required, as shown above. PROGRAM list library (DLL) dependencies of PROGRAM -c, --check-setupshow installed version of PACKAGE and verify integrity (or for all installed packages if none specified) -d, --dump-only just list packages, do not verify (with -c) -s, --sysinfoproduce diagnostic system information (implies -c -d) -r, --registry also scan registry for Cygwin settings (with -s) -k, --keycheck perform a keyboard check session (must be run from a plain console only, not from a pty/rxvt/xterm) -f, --find-package find the package that FILE belongs to -l, --list-package list contents of PACKAGE (or all packages if none given) -p, --package-query search for REGEXP in the entire cygwin.com package repository (requies internet connectivity) -v, --verboseproduce more verbose output -h, --help annotate output with explanatory comments when given with another command, otherwise print this help -V, --versionprint the version of cygcheck and exit Note: -c, -f, and -l only report on packages that are currently installed. To search all official Cygwin packages use -p instead. The -p REGEXP matches package names, descriptions, and names of files/paths within all packages. The new --package-query command works more or less as you would expect. Whatever you supply after -p is passed along to the CGI as if you'd entered it in the web form. The only thing I changed was I omitted the directory name that the package is in, so save a little bit of screen width. Here is a sample: $ cygcheck -p 'cygintl-2\.dll' Found 1 matches for 'cygintl-2\.dll'. libintl2-0.12.1-3 GNU Internationalization runtime library $ cygcheck -p 'libexpat.*\.a' Found 2 matches for 'libexpat.*\.a'. expat-1.95.7-1XML parser library written in C expat-1.95.8-1XML parser library written in C $ cygcheck -p '/ls\.exe' Found 2 matches for '/ls\.exe'. coreutils-5.2.1-5 GNU core utilities (includes fileutils, sh-utils and textutils) coreutils-5.3.0-6 GNU core utilities (includes fileutils, sh-utils and textutils) There is an additional unrelated bugfix that I included with this patch. The bug was introduced with my patch to cygcheck that calls cygrunsrv. It did not properly null-terminate the buffer that was read from popen(), which would result in the strtok() loop erroniously calling cygrunsrv --query garbage after the last service. This resulted in an occasional spurious cygrunsrv error to stdout if you ran cygcheck -s without -v. If the package-grep part of the patch is not yet ready for primetime, I will resubmit just this fix by itself since it's a pretty dumb bug. I have tested the WinInet stuff on WinXP and Win98 and it seemed ok on both. MSDN claims that this API exists as far back as Win95 and only requires IE 3.0. I have tested the package-grep.cgi script locally. In terms of error reporting cygcheck will emit an error if the HTTP response code was not 200. It will also emit an error (and call FormatMessage to get a textual version) if there is a problem connecting or resolving the domain. Brian winsup/utils: 2005-06-20 Brian Dessent [EMAIL PROTECTED] * Makefile.in: Link cygcheck with libwininet.a. * cygcheck.cc
Re: [Patch]: Changes to how-programming.texinfo
Christopher Faylor wrote: Btw, the other license provision in the cygwin licensing web page was really meant as a way to accommodate other, already existing projects. And it was very gracious of them to do that. For an example of why this makes life a lot easier, consider MySQL (GPL) and OpenSSL (BSD). Now, the MySQL license has an OpenSSL exemption which means it's fine to link MySQL binaries against OpenSSL without forcing OpenSSL to the GPL. But, most GPL projects use the standard GPL with no execeptions. This means that if your distro packages ssl-enabled MySQL packages, including libmysqlclient, then using -lmysqlclient with your pure-GPL program violates a license because it pulls in the BSD OpenSSL code. http://bugs.debian.org/cgi-bin/bugreport.cgi?bug=283786 http://bugs.mysql.com/bug.php?id=6924 MySQL at some point figured out what kind of hell a widely used library that is only licensed under pure GPL could cause, and added their FLOSS exception which lists a number of acceptable licenses that can be used as an exception, much like Cygwin. http://www.mysql.com/company/legal/licensing/foss-exception.html But I think that was a relatively new thing, and until recently most distros were stuck with the prehistoric 3.23 version of mysql due to its libmysqlclient being the last LGPL version available. I presume this was done so that e.g. BSD-licensed programs can still use -lmysqlclient. This really hurt MySQL adoption though because if the vast majority of the world is still using 3.x then you really can't write software that depends on the great features in 4.0 and 4.1 or even 5.0. Last I checked RHEL and FC were *still* packaging this ancient version as their default, though that might have finally changed in RHEL4 and FC4, I don't know. Brian (sorry for the semi-off-topic rant.)
Re: [PATCH] Fix cygrunsrv invocation in cygcheck
Brian Dessent wrote: why not simply run cygrunsrv --list --verbose in verbose mode, instead of actually going through one iteration of the loop? Simply to reuse the copy output code? Brian? The reason I did it that way is because if I had run cygrunsrv --list Now that I re-read what you said I think I misunderstood. You're right, it could have simply done if(verbose) cygrunsrv --list --verbose else foreach cygrunsrv --list cygrunsrv --query And that would be somewhat more efficient. But you're right, I did it that way so that I wouldn't have to duplicate code between the two cases... laziness. Brian
Re: [PATCH] Fix cygrunsrv invocation in cygcheck
Christopher Faylor wrote: Go ahead and check this in. Thanks. Ok. Thanks, Igor, for bringing this up (again). Thanks Igor, I had meant to bring this up but forgot. There's no need to ping anybody. I do read this list and I haven't forgotten about the patch. If it didn't require changs to a file on sourceware.org, I'd have checked it in by now. I can rework the patch if there are parts of it that are objectionable. I figured that parsing html in C without external libraries was kind of silly when we have control of the cgi script on the other end. Brian
[patch] load wininet dynamically in cygcheck
This uses LoadLibrary and GetProcAddress instead of -lwininet so that systems
lacking IE3 can still run cygcheck. Tested on XP and NT4, and verified that
with WININET.DLL renamed cygcheck can still function.
2006-01-13 Brian Dessent [EMAIL PROTECTED]
* Makefile.in (cygcheck.exe): Do not link against libwininet.a.
* cygcheck.cc (pInternetCloseHandle): Define global function pointer.
(display_internet_error): Use it.
(package_grep): Attempt to load wininet.dll at runtime. Call WinInet
API through function pointers throughout.
BrianIndex: Makefile.in
===
RCS file: /cvs/src/src/winsup/utils/Makefile.in,v
retrieving revision 1.58
diff -u -p -r1.58 Makefile.in
--- Makefile.in 22 Nov 2005 17:19:17 - 1.58
+++ Makefile.in 13 Jan 2006 18:39:16 -
@@ -99,15 +99,15 @@ else
$(CXX) $(MINGW_CXXFLAGS) -o $@ ${wordlist 1,2,$^} -B$(mingw_build)/
$(MINGW_LDFLAGS)
endif
-cygcheck.exe: cygcheck.o path.o dump_setup.o $(MINGW_DEP_LDLIBS)
$(w32api_lib)/libwininet.a
+cygcheck.exe: cygcheck.o path.o dump_setup.o $(MINGW_DEP_LDLIBS)
ifeq $(libz)
@echo '*** Building cygcheck without package content checking due to
missing mingw libz.a.'
endif
ifdef VERBOSE
- $(CXX) $(MINGW_CXXFLAGS) -o $@ ${wordlist 1,3,$^} -B$(mingw_build)/
$(MINGW_LDFLAGS) $(libz) $(w32api_lib)/libwininet.a
+ $(CXX) $(MINGW_CXXFLAGS) -o $@ ${wordlist 1,3,$^} -B$(mingw_build)/
$(MINGW_LDFLAGS) $(libz)
else
- @echo $(CXX) -o $@ ${wordlist 1,3,$^} ${filter-out -B%,
$(MINGW_CXXFLAGS) $(MINGW_LDFLAGS)} $(libz) $(w32api_lib)/libwininet.a;\
- $(CXX) $(MINGW_CXXFLAGS) -o $@ ${wordlist 1,3,$^} -B$(mingw_build)/
$(MINGW_LDFLAGS) $(libz) $(w32api_lib)/libwininet.a
+ @echo $(CXX) -o $@ ${wordlist 1,3,$^} ${filter-out -B%,
$(MINGW_CXXFLAGS) $(MINGW_LDFLAGS)} $(libz);\
+ $(CXX) $(MINGW_CXXFLAGS) -o $@ ${wordlist 1,3,$^} -B$(mingw_build)/
$(MINGW_LDFLAGS) $(libz)
endif
dumper.o: dumper.cc dumper.h
Index: cygcheck.cc
===
RCS file: /cvs/src/src/winsup/utils/cygcheck.cc,v
retrieving revision 1.83
diff -u -p -r1.83 cygcheck.cc
--- cygcheck.cc 13 Jan 2006 13:39:05 - 1.83
+++ cygcheck.cc 13 Jan 2006 18:39:16 -
@@ -37,6 +37,10 @@ int find_package = 0;
int list_package = 0;
int grep_packages = 0;
+/* This is global because it's used in both internet_display_error as well
+ as package_grep. */
+BOOL (WINAPI *pInternetCloseHandle) (HINTERNET);
+
#ifdef __GNUC__
typedef long long longlong;
#else
@@ -161,7 +165,7 @@ display_internet_error (const char *mess
va_start (hptr, message);
while ((h = va_arg (hptr, HINTERNET)) != 0)
-InternetCloseHandle (h);
+pInternetCloseHandle (h);
va_end (hptr);
return 1;
@@ -1588,6 +1592,43 @@ package_grep (char *search)
{
char buf[1024];
+ /* Attempt to dynamically load the necessary WinInet API functions so that
+ cygcheck can still function on older systems without IE. */
+ HMODULE hWinInet;
+ if (!(hWinInet = LoadLibrary (wininet.dll)))
+{
+ fputs (Unable to locate WININET.DLL. This feature requires Microsoft
+ Internet Explorer v3 or later to function.\n, stderr);
+ return 1;
+}
+
+ /* InternetCloseHandle is used outside this function so it is declared
+ global. The rest of these functions are only used here, so declare them
+ and call GetProcAddress for each of them with the following macro. */
+
+ pInternetCloseHandle = (BOOL (WINAPI *) (HINTERNET))
+GetProcAddress (hWinInet, InternetCloseHandle);
+#define make_func_pointer(name, ret, args) ret (WINAPI * p##name) args = \
+(ret (WINAPI *) args) GetProcAddress (hWinInet, #name);
+ make_func_pointer (InternetAttemptConnect, DWORD, (DWORD));
+ make_func_pointer (InternetOpenA, HINTERNET, (LPCSTR, DWORD, LPCSTR, LPCSTR,
+DWORD));
+ make_func_pointer (InternetOpenUrlA, HINTERNET, (HINTERNET, LPCSTR, LPCSTR,
+ DWORD, DWORD, DWORD));
+ make_func_pointer (InternetReadFile, BOOL, (HINTERNET, PVOID, DWORD,
PDWORD));
+ make_func_pointer (HttpQueryInfoA, BOOL, (HINTERNET, DWORD, PVOID, PDWORD,
+PDWORD));
+#undef make_func_pointer
+
+ if(!pInternetCloseHandle || !pInternetAttemptConnect || !pInternetOpenA
+ || !pInternetOpenUrlA || !pInternetReadFile || !pHttpQueryInfoA)
+{
+ fputs (Unable to load one or more functions from WININET.DLL. This
+ feature requires Microsoft Internet Explorer v3 or later to
+ function.\n, stderr);
+ return 1;
+}
+
/* construct the actual URL by escaping */
char *url = (char *) alloca (sizeof (base_url) + strlen (search) * 3);
strcpy (url, base_url);
@@ -1610,7 +1651,7 @@ package_grep
Re: [Patch] regtool: Add load/unload commands and --binary option
Christian Franke wrote: At least when regtool is used interactively, it is IMO not very useful to have modem-line-noise-like output for REG_BINARY, but human readable output for the other value types. But this is the current behavior of regtool get Instead of an explicit -b flag, perhaps it should just call isatty() and if being run interactively, output human readable hex dump, otherwise output raw binary. Brian
Re: [Patch] regtool: Add load/unload commands and --binary option
Igor Peshansky wrote: What if you want to redirect the hex dump to a file? IMO, isatty() checks are only useful if the output won't change qualitatively on redirection (e.g., for coloring). Otherwise, it's always better to use an explicit flag. Good point. Why don't we just emulate the behavior of 'cat' here? If isatty() is true and non-ascii characters are in the output, then prompt first before possibly fubaring the user's terminal, otherwise just output the raw data. And just as 'cat' does not have any internal code for formatting binary data as a hex dump, neither should regtool, since 'od' works perfectly for that and already has the veritable kitchen sink of formatting options. Brian
[patch] fix spurious SIGSEGV faults under Cygwin
Recently Cygwin has changed the way that it internally checks arguments for bad
pointers. Where before it used IsBad{Read,Write}Ptr() from the win32 API, now
it installs its own temporary SEH fault handler. However, gdb is not aware of
this and so it leads to the program stopping on a SIGSEGV on perfectly
legitimate code -- when not run under the debugger, Cygwin's fault handler
catches the attempt and returns the appropriate error. Here is a minimal
example:
$ cat ss.cc
#include string
int main()
{
std::string foo;
return 0;
}
$ g++ -g ss.cc -o ss
$ gdb --quiet ./ss
(gdb) r
Starting program: /tmp/false_sigsegv/ss.exe
Program received signal SIGSEGV, Segmentation fault.
0x610af8b8 in pthread_key_create (key=0x482c08, destructor=0) at
/usr/src/sourceware/winsup/cygwin/thread.cc:129
129 if ((*object)-magic != magic)
(gdb) c
Continuing.
Program exited normally.
(gdb)
Here pthread_key_create() was simply checking to see if 'key' happened to be a
previously-initiaized object, which in most cases is not true, and so the fault
is expected and handled. However, it's very inconvenient and unintuitive that
gdb still stops the program, and it makes the user think there is something
wrong with his program and/or the Cygwin library.
The attached patch adds a very simple mechanism by which the Cygwin program
shall signal to gdb that it has temporarily installed its own fault handler for
EXCEPTION_ACCESS_VIOLATION, so that gdb can deal with this more gracefully by
ignoring these faults. This uses the existing WriteDebugString() mechanism that
Cygwin already uses to communicate to the debugger, but just extends it by
defining two new magic string values.
I've split the patch into two parts since it requires changes to both cygwin and
gdb.
Brian
winsup/cygwin:
2006-02-02 Brian Dessent [EMAIL PROTECTED]
* cygtls.h: Include sys/cygwin.h.
(myfault::~myfault): If a debugger is present, inform it that our fault
handler has been removed.
(myfault::faulted): Likewise for when the fault handler is installed.
* include/sys/cygwin.h (_CYGWIN_FAULT_IGNORE_STRING): Add define.
(_CYGWIN_FAULT_NOIGNORE_STRING): Ditto.
gdb:
2006-02-02 Brian Dessent [EMAIL PROTECTED]
* win32-nat.c (_CYGWIN_SIGNAL_STRING): Remove duplicated definition.
(ignoring_faults): Define new static global.
(handle_output_debug_string): Check for fault ignore/noignore signals
from the inferior.
(handle_exception): Do not process the fault if 'ignoring_faults' is
set.
(do_initial_win32_stuff): Initialize 'ignoring_faults'.Index: cygtls.h
===
RCS file: /cvs/src/src/winsup/cygwin/cygtls.h,v
retrieving revision 1.42
diff -u -p -r1.42 cygtls.h
--- cygtls.h23 Dec 2005 22:50:20 - 1.42
+++ cygtls.h2 Feb 2006 11:43:23 -
@@ -1,6 +1,6 @@
/* cygtls.h
- Copyright 2003, 2004, 2005 Red Hat, Inc.
+ Copyright 2003, 2004, 2005, 2006 Red Hat, Inc.
This software is a copyrighted work licensed under the terms of the
Cygwin license. Please consult the file CYGWIN_LICENSE for
@@ -22,6 +22,7 @@ details. */
#include netinet/in.h
typedef unsigned int SOCKET;
#endif
+#include sys/cygwin.h
#define CYGTLS_INITIALIZED 0x43227
#define CYGTLSMAGIC D0Ub313v31nmG1c?;
@@ -242,9 +243,16 @@ class myfault
jmp_buf buf;
san sebastian;
public:
- ~myfault () __attribute__ ((always_inline)) { _my_tls.reset_fault
(sebastian); }
+ ~myfault () __attribute__ ((always_inline))
+ {
+_my_tls.reset_fault (sebastian);
+if (being_debugged ())
+ OutputDebugString (_CYGWIN_FAULT_NOIGNORE_STRING);
+ }
inline int faulted (int myerrno = 0) __attribute__ ((always_inline))
{
+if (being_debugged ())
+ OutputDebugString (_CYGWIN_FAULT_IGNORE_STRING);
return _my_tls.setup_fault (buf, sebastian, myerrno);
}
};
Index: include/sys/cygwin.h
===
RCS file: /cvs/src/src/winsup/cygwin/include/sys/cygwin.h,v
retrieving revision 1.61
diff -u -p -r1.61 cygwin.h
--- include/sys/cygwin.h1 Nov 2005 05:55:30 - 1.61
+++ include/sys/cygwin.h2 Feb 2006 11:43:23 -
@@ -1,6 +1,6 @@
/* sys/cygwin.h
- Copyright 1997, 1998, 2000, 2001, 2002 Red Hat, Inc.
+ Copyright 1997, 1998, 2000, 2001, 2002, 2006 Red Hat, Inc.
This file is part of Cygwin.
@@ -18,6 +18,8 @@ extern C {
#endif
#define _CYGWIN_SIGNAL_STRING cYgSiGw00f
+#define _CYGWIN_FAULT_IGNORE_STRING cYgfAuLtIg
+#define _CYGWIN_FAULT_NOIGNORE_STRING cYgNofAuLtIg
extern pid_t cygwin32_winpid_to_pid (int);
extern void cygwin32_win32_to_posix_path_list (const char *, char *);
Index: win32-nat.c
===
RCS file: /cvs/src/src/gdb/win32-nat.c,v
retrieving revision 1.119
diff -u -p -r1.119 win32-nat.c
--- win32-nat.c 24 Jan 2006 22:09:28
Re: [patch] fix spurious SIGSEGV faults under Cygwin
Brian Dessent wrote: #define _CYGWIN_SIGNAL_STRING cYgSiGw00f +#define _CYGWIN_FAULT_IGNORE_STRING cYgfAuLtIg +#define _CYGWIN_FAULT_NOIGNORE_STRING cYgNofAuLtIg Sigh, this breaks strace under Cygwin, I should have tested more. Sorry about that. Apparently strace expects anything starting with the 'cYg' prefix to be followed by a hex number. I thought that since _CYGWIN_SIGNAL_STRING already existed and didn't follow that format it was safe to add more, but that's not the case. So, should I pick another prefix that's not 'cYg'? Or instead use something like cYg0 ... since strace seems to just ignore the string if its value is 0? Or something else? Brian
Re: [patch] fix spurious SIGSEGV faults under Cygwin
Christopher Faylor wrote: Thanks for the patch but I've been working on this too and, so far, I think it is possible to have a very minimal way of dealing with this problem. I haven't had time to delve into it too deeply but I have been exploring this problem on and off for a couple of weeks. If the situation at work calms down a little I may be able to finish up what I've been working on. OTOH, if what I have is really not working then I'll take a look at what you've done. Okay, no rush. FWIW after noticing that strace was broken I tested a version that used #define _CYGWIN_SIGNAL_STRING cYgSiGw00f +#define _CYGWIN_FAULT_IGNORE_STRING cYg0 faultig +#define _CYGWIN_FAULT_NOIGNORE_STRING cYg0 nofaultig ...which seems to fix the problem since the strtold() just picks up 0 after 'cYg' and strace ignores the rest. The main problem I see with this approach is the extra call to IsDebuggerPresent() every time a 'myfault' is created/destroyed, which potentially could be a lot. I'm presuming this is a relatively cheap call so it wasn't something I worried too much about. But then I didn't actually try to measure it. If it turns out that it's expensive, I was thinking that the inferior could maintain this information in some variable, and just communicate its location to gdb once at startup, then gdb could simply read that variable in the process' memory before deciding whether to handle the fault. Or it could try to look at the SEH chain and see if a handler residing in cygwin1.dll is setup to handle the fault, and if so just silently pass it along. But that seemed really complicated when there already exists this mechanism for the process to communicate with the debugger. Brian
Re: [patch] fix spurious SIGSEGV faults under Cygwin
Dave Korn wrote: I'm having a conceptual difficulty here: Under what circumstances would you expect there *not* to be a debugger attached to the inferior to which the debugger is attached? That's a bit zen, isn't it? The code in question here runs many times in the normal course of any Cygwin program -- debugger or no. The idea behind guarding the call to OutputDebugString() with if (being_debugged()) was that the call to IsDebuggerPresent() was cheaper than the call to OutputDebugString(), and that a well-behaived, non-debug build of a binary should not needlessly send tons and tons of nonsense to OutputDebugString unless it's actually being debugged and there is something there to interpret the nonsense. Brian
Re: Patch for silent crash with Cygwin1.dll v 1.5.19-4
Gary Zablackis wrote: I did some more research over the weekend and my problem seems to only come when loading a dll via dlopen() and run_ctors() is called from dll:init() and pthread_key_create() is called during the time that run_ctors() is active. I still have not found who is calling pthread_key_create(), but will be working on this as time permits this week. If you are trying to track down why you get a SIGSEGV in pthread_key_create while running your app in gdb you are wasting your time. This is not a fault, it is expected and normal. Search the archives. Brian
Re: Patch to mapping up to 128 SCSI Disk Devices
Corinna Vinschen wrote: I must admit that I don't quite understand why that happens, but when I save your patch into a file, all '=' characters are converted into a '=3D' sequence. This is a bit weird given that you're using us-ascii encoding. Does anybody know why this happens? That's because of: Content-Transfer-Encoding: quoted-printable ..but your email client should undo the encoding if you tell it to save the message as a file. Otherwise: perl -MMIME::QuotedPrint -ne 'print decode_qp($_)' in out The patch is also broken due to unexpected line breaks, see above. That's always a pain... attachments are really the way to go. Brian
[patch] support -gdwarf-2 when creating cygwin1.dbg
The attached patch allows for dllfixdbg to copy DWARF-2 debug sections
into the .dbg file. There was also an (accidently?) duplicated section
in the cygwin.sc linker script that I removed while I was there.
The advantages of being able to build newlib/winsup with -gdwarf-2 in
C/CXXFLAGS are a ~38% smaller .dbg file, but more importantly a much
more pleasant debugger experience. gdb is not nearly as confused about
the sigfe/sigbe signal wrappers, and can unwind the stack cleanly all
the way up to mainCRTStartup() even when stepping through deep internal
cygwin1.dll guts.
Here's an example from a simple hello world exe. With -gdwarf-2:
(gdb) bt
#0 fstat64 (fd=1, buf=0x22afd0)
at /usr/src/sourceware/winsup/cygwin/syscalls.cc:1102
#1 0x610b4928 in _fstat64_r (ptr=0x50001, fd=327681, buf=0x50001)
at /usr/src/sourceware/winsup/cygwin/syscalls.cc:1115
#2 0x61107174 in __smakebuf_r (ptr=0x22d008, fp=0x64f8)
at /usr/src/sourceware/newlib/libc/stdio/makebuf.c:53
#3 0x6110667b in __swsetup_r (ptr=0x50001, fp=0x64f8)
at /usr/src/sourceware/newlib/libc/stdio/wsetup.c:67
#4 0x610f21a6 in _vfprintf_r (data=0x22d008, fp=0x64f8,
fmt0=0x402000 Hello world\n, ap=0x22cca4 /)
at /usr/src/sourceware/newlib/libc/stdio/vfprintf.c:547
#5 0x610ff208 in printf (
fmt=0x22eea8e0 Address 0x22eea8e0 out of bounds)
at /usr/src/sourceware/newlib/libc/stdio/printf.c:51
#6 0x610a5498 in _sigfe ()
at /usr/src/sourceware/winsup/cygwin/cygserver.h:82
#7 0x0009 in ?? ()
#8 0x61005efa in dll_crt0_1 ()
at /usr/src/sourceware/winsup/cygwin/dcrt0.cc:943
#9 0x61004216 in _cygtls::call2 (this=0x22ce64,
func=0x610052a0 dll_crt0_1(void*), arg=0x0, buf=0x22cdd0)
at /usr/src/sourceware/winsup/cygwin/cygtls.cc:74
#10 0x61004290 in _cygtls::call (func=0x610a5498 _sigbe, arg=0x0)
at /usr/src/sourceware/winsup/cygwin/cygtls.cc:67
#11 0x61005171 in _dll_crt0 ()
at /usr/src/sourceware/winsup/cygwin/dcrt0.cc:956
#12 0x004010e3 in cygwin_crt0 (f=0x401040 main)
at /usr/src/sourceware/winsup/cygwin/lib/cygwin_crt0.c:32
#13 0x0040103d in mainCRTStartup ()
at /usr/src/sourceware/winsup/cygwin/crt0.c:51
Exact same breakpoint, -g (stabs):
(gdb) bt
#0 fstat64 (fd=1628141592, buf=0x1)
at /usr/src/sourceware/winsup/cygwin/syscalls.cc:1102
#1 0x611b5708 in _libntdll_a_iname () from /bin/cygwin1.dll
#2 0x in ?? ()
Brian2007-04-18 Brian Dessent [EMAIL PROTECTED]
* cygwin.sc: Remove duplicated .debug_macinfo section.
* dllfixdbg: Also copy DWARF-2 sections into .dbg file.
Index: cygwin.sc
===
RCS file: /cvs/src/src/winsup/cygwin/cygwin.sc,v
retrieving revision 1.21
diff -u -p -r1.21 cygwin.sc
--- cygwin.sc 12 Jan 2007 19:40:20 - 1.21
+++ cygwin.sc 18 Apr 2007 12:16:52 -
@@ -138,6 +138,5 @@ SECTIONS
.debug_str ALIGN(__section_alignment__) (NOLOAD) : { *(.debug_str) }
.debug_loc ALIGN(__section_alignment__) (NOLOAD) : { *(.debug_loc) }
.debug_macinfo ALIGN(__section_alignment__) (NOLOAD) : { *(.debug_macinfo) }
- .debug_macinfo ALIGN(__section_alignment__) (NOLOAD) : { *(.debug_macinfo) }
.debug_ranges ALIGN(__section_alignment__) (NOLOAD) : { *(.debug_ranges) }
}
Index: dllfixdbg
===
RCS file: /cvs/src/src/winsup/cygwin/dllfixdbg,v
retrieving revision 1.4
diff -u -p -r1.4 dllfixdbg
--- dllfixdbg 14 Jul 2006 19:33:55 - 1.4
+++ dllfixdbg 18 Apr 2007 12:16:52 -
@@ -16,7 +16,10 @@ my $objdump = shift;
my @objcopy = ((shift));
my $dll = shift;
my $dbg = shift;
-xit 0, @objcopy, '-j', '.stab', '-j', '.stabstr', $dll, $dbg;
+xit 0, @objcopy, '-j', '.stab', '-j', '.stabstr', '-j', '.debug_aranges',
+ '-j', '.debug_pubnames', '-j', '.debug_info', '-j', '.debug_abbrev',
+ '-j', '.debug_line', '-j', '.debug_frame', '-j', '.debug_str', '-j',
+ '.debug_loc', '-j', '.debug_macinfo', '-j', '.debug_ranges', $dll, $dbg;
xit 0, @objcopy, '-g', '--add-gnu-debuglink=' . $dbg, $dll;
open(OBJDUMP, '-|', $objdump --headers $dll);
my %section;
Re: [patch] support -gdwarf-2 when creating cygwin1.dbg
Christopher Faylor wrote: Thanks for doing this. Please check in. Can we switch to dwarf-2 by default in the cygwin makefile(s)? I thought about that, but the problem is anything you do to *FLAGS in winsup/cygwin won't affect flags used in the other dirs like libiberty or newlib, and so unless you set CXXFLAGS and CFLAGS when you do toplevel configure, you'll end up with a mish-mash of some stabs and some DW2 in the .dbg file. Corinna Vinschen wrote: As long as we use a 3.x or 4.0.x gcc it should be ok. Later gcc's explicitely switch off the generation of a DW_CFA_offset column in the .debug_frame CIE header information, which breaks backtracing in GDB. Hmm, I think I read something about that on the gcc list. Is this just a case of gcc switching to doing TheActualRightThing and gdb not having being updated yet? There's an explicit #define DWARF2_UNWIND_INFO 0 in gcc/config/i386/cygming.h right now. The accompanying comment is Aren't we talking about two different things here? That's for unwinding during exception handling, but you can still leave that at 0 (and use --enable-sjlj-exceptions) and still get the benefit of -gdwarf-2 for gdb's consumption. Brian
Re: [patch] support -gdwarf-2 when creating cygwin1.dbg
Christopher Faylor wrote:
So, maybe a top-level configure option would be useful? At the very least
we can get rid of the -gstabs specific use in configure.
Oh, I didn't know there was anything that specifically mentions -gstabs
in there, just that if you don't set {C,CXX}FLAGS yourself autoconf uses
-g -O2. To me it seems simple enough for now just to require:
.../toplev/configure CXXFLAGS=-gdwarf-2 -O2 CFLAGS=-gdwarf-2 -O2
And then at some point in the future, release updated gcc packages where
-g equals -gdwarf-2 rather than -gstabs (but still without touching any
of the exception handling stuff.)
Aren't we talking about two different things here? That's for unwinding
during exception handling, but you can still leave that at 0 (and use
--enable-sjlj-exceptions) and still get the benefit of -gdwarf-2 for
gdb's consumption.
IIRC, this is turned on because of funkiness with exception unwinding in
DLLs.
I think there were/are two issues:
1. throw/catch unwinding across DLL boundaries currently works but
requires MinGW/Cygwin-local patches that were never championed/accepted
upstream because they were too ugly (and I think Danny said this area
has changed enough in 4.x that they don't forward port at all.) The
pain here might have been related to the fact that currently all target
libraries are static (libgcc, libstdc++, etc) which means there are 'n'
copies of the libgcc code in memory instead of just one in a DLL. So we
need shared target libs, then this becomes simpler.
2. Dwarf-2 EH unwinding through a foreign frame causes problems, one
example of which is where your code is registered with a Win32 API as a
callback and you want to throw from inside that callback. When the
unwinder goes up the stack there is one or more frames inside of
USER32.DLL or NTDLL.DLL or something and it gets lost. This is the one
that is somewhat intractible and would cause us to either foresake that
use pattern or stick with SJLJ. Or maybe somebody will have a bright
idea.
Brian
Re: [patch] support -gdwarf-2 when creating cygwin1.dbg
Corinna Vinschen wrote: I debugged Cygwin native GDB a couple of days ago with code created by gcc 4.2. It turned out that the DWARF2_UNWIND_INFO define set to 0 resulted in the DW_CFA_offset column missing. The result is that GDB is unable to get the return address on the stack when using the dwarf2 frame sniffer. Setting DWARF2_UNWIND_INFO to 1 in gcc/config/i386/cygming.h results in gcc emitting the missing DW_CFA_offset column and GDB is happy again. Older gcc's = 4.0.1 always created the DW_CFA_offset column, so GDB is always happy with the created debug info. Ah, I see. That makes sense. I'm not at all fluent with this stuff. Is that really important for Cygwin? It's not at all important to cygwin1.dll itself but it could be very relevant to Cygwin users that want to make use of C++ code that makes use of exceptions. Danny Smith said he was preparing to release a gcc 4.2.x for MinGW in the somewhat-near future, and since he knows the most about all of this we can wait and see what he decides to do about it. If it's possible to get Dwarf exceptions working in those few corner cases, then that would be great; it's much faster than SJLJ and obviously for the purposes of debug info it's way better. If it turns out that we'll be sticking with SJLJ EH then I think it would be reasonable to try to work up a patch that causes DW_CFA_offset to be set even if not using Dwarf for EH. Brian
Re: [Patch] strace ./app.exe probably runs application from /bin
Christopher Faylor wrote: Let me rephrase the problem: cygpath does not properly deal with the current directory Thanks for the patch but we won't be applying it in this form. I've been meaning to take a look at fixing this myself, because I'm tired of: $ cd /usr/bin $ cygcheck ./ls .\.\.\.\ - Cannot open $ cygcheck ls - Cannot open Error: could not find ls $ cygcheck ls.exe - Cannot open Error: could not find ls.exe $ cygcheck ./ls.exe .\ls.exe .\cygwin1.dll C:\WINXP\system32\ADVAPI32.DLL C:\WINXP\system32\ntdll.dll C:\WINXP\system32\KERNEL32.dll C:\WINXP\system32\RPCRT4.dll .\cygintl-8.dll .\cygiconv-2.dll Brian
Re: Failure in rebuilding Cygwin-1.5.24-2 with recent newlib
Angelo Graziosi wrote: I want to flag that rebuilding Cygwin-1.5.24-2 with recent checkout of newlib fails in this way: It's really not a good idea to mix and match like that, you can run into subtle breakage that way as the two are meant to be kept in sync. For example, if newlib added a new function (as in this case) it will be present in the headers but it won't get exported by the DLL since that requires changes in cygwin.din. And thus if you try to use the combination of lib+headers that you just built you'll get failures since the latter declares an interface that the former doesn't export. And besides, Newlib and Cygwin are in the same CVS repository so all you have to do is check out the cygwin module and you get the latest newlib module automatically. '/home/Angelo/Downloads/cygwin_varie/Snapshots/cygwin-1.5.24-2p5/newlib/libc/string/'`strcasestr.c /home/Angelo/Downloads/cygwin_varie/Snapshots/cygwin-1.5.24-2p5/newlib/libc/string/strcasestr.c:72: error: parse error before string constant /home/Angelo/Downloads/cygwin_varie/Snapshots/cygwin-1.5.24-2p5/newlib/libc/string/strcasestr.c:72: warning: data definition has no type or storage class This is just due to __FBSDID not getting #defined to blank properly. The file includes sys/cdefs.h and newlib's copy contains the required bit (#define __FBSDID(x) /* nothing */) however when building with Cygwin, the Cygwin headers are used and Cygwin's sys/cdefs.h doesn't contain this. The appropriate fix is either to modify strcasestr.c or to fix Cygwin's sys/cdefs.h. I think the latter is probably the better choice, since it seems that there is precedent already in newlib for being able to just #include sys/cdefs.h followed by use of __FBSDID without having to explicitly undefine it. Patch attached which fixes the build for me. Brian2007-06-16 Brian Dessent [EMAIL PROTECTED] * include/sys/cdefs.h (__FBSDID): Define. Index: include/sys/cdefs.h === RCS file: /cvs/src/src/winsup/cygwin/include/sys/cdefs.h,v retrieving revision 1.4 diff -u -p -r1.4 cdefs.h --- include/sys/cdefs.h 8 Aug 2005 18:54:28 - 1.4 +++ include/sys/cdefs.h 16 Jun 2007 15:28:58 - @@ -21,3 +21,4 @@ details. */ #define __CONCAT(__x,__y) __x##__y #endif +#define __FBSDID(x) /* nothing */
Re: Doc change request
Christopher Faylor wrote: Could I ask someone to do a search and replace on the docs and change all occurrences of /usr/man and /usr/doc to /usr/share/man and /usr/share/doc? Brian, do you have time to do this? I think you touched the documentation list so you're it. I can only find a total of three references to either directory, and two of them mention it the context of this was the old location: faq-resources.xml-15-list what man pages the package includes.) Some older packages still keep faq-resources.xml:16:their documentation in literal/usr/doc//literal faq-resources.xml-17-instead of literal/usr/share/doc//literal. setup-net.sgml:235:Relevant documentation can be found in the literal/usr/doc/Cygwin//literal setup-net.sgml-236-or literal/usr/share/doc/Cygwin//literal directory. The only remaining one is a glancing reference in the FAQ to rxvt, and it needs cleaning up anyway as it refers to ash. If the attached fix is OK I will update the htdocs copy too. faq-using.xml-864-paraDon't invoke as simply ``rxvt'' because that will run /bin/sh (really faq-using.xml-865-ash) which is not a good interactive shell. For details see faq-using.xml:866:literal/usr/doc/Cygwin/rxvt-lt;vergt;.README/literal. Unless my grep-fu failed that's it. Brian2007-07-18 Brian Dessent [EMAIL PROTECTED] * faq-using.xml (faq.using.console-window): Mention FHS location of docs and remove outdated reference to ash. Index: faq-using.xml === RCS file: /cvs/src/src/winsup/doc/faq-using.xml,v retrieving revision 1.6 diff -u -p -r1.6 faq-using.xml --- faq-using.xml 26 Aug 2006 19:11:00 - 1.6 +++ faq-using.xml 18 Jul 2007 14:59:31 - @@ -859,11 +859,8 @@ this message from the Cygwin mailing lis You can use it with or without X11. You can resize it easily by dragging an edge or corner. Copy and paste is easy with the left and middle mouse buttons, respectively. It will honor settings in your -~/.Xdefaults file, even without X. -/para -paraDon't invoke as simply ``rxvt'' because that will run /bin/sh (really -ash) which is not a good interactive shell. For details see -literal/usr/doc/Cygwin/rxvt-lt;vergt;.README/literal. +~/.Xdefaults file, even without X. For details see +literal/usr/share/doc/Cygwin/rxvt-lt;vergt;.README/literal. /para /answer/qandaentry
[patch] Fix multithreaded snprintf
I tracked down the problem reported in
http://www.cygwin.com/ml/cygwin/2007-09/msg00120.html. The crash was
occuring in pthread_mutex_lock, but that's a bit of a red herring. The
real problem is that both newlib and Cygwin provide a
include/sys/stdio.h file, however they were out of sync with regard to
the _flockfile definition.
This comes about because vsnprintf() is implemented by creating a struct
FILE that represents the string buffer and then this is passed to the
standard vfprintf(). The 'flags' member of this FILE has the __SSTR
flag set to indicate that this is just a string buffer, and consequently
no locking should or can be performed; the lock member isn't even
initialized.
The newlib/libc/include/sys/stdio.h therefore has this:
#if !defined(_flockfile)
#ifndef __SINGLE_THREAD__
# define _flockfile(fp) (((fp)-_flags __SSTR) ? 0 :
__lock_acquire_recursive((fp)-_lock))
#else
# define _flockfile(fp)
#endif
#endif
#if !defined(_funlockfile)
#ifndef __SINGLE_THREAD__
# define _funlockfile(fp) (((fp)-_flags __SSTR) ? 0 :
__lock_release_recursive((fp)-_lock))
#else
# define _funlockfile(fp)
#endif
#endif
However, the Cygwin version of this header with the same name gets
preference and doesn't know to check the flags like this, and thus
unconditionally tries to lock the stream. This leads ultimately to a
crash in pthread_mutex_lock because the lock member is just
uninitialized junk.
The attached patch fixes Cygwin's copy of the header and makes the
poster's testcase function as expected. This only would appear in a
multithreaded program because the __cygwin_lock_* functions expand to
no-op in the case where there's only one thread.
Since this is used in a C++ file (syscalls.cc) I couldn't do the test ?
0 : func() idiom where void is the return type as that generates a
compiler error, so I use an 'if'.
Brian2007-09-06 Brian Dessent [EMAIL PROTECTED]
* include/sys/stdio.h (_flockfile): Don't try to lock a FILE
that has the __SSTR flag set.
(_ftrylockfile): Likewise.
(_funlockfile): Likewise.
Index: include/sys/stdio.h
===
RCS file: /cvs/src/src/winsup/cygwin/include/sys/stdio.h,v
retrieving revision 1.6
diff -u -p -r1.6 stdio.h
--- include/sys/stdio.h 5 Feb 2006 20:30:24 - 1.6
+++ include/sys/stdio.h 6 Sep 2007 18:27:33 -
@@ -16,13 +16,16 @@ details. */
#if !defined(__SINGLE_THREAD__)
# if !defined(_flockfile)
-#define _flockfile(fp) __cygwin_lock_lock ((_LOCK_T *)(fp)-_lock)
+#define _flockfile(fp) ({ if (!((fp)-_flags __SSTR)) \
+ __cygwin_lock_lock ((_LOCK_T *)(fp)-_lock); })
# endif
# if !defined(_ftrylockfile)
-#define _ftrylockfile(fp) __cygwin_lock_trylock ((_LOCK_T *)(fp)-_lock)
+#define _ftrylockfile(fp) (((fp)-_flags __SSTR) ? 0 : \
+ __cygwin_lock_trylock ((_LOCK_T *)(fp)-_lock))
# endif
# if !defined(_funlockfile)
-#define _funlockfile(fp) __cygwin_lock_unlock ((_LOCK_T *)(fp)-_lock)
+#define _funlockfile(fp) ({ if (!((fp)-_flags __SSTR)) \
+ __cygwin_lock_unlock ((_LOCK_T *)(fp)-_lock); })
# endif
#endif
Re: [patch] Fix multithreaded snprintf
Christopher Faylor wrote: Go ahead and check this in but could you add a comment indicating that this part of include/sys/stdio.h has to be kept in sync with newlib? Done. Nice catch! I wish I could say I caught this by inspection but it was only by single stepping through python guts that it became apparent what was going on. Brian
Re: [patch] inline __getreent in newlib
CC'd to newlib: I've checked in the attached change to
libc/reent/getreent.c as obvious, please let me know if it breaks
anything.
Christopher Faylor wrote:
So, I guess I'll come down on the side of speed over clarity. I'm sure
that Jeff won't mind your checking in the undef in newlib. So, please
check in everything but, again, document heavily what you're doing with
the reent macro.
Done. I added the following comment to config.h to hopefully clarify
the situation:
/* The following provides an inline version of __getreent() for newlib,
which will be used throughout the library whereever there is a _r
version of a function that takes _REENT. This saves the overhead
of a function call for what amounts to a simple computation.
The definition below is essentially equivalent to the one in cygtls.h
(_my_tls.local_clib) however it uses a fixed precomputed
offset rather than dereferencing a field of a structure.
Including tlsoffets.h here in order to get this constant offset
tls_local_clib is a bit of a hack, but the alternative would require
dragging the entire definition of struct _cygtls (a large and complex
Cygwin internal data structure) into newlib. The machinery to
compute these offsets already exists for the sake of gendef so
we might as well just use it here. */
Brian2007-09-06 Brian Dessent [EMAIL PROTECTED]
* libc/reent/getreent.c: Allow for case where __getreent is
defined as a macro.
Index: libc/reent/getreent.c
===
RCS file: /cvs/src/src/newlib/libc/reent/getreent.c,v
retrieving revision 1.1
diff -u -p -r1.1 getreent.c
--- libc/reent/getreent.c 17 May 2002 23:39:37 - 1.1
+++ libc/reent/getreent.c 6 Sep 2007 23:13:10 -
@@ -3,6 +3,10 @@
#include _ansi.h
#include reent.h
+#ifdef __getreent
+#undef __getreent
+#endif
+
struct _reent *
_DEFUN_VOID(__getreent)
{
[patch] inline __getreent in newlib
I noticed today that all instances of _REENT in newlib go through a
function call to __getreent(). All this function does is get the value
of %fs:4 and subtract a fixed offset from it, so this seems rather
wasteful. And we already have the required value of this offset
computed for us in tlsoffsets.h, so we have everything we need to
provide newlib with an inline version of this function, saving the
overhead of a function call. It would obviously be cleaner to be able
to do:
#define __getreent() (_my_tls.local_clib)
...however this would require dragging all kinds of internal Cygwin
definitions into a newlib header and since we already have the required
offset in tlsoffsets.h we might as well just use that. The attached
patch does this; the second part would obviously have to be approved by
the newlib maintainers, but I thought I'd see if there's any interest in
this idea first before bothering them.
The following is the result of the iospeed output from the testsuite:
(units are ms elapsed as returned by GetTickCount, so smaller is better,
but note that the resolution here is at best 10ms.)
Before:
- text - binary
lineszcr getc fread fgets getc fread fgets
4 0 1906 110 656 189078 719
64 0 190694 218 190746 110
4096 0 1922 125 172 23136263
4 1 1438 203 640 189063 719
64 1 1891 109 219 19226394
4096 1 193893 188 19227878
After:
- text - binary
lineszcr getc fread fgets getc fread fgets
4 0 1781 125 672 178262 703
64 0 1765 110 218 175062 109
4096 0 179793 188 17667878
4 1 1328 188 609 175062 719
64 1 1750 109 203 178147 109
4096 1 1797 125 172 17666263
I don't pretend to claim that this is a very scientific benchmark at
all, but there does seem to be a slight improvement especially in the
getc column which represents reading the whole 16MB file one byte at a
time, where this _REENT overhead would be most pronounced.
So, valid optimization or just complication?
Brian2007-09-06 Brian Dessent [EMAIL PROTECTED]
* include/cygwin/config.h (__getreent): Define inline version.
Index: include/cygwin/config.h
===
RCS file: /cvs/src/src/winsup/cygwin/include/cygwin/config.h,v
retrieving revision 1.5
diff -u -p -r1.5 config.h
--- include/cygwin/config.h 15 Nov 2003 17:04:10 - 1.5
+++ include/cygwin/config.h 6 Sep 2007 23:12:33 -
@@ -20,6 +20,9 @@ extern C {
#define _CYGWIN_CONFIG_H
#define __DYNAMIC_REENT__
+#include ../tlsoffsets.h
+extern char *_tlsbase __asm__ (%fs:4);
+#define __getreent() (struct _reent *)(_tlsbase + tls_local_clib)
#define __FILENAME_MAX__ (260 - 1 /* NUL */)
#define _READ_WRITE_RETURN_TYPE _ssize_t
#define __LARGE64_FILES 1
2007-09-06 Brian Dessent [EMAIL PROTECTED]
* libc/reent/getreent.c: Allow for case where __getreent is
defined as a macro.
Index: libc/reent/getreent.c
===
RCS file: /cvs/src/src/newlib/libc/reent/getreent.c,v
retrieving revision 1.1
diff -u -p -r1.1 getreent.c
--- libc/reent/getreent.c 17 May 2002 23:39:37 - 1.1
+++ libc/reent/getreent.c 6 Sep 2007 23:13:10 -
@@ -3,6 +3,10 @@
#include _ansi.h
#include reent.h
+#ifdef __getreent
+#undef __getreent
+#endif
+
struct _reent *
_DEFUN_VOID(__getreent)
{
Re: [patch] inline __getreent in newlib
Brian Dessent wrote:
Done. I added the following comment to config.h to hopefully clarify
the situation:
/* The following provides an inline version of __getreent() for newlib,
which will be used throughout the library whereever there is a _r
version of a function that takes _REENT. This saves the overhead
of a function call for what amounts to a simple computation.
The definition below is essentially equivalent to the one in cygtls.h
(_my_tls.local_clib) however it uses a fixed precomputed
offset rather than dereferencing a field of a structure.
Including tlsoffets.h here in order to get this constant offset
tls_local_clib is a bit of a hack, but the alternative would require
dragging the entire definition of struct _cygtls (a large and complex
Cygwin internal data structure) into newlib. The machinery to
compute these offsets already exists for the sake of gendef so
we might as well just use it here. */
Turns out that sys/config.h includes cygwin/config.h, which leads to
this breakage when the winsup headers are installed in the system
location:
$ echo #include math.h | gcc -x c -
In file included from /usr/include/sys/config.h:180,
from /usr/include/_ansi.h:16,
from /usr/include/sys/reent.h:13,
from /usr/include/math.h:5,
from stdin:1:
/usr/include/cygwin/config.h:22:27: ../tlsoffsets.h: No such file or
directory
Attached patch fixes the situation by only exposing this when
_COMPILING_NEWLIB. Ok?
Brian2007-09-07 Brian Dessent [EMAIL PROTECTED]
* include/cygwin/config.h: Conditionalize inline __getreent()
definition on _COMPILING_NEWLIB.
Index: include/cygwin/config.h
===
RCS file: /cvs/src/src/winsup/cygwin/include/cygwin/config.h,v
retrieving revision 1.6
diff -u -p -r1.6 config.h
--- include/cygwin/config.h 7 Sep 2007 00:44:27 - 1.6
+++ include/cygwin/config.h 7 Sep 2007 23:15:23 -
@@ -37,9 +37,11 @@ extern C {
compute these offsets already exists for the sake of gendef so
we might as well just use it here. */
+#ifdef _COMPILING_NEWLIB
#include ../tlsoffsets.h
extern char *_tlsbase __asm__ (%fs:4);
#define __getreent() (struct _reent *)(_tlsbase + tls_local_clib)
+#endif /* _COMPILING_NEWLIB */
#define __FILENAME_MAX__ (260 - 1 /* NUL */)
#define _READ_WRITE_RETURN_TYPE _ssize_t
Re: Rewrite/fix cygwin1.dbg generation
Pedro Alves wrote:
The dllfixdbg hunk looks hard to read. Here's what is looks
like after patching:
I think that if whatever bugs used to exist in older binutils PE support
that necessitated this hackery are now gone, we can just do away with
dllfixdbg alltogether and just put this:
${STRIP} --strip-debug ${DLL} -o stripped-${DLL}
${STRIP} --only-keep-debug ${DLL} -o ${DBG}
${OBJCOPY} --add-gnu-debuglink=${DBG} stripped-${DLL} ${DLL}
rm -f stripped-${DLL}
...in the Makefile.
Brian
Re: Rewrite/fix cygwin1.dbg generation
Pedro Alves wrote: 10 .cygheap 000a 611e 611e 2**2 ALLOC 11 .gnu_debuglink 0010 6128 6128 001d0a00 2**2 CONTENTS, READONLY, DEBUGGING I'll come up with a different fix. Just thinking out loud here... what about teaching objcopy that when doing --add-gnu-debuglink if there'a already a section named .gnu_debuglink (and it's of sufficient length to hold the .dbg filename) that it can just rewrite its contents, rather than appending a new section? That way we can continue to allocate the section in the link script (except without having to call it .gnu_debuglink_overlay) so that we can put the .cygheap last, but we don't have to do the dllfixdbg hackery to get the ordering correct. The downside here would be that if we rely on this feature of objcopy then we'd need to either require bleeding edge binutils to build Cygwin or do some kind of autoconf runtime test to test for behavior. Still, it would be nice to lay the groundwork for being able to one day retire the hackery. Brian
[patch] un-NT-ify cygcheck (was: cygwin 1.5.25-7: cygcheck does not work?)
Brian Dessent wrote:
Fortunately, I have VMware with a Win98 image here.
The problem is that bloda.c calls NtQuerySystemInformation without using
any kind of autoload.cc-type indirection, and so cygcheck gets a hard
dependency on ntdll.dll which doesn't exist on 9x/ME. We need to do one
of:
- Revert the bloda-check feature on the 1.5 branch
- Check windows version at runtime and only do NT calls through
LoadLibrary/GetProcAddress
- Use the autoload.cc trick in cygcheck
If we're going to make releases from the 1.5 branch then I don't think
it's quite acceptible just yet to shaft 9x users, after all that's the
whole point of the branch.
The attached patch un-NT-ifies bloda.cc but sadly a similar cleanup is
still required for cygpath as well if we are to support 9x/ME out of the
1.5 branch. In that case I suppose you could just revert cygpath.cc to
an older revision before the native APIs were added.
Brian2007-12-20 Brian Dessent [EMAIL PROTECTED]
* Makefile.in (cygcheck.exe): Don't link to ntdll.
* bloda.cc (pNtQuerySystemInformation): Add.
(pRtlAnsiStringToUnicodeString): Add.
(get_process_list): Use function pointers for NT functions.
(dump_dodgy_apps): Skip dodgy app check on non-NT platforms.
Use GetProcAddress for NT-specific functions.
Index: Makefile.in
===
RCS file: /cvs/src/src/winsup/utils/Makefile.in,v
retrieving revision 1.67
diff -u -p -r1.67 Makefile.in
--- Makefile.in 3 Aug 2007 19:41:48 - 1.67
+++ Makefile.in 20 Dec 2007 15:08:23 -
@@ -104,10 +104,10 @@ ifeq $(libz)
@echo '*** Building cygcheck without package content checking due to
missing mingw libz.a.'
endif
ifdef VERBOSE
- $(CXX) $(MINGW_CXXFLAGS) -o $@ ${wordlist 1,4,$^} -B$(mingw_build)/
$(MINGW_LDFLAGS) $(libz) -lntdll
+ $(CXX) $(MINGW_CXXFLAGS) -o $@ ${wordlist 1,4,$^} -B$(mingw_build)/
$(MINGW_LDFLAGS) $(libz)
else
- @echo $(CXX) -o $@ ${wordlist 1,4,$^} ${filter-out -B%,
$(MINGW_CXXFLAGS) $(MINGW_LDFLAGS)} $(libz) -lntdll;\
- $(CXX) $(MINGW_CXXFLAGS) -o $@ ${wordlist 1,4,$^} -B$(mingw_build)/
$(MINGW_LDFLAGS) $(libz) -lntdll
+ @echo $(CXX) -o $@ ${wordlist 1,4,$^} ${filter-out -B%,
$(MINGW_CXXFLAGS) $(MINGW_LDFLAGS)} $(libz);\
+ $(CXX) $(MINGW_CXXFLAGS) -o $@ ${wordlist 1,4,$^} -B$(mingw_build)/
$(MINGW_LDFLAGS) $(libz)
endif
dumper.o: dumper.cc dumper.h
Index: bloda.cc
===
RCS file: /cvs/src/src/winsup/utils/bloda.cc,v
retrieving revision 1.1
diff -u -p -r1.1 bloda.cc
--- bloda.cc3 Aug 2007 19:41:48 - 1.1
+++ bloda.cc20 Dec 2007 15:08:23 -
@@ -104,13 +104,20 @@ static const size_t num_of_dodgy_apps =
to be looked up at runtime and called through a pointer. */
VOID NTAPI (*pRtlFreeUnicodeString)(PUNICODE_STRING) = NULL;
+NTSTATUS NTAPI (*pNtQuerySystemInformation) (SYSTEM_INFORMATION_CLASS,
+ PVOID, ULONG, PULONG) = NULL;
+
+NTSTATUS NTAPI (*pRtlAnsiStringToUnicodeString) (PUNICODE_STRING, PANSI_STRING,
+ BOOLEAN) = NULL;
+
+
static PSYSTEM_PROCESSES
get_process_list (void)
{
int n_procs = 0x100;
PSYSTEM_PROCESSES pslist = (PSYSTEM_PROCESSES) malloc (n_procs * sizeof
*pslist);
- while (NtQuerySystemInformation (SystemProcessesAndThreadsInformation,
+ while (pNtQuerySystemInformation (SystemProcessesAndThreadsInformation,
pslist, n_procs * sizeof *pslist, 0) == STATUS_INFO_LENGTH_MISMATCH)
{
n_procs *= 2;
@@ -126,7 +133,7 @@ get_module_list (void)
int modsize = 0x1000;
PSYSTEM_MODULE_INFORMATION modlist = (PSYSTEM_MODULE_INFORMATION) malloc
(modsize);
- while (NtQuerySystemInformation (SystemModuleInformation,
+ while (pNtQuerySystemInformation (SystemModuleInformation,
modlist, modsize, NULL) == STATUS_INFO_LENGTH_MISMATCH)
{
modsize *= 2;
@@ -284,19 +291,14 @@ detect_dodgy_app (const struct bad_app_d
/* Equivalent of RtlInitAnsiString. */
ansiname.Length = ansiname.MaximumLength = strlen (det-param);
ansiname.Buffer = (CHAR *) det-param;
- rv = RtlAnsiStringToUnicodeString (unicodename, ansiname, TRUE);
+ rv = pRtlAnsiStringToUnicodeString (unicodename, ansiname, TRUE);
if (rv != STATUS_SUCCESS)
{
printf (Ansi to unicode conversion failure $%08x\n, (unsigned int)
rv);
break;
}
found = find_process_in_list (pslist, unicodename);
- if (!pRtlFreeUnicodeString)
- pRtlFreeUnicodeString = (VOID NTAPI (*)(PUNICODE_STRING))
GetProcAddress (LoadLibrary (ntdll.dll), RtlFreeUnicodeString);
- if (pRtlFreeUnicodeString)
-pRtlFreeUnicodeString (unicodename);
- else
-printf (leaking mem...oops\n);
+ pRtlFreeUnicodeString (unicodename);
if (found
Re: [patch] un-NT-ify cygcheck (was: cygwin 1.5.25-7: cygcheck does not work?)
Brian Dessent wrote: ... but sadly a similar cleanup is still required for cygpath as well if we are to support 9x/ME out of the 1.5 branch. In that case I suppose you could just revert cygpath.cc to an older revision before the native APIs were added. Er, nevermind. I was accidently looking at HEAD, but the native stuff in cygpath is not on the branch. So I think only the bloda.cc change is necessary to restore 9x/ME capability on the branch. Brian
Re: [patch] un-NT-ify cygcheck (was: cygwin 1.5.25-7: cygcheck does not work?)
Christopher Faylor wrote:
Unless Corinna says differently, I think she wants to be in control of
what goes into the branch so I don't want to suggest that you should
check it in there too.
Okay, I'll let her take care of the branch since she's been handling all
the releases from it.
The problem in this case would be Hey! Look at what cygcheck is saying!
You are using Windows 9x! You can't do that!
In a sense it already does this:
case VER_PLATFORM_WIN32_WINDOWS:
switch (osversion.dwMinorVersion)
{
case 0:
osname = 95 (not supported);
break;
case 10:
osname = 98 (not supported);
break;
case 90:
osname = ME (not supported);
break;
default:
osname = 9X (not supported);
break;
}
break;
Brian
[patch] fix strfuncs-related breakage of cygserver
The recent addition of the sys_{wcstombs,mbstowcs}_alloc() functions to
strfuncs.cc causes cygserver to no longer build. The problem is simply
that we can't call ccalloc() from within cygserver, but cygserver needs
__small_vsprintf() which in turn calls sys_wcstombs_alloc(), which in
turn wants to call ccalloc(). To get around this, I just
conditionalized the foo_alloc() functions to always use plain calloc()
when inside cygserver, and changed cygserver's Makefile to rebuild
strfuncs.cc again instead of sharing the .o from the DLL.
There is also a small additional buglet in that the call to
sys_wcstombs_alloc() in __small_vsprintf() was passing PATH_MAX as the
heap type, and that is not a valid cygheap_types. I changed it to
HEAP_NOTHEAP as that is the only value that makes sense here since this
pointer is subsequently free()'d and not cfree()'d.
Attached are two patches, one for cygwin/ and one in cygserver/.
Brian2008-02-03 Brian Dessent [EMAIL PROTECTED]
* smallprint.cc (__small_vsprintf): Use HEAP_NOTHEAP for type.
* strfuncs.cc (sys_wcstombs_alloc): Guard use of ccalloc
to !__OUTSIDE_CYGWIN__ for use in cygserver.
(sys_mbstowcs_alloc): Ditto.
Index: smallprint.cc
===
RCS file: /cvs/src/src/winsup/cygwin/smallprint.cc,v
retrieving revision 1.4
diff -u -p -r1.4 smallprint.cc
--- smallprint.cc 31 Jan 2008 20:26:01 - 1.4
+++ smallprint.cc 4 Feb 2008 03:18:45 -
@@ -197,7 +197,7 @@ __small_vsprintf (char *dst, const char
{
char *tmp;
- if (!sys_wcstombs_alloc (tmp, PATH_MAX, us-Buffer,
+ if (!sys_wcstombs_alloc (tmp, HEAP_NOTHEAP, us-Buffer,
us-Length / sizeof (WCHAR)))
{
s = invalid UNICODE_STRING;
Index: strfuncs.cc
===
RCS file: /cvs/src/src/winsup/cygwin/strfuncs.cc,v
retrieving revision 1.4
diff -u -p -r1.4 strfuncs.cc
--- strfuncs.cc 31 Jan 2008 20:26:01 - 1.4
+++ strfuncs.cc 4 Feb 2008 03:18:45 -
@@ -60,7 +60,11 @@ sys_wcstombs (char *tgt, int tlen, const
value is the number of bytes written to the buffer, as usual.
The type argument determines where the resulting buffer is stored.
It's either one of the cygheap_types values, or it's HEAP_NOTHEAP.
- In the latter case the allocation uses simple calloc. */
+ In the latter case the allocation uses simple calloc.
+
+ Note that this code is shared by cygserver (which requires it via
+ __small_vsprintf) and so when built there plain calloc is the
+ only choice. */
int __stdcall
sys_wcstombs_alloc (char **tgt_p, int type, const PWCHAR src, int slen)
{
@@ -71,10 +75,14 @@ sys_wcstombs_alloc (char **tgt_p, int ty
{
size_t tlen = (slen == -1 ? ret : ret + 1);
+#ifndef __OUTSIDE_CYGWIN__
if (type == HEAP_NOTHEAP)
+#endif
*tgt_p = (char *) calloc (tlen, sizeof (char));
+#ifndef __OUTSIDE_CYGWIN__
else
*tgt_p = (char *) ccalloc ((cygheap_types) type, tlen, sizeof (char));
+#endif
if (!*tgt_p)
return 0;
ret = sys_wcstombs (*tgt_p, tlen, src, slen);
@@ -98,10 +106,14 @@ sys_mbstowcs_alloc (PWCHAR *tgt_p, int t
ret = MultiByteToWideChar (get_cp (), 0, src, -1, NULL, 0);
if (ret)
{
+#ifndef __OUTSIDE_CYGWIN__
if (type == HEAP_NOTHEAP)
+#endif
*tgt_p = (PWCHAR) calloc (ret, sizeof (WCHAR));
+#ifndef __OUTSIDE_CYGWIN__
else
*tgt_p = (PWCHAR) ccalloc ((cygheap_types) type, ret, sizeof (WCHAR));
+#endif
if (!*tgt_p)
return 0;
ret = sys_mbstowcs (*tgt_p, src, ret);
2008-02-03 Brian Dessent [EMAIL PROTECTED]
* Makefile.in: Don't link strfuncs.o from the Cygwin build dir.
Build it again with __OUTSIDE_CYGWIN__ defined.
Index: Makefile.in
===
RCS file: /cvs/src/src/winsup/cygserver/Makefile.in,v
retrieving revision 1.16
diff -u -p -r1.16 Makefile.in
--- Makefile.in 2 Aug 2007 14:23:22 - 1.16
+++ Makefile.in 4 Feb 2008 03:22:32 -
@@ -43,7 +43,7 @@ OBJS:=cygserver.o client.o process.o ms
sysv_msg.o sysv_sem.o sysv_shm.o
LIBOBJS:=${patsubst %.o,lib%.o,$(OBJS)}
-CYGWIN_OBJS:=$(cygwin_build)/smallprint.o $(cygwin_build)/strfuncs.o
$(cygwin_build)/version.o
+CYGWIN_OBJS:=$(cygwin_build)/smallprint.o $(cygwin_build)/version.o
CYGWIN_LIB:=$(cygwin_build)/libcygwin.a
@@ -67,7 +67,7 @@ libclean:
fullclean: clean libclean
-cygserver.exe: $(CYGWIN_LIB) $(OBJS) $(CYGWIN_OBJS)
+cygserver.exe: $(CYGWIN_LIB) $(OBJS) $(CYGWIN_OBJS) strfuncs.o
$(CXX) -o $@ ${wordlist 2,999,$^} -L$(cygwin_build) -lntdll
$(cygwin_build)/%.o: $(cygwin_source)/%.cc
@@ -81,6 +81,9 @@ Makefile: Makefile.in configure
lib%.o: %.cc
${filter
Re: PATCH: avoid system shared memory version mismatch detected by versioning shared memory name
Noel Burton-Krahn wrote: The problem is there are several installable apps built on Cygwin, like EAC, ClamAV, and one I just found which is a Cygwin-on-a-thumbdrive. The problem is they can't all coexist because they're distributed with different versions of the cygwin dlls. Making them work with the current cygwin means hand-copying cygwin dlls into application directories, and repeating that every time you upgrade. People used to give Windows a hard time for DLL hell! I don't see the benefit of forcing users to hand-maintain cygwin dlls across multiple applications. But we go to great pains to make the DLL binary backwards compatible always. So in this case all you have to do is maintain one copy in the PATH and make sure it's the most recent, deleting all others. This should be a job for the installers of those 3PPs. Besides, the shared memory region is only one of a whole bunch of IPC objects that would need their own namespace if you wanted the true ability to insulate two versions of Cygwin. Fire up process explorer, click on the handle search, and enter cygwin1S4 to get a rough list. To truly make this work you'd need to change shared_prefix. And the registry key for the mount table. And probably other things too. Brian
[patch] handle_to_fn: null terminate
I noticed in strace some lines like: fhandler_base::close: closing '/Device/NamedPipe/Win32Pipes.08e0.0002several junk bytes' handle 0x740 This was caused by handle_to_fn simply forgetting to add a \0 when converting, as in the attached patch. Brian2008-03-07 Brian Dessent [EMAIL PROTECTED] * dtable.cc (handle_to_fn): Null-terminate posix_fn in the case of justslash = true. Index: dtable.cc === RCS file: /cvs/src/src/winsup/cygwin/dtable.cc,v retrieving revision 1.182 diff -u -p -r1.182 dtable.cc --- dtable.cc 15 Feb 2008 17:53:10 - 1.182 +++ dtable.cc 8 Mar 2008 01:33:52 - @@ -952,6 +952,7 @@ handle_to_fn (HANDLE h, char *posix_fn) *d = '/'; else *d = *s; + *d = 0; } debug_printf (derived path '%s', posix '%s', w32, posix_fn);
[patch] reorganize utils/Makefile.in
This patch is a revamping of the Makefile in winsup/utils. The current
Makefile.in is fugly, in my humble opinion. It's got lots of repeated
rules and it's not very clear how one is supposed to add or change
things. This patch does use GNU make specific features, but I'm quite
sure we already use them in other places, e.g. $(wildcard ...),
$(patsubst ...), $(shell ...) etc. are all GNU make features AFAIK and
those are all over the place in winsup.
I've also attached a copy of the patched Makefile.in if it's easier to
review that way; the diff is quite ugly. As you can see the total
number of lines in the file is decreased by about 70, and that's
including a number of comments that I have added to document how to use
the file.
I have tried fairly hard to make sure that no actual behavior has
changed. I diffed a before and after run and the only real difference
seemed to be the order that things ran, as well as a couple of flags
moved to a different spot in their commands. I also tested the case
where libbfd.a/libintl.a are absent, causing the dumper parts to be
skipped as well as missing a MinGW zlib for cygcheck. I haven't tested
a crosscompile but I can if that's necessary. I have tested builds in
both a combined tree as well as just winsup.
Brian2008-03-08 Brian Dessent [EMAIL PROTECTED]
* Makefile.in: Reorganize considerably, using GNU make's
static pattern rules and target-specific variables.
Makefile.in | 229 +---
1 file changed, 81 insertions(+), 148 deletions(-)
Index: Makefile.in
===
RCS file: /cvs/src/src/winsup/utils/Makefile.in,v
retrieving revision 1.68
diff -u -p -r1.68 Makefile.in
--- Makefile.in 21 Dec 2007 03:32:46 - 1.68
+++ Makefile.in 8 Mar 2008 13:57:57 -
@@ -1,6 +1,6 @@
# Makefile for Cygwin utilities
# Copyright 1996, 1997, 1998, 1999, 2000, 2001, 2002, 2003, 2004,
-# 2005, 2006, 2007 Red Hat, Inc.
+# 2005, 2006, 2007, 2008 Red Hat, Inc.
# This file is part of Cygwin.
@@ -36,161 +36,115 @@ override CXXFLAGS+=-fno-exceptions -fno-
include $(srcdir)/../Makefile.common
-LIBICONV:[EMAIL PROTECTED]@
-libbfd:=${shell $(CC) -B$(bupdir2)/bfd/ --print-file-name=libbfd.a}
-libintl:=${shell $(CC) -B$(bupdir2)/intl/ --print-file-name=libintl.a}
-build_dumper:=${shell test -r $(libbfd) -a -r $(libintl) -a -n $(LIBICONV)
echo 1}
-
-libz:=${shell x=$$($(CC) -mno-cygwin --print-file-name=libz.a); cd $$(dirname
$$x); dir=$$(pwd); case $$dir in *mingw*) echo $$dir/libz.a ;; esac}
-zlib_h:=-include ${patsubst %/lib/mingw/libz.a,%/include/zlib.h,${patsubst
%/lib/libz.a,%/include/zlib.h,$(libz)}}
-zconf_h:=${patsubst %/zlib.h,%/zconf.h,$(zlib_h)}
-ifeq ${libz}
-zlib_h:=
-zconf_h:=
-libz:=
-endif
-
-DUMPER_INCLUDES:=-I$(bupdir2)/bfd -I$(updir1)/include
-
-libcygwin:=$(cygwin_build)/libcygwin.a
-libuser32:=$(w32api_lib)/libuser32.a
-libkernel32:=$(w32api_lib)/libkernel32.a
-ALL_DEP_LDLIBS:=$(libcygwin) $(w32api_lib)/libnetapi32.a \
- $(w32api_lib)/libadvapi32.a $(w32api_lib)/libkernel32.a \
- $(w32api_lib)/libuser32.a
-
-ALL_LDLIBS:=${patsubst $(w32api_lib)/lib%.a,-l%,\
- ${filter-out $(libuser32),\
- ${filter-out $(libkernel32),\
- ${filter-out $(libcygwin), $(ALL_DEP_LDLIBS)
-
-MINGW_LIB:=$(mingw_build)/libmingw32.a
-DUMPER_LIB:=${libbfd} ${libintl} -L$(bupdir1)/libiberty $(LIBICONV) -liberty
-MINGW_LDLIBS:=${filter-out $(libcygwin),$(ALL_LDLIBS) $(MINGW_LIB)}
-MINGW_DEP_LDLIBS:=${ALL_DEP_LDLIBS} ${MINGW_LIB}
-ALL_LDFLAGS:=-B$(newlib_build)/libc -B$(w32api_lib) $(LDFLAGS) $(ALL_LDLIBS)
-DUMPER_LDFLAGS:=$(ALL_LDFLAGS) $(DUMPER_LIB)
-MINGW_CXX:=${patsubst %/cygwin/include,%/mingw/include,${filter-out
-I$(newlib_source)/%,$(COMPILE_CXX)}} -I$(updir)
-
-PROGS:=cygcheck.exe cygpath.exe getfacl.exe kill.exe mkgroup.exe \
- mkpasswd.exe mount.exe passwd.exe ps.exe regtool.exe setfacl.exe \
- setmetamode.exe ssp.exe strace.exe umount.exe ipcrm.exe ipcs.exe
-
-CLEAN_PROGS:=$(PROGS)
-ifndef build_dumper
-PROGS:=warn_dumper $(PROGS)
-else
-PROGS+=dumper$(EXEEXT)
-CLEAN_PROGS+=dumper.exe
-endif
-
.SUFFIXES:
.NOEXPORT:
+.PHONY: all install clean realclean warn_dumper warn_cygcheck_zlib
-.PHONY: all install clean realclean warn_dumper
-
-all: Makefile $(PROGS)
-
-strace.exe: strace.o path.o $(MINGW_DEP_LDLIBS)
-ifdef VERBOSE
- $(CXX) $(MINGW_CXXFLAGS) -o $@ ${wordlist 1,2,$^} -B$(mingw_build)/
$(MINGW_LDFLAGS)
-else
- @echo $(CXX) -o $@ ${wordlist 1,2,$^} ${filter-out -B%,
$(MINGW_CXXFLAGS) $(MINGW_LDFLAGS)};\
- $(CXX) $(MINGW_CXXFLAGS) -o $@ ${wordlist 1,2,$^} -B$(mingw_build)/
$(MINGW_LDFLAGS)
-endif
-
-cygcheck.exe: cygcheck.o bloda.o path.o dump_setup.o $(MINGW_DEP_LDLIBS)
-ifeq $(libz)
- @echo '*** Building cygcheck without package content checking due to
missing mingw libz.a.'
-endif
-ifdef VERBOSE
[patch] utils/path.cc fixes and testsuite
The winsup/utils/path.cc file implements a primative POSIX-Win32 path
conversion API that is independant of the real one in Cygwin. This is
used by cygcheck as well as strace (for opening the -o parameter).
Currently this code has bitrotted a bit, and its handling of relative
paths seems to be a little awkward. Examples of how it's broken:
strace -o foo.out cmd will place foo.out file in / (or some other
strange location, usually the root of whatever prefix of the mount table
matched CWD) instead of the CWD.
cd /bin cygcheck ls reports Error: could not find ls, but cd
/bin cygcheck ./ls does work.
cd / cygcheck usr/bin/ls reports usr/bin/ls - Cannot open but cd
/ cygcheck bin/ls works.
cp /bin/ls /tmp cd / cygcheck tmp/ls reports tmp/ls - Cannot
open.
Anyway, I took a look at fixing this so that the path.cc code is a
little more robust. But as I'm sure you're aware, messing with path
conversion code is really annoying when you factor in all sorts of weird
corner cases, so I decided to make a testsuite for this code so that it
would be possible to both cure the bitrot as well as know if it has
regressed again. That attached patch does both.
The harness that I came up with consists of simply a header
(testsuite.h) which contains definitions for both a toy mount table as
well as a series of {CWD,POSIX input,expected Win32 output} tuples that
form the tests. The driver is testsuite.c, and it works by recompiling
path.cc with -DTESTSUITE to activate the hooks. It's pretty simplistic,
but it has allowed me to fix a number of corner cases in the code such
that all of the above mentioned oddities are now gone. In order to make
the testsuite pass I also had to add a little bit of normalization code
so that e.g. the Win32 output always uses backslashes and that it
doesn't leave repeated slashes between parts, e.g. foo\\/bar. For the
sample mount table I tried to emulate what a real mount table is
typically like, with a couple additional mounts.
The Makefile changes to support this are pretty straightforward: nothing
changes for the default make all case. If you run make check it
builds the required files and runs the testsuite. I've attached a
sample output.
Brian2008-03-08 Brian Dessent [EMAIL PROTECTED]
* Makefile.in: Add a 'check' target that builds and runs
testsuite.exe from path-testsuite.o and testsuite.o.
* path.cc: Include testsuite.h.
(struct mnt): Change to a mnt_t typedef and don't define
mount_table when TESTSUITE is defined.
(find2): Don't include when TESTSUITE is defined to avoid warning.
(get_cygdrive0): Ditto.
(get_cygdrive): Ditto.
(read_mounts): Provide empty implementation when TESTSUITE is
defined.
(vconcat): Use the isslash macro.
(unconvert_slashes): New helper to convert to backslashses.
(rel_vconcat): Handle relative paths more gracefully.
(cygpath): Skip a leading ./ sequence. Avoid double-slashes.
Normalize final output to backslashes and remove redundant path
sequences.
* testsuite.cc: New file implementing testsuite driver.
* testsuite.h: New header implementing harness mount table and
series of tests.
Makefile.in | 19 +++-
path.cc | 57 +
testsuite.cc | 89
testsuite.h | 130 +++
4 files changed, 283 insertions(+), 12 deletions(-)
Index: Makefile.in
===
RCS file: /cvs/src/src/winsup/utils/Makefile.in,v
retrieving revision 1.69
diff -u -p -r1.69 Makefile.in
--- Makefile.in 8 Mar 2008 17:52:49 - 1.69
+++ Makefile.in 8 Mar 2008 18:11:10 -
@@ -99,10 +99,23 @@ else
all: warn_cygcheck_zlib
endif
-# the rest of this file contains generic rules
-
all: Makefile $(CYGWIN_BINS) $(MINGW_BINS)
+# test harness support (note: the MINGW_BINS += should come after the
+# all: above so that the testsuite is not run for make but only
+# make check.)
+MINGW_BINS += testsuite.exe
+MINGW_OBJS += path-testsuite.o testsuite.o
+testsuite.exe: path-testsuite.o
+path-testsuite.cc: path.cc ; ln -sf ${filter %.cc,$^} $@
+path-testsuite.o: MINGW_CXXFLAGS += -DTESTSUITE
+# this is necessary because this .c lives in the build dir instead of src
+path-testsuite.o: MINGW_CXX := ${patsubst -I.,-I$(utils_source),$(MINGW_CXX)}
+path-testsuite.cc path.cc testsuite.cc: testsuite.h
+check: testsuite.exe ; $(D)/$(F)
+
+# the rest of this file contains generic rules
+
# how to compile a MinGW object
$(MINGW_OBJS): %.o: %.cc
ifdef VERBOSE
@@ -137,7 +150,7 @@ $(MINGW_BINS): $(MINGW_DEP_LDLIBS)
$(CYGWIN_BINS): $(ALL_DEP_LDLIBS)
clean:
- rm -f *.o $(CYGWIN_BINS) $(MINGW_BINS)
+ rm -f *.o $(CYGWIN_BINS) $(MINGW_BINS) path-testsuite.cc testsuite.exe
realclean: clean
rm -f Makefile config.cache
Index
Re: [patch] utils/path.cc fixes and testsuite
Christopher Faylor wrote:
Would it be possible to uncollapse this if block and make it a little
clearer? The else nine lines away makes it a little hard to follow.
So, wouldn't just inverting the if (match) to if (!match) and
putting the else condition cause some unnesting?
Here's a v2 patch. Changes:
- As suggested I used path[max_len] instead of *(path + max_len). I had
done it that way originally to try to make it clear that I was looking
at what the first chararacter of the path + max_len argument to
concat, but I now agree it's kind of unidiomatic C.
- Rearranged the if/else block, and added a comment for the logic of
each branch.
- Added a test for UNC paths.
- Minor tweak on the Makefile rule that symlinks the source file to
another name to prevent it from running every time. In general I'm not
all that crazy with this idea of symlinking a file in order to compile
it to a differently-named object, but doing it otherwise would require
repeating the compile rule with all its ugly VERBOSE casing and I just
went to the trouble to eliminate all such repetition in the Makefile.
Brian2008-03-08 Brian Dessent [EMAIL PROTECTED]
* Makefile.in: Add a 'check' target that builds and runs
testsuite.exe from path-testsuite.o and testsuite.o.
* path.cc: Include testsuite.h.
(struct mnt): Change to a mnt_t typedef and don't define
mount_table when TESTSUITE is defined.
(find2): Don't include when TESTSUITE is defined to avoid warning.
(get_cygdrive0): Ditto.
(get_cygdrive): Ditto.
(read_mounts): Provide empty implementation when TESTSUITE is
defined.
(vconcat): Use the isslash macro.
(unconvert_slashes): New helper to convert to backslashses.
(rel_vconcat): Handle relative paths more gracefully.
(cygpath): Skip a leading ./ sequence. Avoid double-slashes.
Normalize final output to backslashes and remove redundant path
sequences.
* testsuite.cc: New file implementing testsuite driver.
* testsuite.h: New header implementing harness mount table and
series of tests.
Makefile.in | 19 +++-
path.cc | 71 +++
testsuite.cc | 89
testsuite.h | 131 +++
4 files changed, 298 insertions(+), 12 deletions(-)
Index: Makefile.in
===
RCS file: /cvs/src/src/winsup/utils/Makefile.in,v
retrieving revision 1.69
diff -u -p -r1.69 Makefile.in
--- Makefile.in 8 Mar 2008 17:52:49 - 1.69
+++ Makefile.in 9 Mar 2008 03:01:17 -
@@ -99,10 +99,23 @@ else
all: warn_cygcheck_zlib
endif
-# the rest of this file contains generic rules
-
all: Makefile $(CYGWIN_BINS) $(MINGW_BINS)
+# test harness support (note: the MINGW_BINS += should come after the
+# all: above so that the testsuite is not run for make but only
+# make check.)
+MINGW_BINS += testsuite.exe
+MINGW_OBJS += path-testsuite.o testsuite.o
+testsuite.exe: path-testsuite.o
+path-testsuite.cc: path.cc ; @test -L $@ || ln -sf ${filter %.cc,$^} $@
+path-testsuite.o: MINGW_CXXFLAGS += -DTESTSUITE
+# this is necessary because this .c lives in the build dir instead of src
+path-testsuite.o: MINGW_CXX := ${patsubst -I.,-I$(utils_source),$(MINGW_CXX)}
+path-testsuite.cc path.cc testsuite.cc: testsuite.h
+check: testsuite.exe ; $(D)/$(F)
+
+# the rest of this file contains generic rules
+
# how to compile a MinGW object
$(MINGW_OBJS): %.o: %.cc
ifdef VERBOSE
@@ -137,7 +150,7 @@ $(MINGW_BINS): $(MINGW_DEP_LDLIBS)
$(CYGWIN_BINS): $(ALL_DEP_LDLIBS)
clean:
- rm -f *.o $(CYGWIN_BINS) $(MINGW_BINS)
+ rm -f *.o $(CYGWIN_BINS) $(MINGW_BINS) path-testsuite.cc testsuite.exe
realclean: clean
rm -f Makefile config.cache
Index: path.cc
===
RCS file: /cvs/src/src/winsup/utils/path.cc,v
retrieving revision 1.12
diff -u -p -r1.12 path.cc
--- path.cc 4 Jun 2007 01:57:16 - 1.12
+++ path.cc 9 Mar 2008 03:01:17 -
@@ -1,6 +1,6 @@
/* path.cc
- Copyright 2001, 2002, 2003, 2005, 2006, 2007 Red Hat, Inc.
+ Copyright 2001, 2002, 2003, 2005, 2006, 2007, 2008 Red Hat, Inc.
This file is part of Cygwin.
@@ -22,6 +22,7 @@ details. */
#include cygwin/include/cygwin/version.h
#include cygwin/include/sys/mount.h
#include cygwin/include/mntent.h
+#include testsuite.h
/* Used when treating / and \ as equivalent. */
#define isslash(ch) \
@@ -227,16 +228,27 @@ readlink (HANDLE fh, char *path, int max
return true;
}
-static struct mnt
+typedef struct mnt
{
const char *native;
char *posix;
unsigned flags;
int issys;
- } mount_table[255];
+ } mnt_t;
+
+#ifndef TESTSUITE
+static mnt_t mount_table[255];
+#else
+# define TESTSUITE_MOUNT_TABLE
+# include testsuite.h
+# undef
Re: [patch] cygcheck.cc update for cygpath()
Christopher Faylor wrote:
It looks like yo can still unindent this by changing the == to !=, putting
the temppath under that and keeping all of the if's at the same level:
Oh, I see now what you mean.
If the if block is that small, then I think I'd prefer just one comment
at the beginning which describes what it is doing. Otherwise, I got
lost in what was happening while trying to see where the comments line
up. I don't feel really strongly about that, though, so feel free to
ignore me. I would prefer not having the nested if's though.
Otherwise, this looks good. If you make the above suggestions, feel
free to check this in.
I chopped each comment down to a single line //-style which I think
helps the clutter, and removed the nesting.
Also, there are a few tweaks to cygcheck.cc necessary as a result, see
attached. The main idea is that when normalizing a link's target in
find_app_on_path, we should temporarily set the CWD to the same dir as
the link, otherwise relative links will get normalized relative to
whatever dir cygcheck was run from.
Brian2008-03-08 Brian Dessent [EMAIL PROTECTED]
* cygcheck.cc (save_cwd_helper): New helper function for saving
the current directory.
(find_app_on_path): Use sizeof instead of hardcoded array sizes
throughout. Change into the dir of the link when normalizing
its target. Don't worry about converting slashes as cygpath ()
now handles that.
cygcheck.cc | 45 -
1 file changed, 36 insertions(+), 9 deletions(-)
Index: cygcheck.cc
===
RCS file: /cvs/src/src/winsup/utils/cygcheck.cc,v
retrieving revision 1.97
diff -u -p -r1.97 cygcheck.cc
--- cygcheck.cc 13 Jan 2008 13:41:45 - 1.97
+++ cygcheck.cc 9 Mar 2008 03:52:07 -
@@ -807,6 +807,31 @@ ls (char *f)
display_error (ls: CloseHandle());
}
+/* If s is non-NULL, save the CWD in a static buffer and set the CWD
+ to the dirname part of s. If s is NULL, restore the CWD last
+ saved. */
+static
+void save_cwd_helper (const char *s)
+{
+ static char cwdbuf[MAX_PATH + 1];
+ char dirnamebuf[MAX_PATH + 1];
+
+ if (s)
+{
+ GetCurrentDirectory (sizeof (cwdbuf), cwdbuf);
+
+ /* Remove the filename part from s. */
+ strncpy (dirnamebuf, s, MAX_PATH);
+ dirnamebuf[MAX_PATH] = '\0'; // just in case strlen(s) MAX_PATH
+ char *lastsep = strrchr (dirnamebuf, '\\');
+ if (lastsep)
+lastsep[1] = '\0';
+ SetCurrentDirectory (dirnamebuf);
+}
+ else
+SetCurrentDirectory (cwdbuf);
+}
+
// Find a real application on the path (possibly following symlinks)
static const char *
find_app_on_path (const char *app, bool showall = false)
@@ -822,25 +847,27 @@ find_app_on_path (const char *app, bool
if (is_symlink (fh))
{
static char tmp[4000] = ;
- char *ptr;
- if (!readlink (fh, tmp, 3999))
+ if (!readlink (fh, tmp, sizeof (tmp)-1))
display_error(readlink failed);
- ptr = cygpath (tmp, NULL);
- for (char *p = ptr; (p = strchr (p, '/')); p++)
- *p = '\\';
+
+ /* When resolving the linkname, set the CWD to the context of
+ the link, so that relative links are correctly resolved. */
+ save_cwd_helper (papp);
+ char *ptr = cygpath (tmp, NULL);
+ save_cwd_helper (NULL);
printf ( - %s\n, ptr);
if (!strchr (ptr, '\\'))
{
char *lastsep;
- strncpy (tmp, cygpath (papp, NULL), 3999);
- for (char *p = tmp; (p = strchr (p, '/')); p++)
- *p = '\\';
+ strncpy (tmp, cygpath (papp, NULL), sizeof (tmp)-1);
lastsep = strrchr (tmp, '\\');
- strncpy (lastsep+1, ptr, 3999-(lastsep-tmp));
+ strncpy (lastsep+1, ptr, (sizeof (tmp)-1) - (lastsep-tmp));
ptr = tmp;
}
if (!CloseHandle (fh))
display_error (find_app_on_path: CloseHandle());
+ /* FIXME: We leak the ptr returned by cygpath() here which is a
+ malloc()d string. */
return find_app_on_path (ptr, showall);
}
Re: [patch] utils/path.cc fixes and testsuite
Corinna Vinschen wrote:
Doesn't that install testsuite.exe at `make install' time?
Ack, how about the attached?
Brian2008-03-09 Brian Dessent [EMAIL PROTECTED]
* Makefile.in (install): Don't install the testsuite.
Index: Makefile.in
===
RCS file: /cvs/src/src/winsup/utils/Makefile.in,v
retrieving revision 1.70
diff -u -p -r1.70 Makefile.in
--- Makefile.in 9 Mar 2008 04:10:10 - 1.70
+++ Makefile.in 9 Mar 2008 08:52:06 -
@@ -157,7 +157,7 @@ realclean: clean
install: all
$(SHELL) $(updir1)/mkinstalldirs $(bindir)
- for i in $(CYGWIN_BINS) $(MINGW_BINS) ; do \
+ for i in $(CYGWIN_BINS) ${filter-out testsuite.exe,$(MINGW_BINS)} ; do \
n=`echo $$i | sed '$(program_transform_name)'`; \
$(INSTALL_PROGRAM) $$i $(bindir)/$$n; \
done
Re: [patch] cygcheck.cc update for cygpath()
Corinna Vinschen wrote: Given that Cygwin changes to support long path names, I don't really like to see new code still using MAX_PATH and Win32 Ansi functions in the utils dir. Regardless of this patch, path.cc:rel_vconcat() currently uses GetCurrentDirectory() to resolve relative paths. It would be nice if rel_vconcat() (or really, cygpath()) had an interface that let the caller supply a CWD instead, as that would bypass the whole issue of length since what this patch is doing is simply setting CWD just so that rel_vconcat() can then get it again. I thought about doing it that way but it seemed more invasive. I know that the Win32 cwd is always restricted to 259 chars. However, there *is* a way to recognize the current working directory of the parent Cygwin application. Bash as well as tcsh, as well as zsh (and probbaly pdksh, too) create an environment variable $PWD. Maybe Cygwin should create $PWD for native apps if it's not already available through the parent shell. I'd suggest that the Cygwin utils first try to fetch $PWD from the environment and use that as cwd. Only if that fails, use GetCurrentDirectory. I will work on a patch that both adds an interface to allow the caller to supply a CWD as well as trying to use $PWD to get the value otherwise. Never use SetCurrentDirectory, rather convert the path to an absolute path, prepend \\?\ and call the Win32 unicode functions (CreateFileW, etc). Setting the CWD can be totally avoided I think, by the above replumbing. SYMLINK_MAX is PATH_MAX - 1 == 4095. I'm wondering if you would like to tweak the readlink functions, too. Cygwin shortcuts can now have the path name appended to the actual shortcut data. This hack was necessary to support pathnames longer than 2000 chars. See the comment and code in cygwin/path.cc, line 3139ff. Oh, I didn't know that. I'll add that to the list. Brian
Re: [patch] cygcheck.cc update for cygpath()
Corinna Vinschen wrote: I'm wondering if you would like to tweak the readlink functions, too. Cygwin shortcuts can now have the path name appended to the actual shortcut data. This hack was necessary to support pathnames longer than 2000 chars. See the comment and code in cygwin/path.cc, line 3139ff. Oh, I didn't know that. I'll add that to the list. Thanks again. I'm finally seeing light at the end of the long path name tunnel :) Actually I'm a little confused now. It seems like the code in utils/path.cc:readlink() reads the Win32 path out of shortcut symlinks but the POSIX path out of old-style symlinks -- not that it has any choice since they don't contain the win32 path. If that is the case (and assuming I'm reading the new long filename symlink code correctly) then it doesn't need any chaging since the [path too long] workaround only applies to the POSIX link target stored in the 'description' field, right? But the semantics of sometimes you get an absolute Win32 path, sometimes you get a relative POSIX path that readlink() seems to provide baffles my mind. More and more it seems that a lot of how these non-Cygwin tools successfully process paths happens seemingly out of luck. I'll have to go and research the callers of readlink() but it seems like it should be always returning the POSIX target, since that's the only sane behavior in the face of old and new styles. Brian
Re: [patch] cygcheck.cc update for cygpath()
Corinna Vinschen wrote: Now that you mention it... did you see the comment in path.cc, line 3112ff? There's a good chance that Windows shortcuts are not capable of long path names. I didn't test it so far, but it would be certainly better for readlink to use the POSIX path in the symlink either way. Check this out. I modified cygcheck to have a command line option to dump out whatever readlink() returns. $ cd /tmp $ echo fileone fileone $ ln -s fileone linkone $ ln -s filetwo linktwo# filetwo doesn't exist yet $ echo filetwo filetwo $ cat linkone fileone $ cat linktwo filetwo $ ./cygcheck -x linkone linktwo linkone - fileone linktwo - c:\tmp\filetwo $ ls -l linkone linktwo lrwxrwxrwx 1 brian None 7 Mar 9 04:56 linkone - fileone lrwxrwxrwx 1 brian None 7 Mar 9 04:56 linktwo - filetwo So, the fact that the link was dangling at the time it was created caused readlink to read it as a Win32 path -- which also causes it to now be an absolute target instead of a relative target. Apparently this is due to the fact that if the target doesn't exist at creation time the ParseDisplayName thing fails which results in a different structure for the .lnk file, which confuses readlink()'s puny little brain which only knows how to look at a fixed offset in the file, which results in it reading a different slot. Sigh. Brian
Re: [patch] cygcheck.cc update for cygpath()
Christopher Faylor wrote: I guess I misunderstood. I thought that the current working directory could be derived through some complicated combination of Nt*() calls. I could be wrong here but the way I understood it, there is no concept of a working directory at the NT level, that is something that is maintained by the Win32 layer. My question is, what does GetCurrentDirectoryW() return if the current directory is greater than the 260 limit? Does it choke or does it switch to the \.\c:\foo syntax? Brian
Re: [patch] cygcheck.cc update for cygpath()
Corinna Vinschen wrote: The problem is that the cwd is stored as UNICODE_STRING with a statically allocated buffer in the user parameter block. The MaximumLength is set to 520. SetCurrentDirectory refuses to take paths longer than that. In theory it would be possible to define a longer cwd by defining a new buffer in the cwd's UNICODE_STRING. But I never tried that. I don't really know if that's possible and what happens if you call CreateProcess or, FWIW, any Win32 file access function after doing that. Nobody keeps us from trying, but I have this gut feeling that different NT versions will show different behaviour... How does this fit in with the practice of maintaining our own Cygwin CWD state separate from the Win32 CWD so that unlink(.) and the like can succeed? Or is that precisely how we are able to support a CWD = 260? If it is the case that we can only support a CWD = 260 by faking it, i.e. not trying to sync the Win32 CWD to the actual long CWD, then it seems we are limited to two choices for how to deal with a long CWD in a non-Cygwin process: a) not supported, b) supported only through some special hook (such as cgf's idea of handle inheritance of the Cygwin CWD handle) or through relying on the shell to set PWD. Brian
Re: [patch] cygcheck.cc update for cygpath()
Corinna Vinschen wrote: Btw., you don't need to make the buffers MAX_PATH + 1. MAX_PATH is defined including the trailing NUL. Existing code shows a lot of irritation about this... Oh, I wasn't even thinking of that... the reason I used MAX_PATH + 1 was because earlier I had written + static char tmp[SYMLINK_MAX + 1]; so that the following sizes would not need to be SYMLINK_MAX - 1, + if (!readlink (fh, tmp, SYMLINK_MAX)) + strncpy (tmp, cygpath (papp, NULL), SYMLINK_MAX); + strncpy (lastsep+1, ptr, SYMLINK_MAX - (lastsep-tmp)); I.e. pure lazyness of wanting to type the least necessary. But now that you mention it, it makes more sense to have the - 1 than the + 1 form, so I'll change that. Brian
Re: [patch] cygcheck.cc update for cygpath()
Corinna Vinschen wrote: Urgh. MAX_PATH is defined with trailing 0, SYMLINK_MAX is defined without trailing 0 (like NAME_MAX). You should better change the SYMLINK_MAX stuff back, afaics... D'oh! 'Kay.
[patch] recognise when an exec()d process terminates due to unhandled exception
As we all know, Cygwin calls SetErrorMode (SEM_FAILCRITICALERRORS) to suppress those pop up GUI messageboxes from the operating system when a process encounters an unhandled exception. This has the advantage of making things more POSIX-like, and I'm sure people that run long testsuites or unattended headless servers appreciate not coming in after a long weeked to find that their server has been wedged for days waiting for someone to click on OK. But of course if you follow the user list you also know that this is a double edged sword, in that currently if a required DLL is missing there is zero indication other than an curiously arbitrary exit status code of 53 decimal, or 0x35 hex which is the low byte of 0xC135, STATUS_DLL_NOT_FOUND. Anyway, the attached patch fixes all that by adding logic to let the actual NTSTATUS logic percolate up to the waiting parent, so that it can recognise these kinds of common(ish) faults and print a friendly message -- or at least something other than silently dieing with no output. After printing the message, the NTSTATUS is discarded and the exit status code is replaced with a synthetic exit status corresponding to killed by SIGKILL as read by the wait()ing parent, which means the shell will also append Killed to that message. I tried 0x80 | SIGSEGV, corresponding to segmentation fault and core dumped but since we aren't actually generating a core file, it seemed a little weird to see the shell say that there was one generated. The point here is that the exit status that the parent (in most cases the shell) sees is totally arbitrary, so we can put whatever makes the most sense there. I just figured that the shell printing Killed most closely corresponds to the actual situation of the OS terminating the process due to an unhandled exception. There are three specific cases that I had in mind to handle with a graceful message: 1. the user is missing a DLL 2. the DLL that is found is missing symbols 3. relocs in the .rdata section In addition to catching and hopefully explaining those, it also prints a generic default case for any other exception code. Also, I'm attaching a Makefile that will create a test executable for each of the three cases above. It's totally standalone, you can type make check and it will build and run the checks. This is what the current output looks like: $ ./dll_not_found dll_not_found.exe: one or more DLLs that this program requires cannot be located by the system. Make sure the PATH is correct and re-run the setup program to install any packages indicated as necessary to satisfy library dependencies. Killed $ ./missing_import missing_import.exe: an entry point for one of more symbols could not be found during program initialization. Usually this means an incorrect or out of date version of one or more DLLs is being erroniously found on the PATH. Killed $ ./rdata_relocs rdata_relocs.exe: the process encountered an unhandled access violation fault. If this happens immediately and consistently at process startup, one likely cause is relocs in the .rdata section as a result of the runtime pseudo-reloc feature being applied to data imports in 'const' structures. Relinking with a linker script that marks the .rdata section writeable can solve this problem. Killed In all three of these cases, the current behavior is that you would get a GUI popup box from csrss.exe if you ran them from strace (since strace does not call SetErrorMode() and that setting is inherited) but you get absolutely no indication of an error if you run them from a Cygwin process... other than $? if you know to check it. I'm not 100% convinced that the change to the sigproc/pinfo stuff is totally correct and safe, as it's pretty involved code and I had to scatch my head for a while to figure out how everything interacts. So please do kick the tires. BTW, when you *do* get the GUI popup messageboxes, they are helpful in that the identify the precise DLL that's missing or the function that isn't present, etc. I was really hoping to figure out a cool way to get that info, perhaps by poking around in the TEB or PEB somewhere, but I haven't gotten that far. If anyone has any general ideas where to look for NTLDR's internal state, I'm all ears. I have a hunch it would be possible to get if we were running the exec'd process in a debugger loop and pumping WaitForDebugEvent() messages, since those can have parameters attached to exception codes. But that's a little too extreme. Brian2008-03-13 Brian Dessent [EMAIL PROTECTED] * ntdll.h: Add several missing NTSTATUS defines. * pinfo.cc (pinfo::maybe_set_exit_code_from_windows): Recognise and preserve the exit status of a child that terminated with an unhandled exception. (pinfo::exit): Make the whole NTSTATUS exit code available to the wait()ing parent when an exec()d process fails due to an unhandled exception. * pinfo.h (class _pinfo): Fix
Re: [patch] recognise when an exec()d process terminates due to unhandled exception
Eric Blake wrote: Should we also mention 'cygcheck ./dll_not_found' to find out which ones are missing? It might be a good idea. On the other hand it's kind of long already. I'm totally not married to what I've got for the wording though, consider it a very rough draft. | missing_import.exe: an entry point for one of more symbols could not be | found during program initialization. Usually this means an incorrect | or out of date version of one or more DLLs is being erroniously found | on the PATH. | Killed s/erroniously/erroneously/ Drat and s/one of more/one or more/ as well. Brian
Re: [patch] recognise when an exec()d process terminates due to unhandled exception
Brian Dessent wrote: isn't present, etc. I was really hoping to figure out a cool way to get that info, perhaps by poking around in the TEB or PEB somewhere, but I haven't gotten that far. If anyone has any general ideas where to look for NTLDR's internal state, I'm all ears. I have a hunch it would be possible to get if we were running the exec'd process in a debugger loop and pumping WaitForDebugEvent() messages, since those can have parameters attached to exception codes. But that's a little too extreme. For anyone curious, it's absolutely possible to do the above. For the C139 fault (missing procedure point entry), %ebx at the time of the fault points right at the AsciiZ name of the missing import in the .idata section, -8(%ebp) points to the import name in UNICODE, and -10(%ebp) points to the DLL name in UNICODE. For the C135 fault (the unable to locate component popup), %esi at the time of the fault points right to the missing library name in UNICODE. For the C005 fault (the LDR hits an access violation trying to fixup a reloc .rdata), %ebx points to an AsciiZ name of the symbol it was relocating and 24(%ebp) points to an AsciiZ filename of the module which that symbol is supposed to be pointing into. Now I'm sure a lot of those above offsets are just coincidental, as I haven't done much testing to see if it's reliably set as above. However it does mean that it would be relatively easy to use the debug API to step a process through its initialization and find out exactly why it's faulting. I've been working on something along those lines for cygcheck which will also give dynamic process tracing, i.e. runtime LoadLibrary stuff. Combined with enabling the LDR snaps debug output, there is a tremendous amount of debug capability hidden here. Brian
Re: [patch] recognise when an exec()d process terminates due to unhandled exception
Christopher Faylor wrote: That was going to be my first observation, actually. I'm still trying to digest the patch but it seems like it wouldn't work well with the fork retry code. The patch doesn't change any behavior though: in current Cygwin if the thing we're exec()ing returns a Win32 exit code of C135 (or whatever) then we retry creating it 5 times. With the patch, we still retry starting it 5 times but after the last one fails we recognise the situation and print a message. Brian
Re: [patch] cygcheck.cc update for cygpath()
Corinna Vinschen wrote:
Yuk. I guess it would help a lot to reproduce path.cc:check_shortcut(*)
in utils as close as possible. Afaics it doesn't use any code which
would be restricted to Cygwin, except for the call to
mount_table-conv_to_posix_path in the posixify method.
I started down that road, trying to come up with a slick method for
sharing the code so that we don't need to maintain two copies. After
poking at it for a while I came to the realization that the two are just
plumbed too differently to share code directly. In the Cygwin code, the
check if this is a valid symlink and the read its contents are kind
of lumped together as the symlink_info class provides a convenient
location to store the data. But in the cygcheck implementation we have
only is_symlink() and readlink(), with both take just a HANDLE and share
nothing, and no pflags to worry about, etc.
And of course there's the obvious that cygcheck has no Win32-POSIX
conversion capability which makes it hard to support reading reparse
points because readlink() is supposed to return the POSIX link target.
Anyway, the attached is the result of what happened when I started with
the Cygwin code and whittled it down. It fixes the bug in the
grandparent of this email where it was reading the Win32 path out of a
shortcut that didn't have an ITEMIDLIST, and it supports the new-style
shortcut where the target MAX_PATH gets stored at the end. It does
not attempt to do anything with reparse points however.
Another factor was that the file IO in symlink_info::check_shortcut was
using the native API, which I rewrote to use the plain Win32 flavor in
case we want to keep cygcheck working on 9x/ME. I don't think this will
matter if we want to make cygcheck support long paths though, since it's
just ReadFile, SetFilePointer, and GetFileInformationByHandle -- the
HANDLE is already open so this should require no change to support
WCHAR.
Brian2008-03-15 Brian Dessent [EMAIL PROTECTED]
* path.cc: Include malloc.h for alloca.
(is_symlink): Rewrite. Just read the whole file in memory rather
than by parts. Account for an ITEMIDLIST if present, as well as
the new style of Cygwin shortcut supporting targets MAX_PATH.
path.cc | 96 +++-
1 file changed, 47 insertions(+), 49 deletions(-)
Index: path.cc
===
RCS file: /cvs/src/src/winsup/utils/path.cc,v
retrieving revision 1.14
diff -u -p -r1.14 path.cc
--- path.cc 11 Mar 2008 17:20:02 - 1.14
+++ path.cc 15 Mar 2008 13:12:45 -
@@ -18,6 +18,7 @@ details. */
#include windows.h
#include stdio.h
#include stdlib.h
+#include malloc.h
#include path.h
#include cygwin/include/cygwin/version.h
#include cygwin/include/sys/mount.h
@@ -172,60 +173,57 @@ is_symlink (HANDLE fh)
bool
readlink (HANDLE fh, char *path, int maxlen)
{
- int got;
- int magic = get_word (fh, 0x0);
+ DWORD rv;
+ char *buf, *cp;
+ unsigned short len;
+ win_shortcut_hdr *file_header;
+ BY_HANDLE_FILE_INFORMATION fi;
+
+ if (!GetFileInformationByHandle (fh, fi)
+ || fi.nFileSizeHigh != 0
+ || fi.nFileSizeLow 8192)
+return false;
- if (magic == SHORTCUT_MAGIC)
-{
- int offset = get_word (fh, 0x4c);
- int slen = get_word (fh, 0x4c + offset + 2);
- if (slen = maxlen)
- {
- SetLastError (ERROR_FILENAME_EXCED_RANGE);
- return false;
- }
- if (SetFilePointer (fh, 0x4c + offset + 4, 0, FILE_BEGIN) ==
- INVALID_SET_FILE_POINTER GetLastError () != NO_ERROR)
- return false;
+ buf = (char *) alloca (fi.nFileSizeLow + 1);
+ file_header = (win_shortcut_hdr *) buf;
- if (!ReadFile (fh, path, slen, (DWORD *) got, 0))
- return false;
- else if (got slen)
- {
- SetLastError (ERROR_READ_FAULT);
- return false;
- }
- else
- path[got] = '\0';
-}
- else if (magic == SYMLINK_MAGIC)
+ if (SetFilePointer (fh, 0L, NULL, FILE_BEGIN) == INVALID_SET_FILE_POINTER
+ || !ReadFile (fh, buf, fi.nFileSizeLow, rv, NULL)
+ || rv != fi.nFileSizeLow)
+return false;
+
+ if (fi.nFileSizeLow sizeof (file_header)
+ cmp_shortcut_header (file_header))
{
- char cookie_buf[sizeof (SYMLINK_COOKIE) - 1];
-
- if (SetFilePointer (fh, 0, 0, FILE_BEGIN) == INVALID_SET_FILE_POINTER
- GetLastError () != NO_ERROR)
- return false;
-
- if (!ReadFile (fh, cookie_buf, sizeof (cookie_buf), (DWORD *) got, 0))
- return false;
- else if (got == sizeof (cookie_buf)
- memcmp (cookie_buf, SYMLINK_COOKIE, sizeof (cookie_buf)) == 0)
- {
- if (!ReadFile (fh, path, maxlen, (DWORD *) got, 0))
- return false;
- else if (got = maxlen)
- {
- SetLastError (ERROR_FILENAME_EXCED_RANGE);
- path[0] = '\0';
- return
[PATCH] normalize_posix_path and c:/foo/bar
There's a small buglet in normalize_posix_path in that it doesn't see c:/foo/bar type paths as being win32 and so it treats them as a relative path and prepends cwd. Things go downhill from there. The testcase that exposed this was insight failing to load because some of the tcl parts use win32 paths, but really a reduced testcase is simply open(c:/cygwin/bin/ls.exe, O_RDONLY) = ENOENT. Brian2008-03-16 Brian Dessent [EMAIL PROTECTED] * path.cc (normalize_posix_path): Correctly identify a win32 path beginning with a drive letter and forward slashes. path.cc |2 +- 1 file changed, 1 insertion(+), 1 deletion(-) Index: path.cc === RCS file: /cvs/src/src/winsup/cygwin/path.cc,v retrieving revision 1.479 diff -u -p -r1.479 path.cc --- path.cc 12 Mar 2008 16:07:04 - 1.479 +++ path.cc 16 Mar 2008 07:18:41 - @@ -253,7 +253,7 @@ normalize_posix_path (const char *src, c char *dst_start = dst; syscall_printf (src %s, src); - if ((isdrive (src) src[2] == '\\') || *src == '\\') + if ((isdrive (src) (src[2] == '\\' || isslash (src[2]))) || *src == '\\') goto win32_path; tail = dst;
Re: [PATCH] QueryDosDevice in handle_to_fn
Corinna Vinschen wrote: len is a const value. Checking len for being 65536 is a constant expression which always results in qddlen being 65535 so the ?: is a noop, more or less. Yeah, I realized that, and the compiler should optimize it away completely. I put it explicitly as a test in the source just for safety so that if in the future somebody changes the definition of NT_MAX_PATH or len for some reason and forgets to touch this part that things work correctly without any overflows. Did you test if QueryDosDeviceW has the same problem as QueryDosDeviceA? If not, we should use that function. No, but that's a good idea. I'll check that and if it doesn't have this quirk I'll see about moving to do the bulk of the function with WCHARs. Brian
Re: [PATCH] normalize_posix_path and c:/foo/bar
Corinna Vinschen wrote: Actually that was intended, but unfortunately the current path handling deliberately creates DOS paths with slashes (in find_exec) right now, so that doesn't work ATM. I guess what I don't understand is how it's both possible for open(c:/foo/bar.exe) to succeed and for this code to treat it as a relative posix path instead of absolute win32. Or is that the point, that forward-slash win32 paths are intended not to work? That I think is going to be quite a lot of affected code unfortunately... as I said the only real reason I went looking here is I updated my tree to current CVS and insight stopped functioning. Brian
[PATCH] better stackdumps
This patch adds the ability to see functions/symbols in the .stackdump
files generated when there's a fault. It parses the export sections of
each loaded module and finds the closest exported address for each stack
frame address. This of course won't be perfect as it will show the
wrong function if the frame is in the middle of a non-exported function,
but it's better than what we have now.
This also uses a couple of tricks to make the output more sensible. It
can see through the sigfe wrappers and print the actual functions
being wrapped. It also has a set of internal symbols that it consults
for symbols in Cygwin. This allows it to get the bottom frame correct
(_dll_crt0_1) even though that function isn't exported. If there are
any other such functions they can be easily added to the 'hints' array.
Also attached is a sample output of an invalid C program and the
resulting stackdump. Note that the frame labeled _sigbe really should
be a frame somewhere inside the main .exe. I pondered trying to extract
the sigbe's return address off the signal stack and using that for the
label but I haven't quite gotten there, since I can't think of a
reliable way to figure out the correct location on the tls stack where
the real return address is stored.
Of course the labeling works for any module/dll, not just cygwin1.dll,
but I didn't have a more elaborate testcase to demonstrate.
Brian2008-03-18 Brian Dessent [EMAIL PROTECTED]
* exceptions.cc (maybe_adjust_va_for_sigfe): New function to cope
with signal wrappers.
(prettyprint_va): New function that attempts to find a symbolic
name for a memory location by walking the export sections of all
modules.
(stackdump): Call it.
* gendef: Mark __sigfe as a global so that its address can be
used by the backtrace code.
* ntdll.h (struct _PEB_LDR_DATA): Declare.
(struct _LDR_MODULE): Declare.
(struct _PEB): Use actual LDR_DATA type for LdrData.
(RtlImageDirectoryEntryToData): Declare.
Index: exceptions.cc
===
RCS file: /cvs/src/src/winsup/cygwin/exceptions.cc,v
retrieving revision 1.319
diff -u -p -r1.319 exceptions.cc
--- exceptions.cc 12 Mar 2008 12:41:49 - 1.319
+++ exceptions.cc 19 Mar 2008 00:04:13 -
@@ -284,6 +284,158 @@ stack_info::walk ()
return 1;
}
+/* These symbols are used by the below functions to put a prettier face
+ on a stack backtrace. */
+extern u_char etext asm (etext); /* End of .text */
+extern u_char _sigfe, _sigbe;
+void dll_crt0_1 (void *);
+
+const struct {
+ DWORD va;
+ const char *label;
+} hints[] = {
+ { (DWORD) _sigbe, _sigbe },
+ { (DWORD) dll_crt0_1, _dll_crt0_1 }
+};
+
+/* Helper function to assist with backtraces. This tries to detect if
+ an entrypoint is really a sigfe wrapper and returns the actual address
+ of the function. Here's an example:
+
+ 610ab9f0 __sigfe_printf:
+ 610ab9f0: 68 40 a4 10 61 push $0x6110a440
+ 610ab9f5: e9 bf eb ff ff jmp610aa5b9 __sigfe
+
+ Suppose that we are passed 0x610ab9f0. We need to recognise the
+ push/jmp combination and return 0x6110a440 _printf instead. Note
+ that this is a relative jump. */
+static DWORD
+maybe_adjust_va_for_sigfe (DWORD va)
+{
+ if (va (DWORD) user_data-hmodule || va (DWORD) etext)
+return va;
+
+ unsigned char *x = (unsigned char *) va;
+
+ if (x[0] == 0x68 x[5] == 0xe9)
+{
+ DWORD jmprel = *(DWORD *)(x + 6);
+
+ if ((unsigned) va + 10 + (unsigned) jmprel == (unsigned) _sigfe)
+return *(DWORD *)(x + 1);
+}
+ return va;
+}
+
+/* Walk the list of modules in the current process and parse their
+ export tables in order to find the entrypoint closest to but less
+ than 'faultva'. This won't be perfect, such as when 'faultva'
+ actually resides in a non-exported function, but it is still better
+ than nothing. Note that this algorithm could be made much more
+ efficient by both sorting the export tables as well as saving the
+ result between calls. However, this implementation requires no
+ allocation of memory and minimal system calls, so it should be safe
+ in the context of an exception handler. And we're probably about to
+ terminate the process anyway, so performance is not critical. */
+static char *
+prettyprint_va (DWORD faultva)
+{
+ static char buf[256];
+
+ ULONG bestmatch_va = 0;
+
+ PLIST_ENTRY head = NtCurrentTeb()-Peb-LdrData-InMemoryOrderModuleList;
+ for (PLIST_ENTRY x = head-Flink; x != head; x = x-Flink)
+{
+ PLDR_MODULE mod = CONTAINING_RECORD (x, LDR_MODULE,
+ InMemoryOrderModuleList);
+ if ((DWORD) mod-BaseAddress faultva)
+continue;
+
+ DWORD len;
+ IMAGE_EXPORT_DIRECTORY *edata_va = (IMAGE_EXPORT_DIRECTORY *)
+ RtlImageDirectoryEntryToData
Re: [PATCH] better stackdumps
Brian Dessent wrote: Of course the labeling works for any module/dll, not just cygwin1.dll, but I didn't have a more elaborate testcase to demonstrate. Forgot to mention... The symbols are just tacked on on the right hand side there for now. I wasn't really sure how to handle that. I didn't want to remove display of the actual EIP for each frame as that could be removing useful info, but I wasn't quite sure where to put everything or how to align it... so as it is now it wraps wider than 80 chars which is probably pretty ugly on a default size terminal. Brian
Re: [PATCH] better stackdumps
Igor Peshansky wrote: Would it make sense to force a newline before the function name and to display it with a small indent? That way people who want the old-style stackdump could just feed the new one into grep -v '^ ' or something... Yes, that would be one way. That actually reminds me of another issue that I forgot to mention: glibc has a backtrace API that can be called from user-code at any time, not just at faults. At the moment we are exporting something similar called cygwin_stackdump but we don't declare it in any header. Would it be worthwhile to try to match the glibc API and export it under the same name/output format? Brian
Re: [PATCH] better stackdumps
Christopher Faylor wrote: Sorry, but I don't like this concept. This bloats the cygwin DLL for a condition that would be better served by either using gdb or generating a real coredump. I hear you, but part of the motivation for writing this was a recent thread the other week on the gdb list where the poster asked how to get symbols in a Cygwin stackdump file. I suggested the same thing, setting error_start=dumper to get a real core dump. They did, and the result was completely useless. Here is what dumper gives you for the same simple testcase: $ gdb (gdb) core a.exe.core [New process 1] [New process 0] [New process 0] #0 0x7c90eb94 in ?? () (gdb) thr apply all bt Thread 3 (process 0): #0 0x7c90eb94 in ?? () Thread 2 (process 0): #0 0x7c90eb94 in ?? () Thread 1 (process 1): #0 0x7c90eb94 in ?? () You can't even make out the names of any of the loaded modules from the core: (gdb) i tar Local core dump file: `/home/brian/testcases/backtrace/a.exe.core', file type elf32-i386. 0x0001 - 0x00011000 is load11 0x0002 - 0x00021000 is load12 0x001ff000 - 0x00233000 is load13 0x0024 - 0x00248000 is load14 0x00253000 - 0x00254000 is load15 0x0034 - 0x00346000 is load16 0x0035 - 0x00353000 is load17 0x0036 - 0x00376000 is load18 0x0038 - 0x003bd000 is load19 0x003c - 0x003c6000 is load20 0x003d - 0x003d1000 is load21 0x003e - 0x003ee000 is load22 0x003f - 0x003f1000 is load23 0x0040 - 0x00401000 is load24 0x004013d0 - 0x00405000 is load25 0x0042 - 0x00461000 is load26 0x0066c000 - 0x006a is load27 0x1867f000 - 0x1868 is load28 0x60fd - 0x60fd5000 is load29 0x60ff - 0x60ff9000 is load30 0x6100 - 0x61001000 is load31 0x61118994 - 0x61119000 is load32 0x6111a3d8 - 0x611fa000 is load33 0x611fb000 - 0x612a is load34 0x77b4 - 0x77b41000 is load35 0x77b5d60c - 0x77b62000 is load36 0x77dd - 0x77dd1000 is load37 0x77e452d9 - 0x77e6b000 is load38 0x77e7 - 0x77e71000 is load39 0x77ef3353 - 0x77ef4000 is load40 0x77efa90d - 0x77f02000 is load41 0x77fe - 0x77fe1000 is load42 0x77fed1dc - 0x77ff1000 is load43 0x7c80 - 0x7c801000 is load44 0x7c883111 - 0x7c8f5000 is load45 0x7c90 - 0x7c901000 is load46 0x7c97b6fe - 0x7c9b is load47 0x7f6f - 0x7f6f7000 is load48 0x7ffb - 0x7ffd4000 is load49 0x7ffdc000 - 0x7ffe1000 is load50 addr2line also seems to be totally unequipped to deal with separate .dbg information, as I can't get it to output a thing even though both a.exe and cygwin1.dll have full debug symbols: $ addr2line -e a.exe 0x610F74B1 ??:0 $ addr2line -e a.exe 0x610FDD3B ??:0 $ addr2line -e a.exe 0x6110A310 ??:0 $ addr2line -e a.exe 0x610AA4A8 ??:0 The situation with error_start=gdb isn't really all that better: (gdb) thr apply all bt Thread 3 (thread 4552.0x16a8): #0 0x7c901231 in ntdll!DbgUiConnectToDbg () from /winxp/system32/ntdll.dll #1 0x7c9507a8 in ntdll!KiIntSystemCall () from /winxp/system32/ntdll.dll #2 0x0005 in ~cygheap_fdget (this=0x1) at /usr/src/sourceware/winsup/cygwin/times.cc:518 #3 0x in ?? () Thread 2 (thread 4552.0x132c): #0 0x7c90eb94 in ntdll!LdrAccessResource () from /winxp/system32/ntdll.dll #1 0x7c90e288 in ntdll!ZwReadFile () from /winxp/system32/ntdll.dll #2 0x7c801875 in ReadFile () from /winxp/system32/kernel32.dll #3 0x0754 in ?? () at /usr/src/sourceware/winsup/cygwin/dtable.h:33 #4 0x in ?? () Thread 1 (thread 4552.0x18b0): #0 0x7c90eb94 in ntdll!LdrAccessResource () from /winxp/system32/ntdll.dll #1 0x7c90e21f in ntdll!ZwQueryVirtualMemory () from /winxp/system32/ntdll.dll #2 0x7c937b93 in ntdll!RtlUpcaseUnicodeChar () from /winxp/system32/ntdll.dll #3 0x in ?? () #4 0x61027c20 in sigpacket::process () at /usr/src/sourceware/winsup/cygwin/exceptions.cc:1444 #5 0x7c93783a in ntdll!LdrFindCreateProcessManifest () from /winxp/system32/ntdll.dll #6 0x61027c20 in sigpacket::process () at /usr/src/sourceware/winsup/cygwin/exceptions.cc:1444 #7 0x7c90eafa in ntdll!LdrDisableThreadCalloutsForDll () from /winxp/system32/ntdll.dll #8 0x in ?? () #0 0x7c901231 in ntdll!DbgUiConnectToDbg () from /winxp/system32/ntdll.dll None of this has anything to do with the actual call chain that triggered the fault which was printf-fputc-strlen. Yes, you usually have to continue to get the fault re-triggered, but for some reason when I type continue in this simple testcase gdb just hangs completely. Even if the user gets this far they will still need to have debug symbols installed for cygwin1.dll which in of itself is a whole other task that most users cringe at taking on. On contrast, the
addr2line [ Was: better stackdumps ]
Corinna Vinschen wrote: Is it a big problem to fix addr2line to deal with .dbg files? I like your idea to add names to the stackdump especially because of addr2line's brokenness. But, actually, if addr2line would work with .dbg files, there would be no reason to add this to the stackdump file. I absolutely agree that addr2line and/or dumper and/or gdb should be fixed, regardless of this patch. I never meant to imply an either/or situation, and in fact I have debugged addr2line and here are the reasons it's broken: Firstly it's got nothing to do with .gnu_debuglink separate debug file, that part works just fine. And secondly addr2line only loads the debug information for the module that you supply with -e, meaning that if you give -e a.exe it will look at symbols for a.exe, but it doesn't know that a.exe is dynamically linked to cygwin1.dll and it won't try to load symbols for cygwin1.dll. This means to use it you need to know beforehand which module the address is in, which right there makes it kind of a pain to use for DLLs, and to me it rather dilutes the argument that you can just postprocess a stackdump file with it since you need more information than what's there. The next problem is that addr2line first tries to read STABS, and if that fails it falls back to DWARF-2. I always build Cygwin and most other things with DWARF-2 debug symbols, mainly to make sure they work but really aren't we eventually hoping to get rid of STABS? Anyway, this exposed another problem in that even if you build all of Newlib and Cygwin with -gdwarf-2 or -ggdb3, you still get a handful STABS symbols which are hardcoded in various assembler files: mktemp.cc:20: asm (.stabs \ msg \,30,0,0,0\n\t \ mktemp.cc:21: .stabs \_ #symbol \,1,0,0,0\n); This is used to insert a linktime warning for using mktemp(). sigfe.s:3: .stabs _sigfe:F(0,1),36,0,0,__sigfe sigfe.s:44: .stabs _sigbe:F(0,1),36,0,0,__sigbe sigfe.s:70: .stabs sigreturn:F(0,1),36,0,0,_sigreturn sigfe.s:108:.stabs sigdelayed:F(0,1),36,0,0,_sigdelayed This becomes a problem in that when bfd tries to find an address in the debug data it sees these minimal STABS and considers them a match -- even though they are mostly irrelevant, they are present and since it's only got an address to go by it doesn't know that there is a much better match in the DWARF-2 data. It just sees that it has gotten a (bad) match, so it doesn't bother looking in the DWARF-2 data. And since those hand-coded .stabs above only give symbol name locations, not line number information, that means that regardless of what you ask addr2line it's going to return nothing because it only cares about line number info. I see two potential fixes here, the first being that Cygwin could be adapted to not hardcode .stabs but rather detect whether it's being built with DWARF-2 or STABS and use the appropriate kind. The other fix is to teach BFD to try DWARF-2 first before STABS. The attached patch does this, for the purposes of illustration -- I don't really claim this is correct. Once that is applied, here is the result of running the patched addr2line on the addresses in the stackdump of this testcase: $ for F in 610F74B1 610FDD3B 6110A310 610AA4A8 61006094; do /build/combined/binutils/.libs/addr2line.exe -e /bin/cygwin1.dll -f 0x$F; done ?? ??:0 _vfprintf_r /usr/src/sourceware/newlib/libc/stdio/vfprintf.c:1197 printf /usr/src/sourceware/newlib/libc/stdio/printf.c:55 ?? ??:0 _Z10dll_crt0_1Pv /usr/src/sourceware/winsup/cygwin/dcrt0.cc:930 It now gets 3 out of 5 correct. It got tripped up on _sigbe because again addr2line only cares about line number info, not general address information, and while there is information for the location of _sigbe, they don't contain line number info: (gdb) i ad _sigbe Symbol _sigbe is at 0x610aa4a8 in a file compiled without debugging. For the top frame (strlen), addr2line could not print anything because while there is location information, there is no line number information: (gdb) i li *0x610F74B1 No line number information available for address 0x610f74b1 strlen+17 This is due to the fact that strlen is implemented in newlib as libc/machine/i386/strlen.S which is a straight assembler version, and hence no line number debug records. *** To summarize thus far: 1. addr2line can be made to work again by one of a) dictating the use of STABS (boo!), b) modifying Cygwin to not emit hardcoded .stabs directives directly, c) modifying BFD to prefer DWARF-2 to STABS when reading COFF files. 2. addr2line requires the user to know beforehand which DLL a symbol is in, because it can't resolve runtime dependencies. 3. addr2line only cares about line number debug records, which means it will be incapable of representing many symbols. 4. As an implication of 3), addr2line is totally useless on DLLs/EXEs without debug information available. I think point number 4 is worth repeating: we as developers take for granted having debug
Re: addr2line [ Was: better stackdumps ]
Brian Dessent wrote: I think I see what's going on here though, the Cygwin fault handler took the first chance exception and wrote the stackdump file, and only then passed it on to the debugger, so that by the time gdb got notice of the fault the stack was all fubar. This could be the reason why dumper is not working too. I thought there was a IsBeingDebugged() check in the Silly me, this is good old set cygwin-exceptions defaulting to off... of course gdb was ignoring the fault and letting Cygwin handle it. With it set to on everything works as expected, and the issue of why the process state that dumper records is so trashed is unrelated. Brian
[PATCH] stackdump rev2
Brian Dessent wrote:
Yes, it means there is one frame that says sigbe instead of the actual
return location somewhere else. I don't think that's impossible to fix
either: the fault handler gets the context of the faulting thread so it
can look up its tls area through %fs:4 and peek at the top of the signal
stack for the value. I will investigate if this is workable.
In fact, since the fault handler runs in the context of the thread that
fauled, this turns out to be trivial. I started with an implementation
that calls GetThreadSelectorEntry to resolve the %fs:4 in the CONTEXT,
but it turned out to always be equal to simply _my_tls. This updated
version of the patch simply subsitutes _my_tls.retaddr() in place of
thestack.sf.AddrPC.Offset if it is equal to _sigbe, allowing for proper
unwinding through these frames. I tested this when the faulting thread
is the main thread as well as a user created thread and it seems to do
the right thing. Am I missing something hideous here?
Example when it's the main thread:
Exception: STATUS_ACCESS_VIOLATION at eip=610F7501
eax= ebx= ecx= edx=FEED esi= edi=FEED
ebp=0022C568 esp=0022C564
program=\\?\C:\cygwin\home\brian\testcases\backtrace\a.exe, pid 7720, thread
main
cs=001B ds=0023 es=0023 fs=003B gs= ss=0023
Stack trace:
Frame Function Args
0022C568 610F7501 (FEED, 0022C676, 00402008, 0001)
cygwin1.dll!_strlen+0x11
0022CC98 610FDD8B (0022D008, 6111C668, 00402000, 0022CCC8)
cygwin1.dll!_fputc+0x34EB
0022CCB8 6110A360 (00402000, FEED, 0009, 00401065)
cygwin1.dll!_printf+0x30
0022CCE8 00401084 (0001, 61290908, 00680098, ) a.exe+0x84
0022CD98 61006094 (, 0022CDD0, 61005430, 0022CDD0)
cygwin1.dll!_dll_crt0_1+0xC64
End of stack trace
Example when it's a user created thread:
Exception: STATUS_ACCESS_VIOLATION at eip=610F7501
eax= ebx= ecx= edx=FEED esi= edi=FEED
ebp=1886C5E8 esp=1886C5E4
program=\\?\C:\cygwin\home\brian\testcases\backtrace\tc2.exe, pid 8108, thread
unknown (0x1BA4)
cs=001B ds=0023 es=0023 fs=003B gs= ss=0023
Stack trace:
Frame Function Args
1886C5E8 610F7501 (FEED, 1886C6F6, 0040200F, 0001)
cygwin1.dll!_strlen+0x11
1886CD18 610FDD8B (1886D008, 6111C668, 00402005, 1886CD48)
cygwin1.dll!_fputc+0x34EB
1886CD38 6110A360 (00402005, FEED, 1886CD58, 611004C0)
cygwin1.dll!_printf+0x30
1886CD58 0040106C (00402012, 000A, 1886CD78, 0040109E) tc2.exe+0x6C
1886CD68 00401085 (00402017, 1886CE64, 1886CDB8, 610C85AB) tc2.exe+0x85
1886CD78 0040109E (, , 611FD6B0, 6100415A) tc2.exe+0x9E
1886CDB8 610C85AB (006901F0, 1886CDF0, 610C8530, 1886CDF0)
cygwin1.dll!pthread::thread_init_wrapper+0x7B
End of stack trace
As you can see I also added special-casing for the thread_init_wrapper
function that forms the bottom of the stack for user created threads.
Christopher Faylor wrote:
That's not a patch that I can approve, unfortunately.
That's okay, it was just illustrative anyway. I think I can fix this
purely in Cygwin by simply not emitting and .stabs if the effective
CFLAGS indicates DWARF-2 is desired. That's next on my plate.
Brian2008-03-20 Brian Dessent [EMAIL PROTECTED]
* exceptions.cc (maybe_adjust_va_for_sigfe): New function to cope
with signal wrappers.
(prettyprint_va): New function that attempts to find a symbolic
name for a memory location by walking the export sections of all
modules.
(stackdump): Call it. Use the signal frame return address
instead of _sigbe.
* gendef: Mark __sigfe as a global so that its address can be
used by the backtrace code.
* ntdll.h (struct _PEB_LDR_DATA): Declare.
(struct _LDR_MODULE): Declare.
(struct _PEB): Use actual LDR_DATA type for LdrData.
(RtlImageDirectoryEntryToData): Declare.
Index: exceptions.cc
===
RCS file: /cvs/src/src/winsup/cygwin/exceptions.cc,v
retrieving revision 1.319
diff -u -p -r1.319 exceptions.cc
--- exceptions.cc 12 Mar 2008 12:41:49 - 1.319
+++ exceptions.cc 20 Mar 2008 21:06:07 -
@@ -284,6 +284,159 @@ stack_info::walk ()
return 1;
}
+/* These symbols are used by the below functions to put a prettier face
+ on a stack backtrace. */
+extern u_char etext asm (etext); /* End of .text */
+extern u_char thread_init_wrapper asm ([EMAIL PROTECTED]);
+extern u_char _sigfe, _sigbe;
+void dll_crt0_1 (void *);
+
+const struct {
+ DWORD va;
+ const char *label;
+} hints[] = {
+ { (DWORD) thread_init_wrapper, pthread::thread_init_wrapper },
+ { (DWORD) dll_crt0_1, _dll_crt0_1 }
+};
+
+/* Helper function to assist with backtraces. This tries to detect if
+ an entrypoint is really a sigfe wrapper and returns the actual address
+ of the function. Here's an example:
+
+ 610ab9f0 __sigfe_printf
Re: [patch] recognise when an exec()d process terminates due to unhandled exception
Christopher Faylor wrote: After poking at this a little, I think it would be better to issue a linux-like error message. In my sandbox, I now have this: bash-3.2$ ./libtest /cygdrive/s/test/libtest.exe: error while loading shared libraries: liba.dll: cannot open shared object file: No such file or directory I haven't done the work to report on missing symbols yet but I think that's a much less common error condition. Excellent. The wording isn't really that important to me. But I think what is important is that we don't allow the situation where something was unable to start and we are totally silent. That leads to confusion because people start to try to debug or blame the program being run when in fact the program never saw the light of day in the first place. It's totally baffling when it happens and you're not aware to look for it. So even if we can't give the name of the symbol in the case of a missing import, I think it's still important to say something. Brian
[PATCH] fix profiling
Long story short: some asm()s have missing volatile modifiers. The mcount() profiling hook is implemented with a short wrapper around the actual mcount function. The wrapper's purpose is to get the pc of the caller as well as the return value of the caller's frame, and pass those on as arguments to the actual mcount function. Because it's a local static function the compiler inlines all this into one function. The problem is these asm()s aren't marked volatile and so the compiler freely rearranges them and interleaves them with the prologue of the inlined function. Thus mcount gets some bogus value for the pc and ignores the data because it's not in the valid range of .text. Since this code is lifted from the BSDs I did check that this change was made there as well, e.g. http://www.openbsd.org/cgi-bin/cvsweb/src/sys/arch/i386/include/profile.h?rev=1.10content-type=text/x-cvsweb-markup. Unfortuantely there seems to also be some bitrot in the gprof side, as the codepath to read BSD style gmon.out files is also broken. I've posted a separate patch to the binutils list. With both these fixes, gprof again works with Cygwin. Brian2008-08-04 Brian Dessent [EMAIL PROTECTED] * config/i386/profile.h (mcount): Mark asms volatile. Index: config/i386/profile.h === RCS file: /cvs/src/src/winsup/cygwin/config/i386/profile.h,v retrieving revision 1.1.1.1 diff -u -p -r1.1.1.1 profile.h --- config/i386/profile.h 17 Feb 2000 19:38:31 - 1.1.1.1 +++ config/i386/profile.h 5 Aug 2008 05:02:25 - @@ -48,11 +48,11 @@ mcount() \ * \ * selfpc = pc pushed by mcount call\ */ \ - __asm(movl 4(%%ebp),%0 : =r (selfpc)); \ + __asm __volatile (movl 4(%%ebp),%0 : =r (selfpc)); \ /* \ * frompcindex = pc pushed by call into self. \ */ \ - __asm(movl (%%ebp),%0;movl 4(%0),%0 : =r (frompcindex));\ + __asm __volatile (movl (%%ebp),%0;movl 4(%0),%0 : =r (frompcindex));\ _mcount(frompcindex, selfpc); \ }
Re: [PATCH] fix profiling
Christopher Faylor wrote: Please use your best judgement about the +r/=r thing given Dave's comments. I think Dave's right, because when I compile with +r I get a may be used uninitialized warning, so I'll just leave it as it is. Both patches are now committed. I wonder how long this was broken... Brian
