Re: Seg Fault in strftime

[Brian Inglis <Brian.Inglis@SystematicSw.ab.ca>]

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