1

So as the title says, my code keeps seg-faulting unfortunately. I'm pretty sure I malloc'd everything correctly and I believe that my functions are correct for the test case I have but it still seg-faults. I just wanted to see if anyone could maybe spot why it may be seg-faulting? Here is my code that I have so far:

#include <stdio.h>
#include <stdlib.h>
#include <ucontext.h>
#include "t_lib.h"
#include <sys/mman.h>

struct ready_queue *ready;
struct tcb *running;

void t_yield()
{
    tcb *tmp;
    tmp->thread_context = running->thread_context;

    running->thread_context = ready->head->thread_context;
    ready->head->thread_context = tmp->thread_context;
    swapcontext(ready->head->thread_context, running->thread_context);
}

void t_init()
{
    tcb *tmp;
    tmp = (tcb *) malloc(sizeof(tcb));

    running = NULL;
    ready->head = ready->tail = NULL;
}

void t_create(void (*fct)(int), int id, int pri)
{
    size_t sz = 0x10000;

    tcb *tmp;
    tmp = (tcb *) malloc(sizeof(tcb));

    ucontext_t *uc;
    uc = (ucontext_t *) malloc(sizeof(ucontext_t));

    tmp->thread_context = uc;
    tmp->thread_priority = pri;
    tmp->thread_id = id;
    getcontext(uc);
    uc->uc_stack.ss_sp = mmap(0, sz,
        PROT_READ | PROT_WRITE | PROT_EXEC,
        MAP_PRIVATE | MAP_ANON, -1, 0);
    uc->uc_stack.ss_size = sz;
    uc->uc_stack.ss_flags = 0;
    uc->uc_link = running->thread_context;

    makecontext(uc, fct, 1, id);

    if(running == NULL)
        running = tmp;
    else
    {
        if(ready->head == NULL)
        {
             ready->head = tmp;
             ready->tail = tmp;
        }
        else
        {
             ready->tail->next = tmp;
             ready->tail = tmp;
        }
    }
 }

TEST CASE

void assign(int pri)
{
  int i;

  for (i = 0; i < 3; i++)
    printf("in assign(1): %d\n", i);

  t_yield();

  for (i = 10; i < 13; i++)
    printf("in assign(2): %d\n", i);

  t_yield();

  for (i = 20; i < 23; i++)
    printf("in assign(3): %d\n", i);
}

int main(int argc, char **argv)
{
  t_init();
  t_create(assign, 1, 1);

  printf("in main(): 0\n");

  t_yield();

  printf("in main(): 1\n");
  t_yield();

  printf("in main(): 2\n");
  t_yield();

  printf("done...\n");

  return (0);

}

VALGRIND RESULTS:

==4596== Memcheck, a memory error detector
==4596== Copyright (C) 2002-2012, and GNU GPL'd, by Julian Seward et al.
==4596== Using Valgrind-3.8.1 and LibVEX; rerun with -h for copyright info
==4596== Command: ./test00
==4596==
==4596== Invalid write of size 8
==4596==    at 0x400810: t_init (t_lib.c:24)
==4596==    by 0x40072C: main (test00.c:24)
==4596==  Address 0x8 is not stack'd, malloc'd or (recently) free'd
==4596==
==4596==
==4596== Process terminating with default action of signal 11 (SIGSEGV)
==4596==  Access not within mapped region at address 0x8
==4596==    at 0x400810: t_init (t_lib.c:24)
==4596==    by 0x40072C: main (test00.c:24)
==4596==  If you believe this happened as a result of a stack
==4596==  overflow in your program's main thread (unlikely but
==4596==  possible), you can try to increase the size of the
==4596==  main thread stack using the --main-stacksize= flag.
==4596==  The main thread stack size used in this run was 10485760.
==4596==
==4596== HEAP SUMMARY:
==4596==     in use at exit: 0 bytes in 0 blocks
==4596==   total heap usage: 0 allocs, 0 frees, 0 bytes allocated
==4596==
==4596== All heap blocks were freed -- no leaks are possible
==4596==
==4596== For counts of detected and suppressed errors, rerun with: -v
==4596== ERROR SUMMARY: 1 errors from 1 contexts (suppressed: 6 from 6)
JNYRanger
  • 6,829
  • 12
  • 53
  • 81
