Re: [PATCH 7/7] DWARF: add the config option

[hpa@zytor.com]

On May 8, 2017 8:38:31 PM PDT, Josh Poimboeuf <jpoimboe@redhat.com> wrote:
>On Mon, May 08, 2017 at 07:31:50PM -0700, Andy Lutomirski wrote:
>> On Mon, May 8, 2017 at 6:38 PM, Josh Poimboeuf <jpoimboe@redhat.com>
>wrote:
>> >> Also, don't you need some indication of which reg is the base from
>> >> which you find previous frame?  After all, sometimes GCC will emit
>a
>> >> frame pointer even in an otherwise frame-pointer-omitting kernel.
>> >
>> > I don't think we *need* to do that.  I believe the base reg can
>just
>> > always[*] be the stack pointer, even with frame pointers.
>> 
>> What if there are functions that use alloca or variable length arrays
>> on the stack?  Functions using AHASH_REQUEST_ON_STACK come to mind.
>
>Wow, mind blown.  This is why I added you to CC!
>
>Ok, I guess we'll need to be able to use the frame pointer as a base
>reg.  It should be easy anyway.

[Adding H.J. Lu for obvious expertise - H.J., the issue at hand is doing stack unwinding to display the call stack on a kernel failure.  Right now the standard kernel configuration is using frame pointers even on x86-64 because we had very bad experiences with fragility of .eh_frame-based unwinding.  However, on some kernel workloads the cost of frame pointers  I is quite high.]

Yes is exactly why.  I believe gcc will still use a frame pointer when the relationship between sp and the incoming stack frame is indeterminate for some reason, but I have to admit that a) I'm not 100% sure and b) there are DWARF annotations for exactly this, so even if it is true for current gcc that a frame pointer is not needed it might not be in the future.

As far as I understand, the .eh_frame section is supposed to contain the subset of the DWARF bytecode needed to do a stack unwind when an exception is thrown, whereas the .debug* sections contain the full DWARF data a debugger might want.  Thus .eh_frame is mapped into the runtime process while .debug* [usually?] is not.  .debug* can easily be 10x larger than the executable text segments.

Since C doesn't *have* exceptions, the main user of this for C code is the case of calls C++ > C > C++ or equivalent; thus the vulnerability to toolchain filters failures – the case of an exception-caused unwind in the middle of a C function might not have been extensively tested.  [H.J.: is that correct?  Or could an asynchronous event like a signal cause an unwind of the .eh_frame from an arbitrary point?  If so, how is that tested?]

In the case of the kernel, size matters in a different way, because even though it will be cache cold this data takes up unreclaimable RAM.  However, frame pointer-related code eats up not just RAM but cache.

Assembly language routines become problematic no matter what you do unless you restrict the way the assembly can be written.  Experience has shown us that hand-maintaining annotations in assembly code is doomed to failure, and in many cases it isn't even clear to even experienced programmers how to do it.  [H.J.: is the .cfi_* operations set even documented anywhere in a way that non-compiler-writers can comprehend it?] Inline assembly becomes problematic in the case of non-frame-pointer builds if the assembly changes the stack pointer.  A static tool might be able to annotate simple cases like push/pop.

I'm, ahem, highly skeptical to creating our own unwinding data format unless there is *documented, supported, and tested* way to force the compiler to *automatically* fall back to frame pointers any time there may be complexity involved, which at a very minimum includes any kind of data-dependent manipulation of the stack pointer.  Otherwise you will have to fail the kernel build when your static tool runs into instruction sequences it can't deal with, but the compiler may generate them - boom.  Worse, your tool will not even recognize the problem and you're in a worse place than when you started.


-- 
Sent from my Android device with K-9 Mail. Please excuse my brevity.