public inbox for cygwin@cygwin.com
 help / color / mirror / Atom feed
* Seg Fault in strftime
@ 2015-07-31  0:16 Michael Enright
  2015-07-31 12:51 ` Jon TURNEY
  2015-08-01 21:47 ` Brian Inglis
  0 siblings, 2 replies; 19+ messages in thread
From: Michael Enright @ 2015-07-31  0:16 UTC (permalink / raw)
  To: cygwin

In a thread about navigating a stackdump to find out what's going
wrong, I posted the output of a GDB session as follows.

On Thu, Jul 30, 2015 at 11:48 AM, Michael Enright  wrote:
>
> (gdb) print tznam
> $3 = 0xc07a4000 <error: Cannot access memory at address 0xc07a4000>
> (gdb) list
> 1339                    tznam = _tzname[tim_p->tm_isdst > 0];
> 1340                  /* Note that in case of wcsftime this loop only works for
> 1341                     timezone abbreviations using the portable
> codeset (aka ASCII).
> 1342                     This seems to be the case, but if that ever
> changes, this
> 1343                     loop needs revisiting. */
> 1344                  size = strlen (tznam);
> 1345                  for (i = 0; i < size; i++)
> 1346                    {
> 1347                      if (count < maxsize - 1)
> 1348                        s[count++] = tznam[i];
> (gdb) print _tzname
> $4 = {0x800cfc48 "\200", <incomplete sequence \356\066>, 0x800cfc44 "PDT"}
> (gdb) print _tzname[0]
> $5 = 0x800cfc48 "\200", <incomplete sequence \356\066>
> (gdb) print _tzname[1]
> $6 = 0x800cfc44 "PDT"
>

This is a little misleading. The code at or near line 1339 looks like this:
1331 #if defined (__CYGWIN__)
1332     /* See above. */
1333      extern const char *__cygwin_gettzname (const struct tm *tmp);
1334      tznam = __cygwin_gettzname (tim_p);
1335 #elif defined (__TM_ZONE)
1336      tznam = tim_p->__TM_ZONE;
1337 #endif
1338      if (!tznam)
1339        tznam = _tzname[tim_p->tm_isdst > 0];

The tznam is set from the tmzone member and when this happens that
member is garbage. This member is garbage POSSIBLY because of a
configuration option in libmozjs. The calling code is in prmjtime.cpp
fills in a struct tm from Spidermonkey's own broken-down time
structure, 'a', and then if the configuration enables, it makes
*another* struct tm with more fields filled in in order to get
a.tm_zone's proper value. My guess is that the path is not enabled but
the bits delivered to me do not disclose whether this righteous code
path is enabled. __cygwin_gettzname is evidently compiled to expect
the tm_zone member to exist because GDB shows it does exist.

So did any aspect of this change recently? The application and library
were getting along okay before I did cygwin updates. The last time I
had tried to run this code was early June, at which time I was running
it dozens of times a day.

Also it appears that the tm_zone member is an extension. I haven't
been able to find POSIX guidance about how applications are supposed
use struct tm in compliance in the presence of implementation-defined
fields. POSIX example code shows a usage that does access the 'at
least' fields. The language allows for implementation-defined fields.
No mechanism is provided within POSIX to allow an application to
discover additional fields and take care of them. It seems to me that
an application can then assume that when it provides a struct tm as
input, filling in the time and date reasonably, it is always
sufficient to fill in the 'at least' fields and the implementation is
the one who has to assume that the rest of the fields might not be
filled in.

--
Problem reports:       http://cygwin.com/problems.html
FAQ:                   http://cygwin.com/faq/
Documentation:         http://cygwin.com/docs.html
Unsubscribe info:      http://cygwin.com/ml/#unsubscribe-simple

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: Seg Fault in strftime
  2015-07-31  0:16 Seg Fault in strftime Michael Enright
@ 2015-07-31 12:51 ` Jon TURNEY
  2015-07-31 19:50   ` Michael Enright
  2015-08-01  0:47   ` Michael Enright
  2015-08-01 21:47 ` Brian Inglis
  1 sibling, 2 replies; 19+ messages in thread
From: Jon TURNEY @ 2015-07-31 12:51 UTC (permalink / raw)
  To: cygwin; +Cc: mike

On 31/07/2015 01:16, Michael Enright wrote:
> The tznam is set from the tmzone member and when this happens that
> member is garbage. This member is garbage POSSIBLY because of a
> configuration option in libmozjs. The calling code is in prmjtime.cpp
> fills in a struct tm from Spidermonkey's own broken-down time
> structure, 'a', and then if the configuration enables, it makes
> *another* struct tm with more fields filled in in order to get
> a.tm_zone's proper value. My guess is that the path is not enabled but
> the bits delivered to me do not disclose whether this righteous code
> path is enabled. __cygwin_gettzname is evidently compiled to expect
> the tm_zone member to exist because GDB shows it does exist.

Thanks for this investigation and analysis.

> So did any aspect of this change recently? The application and library
> were getting along okay before I did cygwin updates. The last time I
> had tried to run this code was early June, at which time I was running
> it dozens of times a day.

[1] looks like a highly relevant change and [2] is the associated discussion

It would be very helpful if you could tweak the testcase there and 
produce one which reproduces your problem.

[1] 
https://sourceware.org/git/gitweb.cgi?p=newlib-cygwin.git;a=commitdiff;h=75d5f68aabf62c42884ff935f888b12bbcd00001
[2] https://sourceware.org/ml/newlib/2015/msg00321.html

> Also it appears that the tm_zone member is an extension. I haven't
> been able to find POSIX guidance about how applications are supposed
> use struct tm in compliance in the presence of implementation-defined
> fields. POSIX example code shows a usage that does access the 'at
> least' fields. The language allows for implementation-defined fields.
> No mechanism is provided within POSIX to allow an application to
> discover additional fields and take care of them. It seems to me that
> an application can then assume that when it provides a struct tm as
> input, filling in the time and date reasonably, it is always
> sufficient to fill in the 'at least' fields and the implementation is
> the one who has to assume that the rest of the fields might not be
> filled in.

Yeah, this seems a bit of an under-specified area.


--
Problem reports:       http://cygwin.com/problems.html
FAQ:                   http://cygwin.com/faq/
Documentation:         http://cygwin.com/docs.html
Unsubscribe info:      http://cygwin.com/ml/#unsubscribe-simple

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: Seg Fault in strftime
  2015-07-31 12:51 ` Jon TURNEY
@ 2015-07-31 19:50   ` Michael Enright
  2015-07-31 23:43     ` Michael Enright
  2015-08-01  0:47   ` Michael Enright
  1 sibling, 1 reply; 19+ messages in thread
From: Michael Enright @ 2015-07-31 19:50 UTC (permalink / raw)
  To: cygwin

On Fri, Jul 31, 2015 at 5:51 AM, Jon TURNEY wrote:
> On 31/07/2015 01:16, Michael Enright wrote:
>>
> It would be very helpful if you could tweak the testcase there and produce
> one which reproduces your problem.
>
> [1]
> https://sourceware.org/git/gitweb.cgi?p=newlib-cygwin.git;a=commitdiff;h=75d5f68aabf62c42884ff935f888b12bbcd00001
> [2] https://sourceware.org/ml/newlib/2015/msg00321.html
>
I have not found it simple to reproduce the symptoms in the
intepreter, but I have found that if I do
struct tm manually_initialized;
manually_initialized.tm_year = 2000;
strftime(buf, buf_size, "%Z", &manually_initialized);
The printed time zone is garbage, which is an indication of danger.
Unlike my complex case, the tzname twins are not corrupted by this
exercise.

If I do
struct tm zeroed = {0};
manually_initialized.tm_year = 2000;
strftime(buf, buf_size, "%Z", &zero_initialized);
The printed time zone is PST (TZ happens to be "America/Los_Angeles")

Aside: I am not super-certain of the zero-initialized semantics with
respect to the language. The idiom I used is defined in C++ but the
program is C.

--
Problem reports:       http://cygwin.com/problems.html
FAQ:                   http://cygwin.com/faq/
Documentation:         http://cygwin.com/docs.html
Unsubscribe info:      http://cygwin.com/ml/#unsubscribe-simple

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: Seg Fault in strftime
  2015-07-31 19:50   ` Michael Enright
@ 2015-07-31 23:43     ` Michael Enright
  0 siblings, 0 replies; 19+ messages in thread
From: Michael Enright @ 2015-07-31 23:43 UTC (permalink / raw)
  To: cygwin

On Fri, Jul 31, 2015 at 12:50 PM, Michael Enright  wrote:
> If I do
> struct tm zeroed = {0};
> manually_initialized.tm_year = 2000;
> strftime(buf, buf_size, "%Z", &zero_initialized);
> The printed time zone is PST (TZ happens to be "America/Los_Angeles")
>

Apologies, I actually did this consistently but the report above is wrong.
> struct tm zeroed = {0};
> zeroed.tm_year = 2000;
> strftime(buf, buf_size, "%Z", &zeroed);

--
Problem reports:       http://cygwin.com/problems.html
FAQ:                   http://cygwin.com/faq/
Documentation:         http://cygwin.com/docs.html
Unsubscribe info:      http://cygwin.com/ml/#unsubscribe-simple

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: Seg Fault in strftime
  2015-07-31 12:51 ` Jon TURNEY
  2015-07-31 19:50   ` Michael Enright
@ 2015-08-01  0:47   ` Michael Enright
  1 sibling, 0 replies; 19+ messages in thread
From: Michael Enright @ 2015-08-01  0:47 UTC (permalink / raw)
  To: cygwin

On Fri, Jul 31, 2015 at 5:51 AM, Jon TURNEY wrote:
> It would be very helpful if you could tweak the testcase there and produce
> one which reproduces your problem.
>
> [1]
> https://sourceware.org/git/gitweb.cgi?p=newlib-cygwin.git;a=commitdiff;h=75d5f68aabf62c42884ff935f888b12bbcd00001
> [2] https://sourceware.org/ml/newlib/2015/msg00321.html
>
I took one more shot at reproduction and I think the problem is that
if code does a member-by-member initialization based on the fields
defined by POSIX, it is likely that tm_zone won't be initialized and
it could end up with a really bad value. Then strftime is likely to
dereference it.

You can improve the likelihood of a crash by filling a struct tm with
0x54, like I did, but random circumstances could also effectively
cause the same thing.

We could implore the local Cygwin maintainer of mozjs to make sure
that the code I mentioned earlier in that library zeros the struct tm,
but there is only defensive programming recommending that, not a
specification. And there could be other libraries or applications
already tripping over this but not yet spending time on investigating
it.

If I took the time to think it through I think the additional logic
for handling a NULL tm_zone is not necessarily the cause of the
regression I'm facing, because the code I'm using through mozjs was
not setting that field to NULL in the first place.

I'm going to be away from the machines where I have this code at hand
for the next two weeks, reading email but not equipped to do anything
complicated about it.

--
Problem reports:       http://cygwin.com/problems.html
FAQ:                   http://cygwin.com/faq/
Documentation:         http://cygwin.com/docs.html
Unsubscribe info:      http://cygwin.com/ml/#unsubscribe-simple

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: Seg Fault in strftime
  2015-07-31  0:16 Seg Fault in strftime Michael Enright
  2015-07-31 12:51 ` Jon TURNEY
@ 2015-08-01 21:47 ` Brian Inglis
  2015-08-02  1:47   ` Michael Enright
  1 sibling, 1 reply; 19+ messages in thread
From: Brian Inglis @ 2015-08-01 21:47 UTC (permalink / raw)
  To: cygwin

Michael Enright <mike <at> kmcardiff.com> writes:
> The tznam is set from the tmzone member and when this happens that
> member is garbage. This member is garbage POSSIBLY because of a
> configuration option in libmozjs. The calling code is in prmjtime.cpp
> fills in a struct tm from Spidermonkey's own broken-down time
> structure, 'a', and then if the configuration enables, it makes
> *another* struct tm with more fields filled in in order to get
> a.tm_zone's proper value. My guess is that the path is not enabled but
> the bits delivered to me do not disclose whether this righteous code
> path is enabled. __cygwin_gettzname is evidently compiled to expect
> the tm_zone member to exist because GDB shows it does exist.
> 
> So did any aspect of this change recently? The application and library
> were getting along okay before I did cygwin updates. The last time I
> had tried to run this code was early June, at which time I was running
> it dozens of times a day.
> 
> Also it appears that the tm_zone member is an extension. I haven't
> been able to find POSIX guidance about how applications are supposed
> use struct tm in compliance in the presence of implementation-defined
> fields. POSIX example code shows a usage that does access the 'at
> least' fields. The language allows for implementation-defined fields.
> No mechanism is provided within POSIX to allow an application to
> discover additional fields and take care of them. It seems to me that
> an application can then assume that when it provides a struct tm as
> input, filling in the time and date reasonably, it is always
> sufficient to fill in the 'at least' fields and the implementation is
> the one who has to assume that the rest of the fields might not be
> filled in.

Two problems I have encountered in the past with manually constructed struct tm:
- failing to set struct tm.tm_isdst member to -1, or any negative value, so
that mktime(3) will determine whether DST is in effect, and set the struct
tm.tzname array from the tzdb
- failing to call mktime(3) for each struct tm variable to normalize the
struct tm members, determine if DST is in effect if struct tm.tm_isdst
member is -1, and set the struct tm.tzname array from the tzdb.
Check back in the code to see if struct tm.tm_isdst is set and to what
value, and if mktime(3) is called on each struct tm after it is filled. 

- failing to call mktime()


--
Problem reports:       http://cygwin.com/problems.html
FAQ:                   http://cygwin.com/faq/
Documentation:         http://cygwin.com/docs.html
Unsubscribe info:      http://cygwin.com/ml/#unsubscribe-simple

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: Seg Fault in strftime
  2015-08-01 21:47 ` Brian Inglis
@ 2015-08-02  1:47   ` Michael Enright
  2015-08-03  3:37     ` Brian Inglis
  2015-08-03  8:36     ` Corinna Vinschen
  0 siblings, 2 replies; 19+ messages in thread
From: Michael Enright @ 2015-08-02  1:47 UTC (permalink / raw)
  To: cygwin

Brian,
In reference to your comments below I found this link to a repo of
SpiderMonkey source code.

http://hg.mozilla.org/releases/mozilla-1.9.1/file/920bcf17a9e1/js/src/prmjtime.cpp
And the function that calls strftime specifically:
http://hg.mozilla.org/releases/mozilla-1.9.1/file/920bcf17a9e1/js/src/prmjtime.cpp#l586

On Sat, Aug 1, 2015 at 2:47 PM, Brian Inglis  wrote:
>
> Two problems I have encountered in the past with manually constructed struct tm:
> - failing to set struct tm.tm_isdst member to -1, or any negative value, so
> that mktime(3) will determine whether DST is in effect, and set the struct
> tm.tzname array from the tzdb

The code calls strftime after setting tm_isdst from its own struct's
corresponding flag.

> - failing to call mktime(3) for each struct tm variable to normalize the
> struct tm members, determine if DST is in effect if struct tm.tm_isdst
> member is -1, and set the struct tm.tzname array from the tzdb.
> Check back in the code to see if struct tm.tm_isdst is set and to what
> value, and if mktime(3) is called on each struct tm after it is filled.

The code doesn't call mktime at all.
>
> - failing to call mktime()
>

See above.

There is a section of the code that I believe is meant to be
configured in but it is not. This code calls localtime_r with a time_t
of zero and copies the resulting tm_gmtoff and tm_zone into the struct
tm that the routine will call strftime on. This code starts at line
621, http://hg.mozilla.org/releases/mozilla-1.9.1/file/920bcf17a9e1/js/src/prmjtime.cpp#l621
to jump to that line.

The things you advocate doing are super-responsible things to do. I
have a huge investment in using this particular library and now I'm
twisting in the wind because someone else appears not to have done all
the super responsible things they should have done.

I have found there is tons of code out there manually filling in
struct tm's and then filing bugs in glibc (not just newlib problem)
when things go wrong. And then without even the courtesy of a citation
of a spec these bugs are resolved WONTFIX because these upstreams
believe they have the right to insist that struct tm's should NEVER
manually be filled in and why would you do it anyway. I think the
minimum struct members specified on POSIX should be considered the API
to any function that reads struct tm, not because POSIX says so but
because it is the way to keep machines from getting pwned through
crash bugs.

I originally did my project with a bespoke build of SpiderMonkey
because libmozjs didn't exist on any platform (it also can be found
for Debian which is convenient for me and my some-day customers) but
maintaining a library when it's available prebuilt is not supposed to
be the better option.

