2020-10-29 08:50:55

by Lukas Bulwahn

[permalink] [raw]
Subject: [PATCH] x86/unwind: remove unneeded initialization

make clang-analyzer on x86_64 defconfig caught my attention with:

arch/x86/kernel/unwind_orc.c:38:7: warning: Value stored to 'mid' during
its initialization is never read [clang-analyzer-deadcode.DeadStores]
int *mid = first, *found = first;
^

Commit ee9f8fce9964 ("x86/unwind: Add the ORC unwinder") introduced
__orc_find() with this unneeded dead-store initialization.

Put the variable in local scope and initialize only once the value is
needed to make clang-analyzer happy.

As compilers will detect this unneeded assignment and optimize this
anyway, the resulting object code is effectively identical before and
after this change.

No functional change. Effectively, no change to object code.

Signed-off-by: Lukas Bulwahn <[email protected]>
---
applies cleanly on current master and next-20201028

Josh, please ack.
Ingo, Borislav, please pick this minor non-urgent clean-up patch.

arch/x86/kernel/unwind_orc.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/arch/x86/kernel/unwind_orc.c b/arch/x86/kernel/unwind_orc.c
index 6a339ce328e0..5c64eed08257 100644
--- a/arch/x86/kernel/unwind_orc.c
+++ b/arch/x86/kernel/unwind_orc.c
@@ -35,7 +35,7 @@ static struct orc_entry *__orc_find(int *ip_table, struct orc_entry *u_table,
{
int *first = ip_table;
int *last = ip_table + num_entries - 1;
- int *mid = first, *found = first;
+ int *found = first;

if (!num_entries)
return NULL;
@@ -47,7 +47,7 @@ static struct orc_entry *__orc_find(int *ip_table, struct orc_entry *u_table,
* ignored when they conflict with a real entry.
*/
while (first <= last) {
- mid = first + ((last - first) / 2);
+ int *mid = first + ((last - first) / 2);

if (orc_ip(mid) <= ip) {
found = mid;
--
2.17.1


2020-10-29 08:57:13

by Nathan Chancellor

[permalink] [raw]
Subject: Re: [PATCH] x86/unwind: remove unneeded initialization

On Wed, Oct 28, 2020 at 01:21:02PM +0100, Lukas Bulwahn wrote:
> make clang-analyzer on x86_64 defconfig caught my attention with:
>
> arch/x86/kernel/unwind_orc.c:38:7: warning: Value stored to 'mid' during
> its initialization is never read [clang-analyzer-deadcode.DeadStores]
> int *mid = first, *found = first;
> ^
>
> Commit ee9f8fce9964 ("x86/unwind: Add the ORC unwinder") introduced
> __orc_find() with this unneeded dead-store initialization.
>
> Put the variable in local scope and initialize only once the value is
> needed to make clang-analyzer happy.
>
> As compilers will detect this unneeded assignment and optimize this
> anyway, the resulting object code is effectively identical before and
> after this change.
>
> No functional change. Effectively, no change to object code.
>
> Signed-off-by: Lukas Bulwahn <[email protected]>

Seems fine to me.

Reviewed-by: Nathan Chancellor <[email protected]>

> ---
> applies cleanly on current master and next-20201028
>
> Josh, please ack.
> Ingo, Borislav, please pick this minor non-urgent clean-up patch.
>
> arch/x86/kernel/unwind_orc.c | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/arch/x86/kernel/unwind_orc.c b/arch/x86/kernel/unwind_orc.c
> index 6a339ce328e0..5c64eed08257 100644
> --- a/arch/x86/kernel/unwind_orc.c
> +++ b/arch/x86/kernel/unwind_orc.c
> @@ -35,7 +35,7 @@ static struct orc_entry *__orc_find(int *ip_table, struct orc_entry *u_table,
> {
> int *first = ip_table;
> int *last = ip_table + num_entries - 1;
> - int *mid = first, *found = first;
> + int *found = first;
>
> if (!num_entries)
> return NULL;
> @@ -47,7 +47,7 @@ static struct orc_entry *__orc_find(int *ip_table, struct orc_entry *u_table,
> * ignored when they conflict with a real entry.
> */
> while (first <= last) {
> - mid = first + ((last - first) / 2);
> + int *mid = first + ((last - first) / 2);
>
> if (orc_ip(mid) <= ip) {
> found = mid;
> --
> 2.17.1
>

2020-10-29 11:51:52

by walter harms

[permalink] [raw]
Subject: AW: [PATCH] x86/unwind: remove unneeded initialization

this looks like a reimplementation of bsearch()
perhaps the maintainer can add a comment why the
kernel implementation is not suitable here ?


jm2c
wh

________________________________________
Von: Lukas Bulwahn [[email protected]]
Gesendet: Mittwoch, 28. Oktober 2020 13:21
An: Thomas Gleixner; Ingo Molnar; Borislav Petkov; Josh Poimboeuf; [email protected]
Cc: H . Peter Anvin; Peter Zijlstra; Nathan Chancellor; Nick Desaulniers; [email protected]; [email protected]; [email protected]; [email protected]; Lukas Bulwahn
Betreff: [PATCH] x86/unwind: remove unneeded initialization

make clang-analyzer on x86_64 defconfig caught my attention with:

arch/x86/kernel/unwind_orc.c:38:7: warning: Value stored to 'mid' during
its initialization is never read [clang-analyzer-deadcode.DeadStores]
int *mid = first, *found = first;
^

Commit ee9f8fce9964 ("x86/unwind: Add the ORC unwinder") introduced
__orc_find() with this unneeded dead-store initialization.

Put the variable in local scope and initialize only once the value is
needed to make clang-analyzer happy.

As compilers will detect this unneeded assignment and optimize this
anyway, the resulting object code is effectively identical before and
after this change.

No functional change. Effectively, no change to object code.

Signed-off-by: Lukas Bulwahn <[email protected]>
---
applies cleanly on current master and next-20201028

Josh, please ack.
Ingo, Borislav, please pick this minor non-urgent clean-up patch.

arch/x86/kernel/unwind_orc.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/arch/x86/kernel/unwind_orc.c b/arch/x86/kernel/unwind_orc.c
index 6a339ce328e0..5c64eed08257 100644
--- a/arch/x86/kernel/unwind_orc.c
+++ b/arch/x86/kernel/unwind_orc.c
@@ -35,7 +35,7 @@ static struct orc_entry *__orc_find(int *ip_table, struct orc_entry *u_table,
{
int *first = ip_table;
int *last = ip_table + num_entries - 1;
- int *mid = first, *found = first;
+ int *found = first;

if (!num_entries)
return NULL;
@@ -47,7 +47,7 @@ static struct orc_entry *__orc_find(int *ip_table, struct orc_entry *u_table,
* ignored when they conflict with a real entry.
*/
while (first <= last) {
- mid = first + ((last - first) / 2);
+ int *mid = first + ((last - first) / 2);

if (orc_ip(mid) <= ip) {
found = mid;
--
2.17.1

2020-10-29 12:07:08

by Peter Zijlstra

[permalink] [raw]
Subject: Re: [PATCH] x86/unwind: remove unneeded initialization

On Thu, Oct 29, 2020 at 11:49:50AM +0000, Walter Harms wrote:
> this looks like a reimplementation of bsearch()
> perhaps the maintainer can add a comment why the
> kernel implementation is not suitable here ?

If you look carefully it doesn't do an exact match, which is what
bsearch() does.

bsearch() also isn't stable in the precense of duplicates.