Re: [PATCH v11 00/20] Compile-time stack validation

[Ingo Molnar <mingo@kernel.org>]

* Josh Poimboeuf <jpoimboe@redhat.com> wrote:

> On Wed, Aug 26, 2015 at 09:07:23AM +0200, Ingo Molnar wrote:
> > 
> > * Josh Poimboeuf <jpoimboe@redhat.com> wrote:
> > 
> > > BTW, since I'm planning on adding a CFI generation option to this tool, I wonder 
> > > if "stackvalidate" is a misnomer.  I can't think of anything better.  Any naming 
> > > ideas are welcome :-)
> > 
> > I'd suggest 'debuginfo' and a Git-alike set of subcommands:
> > 
> >    debuginfo help
> >    debuginfo check
> >    debuginfo check framepointers
> >    debuginfo check cfi
> > 
> > Such a command scheme keeps open various future commands, such as:
> > 
> >    debuginfo show
> >    debuginfo size
> >    debuginfo compress
> >    debuginfo strip
> > 
> > etc.
> > 
> > I also checked that there does not appear to be such a Linux tool with such a name 
> > at the moment.
> 
> My feeling is that the subcommand model wouldn't fit this tool very well.  Its 
> core functionality is to analyze code paths -- which it does in a single pass, 
> regardless of whether it's checking frame pointers, checking CFI, generating 
> CFI, or some combination.  Splitting it up into subcommands would mean having to 
> repeat the same code analysis pass multiple times unnecessarily.

Huh?

The subcommand approach is a user UI that does not limit the tool in any way: you 
are free to provide subcommands that combine more atomic functionality - similarly 
to how Git provides a 'git pull' subcommand that is a combination of 'fetch' and 
'merge' steps.

In this case it would be a simple:

   debuginfo check all

to check everything. You can also make the selection of debuginfo components to 
check a regular option, not a subcommand.

The important step is to not limit the tool to 'checking' only, if there's 
reasonable other tasks it could perform:

 - For example in the future we might want to sanity check the dwarf debuginfo 
   whether it's all well suited for kernel probing.

 - And if the tool is doing such a good job analyzing stacks, why not extend it 
   trivially to print max stack backtrace estimations?

etc. By limiting the name at inception unreasonably you make all these things less 
obvious to add.

The tool should obviously not be named 'kitchensink' just for future 
extensability, but at least don't limit it to stacks and checking only ...

> Also, since it's really focused on stack-related code path analysis, I wouldn't 
> expect it to be branching out into other debuginfo-related tasks.
> 
> As far as naming goes, "debuginfo" usually refers to DWARF metadata. But this 
> tool isn't DWARF-specific, so that would be confusing IMO.  I also agree with 
> Jiri that re-using a generic name which already has another meaning would add to 
> the confusion.
> 
> The goal of this tool is to fix stack traces, so how about "stackfix"?

Sigh, please don't limit the tool's purpose with its name ...

and choose a better name! :-) The tool does not 'fix' anything, and the normal 
case would be for it to produce no warnings. I.e. it's not a fixing, it's a 
checking tool.

So something like 'stackcheck' or 'stackinfo' - but I really think 'debuginfo' is 
better, because for more complex kernel image operations like live patching the 
kernel has to double check the untrusted debuginfo that compilers barf out ...

Frame pointers are simply a legacy debuginfo variant, one that the kernel happens 
to use as its primary debuginfo. I don't think it's confusing at all, especially 
if any dwarf logic is added ...

Thanks,

	Ingo