2020-09-15 18:00:39

by Nick Desaulniers

[permalink] [raw]
Subject: Re: [tip:x86/seves] BUILD SUCCESS WITH WARNING e6eb15c9ba3165698488ae5c34920eea20eaa38e

On Tue, Sep 15, 2020 at 10:21 AM Borislav Petkov <[email protected]> wrote:
>
> On Tue, Sep 15, 2020 at 12:02:48PM -0500, Josh Poimboeuf wrote:
> > If somebody can share the .o file, I can take a look.
>
> If only I could reproduce...
>
> So I built:
>
> /home/share/src/llvm/tc-build/install/bin/clang-12 --version
> ClangBuiltLinux clang version 12.0.0 (https://github.com/llvm/llvm-project 74a9c6d7e1c49cd0e3a8e8072b8aa03f7a84caff)
> Target: x86_64-unknown-linux-gnu
> Thread model: posix
> InstalledDir: /home/share/src/llvm/tc-build/install/bin
>
> and I don't trigger that warning even with that compiler.
>
> What I do get is a lot of those pairs:
>
> init/calibrate.o: warning: objtool: asan.module_ctor()+0xc: call without frame pointer save/setup
> init/calibrate.o: warning: objtool: asan.module_dtor()+0xc: call without frame pointer save/setup
> init/version.o: warning: objtool: asan.module_ctor()+0xc: call without frame pointer save/setup
> init/version.o: warning: objtool: asan.module_dtor()+0xc: call without frame pointer save/setup
> certs/system_keyring.o: warning: objtool: asan.module_ctor()+0xc: call without frame pointer save/setup
> certs/system_keyring.o: warning: objtool: asan.module_dtor()+0xc: call without frame pointer save/setup

+ Kernel-dynamic-tools

> ...
>
> and a link fail at the end.
>
> ld: arch/x86/events/core.o: in function `events_sysfs_show':
> core.c:(.text+0x469b): undefined reference to `stpcpy'
> ld: arch/x86/events/core.o: in function `events_ht_sysfs_show':
> core.c:(.text+0x46f7): undefined reference to `stpcpy'
> ld: drivers/tty/tty_io.o: in function `alloc_tty_struct':
> tty_io.c:(.text+0x2da5): undefined reference to `stpcpy'
> ld: drivers/tty/tty_io.o: in function `tty_register_device_attr':
> tty_io.c:(.text+0x6a09): undefined reference to `stpcpy'
> ld: drivers/tty/tty_io.o: in function `show_cons_active':
> tty_io.c:(.text+0xa819): undefined reference to `stpcpy'
> ld: drivers/scsi/scsi_transport_sas.o:scsi_transport_sas.c:(.text+0x6139): more undefined references to `stpcpy' follow
> make: *** [Makefile:1166: vmlinux] Error 1

b4 am https://lore.kernel.org/lkml/[email protected]/
-o | git am

>
>
> I'm thinking clang12 is too unstable to take it seriously...

--
Thanks,
~Nick Desaulniers


2020-09-15 18:19:13

by Marco Elver

[permalink] [raw]
Subject: Re: [tip:x86/seves] BUILD SUCCESS WITH WARNING e6eb15c9ba3165698488ae5c34920eea20eaa38e

On Tue, 15 Sep 2020 at 19:40, Nick Desaulniers <[email protected]> wrote:
>
> On Tue, Sep 15, 2020 at 10:21 AM Borislav Petkov <[email protected]> wrote:
> >
> > On Tue, Sep 15, 2020 at 12:02:48PM -0500, Josh Poimboeuf wrote:
> > > If somebody can share the .o file, I can take a look.
> >
> > If only I could reproduce...
> >
> > So I built:
> >
> > /home/share/src/llvm/tc-build/install/bin/clang-12 --version
> > ClangBuiltLinux clang version 12.0.0 (https://github.com/llvm/llvm-project 74a9c6d7e1c49cd0e3a8e8072b8aa03f7a84caff)
> > Target: x86_64-unknown-linux-gnu
> > Thread model: posix
> > InstalledDir: /home/share/src/llvm/tc-build/install/bin
> >
> > and I don't trigger that warning even with that compiler.
> >
> > What I do get is a lot of those pairs:
> >
> > init/calibrate.o: warning: objtool: asan.module_ctor()+0xc: call without frame pointer save/setup
> > init/calibrate.o: warning: objtool: asan.module_dtor()+0xc: call without frame pointer save/setup
> > init/version.o: warning: objtool: asan.module_ctor()+0xc: call without frame pointer save/setup
> > init/version.o: warning: objtool: asan.module_dtor()+0xc: call without frame pointer save/setup
> > certs/system_keyring.o: warning: objtool: asan.module_ctor()+0xc: call without frame pointer save/setup
> > certs/system_keyring.o: warning: objtool: asan.module_dtor()+0xc: call without frame pointer save/setup

This one also appears with Clang 11. This is new I think because we
started emitting ASAN ctors for globals redzone initialization.

I think we really do not care about precise stack frames in these
compiler-generated functions. So, would it be reasonable to make
objtool ignore all *san.module_ctor and *san.module_dtor functions (we
have them for ASAN, TSAN, MSAN)?

Thanks,
-- Marco

2020-09-16 08:31:41

by Peter Zijlstra

[permalink] [raw]
Subject: Re: [tip:x86/seves] BUILD SUCCESS WITH WARNING e6eb15c9ba3165698488ae5c34920eea20eaa38e

On Tue, Sep 15, 2020 at 08:09:16PM +0200, Marco Elver wrote:
> On Tue, 15 Sep 2020 at 19:40, Nick Desaulniers <[email protected]> wrote:
> > On Tue, Sep 15, 2020 at 10:21 AM Borislav Petkov <[email protected]> wrote:

> > > init/calibrate.o: warning: objtool: asan.module_ctor()+0xc: call without frame pointer save/setup
> > > init/calibrate.o: warning: objtool: asan.module_dtor()+0xc: call without frame pointer save/setup
> > > init/version.o: warning: objtool: asan.module_ctor()+0xc: call without frame pointer save/setup
> > > init/version.o: warning: objtool: asan.module_dtor()+0xc: call without frame pointer save/setup
> > > certs/system_keyring.o: warning: objtool: asan.module_ctor()+0xc: call without frame pointer save/setup
> > > certs/system_keyring.o: warning: objtool: asan.module_dtor()+0xc: call without frame pointer save/setup
>
> This one also appears with Clang 11. This is new I think because we
> started emitting ASAN ctors for globals redzone initialization.
>
> I think we really do not care about precise stack frames in these
> compiler-generated functions. So, would it be reasonable to make
> objtool ignore all *san.module_ctor and *san.module_dtor functions (we
> have them for ASAN, TSAN, MSAN)?

The thing is, if objtool cannot follow, it cannot generate ORC data and
our unwinder cannot unwind through the instrumentation, and that is a
fail.

Or am I missing something here?

2020-09-16 08:48:41

by Marco Elver

[permalink] [raw]
Subject: Re: [tip:x86/seves] BUILD SUCCESS WITH WARNING e6eb15c9ba3165698488ae5c34920eea20eaa38e

On Wed, 16 Sep 2020 at 10:30, <[email protected]> wrote:
> On Tue, Sep 15, 2020 at 08:09:16PM +0200, Marco Elver wrote:
> > On Tue, 15 Sep 2020 at 19:40, Nick Desaulniers <[email protected]> wrote:
> > > On Tue, Sep 15, 2020 at 10:21 AM Borislav Petkov <[email protected]> wrote:
>
> > > > init/calibrate.o: warning: objtool: asan.module_ctor()+0xc: call without frame pointer save/setup
> > > > init/calibrate.o: warning: objtool: asan.module_dtor()+0xc: call without frame pointer save/setup
> > > > init/version.o: warning: objtool: asan.module_ctor()+0xc: call without frame pointer save/setup
> > > > init/version.o: warning: objtool: asan.module_dtor()+0xc: call without frame pointer save/setup
> > > > certs/system_keyring.o: warning: objtool: asan.module_ctor()+0xc: call without frame pointer save/setup
> > > > certs/system_keyring.o: warning: objtool: asan.module_dtor()+0xc: call without frame pointer save/setup
> >
> > This one also appears with Clang 11. This is new I think because we
> > started emitting ASAN ctors for globals redzone initialization.
> >
> > I think we really do not care about precise stack frames in these
> > compiler-generated functions. So, would it be reasonable to make
> > objtool ignore all *san.module_ctor and *san.module_dtor functions (we
> > have them for ASAN, TSAN, MSAN)?
>
> The thing is, if objtool cannot follow, it cannot generate ORC data and
> our unwinder cannot unwind through the instrumentation, and that is a
> fail.
>
> Or am I missing something here?

They aren't about the actual instrumentation. The warnings are about
module_ctor/module_dtor functions which are compiler-generated, and
these are only called on initialization/destruction (dtors only for
modules I guess).

E.g. for KASAN it's the calls to __asan_register_globals that are
called from asan.module_ctor. For KCSAN the tsan.module_ctor is
effectively a noop (because __tsan_init() is a noop), so it really
doesn't matter much.

Is my assumption correct that the only effect would be if something
called by them fails, we just don't see the full stack trace? I think
we can live with that, there are only few central places that deal
with ctors/dtors (do_ctors(), ...?).

The "real" fix would be to teach the compilers about "frame pointer
save/setup" for generated functions, but I don't think that's
realistic.

Thanks,
-- Marco

2020-09-16 09:09:25

by Peter Zijlstra

[permalink] [raw]
Subject: Re: [tip:x86/seves] BUILD SUCCESS WITH WARNING e6eb15c9ba3165698488ae5c34920eea20eaa38e

On Wed, Sep 16, 2020 at 10:46:41AM +0200, Marco Elver wrote:
> On Wed, 16 Sep 2020 at 10:30, <[email protected]> wrote:
> > On Tue, Sep 15, 2020 at 08:09:16PM +0200, Marco Elver wrote:
> > > On Tue, 15 Sep 2020 at 19:40, Nick Desaulniers <[email protected]> wrote:
> > > > On Tue, Sep 15, 2020 at 10:21 AM Borislav Petkov <[email protected]> wrote:
> >
> > > > > init/calibrate.o: warning: objtool: asan.module_ctor()+0xc: call without frame pointer save/setup
> > > > > init/calibrate.o: warning: objtool: asan.module_dtor()+0xc: call without frame pointer save/setup
> > > > > init/version.o: warning: objtool: asan.module_ctor()+0xc: call without frame pointer save/setup
> > > > > init/version.o: warning: objtool: asan.module_dtor()+0xc: call without frame pointer save/setup
> > > > > certs/system_keyring.o: warning: objtool: asan.module_ctor()+0xc: call without frame pointer save/setup
> > > > > certs/system_keyring.o: warning: objtool: asan.module_dtor()+0xc: call without frame pointer save/setup
> > >
> > > This one also appears with Clang 11. This is new I think because we
> > > started emitting ASAN ctors for globals redzone initialization.
> > >
> > > I think we really do not care about precise stack frames in these
> > > compiler-generated functions. So, would it be reasonable to make
> > > objtool ignore all *san.module_ctor and *san.module_dtor functions (we
> > > have them for ASAN, TSAN, MSAN)?
> >
> > The thing is, if objtool cannot follow, it cannot generate ORC data and
> > our unwinder cannot unwind through the instrumentation, and that is a
> > fail.
> >
> > Or am I missing something here?
>
> They aren't about the actual instrumentation. The warnings are about
> module_ctor/module_dtor functions which are compiler-generated, and
> these are only called on initialization/destruction (dtors only for
> modules I guess).
>
> E.g. for KASAN it's the calls to __asan_register_globals that are
> called from asan.module_ctor. For KCSAN the tsan.module_ctor is
> effectively a noop (because __tsan_init() is a noop), so it really
> doesn't matter much.
>
> Is my assumption correct that the only effect would be if something
> called by them fails, we just don't see the full stack trace? I think
> we can live with that, there are only few central places that deal
> with ctors/dtors (do_ctors(), ...?).

Not only fails, lockdep for example likes to store stack traces of
various callsites etc.. Also perf (NMI) likes to think it can unwind at
all times.

> The "real" fix would be to teach the compilers about "frame pointer
> save/setup" for generated functions, but I don't think that's
> realistic.

How is that unrealistic? If you build with framepointers enabled, the
compiler is supposed to know about this stuff.

2020-09-16 09:36:56

by Marco Elver

[permalink] [raw]
Subject: Re: [tip:x86/seves] BUILD SUCCESS WITH WARNING e6eb15c9ba3165698488ae5c34920eea20eaa38e

On Wed, 16 Sep 2020 at 11:06, <[email protected]> wrote:
> On Wed, Sep 16, 2020 at 10:46:41AM +0200, Marco Elver wrote:
> > On Wed, 16 Sep 2020 at 10:30, <[email protected]> wrote:
> > > On Tue, Sep 15, 2020 at 08:09:16PM +0200, Marco Elver wrote:
> > > > On Tue, 15 Sep 2020 at 19:40, Nick Desaulniers <[email protected]> wrote:
> > > > > On Tue, Sep 15, 2020 at 10:21 AM Borislav Petkov <[email protected]> wrote:
> > >
> > > > > > init/calibrate.o: warning: objtool: asan.module_ctor()+0xc: call without frame pointer save/setup
> > > > > > init/calibrate.o: warning: objtool: asan.module_dtor()+0xc: call without frame pointer save/setup
> > > > > > init/version.o: warning: objtool: asan.module_ctor()+0xc: call without frame pointer save/setup
> > > > > > init/version.o: warning: objtool: asan.module_dtor()+0xc: call without frame pointer save/setup
> > > > > > certs/system_keyring.o: warning: objtool: asan.module_ctor()+0xc: call without frame pointer save/setup
> > > > > > certs/system_keyring.o: warning: objtool: asan.module_dtor()+0xc: call without frame pointer save/setup
> > > >
> > > > This one also appears with Clang 11. This is new I think because we
> > > > started emitting ASAN ctors for globals redzone initialization.
> > > >
> > > > I think we really do not care about precise stack frames in these
> > > > compiler-generated functions. So, would it be reasonable to make
> > > > objtool ignore all *san.module_ctor and *san.module_dtor functions (we
> > > > have them for ASAN, TSAN, MSAN)?
> > >
> > > The thing is, if objtool cannot follow, it cannot generate ORC data and
> > > our unwinder cannot unwind through the instrumentation, and that is a
> > > fail.
> > >
> > > Or am I missing something here?
> >
> > They aren't about the actual instrumentation. The warnings are about
> > module_ctor/module_dtor functions which are compiler-generated, and
> > these are only called on initialization/destruction (dtors only for
> > modules I guess).
> >
> > E.g. for KASAN it's the calls to __asan_register_globals that are
> > called from asan.module_ctor. For KCSAN the tsan.module_ctor is
> > effectively a noop (because __tsan_init() is a noop), so it really
> > doesn't matter much.
> >
> > Is my assumption correct that the only effect would be if something
> > called by them fails, we just don't see the full stack trace? I think
> > we can live with that, there are only few central places that deal
> > with ctors/dtors (do_ctors(), ...?).
>
> Not only fails, lockdep for example likes to store stack traces of
> various callsites etc.. Also perf (NMI) likes to think it can unwind at
> all times.

That's fair, and I would also prefer a proper fix. :-)

> > The "real" fix would be to teach the compilers about "frame pointer
> > save/setup" for generated functions, but I don't think that's
> > realistic.
>
> How is that unrealistic? If you build with framepointers enabled, the
> compiler is supposed to know about this stuff.

If it's a bug in current compilers, it'll be hard to get the fix into
those. My suspicion is there's a bug somewhere. We can try to make new
compiler versions do the right thing. Or maybe we're just missing some
flags, which would be nice. I'll investigate some more.

Thanks,
-- Marco

2020-09-16 18:51:53

by Nick Desaulniers

[permalink] [raw]
Subject: Re: [tip:x86/seves] BUILD SUCCESS WITH WARNING e6eb15c9ba3165698488ae5c34920eea20eaa38e

On Wed, Sep 16, 2020 at 1:46 AM Marco Elver <[email protected]> wrote:
>
> On Wed, 16 Sep 2020 at 10:30, <[email protected]> wrote:
> > On Tue, Sep 15, 2020 at 08:09:16PM +0200, Marco Elver wrote:
> > > On Tue, 15 Sep 2020 at 19:40, Nick Desaulniers <[email protected]> wrote:
> > > > On Tue, Sep 15, 2020 at 10:21 AM Borislav Petkov <[email protected]> wrote:
> >
> > > > > init/calibrate.o: warning: objtool: asan.module_ctor()+0xc: call without frame pointer save/setup
> > > > > init/calibrate.o: warning: objtool: asan.module_dtor()+0xc: call without frame pointer save/setup
> > > > > init/version.o: warning: objtool: asan.module_ctor()+0xc: call without frame pointer save/setup
> > > > > init/version.o: warning: objtool: asan.module_dtor()+0xc: call without frame pointer save/setup
> > > > > certs/system_keyring.o: warning: objtool: asan.module_ctor()+0xc: call without frame pointer save/setup
> > > > > certs/system_keyring.o: warning: objtool: asan.module_dtor()+0xc: call without frame pointer save/setup
> > >
> > > This one also appears with Clang 11. This is new I think because we
> > > started emitting ASAN ctors for globals redzone initialization.
> > >
> > > I think we really do not care about precise stack frames in these
> > > compiler-generated functions. So, would it be reasonable to make
> > > objtool ignore all *san.module_ctor and *san.module_dtor functions (we
> > > have them for ASAN, TSAN, MSAN)?
> >
> > The thing is, if objtool cannot follow, it cannot generate ORC data and
> > our unwinder cannot unwind through the instrumentation, and that is a
> > fail.
> >
> > Or am I missing something here?
>
> They aren't about the actual instrumentation. The warnings are about
> module_ctor/module_dtor functions which are compiler-generated, and
> these are only called on initialization/destruction (dtors only for
> modules I guess).
>
> E.g. for KASAN it's the calls to __asan_register_globals that are
> called from asan.module_ctor. For KCSAN the tsan.module_ctor is
> effectively a noop (because __tsan_init() is a noop), so it really
> doesn't matter much.
>
> Is my assumption correct that the only effect would be if something
> called by them fails, we just don't see the full stack trace? I think
> we can live with that, there are only few central places that deal
> with ctors/dtors (do_ctors(), ...?).
>
> The "real" fix would be to teach the compilers about "frame pointer
> save/setup" for generated functions, but I don't think that's
> realistic.

So this has come up before, specifically in the context of gcov:
https://github.com/ClangBuiltLinux/linux/issues/955.

I looked into this a bit, and IIRC, the issue was that compiler
generated functions aren't very good about keeping track of whether
they should or should not emit framepointer setup/teardown
prolog/epilogs. In LLVM's IR, -fno-omit-frame-pointer gets attached
to every function as a function level attribute.
https://godbolt.org/z/fcn9c6 ("frame-pointer"="all").

There were some recent LLVM patches for BTI (arm64) that made some BTI
related command line flags module level attributes, which I thought
was interesting; I was wondering last night if -fno-omit-frame-pointer
and maybe even the level of stack protector should be? I guess LTO
would complicate things; not sure it would be good to merge modules
with different attributes; I'm not sure how that's handled today in
LLVM.

Basically, when the compiler is synthesizing a new function
definition, it should check whether a frame pointer should be emitted
or not. We could do that today by maybe scanning all other function
definitions for the presence of "frame-pointer"="all" fn attr,
breaking early if we find one, and emitting the frame pointer setup in
that case. Though I guess it's "frame-pointer"="none" otherwise, so
maybe checking any other fn def would be fine; I don't see any C fn
attr's that allow you to keep frame pointers or not. What's tricky is
that the front end flag was resolved much earlier than where this code
gets generated, so it would need to look for traces that the flag ever
existed, which sounds brittle on paper to me.
--
Thanks,
~Nick Desaulniers

2020-09-16 18:53:46

by Marco Elver

[permalink] [raw]
Subject: Re: [tip:x86/seves] BUILD SUCCESS WITH WARNING e6eb15c9ba3165698488ae5c34920eea20eaa38e

On Wed, 16 Sep 2020 at 20:22, 'Nick Desaulniers' via kasan-dev
<[email protected]> wrote:
>
> On Wed, Sep 16, 2020 at 1:46 AM Marco Elver <[email protected]> wrote:
> >
> > On Wed, 16 Sep 2020 at 10:30, <[email protected]> wrote:
> > > On Tue, Sep 15, 2020 at 08:09:16PM +0200, Marco Elver wrote:
> > > > On Tue, 15 Sep 2020 at 19:40, Nick Desaulniers <[email protected]> wrote:
> > > > > On Tue, Sep 15, 2020 at 10:21 AM Borislav Petkov <[email protected]> wrote:
> > >
> > > > > > init/calibrate.o: warning: objtool: asan.module_ctor()+0xc: call without frame pointer save/setup
> > > > > > init/calibrate.o: warning: objtool: asan.module_dtor()+0xc: call without frame pointer save/setup
> > > > > > init/version.o: warning: objtool: asan.module_ctor()+0xc: call without frame pointer save/setup
> > > > > > init/version.o: warning: objtool: asan.module_dtor()+0xc: call without frame pointer save/setup
> > > > > > certs/system_keyring.o: warning: objtool: asan.module_ctor()+0xc: call without frame pointer save/setup
> > > > > > certs/system_keyring.o: warning: objtool: asan.module_dtor()+0xc: call without frame pointer save/setup
> > > >
> > > > This one also appears with Clang 11. This is new I think because we
> > > > started emitting ASAN ctors for globals redzone initialization.
> > > >
> > > > I think we really do not care about precise stack frames in these
> > > > compiler-generated functions. So, would it be reasonable to make
> > > > objtool ignore all *san.module_ctor and *san.module_dtor functions (we
> > > > have them for ASAN, TSAN, MSAN)?
> > >
> > > The thing is, if objtool cannot follow, it cannot generate ORC data and
> > > our unwinder cannot unwind through the instrumentation, and that is a
> > > fail.
> > >
> > > Or am I missing something here?
> >
> > They aren't about the actual instrumentation. The warnings are about
> > module_ctor/module_dtor functions which are compiler-generated, and
> > these are only called on initialization/destruction (dtors only for
> > modules I guess).
> >
> > E.g. for KASAN it's the calls to __asan_register_globals that are
> > called from asan.module_ctor. For KCSAN the tsan.module_ctor is
> > effectively a noop (because __tsan_init() is a noop), so it really
> > doesn't matter much.
> >
> > Is my assumption correct that the only effect would be if something
> > called by them fails, we just don't see the full stack trace? I think
> > we can live with that, there are only few central places that deal
> > with ctors/dtors (do_ctors(), ...?).
> >
> > The "real" fix would be to teach the compilers about "frame pointer
> > save/setup" for generated functions, but I don't think that's
> > realistic.
>
> So this has come up before, specifically in the context of gcov:
> https://github.com/ClangBuiltLinux/linux/issues/955.
>
> I looked into this a bit, and IIRC, the issue was that compiler
> generated functions aren't very good about keeping track of whether
> they should or should not emit framepointer setup/teardown
> prolog/epilogs. In LLVM's IR, -fno-omit-frame-pointer gets attached
> to every function as a function level attribute.
> https://godbolt.org/z/fcn9c6 ("frame-pointer"="all").
>
> There were some recent LLVM patches for BTI (arm64) that made some BTI
> related command line flags module level attributes, which I thought
> was interesting; I was wondering last night if -fno-omit-frame-pointer
> and maybe even the level of stack protector should be? I guess LTO
> would complicate things; not sure it would be good to merge modules
> with different attributes; I'm not sure how that's handled today in
> LLVM.
>
> Basically, when the compiler is synthesizing a new function
> definition, it should check whether a frame pointer should be emitted
> or not. We could do that today by maybe scanning all other function
> definitions for the presence of "frame-pointer"="all" fn attr,
> breaking early if we find one, and emitting the frame pointer setup in
> that case. Though I guess it's "frame-pointer"="none" otherwise, so
> maybe checking any other fn def would be fine; I don't see any C fn
> attr's that allow you to keep frame pointers or not. What's tricky is
> that the front end flag was resolved much earlier than where this code
> gets generated, so it would need to look for traces that the flag ever
> existed, which sounds brittle on paper to me.

Thanks for the summary -- yeah, that was my suspicion, that some
attribute was being lost somewhere. And I think if we generalize this,
and don't just try to attach "frame-pointer" attr to the function, we
probably also solve the BTI issue that Mark still pointed out with
these module_ctor/dtors.

I was trying to see if there was a generic way to attach all the
common attributes to the function generated here:
https://github.com/llvm/llvm-project/blob/master/llvm/lib/Transforms/Utils/ModuleUtils.cpp#L122
-- but we probably can't attach all attributes, and need to remove a
bunch of them again like the sanitizers (or alternatively just select
the ones we need). But, I'm still digging for the function that
attaches all the common attributes...

Thanks,
-- Marco

2020-09-17 04:22:20

by Fangrui Song

[permalink] [raw]
Subject: Re: [tip:x86/seves] BUILD SUCCESS WITH WARNING e6eb15c9ba3165698488ae5c34920eea20eaa38e

On 2020-09-16, 'Marco Elver' via Clang Built Linux wrote:
>On Wed, 16 Sep 2020 at 20:22, 'Nick Desaulniers' via kasan-dev
><[email protected]> wrote:
>>
>> On Wed, Sep 16, 2020 at 1:46 AM Marco Elver <[email protected]> wrote:
>> >
>> > On Wed, 16 Sep 2020 at 10:30, <[email protected]> wrote:
>> > > On Tue, Sep 15, 2020 at 08:09:16PM +0200, Marco Elver wrote:
>> > > > On Tue, 15 Sep 2020 at 19:40, Nick Desaulniers <[email protected]> wrote:
>> > > > > On Tue, Sep 15, 2020 at 10:21 AM Borislav Petkov <[email protected]> wrote:
>> > >
>> > > > > > init/calibrate.o: warning: objtool: asan.module_ctor()+0xc: call without frame pointer save/setup
>> > > > > > init/calibrate.o: warning: objtool: asan.module_dtor()+0xc: call without frame pointer save/setup
>> > > > > > init/version.o: warning: objtool: asan.module_ctor()+0xc: call without frame pointer save/setup
>> > > > > > init/version.o: warning: objtool: asan.module_dtor()+0xc: call without frame pointer save/setup
>> > > > > > certs/system_keyring.o: warning: objtool: asan.module_ctor()+0xc: call without frame pointer save/setup
>> > > > > > certs/system_keyring.o: warning: objtool: asan.module_dtor()+0xc: call without frame pointer save/setup
>> > > >
>> > > > This one also appears with Clang 11. This is new I think because we
>> > > > started emitting ASAN ctors for globals redzone initialization.
>> > > >
>> > > > I think we really do not care about precise stack frames in these
>> > > > compiler-generated functions. So, would it be reasonable to make
>> > > > objtool ignore all *san.module_ctor and *san.module_dtor functions (we
>> > > > have them for ASAN, TSAN, MSAN)?
>> > >
>> > > The thing is, if objtool cannot follow, it cannot generate ORC data and
>> > > our unwinder cannot unwind through the instrumentation, and that is a
>> > > fail.
>> > >
>> > > Or am I missing something here?
>> >
>> > They aren't about the actual instrumentation. The warnings are about
>> > module_ctor/module_dtor functions which are compiler-generated, and
>> > these are only called on initialization/destruction (dtors only for
>> > modules I guess).
>> >
>> > E.g. for KASAN it's the calls to __asan_register_globals that are
>> > called from asan.module_ctor. For KCSAN the tsan.module_ctor is
>> > effectively a noop (because __tsan_init() is a noop), so it really
>> > doesn't matter much.
>> >
>> > Is my assumption correct that the only effect would be if something
>> > called by them fails, we just don't see the full stack trace? I think
>> > we can live with that, there are only few central places that deal
>> > with ctors/dtors (do_ctors(), ...?).
>> >
>> > The "real" fix would be to teach the compilers about "frame pointer
>> > save/setup" for generated functions, but I don't think that's
>> > realistic.
>>
>> So this has come up before, specifically in the context of gcov:
>> https://github.com/ClangBuiltLinux/linux/issues/955.
>>
>> I looked into this a bit, and IIRC, the issue was that compiler
>> generated functions aren't very good about keeping track of whether
>> they should or should not emit framepointer setup/teardown
>> prolog/epilogs. In LLVM's IR, -fno-omit-frame-pointer gets attached
>> to every function as a function level attribute.
>> https://godbolt.org/z/fcn9c6 ("frame-pointer"="all").
>>
>> There were some recent LLVM patches for BTI (arm64) that made some BTI
>> related command line flags module level attributes, which I thought
>> was interesting; I was wondering last night if -fno-omit-frame-pointer
>> and maybe even the level of stack protector should be? I guess LTO
>> would complicate things; not sure it would be good to merge modules
>> with different attributes; I'm not sure how that's handled today in
>> LLVM.
>>
>> Basically, when the compiler is synthesizing a new function
>> definition, it should check whether a frame pointer should be emitted
>> or not. We could do that today by maybe scanning all other function
>> definitions for the presence of "frame-pointer"="all" fn attr,
>> breaking early if we find one, and emitting the frame pointer setup in
>> that case. Though I guess it's "frame-pointer"="none" otherwise, so
>> maybe checking any other fn def would be fine; I don't see any C fn
>> attr's that allow you to keep frame pointers or not. What's tricky is
>> that the front end flag was resolved much earlier than where this code
>> gets generated, so it would need to look for traces that the flag ever
>> existed, which sounds brittle on paper to me.
>
>Thanks for the summary -- yeah, that was my suspicion, that some
>attribute was being lost somewhere. And I think if we generalize this,
>and don't just try to attach "frame-pointer" attr to the function, we
>probably also solve the BTI issue that Mark still pointed out with
>these module_ctor/dtors.
>
>I was trying to see if there was a generic way to attach all the
>common attributes to the function generated here:
>https://github.com/llvm/llvm-project/blob/master/llvm/lib/Transforms/Utils/ModuleUtils.cpp#L122
>-- but we probably can't attach all attributes, and need to remove a
>bunch of them again like the sanitizers (or alternatively just select
>the ones we need). But, I'm still digging for the function that
>attaches all the common attributes...
>
>Thanks,
>-- Marco

Speaking of gcov, do people know whether frame pointers in
kernel's libgcov implementation help?

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=94394 "random kernel panic during collecting kernel code coverage"

2020-09-17 18:43:17

by Josh Poimboeuf

[permalink] [raw]
Subject: Re: [tip:x86/seves] BUILD SUCCESS WITH WARNING e6eb15c9ba3165698488ae5c34920eea20eaa38e

On Wed, Sep 16, 2020 at 11:22:02AM -0700, Nick Desaulniers wrote:
> I looked into this a bit, and IIRC, the issue was that compiler
> generated functions aren't very good about keeping track of whether
> they should or should not emit framepointer setup/teardown
> prolog/epilogs. In LLVM's IR, -fno-omit-frame-pointer gets attached
> to every function as a function level attribute.
> https://godbolt.org/z/fcn9c6 ("frame-pointer"="all").
>
> There were some recent LLVM patches for BTI (arm64) that made some BTI
> related command line flags module level attributes, which I thought
> was interesting; I was wondering last night if -fno-omit-frame-pointer
> and maybe even the level of stack protector should be? I guess LTO
> would complicate things; not sure it would be good to merge modules
> with different attributes; I'm not sure how that's handled today in
> LLVM.
>
> Basically, when the compiler is synthesizing a new function
> definition, it should check whether a frame pointer should be emitted
> or not. We could do that today by maybe scanning all other function
> definitions for the presence of "frame-pointer"="all" fn attr,
> breaking early if we find one, and emitting the frame pointer setup in
> that case. Though I guess it's "frame-pointer"="none" otherwise, so
> maybe checking any other fn def would be fine; I don't see any C fn
> attr's that allow you to keep frame pointers or not. What's tricky is
> that the front end flag was resolved much earlier than where this code
> gets generated, so it would need to look for traces that the flag ever
> existed, which sounds brittle on paper to me.

For code generated by the kernel at runtime, our current (x86) policy is
"always use frame pointers for non-leaf functions".

A lot of this compiler talk is over my head, but if *non-leaf* generated
functions are rare enough then it might be worth considering to just
always use frame pointers for them.

--
Josh