[lttng-dev] RFC: Fix crash in dlerror()

[Mathieu Desnoyers]

(adding back lttng-dev, and CC Paul E. McKenney. He may have
some interesting insight in this compiler reordering of store
vs function call.)

----- Original Message -----
> From: "Paul Woegerer" <Paul_Woegerer at mentor.com>
> To: "Mathieu Desnoyers" <mathieu.desnoyers at efficios.com>
> Cc: "Stefan Seefeld" <Stefan_Seefeld at mentor.com>
> Sent: Thursday, February 13, 2014 10:39:30 AM
> Subject: Re: [lttng-dev] RFC: Fix crash in dlerror()
> 
> On 02/13/2014 03:47 PM, Mathieu Desnoyers wrote:
> > ----- Original Message -----
> >> From: "Mathieu Desnoyers" <mathieu.desnoyers at efficios.com>
> >> To: "Paul Woegerer" <Paul_Woegerer at mentor.com>
> >> Cc: "Stefan Seefeld" <Stefan_Seefeld at mentor.com>
> >> Sent: Thursday, February 13, 2014 9:29:56 AM
> >> Subject: Re: [lttng-dev] RFC: Fix crash in dlerror()
> >>
> >> ----- Original Message -----
> >>> From: "Mathieu Desnoyers" <mathieu.desnoyers at efficios.com>
> >>> To: "Paul Woegerer" <Paul_Woegerer at mentor.com>
> >>> Cc: "Stefan Seefeld" <Stefan_Seefeld at mentor.com>
> >>> Sent: Thursday, February 13, 2014 9:11:57 AM
> >>> Subject: Re: [lttng-dev] RFC: Fix crash in dlerror()
> >>>
> >>> ----- Original Message -----
> >>>> From: "Paul Woegerer" <Paul_Woegerer at mentor.com>
> >>>> To: "Mathieu Desnoyers" <mathieu.desnoyers at efficios.com>
> >>>> Cc: "Stefan Seefeld" <Stefan_Seefeld at mentor.com>
> >>>> Sent: Thursday, February 13, 2014 8:46:58 AM
> >>>> Subject: Re: [lttng-dev] RFC: Fix crash in dlerror()
> >>>>
> >>>> On 02/13/2014 02:17 PM, Mathieu Desnoyers wrote:
[...]
> >>>
> >>> I've been able to reproduce with
> >>>
> >>> gcc version 4.8.2 (Debian 4.8.2-14)
> >>>
> >>> and indeed adding the volatile fixes the issue. But it seems wrong that
> >>> the
> >>> compiler thinks it is within its rights to assume it can move the store
> >>> to that structure after the call to dlsym().
> >>

[...]

> > 
> > Adding a cmm_barrier() between the call to setup_static_allocator() and
> > the first dlsym() call fixes the issue too.
> > 
> > Thoughts ?
> 
> Intuitively I would have thought that the def-use analysis of the compiler
> recognizes the first def to the fields as redundant.
> 
> e.g.
> 
> def cur_alloc.calloc (1)
> def cur_alloc.calloc (2)
> use cur_alloc.calloc
> 
> so from compiler perspective (1) is redundant.

Well actually, it's more:

def cur_alloc.calloc (1)

  calls to dlsym() (should act as a compiler memory barrier)

def cur_alloc.calloc (2)

(where the use of cur_alloc.calloc is within the dlsym() call)

What seems to happen here is that gcc somehow assumes that dlsym() will never
be interested in the value of the static variable cur_alloc.calloc. But I doubt
this assumption is valid, because dlsym() could very well have access to a
function pointer leading to a function within the object containing cur_alloc,
and thus have to access cur_alloc. How can this store be optimized away without
breaking sequential consistency ?

> 
> It cannot know about the indirect recursion via dlsym. To express this to the
> compiler you have to make the fields of your struct volatile.

AFAIK, the compiler should assume that there may be a call to a function using
cur_alloc within dlsym().

> 
> BTW, for copying cur_alloc, let the compiler create the memcpy for you.

Good point!

I don't see how volatile would be required here. Maybe it fixes the issue
(so does adding a compiler barrier), but I feel uneasy just fixing it up
within lttng-ust if there is a deeper issue in gcc 4.8.

Thoughts ?

Thanks,

Mathieu


-- 
Mathieu Desnoyers
EfficiOS Inc.
http://www.efficios.com