From patchwork Mon Mar 15 20:18:26 2010 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit Subject: Ouch: romcc "x[0] |= something" patch causes another crash Date: Mon, 15 Mar 2010 20:18:26 -0000 From: Eric W. Biederman X-Patchwork-Id: 1056 Message-Id: To: Stefan Reinauer Cc: ron minnich , coreboot@coreboot.org Stefan Reinauer writes: > On 3/15/10 8:28 PM, ron minnich wrote: >> On Mon, Mar 15, 2010 at 10:27 AM, Eric W. Biederman >> wrote: >> >> >>> 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; >>> } >>> >> The bigger problem is that people are trying to take this compiler >> beyond what makes sense (to me). I'm not sure we're going to cease to >> exist if we don't have arrays. >> > I agree we don't necessarily need to have such arrays. I think we just > naturally assumed it should work, > so no attempt to sneak anything in. > > What's implemented and what is not is hidden in Eric's brain and in a > single file of 25k lines of code. > > If we don't want non-static non-const arrays, can we easily detect them > in the code and give the user an error message that is better than a > segfault? > "You're a fool because you used non-const non-static arrays in romcc" > would be fine. Just dropping dead without error is certainly less helpful. We should be able to. I thought such a warning existed. So I have been scratching my head a bit to understand why it doesn't. >> If romcc can't do something, then work around it; we can warn people >> about no arrays. But given that nobody has the time to really support >> romcc (as, e.g., gcc or llvm are supported) we're taking some real >> risks just plugging changes in. >> > The changes were merely trying to fix the segfaults, not implement or > change anything big. I think we do want fixes for segfaults. Always. Fixing segfaults and then getting code that doesn't work correctly is worse. Always. I did my best with romcc to ensure the compile fails if we are not going to generate the correct code. At the same time I would much rather it be an assert than a random segfault. I have to run but I think this patch adds the missing check to catch non-static arrays. Eric Acked-by: Stefan Reinauer Signed-off-by: "Eric W. Biederman" Index: romcc.c =================================================================== --- romcc.c (revision 4892) +++ romcc.c (working copy) @@ -13458,6 +13458,10 @@ if ((type->type & TYPE_MASK) == TYPE_FUNCTION) { error(state, 0, "Function prototypes not supported"); } + if (ident && + ((type->type & TYPE_MASK) == TYPE_ARRAY) && + ((type->type & STOR_MASK) != STOR_STATIC)) + error(state, 0, "non static arrays not supported"); if (ident && ((type->type & STOR_MASK) == STOR_STATIC) && ((type->type & QUAL_CONST) == 0)) {