[coreboot] Ouch: romcc "x[0] |= something" patch causes another crash

[Eric W. Biederman]

Stefan Reinauer <stepan at coresystems.de> writes:

> On 3/15/10 10:59 AM, Patrick Georgi wrote:
>
>     Am 15.03.2010 03:32, schrieb Keith Hui:
>
>
>         > Hi all,
>         >
>         > I regret to report that the romcc patch circulated earlier to fix the
>         > segfault I reported, is now causing another segfault. This also seems to
>         > be triggered by something in the 440BX code, as it didn't segfault when
>         > I compile for any mainboards that isn't 440BX. As of now I don't know
>         > what this new segfault is. I'll report back with more findings.
>
>
>     It seems the problem was that copy_triple() isn't supposed to be used on
>     flattened (and simple) nodes.
>     I built a simple test case that failed:
>     void main(void) {
>             int c = 0;
>             c |= 4;
>     }
>
>     With the attached patch, this testcase, your testcase, and a full abuild
>     run work.
>
>     Signed-off-by: Patrick Georgi <patrick.georgi at coresystems.de>
>
>
> I can't really verify if this is the correct thing to do, but since it fixes
> abuild...
>
> Acked-by: Stefan Reinauer <stepan at coresystems.de>

Looking at the rest fragment that has been passed around I think the
actual bug is that romcc allows non-static non-const arrays to be
declared.  I can not find any indication that I ever added support for
this when I wrote romcc.

My practical concern is that there is no support for the general case where
you do:
char array[5];
for (i = 0; i < 5; i++) {
    array[i] = 7;
}

I certainly don't see a test case for arrays declared in variables.

Until we have solid support for array indexing of variables stored in
registers there is no real point in implementing any other support for
arrays stored in registers.  It will lead to false expectations and
strange debugging sessions.

Furthermore there is no point in using arrays stored in registers if
you have to refer to the elements without indirect addressing.

This feels like feature addition through the back door of bug reports,
which is really the wrong way to go about it.

Eric