> -----Original Message----- > From: Patrick Donnelly [mailto:pdonnell@xxxxxxxxxx] > Sent: Tuesday, July 26, 2016 7:34 PM > To: Brad Hubbard <bhubbard@xxxxxxxxxx> > Cc: Allen Samuels <Allen.Samuels@xxxxxxxxxxx>; Jesse Williamson > <jwilliamson@xxxxxxx>; kefu chai <tchaikov@xxxxxxxxx>; John Spray > <jspray@xxxxxxxxxx>; ceph-devel@xxxxxxxxxxxxxxx > Subject: Re: static member variable in libcommon and double free > > On Tue, Jul 26, 2016 at 9:54 PM, Brad Hubbard <bhubbard@xxxxxxxxxx> > wrote: > >> I'm guessing that the source of the problem is that the AsyncMessager.cc > file is included twice, one in each library. If so, then the solution is to break > out the offending *.cc file into a separate third library that's linked in the right > combination with the others so that there's no duplication. > >> > > > > I believe the problem can also be solved with a static member function > > or a wrapper function for the static variable. I tried this and it > > appears to resolve the issue. > > Another solution is to use anonymous namespaces which will force the > symbol to be local to the translation unit. Won't this result in two instantiations of the "global" variable? That seems like a huge risk to the integrity of the operation of that class.... IMO, that's the wrong solution as it will eliminate a clear signal of a problem but would allow the language semantics to be degraded silently. I think the solution is still to properly implement the ODR rule, break this class into a separate library that's only included once in ANY build. As was mentioned, a script with objdump and nm tools ought to be able to detect this situation -- provided that it has an accurate model of what libraries are being combined in what executables. IMO, I'd just rather see us switch to static linking -- simpler to reason about... But I guess that ship sailed :( > > > It would be possible to audit use of such variables using nm or > > objdump but this might be a bit difficult to maintain? > > There was a proposal to add -Wglobal-variable to gcc about this same issue > but it didn't appear to go anywhere: > > https://gcc.gnu.org/bugzilla/show_bug.cgi?id=46097 > > > Since this is easy to reproduce it would also be simple to write a > > test to check for this specific issue along the lines of the following. > > > > $ g++ -O2 -x c++ - -Wl,--rpath=./lib -L./lib -lrados -lcephfs<<EOF int > > main(int, char**) { > > return 0; > > } > > EOF > > Ideally the shared libraries should not export any global variables. > There are numerous bugs other than double delete. For example, if a global > variable (address) is used as a sentinel in librados and then later libceph is > loaded defining a new sentinel (same type), any data structures created > relying on the address of the previous sentinel may break. [This is not purely > limited to global variables or even C++, you can also have magnificent > breakage if functions defined in a common static library with external linkage > are replaced. In that case, code which depends on variables with local linkage > may also break. Ultimately, the fix is to explicitly export only the desired > functions of some API in the shared library.] > > A test which checks for the double delete is a good start... > > -- > Patrick Donnelly ��.n��������+%������w��{.n����z��u���ܨ}���Ơz�j:+v�����w����ޙ��&�)ߡ�a����z�ޗ���ݢj��w�f
