2017-12-26 21:27:49

by Josh Poimboeuf

[permalink] [raw]
Subject: [PATCH] objtool: Fix clang enum conversion warning

From: Lukas Bulwahn <[email protected]>

Fix the following clang enum conversion warning:

arch/x86/decode.c:141:20: error: implicit conversion from enumeration
type 'enum op_src_type' to different enumeration
type 'enum op_dest_type' [-Werror,-Wenum-conversion]

op->dest.type = OP_SRC_REG;
~ ^~~~~~~~~~

It just happened to work before because OP_SRC_REG and OP_DEST_REG have
the same value.

Fixes: baa41469a7b9 ("objtool: Implement stack validation 2.0")
Signed-off-by: Lukas Bulwahn <[email protected]>
Signed-off-by: Josh Poimboeuf <[email protected]>
---
tools/objtool/arch/x86/decode.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/tools/objtool/arch/x86/decode.c b/tools/objtool/arch/x86/decode.c
index 8acfc47af70e..540a209b78ab 100644
--- a/tools/objtool/arch/x86/decode.c
+++ b/tools/objtool/arch/x86/decode.c
@@ -138,7 +138,7 @@ int arch_decode_instruction(struct elf *elf, struct section *sec,
*type = INSN_STACK;
op->src.type = OP_SRC_ADD;
op->src.reg = op_to_cfi_reg[modrm_reg][rex_r];
- op->dest.type = OP_SRC_REG;
+ op->dest.type = OP_DEST_REG;
op->dest.reg = CFI_SP;
}
break;
--
2.13.6


2017-12-27 00:35:15

by Nick Desaulniers

[permalink] [raw]
Subject: Re: [PATCH] objtool: Fix clang enum conversion warning

I sent a similar one recently:
https://patchwork.kernel.org/patch/10131815/ (maybe Josh is just
forwarding me an earlier fix?)

Reviewed-by: Nick Desaulniers <[email protected]>

On Tue, Dec 26, 2017 at 4:27 PM, Josh Poimboeuf <[email protected]> wrote:
> From: Lukas Bulwahn <[email protected]>
>
> Fix the following clang enum conversion warning:
>
> arch/x86/decode.c:141:20: error: implicit conversion from enumeration
> type 'enum op_src_type' to different enumeration
> type 'enum op_dest_type' [-Werror,-Wenum-conversion]
>
> op->dest.type = OP_SRC_REG;
> ~ ^~~~~~~~~~
>
> It just happened to work before because OP_SRC_REG and OP_DEST_REG have
> the same value.
>
> Fixes: baa41469a7b9 ("objtool: Implement stack validation 2.0")
> Signed-off-by: Lukas Bulwahn <[email protected]>
> Signed-off-by: Josh Poimboeuf <[email protected]>
> ---
> tools/objtool/arch/x86/decode.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/tools/objtool/arch/x86/decode.c b/tools/objtool/arch/x86/decode.c
> index 8acfc47af70e..540a209b78ab 100644
> --- a/tools/objtool/arch/x86/decode.c
> +++ b/tools/objtool/arch/x86/decode.c
> @@ -138,7 +138,7 @@ int arch_decode_instruction(struct elf *elf, struct section *sec,
> *type = INSN_STACK;
> op->src.type = OP_SRC_ADD;
> op->src.reg = op_to_cfi_reg[modrm_reg][rex_r];
> - op->dest.type = OP_SRC_REG;
> + op->dest.type = OP_DEST_REG;
> op->dest.reg = CFI_SP;
> }
> break;
> --
> 2.13.6
>

2017-12-27 12:34:59

by Lukas Bulwahn

[permalink] [raw]
Subject: Re: [PATCH] objtool: Fix clang enum conversion warning


On Tue, 26 Dec 2017, Nick Desaulniers wrote:

> I sent a similar one recently:
> https://patchwork.kernel.org/patch/10131815/ (maybe Josh is just
> forwarding me an earlier fix?)
>
> Reviewed-by: Nick Desaulniers <[email protected]>
>

I actually submitted this (other) patch to LKML on 2017-12-10:

https://patchwork.kernel.org/patch/10103977/

I also pointed this out on the llvmlinux mailing list:

https://lists.linuxfoundation.org/pipermail/llvmlinux/2017-December/001535.html

(The mail might not have been distributed yet to its recipients, because I
am on the llvmlinux mailing list only for a few days, and I might have not
been whitelisted for getting through the spam filtering of that list.)

Nick submitted another patch to LKML on 2017-12-24 (see above).

The source code change is the same; but the commit message was
different. Now the third patch from Josh here is another equal patch with
yet another commit message, combining information from both patches.

