Re: [PATCH v2] objtool: move libelf detection to Kconfig from Makefile

[Josh Poimboeuf]

On Tue, Jul 10, 2018 at 12:47:49PM +0900, Masahiro Yamada wrote:
> Hi.
> 
> 
> 2018-07-10 11:29 GMT+09:00 Josh Poimboeuf <jpoimboe@xxxxxxxxxx>:
> > On Tue, Jul 10, 2018 at 10:35:16AM +0900, Masahiro Yamada wrote:
> >> Currently, users are allowed to enable STACK_VALIDATION regardless
> >> of the compiler capability.  The top-level Makefile warns or breaks
> >> the build if it turns out that the host compiler cannot link libelf.
> >>
> >> Move the libelf test to Kconfig so that users can enable the feature
> >> only when the host compiler can build the objtool.  The ugly check
> >> in the Makefile will go away.
> >>
> >> Signed-off-by: Masahiro Yamada <yamada.masahiro@xxxxxxxxxxxxx>
> >> Acked-by: Josh Poimboeuf <jpoimboe@xxxxxxxxxx>
> >
> > Actually, looking at this again, I want to rescind my ack.
> >
> > This patches changes the behavior in a subtle (but important) way.
> >
> > Before, if I did "make defconfig", it would *always* choose the ORC
> > unwinder.  Then, if I didn't have libelf-devel, the build would fail and
> > it would tell me what I need to install.
> >
> > But now with this patch, if I do "make defconfig", the generated config
> > actually changes based on what I happen to have installed on my build
> > system.  If I don't have libelf-devel, then it silently chooses the
> > non-default unwinder (frame pointer).
> >
> > This is a subtle difference, but IMO it's dangerous, because it will
> > silently enable the frame pointer unwinder for the majority of new
> > systems, even though it's not the default.
> >
> > I strongly prefer the original behavior, because we really want to
> > encourage people to use the ORC unwinder, even if that means they have
> > to install a package to build it.
> >
> > Also -- in general -- I suspect that silently changing the defaults like
> > this will create a lot of bad surprises.  The output of "make defconfig"
> > should be predictable and not dependent on what RPMs I happen to have
> > installed.
> 
> 
> 
> Actually, we had similar discussion for stack protector.
> 
> 
> First, Kees liked to let the build fail
> instead of disabling the stack protector silently:
> 
> https://lkml.org/lkml/2018/2/9/697
> 
> 
> 
> Linus said:
> But yes, I also reacted to your earlier " It can't silently rewrite it
> to _REGULAR because the compiler support for _STRONG regressed."
> Because it damn well can. If the compiler doesn't support
> -fstack-protector-strong, we can just fall back on -fstack-protector.
> Silently. No extra crazy complex logic for that either.
> 
> (https://lkml.org/lkml/2018/2/10/281)
> 
> 
> 
> I hope this is the same pattern.

I wasn't a part of the -fstack-protector conversation, but I doubt it's
the same pattern.  We're trying to phase out frame pointers, for several
reasons.  One big reason is that they cause a general slowdown across
the entire kernel.

Since we switched the x86_64 default to the ORC unwinder, a lot of
people have switched over.  But this patch will reverse (or at least
slow down) that trend, because almost nobody has the libelf devel
packaged installed by default.  So over time, it will effectively make
frame pointers the default again in many cases.  That's exactly what we
*don't* want to do.  It will also cause people to accidentally re-enable
frame pointers when they thought they had ORC.

-- 
Josh