Bug found in **TargetPrinter.ml** of **x86_32** backend

[ChrisYin]

Background

SystemV ABI: Functions Returning structures

According to Page 40-41 of SystenV ABI standard(Intel386) :

If a function returns a structure or union, then the caller provides space for the return value and places its address on the stack as argument word zero.  
In effect,this address becomes a ‘‘hidden’’ first argument.  
Having the caller supply the return object’s space allows re-entrancy.

A function that returns a structure or union also sets %eax to the value of the original address of the caller’s area before it returns. 
Thus when the caller receives control again, the address of the returned object resides in register %eax and can be used to access the object. 
Both the calling and the called functions must cooperate to pass the return value successfully:

+ The calling function must supply space for the return value and pass its address in the stack frame;
+ The called function must use the address from the frame and copy the return value to the object so supplied;
+ The called function must remove this address from the stack before return-ing.

Bug Description

Functions returning structures in CompCert x86_32 backend

• Add -fstruct-passing option to support fuctions returning a struct
• In the parsing phase, it treats the function returning a structure as a function returning void with a new hidden first argument which represents the address of the returning structure. A new attribute is also added for this function.

//original code
struct twoint test(int x, int y){
  struct twoint t;
  t.x = x;
  t.y = y;
  return t;
}

//after parsing
void __attribute__((__structreturn)) test(struct twoint * _res, int x, int y)
{
  struct twoint t;
  t.x = x;
  t.y = y;
  *_res = t;
  return;
}

• When generating the assembly in x86/TargetPrinter.ml, there is a special trick for functions with a __structreturn attribute.

Before returning to its parent, the callee needs to set register eax to the address of the returning structure.
Then, it needs to pop it out from the stack according to SystemV conventions.

733      | Pret ->
734          if (not Archi.ptr64)
735          && (!current_function_sig).sig_cc.cc_structret then begin
736            fprintf oc "	movl	0(%%esp), %%eax\n";      %% '0(%%esp)' sould be '4(%%esp)'
737            fprintf oc "	ret	$4\n"
738          end else begin
739            fprintf oc "	ret\n"
740          end

However, the instruction movl 0(%%esp), %%eax\n is wrong, 0(%%esp) is the return address rather than the hidden first argument(stored structure's address).
The correct one should be movl 4(%%esp), %%eax\n.

After returning from the callee, the caller needs to push register eax(address of the returning structure) to its stack in order to restore its original stack frame.

725      | Pcall_s(f, sg) ->
726          fprintf oc "	call	%a\n" symbol f;
727          if (not Archi.ptr64) && sg.sig_cc.cc_structret then
728            fprintf oc "	pushl	%%eax\n"
729      | Pcall_r(r, sg) ->
730          fprintf oc "	call	*%a\n" ireg r;
731          if (not Archi.ptr64) && sg.sig_cc.cc_structret then
732            fprintf oc "	pushl	%%eax\n"

Evaluation

In CompCert, the error address of the structure in register eax won't result in a crash program.
This is because the register eax won't be used to access that structure.
Instead, it uses esp with an offset to access the structure object.
While according to the SystemV ABI standard(See the background part), register eax can be used to access the object.
I wrote an inline assembly to do it, and the program will be crashed.
(Because it tries to modify the code at the returning address stored in eax)

This bug only exists in x86_32 backend, in x86_64 backend it has a different convention for returning a structure.
It will pass the address of the structure object in register %rdi rather than in the stack.

Here is my testing code:
(Please compile it with options: -fstruct-passing -finline-asm)

#include<stdio.h>

// simple structure with two integers
struct twoint {
  int x;
  int y;
};

// simple test function to setup the structure
struct twoint test(int x, int y){
  struct twoint t;
  t.x = x;
  t.y = y;
  return t;
}

// call the test function within C
int callinC(){
  struct twoint x;
  // set the structure value
  x = test(42,42);
  // modify the structure value
  x.x = 13;
  return x.x+x.y;
}

// call the test function within Asm
int callinAsm(){
  struct twoint x;
  struct twoint *addr;
  asm (
    "movl	$42, %edx\n\t"
    "movl	$42, %ebx\n\t"
    "movl	%ebx, 8(%esp)\n\t"
    "movl	%edx, 4(%esp)\n\t"
    "movl	%1, 0(%esp)\n\t"
    "call	test\n\t" // call the test function to setup the structure value
    "pushl	%eax\n\t" 
    "mov %eax, %0\n\t" // save the returning address to variable addr
    : "=r" (addr) 
    : "r" (&x));
  addr->x = 13; // modify the structure value
  return x.x+x.y;
}

int main(){
  printf("Call the test function in C: %d\n", callinC());  
  printf("Call the test function in Asm: %d\n", callinAsm());
  return 0;
}

Bug Fixing

Correct the instruction located at line 736 in x86/TargetPrinter.ml:

733      | Pret ->
734          if (not Archi.ptr64)
735          && (!current_function_sig).sig_cc.cc_structret then begin
736            fprintf oc "	movl	4(%%esp), %%eax\n";
737            fprintf oc "	ret	$4\n"
738          end else begin
739            fprintf oc "	ret\n"
740          end

[xavierleroy]

Well spotted. Thanks for the report and the proposed fix. I guess x86-32 C compilers don't really use the pointer returned in EAX, otherwise our compiler interoperability tests would have run into this mistake.