Assuming that the authorship of this one-line change does not matter, as
it is largely suggested by the clang compiler anyway, and we want to move
the change forward, we should decide on which of three patches to move
forward. I can give my Reviewed-by and Tested-by to any of them.

Lukas

2017-12-27 17:38:22

by Josh Poimboeuf

[permalink] [raw]
Subject: Re: [PATCH] objtool: Fix clang enum conversion warning

On Wed, Dec 27, 2017 at 01:34:34PM +0100, Lukas Bulwahn wrote:
>
> On Tue, 26 Dec 2017, Nick Desaulniers wrote:
>
> > I sent a similar one recently:
> > https://patchwork.kernel.org/patch/10131815/ (maybe Josh is just
> > forwarding me an earlier fix?)
> >
> > Reviewed-by: Nick Desaulniers <[email protected]>
> >
>
> I actually submitted this (other) patch to LKML on 2017-12-10:
>
> https://patchwork.kernel.org/patch/10103977/
>
> I also pointed this out on the llvmlinux mailing list:
>
> https://lists.linuxfoundation.org/pipermail/llvmlinux/2017-December/001535.html
>
> (The mail might not have been distributed yet to its recipients, because I
> am on the llvmlinux mailing list only for a few days, and I might have not
> been whitelisted for getting through the spam filtering of that list.)
>
> Nick submitted another patch to LKML on 2017-12-24 (see above).
>
> The source code change is the same; but the commit message was different.
> Now the third patch from Josh here is another equal patch with yet another
> commit message, combining information from both patches.
>
> Assuming that the authorship of this one-line change does not matter, as it
> is largely suggested by the clang compiler anyway, and we want to move the
> change forward, we should decide on which of three patches to move
> forward. I can give my Reviewed-by and Tested-by to any of them.

The patch from Lukas was the first one I received, so that's the one I
used. I rewrote the commit msg for clarity and added my SOB and sent it
to Ingo for merging.

--
Josh

2017-12-28 03:42:48

by Nick Desaulniers

[permalink] [raw]
Subject: Re: [PATCH] objtool: Fix clang enum conversion warning

On Wed, Dec 27, 2017 at 12:38 PM, Josh Poimboeuf <[email protected]> wrote:
> On Wed, Dec 27, 2017 at 01:34:34PM +0100, Lukas Bulwahn wrote:
>> Assuming that the authorship of this one-line change does not matter, as it
>> is largely suggested by the clang compiler anyway, and we want to move the
>> change forward, we should decide on which of three patches to move
>> forward. I can give my Reviewed-by and Tested-by to any of them.

I suppose Ingo would take the first and accumulate Reviewed-By tags.
I don't particularly care about authorship (please just fix the bug).

> The patch from Lukas was the first one I received, so that's the one I
> used. I rewrote the commit msg for clarity and added my SOB and sent it
> to Ingo for merging.

I think you should have kept Nicholas Mc Guire's Reviewed by tag?
Maybe Ingo can re-add his and mine when merging?

2017-12-28 05:12:11

by Josh Poimboeuf

[permalink] [raw]
Subject: Re: [PATCH] objtool: Fix clang enum conversion warning

On Wed, Dec 27, 2017 at 10:42:45PM -0500, Nick Desaulniers wrote:
> On Wed, Dec 27, 2017 at 12:38 PM, Josh Poimboeuf <[email protected]> wrote:
> > On Wed, Dec 27, 2017 at 01:34:34PM +0100, Lukas Bulwahn wrote:
> >> Assuming that the authorship of this one-line change does not matter, as it
> >> is largely suggested by the clang compiler anyway, and we want to move the
> >> change forward, we should decide on which of three patches to move
> >> forward. I can give my Reviewed-by and Tested-by to any of them.
>
> I suppose Ingo would take the first and accumulate Reviewed-By tags.
> I don't particularly care about authorship (please just fix the bug).
>
> > The patch from Lukas was the first one I received, so that's the one I
> > used. I rewrote the commit msg for clarity and added my SOB and sent it
> > to Ingo for merging.
>
> I think you should have kept Nicholas Mc Guire's Reviewed by tag?
> Maybe Ingo can re-add his and mine when merging?

Yes, sorry, I missed that one.

Ingo, can you please add the following?

Reviewed-by: Nicholas Mc Guire <[email protected]>
Reviewed-by: Nick Desaulniers <[email protected]>

--
Josh

2017-12-28 08:19:05

by Lukas Bulwahn

[permalink] [raw]
Subject: Re: [PATCH] objtool: Fix clang enum conversion warning



