epicsTime <=> aitTimeStamp conversions dont work on 64 bit hosts

[Jeff Hill]

epicsTime.cpp doesnt want to be dependent on gdd.h so it made a private definition of aitTimeStamp. Unfortunately, it appears that gdd was changed to use size locked types and epicsTime.cpp wasnt updated to match.

epicsTime.cpp
class epicsShareClass aitTimeStamp {
...
//private:
 unsigned long tv_sec;
 unsigned long tv_nsec;
private:
 static const unsigned epicsEpochSecPast1970; };

aitHelpers.h
typedef struct {
 aitUint32 tv_sec;
 aitUint32 tv_nsec;
} aitTimeStamp;

Original Mantis Bug: mantis-266
    http://www.aps.anl.gov/epics/mantis/view_bug_page.php?f_id=266

[Jeff Hill (johill-lanl)]

I am experimenting with this patch

cvs diff -u -wb -i -- epicsTime.cpp (in directory D:\users\hill\R3.14.dll_hell_fix\epics\base\src\libCom\osi\)
Index: epicsTime.cpp
===================================================================
RCS file: /net/phoebus/epicsmgr/cvsroot/epics/base/src/libCom/osi/epicsTime.cpp,v
retrieving revision 1.25.2.8
diff -c -u -w -b -i -r1.25.2.8 epicsTime.cpp
cvs diff: conflicting specifications of output style
--- epicsTime.cpp 21 Mar 2006 00:56:30 -0000 1.25.2.8
+++ epicsTime.cpp 22 Jun 2006 00:04:01 -0000
@@ -56,8 +56,8 @@
 //
 class aitTimeStamp {
 public:
- unsigned long tv_sec;
- unsigned long tv_nsec;
+ epicsUInt32 tv_sec;
+ epicsUInt32 tv_nsec;
 };

 static const unsigned tmStructEpochYear = 1900;

***** CVS exited normally with code 1 *****

[Jeff Hill (johill-lanl)]

I committed the above patch

[Jeff Hill (johill-lanl)]

> From: Ernest L. Williams Jr.
>
> That did not work; same results undefined timestamp.
>
> So based on Andrei's previous email where he noticed an issue in the
> <EPICS_BASE>/src/gdd area.
>
> I applied the following patch and it did not work either.
>
> [williams@dragon gdd]$ cvs diff -r HEAD aitTypes.h
> <email address hidden>'s password:
> Index: aitTypes.h
> ===================================================================
> RCS
> file: /sns/ADE/cvsroot/epics/supTop/base/3.14.8.2/src/gdd/aitTypes.h,v
> retrieving revision 1.1.1.1
> diff -r1.1.1.1 aitTypes.h
> 55,56c55,56
> < aitUint32 tv_sec;
> < aitUint32 tv_nsec;
> ---
> > unsigned long tv_sec;
> > unsigned long tv_nsec;
> [williams@dragon gdd]
>
>

[Jeff Hill (johill-lanl)]

Drats! I was convinced we had this one. Are you certain that the all of base rebuilt including excas, and that excas ran out of that copy of base.

[Jeff Hill (johill-lanl)]

> That did not work; same results undefined timestamp.

This might possibly be a clue. Which one of these situations do you see:

1) The timestamp is wacko (with a very wrong - and possibly randomly changing - date)

2) All of the private memberes in the timestamp are zero. This indicates probably that the constructor w/o arguments ran and that, formally, the timestamp is undefined.

If the time stamp fields are all zero it will have a date matching the EPICS epoch (see below).

static const unsigned epicsEpochYear = 1990;
static const unsigned epicsEpocMonth = 0; // January
static const unsigned epicsEpocDayOfTheMonth = 1; // the 1st day of the month

edited on: 2006-06-26 10:04

[Jeff Hill (johill-lanl)]

It looks like something weird with the compiler at line 78 in exScalarPV.cpp (see tagged source line below). I step into epicsTime::operator aitTimeStamp () and the variable "ts" has the aitTimeStamp POSIX epoch. Yet, when I return from the function the gddts has an epicsTime epoch.

Inside epicsTime::operator aitTimeStamp ()
(gdb) print ts
$17 = {tv_sec = 1151452219, tv_nsec = 756073000}

