| Message ID | CAGG=3QWNQKejpwhbgDy-WSV1C2sw9Ms0TUGwVk8fgEbg9n0ryg@mail.gmail.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | [kvm-unit-tests] x86: setjmp: ignore clang's "-Wsomtimes-uninitialized" flag | expand |
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?
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.
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
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
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;
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(+)