Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1593596 > unrolled thread

[PATCH] objtool: drop redundant flags generation

Back to article view | Back to linux.kernel

Contents

  [PATCH] objtool: drop redundant flags generation Nicholas Mc Guire <der.herr@hofr.at> - 2017-03-06 20:00 +0100
    Re: [PATCH] objtool: drop redundant flags generation Masami Hiramatsu <mhiramat@kernel.org> - 2017-03-07 09:20 +0100

#1593596 — [PATCH] objtool: drop redundant flags generation

The generator was emitting quite a few duplicate flags which was making
doublebitand.cocci nervous. This awk hack resolves the duplicate issue.

Signed-off-by: Nicholas Mc Guire <der.herr@hofr.at>
---

The coccinelle complaints emitted was about 230 findings total:
./arch/x86/lib/inat-tables.c:214:10-20: duplicated argument to & or |
./arch/x86/lib/inat-tables.c:214:23-33: duplicated argument to & or |
./arch/x86/lib/inat-tables.c:218:10-20: duplicated argument to & or |
./arch/x86/lib/inat-tables.c:218:23-33: duplicated argument to & or |
....
./tools/objtool/arch/x86/insn/inat-tables.c:214:10-20: duplicated argument to & or |
./tools/objtool/arch/x86/insn/inat-tables.c:214:23-33: duplicated argument to & or |
./tools/objtool/arch/x86/insn/inat-tables.c:218:10-20: duplicated argument to & or |
./tools/objtool/arch/x86/insn/inat-tables.c:218:23-33: duplicated argument to & or |
...
spatch --sp-file scripts/coccinelle/tests/doublebitand.cocci inat-tables.c -D report
will give you the full list - all are caused by duplicates in the generated
output by the add_flags function in the two instances of gen-insn-attr-x86.awk.

Q: The two copies of gen-insn-attr-x86.awk are identical and its not actually clear
   why this duplication is needed ? Further the maintainers list emitted for the
   two files differ.

Patch was checked by manual review of the diff between the initial file and the
regenerated file after the below patch was applied.
Second verification was by make tools/objtool and comparing the generated binaries
in tools/objtool/arch/x86/decode.o with diff.

Patch is against 4.11-rc1 (localversion-next is next-20170306)

 arch/x86/tools/gen-insn-attr-x86.awk              | 12 ++++++++++--
 tools/objtool/arch/x86/insn/gen-insn-attr-x86.awk | 12 ++++++++++--
 2 files changed, 20 insertions(+), 4 deletions(-)

