[kvm-unit-tests PATCH] x86: setjmp: ignore clang's "-Wsomtimes-uninitialized" flag

[kvm-unit-tests PATCH] x86: setjmp: ignore clang's "-Wsomtimes-uninitialized" flag

[Bill Wendling]

Clang complains that "i" might be uninitialized in the "printf"
statement. This is a false negative, because it's set in the "if"
statement and then incremented in the loop created by the "longjmp".

Signed-off-by: Bill Wendling <morbo@google.com>
---
 x86/setjmp.c | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/x86/setjmp.c b/x86/setjmp.c
index 976a632..cf9adcb 100644
--- a/x86/setjmp.c
+++ b/x86/setjmp.c
@@ -1,6 +1,10 @@
 #include "libcflat.h"
 #include "setjmp.h"

+#ifdef __clang__
+#pragma clang diagnostic ignored "-Wsometimes-uninitialized"
+#endif
+
 int main(void)
 {
     volatile int i;

Re: [kvm-unit-tests PATCH] x86: setjmp: ignore clang's "-Wsomtimes-uninitialized" flag

[Jim Mattson]

On Mon, Sep 9, 2019 at 2:10 PM Bill Wendling <morbo@google.com> wrote:
>
> Clang complains that "i" might be uninitialized in the "printf"
> statement. This is a false negative, because it's set in the "if"
> statement and then incremented in the loop created by the "longjmp".
>
> Signed-off-by: Bill Wendling <morbo@google.com>
> ---
>  x86/setjmp.c | 4 ++++
>  1 file changed, 4 insertions(+)
>
> diff --git a/x86/setjmp.c b/x86/setjmp.c
> index 976a632..cf9adcb 100644
> --- a/x86/setjmp.c
> +++ b/x86/setjmp.c
> @@ -1,6 +1,10 @@
>  #include "libcflat.h"
>  #include "setjmp.h"
>
> +#ifdef __clang__
> +#pragma clang diagnostic ignored "-Wsometimes-uninitialized"
> +#endif
> +
>  int main(void)
>  {
>      volatile int i;

Can we just add an initializer here instead?

Re: [kvm-unit-tests PATCH] x86: setjmp: ignore clang's "-Wsomtimes-uninitialized" flag

[Sean Christopherson]

On Tue, Sep 10, 2019 at 09:46:36AM -0700, Jim Mattson wrote:
> On Mon, Sep 9, 2019 at 2:10 PM Bill Wendling <morbo@google.com> wrote:
> >
> > Clang complains that "i" might be uninitialized in the "printf"
> > statement. This is a false negative, because it's set in the "if"
> > statement and then incremented in the loop created by the "longjmp".
> >
> > Signed-off-by: Bill Wendling <morbo@google.com>
> > ---
> >  x86/setjmp.c | 4 ++++
> >  1 file changed, 4 insertions(+)
> >
> > diff --git a/x86/setjmp.c b/x86/setjmp.c
> > index 976a632..cf9adcb 100644
> > --- a/x86/setjmp.c
> > +++ b/x86/setjmp.c
> > @@ -1,6 +1,10 @@
> >  #include "libcflat.h"
> >  #include "setjmp.h"
> >
> > +#ifdef __clang__
> > +#pragma clang diagnostic ignored "-Wsometimes-uninitialized"
> > +#endif
> > +
> >  int main(void)
> >  {
> >      volatile int i;
> 
> Can we just add an initializer here instead?

Doing so would also be a good opportunity to actually report on the
expected vs. actual value of 'i' instead of printing numbers that are
meaningless without diving into the code.

Re: [kvm-unit-tests PATCH] x86: setjmp: ignore clang's "-Wsomtimes-uninitialized" flag

[Bill Wendling]

On Tue, Sep 10, 2019 at 12:59 PM Sean Christopherson
<sean.j.christopherson@intel.com> wrote:
>
> On Tue, Sep 10, 2019 at 09:46:36AM -0700, Jim Mattson wrote:
> > On Mon, Sep 9, 2019 at 2:10 PM Bill Wendling <morbo@google.com> wrote:
> > >
> > > Clang complains that "i" might be uninitialized in the "printf"
> > > statement. This is a false negative, because it's set in the "if"
> > > statement and then incremented in the loop created by the "longjmp".
> > >
> > > Signed-off-by: Bill Wendling <morbo@google.com>
> > > ---
> > >  x86/setjmp.c | 4 ++++
> > >  1 file changed, 4 insertions(+)
> > >
> > > diff --git a/x86/setjmp.c b/x86/setjmp.c
> > > index 976a632..cf9adcb 100644
> > > --- a/x86/setjmp.c
> > > +++ b/x86/setjmp.c
> > > @@ -1,6 +1,10 @@
> > >  #include "libcflat.h"
> > >  #include "setjmp.h"
> > >
> > > +#ifdef __clang__
> > > +#pragma clang diagnostic ignored "-Wsometimes-uninitialized"
> > > +#endif
> > > +
> > >  int main(void)
> > >  {
> > >      volatile int i;
> >
> > Can we just add an initializer here instead?
>
> Doing so would also be a good opportunity to actually report on the
> expected vs. actual value of 'i' instead of printing numbers that are
> meaningless without diving into the code.

My initial thought about adding an initializer was that the original
test wanted to ensure that "i" was initialized after the "setjmp"
call. But if we report the expected/actual value instead it wouldn't
be an issue as we can set it to something not expected, etc... I'll
create a patch.

-bw

Re: [kvm-unit-tests PATCH] x86: setjmp: ignore clang's "-Wsomtimes-uninitialized" flag

[Bill Wendling]

I sent out a separate patch. PTAL. :-)

On Tue, Sep 10, 2019 at 8:42 PM Bill Wendling <morbo@google.com> wrote:
>
> On Tue, Sep 10, 2019 at 12:59 PM Sean Christopherson
> <sean.j.christopherson@intel.com> wrote:
> >
> > On Tue, Sep 10, 2019 at 09:46:36AM -0700, Jim Mattson wrote:
> > > On Mon, Sep 9, 2019 at 2:10 PM Bill Wendling <morbo@google.com> wrote:
> > > >
> > > > Clang complains that "i" might be uninitialized in the "printf"
> > > > statement. This is a false negative, because it's set in the "if"
> > > > statement and then incremented in the loop created by the "longjmp".
> > > >
> > > > Signed-off-by: Bill Wendling <morbo@google.com>
> > > > ---
> > > >  x86/setjmp.c | 4 ++++
> > > >  1 file changed, 4 insertions(+)
> > > >
> > > > diff --git a/x86/setjmp.c b/x86/setjmp.c
> > > > index 976a632..cf9adcb 100644
> > > > --- a/x86/setjmp.c
> > > > +++ b/x86/setjmp.c
> > > > @@ -1,6 +1,10 @@
> > > >  #include "libcflat.h"
> > > >  #include "setjmp.h"
> > > >
> > > > +#ifdef __clang__
> > > > +#pragma clang diagnostic ignored "-Wsometimes-uninitialized"
> > > > +#endif
> > > > +
> > > >  int main(void)
> > > >  {
> > > >      volatile int i;
> > >
> > > Can we just add an initializer here instead?
> >
> > Doing so would also be a good opportunity to actually report on the
> > expected vs. actual value of 'i' instead of printing numbers that are
> > meaningless without diving into the code.
>
> My initial thought about adding an initializer was that the original
> test wanted to ensure that "i" was initialized after the "setjmp"
> call. But if we report the expected/actual value instead it wouldn't
> be an issue as we can set it to something not expected, etc... I'll
> create a patch.
>
> -bw