Just after line 78 in exScalarPV.cpp
(gdb) print gddts
$18 = {tv_sec = 520300219, tv_nsec = 756073000, static epicsEpochSecPast1970 = 631152000}

Now, I must admit that epicsTime.cc has aitTimeStamp defined in a different file (epicsTime.cpp) than does exScalarPV.cc (aitHelpers.h), but as discussed earlier these definitions should be now perfectly identical.

What the compiler seems to be doing is copying the epicsTime directly to the aitTimeStamp (it should never do that). I wonder if this would be a possible work around. I tried this here and it appears to compile and work ok with the 32 bit ms compiler.

aitTimeStamp gddts ( this->currentTime );

[Jeff Hill (johill-lanl)]

relevant code snippets

epicsTime::operator aitTimeStamp () const {
    aitTimeStamp ts;
    time_t_wrapper ansiTimeTicks;

    ansiTimeTicks = *this;
    ts.tv_sec = ansiTimeTicks.ts;
    ts.tv_nsec = this->nSec;
    return ts;
}

oid exScalarPV::scan()
{
    caStatus status;
    double radians;
    smartGDDPointer pDD;
    float newValue;
    float limit;
    int gddStatus;

    //
    // update current time (so we are not required to do
    // this every time that we write the PV which impacts
    // throughput under sunos4 because gettimeofday() is
    // slow)
    //
    this->currentTime = epicsTime::getCurrent ();

    pDD = new gddScalar ( gddAppType_value, aitEnumFloat64 );
    if ( ! pDD.valid () ) {
        return;
    }

    //
    // smart pointer class manages reference count after this point
    //
    gddStatus = pDD->unreference ();
    assert ( ! gddStatus );

    radians = ( rand () * 2.0 * myPI ) / RAND_MAX;
    if ( this->pValue.valid () ) {
        this->pValue->getConvert(newValue);
    }
    else {
        newValue = 0.0f;
    }
    newValue += (float) ( sin (radians) / 10.0 );
    limit = (float) this->info.getHopr ();
    newValue = tsMin ( newValue, limit );
    limit = (float) this->info.getLopr ();
    newValue = tsMax ( newValue, limit );
    *pDD = newValue;
    ===>> aitTimeStamp gddts = this->currentTime; <<====== compiler bug
    pDD->setTimeStamp ( & gddts );
    status = this->update ( *pDD );
    if (status!=S_casApp_success) {
        errMessage ( status, "scalar scan update failed\n" );
    }
}

[Jeff Hill (johill-lanl)]

I am suspecting something fishy with the 64 bit compiler. This is because I see in the debugger that epicsTime::operator aitTimeStamp () runs and that it works correctly. It returns an aitTimeStamp but somehow that epicsTime.cc defined aitTimeStamp doesn’t end up in the aitHelepers.h defined aitTimeStamp, but what is really pointing at the compiler is that the aitTimeStamp ends up with the fields directly from epicsTime - unconverted for epoch offset. That shouldn’t be possible. Nevertheless, we can't help but suspect that we are somehow influencing the bug to occur because we have two different definitions of aitTimeStamp, but why this is occurring isn’t clear since the two classes have identical data members.

Here was the dilemma that caused this mess:
In the past gdd was designed to not be dependent on anything in epics base. Now we think that gdd is going to go away so epicsTime can't be dependent on gdd.

Two possible solutions:
1) Perform all aitTimeStamp <=> epicsTime conversions indirectly through epicsTimeStamp (which gdd is able to convert to and from). That would be slightly slower.
2) Add epicsTime <=> aitTimeStamp conversion to aitTimeStamp, and remove all references to aitTimeStamp from epicsTime. That should be transparent at compile time, but not at DLL link time, but with the GDD stuff that is probably sufficiently transparent as GDD is less widely used. This will not be a bad idea as gdd is already dependent on epicsTimeStamp (a component from EPICS base).

So I think 2 should be done.

[Jeff Hill (johill-lanl)]

Here is the patch:

