From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1752109AbdEHFfw (ORCPT ); Mon, 8 May 2017 01:35:52 -0400 Received: from mail-vk0-f52.google.com ([209.85.213.52]:32950 "EHLO mail-vk0-f52.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1750797AbdEHFfu (ORCPT ); Mon, 8 May 2017 01:35:50 -0400 MIME-Version: 1.0 In-Reply-To: <20170507165524.cdxfuwbd5alr7v6k@treble> References: <20170505122200.31436-1-jslaby@suse.cz> <20170505122200.31436-7-jslaby@suse.cz> <20170507165524.cdxfuwbd5alr7v6k@treble> From: Andy Lutomirski Date: Sun, 7 May 2017 22:35:28 -0700 Message-ID: Subject: Re: [PATCH 7/7] DWARF: add the config option To: Josh Poimboeuf Cc: Linus Torvalds , Jiri Slaby , Andrew Morton , live-patching@vger.kernel.org, Linux Kernel Mailing List , Thomas Gleixner , Ingo Molnar , "H. Peter Anvin" , "the arch/x86 maintainers" , Andy Lutomirski , Jiri Kosina Content-Type: text/plain; charset=UTF-8 Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Sun, May 7, 2017 at 9:55 AM, Josh Poimboeuf wrote: > On Fri, May 05, 2017 at 12:57:11PM -0700, Linus Torvalds wrote: >> On Fri, May 5, 2017 at 5:22 AM, Jiri Slaby wrote: >> > The DWARF unwinder is in place and ready. So introduce the config option >> > to allow users to enable it. It is by default off due to missing >> > assembly annotations. >> >> Who actually ends up using this? >> >> Because from the last time we had fancy unwindoers, and all the >> problems it caused for oops handling with absolutely _zero_ upsides >> ever, I do not ever again want to see fancy unwinders with complex >> state machine handling used by the oopsing code. >> >> The fact that it gets disabled for KASAN also makes me suspicious. It >> basically means that now all the accesses it does are not even >> validated. >> >> The fact that the most of the code seems to be disabled for the first >> six patches, and then just enabled in the last patch, also seems to >> mean that the series also gets no bisection coverage or testing that >> the individual patches make any sense. (ie there's a lot of code >> inside "CONFIG_DWARF_UNWIND" in the early patches but that config >> option cannot even be enabled until the last patch). >> >> We used to have nasty issues with not just missing dwarf info, but >> also actively *wrong* dwarf info. Compiler versions that generated >> subtly wrong info, because nobody actually really depended on it, and >> the people who had tested it seldom did the kinds of things we do in >> the kernel (eg inline asms etc). >> >> So I'm personally still very suspicious of these things. >> >> Last time I had huge issues with people also always blaming *anything* >> else than that unwinder. It was always "oh, somebody wrote asm without >> getting it right". Or "oh, the compiler generated bad tables, it's not >> *my* fault that now the kernel oopsing code no longer works". >> >> When I asked for more stricter debug table validation to avoid issues, >> it was always "oh, we fixed it, no worries", and then two months later >> somebody hit another issue. >> >> Put another way; the last time we did crazy stuff like this, it got >> reverted. For a damn good reason, despite some people being in denial >> about those reasons. > > Here's another possible idea that's been rattling around in my head. > It's purely theoretical at this point, so I don't know for sure that it > would work. But I haven't been able to find any major issues with it > yet. > > DWARF is great for debuggers. It helps you find all the registers on > the stack, so you can see function arguments and local variables. All > expressed in a nice compact format. > > But that's overkill for unwinders. We don't need all those registers, > and the state machine is too complicated. Unwinders basically only need > to know one thing: given an instruction address and a stack pointer, > where is the caller's stack frame? I think that, if the code were sufficiently robust, it would be handy if the unwinder displayed function arguments. DWARF can do that to a limited extent. That being said, having a simpler table format would probably cover most of the use cases.