Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Uninitialized warning when building testsuite #55

Open
bryceharrington opened this issue Oct 3, 2024 · 3 comments
Open

Uninitialized warning when building testsuite #55

bryceharrington opened this issue Oct 3, 2024 · 3 comments

Comments

@bryceharrington
Copy link

In building the testsuite on a few different architectures, I noticed a warning on s390x that might be worth consideration:

221s sg_scat_gath.cpp: In member function ‘int scat_gath_list::append_1or(int64_t, int64_t)’:
221s sg_scat_gath.cpp:603:23: warning: ‘sge.scat_gath_elem::lba’ may be used uninitialized [-Wmaybe-uninitialized]
221s 603 | high_lba_p1 = sge.lba + sge.num;
221s | ~~~~^~~
221s sg_scat_gath.cpp:563:26: note: ‘sge’ declared here
221s 563 | class scat_gath_elem sge;
221s | ^~~
221s sg_scat_gath.cpp:603:33: warning: ‘sge.scat_gath_elem::num’ may be used uninitialized [-Wmaybe-uninitialized]
221s 603 | high_lba_p1 = sge.lba + sge.num;
221s | ~~~~^~~
221s sg_scat_gath.cpp:563:26: note: ‘sge’ declared here
221s 563 | class scat_gath_elem sge;
221s | ^~~
221s sg_scat_gath.cpp:603:23: warning: ‘sge.scat_gath_elem::lba’ may be used uninitialized [-Wmaybe-uninitialized]
221s 603 | high_lba_p1 = sge.lba + sge.num;
221s | ~~~~^~~
221s sg_scat_gath.cpp:563:26: note: ‘sge’ declared here
221s 563 | class scat_gath_elem sge;
221s | ^~~
221s sg_scat_gath.cpp:603:33: warning: ‘sge.scat_gath_elem::num’ may be used uninitialized [-Wmaybe-uninitialized]
221s 603 | high_lba_p1 = sge.lba + sge.num;
221s | ~~~~^~~
221s sg_scat_gath.cpp:563:26: note: ‘sge’ declared here
221s 563 | class scat_gath_elem sge;
221s | ^~~
221s sg_scat_gath.cpp:603:23: warning: ‘sge.scat_gath_elem::lba’ may be used uninitialized [-Wmaybe-uninitialized]
221s 603 | high_lba_p1 = sge.lba + sge.num;
221s | ~~~~^~~
221s sg_scat_gath.cpp:563:26: note: ‘sge’ declared here
221s 563 | class scat_gath_elem sge;
221s | ^~~
221s sg_scat_gath.cpp:603:33: warning: ‘sge.scat_gath_elem::num’ may be used uninitialized [-Wmaybe-uninitialized]
221s 603 | high_lba_p1 = sge.lba + sge.num;
221s | ~~~~^~~
221s sg_scat_gath.cpp:563:26: note: ‘sge’ declared here
221s 563 | class scat_gath_elem sge;
221s | ^~~

I'm not entirely sure why this warning shows only on s390x, but the logic in logic in scat_gath_list::append_1or() it looks like it takes INT32_MAX into consideration, so perhaps the value on s390x is causing a clause to be skipped?

I was able to work around it by manually initializing the flagged structure members, but this may be only papering over whatever the real issue is:

@@ -562,6 +562,8 @@ scat_gath_list::append_1or(int64_t extra_blks, int64_t start_lba)
const int max_nbs = MAX_SGL_NUM_VAL;
int64_t cnt = 0;
class scat_gath_elem sge;

  • sge.lba = 0;

  • sge.num = 0;

    if ((extra_blks <= 0) && (start_lba < 0))
    return o_num; /* nothing to do */

@mwilck
Copy link

mwilck commented Oct 4, 2024

Are you using the same compiler on s390 and other architectures?

From my experience, these warnings are often false positives, and vary between compilers and versions of the same compiler. They can also depend on header files, as you conjectured already. "Papering over" is often a reasonable approach.

In this specific case, I believe that the scat_gath_list() constructor in sg_scat_gath.h is simply missing initializers for some elements. Anyway, this code is just part of the unit tests, so unless you observe test failures, it's perhaps not so important after all.

@bryceharrington
Copy link
Author

It should be the same compiler, yes.
We ended up deciding not to run this testsuite as part of Ubuntu's integration CI, so this issue doesn't hinder us and we won't be carrying this as a patch. But I did want to mention it to you in case it would be worth correcting in the upstream code, as the initialization looked missing to me as well, for this particular situation.

@mwilck
Copy link

mwilck commented Oct 4, 2024

Note that I'm listening here but I'm not the maintainer and I can't commit to this repo.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants