From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1751083AbcGWMxz (ORCPT ); Sat, 23 Jul 2016 08:53:55 -0400 Received: from mx1.redhat.com ([209.132.183.28]:40340 "EHLO mx1.redhat.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1750797AbcGWMxy (ORCPT ); Sat, 23 Jul 2016 08:53:54 -0400 Date: Sat, 23 Jul 2016 07:53:50 -0500 From: Josh Poimboeuf To: Linus Torvalds Cc: Andy Lutomirski , Thomas Gleixner , Ingo Molnar , "H . Peter Anvin" , the arch/x86 maintainers , Linux Kernel Mailing List , Steven Rostedt , Brian Gerst , Kees Cook , Peter Zijlstra , Frederic Weisbecker , Byungchul Park Subject: Re: [PATCH 00/19] x86/dumpstack: rewrite x86 stack dump code Message-ID: <20160723125350.jtx7b4b6tpkztjx6@treble> References: <20160723053506.sh6exe5gkpx5rudk@treble> MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Disposition: inline In-Reply-To: User-Agent: Mutt/1.6.0.1 (2016-04-01) X-Greylist: Sender IP whitelisted, not delayed by milter-greylist-4.5.16 (mx1.redhat.com [10.5.110.25]); Sat, 23 Jul 2016 12:53:53 +0000 (UTC) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Sat, Jul 23, 2016 at 02:39:52PM +0900, Linus Torvalds wrote: > On Sat, Jul 23, 2016 at 2:35 PM, Josh Poimboeuf wrote: > > > > While doing the scanning and printing, it does call the frame pointer > > unwinder in parallel, but like before, that's *only* used to determine > > whether a found address should be printed without a question mark. If > > the unwinder goes off the rails, the scanning and printing of text > > addresses goes on, undisturbed. > > > > The frame pointer unwinder code itself is quite careful not to > > dereference anything it shouldn't (though of course I welcome any review > > comments that find otherwise). > > So this was the bug the last time around we did unwinders - the code > would dereference the unwind tables, and the tables would be > corrupted. End result: recursive oops. > > And they were corrupted not even because of memory corruption, but > simply because they contained incorrect data, due to compiler bugs and > other issues. > > I have really bad memories from that time. Several years after the > fact. It took months to finally revert the crap, because the author > continued to insist that "this was the last bug" for several passes > through that thing. > > As they say, "Once burned, twice shy". But in this case, it's more > like "Four times burned, sixteen times as shy". But that was DWARF, right? This is still just simple frame pointers. Don't think of it as a new unwinder. Think of it instead as a "gentle reshuffling of the existing code to vastly improve readability and maintenance." Yes, I would like to eventually propose a DWARF unwinder, which hopefully learns from the mistakes of previous attempts. But either way, I think this patch set stands on its own as a big improvement. -- Josh