--
Problem reports:       http://cygwin.com/problems.html
FAQ:                   http://cygwin.com/faq/
Documentation:         http://cygwin.com/docs.html
Unsubscribe info:      http://cygwin.com/ml/#unsubscribe-simple

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: Seg Fault in strftime
  2015-08-02  1:47   ` Michael Enright
@ 2015-08-03  3:37     ` Brian Inglis
  2015-08-03  8:36     ` Corinna Vinschen
  1 sibling, 0 replies; 19+ messages in thread
From: Brian Inglis @ 2015-08-03  3:37 UTC (permalink / raw)
  To: cygwin

Michael Enright <mike <at> kmcardiff.com> writes:

> 
> Brian,
> In reference to your comments below I found this link to a repo of
> SpiderMonkey source code.
> 
>
http://hg.mozilla.org/releases/mozilla-1.9.1/file/920bcf17a9e1/js/src/prmjtime.cpp
> And the function that calls strftime specifically:
>
http://hg.mozilla.org/releases/mozilla-1.9.1/file/920bcf17a9e1/js/src/prmjtime.cpp#l586
> 
> On Sat, Aug 1, 2015 at 2:47 PM, Brian Inglis  wrote:
> >
> > Two problems I have encountered in the past with manually constructed
struct tm:
> > - failing to set struct tm.tm_isdst member to -1, or any negative value, so
> > that mktime(3) will determine whether DST is in effect, and set the struct
> > tm.tzname array from the tzdb
> 
> The code calls strftime after setting tm_isdst from its own struct's
> corresponding flag.
> 
> > - failing to call mktime(3) for each struct tm variable to normalize the
> > struct tm members, determine if DST is in effect if struct tm.tm_isdst
> > member is -1, and set the struct tm.tzname array from the tzdb.
> > Check back in the code to see if struct tm.tm_isdst is set and to what
> > value, and if mktime(3) is called on each struct tm after it is filled.
> 
> The code doesn't call mktime at all.

> There is a section of the code that I believe is meant to be
> configured in but it is not. This code calls localtime_r with a time_t
> of zero and copies the resulting tm_gmtoff and tm_zone into the struct
> tm that the routine will call strftime on. This code starts at line
> 621,
http://hg.mozilla.org/releases/mozilla-1.9.1/file/920bcf17a9e1/js/src/prmjtime.cpp#l621
> to jump to that line.
> 
> The things you advocate doing are super-responsible things to do. I
> have a huge investment in using this particular library and now I'm
> twisting in the wind because someone else appears not to have done all
> the super responsible things they should have done.
> 
> I have found there is tons of code out there manually filling in
> struct tm's and then filing bugs in glibc (not just newlib problem)
> when things go wrong. And then without even the courtesy of a citation
> of a spec these bugs are resolved WONTFIX because these upstreams
> believe they have the right to insist that struct tm's should NEVER
> manually be filled in and why would you do it anyway. I think the
> minimum struct members specified on POSIX should be considered the API
> to any function that reads struct tm, not because POSIX says so but
> because it is the way to keep machines from getting pwned through
> crash bugs.

Looks like the function attempts to duplicate the normalization of
mktime(3), but may not deal correctly with all of the edge cases, 
or handle other settings required to properly deal with time zones;  
and testing may not do all the regressions applied to most library functions. 

I would replace the fiddly code with a call to mktime(3), so all the
library's struct tm requirements are met; then there may be a case for 
filing a bug against the library mktime() or downstream functions if results
are not as expected. 



--
Problem reports:       http://cygwin.com/problems.html
FAQ:                   http://cygwin.com/faq/
Documentation:         http://cygwin.com/docs.html
Unsubscribe info:      http://cygwin.com/ml/#unsubscribe-simple

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: Seg Fault in strftime
  2015-08-02  1:47   ` Michael Enright
  2015-08-03  3:37     ` Brian Inglis
@ 2015-08-03  8:36     ` Corinna Vinschen
  2015-08-03 10:54       ` Michael Enright
  1 sibling, 1 reply; 19+ messages in thread
From: Corinna Vinschen @ 2015-08-03  8:36 UTC (permalink / raw)
  To: cygwin

[-- Attachment #1: Type: text/plain, Size: 3615 bytes --]

On Aug  1 18:47, Michael Enright wrote:
> On Sat, Aug 1, 2015 at 2:47 PM, Brian Inglis  wrote:
> > Two problems I have encountered in the past with manually constructed struct tm:
> > - failing to set struct tm.tm_isdst member to -1, or any negative value, so
> > that mktime(3) will determine whether DST is in effect, and set the struct
> > tm.tzname array from the tzdb
> 
> The code calls strftime after setting tm_isdst from its own struct's
> corresponding flag.
> 
> > - failing to call mktime(3) for each struct tm variable to normalize the
> > struct tm members, determine if DST is in effect if struct tm.tm_isdst
> > member is -1, and set the struct tm.tzname array from the tzdb.
> > Check back in the code to see if struct tm.tm_isdst is set and to what
> > value, and if mktime(3) is called on each struct tm after it is filled.
> 
> The code doesn't call mktime at all.
> >
> > - failing to call mktime()
> >
> 
> See above.
> 
> There is a section of the code that I believe is meant to be
> configured in but it is not. This code calls localtime_r with a time_t
> of zero and copies the resulting tm_gmtoff and tm_zone into the struct
> tm that the routine will call strftime on. This code starts at line
> 621, http://hg.mozilla.org/releases/mozilla-1.9.1/file/920bcf17a9e1/js/src/prmjtime.cpp#l621
> to jump to that line.
> 
> The things you advocate doing are super-responsible things to do. I
> have a huge investment in using this particular library and now I'm
> twisting in the wind because someone else appears not to have done all
> the super responsible things they should have done.
> 
> I have found there is tons of code out there manually filling in
> struct tm's and then filing bugs in glibc (not just newlib problem)
> when things go wrong. And then without even the courtesy of a citation
> of a spec these bugs are resolved WONTFIX because these upstreams
> believe they have the right to insist that struct tm's should NEVER
> manually be filled in and why would you do it anyway. I think the
> minimum struct members specified on POSIX should be considered the API
> to any function that reads struct tm, not because POSIX says so but
> because it is the way to keep machines from getting pwned through
> crash bugs.

The core thingy in POSIX is "The <time.h> header shall declare the tm
structure, which shall include at least the following members:"
                               ^^^^^^^^

A conforming application does not use such a structure which isn't
*at least* initialized to all 0 (memset).

The Cygwin implementation is basically equivalent here to the Glibc
implementation.  For the reason why doing that is the right thing,
see https://sourceware.org/ml/newlib/2015/msg00008.html

If your executable has been built prior to releasing this new code,
Cygwin won't require tm_zone and tm_gmtoff anyway.  However, for later
built executables it will, and then there's no way around the crash
if tm_zone is uninitialized.  If it's NULL, you'll get the current
timezone.  But if it's not NULL it's suppsoed to be a pointer to
a valid string.  How is a library supposed to know that the pointer
value is just garbage?

Therefore, if you (or the code you have to maintain) wants to fill out
struct tm manually, fine, just make sure to memset it beforehand.  In
that case you can be sure that both, glibc and newlib/Cygwin, will
not crash.


Corinna

-- 
Corinna Vinschen                  Please, send mails regarding Cygwin to
Cygwin Maintainer                 cygwin AT cygwin DOT com
Red Hat

[-- Attachment #2: Type: application/pgp-signature, Size: 819 bytes --]

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: Seg Fault in strftime
  2015-08-03  8:36     ` Corinna Vinschen
