error: 'XXX' may be used uninitialized in this function

error: 'XXX' may be used uninitialized in this function

[Michal Nowak]

Hello,

I work on updating mandoc to 1.14.5 on illumos and there are few errors 
(warnings treated as errors) when building mandoc with GCC 4.4.4 and 7.4.0:

+ /opt/gcc/4.4.4/bin/gcc -fident -finline -fno-inline-functions 
-fno-builtin -fno-asm -fdiagnostics-show-option -nodefaultlibs -D__sun 
-O -m32 -Wall -Wextra -Werror -Wno-missing-braces -Wno-sign-compare 
-Wno-unknown-pragmas -Wno-unused-parameter 
-Wno-missing-field-initializers -Wno-array-bounds -std=gnu99 
-fno-inline-small-functions -fno-inline-functions-called-once 
-fno-ipa-cp -D_TS_ERRNO -DOSNAME="illumos" -D_FILE_OFFSET_BITS=64 
-nostdinc -I. -include fts.h -I/usr/include -c -o mdoc_term.o 
/export/home/newman/ws/oi-userland/components/openindiana/illumos-gate/illumos-gate/usr/src/cmd/mandoc/mdoc_term.c
cc1: warnings being treated as errors
/export/home/newman/ws/oi-userland/components/openindiana/illumos-gate/illumos-gate/usr/src/cmd/mandoc/mdoc_term.c: 
In function 'print_mdoc_node':
/export/home/newman/ws/oi-userland/components/openindiana/illumos-gate/illumos-gate/usr/src/cmd/mandoc/mdoc_term.c:310: 
error: 'act' may be used uninitialized in this function [-Wuninitialized]

+ /usr/gcc/7/bin/gcc -fident -finline -fno-inline-functions -fno-builtin 
-fno-asm -fdiagnostics-show-option -nodefaultlibs -D__sun -O -m32 -Wall 
-Wextra -Werror -Wno-missing-braces -Wno-sign-compare 
-Wno-unknown-pragmas -Wno-unused-parameter 
-Wno-missing-field-initializers -Wno-array-bounds -std=gnu99 
-fno-inline-small-functions -fno-inline-functions-called-once 
-fno-ipa-cp -fno-ipa-icf -fno-clone-functions -fno-reorder-functions 
-D_TS_ERRNO -DOSNAME="illumos" -D_FILE_OFFSET_BITS=64 -nostdinc -I. 
-include fts.h -I/usr/include -c -o 
/tmp/nightly.tmpdir.3713/cw.YdaGE4/cwYdaWE4.o 
/export/home/newman/ws/oi-userland/components/openindiana/illumos-gate/illumos-gate/usr/src/cmd/mandoc/mdoc_term.c
/export/home/newman/ws/oi-userland/components/openindiana/illumos-gate/illumos-gate/usr/src/cmd/mandoc/mdoc_term.c: 
In function 'print_mdoc_node':
/export/home/newman/ws/oi-userland/components/openindiana/illumos-gate/illumos-gate/usr/src/cmd/mandoc/mdoc_term.c:418:10: 
error: 'act' may be used uninitialized in this function 
[-Werror=maybe-uninitialized]
    if (act->post == NULL || n->flags & NODE_ENDED)
        ~~~^~~~~~
cc1: all warnings being treated as errors
*** Error code 1

+ /opt/gcc/4.4.4/bin/gcc -fident -finline -fno-inline-functions 
-fno-builtin -fno-asm -fdiagnostics-show-option -nodefaultlibs -D__sun 
-O -m32 -Wall -Wextra -Werror -Wno-missing-braces -Wno-sign-compare 
-Wno-unknown-pragmas -Wno-unused-parameter 
-Wno-missing-field-initializers -Wno-array-bounds -std=gnu99 
-fno-inline-small-functions -fno-inline-functions-called-once 
-fno-ipa-cp -D_TS_ERRNO -DOSNAME="illumos" -D_FILE_OFFSET_BITS=64 
-nostdinc -I. -include fts.h -I/usr/include -c -o read.o 
/export/home/newman/ws/oi-userland/components/openindiana/illumos-gate/illumos-gate/usr/src/cmd/mandoc/read.c
cc1: warnings being treated as errors
/export/home/newman/ws/oi-userland/components/openindiana/illumos-gate/illumos-gate/usr/src/cmd/mandoc/read.c: 
In function 'mparse_buf_r':
/export/home/newman/ws/oi-userland/components/openindiana/illumos-gate/illumos-gate/usr/src/cmd/mandoc/read.c:147: 
error: 'lastln' may be used uninitialized in this function [-Wuninitialized]

