Re: [Qemu-devel] RFC: fix for random Qemu crashes

[andrzej zaborowski]

On 17/11/2007, J. Mayer <address@hidden> wrote:
>
> On Fri, 2007-11-16 at 21:32 +0100, andrzej zaborowski wrote:
> > On 16/11/2007, Jocelyn Mayer <address@hidden> wrote:
> > >
> > > On Fri, 2007-11-16 at 15:52 +0000, Paul Brook wrote:
> > > > > Then, I choosed to replace 'inline' by 'always_inline', which is more
> > > > > invasive but have less risks of side effects. The diff is attached in
> > > > > always_inline.diff.
> > > > > The last thing that helps solve the problem is to change the inlining
> > > > > limits of gcc, at least to compile the op.o file.
> > > >
> > > > Presumably we only need one of the last two patches? It seems rather 
> > > > pointless
> > > > to have always_inline *and* change the inlining heuristics.
> > >
> > > >From the tests I made, it seems that adding always_inline helps but
> > > unfortunatelly does not solve all cases. Should check in the gcc source
> > > code why it is so...
> > >
> > > > I'm ok with using always_inline for op.o (and things it uses directly) 
> > > > as this
> > > > is required for correctness. I'm not convinced that that using 
> > > > always_inline
> > > > everywhere is such a good idea.
> > >
> > > That's exactly what I did: I changed 'inline' to 'always_inline' in
> > > headers that are included by op.c, I did not made any change in other
> > > headers.
> >
> > I think a line like
> >
> > #define inline __attribute__ (( always_inline )) inline
> >
> > in dyngen-exec.h should be
>
> As I already pointed it in the first message of the thread, this kind of
> define would expand recursivelly, which is particullary ugly, and which
> can in some cases lead to compiler warnings or errors. I already had
> this kind of problems using the linux kernel headers which preciselly
> uses this definitition.

My point here is that you can use dyngen-exec.h for the macro so that
the functions are only always_inline'd when used in op.c, not in other
files, I think that's what pbrook mean too. For example cpsr_write
from target-arm/exec.h was used in op.c as well as in vl.c. There's no
problem if it isn't inlined in vl.c, the fix should only affect op.c
which is a special case, for other files let gcc decide in the way it
was designed by gcc authors.

> But, once again, adding always_inline to functions does not completelly
> solve the problem (please read the thread !) or at least does not solves
> it with all gcc versions.

Yes, I'm not saying anything about the other part.
Regards