clang warns several times:
drivers/infiniband/sw/siw/siw_cq.c:31:4: warning: implicit conversion
from enumeration type 'enum siw_wc_status' to different enumeration type
'enum siw_opcode' [-Wenum-conversion]
{ SIW_WC_SUCCESS, IB_WC_SUCCESS },
~ ^~~~~~~~~~~~~~
Fixes: b0fff7317bb4 ("rdma/siw: completion queue methods")
Link: https://github.com/ClangBuiltLinux/linux/issues/596
Signed-off-by: Nathan Chancellor <[email protected]>
---
drivers/infiniband/sw/siw/siw_cq.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/infiniband/sw/siw/siw_cq.c b/drivers/infiniband/sw/siw/siw_cq.c
index e2a0ee40d5b5..e381ae9b7d62 100644
--- a/drivers/infiniband/sw/siw/siw_cq.c
+++ b/drivers/infiniband/sw/siw/siw_cq.c
@@ -25,7 +25,7 @@ static int map_wc_opcode[SIW_NUM_OPCODES] = {
};
static struct {
- enum siw_opcode siw;
+ enum siw_wc_status siw;
enum ib_wc_status ib;
} map_cqe_status[SIW_NUM_WC_STATUS] = {
{ SIW_WC_SUCCESS, IB_WC_SUCCESS },
--
2.22.0
-----"Nathan Chancellor" <[email protected]> wrote: -----
>To: "Bernard Metzler" <[email protected]>, "Doug Ledford"
><[email protected]>, "Jason Gunthorpe" <[email protected]>
>From: "Nathan Chancellor" <[email protected]>
>Date: 07/10/2019 07:48PM
>Cc: [email protected], [email protected],
>[email protected], "Nathan Chancellor"
><[email protected]>
>Subject: [EXTERNAL] [PATCH] rdma/siw: Use proper enumerated type in
>map_cqe_status
>
>clang warns several times:
>
>drivers/infiniband/sw/siw/siw_cq.c:31:4: warning: implicit conversion
>from enumeration type 'enum siw_wc_status' to different enumeration
>type
>'enum siw_opcode' [-Wenum-conversion]
> { SIW_WC_SUCCESS, IB_WC_SUCCESS },
> ~ ^~~~~~~~~~~~~~
>
>Fixes: b0fff7317bb4 ("rdma/siw: completion queue methods")
>Link:
>https://urldefense.proofpoint.com/v2/url?u=https-3A__github.com_Clang
>BuiltLinux_linux_issues_596&d=DwIDAg&c=jf_iaSHvJObTbx-siA1ZOg&r=2TaYX
>Q0T-r8ZO1PP1alNwU_QJcRRLfmYTAgd3QCvqSc&m=1dqKSwiEVgePsLNbxXRmdhXDxww4
>AEGxKq-g-MmQHBo&s=IFwaU5yLu598NLBtKkAxLXzRNmACfnhxCpg3QVeJpB0&e=
>Signed-off-by: Nathan Chancellor <[email protected]>
>---
> drivers/infiniband/sw/siw/siw_cq.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
>diff --git a/drivers/infiniband/sw/siw/siw_cq.c
>b/drivers/infiniband/sw/siw/siw_cq.c
>index e2a0ee40d5b5..e381ae9b7d62 100644
>--- a/drivers/infiniband/sw/siw/siw_cq.c
>+++ b/drivers/infiniband/sw/siw/siw_cq.c
>@@ -25,7 +25,7 @@ static int map_wc_opcode[SIW_NUM_OPCODES] = {
> };
>
> static struct {
>- enum siw_opcode siw;
>+ enum siw_wc_status siw;
> enum ib_wc_status ib;
> } map_cqe_status[SIW_NUM_WC_STATUS] = {
> { SIW_WC_SUCCESS, IB_WC_SUCCESS },
>--
>2.22.0
>
>
>
Nathan, thanks very much. That's correct.
I don't know how this could pass w/o warning.
Many thanks,
Bernard.
Hi Bernard,
On Thu, Jul 11, 2019 at 07:44:22AM +0000, Bernard Metzler wrote:
> Nathan, thanks very much. That's correct.
Thanks for the confirmation that the fix was correct.
> I don't know how this could pass w/o warning.
Unfortunately, it appears that GCC only warns when two different
enumerated types are directly compared, not when they are implicitly
converted between.
https://gcc.gnu.org/onlinedocs/gcc/Warning-Options.html#index-Wenum-compare
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=78736
If it did, I wouldn't have fixed as many warnings as I have.
https://github.com/ClangBuiltLinux/linux/issues?q=is%3Aissue+is%3Aclosed+label%3A-Wenum-conversion
Maybe time to start plumbing Clang into your test flow until it can get
intergrated with more CI setups? :) It can catch some pretty dodgy
behavior that GCC doesn't:
https://github.com/ClangBuiltLinux/linux/issues/390
https://github.com/ClangBuiltLinux/linux/issues/544
Kernel CI has added support for it (although they don't email the
authors of patches individually) and 0day is currently working on it.
Feel free to reach out if you decide to explore it, I'm always happy
to help.
Cheers,
Nathan
On Thu, Jul 11, 2019 at 01:14:34AM -0700, Nathan Chancellor wrote:
> Hi Bernard,
>
> On Thu, Jul 11, 2019 at 07:44:22AM +0000, Bernard Metzler wrote:
> > Nathan, thanks very much. That's correct.
>
> Thanks for the confirmation that the fix was correct.
>
> > I don't know how this could pass w/o warning.
>
> Unfortunately, it appears that GCC only warns when two different
> enumerated types are directly compared, not when they are implicitly
> converted between.
>
> https://gcc.gnu.org/onlinedocs/gcc/Warning-Options.html#index-Wenum-compare
>
> https://gcc.gnu.org/bugzilla/show_bug.cgi?id=78736
>
> If it did, I wouldn't have fixed as many warnings as I have.
>
> https://github.com/ClangBuiltLinux/linux/issues?q=is%3Aissue+is%3Aclosed+label%3A-Wenum-conversion
>
> Maybe time to start plumbing Clang into your test flow until it can get
> intergrated with more CI setups? :) It can catch some pretty dodgy
> behavior that GCC doesn't:
I keep asking how to use clang to build the kernel and last I was told
it still wasn't ready..
Is it ready now? Is there some flow that will compile with clang
warning free, on any arch? (at least the portion of the kernel I check)
> Kernel CI has added support for it (although they don't email the
> authors of patches individually)
Well.. we didn't see any emails till yours, so if others are looking
they aren't communicating?
Jason
On Thu, Jul 11, 2019 at 6:39 AM Jason Gunthorpe <[email protected]> wrote:
>
> On Thu, Jul 11, 2019 at 01:14:34AM -0700, Nathan Chancellor wrote:
> > Maybe time to start plumbing Clang into your test flow until it can get
> > intergrated with more CI setups? :) It can catch some pretty dodgy
> > behavior that GCC doesn't:
>
> I keep asking how to use clang to build the kernel and last I was told
> it still wasn't ready..
>
> Is it ready now? Is there some flow that will compile with clang
> warning free, on any arch? (at least the portion of the kernel I check)
$ make CC=clang ...
Let us know if you find something we haven't already.
https://clangbuiltlinux.github.io/
https://github.com/ClangBuiltLinux/linux/issues
--
Thanks,
~Nick Desaulniers
On Thu, Jul 11, 2019 at 10:16:44AM -0700, Nick Desaulniers wrote:
> On Thu, Jul 11, 2019 at 6:39 AM Jason Gunthorpe <[email protected]> wrote:
> >
> > On Thu, Jul 11, 2019 at 01:14:34AM -0700, Nathan Chancellor wrote:
> > > Maybe time to start plumbing Clang into your test flow until it can get
> > > intergrated with more CI setups? :) It can catch some pretty dodgy
> > > behavior that GCC doesn't:
> >
> > I keep asking how to use clang to build the kernel and last I was told
> > it still wasn't ready..
> >
> > Is it ready now? Is there some flow that will compile with clang
> > warning free, on any arch? (at least the portion of the kernel I check)
>
> $ make CC=clang ...
>
> Let us know if you find something we haven't already.
> https://clangbuiltlinux.github.io/
> https://github.com/ClangBuiltLinux/linux/issues
What clang version?
Jason
On Thu, Jul 11, 2019 at 02:18:08PM -0300, Jason Gunthorpe wrote:
> On Thu, Jul 11, 2019 at 10:16:44AM -0700, Nick Desaulniers wrote:
> > On Thu, Jul 11, 2019 at 6:39 AM Jason Gunthorpe <[email protected]> wrote:
> > >
> > > On Thu, Jul 11, 2019 at 01:14:34AM -0700, Nathan Chancellor wrote:
> > > > Maybe time to start plumbing Clang into your test flow until it can get
> > > > intergrated with more CI setups? :) It can catch some pretty dodgy
> > > > behavior that GCC doesn't:
> > >
> > > I keep asking how to use clang to build the kernel and last I was told
> > > it still wasn't ready..
> > >
> > > Is it ready now? Is there some flow that will compile with clang
> > > warning free, on any arch? (at least the portion of the kernel I check)
> >
> > $ make CC=clang ...
> >
> > Let us know if you find something we haven't already.
> > https://clangbuiltlinux.github.io/
> > https://github.com/ClangBuiltLinux/linux/issues
>
> What clang version?
>
> Jason
You'll need clang-9 for x86 because of the asm-goto requirement (or a
selective set of reverts for clang-8) but everything else should be
good with clang-8:
https://travis-ci.com/ClangBuiltLinux/continuous-integration/builds/118745131
We wrote a Python script that builds an LLVM 9 toolchain suitable for
kernel development that is self contained (doesn't install anything to
your system):
https://github.com/ClangBuiltLinux/tc-build
Let me know if there are any issues with it if you give it a go, I've
already fixed one from Thomas Gleixner:
https://lore.kernel.org/lkml/[email protected]/
Cheers,
Nathan
On Thu, Jul 11, 2019 at 10:30:30AM -0700, Nathan Chancellor wrote:
> On Thu, Jul 11, 2019 at 02:18:08PM -0300, Jason Gunthorpe wrote:
> > On Thu, Jul 11, 2019 at 10:16:44AM -0700, Nick Desaulniers wrote:
> > > On Thu, Jul 11, 2019 at 6:39 AM Jason Gunthorpe <[email protected]> wrote:
> > > >
> > > > On Thu, Jul 11, 2019 at 01:14:34AM -0700, Nathan Chancellor wrote:
> > > > > Maybe time to start plumbing Clang into your test flow until it can get
> > > > > intergrated with more CI setups? :) It can catch some pretty dodgy
> > > > > behavior that GCC doesn't:
> > > >
> > > > I keep asking how to use clang to build the kernel and last I was told
> > > > it still wasn't ready..
> > > >
> > > > Is it ready now? Is there some flow that will compile with clang
> > > > warning free, on any arch? (at least the portion of the kernel I check)
> > >
> > > $ make CC=clang ...
> > >
> > > Let us know if you find something we haven't already.
> > > https://clangbuiltlinux.github.io/
> > > https://github.com/ClangBuiltLinux/linux/issues
> >
> > What clang version?
> >
> > Jason
>
> You'll need clang-9 for x86 because of the asm-goto requirement (or a
> selective set of reverts for clang-8) but everything else should be
> good with clang-8:
The latest clang-9 packages from apt.llvm.org do seem to build the
kernel, I get one puzzling warning under RDMA:
drivers/infiniband/hw/hfi1/platform.o: warning: objtool: tune_serdes()+0x1f4: can't find jump dest instruction at .text+0x118a
And a BPF one:
kernel/bpf/core.o: warning: objtool: ___bpf_prog_run()+0xd: sibling call from callable instruction with modified stack frame
Jason
On Mon, Aug 26, 2019 at 08:38:00AM -0700, Nathan Chancellor wrote:
> Hi Jason,
>
> On Fri, Aug 23, 2019 at 11:24:27AM -0300, Jason Gunthorpe wrote:
> > On Thu, Jul 11, 2019 at 10:30:30AM -0700, Nathan Chancellor wrote:
> > > On Thu, Jul 11, 2019 at 02:18:08PM -0300, Jason Gunthorpe wrote:
> > > > On Thu, Jul 11, 2019 at 10:16:44AM -0700, Nick Desaulniers wrote:
> > > > > On Thu, Jul 11, 2019 at 6:39 AM Jason Gunthorpe <[email protected]> wrote:
> > > > > >
> > > > > > On Thu, Jul 11, 2019 at 01:14:34AM -0700, Nathan Chancellor wrote:
> > > > > > > Maybe time to start plumbing Clang into your test flow until it can get
> > > > > > > intergrated with more CI setups? :) It can catch some pretty dodgy
> > > > > > > behavior that GCC doesn't:
> > > > > >
> > > > > > I keep asking how to use clang to build the kernel and last I was told
> > > > > > it still wasn't ready..
> > > > > >
> > > > > > Is it ready now? Is there some flow that will compile with clang
> > > > > > warning free, on any arch? (at least the portion of the kernel I check)
> > > > >
> > > > > $ make CC=clang ...
> > > > >
> > > > > Let us know if you find something we haven't already.
> > > > > https://clangbuiltlinux.github.io/
> > > > > https://github.com/ClangBuiltLinux/linux/issues
> > > >
> > > > What clang version?
> > > >
> > > > Jason
> > >
> > > You'll need clang-9 for x86 because of the asm-goto requirement (or a
> > > selective set of reverts for clang-8) but everything else should be
> > > good with clang-8:
> >
> > The latest clang-9 packages from apt.llvm.org do seem to build the
> > kernel, I get one puzzling warning under RDMA:
> >
> > drivers/infiniband/hw/hfi1/platform.o: warning: objtool: tune_serdes()+0x1f4: can't find jump dest instruction at .text+0x118a
>
> Any particular config that I should use to easily reproduce this?
Sure, attached. With the clang-9 build for Bionic
Jason
Hi Jason,
On Fri, Aug 23, 2019 at 11:24:27AM -0300, Jason Gunthorpe wrote:
> On Thu, Jul 11, 2019 at 10:30:30AM -0700, Nathan Chancellor wrote:
> > On Thu, Jul 11, 2019 at 02:18:08PM -0300, Jason Gunthorpe wrote:
> > > On Thu, Jul 11, 2019 at 10:16:44AM -0700, Nick Desaulniers wrote:
> > > > On Thu, Jul 11, 2019 at 6:39 AM Jason Gunthorpe <[email protected]> wrote:
> > > > >
> > > > > On Thu, Jul 11, 2019 at 01:14:34AM -0700, Nathan Chancellor wrote:
> > > > > > Maybe time to start plumbing Clang into your test flow until it can get
> > > > > > intergrated with more CI setups? :) It can catch some pretty dodgy
> > > > > > behavior that GCC doesn't:
> > > > >
> > > > > I keep asking how to use clang to build the kernel and last I was told
> > > > > it still wasn't ready..
> > > > >
> > > > > Is it ready now? Is there some flow that will compile with clang
> > > > > warning free, on any arch? (at least the portion of the kernel I check)
> > > >
> > > > $ make CC=clang ...
> > > >
> > > > Let us know if you find something we haven't already.
> > > > https://clangbuiltlinux.github.io/
> > > > https://github.com/ClangBuiltLinux/linux/issues
> > >
> > > What clang version?
> > >
> > > Jason
> >
> > You'll need clang-9 for x86 because of the asm-goto requirement (or a
> > selective set of reverts for clang-8) but everything else should be
> > good with clang-8:
>
> The latest clang-9 packages from apt.llvm.org do seem to build the
> kernel, I get one puzzling warning under RDMA:
>
> drivers/infiniband/hw/hfi1/platform.o: warning: objtool: tune_serdes()+0x1f4: can't find jump dest instruction at .text+0x118a
Any particular config that I should use to easily reproduce this?
> And a BPF one:
>
> kernel/bpf/core.o: warning: objtool: ___bpf_prog_run()+0xd: sibling call from callable instruction with modified stack frame
I think this might be related to this?
https://lore.kernel.org/lkml/[email protected]/
Cheers,
Nathan
On Mon, Aug 26, 2019 at 12:42:28PM -0300, Jason Gunthorpe wrote:
> On Mon, Aug 26, 2019 at 08:38:00AM -0700, Nathan Chancellor wrote:
> > On Fri, Aug 23, 2019 at 11:24:27AM -0300, Jason Gunthorpe wrote:
> > > The latest clang-9 packages from apt.llvm.org do seem to build the
> > > kernel, I get one puzzling warning under RDMA:
> > >
> > > drivers/infiniband/hw/hfi1/platform.o: warning: objtool: tune_serdes()+0x1f4: can't find jump dest instruction at .text+0x118a
> >
> > Any particular config that I should use to easily reproduce this?
>
> Sure, attached. With the clang-9 build for Bionic
This is reproducible with the kernel config attached and a tip of tree
build of LLVM.
$ make -j$(nproc) CC=clang O=out clean olddefconfig drivers/infiniband/hw/hfi1/platform.o
...
CC drivers/infiniband/hw/hfi1/platform.o
drivers/infiniband/hw/hfi1/platform.o: warning: objtool: tune_serdes()+0x1f4: can't find jump dest instruction at .text+0x117a
I ran creduce on that file and it spits out:
a() {
char *b = a;
switch (b[7] & 240 >> 4) {
case 10 ... 11:
c();
case 0 ... 9:
case 12:
case 14:
d();
case 13:
case 15:;
}
}
to simply reproduce the warning. The original preprocessed file +
interestingness test are available here:
https://github.com/nathanchance/creduce-files/tree/4e252c0ca19742c90be1445e6c722a43ae561144/rdma-objtool
Looks like that comes from tune_qsfp, which gets inlined into
tune_serdes but I am far from an objtool expert so I am not
really sure what kind of issues I am looking for. Adding Josh
and Peter for a little more visibility.
Here is the original .o file as well:
https://github.com/nathanchance/creduce-files/raw/4e252c0ca19742c90be1445e6c722a43ae561144/rdma-objtool/platform.o.orig
Cheers,
Nathan
On Mon, Aug 26, 2019 at 04:38:29PM -0700, Nathan Chancellor wrote:
> Looks like that comes from tune_qsfp, which gets inlined into
> tune_serdes but I am far from an objtool expert so I am not
> really sure what kind of issues I am looking for. Adding Josh
> and Peter for a little more visibility.
>
> Here is the original .o file as well:
>
> https://github.com/nathanchance/creduce-files/raw/4e252c0ca19742c90be1445e6c722a43ae561144/rdma-objtool/platform.o.orig
574: 0f 87 00 0c 00 00 ja 117a <tune_serdes+0xdfa>
It's jumping to la-la-land past the end of the function.
--
Josh
On Tue, Aug 27, 2019 at 10:08:30AM -0500, Josh Poimboeuf wrote:
> On Mon, Aug 26, 2019 at 04:38:29PM -0700, Nathan Chancellor wrote:
> > Looks like that comes from tune_qsfp, which gets inlined into
> > tune_serdes but I am far from an objtool expert so I am not
> > really sure what kind of issues I am looking for. Adding Josh
> > and Peter for a little more visibility.
> >
> > Here is the original .o file as well:
> >
> > https://github.com/nathanchance/creduce-files/raw/4e252c0ca19742c90be1445e6c722a43ae561144/rdma-objtool/platform.o.orig
>
> 574: 0f 87 00 0c 00 00 ja 117a <tune_serdes+0xdfa>
>
> It's jumping to la-la-land past the end of the function.
How is it possible?
Thanks
>
> --
> Josh
On Tue, Aug 27, 2019 at 08:00:18PM +0300, Leon Romanovsky wrote:
> On Tue, Aug 27, 2019 at 10:08:30AM -0500, Josh Poimboeuf wrote:
> > On Mon, Aug 26, 2019 at 04:38:29PM -0700, Nathan Chancellor wrote:
> > > Looks like that comes from tune_qsfp, which gets inlined into
> > > tune_serdes but I am far from an objtool expert so I am not
> > > really sure what kind of issues I am looking for. Adding Josh
> > > and Peter for a little more visibility.
> > >
> > > Here is the original .o file as well:
> > >
> > > https://github.com/nathanchance/creduce-files/raw/4e252c0ca19742c90be1445e6c722a43ae561144/rdma-objtool/platform.o.orig
> >
> > 574: 0f 87 00 0c 00 00 ja 117a <tune_serdes+0xdfa>
> >
> > It's jumping to la-la-land past the end of the function.
>
> How is it possible?
Looks like a compiler bug.
--
Josh
On Tue, Aug 27, 2019 at 12:23 PM Josh Poimboeuf <[email protected]> wrote:
>
> On Tue, Aug 27, 2019 at 08:00:18PM +0300, Leon Romanovsky wrote:
> > On Tue, Aug 27, 2019 at 10:08:30AM -0500, Josh Poimboeuf wrote:
> > > On Mon, Aug 26, 2019 at 04:38:29PM -0700, Nathan Chancellor wrote:
> > > > Looks like that comes from tune_qsfp, which gets inlined into
> > > > tune_serdes but I am far from an objtool expert so I am not
> > > > really sure what kind of issues I am looking for. Adding Josh
> > > > and Peter for a little more visibility.
> > > >
> > > > Here is the original .o file as well:
> > > >
> > > > https://github.com/nathanchance/creduce-files/raw/4e252c0ca19742c90be1445e6c722a43ae561144/rdma-objtool/platform.o.orig
> > >
> > > 574: 0f 87 00 0c 00 00 ja 117a <tune_serdes+0xdfa>
> > >
> > > It's jumping to la-la-land past the end of the function.
> >
> > How is it possible?
>
> Looks like a compiler bug.
Nathan,
Thanks for the reduced test case. I modified it slightly:
https://godbolt.org/z/15xejg
You can see that the label LBB0_5 seemingly points off into space.
Let me play with this one more a bit, then I will file a bug and
report back.
--
Thanks,
~Nick Desaulniers
On Tue, Aug 27, 2019 at 2:27 PM Nick Desaulniers
<[email protected]> wrote:
>
> On Tue, Aug 27, 2019 at 12:23 PM Josh Poimboeuf <[email protected]> wrote:
> >
> > On Tue, Aug 27, 2019 at 08:00:18PM +0300, Leon Romanovsky wrote:
> > > On Tue, Aug 27, 2019 at 10:08:30AM -0500, Josh Poimboeuf wrote:
> > > > On Mon, Aug 26, 2019 at 04:38:29PM -0700, Nathan Chancellor wrote:
> > > > > Looks like that comes from tune_qsfp, which gets inlined into
> > > > > tune_serdes but I am far from an objtool expert so I am not
> > > > > really sure what kind of issues I am looking for. Adding Josh
> > > > > and Peter for a little more visibility.
> > > > >
> > > > > Here is the original .o file as well:
> > > > >
> > > > > https://github.com/nathanchance/creduce-files/raw/4e252c0ca19742c90be1445e6c722a43ae561144/rdma-objtool/platform.o.orig
> > > >
> > > > 574: 0f 87 00 0c 00 00 ja 117a <tune_serdes+0xdfa>
> > > >
> > > > It's jumping to la-la-land past the end of the function.
> > >
> > > How is it possible?
> >
> > Looks like a compiler bug.
>
> Nathan,
> Thanks for the reduced test case. I modified it slightly:
> https://godbolt.org/z/15xejg
>
> You can see that the label LBB0_5 seemingly points off into space.
> Let me play with this one more a bit, then I will file a bug and
> report back.
Something funny going on in one of the earliest optimizations. Seems
related to an analysis that's deducing that the case statement is
exhaustive (so the GNU C case range is unrelated), but it's keeping
the default case and its comparison around. The analysis is correct;
the value should never be > 0xF so there shouldn't be any runtime
bugs, but this would avoid an unnecessary comparison for exhaustive
switch statements and save a few bytes of object code in such cases.
Filed: https://bugs.llvm.org/show_bug.cgi?id=43129
--
Thanks,
~Nick Desaulniers