| Message ID | 20250122065031.1321015-1-arnd@kernel.org (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | EDAC/ie31200: work around false positive build warning | expand |
> From: Arnd Bergmann <arnd@kernel.org> > [...] > Subject: [PATCH] EDAC/ie31200: work around false positive build warning > > From: Arnd Bergmann <arnd@arndb.de> > > gcc-14 produces a bogus warning in some configurations: > > drivers/edac/ie31200_edac.c: In function 'ie31200_probe1.isra': > drivers/edac/ie31200_edac.c:412:26: error: 'dimm_info' is used uninitialized > [-Werror=uninitialized] > 412 | struct dimm_data > dimm_info[IE31200_CHANNELS][IE31200_DIMMS_PER_CHANNEL]; > | ^~~~~~~~~ > drivers/edac/ie31200_edac.c:412:26: note: 'dimm_info' declared here > 412 | struct dimm_data > dimm_info[IE31200_CHANNELS][IE31200_DIMMS_PER_CHANNEL]; > | ^~~~~~~~~ > > I don't see any way the unintialized access could really happen here, but I can It looks like gcc-14 isn't smart enough to recognize that this is a false positive build warning :-). Actually, the dim_info[][] array is always initialized by populate_dimm_info(). > see why the compiler gets confused by the two loops. > > Instead, rework the two nested loops to only read the addr_decode registers > and then keep only one instance of the dimm info structure. > > Signed-off-by: Arnd Bergmann <arnd@arndb.de> > --- > drivers/edac/ie31200_edac.c | 27 +++++++++++++-------------- > 1 file changed, 13 insertions(+), 14 deletions(-) > > diff --git a/drivers/edac/ie31200_edac.c b/drivers/edac/ie31200_edac.c index > 4fc16922dc1a..b0ccc54dc66a 100644 > --- a/drivers/edac/ie31200_edac.c > +++ b/drivers/edac/ie31200_edac.c > @@ -409,10 +409,9 @@ static int ie31200_probe1(struct pci_dev *pdev, int > dev_idx) > int i, j, ret; > struct mem_ctl_info *mci = NULL; > struct edac_mc_layer layers[2]; > - struct dimm_data > dimm_info[IE31200_CHANNELS][IE31200_DIMMS_PER_CHANNEL]; > void __iomem *window; > struct ie31200_priv *priv; > - u32 addr_decode, mad_offset; > + u32 addr_decode[IE31200_CHANNELS], mad_offset; > > /* > * Kaby Lake, Coffee Lake seem to work like Skylake. Please re-visit > @@ -472,17 +471,9 @@ static int ie31200_probe1(struct pci_dev *pdev, int > dev_idx) > > /* populate DIMM info */ This comment also needs to be moved to just above the new location of populate_dimm_info(). > for (i = 0; i < IE31200_CHANNELS; i++) { > - addr_decode = readl(window + mad_offset + > + addr_decode[i] = readl(window + mad_offset + > (i * 4)); > - edac_dbg(0, "addr_decode: 0x%x\n", addr_decode); > - for (j = 0; j < IE31200_DIMMS_PER_CHANNEL; j++) { > - populate_dimm_info(&dimm_info[i][j], addr_decode, > j, > - skl); > - edac_dbg(0, "size: 0x%x, rank: %d, width: %d\n", > - dimm_info[i][j].size, > - dimm_info[i][j].dual_rank, > - dimm_info[i][j].x16_width); > - } > + edac_dbg(0, "addr_decode: 0x%x\n", addr_decode[i]); > } > > /* > @@ -493,14 +484,22 @@ static int ie31200_probe1(struct pci_dev *pdev, int > dev_idx) > */ > for (i = 0; i < IE31200_DIMMS_PER_CHANNEL; i++) { > for (j = 0; j < IE31200_CHANNELS; j++) { > + struct dimm_data dimm_info; > struct dimm_info *dimm; > unsigned long nr_pages; > > - nr_pages = IE31200_PAGES(dimm_info[j][i].size, skl); Move the comment here. > + populate_dimm_info(&dimm_info, addr_decode[j], i, > + skl); > + edac_dbg(0, "size: 0x%x, rank: %d, width: %d\n", > + dimm_info.size, > + dimm_info.dual_rank, > + dimm_info.x16_width); > + > + nr_pages = IE31200_PAGES(dimm_info.size, skl); > if (nr_pages == 0) > continue; > > - if (dimm_info[j][i].dual_rank) { > + if (dimm_info.dual_rank) { > nr_pages = nr_pages / 2; > dimm = edac_get_dimm(mci, (i * 2) + 1, j, 0); > dimm->nr_pages = nr_pages; > -- > 2.39.5 >
On 1/22/25 9:18 AM, Zhuo, Qiuxu wrote: > !-------------------------------------------------------------------| > This Message Is From an External Sender > This message came from outside your organization. > |-------------------------------------------------------------------! > >> From: Arnd Bergmann <arnd@kernel.org> >> [...] >> Subject: [PATCH] EDAC/ie31200: work around false positive build warning >> >> From: Arnd Bergmann <arnd@arndb.de> >> >> gcc-14 produces a bogus warning in some configurations: >> >> drivers/edac/ie31200_edac.c: In function 'ie31200_probe1.isra': >> drivers/edac/ie31200_edac.c:412:26: error: 'dimm_info' is used uninitialized >> [-Werror=uninitialized] >> 412 | struct dimm_data >> dimm_info[IE31200_CHANNELS][IE31200_DIMMS_PER_CHANNEL]; >> | ^~~~~~~~~ >> drivers/edac/ie31200_edac.c:412:26: note: 'dimm_info' declared here >> 412 | struct dimm_data >> dimm_info[IE31200_CHANNELS][IE31200_DIMMS_PER_CHANNEL]; >> | ^~~~~~~~~ >> >> I don't see any way the unintialized access could really happen here, but I can > > It looks like gcc-14 isn't smart enough to recognize that this is a false positive build warning :-). > Actually, the dim_info[][] array is always initialized by populate_dimm_info(). > >> see why the compiler gets confused by the two loops. >> >> Instead, rework the two nested loops to only read the addr_decode registers >> and then keep only one instance of the dimm info structure. >> >> Signed-off-by: Arnd Bergmann <arnd@arndb.de> Seems fine with the comment adjustment. Thanks. Acked-by: Jason Baron <jbaron@akamai.com> >> --- >> drivers/edac/ie31200_edac.c | 27 +++++++++++++-------------- >> 1 file changed, 13 insertions(+), 14 deletions(-) >> >> diff --git a/drivers/edac/ie31200_edac.c b/drivers/edac/ie31200_edac.c index >> 4fc16922dc1a..b0ccc54dc66a 100644 >> --- a/drivers/edac/ie31200_edac.c >> +++ b/drivers/edac/ie31200_edac.c >> @@ -409,10 +409,9 @@ static int ie31200_probe1(struct pci_dev *pdev, int >> dev_idx) >> int i, j, ret; >> struct mem_ctl_info *mci = NULL; >> struct edac_mc_layer layers[2]; >> - struct dimm_data >> dimm_info[IE31200_CHANNELS][IE31200_DIMMS_PER_CHANNEL]; >> void __iomem *window; >> struct ie31200_priv *priv; >> - u32 addr_decode, mad_offset; >> + u32 addr_decode[IE31200_CHANNELS], mad_offset; >> >> /* >> * Kaby Lake, Coffee Lake seem to work like Skylake. Please re-visit >> @@ -472,17 +471,9 @@ static int ie31200_probe1(struct pci_dev *pdev, int >> dev_idx) >> >> /* populate DIMM info */ > > This comment also needs to be moved to just above the new location of populate_dimm_info(). > >> for (i = 0; i < IE31200_CHANNELS; i++) { >> - addr_decode = readl(window + mad_offset + >> + addr_decode[i] = readl(window + mad_offset + >> (i * 4)); >> - edac_dbg(0, "addr_decode: 0x%x\n", addr_decode); >> - for (j = 0; j < IE31200_DIMMS_PER_CHANNEL; j++) { >> - populate_dimm_info(&dimm_info[i][j], addr_decode, >> j, >> - skl); >> - edac_dbg(0, "size: 0x%x, rank: %d, width: %d\n", >> - dimm_info[i][j].size, >> - dimm_info[i][j].dual_rank, >> - dimm_info[i][j].x16_width); >> - } >> + edac_dbg(0, "addr_decode: 0x%x\n", addr_decode[i]); >> } >> >> /* >> @@ -493,14 +484,22 @@ static int ie31200_probe1(struct pci_dev *pdev, int >> dev_idx) >> */ >> for (i = 0; i < IE31200_DIMMS_PER_CHANNEL; i++) { >> for (j = 0; j < IE31200_CHANNELS; j++) { >> + struct dimm_data dimm_info; >> struct dimm_info *dimm; >> unsigned long nr_pages; >> >> - nr_pages = IE31200_PAGES(dimm_info[j][i].size, skl); > > Move the comment here. > >> + populate_dimm_info(&dimm_info, addr_decode[j], i, >> + skl); >> + edac_dbg(0, "size: 0x%x, rank: %d, width: %d\n", >> + dimm_info.size, >> + dimm_info.dual_rank, >> + dimm_info.x16_width); >> + >> + nr_pages = IE31200_PAGES(dimm_info.size, skl); >> if (nr_pages == 0) >> continue; >> >> - if (dimm_info[j][i].dual_rank) { >> + if (dimm_info.dual_rank) { >> nr_pages = nr_pages / 2; >> dimm = edac_get_dimm(mci, (i * 2) + 1, j, 0); >> dimm->nr_pages = nr_pages; >> -- >> 2.39.5 >> >
diff --git a/drivers/edac/ie31200_edac.c b/drivers/edac/ie31200_edac.c index 4fc16922dc1a..b0ccc54dc66a 100644 --- a/drivers/edac/ie31200_edac.c +++ b/drivers/edac/ie31200_edac.c @@ -409,10 +409,9 @@ static int ie31200_probe1(struct pci_dev *pdev, int dev_idx) int i, j, ret; struct mem_ctl_info *mci = NULL; struct edac_mc_layer layers[2]; - struct dimm_data dimm_info[IE31200_CHANNELS][IE31200_DIMMS_PER_CHANNEL]; void __iomem *window; struct ie31200_priv *priv; - u32 addr_decode, mad_offset; + u32 addr_decode[IE31200_CHANNELS], mad_offset; /* * Kaby Lake, Coffee Lake seem to work like Skylake. Please re-visit @@ -472,17 +471,9 @@ static int ie31200_probe1(struct pci_dev *pdev, int dev_idx) /* populate DIMM info */ for (i = 0; i < IE31200_CHANNELS; i++) { - addr_decode = readl(window + mad_offset + + addr_decode[i] = readl(window + mad_offset + (i * 4)); - edac_dbg(0, "addr_decode: 0x%x\n", addr_decode); - for (j = 0; j < IE31200_DIMMS_PER_CHANNEL; j++) { - populate_dimm_info(&dimm_info[i][j], addr_decode, j, - skl); - edac_dbg(0, "size: 0x%x, rank: %d, width: %d\n", - dimm_info[i][j].size, - dimm_info[i][j].dual_rank, - dimm_info[i][j].x16_width); - } + edac_dbg(0, "addr_decode: 0x%x\n", addr_decode[i]); } /* @@ -493,14 +484,22 @@ static int ie31200_probe1(struct pci_dev *pdev, int dev_idx) */ for (i = 0; i < IE31200_DIMMS_PER_CHANNEL; i++) { for (j = 0; j < IE31200_CHANNELS; j++) { + struct dimm_data dimm_info; struct dimm_info *dimm; unsigned long nr_pages; - nr_pages = IE31200_PAGES(dimm_info[j][i].size, skl); + populate_dimm_info(&dimm_info, addr_decode[j], i, + skl); + edac_dbg(0, "size: 0x%x, rank: %d, width: %d\n", + dimm_info.size, + dimm_info.dual_rank, + dimm_info.x16_width); + + nr_pages = IE31200_PAGES(dimm_info.size, skl); if (nr_pages == 0) continue; - if (dimm_info[j][i].dual_rank) { + if (dimm_info.dual_rank) { nr_pages = nr_pages / 2; dimm = edac_get_dimm(mci, (i * 2) + 1, j, 0); dimm->nr_pages = nr_pages;