diff --git a/arch/x86/tools/gen-insn-attr-x86.awk b/arch/x86/tools/gen-insn-attr-x86.awk
index a3d2c62..9cdeefe 100644
--- a/arch/x86/tools/gen-insn-attr-x86.awk
+++ b/arch/x86/tools/gen-insn-attr-x86.awk
@@ -226,8 +226,16 @@ function print_table(tbl,name,fmt,n)
 }
 
 function add_flags(old,new) {
-	if (old && new)
-		return old " | " new
+	if (old == new)
+		return old
+	if (old && new) {
+		if(match(old,new))
+			return old
+		else if(match(new,old))
+			return new
+		else
+			return old " | " new
+        }
 	else if (old)
 		return old
 	else
diff --git a/tools/objtool/arch/x86/insn/gen-insn-attr-x86.awk b/tools/objtool/arch/x86/insn/gen-insn-attr-x86.awk
index a3d2c62..9cdeefe 100644
--- a/tools/objtool/arch/x86/insn/gen-insn-attr-x86.awk
+++ b/tools/objtool/arch/x86/insn/gen-insn-attr-x86.awk
@@ -226,8 +226,16 @@ function print_table(tbl,name,fmt,n)
 }
 
 function add_flags(old,new) {
-	if (old && new)
-		return old " | " new
+	if (old == new)
+		return old
+	if (old && new) {
+		if(match(old,new))
+			return old
+		else if(match(new,old))
+			return new
+		else
+			return old " | " new
+        }
 	else if (old)
 		return old
 	else
-- 
2.1.4

[toc] | [next] | [standalone]

#1593968

On Mon,  6 Mar 2017 18:00:25 +0100
Nicholas Mc Guire <der.herr@hofr.at> wrote:

> The generator was emitting quite a few duplicate flags which was making
> doublebitand.cocci nervous. This awk hack resolves the duplicate issue.

Yes, I know that.
I don't think that the duplicating those flags in "automatic generated"
source code is not so harmful. I personally prefer to keep awk code
simpler...

Thanks,

> 
> Signed-off-by: Nicholas Mc Guire <der.herr@hofr.at>
> ---
> 
> The coccinelle complaints emitted was about 230 findings total:
> ./arch/x86/lib/inat-tables.c:214:10-20: duplicated argument to & or |
> ./arch/x86/lib/inat-tables.c:214:23-33: duplicated argument to & or |
> ./arch/x86/lib/inat-tables.c:218:10-20: duplicated argument to & or |
> ./arch/x86/lib/inat-tables.c:218:23-33: duplicated argument to & or |
> ....
> ./tools/objtool/arch/x86/insn/inat-tables.c:214:10-20: duplicated argument to & or |
> ./tools/objtool/arch/x86/insn/inat-tables.c:214:23-33: duplicated argument to & or |
> ./tools/objtool/arch/x86/insn/inat-tables.c:218:10-20: duplicated argument to & or |
> ./tools/objtool/arch/x86/insn/inat-tables.c:218:23-33: duplicated argument to & or |
> ...
> spatch --sp-file scripts/coccinelle/tests/doublebitand.cocci inat-tables.c -D report
> will give you the full list - all are caused by duplicates in the generated
> output by the add_flags function in the two instances of gen-insn-attr-x86.awk.
> 
> Q: The two copies of gen-insn-attr-x86.awk are identical and its not actually clear
>    why this duplication is needed ? Further the maintainers list emitted for the
>    two files differ.
> 
> Patch was checked by manual review of the diff between the initial file and the
> regenerated file after the below patch was applied.
> Second verification was by make tools/objtool and comparing the generated binaries
> in tools/objtool/arch/x86/decode.o with diff.
> 
> Patch is against 4.11-rc1 (localversion-next is next-20170306)
> 
>  arch/x86/tools/gen-insn-attr-x86.awk              | 12 ++++++++++--
>  tools/objtool/arch/x86/insn/gen-insn-attr-x86.awk | 12 ++++++++++--
>  2 files changed, 20 insertions(+), 4 deletions(-)
> 
> diff --git a/arch/x86/tools/gen-insn-attr-x86.awk b/arch/x86/tools/gen-insn-attr-x86.awk
> index a3d2c62..9cdeefe 100644
> --- a/arch/x86/tools/gen-insn-attr-x86.awk
> +++ b/arch/x86/tools/gen-insn-attr-x86.awk
> @@ -226,8 +226,16 @@ function print_table(tbl,name,fmt,n)
>  }
>  
>  function add_flags(old,new) {
> -	if (old && new)
> -		return old " | " new
> +	if (old == new)
> +		return old
> +	if (old && new) {
> +		if(match(old,new))
> +			return old
> +		else if(match(new,old))
> +			return new
> +		else
> +			return old " | " new
> +        }
>  	else if (old)
>  		return old
>  	else
> diff --git a/tools/objtool/arch/x86/insn/gen-insn-attr-x86.awk b/tools/objtool/arch/x86/insn/gen-insn-attr-x86.awk
> index a3d2c62..9cdeefe 100644
> --- a/tools/objtool/arch/x86/insn/gen-insn-attr-x86.awk
> +++ b/tools/objtool/arch/x86/insn/gen-insn-attr-x86.awk
> @@ -226,8 +226,16 @@ function print_table(tbl,name,fmt,n)
>  }
>  
>  function add_flags(old,new) {
> -	if (old && new)
> -		return old " | " new
> +	if (old == new)
> +		return old
> +	if (old && new) {
> +		if(match(old,new))
> +			return old
> +		else if(match(new,old))
> +			return new
> +		else
> +			return old " | " new
> +        }
>  	else if (old)
>  		return old
>  	else
> -- 
> 2.1.4
> 


-- 
Masami Hiramatsu <mhiramat@kernel.org>

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web