On Wed, 27 Dec 2017, Josh Poimboeuf wrote:
>
> The patch from Lukas was the first one I received, so that's the one I
> used. I rewrote the commit msg for clarity and added my SOB and sent it
> to Ingo for merging.
>

Josh, Ingo,

the first patch I sent to Josh must have been a v0, which I probably did
not even checkpatch.pl. It still was with a github-style commit message
(for nice formatting in a github discussion), not with a kernel-style
commit message. So I would suggest to take the patch:

https://patchwork.kernel.org/patch/10103977/

and add Josh's Signed-off, and Nicholas' and Nick's Reviewed-by.

2017-12-28 12:09:50

by Ingo Molnar

[permalink] [raw]
Subject: Re: [PATCH] objtool: Fix clang enum conversion warning


* Josh Poimboeuf <[email protected]> wrote:

> On Wed, Dec 27, 2017 at 10:42:45PM -0500, Nick Desaulniers wrote:
> > On Wed, Dec 27, 2017 at 12:38 PM, Josh Poimboeuf <[email protected]> wrote:
> > > On Wed, Dec 27, 2017 at 01:34:34PM +0100, Lukas Bulwahn wrote:
> > >> Assuming that the authorship of this one-line change does not matter, as it
> > >> is largely suggested by the clang compiler anyway, and we want to move the
> > >> change forward, we should decide on which of three patches to move
> > >> forward. I can give my Reviewed-by and Tested-by to any of them.
> >
> > I suppose Ingo would take the first and accumulate Reviewed-By tags.
> > I don't particularly care about authorship (please just fix the bug).
> >
> > > The patch from Lukas was the first one I received, so that's the one I
> > > used. I rewrote the commit msg for clarity and added my SOB and sent it
> > > to Ingo for merging.
> >
> > I think you should have kept Nicholas Mc Guire's Reviewed by tag?
> > Maybe Ingo can re-add his and mine when merging?
>
> Yes, sorry, I missed that one.
>
> Ingo, can you please add the following?
>
> Reviewed-by: Nicholas Mc Guire <[email protected]>
> Reviewed-by: Nick Desaulniers <[email protected]>

Sure, done!

Thanks,

Ingo

Subject: [tip:core/urgent] objtool: Fix Clang enum conversion warning

Commit-ID: e7e83dd3ff1dd2f9e60213f6eedc7e5b08192062
Gitweb: https://git.kernel.org/tip/e7e83dd3ff1dd2f9e60213f6eedc7e5b08192062
Author: Lukas Bulwahn <[email protected]>
AuthorDate: Tue, 26 Dec 2017 15:27:20 -0600
Committer: Ingo Molnar <[email protected]>
CommitDate: Thu, 28 Dec 2017 13:11:13 +0100

objtool: Fix Clang enum conversion warning

Fix the following Clang enum conversion warning:

arch/x86/decode.c:141:20: error: implicit conversion from enumeration
type 'enum op_src_type' to different enumeration
type 'enum op_dest_type' [-Werror,-Wenum-conversion]

op->dest.type = OP_SRC_REG;
~ ^~~~~~~~~~

It just happened to work before because OP_SRC_REG and OP_DEST_REG have
the same value.

Signed-off-by: Lukas Bulwahn <[email protected]>
Signed-off-by: Josh Poimboeuf <[email protected]>
Reviewed-by: Nicholas Mc Guire <[email protected]>
Reviewed-by: Nick Desaulniers <[email protected]>
Cc: Jiri Slaby <[email protected]>
Cc: Linus Torvalds <[email protected]>
Cc: Peter Zijlstra <[email protected]>
Cc: Thomas Gleixner <[email protected]>
Fixes: baa41469a7b9 ("objtool: Implement stack validation 2.0")
Link: http://lkml.kernel.org/r/b4156c5738bae781c392e7a3691aed4514ebbdf2.1514323568.git.jpoimboe@redhat.com
Signed-off-by: Ingo Molnar <[email protected]>
---
tools/objtool/arch/x86/decode.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/tools/objtool/arch/x86/decode.c b/tools/objtool/arch/x86/decode.c
index 8acfc47..540a209 100644
--- a/tools/objtool/arch/x86/decode.c
+++ b/tools/objtool/arch/x86/decode.c
@@ -138,7 +138,7 @@ int arch_decode_instruction(struct elf *elf, struct section *sec,
*type = INSN_STACK;
op->src.type = OP_SRC_ADD;
op->src.reg = op_to_cfi_reg[modrm_reg][rex_r];
- op->dest.type = OP_SRC_REG;
+ op->dest.type = OP_DEST_REG;
op->dest.reg = CFI_SP;
}
break;