+ /opt/gcc/4.4.4/bin/gcc -fident -finline -fno-inline-functions 
-fno-builtin -fno-asm -fdiagnostics-show-option -nodefaultlibs -D__sun 
-O -m32 -Wall -Wextra -Werror -Wno-missing-braces -Wno-sign-compare 
-Wno-unknown-pragmas -Wno-unused-parameter 
-Wno-missing-field-initializers -Wno-array-bounds -std=gnu99 
-fno-inline-small-functions -fno-inline-functions-called-once 
-fno-ipa-cp -D_TS_ERRNO -DOSNAME="illumos" -D_FILE_OFFSET_BITS=64 
-nostdinc -I. -include fts.h -I/usr/include -c -o term.o 
/export/home/newman/ws/oi-userland/components/openindiana/illumos-gate/illumos-gate/usr/src/cmd/mandoc/term.c
cc1: warnings being treated as errors
/export/home/newman/ws/oi-userland/components/openindiana/illumos-gate/illumos-gate/usr/src/cmd/mandoc/term.c: 
In function 'term_fill':
/export/home/newman/ws/oi-userland/components/openindiana/illumos-gate/illumos-gate/usr/src/cmd/mandoc/term.c:258: 
error: 'vn' may be used uninitialized in this function [-Wuninitialized]

+ /usr/gcc/7/bin/gcc -fident -finline -fno-inline-functions -fno-builtin 
-fno-asm -fdiagnostics-show-option -nodefaultlibs -D__sun -O -m32 -Wall 
-Wextra -Werror -Wno-missing-braces -Wno-sign-compare 
-Wno-unknown-pragmas -Wno-unused-parameter 
-Wno-missing-field-initializers -Wno-array-bounds -std=gnu99 
-fno-inline-small-functions -fno-inline-functions-called-once 
-fno-ipa-cp -fno-ipa-icf -fno-clone-functions -fno-reorder-functions 
-D_TS_ERRNO -DOSNAME="illumos" -D_FILE_OFFSET_BITS=64 -nostdinc -I. 
-include fts.h -I/usr/include -c -o 
/tmp/nightly.tmpdir.3713/cw.QPaOX4/cwRPa4X4.o 
/export/home/newman/ws/oi-userland/components/openindiana/illumos-gate/illumos-gate/usr/src/cmd/mandoc/term.c
/export/home/newman/ws/oi-userland/components/openindiana/illumos-gate/illumos-gate/usr/src/cmd/mandoc/term.c: 
In function 'term_fill':
/export/home/newman/ws/oi-userland/components/openindiana/illumos-gate/illumos-gate/usr/src/cmd/mandoc/term.c:286:24: 
error: 'vn' may be used uninitialized in this function 
[-Werror=maybe-uninitialized]
     if (breakline || vn > vtarget)
                      ~~~^~~~~~~~~
cc1: all warnings being treated as errors
*** Error code 1


In all cases I was able to eliminate those errors by initializing 
respective variables to NULL. It seems that none of those errors were 
addressed after the 1.14.5 release.

Michal
--
 To unsubscribe send an email to discuss+unsubscribe@mandoc.bsd.lv

Re: error: 'XXX' may be used uninitialized in this function

[Ingo Schwarze]

Hi Michael,

Michal Nowak wrote on Mon, Jun 03, 2019 at 05:28:28PM +0200:

> I work on updating mandoc to 1.14.5 on illumos

Appreciated, thank you!

> and there are few errors (warnings treated as errors) when building
> mandoc with GCC 4.4.4 and 7.4.0:

[text reordered]
> In all cases I was able to eliminate those errors by initializing 
> respective variables to NULL.