@ 2015-08-03 10:54       ` Michael Enright
  2015-08-03 13:42         ` Corinna Vinschen
  0 siblings, 1 reply; 19+ messages in thread
From: Michael Enright @ 2015-08-03 10:54 UTC (permalink / raw)
  To: cygwin

On Mon, Aug 3, 2015 at 1:36 AM, Corinna Vinschen wrote:
>
> The core thingy in POSIX is "The <time.h> header shall declare the tm
> structure, which shall include at least the following members:"
>                                ^^^^^^^^

I saw this language myself.

>
> A conforming application does not use such a structure which isn't
> *at least* initialized to all 0 (memset).
>

I did NOT see any language that said anything about doing that. In any
case, the code I'm using is in another Cygwin package, libmozjs185.


> If your executable has been built prior to releasing this new code,
> Cygwin won't require tm_zone and tm_gmtoff anyway.

I have no idea how to interpret "has been build prior" in this case.

>  However, for later
> built executables it will, and then there's no way around the crash
> if tm_zone is uninitialized.  If it's NULL, you'll get the current
> timezone.  But if it's not NULL it's suppsoed to be a pointer to
> a valid string.  How is a library supposed to know that the pointer
> value is just garbage?

In the case where the spec does not say anything except the members
shall include at least so-and-so, the library probably ought not to
assume that it gets its implementation-defined members defined as
inputs.

