0

I'm working on the sleeping barber problem (using FIFO queue and shared memory), and I have a problem. I try to run this program just to see what is shown, however, I get segmentation fault every time. In the code I check, if semaphores are created successfully, if shared memory is created properly, but the program crashing must come from different place, which I cannot seem to find. I also tried to use Valgrind but what I got is:

Valgrind for the newer version of code: However, when I try to run the code I still get segfault

==187== Memcheck, a memory error detector
==187== Copyright (C) 2002-2017, and GNU GPL'd, by Julian Seward et al.
==187== Using Valgrind-3.15.0 and LibVEX; rerun with -h for copyright info
==187== Command: ./barber
==187==
Error while executing program, invalid amount of arguments==187==
==187== HEAP SUMMARY:
==187==     in use at exit: 0 bytes in 0 blocks
==187==   total heap usage: 1 allocs, 1 frees, 1,024 bytes allocated
==187==
==187== All heap blocks were freed -- no leaks are possible
==187==
==187== For lists of detected and suppressed errors, rerun with: -s
==187== ERROR SUMMARY: 0 errors from 0 contexts (suppressed: 0 from 0)
==124== Memcheck, a memory error detector
==124== Copyright (C) 2002-2017, and GNU GPL'd, by Julian Seward et al.
==124== Using Valgrind-3.15.0 and LibVEX; rerun with -h for copyright info
==124== Command: ./barber
==124==
==124== Invalid read of size 1
==124==    at 0x1094DD: main (in /mnt/c/users/Czaro/ubuntu/lab6posix/barber)
==124==  Address 0x2 is not stack'd, malloc'd or (recently) free'd
==124==
==124==
==124== Process terminating with default action of signal 11 (SIGSEGV)
==124==  Access not within mapped region at address 0x2
==124==    at 0x1094DD: main (in /mnt/c/users/Czaro/ubuntu/lab6posix/barber)
==124==  If you believe this happened as a result of a stack
==124==  overflow in your program's main thread (unlikely but
==124==  possible), you can try to increase the size of the
==124==  main thread stack using the --main-stacksize= flag.
==124==  The main thread stack size used in this run was 8388608.
glut==124==
==124== HEAP SUMMARY:
==124==     in use at exit: 125 bytes in 4 blocks
==124==   total heap usage: 5 allocs, 1 frees, 1,149 bytes allocated
==124==
==124== LEAK SUMMARY:
==124==    definitely lost: 0 bytes in 0 blocks
==124==    indirectly lost: 0 bytes in 0 blocks
==124==      possibly lost: 0 bytes in 0 blocks
==124==    still reachable: 125 bytes in 4 blocks
==124==         suppressed: 0 bytes in 0 blocks
==124== Reachable blocks (those to which a pointer was found) are not shown.
==124== To see them, rerun with: --leak-check=full --show-leak-kinds=all
==124==
==124== For lists of detected and suppressed errors, rerun with: -s
==124== ERROR SUMMARY: 1 errors from 1 contexts (suppressed: 0 from 0)
Segmentation fault

I'd greatly appreciate every kind of help! The code for barber (newer version, with changes given by @AndrewHenle looks like this:

#include <stdio.h>
#include <stdlib.h>
#include <semaphore.h>
#include <sys/stat.h>
#include <fcntl.h>
#include "functions.h"
#include <sys/mman.h>
#include <unistd.h>

int main(char* argv[], int argc)
{
    if(argc < 2){
        printf("Error while executing program, invalid amount of arguments");
        return 0;
    }
    sem_t *barber;
    sem_t *queue;
    int seats;
    int sharedmem, waitRoomSize;
    struct Queue waitroom;
    
    barber = sem_open("Barber", O_CREAT | O_RDWR, 0666, 1);
     if((barber == SEM_FAILED)){
        printf("Error while creating semaphore for barber");
        exit(1);
        }
    queue = sem_open("Queue", O_CREAT | O_RDWR, 0666, 1);
     if((queue  == SEM_FAILED)) {
        printf("Error while creating semaphore for queue");
        exit(1);
      }
    seats = atoi(argv[1]);
    void *space;
    queueinit(waitroom, seats);


    sharedmem = shm_open("Queue", O_RDWR, 0666);
    if(sharedmem==-1){
       printf("Error while getting shared memory");
       exit(1);
    }
    waitRoomSize = ftruncate(sharedmem, sizeof(waitroom));
    if((waitRoomSize ==-1)){
       printf("Error while getting size");
       exit(1);
       }
    space = mmap(NULL, sizeof(struct Queue), PROT_READ | PROT_WRITE, MAP_SHARED, sharedmem, 0);
    if((space == MAP_FAILED)){
       printf("B³¹d podczas mapowania pamiêci");
       exit(1);
       }

    while(1)
    {
        printf("Starting work...");
       sem_wait(queue);
       sem_post(queue);
       if(isEmpty(waitroom)) {
            sem_wait(barber);
            printf("Falling asleep \n");
            sem_post(barber);
            printf("Waking up \n");
       }
       sem_wait(queue);
       sem_post(queue);

       int id = get(waitroom);
       printf("Customer: %d, please sit on the chair", id);
       printf("I start making haircut: %d", id);
       sleep(2);
       printf("Finished making haircut for: %d", id);
    }
    sem_close(barber);
    sem_unlink("Barber");
    sem_close(queue);
    sem_unlink("Queue");
}

