diff mbox series

[1/3] x86: dumpstack: avoid uninitlized variable

Message ID 20180202145634.200291-1-arnd@arndb.de
State Accepted
Commit ebfc15019cfa72496c674ffcb0b8ef10790dcddc
Headers show
Series x86 bugfixes for LTO | expand

Commit Message

Arnd Bergmann Feb. 2, 2018, 2:56 p.m. UTC
In some configurations, 'partial' does not get initialized,
as shown by this gcc-8 warning:

arch/x86/kernel/dumpstack.c: In function 'show_trace_log_lvl':
arch/x86/kernel/dumpstack.c:156:4: error: 'partial' may be used uninitialized in this function [-Werror=maybe-uninitialized]
    show_regs_if_on_stack(&stack_info, regs, partial);
    ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~

This initializes it to false, to get the previous behavior in
this case.

a9cdbe72c4e8 ("x86/dumpstack: Fix partial register dumps")
Signed-off-by: Arnd Bergmann <arnd@arndb.de>

---
 arch/x86/kernel/dumpstack.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

-- 
2.9.0

Comments

Arnd Bergmann Feb. 5, 2018, 9:21 p.m. UTC | #1
On Fri, Feb 2, 2018 at 11:36 PM, tip-bot for Arnd Bergmann
<tipbot@zytor.com> wrote:
> Commit-ID:  ebfc15019cfa72496c674ffcb0b8ef10790dcddc

> Gitweb:     https://git.kernel.org/tip/ebfc15019cfa72496c674ffcb0b8ef10790dcddc

> Author:     Arnd Bergmann <arnd@arndb.de>

> AuthorDate: Fri, 2 Feb 2018 15:56:17 +0100

> Committer:  Thomas Gleixner <tglx@linutronix.de>

> CommitDate: Fri, 2 Feb 2018 23:33:50 +0100

>

> x86/dumpstack: Avoid uninitlized variable

>

> In some configurations, 'partial' does not get initialized, as shown by

> this gcc-8 warning:

>

> arch/x86/kernel/dumpstack.c: In function 'show_trace_log_lvl':

> arch/x86/kernel/dumpstack.c:156:4: error: 'partial' may be used uninitialized in this function [-Werror=maybe-uninitialized]

>     show_regs_if_on_stack(&stack_info, regs, partial);

>     ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~

>

> This initializes it to false, to get the previous behavior in this case.

>

> Fixes: a9cdbe72c4e8 ("x86/dumpstack: Fix partial register dumps")



I just noticed my annotation got lost when I sent the patch. I originally
meant to ask Josh to double-check whether it should be 'false' or 'true'
here, or if we maybe need a larger change.

Josh, could you take a look? Unfortunately I did not really understand
your original commit, so I don't know what the safe choice is here
in those cases in which 'partial' is uninitialized.

       Arnd
Josh Poimboeuf Feb. 5, 2018, 9:39 p.m. UTC | #2
On Mon, Feb 05, 2018 at 10:21:06PM +0100, Arnd Bergmann wrote:
> On Fri, Feb 2, 2018 at 11:36 PM, tip-bot for Arnd Bergmann

> <tipbot@zytor.com> wrote:

> > Commit-ID:  ebfc15019cfa72496c674ffcb0b8ef10790dcddc

> > Gitweb:     https://git.kernel.org/tip/ebfc15019cfa72496c674ffcb0b8ef10790dcddc

> > Author:     Arnd Bergmann <arnd@arndb.de>

> > AuthorDate: Fri, 2 Feb 2018 15:56:17 +0100

> > Committer:  Thomas Gleixner <tglx@linutronix.de>

> > CommitDate: Fri, 2 Feb 2018 23:33:50 +0100

> >

> > x86/dumpstack: Avoid uninitlized variable

> >

> > In some configurations, 'partial' does not get initialized, as shown by

> > this gcc-8 warning:

> >

> > arch/x86/kernel/dumpstack.c: In function 'show_trace_log_lvl':