In the case where the POSIX spec uses "at least" language to define
visiible members AND specifically says that code layering on top of
that struct has to use memset or some other specific means to control
the unknown implementation-defined members, then of course the
implementation can assume that such things were done.

So the reason I'm twisting in the wind is because the implementation
has chosen to depend on the state of implementation-defined members,
the spec doesn't say anything about requiring applications to control
their values, and a library in Cygwin did not control their values,
and my application cannot work around it.

Also as an experiment I've tried to build v8. This has lead to the
subject of a new thread.

--
Problem reports:       http://cygwin.com/problems.html
FAQ:                   http://cygwin.com/faq/
Documentation:         http://cygwin.com/docs.html
Unsubscribe info:      http://cygwin.com/ml/#unsubscribe-simple

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: Seg Fault in strftime
  2015-08-03 10:54       ` Michael Enright
@ 2015-08-03 13:42         ` Corinna Vinschen
  2015-08-03 15:52           ` Michael Enright
  0 siblings, 1 reply; 19+ messages in thread
From: Corinna Vinschen @ 2015-08-03 13:42 UTC (permalink / raw)
  To: cygwin

[-- Attachment #1: Type: text/plain, Size: 2363 bytes --]

On Aug  3 03:54, Michael Enright wrote:
> On Mon, Aug 3, 2015 at 1:36 AM, Corinna Vinschen wrote:
> >
> > The core thingy in POSIX is "The <time.h> header shall declare the tm
> > structure, which shall include at least the following members:"
> >                                ^^^^^^^^
> 
> I saw this language myself.
> 
> >
> > A conforming application does not use such a structure which isn't
> > *at least* initialized to all 0 (memset).
> >
> 
> I did NOT see any language that said anything about doing that. In any
> case, the code I'm using is in another Cygwin package, libmozjs185.

When having more members in a structure than defined as the minimum set
by POSIX, there's no way around that.

> > If your executable has been built prior to releasing this new code,
> > Cygwin won't require tm_zone and tm_gmtoff anyway.
> 
> I have no idea how to interpret "has been build prior" in this case.

prior to the Cygwin version introducing the new members.  Same problem
as in Glibc.

> >  However, for later
> > built executables it will, and then there's no way around the crash
> > if tm_zone is uninitialized.  If it's NULL, you'll get the current
> > timezone.  But if it's not NULL it's suppsoed to be a pointer to
> > a valid string.  How is a library supposed to know that the pointer
> > value is just garbage?
> 
> In the case where the spec does not say anything except the members
> shall include at least so-and-so, the library probably ought not to
> assume that it gets its implementation-defined members defined as
> inputs.
> [...]

Hang on.  So you suggest that Glibc, Cygwin, as well as any other
implementation based on the localtime.c code from Arthur David Olson,
stop using the additional struct members?  Despite them doing the right
thing where the POSIX implementation is lacking?  Because there's one
lib you need which doesn't work as expected, rather than rebuilding it
with a matching fix?  That doesn't sound overly convincing to me.

Other than that, you didn't answer the question:  How is a library
supposed to know that the non-NULL pointer value is just garbage?

I'm firmly with the GLibc guys here...


Corinna

-- 
Corinna Vinschen                  Please, send mails regarding Cygwin to
Cygwin Maintainer                 cygwin AT cygwin DOT com
Red Hat

[-- Attachment #2: Type: application/pgp-signature, Size: 819 bytes --]

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: Seg Fault in strftime
  2015-08-03 13:42         ` Corinna Vinschen
