Use of uninitialised data in tc-sh.c

[Andrew STUBBS <andrew dot stubbs at st dot com>]

Hi Alexandre,

I have tested the assembler under valgrind and come up with the following
error:

==7730== Conditional jump or move depends on uninitialised value(s)
==7730==    at 0x8064D80: get_specific (src/gas/config/tc-sh.c:1576)
==7730==    by 0x80667A5: md_assemble (src/gas/config/tc-sh.c:2852)
==7730==    by 0x8058169: read_a_source_file (src/gas/read.c:889)
==7730==    by 0x804BB15: perform_an_assembly_pass (src/gas/as.c:1054)

Which corresponds to the code below:

          if (SH_MERGE_ARCH_SET_VALID (valid_arch, arch_sh2a_nofpu_up)
              && (   arg == A_DISP_REG_M
                  || arg == A_DISP_REG_N))
            {
              /* Check a few key IMM* fields for overflow.  */
              int opf;
              long val = user->immediate.X_add_number;

              for (opf = 0; opf < 4; opf ++)
                switch (this_try->nibbles[opf])
                  {
                  case IMM0_4:
                  case IMM1_4:
1576 ->             if (val < 0 || val > 15)
                      goto fail;
                    break;
                  case IMM0_4BY2:
                  case IMM1_4BY2:
                    if (val < 0 || val > 15 * 2)
                      goto fail;
                    break;
                  case IMM0_4BY4:
                  case IMM1_4BY4:
                    if (val < 0 || val > 15 * 4)
                      goto fail;
                    break;
                  default:
                    break;
                  }
            }

There are similar failures for the other two if statements.

This code was introduced during your recent SH-2A patch. The problem is that
the code is testing the 'immediate' field even when the operand is not of
immediate type, and thus when the immediate field has not been written.

It is not entirely clear to me what this code is trying to achieve. For
example, why is this range checking sh2a specific? And why only a few 'key'
fields? And which are the key ones anyway?

Please could you review what your patch was intended to do and sort it out.
It appears to be harmless (I haven't seen any valid programs rejected), but
I can't prove it to my satisfaction.

P.S. I think the type of 'val' really ought to be 'long long', to match the
type of 'X_add_number'.

Thanks

--
Andrew Stubbs
andrew.stubbs@st.com
andrew.stubbs@superh.com