cvs diff -u -wb -i -- epicsTime.cpp epicsTime.h (in directory D:\users\hill\R3.14.dll_hell_fix\epics\base\src\libCom\osi\)
Index: epicsTime.cpp
===================================================================
RCS file: /net/phoebus/epicsmgr/cvsroot/epics/base/src/libCom/osi/epicsTime.cpp,v
retrieving revision 1.25.2.9
diff -c -u -w -b -i -r1.25.2.9 epicsTime.cpp
cvs diff: conflicting specifications of output style
--- epicsTime.cpp 22 Jun 2006 00:16:34 -0000 1.25.2.9
+++ epicsTime.cpp 28 Jun 2006 17:11:33 -0000
@@ -49,17 +49,6 @@
 static const unsigned nSecPerUSec = 1000u;
 static const unsigned secPerMin = 60u;

-//
-// force this module to include code that can convert
-// to GDD's aitTimeStamp, but dont require that it must
-// link with gdd. Therefore, gdd.h is not included here.
-//
-class aitTimeStamp {
-public:
- epicsUInt32 tv_sec;
- epicsUInt32 tv_nsec;
-};
-
 static const unsigned tmStructEpochYear = 1900;

 static const unsigned epicsEpochYear = 1990;
@@ -378,35 +367,6 @@
     this->addNanoSec (ts.tv_usec * nSecPerUSec);
 }

-//
-// operator aitTimeStamp ()
-//
-epicsTime::operator aitTimeStamp () const
-{
- aitTimeStamp ts;
- time_t_wrapper ansiTimeTicks;
-
- ansiTimeTicks = *this;
- ts.tv_sec = ansiTimeTicks.ts;
- ts.tv_nsec = this->nSec;
- return ts;
-}
-
-//
-// epicsTime (const aitTimeStamp &ts)
-//
-epicsTime::epicsTime (const aitTimeStamp &ts)
-{
- time_t_wrapper ansiTimeTicks;
-
- ansiTimeTicks.ts = ts.tv_sec;
- *this = epicsTime (ansiTimeTicks);
-
- unsigned long secAdj = ts.tv_nsec / nSecPerSec;
- unsigned long nSecAdj = ts.tv_nsec % nSecPerSec;
- *this = epicsTime (this->secPastEpoch+secAdj, this->nSec+nSecAdj);
-}
-
 // 631152000 (at posix epic) + 2272060800 (btw posix and epics epoch) +
 // 15 ( leap seconds )
 static const unsigned long epicsEpocSecsPastEpochNTP = 2903212815u;
Index: epicsTime.h
===================================================================
RCS file: /net/phoebus/epicsmgr/cvsroot/epics/base/src/libCom/osi/epicsTime.h,v
retrieving revision 1.14.2.2
diff -c -u -w -b -i -r1.14.2.2 epicsTime.h
cvs diff: conflicting specifications of output style
--- epicsTime.h 1 Nov 2005 15:54:36 -0000 1.14.2.2
+++ epicsTime.h 28 Jun 2006 17:18:39 -0000
@@ -35,8 +35,6 @@

 #ifdef __cplusplus

-class aitTimeStamp; /* GDD*/
-
 /*
  * extend ANSI C RTL "struct tm" to include nano seconds within a second
  * and a struct tm that is adjusted for the local timezone
@@ -127,11 +125,6 @@
     epicsTime ( const l_fp & );
     epicsTime & operator = ( const l_fp & );

- /* convert to and from GDDs aitTimeStamp format */
- operator aitTimeStamp () const;
- epicsTime ( const aitTimeStamp & );
- epicsTime & operator = ( const aitTimeStamp & );
-
     /* convert to and from WIN32s FILETIME (implemented only on WIN32) */
     operator struct _FILETIME () const;
     epicsTime ( const struct _FILETIME & );
@@ -310,12 +303,6 @@
     return *this;
 }

-inline epicsTime & epicsTime::operator = ( const aitTimeStamp & rhs )
-{
- *this = epicsTime ( rhs );
- return *this;
-}
-
 inline epicsTime & epicsTime::operator = ( const epicsTimeStamp & rhs )...

[Jeff Hill (johill-lanl)]

In summary, I completely removed the double definition of aitTimeStamp that was in epicsTime. This makes gdd dependent on epicsTime, but not visa versa.

What's new is that epicsTime no longer has any mention of aitTimeStamp within itself, and that aitTimeStamp now has conversion operators for epicsTime.

edited on: 2006-06-28 11:32

[Jeff Hill (johill-lanl)]

fixed in R3.14.9

[Andrew Johnson (anj)]

R3.14.9 Released.