In general, i object to blindly initializing variables without
considering whether it makes sense logically; that is dangerous
behaviour on part of the programmer because it can hide errors in
the code - both existing errors and errors introduced later.

> It seems that none of those errors were 
> addressed after the 1.14.5 release.

No, because no are errors: they are all false positives.

> In function 'print_mdoc_node':
> error: 'act' may be used uninitialized in this function 
> [-Werror=maybe-uninitialized]
>     if (act->post == NULL || n->flags & NODE_ENDED)
>         ~~~^~~~~~

The pointer act is assigned to in the default case of the previous
n->type switch, and it is used only in the default case of the
second n->type switch.

That said, the only way this pointer is used is by dereferencing it.
So (pointlessly) initializing it does no harm because in case a bug
is introduced later, it changes an uninitialized pointer access into
a NULL pointer access which is potentially less dangerous.  So in this
particular case, your suggestion to hide the error is OK.

> In function 'mparse_buf_r':
> /export/home/newman/ws/oi-userland/components/openindiana/illumos-gate/illumos-gate/usr/src/cmd/mandoc/read.c:147: 
> error: 'lastln' may be used uninitialized in this function [-Wuninitialized]

Another false positive: lastln is only used when firstln is already
set, and whenever firstln gets set, lastln is also set at the same
time.  That said, it is logically clear from the purpose of the
variable that initializing lastln is harmless in this case.  Logically,
lastln *should* be NULL as long as firstln is NULL.

> In function 'term_fill':
> /export/home/newman/ws/oi-userland/components/openindiana/illumos-gate/illumos-gate/usr/src/cmd/mandoc/term.c:286:24: 
> error: 'vn' may be used uninitialized in this function 
> [-Werror=maybe-uninitialized]
>      if (breakline || vn > vtarget)

And yet another false positive.  The variable vn is assigned to in
all three cases of the inner switch and only used immediately afterwards.

Apparently, the compiler isn't smart enough to see that the cases
of the inner switch are exhaustive.  Then again, there is no harm
in saying so explicitly.

So, i committed the three patches shown below.

Can you confirm that your gcc no longer complains?

Yours,
  Ingo


Log Message:
-----------
Initialize the local variable "act" in print_mdoc_node().
While there is no bug, it helps clarity, and it is also safer in this
particular code because in case a bug gets introduced later, accessing
a NULL pointer is less dangerous than accessing an uninitialized pointer.

Michal Nowak <mnowak at startmail dot com> reported that gcc 4.4.4 
and 7.4.0 on illumos throw -Wuninitialized false positives.

Modified Files:
--------------
    mandoc:
        mdoc_term.c

Revision Data
-------------
Index: mdoc_term.c
===================================================================
RCS file: /home/cvs/mandoc/mandoc/mdoc_term.c,v
retrieving revision 1.372
retrieving revision 1.373
diff -Lmdoc_term.c -Lmdoc_term.c -u -p -r1.372 -r1.373
--- mdoc_term.c
+++ mdoc_term.c
@@ -352,6 +352,7 @@ print_mdoc_node(DECL_ARGS)
 	 * produce output.  Note that some pre-handlers do so.
 	 */
 
