2

Update: Removing the print statement (line 52) entirely with all else the same as in the code below gave me 0 errors. The errors exist within my functions when printing (using philo[i]) and in this print statement (added for debugging), but do not exist if I run the entire program with no prints. Any ideas what I'm doing wrong???

Thank you for the help so far. I have made a couple of changes based on comments received so far.

***** Original (Edited) Question *****

I can't figure out why I have been getting this error, "Uninitialised value was created by a stack allocation". I started with a completed program that works fine, but gives me a ton of uninitialized value errors. I have traced the problem down to a few lines of code by excluding all functions and adding additional print statements. This code is to solve the dining philosopher's problem using threads (homework), so I don't want to post too much. My code is now:

#include <all needed header files>

#define NUM_PHIL       5
#define MIN_EAT_TIME   10

pthread_t philosopher[NUM_PHIL];         // array to hold IDs for philosopher threads
pthread_mutex_t chopstick[NUM_PHIL];     // array to hold IDs for chopstick mutexes (locks)

// function definitions here: 
int philosopherFun(int *philo);
// All others have been bypassed at the time of my current problem

int main(int argc, char *argv[]) {
    
    int phil[NUM_PHIL];                  // Philosopher numbers ("names")
    
    for(int i = 0; i < NUM_PHIL; i++) {                 
        phil[i] = i + 1;
    }
    
    for(int i = 0; i < NUM_PHIL; i++) {                 
        // Print statement causes valgrind error to exist (as does calling a function using phil)
        printf("Value phil[%d] = %d\n", i, phil[i]);
    }
    
    // Initilize mutexes for chopsticks, report error as needed
    for(int i = 0; i < NUM_PHIL; i++) {
        if(pthread_mutex_init( stuff here) < 0) {
            // error reporting
            // print statement uses i
        }
    }

    for(int i = 0; i < NUM_PHIL; i++) {
        if(pthread_create(&philosopher[i], NULL, (void*) philosopherFun, (void*) &phil[i] ) != 0) {
            // error reporting
            // print statement uses phil[i]
        }
    }
    
    /* Code omitted as this is Homework */
    // Join threads created for philosophers (using pthread_join)
            // error reporting
            // print statement uses phil[i]

    // Destroy chopstick mutexes when done. (using pthread_mutex_destroy)
            // error reporting
            // print statement uses i

    printf("The philosophers have all finished eating and its time for bed. Goodnight...\n");
    return 0;
    
}

int philosopherFun(int *philo) {
    
    return 0;
}

Program output:
Value phil[0] = 1
Value phil[1] = 2
Value phil[2] = 3
Value phil[3] = 4
Value phil[4] = 5
The philosophers have all finished eating and its time for bed. Goodnight...

My Valgrind errors are:
==46556== HEAP SUMMARY:
==46556== in use at exit: 0 bytes in 0 blocks
==46556== total heap usage: 0 allocs, 0 frees, 0 bytes allocated
==46556==
==46556== All heap blocks were freed -- no leaks are possible
==46556==
==46556== ERROR SUMMARY: 10 errors from 1 contexts (suppressed: 0 from 0)
==46556==
==46556== 10 errors in context 1 of 1:
==46556== Conditional jump or move depends on uninitialised value(s)
==46556== at 0x7FFF205A395F: ??? (in /dev/ttys000)
==46556== by 0x7FFF2046FFFA: ??? (in /dev/ttys000)
==46556== by 0x7FFF20478CF0: ??? (in /dev/ttys000)
==46556== by 0x7FFF2049D8B8: ??? (in /dev/ttys000)
==46556== by 0x7FFF20475EF5: ??? (in /dev/ttys000)
==46556== by 0x7FFF20474061: ??? (in /dev/ttys000)
==46556== by 0x1000038CD: main (philo.c:52)
==46556== Uninitialised value was created by a stack allocation
==46556== at 0x7FFF20475FDF: ??? (in /dev/ttys000)
==46556==
==46556== ERROR SUMMARY: 10 errors from 1 contexts (suppressed: 0 from 0)

line 52 is my print statement: printf("Value phil[%d] = %d\n", i, phil[i]);

which I believe accounts for 5 errors (phil[0] - phil[4]) and I call philosopherFun in my pthread_create on line 68 (this line of code included), again accounting for 5 errors (1 for each thread). This function is currently only returning on the first line, so none of the rest of my program is involved (though I started with 50-250 errors depending on my parameters and which functions I excluded).

I feel like my int array (int phil[]) is causing the problem, but it is immediately initialized, so I'm not sure how this is happening. Please help!