> > arch/x86/kernel/dumpstack.c:156:4: error: 'partial' may be used uninitialized in this function [-Werror=maybe-uninitialized]

> >     show_regs_if_on_stack(&stack_info, regs, partial);

> >     ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~

> >

> > This initializes it to false, to get the previous behavior in this case.

> >

> > Fixes: a9cdbe72c4e8 ("x86/dumpstack: Fix partial register dumps")

> 

> 

> I just noticed my annotation got lost when I sent the patch. I originally

> meant to ask Josh to double-check whether it should be 'false' or 'true'

> here, or if we maybe need a larger change.

> 

> Josh, could you take a look? Unfortunately I did not really understand

> your original commit, so I don't know what the safe choice is here

> in those cases in which 'partial' is uninitialized.


I think it doesn't matter, it seems to be a false positive warning.

The 'partial' variable is only used when 'regs' is non-NULL, and 'regs'
is only set in unwind_get_entry_regs() after 'partial' gets initialized.

-- 
Josh
Arnd Bergmann Feb. 5, 2018, 9:48 p.m. UTC | #3
On Mon, Feb 5, 2018 at 10:39 PM, Josh Poimboeuf <jpoimboe@redhat.com> wrote:
> On Mon, Feb 05, 2018 at 10:21:06PM +0100, Arnd Bergmann wrote:

>> On Fri, Feb 2, 2018 at 11:36 PM, tip-bot for Arnd Bergmann

>> <tipbot@zytor.com> wrote:

>> > Commit-ID:  ebfc15019cfa72496c674ffcb0b8ef10790dcddc

>> > Gitweb:     https://git.kernel.org/tip/ebfc15019cfa72496c674ffcb0b8ef10790dcddc

>> > Author:     Arnd Bergmann <arnd@arndb.de>

>> > AuthorDate: Fri, 2 Feb 2018 15:56:17 +0100

>> > Committer:  Thomas Gleixner <tglx@linutronix.de>

>> > CommitDate: Fri, 2 Feb 2018 23:33:50 +0100

>> >

>> > x86/dumpstack: Avoid uninitlized variable

>> >

>> > In some configurations, 'partial' does not get initialized, as shown by

>> > this gcc-8 warning:

>> >

>> > arch/x86/kernel/dumpstack.c: In function 'show_trace_log_lvl':

>> > arch/x86/kernel/dumpstack.c:156:4: error: 'partial' may be used uninitialized in this function [-Werror=maybe-uninitialized]

>> >     show_regs_if_on_stack(&stack_info, regs, partial);

>> >     ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~

>> >

>> > This initializes it to false, to get the previous behavior in this case.

>> >

>> > Fixes: a9cdbe72c4e8 ("x86/dumpstack: Fix partial register dumps")

>>

>>

>> I just noticed my annotation got lost when I sent the patch. I originally

>> meant to ask Josh to double-check whether it should be 'false' or 'true'

>> here, or if we maybe need a larger change.

>>

>> Josh, could you take a look? Unfortunately I did not really understand

>> your original commit, so I don't know what the safe choice is here

>> in those cases in which 'partial' is uninitialized.

>

> I think it doesn't matter, it seems to be a false positive warning.

>

> The 'partial' variable is only used when 'regs' is non-NULL, and 'regs'

> is only set in unwind_get_entry_regs() after 'partial' gets initialized.


Right, got it now. So my patch is correct either way, just the description
could have been better.

       Arnd
diff mbox series

Patch

diff --git a/arch/x86/kernel/dumpstack.c b/arch/x86/kernel/dumpstack.c
index afbecff161d1..a2d8a3908670 100644
--- a/arch/x86/kernel/dumpstack.c
+++ b/arch/x86/kernel/dumpstack.c
@@ -109,7 +109,7 @@  void show_trace_log_lvl(struct task_struct *task, struct pt_regs *regs,
 	struct stack_info stack_info = {0};
 	unsigned long visit_mask = 0;
 	int graph_idx = 0;
-	bool partial;
+	bool partial = false;
 
 	printk("%sCall Trace:\n", log_lvl);