[PATCH for-4.7 0/6] Workaround gcc -Wmaybe-uninitialised false positive reports

[PATCH for-4.7 0/6] Workaround gcc -Wmaybe-uninitialised false positive reports

[Wei Liu]

On my Debian Jessie build box gcc complains about various maybe uninitialised
variables when -g is in use. In fact gcc -Wmaybe-uninitialized is not very
reliable according to gcc manpage, various search results and answer from
someone on freenode #gcc channel.

I go through those failures and try to provide some workaround for them.
Please have a look if these fixes make sense or my analysis is correct.

Wei.

Wei Liu (6):
  rombios/tcgbios: initialise size in tcpa_extend_acpi_log
  rombios/tcgbios: initialise entry in HashLogEvent32
  rombios/tcgbios: initialise logdataptr in HashLogEvent32
  blktap2: initialise buf in vhd_util_check_footer
  blktap2: initialise buf to NULL in img2qcow.c:main
  blktap2: initialise buf in qcow2raw.c:main

 tools/blktap2/drivers/img2qcow.c               | 2 +-
 tools/blktap2/drivers/qcow2raw.c               | 2 +-
 tools/blktap2/vhd/lib/vhd-util-check.c         | 2 +-
 tools/firmware/rombios/32bit/tcgbios/tcgbios.c | 6 +++---
 4 files changed, 6 insertions(+), 6 deletions(-)

-- 
2.1.4


_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel

[PATCH for-4.7 1/6] rombios/tcgbios: initialise size in tcpa_extend_acpi_log

[Wei Liu]

Gcc complains:

tcgbios.c:362:3: error: ‘size’ may be used uninitialized in this function [-Werror=maybe-uninitialized]
   memcpy((char *)lasa_last, (char *)entry_ptr, size);

It fails to figure out if size is used in memcpy it is always initialised.

Signed-off-by: Wei Liu <wei.liu2@citrix.com>
---
 tools/firmware/rombios/32bit/tcgbios/tcgbios.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/tools/firmware/rombios/32bit/tcgbios/tcgbios.c b/tools/firmware/rombios/32bit/tcgbios/tcgbios.c
index beef5a4..d1d1203 100644
--- a/tools/firmware/rombios/32bit/tcgbios/tcgbios.c
+++ b/tools/firmware/rombios/32bit/tcgbios/tcgbios.c
@@ -330,7 +330,7 @@ uint32_t tcpa_extend_acpi_log(uint32_t entry_ptr)
 	uint32_t res = 0;
 	unsigned char *lasa_last = tcpa_get_lasa_last_ptr();
 	unsigned char *lasa_base = tcpa_get_lasa_base_ptr();
-	uint32_t size;
+	uint32_t size = 0;
 	uint16_t entry_count = tcpa_acpi.entry_count;
 	struct pcpes *pcpes = (struct pcpes *)entry_ptr;
 
-- 
2.1.4


_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel

[PATCH for-4.7 2/6] rombios/tcgbios: initialise entry in HashLogEvent32

[Wei Liu]

Gcc complains:

tcgbios.c:1142:22: error: ‘entry’ may be used uninitialized in this function [-Werror=maybe-uninitialized]
    hleo->eventnumber = entry;

It fails to figure out if entry is used it is always initialised in
previous if block.

Signed-off-by: Wei Liu <wei.liu2@citrix.com>
---
 tools/firmware/rombios/32bit/tcgbios/tcgbios.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/tools/firmware/rombios/32bit/tcgbios/tcgbios.c b/tools/firmware/rombios/32bit/tcgbios/tcgbios.c