Previous version:

#include <stdio.h>
#include <stdlib.h>
#include <semaphore.h>
#include <sys/stat.h>
#include <fcntl.h>
#include "functions.h"
#include <sys/mman.h>
#include <unistd.h>

int main(char* argv, int argc)
{
    sem_t *barber;
    sem_t *queue;
    int seats;
    int sharedmem, waitRoomSize;
    struct Queue waitroom;
   
    barber = sem_open("Barber", O_CREAT | O_RDWR, 0666, 1);
     if((barber == SEM_FAILED)){
        printf("Error while creating semaphore for barber");
        exit(1);
        }
    queue = sem_open("Queue", O_CREAT | O_RDWR, 0666, 1);
     if((queue  == SEM_FAILED)) {
        printf("Error while creating semaphore for queue");
        exit(1);
      }
    seats = atoi(argv[1]);
    void *space;
    queueinit(waitroom, seats);



    if((sharedmem = shm_open("Queue", O_RDWR, 0666)==-1)){
       printf("Error while getting shared memory");
       exit(1);
       }
    if((waitRoomSize = ftruncate(sharedmem, sizeof(waitroom))==-1)){
       printf("Error while getting size");
       exit(1);
       }
    if((space = mmap(NULL, sizeof(struct Queue), PROT_READ | PROT_WRITE, MAP_SHARED, sharedmem, 0) == MAP_FAILED)){
       printf("Błąd podczas mapowania pamięci");
       exit(1);
       }

    while(1)
    {
        printf("Starting work...");
       sem_wait(queue);
       sem_post(queue);
       if(isEmpty(waitroom)) {
            sem_wait(barber);
            printf("Falling asleep \n");
            sem_post(barber);
            printf("Waking up \n");
       }
       sem_wait(queue);
       sem_post(queue);

       int id = get(waitroom);
       printf("Customer: %d, please sit on the chair", id);
       printf("I start making haircut: %d", id);
       sleep(2);
       printf("Finished making haircut for: %d", id);
    }
    sem_close(barber);
    sem_unlink("Barber");
    sem_close(queue);
    sem_unlink("Queue");
}

The FIFO queue is implemented like this:

#ifndef FUNCTIONS_H_INCLUDED
#define FUNCTIONS_H_INCLUDED
#include <stdlib.h>
#include <unistd.h>
struct Queue{
    int* elems;
    int qsize;
    int head;
    int tail;
    int capacity;
};

void queueinit(struct Queue q, int capacity)
{
    q.elems = malloc(capacity*(sizeof(int)));
    q.capacity = capacity;
    q.head = 0;
    q.qsize = 0;
    q.tail = capacity-1;

}

int isFull(struct Queue q)
{
    return (q.qsize == q.capacity);
}

int isEmpty(struct Queue q)
{
    return (q.qsize == 0);
}


void push(struct Queue q, int val)
{
    if(isFull(q)){
        printf("Kolejka pe³na");
        return;
    }
    q.tail = (q.tail +1) % q.capacity;
    q.elems[q.tail] = val;
    q.qsize = q.qsize +1;
    printf("%d zajmuje miejsce w kolejce", val);
}

