0

Im trying to implement custom threads in C by creating a simple context switch function plus a FCFS scheduler.

The first step that i want to perform is to copy the entire function stack frame to heap to a saved frame and replace it by the first in queue.

The problem i have is that after finishing task one the stack of the second gets corrupted. I dont have any idea about why.

The code i have is the following:

#include <unistd.h>
#include <string.h>
#include <stdio.h>
#include <stdlib.h>

#define ITERATIONS 10
#define SSIZE 15

int * last;

void kwrite(const char*);
void kyield(int *);

void f1() {
    int i = ITERATIONS;
    while (i--) kwrite("A\n");
}

void f2() {
    int i = ITERATIONS*2;
    while (i--) {
        printf("[%d]", i);
        kwrite("B\n");
        getchar();
    }
}

void kwrite(const char* str) {
    int a[10] = {5, 5, 5, 5, 5, 5, 5, 5, 5, 5};
    write(1, str, strlen(str));

    int *frame = malloc(sizeof(int)*SSIZE);
    memcpy(frame, a, SSIZE*sizeof(int));
    kyield(frame);
    printf("ERROR\n");
}

void kyield(int * from) {
    if (from == NULL) {
        f1();
        from = malloc(sizeof(int)*SSIZE);
        memcpy(from, last, SSIZE*sizeof(int));
    }
    if (last == NULL) {
        last = malloc(sizeof(int)*SSIZE);
        memcpy(last, from, SSIZE*sizeof(int));
        free(from);
        f2();
        exit(0);
    }

    int a[10] = {3, 3, 3, 3, 3, 3, 3, 3, 3, 3};
    memcpy(a, last, SSIZE*sizeof(int));
    memcpy(last, from, SSIZE*sizeof(int));
    free(from);
}

int main(int argc, char** argv) {
    kyield(NULL);
    free(last);
}

It should call 10 times f1 and 20 f2 then exit. But when the var i of f2 is 8 it gets corrupted on the next iteration. Therefore entering an infinite loop.

Any help or suggestions would be appreciated! Have a nice day.

[edit] I suppose the code can be a bit tricky to understand so here it is a little clarification:

main calls kyield with null parameters.

kyield detects it and calls f1

f1 executes until kwrite is called

kwrite calls kyield passing its current stack frame

kyield detects the last stack frame is null so it copies the stack frame (sf from now on) given by kwrite, then calls f2 f2 does the same as f1

when kyield is executed next, both from and last wont be NULL so it will overwrite its current s f with the one in last, swap it with the one in from and lastly it will return, as the stack has been altered it will jump to the return address of the last kwrite, not the actual one, thus. Jumping from the f1 thread to f2.

Your memcpy(frame, a, SSIZE*sizeof(int)) looks wrong. Your SSIZE is defined to 15, but a has only a size of 10.

this is deliberate, as by copying 15 elements of 4 bytes we are copying the rax last value, the last ebp and the return address of the function.

https://eli.thegreenplace.net/2011/09/06/stack-frame-layout-on-x86-64/

Tretorn
  • 365
  • 1
  • 16
  • When `memcpy(from, last, SSIZE*sizeof(int));` is executed, `last == NULL`. What do you expect to happen with this `memcpy()`? – chux - Reinstate Monica Dec 19 '18 at 03:46
  • 1
    @chux This seems quite confusing, but I think `f1` calls `kwrite` which calls `kyield` with `from != NULL` and therefore before the first `kyield` call gets to this line, `last` will be allocated for and `memcpy`ed to through the other `kyield` call. Still doesn't make the intend clear though. –  Dec 19 '18 at 03:52
  • @user10605163 Agrreed. – chux - Reinstate Monica Dec 19 '18 at 04:03
  • @chux you have to understand that there are two concurrent threads going on here. Even though the code seems simple it is really tricky. Answering your question, when the mentioned memcpy is executed last wont be null as another kyield call have previously allocated it. – Tretorn Dec 19 '18 at 08:50
  • Tretorn, There is only one [thread](https://en.wikipedia.org/wiki/Thread_(computing)) here. @user10605163 has already explained the code. – chux - Reinstate Monica Dec 19 '18 at 15:53

2 Answers2

1

I can see a couple of issues with the design. The normal way of scheduling different threads is for them to get complete stacks, not to share the same stack.

What that means in this case, is that the stack depends on the addresses being used.

+---------+--------+---------+--------+---------+
| main    | kyield | f1      | kwrite |  kyield |
|         |        |         |a[10]   |         |
+---------+--------+---------+--------+---------+

                        |-------------|  << copied by the slice of the stack.

The slice of the stack you are taking, is independent of the amount used in the function preceding it, so would be broken if the functions calling kwrite have different stack requirements (different amounts of state would be required).

It is also broken because the amount of information which is captured on the stack is not complete. The state of execution is based on the stack and the current values in non-volatile registers. These values can pollute the alternate threads.

Finally the stack also contains addresses. This scheme could only work, if all the threads executed had identical stack requirements, as if the yield function pastes a value back in which requires addresses, then they always have to align.

mksteve
  • 12,614
  • 3
  • 28
  • 50
  • Thank you for your answer. I knew i had to give them different stacks. i thought i was. Do you have any reference/documentation/example over that approach? – Tretorn Dec 19 '18 at 09:42
0

Your memcpy(frame, a, SSIZE*sizeof(int)) looks wrong. Your SSIZE is defined to 15, but a has only a size of 10.

Georg
  • 94
  • 4
  • Agreed, `int a[SSIZE]` would make more sense. – chux - Reinstate Monica Dec 19 '18 at 03:48
  • @Georg i answered why this cannot be in the main post. Perhaps i was giving too litle background about what i was doing. – Tretorn Dec 19 '18 at 08:52
  • @Tretorn "we are copying the rax last value, the last ebp and the return address of the function." ---> this depends on select _undefined behavior_ (UB) of `memcpy()`, on the compiler/platform (post not tagged with either) and your correct interpretations of these. Without that info, the question is unclear. – chux - Reinstate Monica Dec 19 '18 at 16:01