VakarianWrex
  • 53
  • 2
  • 9
  • 2
    Have you used `gdb` or `valgrind`? – Maria Ines Parnisari Nov 03 '13 at 02:50
  • Valgrind will point this out straight away - well it will point you _near_ it straight away, I'd recommend trying that and then letting us know what it told you, you might need some help zeroing in on it. Do you have a compilable example to reproduce this? I can give it a whack. – Tim Post Nov 03 '13 at 02:53
  • Off-hand, probably not it, but you're not checking for null immediately after malloc, like uc in t_create(). (Also nit-picky, the malloc in t_init is superfluous. Getting the smallest code foot print that repros makes debugging go much faster...) – bishop Nov 03 '13 at 02:54
  • I have never used valgrind before, I will take a look at that. @TimPost I have included the test I am trying it on in the question, take a look. Although I'm not sure if you need my header or makefile? – VakarianWrex Nov 03 '13 at 03:04
  • You never set `tmp->next` to `NULL` before attaching it to the list. You set what appears to be everything else, but not that. As a result, any enumeration algorithm that expects to find end-of-list is in for a rude, undefined-behavior, surprise. (unrelated: [stop casting malloc results](http://stackoverflow.com/questions/605845/do-i-cast-the-result-of-malloc).) – WhozCraig Nov 03 '13 at 03:04
  • @WhozCraig would you suggest I just set that to NULL in my create function where I set the rest of the variables in tmp? – VakarianWrex Nov 03 '13 at 03:17
  • I think the line immediately following `tmp->thread_id = id;` would be a fine place to put it. – WhozCraig Nov 03 '13 at 03:18
  • errrr I'm not sure how to write that in a comment that makes it easy to read. Also @WhozCraig, thanks for that, completely forgot about next. Still segfaulting, but its a step closer :p – VakarianWrex Nov 03 '13 at 03:26
  • @bloodruns4ever don't put it in a comment. Amend your question body with additional information. And this: `Address 0x8` is a hint that you're dereferencing a structure from a NULL pointer, accessing a member at offset 0x8, i.e 64bits deep (so if you're compiling 64bit its a pointer-width or long/int width, if 32bit, its two of them). – WhozCraig Nov 03 '13 at 03:31
  • 1
    Thanks for posting the valgrind report: I believe the error is here: `ready->head = ready->tail = NULL;` (ad I'm betting that is line 24 of that source file). The `ready` global is still NULL when that code executes. The first write happens on the `tail` member which is one 64bit pointer-width inside the structure, thus the invalid write at address 0x8. – WhozCraig Nov 03 '13 at 03:37
  • Nothing to do with your segfault, but why are you allocating a `struct tcb` in `t_init()` but ever doing anything with it (other than leaking it). – Michael Burr Nov 03 '13 at 03:40
  • @WhozCraig ok I added it to the question. Does this line `at 0x400810: t_init (t_lib.c:24)` mean that the problem is occuring when `init` is called? – VakarianWrex Nov 03 '13 at 03:40
  • @bloodruns4ever see my prior comment. And honestly I see no reason that structure should be dynamically allocated *at all*. using `struct ready_queue ready;` and access it via `ready.head` and `ready.tail` should be sufficient for what I think you need. – WhozCraig Nov 03 '13 at 03:41
  • @WhozCraig So I'm guessing when `create` is called it is writing the `tcb` to `tail` rather than the `head`? Is that what you're saying? And I did not know that `ready.head` could be used in C I thought it had to be in this format `ready->head` – VakarianWrex Nov 03 '13 at 03:48
  • Not even *close*. **You're dereferencing `ready` while it is still `NULL`.** Do you see `t_create` anywhere in that call-stack from valgrind? This is much simpler than your trying to make it. `t_init` is being called and `ready` is still NULL. then `ready` is dereferenced and boom. Thats it. Nothing more. Fix that. Line 24 of your test00.c program calls `t_init`, and on line 24 of t_lib.c you dereference a NULL pointer. – WhozCraig Nov 03 '13 at 03:52
  • And regarding `ready.head` vs `ready->head`. Read the entire comment I put that in. I said, "I see no reason that structure should be dynamically allocated at all. Using **`struct ready_queue ready;`** and access it via `ready.head` and `ready.tail` should be sufficient for what I think you need." Note the lack of any *pointer* declaration for the `ready` global variable. – WhozCraig Nov 03 '13 at 03:56
  • @WhozCraig Ok ok I see now! I fixed that part and that no longer shows up in valgrind but an error shows up now in `t_create` with the same message but this time at `at 0x4008D5: t_create (t_lib.c:55)` Now that I kind of know how to use valgrind, I should be able to fix this problem on my own. Thank you though Craig, you've helped a lot – VakarianWrex Nov 03 '13 at 03:56
  • Or maybe not... haha I'm getting `Invalid read of 8` now for this line: `uc->uc_link = running->thread_context;` is it because it calls `init` first in the test case which makes `running = NULL` and then it's trying to set `uc->uc_link = running->thread_context`... but since `running` is `NULL` it can't read anything? – VakarianWrex Nov 03 '13 at 04:25
  • Were I to do this, `ready` would be a non-dynamic struct (with two dynamic pointers `head` and `tail`) and `running` would continue to be a pointer. There are *plenty* of other issues in this code. Ex: in `t_yield()` : `tcb *tmp; tmp->thread_context = running->thread_context;` That declares an uninitialized (therefore indeterminate) pointer `tmp`, then **immediately** dereferences it, which is, one again, **undefined behavior**. You need some quantifiable study time with dynamic memory, pointers, and general usage therein before you continue this project. – WhozCraig Nov 03 '13 at 05:43

1 Answers1

0

In function t_yield you used uninitialized pointer tmp:

tcb *tmp;   // <- definition without initialization
tmp->thread_context = running->thread_context; // <- using
Dmitriy Khaustov
  • 28
  • 1
  • 6