@ 2015-08-03 15:52           ` Michael Enright
  2015-08-04  5:33             ` Michael Enright
  0 siblings, 1 reply; 19+ messages in thread
From: Michael Enright @ 2015-08-03 15:52 UTC (permalink / raw)
  To: cygwin

On Mon, Aug 3, 2015 at 6:42 AM, Corinna Vinschen  wrote:
> Hang on.  So you suggest that Glibc, Cygwin, as well as any other
> implementation based on the localtime.c code from Arthur David Olson,
> stop using the additional struct members?  Despite them doing the right
> thing where the POSIX implementation is lacking?  Because there's one
> lib you need which doesn't work as expected, rather than rebuilding it
> with a matching fix?  That doesn't sound overly convincing to me.

It's not just one library that I use that has tripped over this. My
brief research into workarounds led me to the glibc guys shrugging off
another project's report. I bet there has been plenty of time spent on
this on the application side that has not left a trail for me to find.

>
> Other than that, you didn't answer the question:  How is a library
> supposed to know that the non-NULL pointer value is just garbage?

I think that is the wrong question. The problem is when there is
nothing telling anyone anything until something crashes. There is
nothing in the official documentation of the API to inform a user that
they must prepare the struct any particular way. There are two ways
the library would not find itself crashing from structs it didn't
like: Not use undocumented members that are pointers that can be
garbage, **OR** tell everyone how to initialize the structs, through
the POSIX spec or through man pages.