int get(struct Queue q)
{
    if(isEmpty(q))
    {
        printf("kolejka pusta");
        return -1;
    }
    int val = q.elems[q.head];
    q.head = (q.head +1) % q.capacity;
    q.qsize = q.qsize -1;
    printf("%d opuszcza zak³ad", val);
}
#endif // FUNCTIONS_H_INCLUDED
krysznys
  • 79
  • 1
  • 6
  • 1
    Valgrind will give you line numbers and stack traces if you have compiled with debug info (`-g`), and you can get the same for the segfault from your debugger. That should tell you which pointer in your program is invalid, and will probably help you figure out how it got that way. – Nate Eldredge Jan 06 '21 at 19:40
  • I compiled the code with -g but it did not change pretty much anything. Also, used Valgrind with --leak-check=full, but each time I've got the same text. – krysznys Jan 06 '21 at 19:51
  • The biggest advice is to turn on warnings, because your 'main' signature is unconventional; and you seem to be losing something with precedence when you do: result = function() == result – Halt State Jan 06 '21 at 20:28
  • Can you provide the full text of the valgrind output? Leaks are irrelevant here because your bug is not a memory leak (those cause excessive memory usage but never segfaults). And do you know how to use a debugger, to stop your program at the site of the segfault and be able to inspect its variables and other state? – Nate Eldredge Jan 06 '21 at 20:39
  • Two questions: (1) I don't see where you use `space` for anything? (2) You seem to have a pointer in your `Queue` struct, that is apparently supposed to be shared. That won't work, because the pointer will only be usable by whichever process set it to point to something in its own address space. Even if you point it somewhere in shared memory, the other process may have that memory mapped at a different virtual address. If you want to share data structures, you can't use absolute pointers at all - everything has to be done as array indices or in a similar relative fashion. – Nate Eldredge Jan 06 '21 at 20:43
  • @NateEldredge, I edited the post and pasted output from Valgrind. – krysznys Jan 06 '21 at 21:00
  • You're sure you recompiled with `-g`, and that you're running the recompiled version? You ought to be getting a line number along with the `at 0x1094DD: main (in /mnt/c/users/Czaro/ubuntu/lab6posix/barber)` message. – Nate Eldredge Jan 06 '21 at 21:02
  • @NateEldredge The thing about shared memory is that I need to have Barber and Client as separate programs, the shared memory is supposed to be a queue to the barber, barber checks if queue is empty, client is supposed to check if the queue is full. – krysznys Jan 06 '21 at 21:03
  • @NateEldredge, yes I compiled the code with ```gcc barber.c -g barber -lrt -lpthread``` The input I pasted to the post is all I got, checked few times... – krysznys Jan 06 '21 at 21:05
  • 1
    Should be `-g -o barber`. Without `-o` the executable is being written to `a.out` and the `barber` binary is left as whatever stale version it was before. – Nate Eldredge Jan 06 '21 at 21:06
  • Okay, I checked if it works with -o, it compiled, however, when I try to run it ```./barber 10``` for example I still get segfault – krysznys Jan 06 '21 at 21:14
  • `if((space = mmap(NULL, sizeof(struct Queue), PROT_READ | PROT_WRITE, MAP_SHARED, sharedmem, 0) == MAP_FAILED))` is one ***MASSIVE*** argument for writing `void *space = mmap(NULL, sizeof(struct Queue), PROT_READ | PROT_WRITE, MAP_SHARED, sharedmem, 0);` and then `if ( space == MAP_FAILED ) ...`. If you had put the assignment and the `if` statement ***ON SEPARATE LINES INSTEAD OF CRAMMING IT ALL INTO ONE LINE YOU WOULDN'T HAVE THIS BUG***. Sorry for shouting, but you can blame ***EVERYONE*** at the cargo-cult church of "brevity of code" for all your problems here and the time you wasted. – Andrew Henle Jan 06 '21 at 21:14
  • Doing the assignment in the `if` statement is ***BUG-PRONE BAD CODE*** and you learned why anyone who says to write code that way is wrong. It took 25 views and over two hours before anyone spotted that problem. – Andrew Henle Jan 06 '21 at 21:16
  • @AndrewHenle I changed the code to the form you provided, but I still am getting seg fault. I will edit the code in post, so it shows the newest version. – krysznys Jan 06 '21 at 21:22
  • Please *add* the new code - don't change the original. That's can invalidate a lot of information in the comments. – Andrew Henle Jan 06 '21 at 21:23
  • You see what the problem is with `space = mmap(NULL, sizeof(struct Queue), PROT_READ | PROT_WRITE, MAP_SHARED, sharedmem, 0) == MAP_FAILED)`? It assigns the result of the comparison `mmap(NULL, sizeof(struct Queue), PROT_READ | PROT_WRITE, MAP_SHARED, sharedmem, 0) == MAP_FAILED)` to `space`. – Andrew Henle Jan 06 '21 at 21:25
  • You also should be getting a core file - make sure your core file limit is set to `unlimited`. You can then use `gdb` on the core file to determine exactly where your code gets `SIGSEGV`. `valgrind` might interfere with the core file - I don't recall offhand if it does or doesn't. Or just run it under `gdb`. – Andrew Henle Jan 06 '21 at 21:27
  • @AndrewHenle I changed the code, now it compiles without any warnings (used ```-g -o``` flags, however - when I run the program like this: ```./barber``` I get the error saying that too few arguments were used, which is okay. But when I try to run it like this ```./barber 10``` I get segfault. I used gdb, but every single time I got error that too few arguments were used. – krysznys Jan 06 '21 at 22:09
  • 3
    @krysznys , where did you get ` int main(char* argv, int argc)` - it's usually this: https://stackoverflow.com/questions/3024197/what-does-int-argc-char-argv-mean – Halt State Jan 06 '21 at 22:26
  • @HaltState, corrected it. – krysznys Jan 06 '21 at 22:34
  • 2
    It looks like you have full function definitions in your header, not just declarations. That is not the reason for your segfault, but it is highly non-idiomatic, and it would cause you link problems if you had more than one source file using that header. – John Bollinger Jan 06 '21 at 22:41

0 Answers0