immunant / c2rust

Migrate C code to Rust
https://c2rust.com/
Other
3.95k stars 234 forks source link

Transpiled code doesn't compile due to error[E0308]: mismatched types #478

Open thedataking opened 2 years ago

thedataking commented 2 years ago

Example observed in lighttpd-1.4.64 and lighttpd-1.4.65:

error[E0308]: mismatched types
   --> src/lemon.rs:505:17
    |
505 |                 b"./lemon.c\0" as *const u8 as *const libc::c_char,
    |                 ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ types differ in mutability
    |
    = note: expected raw pointer `*mut i8`
               found raw pointer `*const i8`

Input C code:

define assert(X) if(!(X))myassert(__FILE__,__LINE__)

void myassert(file,line)
char *file;
int line;
{
  fprintf(stderr,"Assertion failed on line %d of file \"%s\"\n",line,file);
  exit(1);
}

// ...

static int actioncmp(ap1,ap2)
struct action *ap1;
struct action *ap2;
{
  int rc;
  rc = ap1->sp->index - ap2->sp->index;
  if( rc==0 ) rc = (int)ap1->type - (int)ap2->type;
  if( rc==0 ){
    assert( ap1->type==REDUCE || ap1->type==RD_RESOLVED || ap1->type==CONFLICT);
    assert( ap2->type==REDUCE || ap2->type==RD_RESOLVED || ap2->type==CONFLICT);
    rc = ap1->x.rp->index - ap2->x.rp->index;
  }
  return rc;
}

According to cppreference.com's page on the const type qualifier:

const semantics apply to lvalue expressions only; whenever a const lvalue expression is used in context that does not require an lvalue, its const qualifier is lost (note that volatile qualifier, if present, isn't lost).

The use of __FILE__ in the statement fprintf(stderr,"Assertion failed on line %d of file \"%s\"\n",line,file); does not require an lvalue, so the transpiler should not cast the variable to *const i8 given that *mut i8 is expected.

kkysen commented 2 years ago

lighttpd is still using old-style K&R function definitions? Does that have anything to do with the error, i.e. the function being untyped?

fw-immunant commented 2 years ago

This is a minimal reproducing example:

void myassert();

int main(int argc, char** argv)
{
    myassert(__FILE__);
    return 0;
}

void myassert(char* file) {}

Removing the forward declaration, adding a parameter declaration to it, or swapping the order of definitions of myassert and main fixes the mistranslation.

kkysen commented 2 years ago

Ah, so it is related to old-style function declarations. We must be getting a different AST from clang, right?

fw-immunant commented 2 years ago

Here's the Clang AST (clang -Xclang -ast-dump -fsyntax-only lemon-minimal.c):

TranslationUnitDecl 0x55555563c208 <<invalid sloc>> <invalid sloc>
|-TypedefDecl 0x55555563ca30 <<invalid sloc>> <invalid sloc> implicit __int128_t '__int128'
| `-BuiltinType 0x55555563c7d0 '__int128'
|-TypedefDecl 0x55555563caa0 <<invalid sloc>> <invalid sloc> implicit __uint128_t 'unsigned __int128'
| `-BuiltinType 0x55555563c7f0 'unsigned __int128'
|-TypedefDecl 0x55555563cda8 <<invalid sloc>> <invalid sloc> implicit __NSConstantString 'struct __NSConstantString_tag'
| `-RecordType 0x55555563cb80 'struct __NSConstantString_tag'
|   `-Record 0x55555563caf8 '__NSConstantString_tag'
|-TypedefDecl 0x55555563ce40 <<invalid sloc>> <invalid sloc> implicit __builtin_ms_va_list 'char *'
| `-PointerType 0x55555563ce00 'char *'
|   `-BuiltinType 0x55555563c2b0 'char'
|-TypedefDecl 0x55555563d138 <<invalid sloc>> <invalid sloc> implicit __builtin_va_list 'struct __va_list_tag[1]'
| `-ConstantArrayType 0x55555563d0e0 'struct __va_list_tag[1]' 1 
|   `-RecordType 0x55555563cf20 'struct __va_list_tag'
|     `-Record 0x55555563ce98 '__va_list_tag'
|-FunctionDecl 0x555555692bf0 <lemon.c:1:1, col:15> col:6 used myassert 'void ()'
|-FunctionDecl 0x555555692e80 <line:3:1, line:7:1> line:3:5 main 'int (int, char **)'
| |-ParmVarDecl 0x555555692cf0 <col:10, col:14> col:14 argc 'int'
| |-ParmVarDecl 0x555555692da0 <col:20, col:27> col:27 argv 'char **'
| `-CompoundStmt 0x555555693060 <line:4:1, line:7:1>
|   |-CallExpr 0x555555692ff0 <line:5:2, col:19> 'void'
|   | |-ImplicitCastExpr 0x555555692fd8 <col:2> 'void (*)()' <FunctionToPointerDecay>
|   | | `-DeclRefExpr 0x555555692f30 <col:2> 'void ()' Function 0x555555692bf0 'myassert' 'void ()'
|   | `-ImplicitCastExpr 0x555555693018 <<scratch space>:2:1> 'char *' <ArrayToPointerDecay>
|   |   `-StringLiteral 0x555555692f88 <col:1> 'char[8]' lvalue "lemon.c"
|   `-ReturnStmt 0x555555693050 <lemon.c:6:2, col:9>
|     `-IntegerLiteral 0x555555693030 <col:9> 'int' 0
`-FunctionDecl 0x555555693158 prev 0x555555692bf0 <line:9:1, col:28> col:6 used myassert 'void (char *)'
  |-ParmVarDecl 0x555555693098 <col:15, col:21> col:21 file 'char *'
  `-CompoundStmt 0x555555693200 <col:27, col:28>

We're apparently looking at the wrong declaration of myassert (which is what Clang's AST points at) when translating its call in main.

gstrauss commented 1 year ago

lighttpd is still using old-style K&R function definitions?

lemon.c and lempar.c were included in the original version of lighttpd 1.4 and is an ancient version of the LEMON parser. https://en.wikipedia.org/wiki/Lemon_(parser_generator)

I am a lighttpd developer and am willing to look into modifications if doing so will assist you in your porting effort.

kkysen commented 1 year ago

I am a lighttpd developer and am willing to look into modifications if doing so will assist you in your porting effort.

Thanks! We're mostly using lighttpd as a test case for c2rust, and in particular the lifting unsafe Rust to safe Rust after the initial transpilation from C to unsafe Rust. So we want to be able to handle these things, as they could be in other codebases that people want to translate, too. I was just surprised by the old function definitios and thought that might be something we don't handle correctly, and it seems we don't when there are mismatched headers, so that's something we need to fix.

gstrauss commented 1 year ago

FYI: The C2x standard currently in development removes K&R-style function declarations.

lighttpd 1.4.69 updates the LEMON parser to the version maintained as part of SQLite: https://www.sqlite.org/src/file/tool/lemon.c https://www.sqlite.org/src/file/tool/lempar.c

For your testing purposes, you may of course continue to use a prior version of lighttpd.

I am posting so that you are aware in your testing that if you use lighttpd 1.4.69 code base or later, this issue should no longer exist, and so your tests for K&R-style function declarations will no longer be testing any K&R-style function declarations in the lighttpd 1.4.69 code base.