>
> I'm firmly with the GLibc guys here...
>

As far as I can tell the Glibc guys did not solve anyone's problem.
I'm interested in a solution at the libmozjs level and I'm interested
in getting struct tm in line with its documentation one way or the
other.

--
Problem reports:       http://cygwin.com/problems.html
FAQ:                   http://cygwin.com/faq/
Documentation:         http://cygwin.com/docs.html
Unsubscribe info:      http://cygwin.com/ml/#unsubscribe-simple

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: Seg Fault in strftime
  2015-08-03 15:52           ` Michael Enright
@ 2015-08-04  5:33             ` Michael Enright
  2015-08-05  8:02               ` Corinna Vinschen
  0 siblings, 1 reply; 19+ messages in thread
From: Michael Enright @ 2015-08-04  5:33 UTC (permalink / raw)
  To: cygwin

On Mon, Aug 3, 2015 at 9:52 AM, Michael Enright wrote:
> I'm interested in a solution at the libmozjs level

Is there anything I can do to advance a solution in libmozjs?

--
Problem reports:       http://cygwin.com/problems.html
FAQ:                   http://cygwin.com/faq/
Documentation:         http://cygwin.com/docs.html
Unsubscribe info:      http://cygwin.com/ml/#unsubscribe-simple

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: Seg Fault in strftime
  2015-08-04  5:33             ` Michael Enright
@ 2015-08-05  8:02               ` Corinna Vinschen
  2015-08-17  3:00                 ` Michael Enright
  0 siblings, 1 reply; 19+ messages in thread
From: Corinna Vinschen @ 2015-08-05  8:02 UTC (permalink / raw)
  To: cygwin