index d1d1203..3501051 100644
--- a/tools/firmware/rombios/32bit/tcgbios/tcgbios.c
+++ b/tools/firmware/rombios/32bit/tcgbios/tcgbios.c
@@ -1100,7 +1100,7 @@ uint32_t HashLogEvent32(struct hlei *hlei, struct hleo *hleo,
 	}
 
 	if (rc == 0) {
-		uint32_t entry;
+		uint32_t entry = 0;
 		hashdataptr = hlei->hashdataptr;
 		hashdatalen = hlei->hashdatalen;
 
-- 
2.1.4


_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel

[PATCH for-4.7 3/6] rombios/tcgbios: initialise logdataptr in HashLogEvent32

[Wei Liu]

Gcc complains:

tcgbios.c: In function ‘HashLogEvent32’:
tcgbios.c:1131:10: error: ‘logdataptr’ may be used uninitialized in this function [-Werror=maybe-uninitialized]
    entry = tcpa_extend_acpi_log(logdataptr);

It fails to figure out when logdataptr is used it is always initialised
in a if block a few line above.

Signed-off-by: Wei Liu <wei.liu2@citrix.com>
---
 tools/firmware/rombios/32bit/tcgbios/tcgbios.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/tools/firmware/rombios/32bit/tcgbios/tcgbios.c b/tools/firmware/rombios/32bit/tcgbios/tcgbios.c
index 3501051..fa22c44 100644
--- a/tools/firmware/rombios/32bit/tcgbios/tcgbios.c
+++ b/tools/firmware/rombios/32bit/tcgbios/tcgbios.c
@@ -1062,7 +1062,7 @@ uint32_t HashLogEvent32(struct hlei *hlei, struct hleo *hleo,
 {
 	uint32_t rc = 0;
 	uint16_t size;
-	uint32_t logdataptr;
+	uint32_t logdataptr = 0;
 	uint32_t logdatalen;
 	uint32_t hashdataptr;
 	uint32_t hashdatalen;
-- 
2.1.4


_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel

[PATCH for-4.7 4/6] blktap2: initialise buf in vhd_util_check_footer

[Wei Liu]

Gcc complains:

vhd-util-check.c: In function ‘vhd_util_check_footer’:
vhd-util-check.c:413:2: error: ‘buf’ may be used uninitialized in this function [-Werror=maybe-uninitialized]
  memcpy(&backup, buf, sizeof(backup));

In fact buf is initialised a few lines above.

Signed-off-by: Wei Liu <wei.liu2@citrix.com>
---
I can't figure out why buf may be uninitialised and this seems to be the
only way to convince gcc it is ok to proceed.
---
 tools/blktap2/vhd/lib/vhd-util-check.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/tools/blktap2/vhd/lib/vhd-util-check.c b/tools/blktap2/vhd/lib/vhd-util-check.c
index af07426..40565ac 100644
--- a/tools/blktap2/vhd/lib/vhd-util-check.c
+++ b/tools/blktap2/vhd/lib/vhd-util-check.c
@@ -335,7 +335,7 @@ vhd_util_check_footer(int fd, vhd_footer_t *footer, int ignore)
 {
 	size_t size;
 	int err, opened;
-	char *msg, *buf;
+	char *msg, *buf = NULL;
 	off_t eof, off;
 	vhd_footer_t primary, backup;
 
-- 
2.1.4


_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel

[PATCH for-4.7 5/6] blktap2: initialise buf to NULL in img2qcow.c:main

[Wei Liu]

Gcc complains:

qcow2raw.c: In function ‘main’:
qcow2raw.c:387:17: error: ‘buf’ may be used uninitialized in this function [-Werror=maybe-uninitialized]
    treq.buf     = buf;
                 ^

But at the point of that assignment, buf is a valid buffer allocated by
posix_memalign and filled in by read.

Signed-off-by: Wei Liu <wei.liu2@citrix.com>
---
 tools/blktap2/drivers/img2qcow.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/tools/blktap2/drivers/img2qcow.c b/tools/blktap2/drivers/img2qcow.c
index 57b931e..7376382 100644
--- a/tools/blktap2/drivers/img2qcow.c
+++ b/tools/blktap2/drivers/img2qcow.c
@@ -166,7 +166,7 @@ int main(int argc, const char *argv[])
         int ret = -1, fd, len, err;
 	struct timeval timeout;
 	uint64_t i;
-	char *buf;
+	char *buf = NULL;
 	td_request_t treq;
         td_disk_info_t info;
         td_vbd_request_t* vreq;
-- 
2.1.4


_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel

[PATCH for-4.7 6/6] blktap2: initialise buf in qcow2raw.c:main

[Wei Liu]

Gcc complains:

qcow2raw.c: In function ‘main’:
qcow2raw.c:387:17: error: ‘buf’ may be used uninitialized in this function [-Werror=maybe-uninitialized]
    treq.buf     = buf;
                 ^

But buf is a valid buffer allocated by posix_memalign at that point.

Signed-off-by: Wei Liu <wei.liu2@citrix.com>
---
 tools/blktap2/drivers/qcow2raw.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/tools/blktap2/drivers/qcow2raw.c b/tools/blktap2/drivers/qcow2raw.c
index a2e417d..5ad7305 100644
--- a/tools/blktap2/drivers/qcow2raw.c
+++ b/tools/blktap2/drivers/qcow2raw.c
@@ -202,7 +202,7 @@ int main(int argc, const char *argv[])
 	uint64_t size;
 	struct timeval timeout;
 	uint64_t i;
-	char *buf;
+	char *buf = NULL;
 	struct stat finfo;
 	td_request_t treq;
 	td_vbd_request_t* vreq;
-- 
2.1.4


_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel

Re: [PATCH for-4.7 0/6] Workaround gcc -Wmaybe-uninitialised false positive reports

[Wei Liu]

On Fri, Apr 29, 2016 at 04:11:11PM +0100, Wei Liu wrote:
> On my Debian Jessie build box gcc complains about various maybe uninitialised
> variables when -g is in use. In fact gcc -Wmaybe-uninitialized is not very
> reliable according to gcc manpage, various search results and answer from
> someone on freenode #gcc channel.
> 
> I go through those failures and try to provide some workaround for them.
> Please have a look if these fixes make sense or my analysis is correct.
> 
> Wei.
> 
> Wei Liu (6):
>   rombios/tcgbios: initialise size in tcpa_extend_acpi_log
>   rombios/tcgbios: initialise entry in HashLogEvent32
>   rombios/tcgbios: initialise logdataptr in HashLogEvent32
>   blktap2: initialise buf in vhd_util_check_footer
>   blktap2: initialise buf to NULL in img2qcow.c:main
>   blktap2: initialise buf in qcow2raw.c:main
> 

Ping?

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel

Re: [PATCH for-4.7 0/6] Workaround gcc -Wmaybe-uninitialised false positive reports

[Doug Goldstein]

[-- Attachment #1.1.1: Type: text/plain, Size: 1234 bytes --]

On 4/29/16 10:11 AM, Wei Liu wrote:
> On my Debian Jessie build box gcc complains about various maybe uninitialised
> variables when -g is in use. In fact gcc -Wmaybe-uninitialized is not very
> reliable according to gcc manpage, various search results and answer from
> someone on freenode #gcc channel.
> 
> I go through those failures and try to provide some workaround for them.
> Please have a look if these fixes make sense or my analysis is correct.
> 
> Wei.
> 
> Wei Liu (6):
>   rombios/tcgbios: initialise size in tcpa_extend_acpi_log
>   rombios/tcgbios: initialise entry in HashLogEvent32
>   rombios/tcgbios: initialise logdataptr in HashLogEvent32
>   blktap2: initialise buf in vhd_util_check_footer
>   blktap2: initialise buf to NULL in img2qcow.c:main
>   blktap2: initialise buf in qcow2raw.c:main
> 
>  tools/blktap2/drivers/img2qcow.c               | 2 +-
>  tools/blktap2/drivers/qcow2raw.c               | 2 +-
>  tools/blktap2/vhd/lib/vhd-util-check.c         | 2 +-
>  tools/firmware/rombios/32bit/tcgbios/tcgbios.c | 6 +++---
>  4 files changed, 6 insertions(+), 6 deletions(-)
> 

For the whole series:

Reviewed-by: Doug Goldstein <cardoe@cardoe.com>

-- 
Doug Goldstein


[-- Attachment #1.2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 959 bytes --]

[-- Attachment #2: Type: text/plain, Size: 126 bytes --]

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel

Re: [PATCH for-4.7 0/6] Workaround gcc -Wmaybe-uninitialised false positive reports

[Wei Liu]

On Fri, May 06, 2016 at 08:37:31AM -0500, Doug Goldstein wrote:
> On 4/29/16 10:11 AM, Wei Liu wrote:
> > On my Debian Jessie build box gcc complains about various maybe uninitialised
> > variables when -g is in use. In fact gcc -Wmaybe-uninitialized is not very
> > reliable according to gcc manpage, various search results and answer from
> > someone on freenode #gcc channel.
> > 
> > I go through those failures and try to provide some workaround for them.
> > Please have a look if these fixes make sense or my analysis is correct.
> > 
> > Wei.
> > 
> > Wei Liu (6):
> >   rombios/tcgbios: initialise size in tcpa_extend_acpi_log
> >   rombios/tcgbios: initialise entry in HashLogEvent32
> >   rombios/tcgbios: initialise logdataptr in HashLogEvent32
> >   blktap2: initialise buf in vhd_util_check_footer
> >   blktap2: initialise buf to NULL in img2qcow.c:main
> >   blktap2: initialise buf in qcow2raw.c:main
> > 
> >  tools/blktap2/drivers/img2qcow.c               | 2 +-
> >  tools/blktap2/drivers/qcow2raw.c               | 2 +-
> >  tools/blktap2/vhd/lib/vhd-util-check.c         | 2 +-
> >  tools/firmware/rombios/32bit/tcgbios/tcgbios.c | 6 +++---
> >  4 files changed, 6 insertions(+), 6 deletions(-)
> > 
> 
> For the whole series:
> 
> Reviewed-by: Doug Goldstein <cardoe@cardoe.com>
> 

Thanks.

I chatted with Ian on IRC he said he didn't quite like this approach but
didn't have better suggestion so I should go ahead.

I've queued this series up for committing.

Wei.

> -- 
> Doug Goldstein
> 




_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel