[PATCH] objtool: Fix clang enum conversion warning

[PATCH] objtool: Fix clang enum conversion warning

[Josh Poimboeuf]

From: Lukas Bulwahn <lukas.bulwahn@gmail.com>

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 <lukas.bulwahn@gmail.com>
Signed-off-by: Josh Poimboeuf <jpoimboe@redhat.com>
---
 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

Re: [PATCH] objtool: Fix clang enum conversion warning

[Nick Desaulniers]

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 <nick.desaulniers@gmail.com>

On Tue, Dec 26, 2017 at 4:27 PM, Josh Poimboeuf <jpoimboe@redhat.com> wrote:
> From: Lukas Bulwahn <lukas.bulwahn@gmail.com>
>
> 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 <lukas.bulwahn@gmail.com>
> Signed-off-by: Josh Poimboeuf <jpoimboe@redhat.com>
> ---
>  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
>

Re: [PATCH] objtool: Fix clang enum conversion warning

[Lukas Bulwahn]

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 <nick.desaulniers@gmail.com>
>

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

Re: [PATCH] objtool: Fix clang enum conversion warning

[Josh Poimboeuf]

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 <nick.desaulniers@gmail.com>
> > 
> 
> 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

Re: [PATCH] objtool: Fix clang enum conversion warning

[Nick Desaulniers]

On Wed, Dec 27, 2017 at 12:38 PM, Josh Poimboeuf <jpoimboe@redhat.com> 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?

Re: [PATCH] objtool: Fix clang enum conversion warning

[Josh Poimboeuf]

On Wed, Dec 27, 2017 at 10:42:45PM -0500, Nick Desaulniers wrote:
> On Wed, Dec 27, 2017 at 12:38 PM, Josh Poimboeuf <jpoimboe@redhat.com> 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 <der.herr@hofr.at>
Reviewed-by: Nick Desaulniers <nick.desaulniers@gmail.com>

-- 
Josh

Re: [PATCH] objtool: Fix clang enum conversion warning

[Lukas Bulwahn]

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.

Re: [PATCH] objtool: Fix clang enum conversion warning

[Ingo Molnar]

* Josh Poimboeuf <jpoimboe@redhat.com> 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 <jpoimboe@redhat.com> 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 <der.herr@hofr.at>
> Reviewed-by: Nick Desaulniers <nick.desaulniers@gmail.com>

Sure, done!

Thanks,

	Ingo

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

[tip-bot for Lukas Bulwahn]

Commit-ID:  e7e83dd3ff1dd2f9e60213f6eedc7e5b08192062
Gitweb:     https://git.kernel.org/tip/e7e83dd3ff1dd2f9e60213f6eedc7e5b08192062
Author:     Lukas Bulwahn <lukas.bulwahn@gmail.com>
AuthorDate: Tue, 26 Dec 2017 15:27:20 -0600
Committer:  Ingo Molnar <mingo@kernel.org>
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 <lukas.bulwahn@gmail.com>
Signed-off-by: Josh Poimboeuf <jpoimboe@redhat.com>
Reviewed-by: Nicholas Mc Guire <der.herr@hofr.at>
Reviewed-by: Nick Desaulniers <nick.desaulniers@gmail.com>
Cc: Jiri Slaby <jslaby@suse.cz>
Cc: Linus Torvalds <torvalds@linux-foundation.org>
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: Thomas Gleixner <tglx@linutronix.de>
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 <mingo@kernel.org>
---
 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;