Re: [Qemu-devel] [PATCH v2 6/7] scripts/dump-guest-memory.py: add vmcoreinfo

[Marc-André Lureau]

Hi

----- Original Message -----
> On 07/11/17 15:35, Marc-André Lureau wrote:
> > Hi
> > 
> > ----- Original Message -----
> >> On 07/11/17 12:04, Marc-André Lureau wrote:
> >>> Hi
> >>>
> >>> ----- Original Message -----
> >>>> On 07/06/17 12:16, Marc-André Lureau wrote:
> >>>>> Add vmcoreinfo ELF note if vmcoreinfo device is ready.
> >>>>>
> >>>>> To help the python script, add a little global vmcoreinfo_gdb
> >>>>> structure, that is populated with vmcoreinfo_gdb_update().
> >>>>>
> >>>>> Signed-off-by: Marc-André Lureau <address@hidden>
> >>
> >>>>> @@ -181,6 +183,7 @@ static void vmcoreinfo_realize(DeviceState *dev,
> >>>>> Error
> >>>>> **errp)
> >>>>>          return;
> >>>>>      }
> >>>>>
> >>>>> +    vmcoreinfo_gdb_helper = VMCOREINFO(dev);
> >>>>>      qemu_register_reset(vmcoreinfo_handle_reset, dev);
> >>>>>  }
> >>>>>
> >>>>>
> >>>>
> >>>> I guess we don't build QEMU with link-time optimization at the
> >>>> moment.
> >>>>
> >>>> With link-time optimization, I think gcc might reasonably optimize
> >>>> away the assignment to "vmcoreinfo_gdb_helper", and
> >>>> "vmcoreinfo_gdb_helper" itself. This is why I suggested "volatile":
> >>>>
> >>>> static VMCoreInfoState * volatile vmcoreinfo_gdb_helper;
> >>>>
> >>>> Do you think volatile is only superfluous, or do you actively dislike
> >>>> it for some reason?
> >>>
> >>> Yeah, I am not convinced volatile is the best way, but nor is static.
> >>>
> >>> Let's export it?
> >>
> >> Volatile guarantees that the assignment will take place according to the
> >> behavior of the "abstract C machine" described by ISO C. From ISO C99,
> >>
> >>   6.7.3 Type qualifiers
> >>
> >>   6 An object that has volatile-qualified type may be modified in ways
> >>     unknown to the implementation or have other unknown side effects.
> >>     Therefore any expression referring to such an object shall be
> >>     evaluated strictly according to the rules of the abstract machine,
> >>     as described in 5.1.2.3. Furthermore, at every sequence point the
> >>     value last stored in the object shall agree with that prescribed by
> >>     the abstract machine, except as modified by the unknown factors
> >>     mentioned previously. [116] What constitutes an access to an object
> >>     that has volatile-qualified type is implementation-defined.
> >>
> >>   Footnote 116: A volatile declaration may be used to describe an object
> >>                 corresponding to a memory-mapped input/output port or an
> >>                 object accessed by an asynchronously interrupting
> >>                 function. Actions on objects so declared shall not be
> >>                 "optimized out" by an implementation or reordered except
> >>                 as permitted by the rules for evaluating expressions.
> >>
> >> So basically if you can find the symbol somehow, it will always have the
> >> right value, due to the volatile qualifier, regardless of the
> >> optimizations the compiler did.
> >>
> >> Internal vs. external linkage ("static" vs. "extern") is a different
> >> question; it might affect whether you find the symbol at all. IME,
> >> symbols for objects with internal linkage are preserved, if: (a) you
> >> don't strip the final binary, and (b) gcc doesn't optimize the variable
> >> away.
> >>
> >> With link-time / full program optimization, gcc is entitled to optimize
> >> away variables with external linkage too, so "extern" in itself is not
> >> bulletproof. Volatile is more important.
> > 
> > Ok, you seem confident about this sort of things, I am not, I'll follow you
> > :)
> >>
> > 
> >> Given volatile, I'm sort of neutral on extern vs. static. However, if
> >> you make the variable extern, that makes abuse from other source files
> >> easier. (The variable should only be looked at with gdb.) This is also
> >> why I originally suggested to limit the scope of even the static
> >> variable to function scope -- this way abuse would completely be
> >> prevented (nothing else could access the variable even from the same
> >> translation unit), and gdb would still find the variable (IME).
> > 
> > How do you access a static inside a function? Gdb can look it up but
> > complains that it's not in current context.
> > 
> 
> According to <https://sourceware.org/gdb/onlinedocs/gdb/Variables.html>,
> the magic incantation is
> 
>   function::variable
> 

Thanks that works! I'll update the patch

> If this doesn't work (in case the realize function isn't on the stack at
> all), then I agree the variable should be made file scope.
> 

Why would it matter if realize is on the stack? It's likely not :)