Re: debugs from an interesting CBDATA case study

On 01/07/2014 08:35 AM, Alex Rousskov wrote:

> 2) [Add cbdata debugging to] find which object stores the invalid cbdata
> pointer. Then find how that invalid pointer gets to that object.

I think it happens here:

>> #8 0x0834f190 in CommCbMemFunT (aMeth=
>> (void (Comm::TcpReceiver::*)(Comm::TcpReceiver * const, const
>> CommCbMemFunT<Comm::TcpReceiver, CommCloseCbParams>::Params &))
>> 0x834e270 <Comm::TcpReceiver::tcpConnectionClosed(CommCloseCbParams
>> const&)>, aJob=..., this=0xbffff268) at ../../../src/CommCalls.h:180

where we are passing a raw job pointer (job.get()) to CommCloseCbParams
object which then uses it as cbdata in CommCommonCbParams constructor:

> CommCbMemFunT(const CbcPointer<C> &job, Method meth): JobDialer<C>(job),
> CommDialerParamsT<Params_>(job.get()),

> CommCloseCbParams(void *aData);

> CommCommonCbParams::CommCommonCbParams(void *aData):
> data(cbdataReference(aData)), ...

Does the attached untested patch help?

FWIW, you may also use the attached test program as a simple test case.
It has the same class hierarchy as yours and illustrates how while
individual objects have different "this" pointers, they all have the
same (and correct) toCbdata() pointer, regardless of the inheritance order:

  BC: TrueCbdata=0xbfb306f8
   A=0xbfb3070c A::cbdata=0xbfb306f8 OK
   B=0xbfb306f8 B::cbdata=0xbfb306f8 OK
   C=0xbfb30700 C::cbdata=0xbfb306f8 OK
  BC=0xbfb306f8 BC::cbdata=0xbfb306f8 OK

  CB: TrueCbdata=0xbfb30714
   A=0xbfb30728 A::cbdata=0xbfb30714 OK
   B=0xbfb3071c B::cbdata=0xbfb30714 OK
   C=0xbfb30714 C::cbdata=0xbfb30714 OK
  CB=0xbfb30714 CB::cbdata=0xbfb30714 OK

The buggy code in Squid was using a raw job pointer from the left column
instead of using toCbdata() pointer from the right column.
Unfortunately, due to cbdata being passed and stored as void*, the
compiler cannot catch such bugs. There may be more of these.

HTH,

Alex.