Prior to the attached patch, there is a problem with realloc on assignment
with kind=4 characters as the string length was compared with the byte size,
which was always true.

I initially thought, looking at the code, that scalars have the same issues,
but they don't; hence, I ended up with a comment and a cleanup.

For arrays: The issue shows up in the testcase (→ PR) because there was
unnecessary reallocation on assignment, which changed the lower bound to 1.

The rest, I found looking at the dump:

(a) cond_null was:
     D.4298 = .a4str != 7 || (character(kind=4)[0:][1:.a4str] *) a4str.data == 0B;
...
           if (D.4298)
               a4str.data = __builtin_malloc (168);
           else
               a4str.data = __builtin_realloc (a4str.data, 168);
which is the wrong condition. It should be just:
           D.4298 = (character(kind=4)[0:][1:.a4str] *) a4str.data == 0B;
to avoid a memory leak.

(b) The rest was removing bogus code; I think it did not do any harm, but makes
the code and the dump rather convoluted.

The dump (with and without the patch) starts with:

           D.4295 = .a4str * 4;
           .a4str = 7;
           D.4298 = (character(kind=4)[0:][1:.a4str] *) a4str.data == 0B;
           if (D.4298) goto L.6;
           if (a4str.dim[0].lbound + 5 != a4str.dim[0].ubound) goto L.6;
           if (D.4295 != 28) goto L.6;
           goto L.7;
           L.6:;
           a4str.dim[0].lbound = 1;
....
           if (D.4298)
               a4str.data = __builtin_malloc (168);
           else
               a4str.data = __builtin_realloc (a4str.data, 168);
           L.7:;

Thus, any code which reaches L.6 should be reallocated and any code
which does not, shouldn't.

The deleted code did add directly after L.6 the following additional code:
         if (D.4298)
             D.4282 = 0;
         else
             D.4282 = MAX_EXPR <a4str.dim[0].ubound - a4str.dim[0].lbound, -1> + 1;
         D.4283 = D.4282 != 6;
and it changed the 'else' into an 'else if' in
           if (D.4298)
               a4str.data = __builtin_malloc (168);
           else if (D.4283)
               a4str.data = __builtin_realloc (a4str.data, 168);

Closely looking at the added condition and at source code, it
does essentially the same check as the code which guarded the L.6 to L.7
code. Thus, the condition should always evaluate as true.

Codewise, the 'D.4282 != 6'  is the 'size1 != size2' array size comparison.

I think it was the now removed code was there before, but then someone
realized the array bounds problem - and the new code was added without
actually removing the old one. The handling of deferred strings both in
the bogus condition for cond_null and by setting 'D.4283' to always true
is not only wrong but implies some early hack.
However, I have not checked the history to confirm my suspicion.

OK for mainline?

Tobias

PS: I have the feeling that there might be an issue with finalization/derived-type
handling in case of 'realloc' as I did not spot finalization code between the size
check and the malloc/realloc. The malloc case should be fine – but if realloc shrinks
the memory, elements beyond the new last element in storage order would access invalid
memory. – However, I have not checked whether there is indeed a problem as
I concentrated on fixing this issue.

PPS: I lost track of pending patches. Are they any which I should review?
-----------------
Siemens Electronic Design Automation GmbH; Anschrift: Arnulfstraße 201, 80634 München; Gesellschaft mit beschränkter Haftung; Geschäftsführer: Thomas Heurung, Frank Thürauf; Sitz der Gesellschaft: München; Registergericht München, HRB 106955