Thanks, Kristine

Edited... Added some comments from omitted code- Threads are joined and mutexes destroyed

I also tried making int phil[NUM_PHIL] global by declaring it outside of my main function, but this made no change to the errors returned by Valgrind. defining phil[5] = {1, 2, 3, 4, 5}; as a global also didn't help.

kris2401
  • 21
  • 2
  • 1
    You can use the `-g` compilation flag to add debugging symbols. This will help programs like valgrind give more helpful output. – Daniel Walker Mar 24 '21 at 00:20
  • 'int phil[NUM_PHIL];' does that array still exist when the thread/s try to access it? – Martin James Mar 24 '21 at 00:22
  • Comments like `// Numbered 1 to i + 1` out to the right like that are worse than useless. You spent a good bit of effort formatting those comments to line up there, and that's in a location that distracts the eyes from looking at the code. Not only that, the comment merely repeats what the code obviously does. That's wasted effort to write a distracting comment that adds no information. – Andrew Henle Mar 24 '21 at 00:29
  • This is also bothersome: `(void*) philosopherFun` Why are you casting `philsopherFun` to `void *`? Using casts to paste over errors is a sign you're doing something wrong. – Andrew Henle Mar 24 '21 at 00:37
  • 1
    With basic fixes to compile, the valgrind error doesn't appear in the code as posted. Please update your question with a [mcve]. – dbush Mar 24 '21 at 00:41
  • Return type of `philosopherFun()` should be `void *`. – H.S. Mar 24 '21 at 03:22
  • `main` should not exit until the threads have been joined – M.M Mar 24 '21 at 03:25
  • Can you make this a true [mcve] that can actually be compiled and run, without stuff left out? I wanted to try and test the program to help you out, but it's a big pain to guess at the headers, what goes in your `stuff here`, and so on. I tried to guess but I got no errors from valgrind. – Nate Eldredge Mar 24 '21 at 05:50
  • What kind of environment are you using? Library functions often cause false positive valgrind errors because they may intentionally read uninitialized memory in some cases (they know what they're doing). On major OS platforms, valgrind is configured to ignore known instances of this, but if you're on some obscure system, that may not be set up correctly. – Nate Eldredge Mar 24 '21 at 05:54

1 Answers1

1

I believe this is where your problem comes from:

int phil[NUM_PHIL];

and here

if(pthread_create(&philosopher[i], NULL, (void*) philosopherFun, (void*) &phil[i] ) != 0) {
    more code ...
}

You're referencing a stack-allocated object (you're getting a pointer to the elements of the philo array). When the main function returns, the memory will be deleted and the other threads will still be holding on to garbage. To be sure this is the problem, move your philo array to a global scope, or allocate the array on the heap instead.

Adebayo
  • 175
  • 8
  • 1
    *When the main function returns...* True, but the program exits when `main()` returns. And that wouldn't cause a `printf()` in the same scope as the array that's accessing the array to emit Valgrind errors. *move your philo array to a global scope, or allocate the array on the heap instead* It wouldn't surprise me to see that cause the error to disappear, but with the details of the errors as currently given I'd think that would merely hide the symptom and not solve whatever the root cause of the issue is. I suspect that the root cause is somehow in the omitted code. – Andrew Henle Mar 24 '21 at 00:35
  • I see your point, but it's merely for debugging. I asked him to do these so he can figure out exactly where the errors are coming from – Adebayo Mar 24 '21 at 00:50
  • Moving the array outside of the main function, making it global, has no impact on the error received by Valgrind – kris2401 Mar 24 '21 at 05:06
  • Anyhow `philosopherFun()` never actually dereferences that pointer; it doesn't do anything at all. – Nate Eldredge Mar 24 '21 at 05:47
  • @NateEldredge It does dereference the pointer, here: if(pthread_create(&philosopher[i], NULL, (void*) philosopherFun, (void*) &phil[i] ) != 0) { the &phil[i] is getting the address of one of the array elements. That's where the issue stems from as far as I can see – Adebayo Mar 24 '21 at 22:47
  • @AdebayoJagunmolu: Yes, but that's in the main thread. The "dereference" happens in computing the arguments to `pthread_create`, before the new thread is started, and there is no doubt that `phil` is still alive at that time. (Moreover, it's not really a dereference at all; `&phil[i]` is equivalent to `&*(phil+i)` is equivalent to `phil+i`, see https://stackoverflow.com/a/51691409/634919.) The new thread itself only runs the function `philosopherFun()` which does not use its argument in any way. – Nate Eldredge Mar 25 '21 at 01:08