Return to Answer

add assembly

Source Link

edited Jul 15, 2015 at 6:10

1.1k
8
18

PIDs should be stored in a variable of type pid_t.

Any memory that you read/write from multiple processes (or threads) must be protected either by some sort of mutex or by atomic operations with an appropriate memory order. It would not be enough to volatile.

PIDs should be stored in a variable of type pid_tEdit: It does indeed happen. Looking at the relevant assembly (at -O3):

# *flag = 1;
  400840:       mov    DWORD PTR [r12],0x1
# child_pid = fork();
  400848:       call   400720 <fork@plt>
  40084d:       mov    esi,0x4009c0
  400852:       mov    r14d,eax
  400855:       mov    edi,0x2
# signal(SIGINT, intHandler); - oh, this should be before the fork!
  40085a:       call   4006c0 <__sysv_signal@plt>
# 3-way branch for child_pid
  40085f:       test   r14d,r14d
  400862:       js     400885 <main+0x155>
  400864:       je     400897 <main+0x167>
# child_pid > 0
  400866:       mov    edx,DWORD PTR [r12]
# if (*flag) - condition only tested once
  40086a:       test   edx,edx
  40086c:       je     400870 <main+0x140>
# infinite loop
  40086e:       jmp    40086e <main+0x13e>
# else
  400870:       mov    esi,DWORD PTR [r13+0x0]
  400874:       mov    edi,0x400af2
  400879:       xor    eax,eax
# printf("The sum is: %d\n", *sum);
  40087b:       call   400680 <printf@plt>
  400880:       jmp    400772 <main+0x42>
# child_pid < 0
  400885:       mov    edi,0x400ae5
# printf("Fork error. \n");
  40088a:       call   400660 <puts@plt>
  40088f:       or     edi,0xffffffff
# exit(-1); - should use 1 or EXIT_FAILURE actually.
  400892:       call   400710 <exit@plt>
# child_pid == 0
  400897:       mov    edi,0x2
  40089c:       mov    esi,0x1
# signal(SIGINT, SIG_IGN); //ignore ctrl+c
  4008a1:       call   4006c0 <__sysv_signal@plt>
  4008a6:       mov    eax,DWORD PTR [rbx]
  4008a8:       add    eax,DWORD PTR [rbp+0x0]
  4008ab:       xor    edi,edi
# *sum = *num1 + *num2;
  4008ad:       mov    DWORD PTR [r13+0x0],eax
# *flag = 0;
  4008b1:       mov    DWORD PTR [r12],0x0
# exit(0);
  4008b9:       call   400710 <exit@plt>

PIDs should be stored in a variable of type pid_t.

Edit: It does indeed happen. Looking at the relevant assembly (at -O3):

# *flag = 1;
  400840:       mov    DWORD PTR [r12],0x1
# child_pid = fork();
  400848:       call   400720 <fork@plt>
  40084d:       mov    esi,0x4009c0
  400852:       mov    r14d,eax
  400855:       mov    edi,0x2
# signal(SIGINT, intHandler); - oh, this should be before the fork!
  40085a:       call   4006c0 <__sysv_signal@plt>
# 3-way branch for child_pid
  40085f:       test   r14d,r14d
  400862:       js     400885 <main+0x155>
  400864:       je     400897 <main+0x167>
# child_pid > 0
  400866:       mov    edx,DWORD PTR [r12]
# if (*flag) - condition only tested once
  40086a:       test   edx,edx
  40086c:       je     400870 <main+0x140>
# infinite loop
  40086e:       jmp    40086e <main+0x13e>
# else
  400870:       mov    esi,DWORD PTR [r13+0x0]
  400874:       mov    edi,0x400af2
  400879:       xor    eax,eax
# printf("The sum is: %d\n", *sum);
  40087b:       call   400680 <printf@plt>
  400880:       jmp    400772 <main+0x42>
# child_pid < 0
  400885:       mov    edi,0x400ae5
# printf("Fork error. \n");
  40088a:       call   400660 <puts@plt>
  40088f:       or     edi,0xffffffff
# exit(-1); - should use 1 or EXIT_FAILURE actually.
  400892:       call   400710 <exit@plt>
# child_pid == 0
  400897:       mov    edi,0x2
  40089c:       mov    esi,0x1
# signal(SIGINT, SIG_IGN); //ignore ctrl+c
  4008a1:       call   4006c0 <__sysv_signal@plt>
  4008a6:       mov    eax,DWORD PTR [rbx]
  4008a8:       add    eax,DWORD PTR [rbp+0x0]
  4008ab:       xor    edi,edi
# *sum = *num1 + *num2;
  4008ad:       mov    DWORD PTR [r13+0x0],eax
# *flag = 0;
  4008b1:       mov    DWORD PTR [r12],0x0
# exit(0);
  4008b9:       call   400710 <exit@plt>

Source Link

answered Jul 15, 2015 at 5:50

o11c

1.1k
8
18

There are a lot of functions that aren't safe to call in a signal handler. For the few that are allowed, look at the Async-signal-safe functions section of the signal(7) man page.

Your current signal handler is almost okay, but any variable that might be changed from a signal handler needs to be declared volatile. Officially it must also be sig_atomic_t instead of int, though I'm not sure how important that is on real-world platforms.

You should be checking the return value of printf and scanf. They will return EOF (which is usually -1 but could theoretically be any negative number) with errno set to EINTR if the signal has been delivered during the underlying system call. That said, there is nothing to prevent the signal from being delivered before or after the syscall itself (before scanf is the nasty case, and doing the syscall yourself won't help unless the signal is blocked around the call).

If you want to handle signals in any sort of sane way, you need to sigblock them, at least some of the time. Either then unblock them during "safe" runs of code and check the flag periodically or else leave them blocked call sigwait (which will not invoke the handler).

My preference is to then use signalfd so that I can stuff it in a select or poll or epoll set along with any other read- or write- file descriptors.

There is no good reason for you to make 4 separate mmap calls. Most likely a single call for 4 * sizeof(int) would be enough, but even if you have a pressing need for them to be on separate pages, you should still mmap 4 pages in a single call.

You don't need to open "/dev/zero" to get anonymous memory, just pass -1 as the FD and add the MAP_ANONYMOUS flag.

Your child should call _exit, not exit.

So the following line may be optimized to be an infinite loop:

while (*flag);

PIDs should be stored in a variable of type pid_t.