[-- Attachment #1: Type: text/plain, Size: 618 bytes --]

On Aug  3 23:33, Michael Enright wrote:
> On Mon, Aug 3, 2015 at 9:52 AM, Michael Enright wrote:
> > I'm interested in a solution at the libmozjs level
> 
> Is there anything I can do to advance a solution in libmozjs?

You could report the problem upstream, ideally.  Since the behaviour
is not restricted to Cygwin (at least glibc and OpenBSD both use the
same way to handle tm_zone/tm_gmtoff in strftime), they should be
interested in a fix.


Corinna

-- 
Corinna Vinschen                  Please, send mails regarding Cygwin to
Cygwin Maintainer                 cygwin AT cygwin DOT com
Red Hat

[-- Attachment #2: Type: application/pgp-signature, Size: 819 bytes --]

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: Seg Fault in strftime
  2015-08-05  8:02               ` Corinna Vinschen
@ 2015-08-17  3:00                 ` Michael Enright
  2015-08-17  8:11                   ` js185 package problem (was Re: Seg Fault in strftime) Corinna Vinschen
  0 siblings, 1 reply; 19+ messages in thread
From: Michael Enright @ 2015-08-17  3:00 UTC (permalink / raw)
  To: cygwin

On Wed, Aug 5, 2015 at 1:02 AM, Corinna Vinschen  wrote:
> On Aug  3 23:33, Michael Enright wrote:
>> On Mon, Aug 3, 2015 at 9:52 AM, Michael Enright wrote:
>> > I'm interested in a solution at the libmozjs level
>>
>> Is there anything I can do to advance a solution in libmozjs?
>
> You could report the problem upstream, ideally.  Since the behaviour
> is not restricted to Cygwin (at least glibc and OpenBSD both use the
> same way to handle tm_zone/tm_gmtoff in strftime), they should be
> interested in a fix.

Looking at the upstream source it seems that they (mozilla.org) have
done something to allow their configure script to detect tm_zone's
presence. If the related configure variable HAVE_TM_ZONE_TM_GMTOFF is
defined as a result of configure's testing, then some code is enabled
that has the goal of getting those fields populated in the struct tm
that is passed to strftime. The steps are to transfer values from the
pseudo-tm struct they use to a temporary struct tm, call mktime with
that to get a time_t, pass the time_t to localtime_r, and then use the
resulting tm_zone and tm_gmtoffset values in the struct tm that they
pass to strftime. To me this all means that mozilla.org has the proper
code available and machinery to activate it.

I think the only reason there's a crash is because this mozilla.org
code is not enabled in cygwin's libmozjs185 for some reason.

I cloned the git repo that mozilla.org makes available and ran the
configure script. I was not able to build from the resulting setup,
but I was able to confirm that the HAVE_TM_ZONE_TM_GMTOFF macro is
defined. So the mozilla.org configure script does detect the members
on current Cygwin headers. Since that is the case the next step is to
look specifically at how libmozjs185 is built for distribution within
Cygwin.

Is there a possibility that the maintainer of Cygwin's library uses
hand-modified configure output to get around some problem, and that
stuff needs to be tweaked?

--
Problem reports:       http://cygwin.com/problems.html
FAQ:                   http://cygwin.com/faq/
Documentation:         http://cygwin.com/docs.html
Unsubscribe info:      http://cygwin.com/ml/#unsubscribe-simple

^ permalink raw reply	[flat|nested] 19+ messages in thread

* js185 package problem (was Re: Seg Fault in strftime)
  2015-08-17  3:00                 ` Michael Enright
@ 2015-08-17  8:11                   ` Corinna Vinschen
  2015-08-24 17:39                     ` Yaakov Selkowitz
  0 siblings, 1 reply; 19+ messages in thread
From: Corinna Vinschen @ 2015-08-17  8:11 UTC (permalink / raw)
  To: cygwin

[-- Attachment #1: Type: text/plain, Size: 2342 bytes --]

On Aug 16 20:00, Michael Enright wrote:
> On Wed, Aug 5, 2015 at 1:02 AM, Corinna Vinschen  wrote:
> > On Aug  3 23:33, Michael Enright wrote:
> >> On Mon, Aug 3, 2015 at 9:52 AM, Michael Enright wrote:
> >> > I'm interested in a solution at the libmozjs level
> >>
> >> Is there anything I can do to advance a solution in libmozjs?
> >
> > You could report the problem upstream, ideally.  Since the behaviour
> > is not restricted to Cygwin (at least glibc and OpenBSD both use the
> > same way to handle tm_zone/tm_gmtoff in strftime), they should be
> > interested in a fix.
> 
> Looking at the upstream source it seems that they (mozilla.org) have
> done something to allow their configure script to detect tm_zone's
> presence. If the related configure variable HAVE_TM_ZONE_TM_GMTOFF is
> defined as a result of configure's testing, then some code is enabled
> that has the goal of getting those fields populated in the struct tm
> that is passed to strftime. The steps are to transfer values from the
> pseudo-tm struct they use to a temporary struct tm, call mktime with
> that to get a time_t, pass the time_t to localtime_r, and then use the
> resulting tm_zone and tm_gmtoffset values in the struct tm that they
> pass to strftime. To me this all means that mozilla.org has the proper
> code available and machinery to activate it.
> 
> I think the only reason there's a crash is because this mozilla.org
> code is not enabled in cygwin's libmozjs185 for some reason.
> 
> I cloned the git repo that mozilla.org makes available and ran the
> configure script. I was not able to build from the resulting setup,
> but I was able to confirm that the HAVE_TM_ZONE_TM_GMTOFF macro is
> defined. So the mozilla.org configure script does detect the members
> on current Cygwin headers. Since that is the case the next step is to
> look specifically at how libmozjs185 is built for distribution within
> Cygwin.
> 
> Is there a possibility that the maintainer of Cygwin's library uses
> hand-modified configure output to get around some problem, and that
> stuff needs to be tweaked?

Maybe the package just needs rebuilding.  Yaakov?


Corinna

-- 
Corinna Vinschen                  Please, send mails regarding Cygwin to
Cygwin Maintainer                 cygwin AT cygwin DOT com
Red Hat

[-- Attachment #2: Type: application/pgp-signature, Size: 819 bytes --]

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: js185 package problem (was Re: Seg Fault in strftime)
  2015-08-17  8:11                   ` js185 package problem (was Re: Seg Fault in strftime) Corinna Vinschen
@ 2015-08-24 17:39                     ` Yaakov Selkowitz
  2015-08-25 17:17                       ` Michael Enright
  0 siblings, 1 reply; 19+ messages in thread
From: Yaakov Selkowitz @ 2015-08-24 17:39 UTC (permalink / raw)
  To: cygwin

On Mon, 2015-08-17 at 10:10 +0200, Corinna Vinschen wrote:
> On Aug 16 20:00, Michael Enright wrote:
> > I think the only reason there's a crash is because this mozilla.org
> > code is not enabled in cygwin's libmozjs185 for some reason.
> > 
> > I cloned the git repo that mozilla.org makes available and ran the
> > configure script. I was not able to build from the resulting setup,
> > but I was able to confirm that the HAVE_TM_ZONE_TM_GMTOFF macro is
> > defined. So the mozilla.org configure script does detect the members
> > on current Cygwin headers. Since that is the case the next step is to
> > look specifically at how libmozjs185 is built for distribution within
> > Cygwin.
> > 
> > Is there a possibility that the maintainer of Cygwin's library uses
> > hand-modified configure output to get around some problem, and that
> > stuff needs to be tweaked?
> 
> Maybe the package just needs rebuilding.  Yaakov?

I have uploaded js185-1.0.0-4.  Please let me know if that helps.

--
Yaakov



--
Problem reports:       http://cygwin.com/problems.html
FAQ:                   http://cygwin.com/faq/
Documentation:         http://cygwin.com/docs.html
Unsubscribe info:      http://cygwin.com/ml/#unsubscribe-simple

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: js185 package problem (was Re: Seg Fault in strftime)
  2015-08-24 17:39                     ` Yaakov Selkowitz
@ 2015-08-25 17:17                       ` Michael Enright
  2015-08-25 18:27                         ` Yaakov Selkowitz
  0 siblings, 1 reply; 19+ messages in thread
From: Michael Enright @ 2015-08-25 17:17 UTC (permalink / raw)
  To: cygwin

On Mon, Aug 24, 2015 at 10:39 AM, Yaakov Selkowitz  wrote:
> On Mon, 2015-08-17 at 10:10 +0200, Corinna Vinschen wrote:
>> Maybe the package just needs rebuilding.  Yaakov?
>
> I have uploaded js185-1.0.0-4.  Please let me know if that helps.
>
> --
> Yaakov
>

Good news.
The upload propagated to my favorite mirror, I tried out my
application and it worked.

Thanks, everyone.

At one point I checked the box for the source to this library, and
looked at the tarball. I saw that there were patches (minor if I
recall correctly) and I wondered if I'd be able to reproduce the build
accurately from the tarball and those patches. Is there a generic
reference for how packages are built to ship in Cygwin, or is this an
ad hoc thing with different steps for different packages?

-- 
mte

--
Problem reports:       http://cygwin.com/problems.html
FAQ:                   http://cygwin.com/faq/
Documentation:         http://cygwin.com/docs.html
Unsubscribe info:      http://cygwin.com/ml/#unsubscribe-simple

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: js185 package problem (was Re: Seg Fault in strftime)
  2015-08-25 17:17                       ` Michael Enright
@ 2015-08-25 18:27                         ` Yaakov Selkowitz
  0 siblings, 0 replies; 19+ messages in thread
From: Yaakov Selkowitz @ 2015-08-25 18:27 UTC (permalink / raw)
  To: cygwin

On Tue, 2015-08-25 at 10:17 -0700, Michael Enright wrote:
> On Mon, Aug 24, 2015 at 10:39 AM, Yaakov Selkowitz  wrote:
> > On Mon, 2015-08-17 at 10:10 +0200, Corinna Vinschen wrote:
> >> Maybe the package just needs rebuilding.  Yaakov?
> >
> > I have uploaded js185-1.0.0-4.  Please let me know if that helps.
> 
> Good news.
> The upload propagated to my favorite mirror, I tried out my
> application and it worked.

Glad to hear that.

> At one point I checked the box for the source to this library, and
> looked at the tarball. I saw that there were patches (minor if I
> recall correctly) and I wondered if I'd be able to reproduce the build
> accurately from the tarball and those patches. Is there a generic
> reference for how packages are built to ship in Cygwin, or is this an
> ad hoc thing with different steps for different packages?

Most packages now use cygport.  Install all the build requirements,
then:

cygport js185.cygport all

--
Yaakov



--
Problem reports:       http://cygwin.com/problems.html
FAQ:                   http://cygwin.com/faq/
Documentation:         http://cygwin.com/docs.html
Unsubscribe info:      http://cygwin.com/ml/#unsubscribe-simple

^ permalink raw reply	[flat|nested] 19+ messages in thread

end of thread, other threads:[~2015-08-25 18:27 UTC | newest]

Thread overview: 19+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2015-07-31  0:16 Seg Fault in strftime Michael Enright
2015-07-31 12:51 ` Jon TURNEY
2015-07-31 19:50   ` Michael Enright
2015-07-31 23:43     ` Michael Enright
2015-08-01  0:47   ` Michael Enright
2015-08-01 21:47 ` Brian Inglis
2015-08-02  1:47   ` Michael Enright
2015-08-03  3:37     ` Brian Inglis
2015-08-03  8:36     ` Corinna Vinschen
2015-08-03 10:54       ` Michael Enright
2015-08-03 13:42         ` Corinna Vinschen
2015-08-03 15:52           ` Michael Enright
2015-08-04  5:33             ` Michael Enright
2015-08-05  8:02               ` Corinna Vinschen
2015-08-17  3:00                 ` Michael Enright
2015-08-17  8:11                   ` js185 package problem (was Re: Seg Fault in strftime) Corinna Vinschen
2015-08-24 17:39                     ` Yaakov Selkowitz
2015-08-25 17:17                       ` Michael Enright
2015-08-25 18:27                         ` Yaakov Selkowitz

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).