Thread: Re: [HACKERS] Win32 WEXITSTATUS too simplistic
I did some research on this, and found a nice Win32 list of STATUS_ error values. Looking at the list, I think the non-exit() return values are much larger than just the second high bit. I am proposing the attached patch, which basically has all system() return values < 0x100 as exit() calls, and everything above that as a signal exits. I also think it is too risky to backpatch to 8.2.X. Also, should we print Win32 WTERMSIG() values as hex because they are so large? I have added that to the patch. --------------------------------------------------------------------------- ITAGAKI Takahiro wrote: > > Tom Lane <tgl@sss.pgh.pa.us> wrote: > > > server process exited with exit code -1073741819 > > from what I suspect is really the equivalent of a SIGSEGV trap, > > ie, attempted access to already-deallocated memory. My calculator > > says the above is equivalent to hex C0000005, and I say that this > > makes it pretty clear that *some* parts of Windows put flag bits into > > the process exit code. Anyone want to run down what we should really > > be using instead of the above macros? > > C0000005 equals to EXCEPTION_ACCESS_VIOLATION. The value returned by > GetExceptionCode() seems to be the exit code in unhandeled exception cases. > > AFAICS, all EXCEPTION_xxx (or STATUS_xxx) values are defined as 0xCxxxxxxx. > I think we can use the second high bit to distinguish exit by exception > from normal exits. > > #define WEXITSTATUS(w) ((int) ((w) & 0x40000000)) > #define WIFEXITED(w) ((w) & 0x40000000) == 0) > #define WIFSIGNALED(w) ((w) & 0x40000000) != 0) > #define WTERMSIG(w) (w) // or ((w) & 0x3FFFFFFF) > > However, it comes from reverse engineering of the headers of Windows. > I cannot find any official documentation. > > Regards, > --- > ITAGAKI Takahiro > NTT Open Source Software Center > > > > ---------------------------(end of broadcast)--------------------------- > TIP 3: Have you checked our extensive FAQ? > > http://www.postgresql.org/docs/faq -- Bruce Momjian bruce@momjian.us EnterpriseDB http://www.enterprisedb.com + If your life is a hard drive, Christ can be your backup. + Index: src/backend/postmaster/postmaster.c =================================================================== RCS file: /cvsroot/pgsql/src/backend/postmaster/postmaster.c,v retrieving revision 1.508 diff -c -c -r1.508 postmaster.c *** src/backend/postmaster/postmaster.c 16 Jan 2007 13:28:56 -0000 1.508 --- src/backend/postmaster/postmaster.c 22 Jan 2007 04:51:01 -0000 *************** *** 2426,2432 **** /*------ translator: %s is a noun phrase describing a child process, such as "server process" */ ! (errmsg("%s (PID %d) was terminated by signal %d", procname, pid, WTERMSIG(exitstatus)))); else ereport(lev, --- 2426,2432 ---- /*------ translator: %s is a noun phrase describing a child process, such as "server process" */ ! (errmsg("%s (PID %d) was terminated by signal "PRINTF_SIGNUM, procname, pid, WTERMSIG(exitstatus)))); else ereport(lev, Index: src/include/c.h =================================================================== RCS file: /cvsroot/pgsql/src/include/c.h,v retrieving revision 1.216 diff -c -c -r1.216 c.h *** src/include/c.h 11 Jan 2007 02:39:52 -0000 1.216 --- src/include/c.h 22 Jan 2007 04:51:02 -0000 *************** *** 788,793 **** --- 788,800 ---- #define SIGNAL_ARGS int postgres_signal_arg #endif + #ifndef WIN32 + #define PRINTF_SIGNUM "%d" + #else + /* Win32 non-exit system() return values are high, see port/win32.h */ + #define PRINTF_SIGNUM "%X" + #endif + /* * When there is no sigsetjmp, its functionality is provided by plain * setjmp. Incidentally, nothing provides setjmp's functionality in Index: src/include/port/win32.h =================================================================== RCS file: /cvsroot/pgsql/src/include/port/win32.h,v retrieving revision 1.65 diff -c -c -r1.65 win32.h *** src/include/port/win32.h 11 Jan 2007 02:42:31 -0000 1.65 --- src/include/port/win32.h 22 Jan 2007 04:51:02 -0000 *************** *** 115,130 **** /* * Signal stuff ! * WIN32 doesn't have wait(), so the return value for children ! * is simply the return value specified by the child, without ! * any additional information on whether the child terminated ! * on its own or via a signal. These macros are also used ! * to interpret the return value of system(). ! */ ! #define WEXITSTATUS(w) (w) ! #define WIFEXITED(w) (true) ! #define WIFSIGNALED(w) (false) ! #define WTERMSIG(w) (0) #define sigmask(sig) ( 1 << ((sig)-1) ) --- 115,134 ---- /* * Signal stuff ! * For WIN32, there is no wait() call so there are no wait() macros ! * to interpret the return value of system(). Instead, system() ! * return values < 0x100 are used for exit() termination, and higher ! * values are used to indicated non-exit() termination, which is ! * similar to a unix-style signal exit (think SIGSEGV == ! * STATUS_ACCESS_VIOLATION). See this URL for a list of WIN32 ! * STATUS_* values from Wine: ! * ! * http://source.winehq.org/source/include/ntstatus.h ! */ ! #define WIFEXITED(w) (((w) & 0xffffff00) == 0) ! #define WIFSIGNALED(w) (!WIFEXITED(w)) ! #define WEXITSTATUS(w) (w) ! #define WTERMSIG(w) (w) #define sigmask(sig) ( 1 << ((sig)-1) ) Index: src/port/exec.c =================================================================== RCS file: /cvsroot/pgsql/src/port/exec.c,v retrieving revision 1.44 diff -c -c -r1.44 exec.c *** src/port/exec.c 5 Jan 2007 22:20:02 -0000 1.44 --- src/port/exec.c 22 Jan 2007 04:51:03 -0000 *************** *** 582,588 **** log_error(_("child process exited with exit code %d"), WEXITSTATUS(exitstatus)); else if (WIFSIGNALED(exitstatus)) ! log_error(_("child process was terminated by signal %d"), WTERMSIG(exitstatus)); else log_error(_("child process exited with unrecognized status %d"), --- 582,588 ---- log_error(_("child process exited with exit code %d"), WEXITSTATUS(exitstatus)); else if (WIFSIGNALED(exitstatus)) ! log_error(_("child process was terminated by signal "PRINTF_SIGNUM), WTERMSIG(exitstatus)); else log_error(_("child process exited with unrecognized status %d"),
I have applied a modified version of this patch. We now print the exception value in hex, and give a URL where the exception can be looked up. --------------------------------------------------------------------------- Bruce Momjian wrote: > > I did some research on this, and found a nice Win32 list of STATUS_ > error values. Looking at the list, I think the non-exit() return values > are much larger than just the second high bit. > > I am proposing the attached patch, which basically has all system() > return values < 0x100 as exit() calls, and everything above that as a > signal exits. I also think it is too risky to backpatch to 8.2.X. > > Also, should we print Win32 WTERMSIG() values as hex because they are so > large? I have added that to the patch. > > --------------------------------------------------------------------------- > > ITAGAKI Takahiro wrote: > > > > Tom Lane <tgl@sss.pgh.pa.us> wrote: > > > > > server process exited with exit code -1073741819 > > > from what I suspect is really the equivalent of a SIGSEGV trap, > > > ie, attempted access to already-deallocated memory. My calculator > > > says the above is equivalent to hex C0000005, and I say that this > > > makes it pretty clear that *some* parts of Windows put flag bits into > > > the process exit code. Anyone want to run down what we should really > > > be using instead of the above macros? > > > > C0000005 equals to EXCEPTION_ACCESS_VIOLATION. The value returned by > > GetExceptionCode() seems to be the exit code in unhandeled exception cases. > > > > AFAICS, all EXCEPTION_xxx (or STATUS_xxx) values are defined as 0xCxxxxxxx. > > I think we can use the second high bit to distinguish exit by exception > > from normal exits. > > > > #define WEXITSTATUS(w) ((int) ((w) & 0x40000000)) > > #define WIFEXITED(w) ((w) & 0x40000000) == 0) > > #define WIFSIGNALED(w) ((w) & 0x40000000) != 0) > > #define WTERMSIG(w) (w) // or ((w) & 0x3FFFFFFF) > > > > However, it comes from reverse engineering of the headers of Windows. > > I cannot find any official documentation. > > > > Regards, > > --- > > ITAGAKI Takahiro > > NTT Open Source Software Center > > > > > > > > ---------------------------(end of broadcast)--------------------------- > > TIP 3: Have you checked our extensive FAQ? > > > > http://www.postgresql.org/docs/faq > > -- > Bruce Momjian bruce@momjian.us > EnterpriseDB http://www.enterprisedb.com > > + If your life is a hard drive, Christ can be your backup. + > > ---------------------------(end of broadcast)--------------------------- > TIP 5: don't forget to increase your free space map settings -- Bruce Momjian bruce@momjian.us EnterpriseDB http://www.enterprisedb.com + If your life is a hard drive, Christ can be your backup. + Index: src/backend/postmaster/postmaster.c =================================================================== RCS file: /cvsroot/pgsql/src/backend/postmaster/postmaster.c,v retrieving revision 1.508 diff -c -c -r1.508 postmaster.c *** src/backend/postmaster/postmaster.c 16 Jan 2007 13:28:56 -0000 1.508 --- src/backend/postmaster/postmaster.c 22 Jan 2007 18:29:01 -0000 *************** *** 2421,2426 **** --- 2421,2427 ---- (errmsg("%s (PID %d) exited with exit code %d", procname, pid, WEXITSTATUS(exitstatus)))); else if (WIFSIGNALED(exitstatus)) + #ifndef WIN32 ereport(lev, /*------ *************** *** 2428,2433 **** --- 2429,2443 ---- "server process" */ (errmsg("%s (PID %d) was terminated by signal %d", procname, pid, WTERMSIG(exitstatus)))); + #else + ereport(lev, + + /*------ + translator: %s is a noun phrase describing a child process, such as + "server process" */ + (errmsg("%s (PID %d) was terminated by exception %X\nSee http://source.winehq.org/source/include/ntstatus.hfor a description\nof the hex value.", + procname, pid, WTERMSIG(exitstatus)))); + #endif else ereport(lev, Index: src/include/port/win32.h =================================================================== RCS file: /cvsroot/pgsql/src/include/port/win32.h,v retrieving revision 1.65 diff -c -c -r1.65 win32.h *** src/include/port/win32.h 11 Jan 2007 02:42:31 -0000 1.65 --- src/include/port/win32.h 22 Jan 2007 18:29:02 -0000 *************** *** 115,130 **** /* * Signal stuff ! * WIN32 doesn't have wait(), so the return value for children ! * is simply the return value specified by the child, without ! * any additional information on whether the child terminated ! * on its own or via a signal. These macros are also used ! * to interpret the return value of system(). ! */ ! #define WEXITSTATUS(w) (w) ! #define WIFEXITED(w) (true) ! #define WIFSIGNALED(w) (false) ! #define WTERMSIG(w) (0) #define sigmask(sig) ( 1 << ((sig)-1) ) --- 115,152 ---- /* * Signal stuff ! * ! * For WIN32, there is no wait() call so there are no wait() macros ! * to interpret the return value of system(). Instead, system() ! * return values < 0x100 are used for exit() termination, and higher ! * values are used to indicated non-exit() termination, which is ! * similar to a unix-style signal exit (think SIGSEGV == ! * STATUS_ACCESS_VIOLATION). Return values are broken up into groups: ! * ! * http://msdn2.microsoft.com/en-gb/library/aa489609.aspx ! * ! * NT_SUCCESS 0 - 0x3FFFFFFF ! * NT_INFORMATION 0x40000000 - 0x7FFFFFFF ! * NT_WARNING 0x80000000 - 0xBFFFFFFF ! * NT_ERROR 0xC0000000 - 0xFFFFFFFF ! * ! * Effectively, we don't care on the severity of the return value from ! * system(), we just need to know if it was because of exit() or generated ! * by the system, and it seems values >= 0x100 are system-generated. ! * See this URL for a list of WIN32 STATUS_* values: ! * ! * Wine (URL used in our error messages) - ! * http://source.winehq.org/source/include/ntstatus.h ! * Descriptions - http://www.comp.nus.edu.sg/~wuyongzh/my_doc/ntstatus.txt ! * MS SDK - http://www.nologs.com/ntstatus.html ! * ! * Some day we might want to print descriptions for the most common ! * exceptions, rather than printing a URL. ! */ ! #define WIFEXITED(w) (((w) & 0xffffff00) == 0) ! #define WIFSIGNALED(w) (!WIFEXITED(w)) ! #define WEXITSTATUS(w) (w) ! #define WTERMSIG(w) (w) #define sigmask(sig) ( 1 << ((sig)-1) ) Index: src/port/exec.c =================================================================== RCS file: /cvsroot/pgsql/src/port/exec.c,v retrieving revision 1.44 diff -c -c -r1.44 exec.c *** src/port/exec.c 5 Jan 2007 22:20:02 -0000 1.44 --- src/port/exec.c 22 Jan 2007 18:29:03 -0000 *************** *** 582,589 **** --- 582,594 ---- log_error(_("child process exited with exit code %d"), WEXITSTATUS(exitstatus)); else if (WIFSIGNALED(exitstatus)) + #ifndef WIN32 log_error(_("child process was terminated by signal %d"), WTERMSIG(exitstatus)); + #else + log_error(_("child process was terminated by exception %X\nSee http://source.winehq.org/source/include/ntstatus.hfor a description\nof the hex value."), + WTERMSIG(exitstatus)); + #endif else log_error(_("child process exited with unrecognized status %d"), exitstatus);
Bruce Momjian wrote:
>
> I have applied a modified version of this patch. We now print the
> exception value in hex, and give a URL where the exception can be looked
> up.
Humm, wouldn't it be more appropriate to put the URL in a errhint()
instead?
> + ereport(lev,
> +
> + /*------
> + translator: %s is a noun phrase describing a child process, such as
> + "server process" */
> + (errmsg("%s (PID %d) was terminated by exception %X\nSee
http://source.winehq.org/source/include/ntstatus.hfor a description\nof the hex value.",
> + procname, pid, WTERMSIG(exitstatus))));
> + #endif
--
Alvaro Herrera http://www.CommandPrompt.com/
The PostgreSQL Company - Command Prompt, Inc.
Alvaro Herrera wrote:
> Bruce Momjian wrote:
> >
> > I have applied a modified version of this patch. We now print the
> > exception value in hex, and give a URL where the exception can be looked
> > up.
>
> Humm, wouldn't it be more appropriate to put the URL in a errhint()
> instead?
>
> > + ereport(lev,
> > +
> > + /*------
> > + translator: %s is a noun phrase describing a child process, such as
> > + "server process" */
> > + (errmsg("%s (PID %d) was terminated by exception %X\nSee
http://source.winehq.org/source/include/ntstatus.hfor a description\nof the hex value.",
> > + procname, pid, WTERMSIG(exitstatus))));
> > + #endif
Oops, forgot to mention that detail. We are using log_error() in one
case, and ereport() in another. Let me do the hint in the report case,
but I have to leave the log_error case alone because it takes only three
arguments.
--
Bruce Momjian bruce@momjian.us
EnterpriseDB http://www.enterprisedb.com
+ If your life is a hard drive, Christ can be your backup. +
Alvaro Herrera <alvherre@commandprompt.com> writes:
> Bruce Momjian wrote:
>> I have applied a modified version of this patch. We now print the
>> exception value in hex, and give a URL where the exception can be looked
>> up.
> Humm, wouldn't it be more appropriate to put the URL in a errhint()
> instead?
It should not be there at all. Do you see URLs in any of our other
error messages?
regards, tom lane
Tom Lane wrote: > Alvaro Herrera <alvherre@commandprompt.com> writes: > > Bruce Momjian wrote: > >> I have applied a modified version of this patch. We now print the > >> exception value in hex, and give a URL where the exception can be looked > >> up. > > > Humm, wouldn't it be more appropriate to put the URL in a errhint() > > instead? > > It should not be there at all. Do you see URLs in any of our other > error messages? Sure, ideally, but how else can we give information about that hex value? -- Bruce Momjian bruce@momjian.us EnterpriseDB http://www.enterprisedb.com + If your life is a hard drive, Christ can be your backup. +
Bruce Momjian <bruce@momjian.us> writes:
> Tom Lane wrote:
>> It should not be there at all. Do you see URLs in any of our other
>> error messages?
> Sure, ideally, but how else can we give information about that hex
> value?
It's not the responsibility of that error message to tell someone to
go look up the error number in Microsoft documentation. If they're
clueful enough to make any sense of the number beyond the strerror
translation we already provide, then they already know where to look.
Even if it were the responsibility of the error message to suggest this,
a URL seems far too transient.
regards, tom lane
Tom Lane wrote: > Bruce Momjian <bruce@momjian.us> writes: > > Tom Lane wrote: > >> It should not be there at all. Do you see URLs in any of our other > >> error messages? > > > Sure, ideally, but how else can we give information about that hex > > value? > > It's not the responsibility of that error message to tell someone to > go look up the error number in Microsoft documentation. If they're > clueful enough to make any sense of the number beyond the strerror > translation we already provide, then they already know where to look. Well, it took me like 25 minutes to find that list, so it isn't obvious. Search for STATUS_CARDBUS_NOT_SUPPORTED, and you get only 75 hits on Google, and our URL is #7. One idea Andrew Dunstan had was to print descriptions for the most popular values. I asked him to give it a try once I applied this patch. > Even if it were the responsibility of the error message to suggest this, > a URL seems far too transient. It is a URL to the Wine CVS repository, so I assume it will be around for a while. One thing we could do is copy that file to a URL on our web site and point error messages to that. We could put the file in our CVS and point to that too. -- Bruce Momjian bruce@momjian.us EnterpriseDB http://www.enterprisedb.com + If your life is a hard drive, Christ can be your backup. +