+	act = NULL;
 	switch (n->type) {
 	case ROFFT_TEXT:
 		if (n->flags & NODE_LINE) {


Log Message:
-----------
Initialize the local variable "lastln" in mparse_buf_r().
While there is no bug, it logically makes sense given the meaning
of the variable that lastln is NULL as long as firstln is NULL.

Michal Nowak <mnowak at startmail dot com> reported that gcc 4.4.4
and 7.4.0 on illumos throw -Wuninitialized false positives.

Modified Files:
--------------
    mandoc:
        read.c

Revision Data
-------------
Index: read.c
===================================================================
RCS file: /home/cvs/mandoc/mandoc/read.c,v
retrieving revision 1.212
retrieving revision 1.213
diff -Lread.c -Lread.c -u -p -r1.212 -r1.213
--- read.c
+++ read.c
@@ -157,7 +157,7 @@ mparse_buf_r(struct mparse *curp, struct
 	ln.sz = 256;
 	ln.buf = mandoc_malloc(ln.sz);
 	ln.next = NULL;
-	firstln = loop = NULL;
+	firstln = lastln = loop = NULL;
 	lnn = curp->line;
 	pos = 0;
 	inloop = 0;


Log Message:
-----------
Initialize the local variable "lastln" in mparse_buf_r().
While there is no bug, it logically makes sense given the meaning
of the variable that lastln is NULL as long as firstln is NULL.

Michal Nowak <mnowak at startmail dot com> reported that gcc 4.4.4
and 7.4.0 on illumos throw -Wuninitialized false positives.

Modified Files:
--------------
    mandoc:
        read.c

Revision Data
-------------
Index: read.c
===================================================================
RCS file: /home/cvs/mandoc/mandoc/read.c,v
retrieving revision 1.212
retrieving revision 1.213
diff -Lread.c -Lread.c -u -p -r1.212 -r1.213
--- read.c
+++ read.c
@@ -157,7 +157,7 @@ mparse_buf_r(struct mparse *curp, struct
 	ln.sz = 256;
 	ln.buf = mandoc_malloc(ln.sz);
 	ln.next = NULL;
-	firstln = loop = NULL;
+	firstln = lastln = loop = NULL;
 	lnn = curp->line;
 	pos = 0;
 	inloop = 0;
--
 To unsubscribe send an email to discuss+unsubscribe@mandoc.bsd.lv

Re: error: 'XXX' may be used uninitialized in this function

[Michal Nowak]

Thank you Ingo very much for your comments. I'll go with your patches.

On 06/03/19 10:37 PM, Ingo Schwarze wrote:
> Hi Michael,
> 
> Michal Nowak wrote on Mon, Jun 03, 2019 at 05:28:28PM +0200:
> 
>> I work on updating mandoc to 1.14.5 on illumos
> 
> Appreciated, thank you!
> 
>> and there are few errors (warnings treated as errors) when building
>> mandoc with GCC 4.4.4 and 7.4.0:
> 
> [text reordered]
>> In all cases I was able to eliminate those errors by initializing
>> respective variables to NULL.
> 
> In general, i object to blindly initializing variables without
> considering whether it makes sense logically; that is dangerous
> behaviour on part of the programmer because it can hide errors in
> the code - both existing errors and errors introduced later.
> 
>> It seems that none of those errors were
>> addressed after the 1.14.5 release.
> 
> No, because no are errors: they are all false positives.
> 
>> In function 'print_mdoc_node':
>> error: 'act' may be used uninitialized in this function
>> [-Werror=maybe-uninitialized]
>>      if (act->post == NULL || n->flags & NODE_ENDED)
>>          ~~~^~~~~~
> 
> The pointer act is assigned to in the default case of the previous
> n->type switch, and it is used only in the default case of the
> second n->type switch.
> 
> That said, the only way this pointer is used is by dereferencing it.
> So (pointlessly) initializing it does no harm because in case a bug
> is introduced later, it changes an uninitialized pointer access into
> a NULL pointer access which is potentially less dangerous.  So in this
> particular case, your suggestion to hide the error is OK.
> 
>> In function 'mparse_buf_r':
>> /export/home/newman/ws/oi-userland/components/openindiana/illumos-gate/illumos-gate/usr/src/cmd/mandoc/read.c:147:
>> error: 'lastln' may be used uninitialized in this function [-Wuninitialized]
> 
> Another false positive: lastln is only used when firstln is already
> set, and whenever firstln gets set, lastln is also set at the same
> time.  That said, it is logically clear from the purpose of the
> variable that initializing lastln is harmless in this case.  Logically,
> lastln *should* be NULL as long as firstln is NULL.
> 
>> In function 'term_fill':
>> /export/home/newman/ws/oi-userland/components/openindiana/illumos-gate/illumos-gate/usr/src/cmd/mandoc/term.c:286:24:
>> error: 'vn' may be used uninitialized in this function
>> [-Werror=maybe-uninitialized]
>>       if (breakline || vn > vtarget)
> 
> And yet another false positive.  The variable vn is assigned to in
> all three cases of the inner switch and only used immediately afterwards.
> 
> Apparently, the compiler isn't smart enough to see that the cases
> of the inner switch are exhaustive.  Then again, there is no harm
> in saying so explicitly.
> 
> So, i committed the three patches shown below.
> 
> Can you confirm that your gcc no longer complains?
> 
> Yours,
>    Ingo
> 
> 
> Log Message:
> -----------
> Initialize the local variable "act" in print_mdoc_node().
> While there is no bug, it helps clarity, and it is also safer in this
> particular code because in case a bug gets introduced later, accessing
> a NULL pointer is less dangerous than accessing an uninitialized pointer.
> 
> Michal Nowak <mnowak at startmail dot com> reported that gcc 4.4.4
> and 7.4.0 on illumos throw -Wuninitialized false positives.
> 
> Modified Files:
> --------------
>      mandoc:
>          mdoc_term.c
> 
> Revision Data
> -------------
> Index: mdoc_term.c
> ===================================================================
> RCS file: /home/cvs/mandoc/mandoc/mdoc_term.c,v
> retrieving revision 1.372
> retrieving revision 1.373
> diff -Lmdoc_term.c -Lmdoc_term.c -u -p -r1.372 -r1.373
> --- mdoc_term.c
> +++ mdoc_term.c
> @@ -352,6 +352,7 @@ print_mdoc_node(DECL_ARGS)
>   	 * produce output.  Note that some pre-handlers do so.
>   	 */
>   
> +	act = NULL;
>   	switch (n->type) {
>   	case ROFFT_TEXT:
>   		if (n->flags & NODE_LINE) {
> 
> 
> Log Message:
> -----------
> Initialize the local variable "lastln" in mparse_buf_r().
> While there is no bug, it logically makes sense given the meaning
> of the variable that lastln is NULL as long as firstln is NULL.
> 
> Michal Nowak <mnowak at startmail dot com> reported that gcc 4.4.4
> and 7.4.0 on illumos throw -Wuninitialized false positives.
> 
> Modified Files:
> --------------
>      mandoc:
>          read.c
> 
> Revision Data
> -------------
> Index: read.c
> ===================================================================
> RCS file: /home/cvs/mandoc/mandoc/read.c,v
> retrieving revision 1.212
> retrieving revision 1.213
> diff -Lread.c -Lread.c -u -p -r1.212 -r1.213
> --- read.c
> +++ read.c
> @@ -157,7 +157,7 @@ mparse_buf_r(struct mparse *curp, struct
>   	ln.sz = 256;
>   	ln.buf = mandoc_malloc(ln.sz);
>   	ln.next = NULL;
> -	firstln = loop = NULL;
> +	firstln = lastln = loop = NULL;
>   	lnn = curp->line;
>   	pos = 0;
>   	inloop = 0;
> 
> 
> Log Message:
> -----------
> Initialize the local variable "lastln" in mparse_buf_r().
> While there is no bug, it logically makes sense given the meaning
> of the variable that lastln is NULL as long as firstln is NULL.
> 
> Michal Nowak <mnowak at startmail dot com> reported that gcc 4.4.4
> and 7.4.0 on illumos throw -Wuninitialized false positives.
> 
> Modified Files:
> --------------
>      mandoc:
>          read.c
> 
> Revision Data
> -------------
> Index: read.c
> ===================================================================
> RCS file: /home/cvs/mandoc/mandoc/read.c,v
> retrieving revision 1.212
> retrieving revision 1.213
> diff -Lread.c -Lread.c -u -p -r1.212 -r1.213
> --- read.c
> +++ read.c
> @@ -157,7 +157,7 @@ mparse_buf_r(struct mparse *curp, struct
>   	ln.sz = 256;
>   	ln.buf = mandoc_malloc(ln.sz);
>   	ln.next = NULL;
> -	firstln = loop = NULL;
> +	firstln = lastln = loop = NULL;
>   	lnn = curp->line;
>   	pos = 0;
>   	inloop = 0;
> 

I think you duplicated the read.c patch here and you meant 
https://mandoc.bsd.lv/cgi-bin/cvsweb/term.c.diff?r1=1.280&r2=1.281 (I'll 
go with that one instead).

Thanks again!

Michal
--
 To unsubscribe send an email to discuss+unsubscribe@mandoc.bsd.lv