0

I have written the following code, but want to know why this program is exhibiting different behavior with compiler optimization

#include <stdio.h>
#include <signal.h>
#include <unistd.h>
#include <pthread.h>

unsigned int counter = 0;

void *thread0_func(void *arg)
{
    unsigned int i = 0;
    printf("Thread 0\n");
    while(1)
    {
        sleep(3);
        counter++;
    }

    return NULL;
}

void action_handler(int sig_no)
{
    printf("SigINT Generated: %d\n",counter);
    counter += 1;
}

int main(int argc, char **argv)
{
    pthread_t thread_id[2];

    struct sigaction sa;

    sa.sa_handler = action_handler;

    if(sigaction(SIGINT, &sa, NULL))
        perror("Cannot Install Sig handler");


    if(pthread_create(&thread_id[0], NULL, thread0_func, NULL))
    {
        perror("Error Creating Thread 0");
    }
    else
    {

    }
    while(counter == 0);
    printf("Counter: %d\n", counter);

//  pthread_join(thread_id[0], NULL);
    
    return (0);
}

Under normal compilation condition (without optimization), ie. gcc example.c -o example -pthread the program works as per the expected logic written i.e. the program waits on the instruction while(counter == 0); for variable counter value to become non zero either from thread or from signal handler, then, loop breaks and the program prints the value and exit.

If the code is compiled with gcc -O3 example.c -o example -pthread, then the program keeps running and incrementing the value from thread as well as whenever signal handler gets executed.

$ ./example 
Thread 0
^CSigINT Generated: 4
^CSigINT Generated: 6
^CSigINT Generated: 8
^CSigINT Generated: 9
^CSigINT Generated: 11
^CSigINT Generated: 12

But some people says that this code is not compliant and will produce a UB across different platforms!

August Karlstrom
  • 10,773
  • 7
  • 38
  • 60
Gaurav Pathak
  • 1,065
  • 11
  • 28
  • sounds like it does and you saw this when it doesn't stop with -O3. I'll let someone who knows the rules better explain why – user253751 Mar 17 '23 at 15:47
  • 3
    The code doesn't "produce" undefined behavior. A subset of the behavior of a program which behavior is undefined is the correct behavior. And as such showing that a program operates correctly is not a proof against it. If a program exhibits undefined behavior, that means it _can_ break, not that it will always break. – Bartek Banachewicz Mar 17 '23 at 15:47
  • My guess is that your loop is "optimized" to `if ( counter==0) while(1);` – tstanisl Mar 17 '23 at 15:53
  • At least add `volatile`: `volatile unsigned int counter = 0;`. – pts Mar 17 '23 at 15:53
  • 2
    The update of `counter` in one thread with a read of `counter` in another thread is a data race, and data races are undefined behavior (according to the C11 memory model). – Paul Hankin Mar 17 '23 at 15:54
  • `But some people says` Who exactly? Did you ask them? – KamilCuk Mar 17 '23 at 15:56
  • 1
    One way to avoid data races is using mutexes (see the `pthread_mutex_init` and `pthread_mutex_lock` functions). Another way is using atomic integers (available in C11 and later, see https://en.cppreference.com/w/c/atomic). – pts Mar 17 '23 at 16:00
  • 1
    `while(counter == 0);` is undefined behavior, nothing is happening in the loop. `sigaction(SIGINT, &sa` is using indeterminate values from `sa`, which most probably results in undefined behavior. `sa` is not initialized fully. – KamilCuk Mar 17 '23 at 16:00

1 Answers1

4
void action_handler(int sig_no)
{
    printf("SigINT Generated: %d\n",counter);
    counter += 1;
}

The signal handler invokes undefined behaviour because it calls an async-signal-unsafe function.

From the C standard:

ISO/IEC 9899:2011 §7.14.1.1 The signal function ¶5 If the signal occurs other than as the result of calling the abort or raise function, the behavior is undefined if the signal handler refers to any object with static or thread storage duration that is not a lock-free atomic object other than by assigning a value to an object declared as volatile sig_atomic_t, or the signal handler calls any function in the standard library other than the abort function, the _Exit function, the quick_exit function, or the signal function with the first argument equal to the signal number corresponding to the signal that caused the invocation of the handler. Furthermore, if such a call to the signal function results in a SIG_ERR return, the value of errno is indeterminate.252)

Furthermore, it modifies a variable that is not of type volatile sigatomic_t.

The first problem can be eliminated by using the write() system call (POSIX defines it to be async-signal-safe), and the latter by declaring the variable as:

volatile sig_atomic_t counter;

That is not all!

struct sigaction sa;
sa.sa_handler = action_handler;

The sa struct was never fully initialised, only the sa_handler member was. The rest of the members remain uninitialised and have indeterminate values. The call to sigaction() is then passed an uninitialized struct which would presumably invoke undefined behaviour.¹ — @KamilCuk

Either use an explicit initializer to zero it out:

struct sigaction sa = { 0 };

Or alternatively, use memset().

There may still be some other problems with your code.

Harith
  • 4,663
  • 1
  • 5
  • 20