open-watcom / open-watcom-v2

Open Watcom V2.0 - Source code repository, Wiki, Latest Binary build, Archived builds including all installers for download.
Other
985 stars 160 forks source link

OW fails with: Error! E1054: Expression must be constant (csmith 00011) #1038

Open winspool opened 1 year ago

winspool commented 1 year ago

OW fails to compile lots of testing source files, generated by csmith, which work with other compiler (tcc or gcc as example).

Most common failure is: Error! E1054: Expression must be constant

An example, stripped down version is attached (00011_reduced.c). I send it also to compiler-explorer: https://compiler-explorer.com/z/boebzGMzj Adding "-fnonconst-initializers" to owcc adds "-aa" to wcc386, but the failure from OW does not change.

00011_reduced.c.txt

The full source file is also attached (00011.c). Command line to compile (needs current csmith installed): owcc -std=c99 -O0 -g -w -I/usr/include/csmith -v -Wstop-after-errors=999 -fnonconst-initializers 00011.c -o 00011_tst

00011.c.txt

pchapin commented 1 year ago

Open Watcom C is fundamentally a C89/90 compiler (support for C99 is spotty). I believe in C89/90, the initializers of array elements must be constant expressions. The address of a local variable is not a constant expression. Thus, the example violates the C89/90 standard.

I see that you added the -aa option, but that didn't help. My understanding is that the -aa option is experimental and incompletely implemented. Most likely it doesn't (yet) know how to handle dynamically computed pointers. However, it should do so!

pchapin commented 1 year ago

So, the situation is more subtle than it appears. The following program compiles and runs fine using the -aa option (but not without that option, as expected).

#include <stdio.h>

int f( )
{
    int x = 1;

    int  a[] = { x + 2 };
    int *b[] = { &x };

    return *b[0] + a[0];
}

int main( void )
{
    printf( "The answer = %d (4 expected)\n", f( ) );
    return 0;
}

The intent of this example was to explore the difference, if any, of using arithmetic expressions such as x + 2 as array element initializers from the effect of taking the address of a local variable. As I said, both work as expected in this example.

pchapin commented 1 year ago

Reducing the given example to this:

int  func_6( void )
{
    int x = 1;

    int *a[1][1] = { { &x } };
    int *b[1]    =   { &x };
    return 0;
}

shows that the problem is related to the 2-dimensional array. The single dimensional array works fine (with the -aa option). This is definitely an issue with the -aa option not being completely implemented.

winspool commented 1 year ago

Thanks for reducing the testcase even more and great, that you where able to nail down, which feature is missing.

Yes, i know, that OW has only partial support for c99, but with every c99 featuture added, OW get's better.

winspool commented 1 year ago

Yes, i know, that OW creates the warnings (and errors) for a reason, but the goal for csmith is to produce working c99 files to find compiler bugs.

Finding bugs can't work, when most of the testfiles wont compile. (For csmith with seeds from 000 to 199, compile with OW fails for 143 files, while tcc and gcc can compile all 200).

Commandline parameter to create the tests (example for 00011.c:) csmith --seed 11 --output /run/user/1000/csmith/00011.c

Current csmith on github: https://github.com/csmith-project/csmith

jmalak commented 1 year ago

Thanks for bug analysis. @pchapin do you have idea why -aa option was created if it is part of C99 standard that it should be handle by -za99?

pchapin commented 1 year ago

@jmalak I don't recall the full history of the -aa option. The functionality it provides, non-constant initializers for local arrays, is part of C99, and so should be also activated as part of -za99. As to if (or why) it should exist as a stand-alone option, I don't know. Maybe there is a compatibility issue with other compilers that was being addressed?

jmalak commented 1 year ago

Hm.. I have also no idea. May be it was before C99 standard. Anyway I will change it to be included by both -za99 and -aa options.