gcc 11 weird bug

[Brian Inglis]

On 2021-10-27 03:19, Thomas Wolff wrote:
> I noticed that mintty did not compile anymore after upgrade to gcc 11, 
> but only on cygwin 32-bit.
> I tried to minimize the test case as much as possible without having the 
> bug vanish, to the attached standalone file.
> Compile this with
> cc -O2 -Wall -Werror m0.c
> and it gives a false positive warning about possible uninitialized data 
> usage.
> While data flow analysis is not perfect, it is weird that this used to 
> happen on 32 bit but not on 64 bit.
> Meanwhile, after updating some other packages (not sure which), but 
> still the same gcc version, the report on the test case also happens on 
> 64 bit, while the original, unstripped file, as part of mintty, still 
> works without error on 64 bit, which is even weirder.
> I have not yet had the opportunity to test this on Linux, sorry, so I'm 
> reporting it here.

Your initialization loops all have i = 0; i < count; which may leave 
types[0] uninitialized.

You should also add -Wextra to your compiles to get these warnings:

$ gcc -g -O2 -Wall -Wextra -c m0.c
m0.c: In function ‘do_bidi’:
m0.c:40:14: warning: unused parameter ‘autodir’ [-Wunused-parameter]
     40 | do_bidi(bool autodir, int paragraphLevel, bool explicitRTL, 
bool box_mirror,
        |              ^
m0.c:40:66: warning: unused parameter ‘box_mirror’ [-Wunused-parameter]
     40 | do_bidi(bool autodir, int paragraphLevel, bool explicitRTL, 
bool box_mirror,
        |                                                                  ^
m0.c:41:21: warning: unused parameter ‘line’ [-Wunused-parameter]
     41 |         bidi_char * line, int count)
        |         ~~~~~~~~~~~~^~~~
m0.c:256:12: warning: ‘*types[0]’ may be used uninitialized 
[-Wmaybe-uninitialized]
    256 |   if (types[0] == NSM /*&& !skip[0]*/)
        |       ~~~~~^~~

with source:

    if (types[0] == NSM /*&& !skip[0]*/)
      types[0] = (paragraphLevel & 1) ? R : L;  // sor

you need to add below:

    int isolateLevel = 0;
    int resLevel = -1;

the following:

    if (!count) {
      return resLevel;
    }

as that seems to be missing, then perhaps after:

   (void)levels;

   types[0] = NSM;

or something appropriate to quiet the warning.

You may also want to look at the control flow through your switch to see 
whether some additional else branches and assignments would ensure the 
values are always set.

You may also want to consider whether adding the following options to 
your gcc command lines would work with your builds:

-fanalyzer -fsanitize-recover=all -fstack-check -fstack-protector-all

as they could give you more warnings about possible issues.

-- 
Take care. Thanks, Brian Inglis, Calgary, Alberta, Canada

This email may be disturbing to some readers as it contains
too much technical detail. Reader discretion is advised.
[Data in binary units and prefixes, physical quantities in SI.]