[libc-commits] [PATCH] D75802: [libc] Add sigaction

[Siva Chandra via Phabricator via libc-commits]

sivachandra added inline comments.


================
Comment at: libc/test/src/signal/sigaction_test.cpp:36
+
+TEST(Sigaction, CustomAction) {
+  // Zero this incase tests get run multiple times in the future.
----------------
abrachet wrote:
> sivachandra wrote:
> > This test is crashing for me. I have not investigated why, but GDB is telling me that `SIGUSR1` is not getting caught after line 49.
> Does it crash with D76271 or just as is? My guess is its a problem with how `__restore_rt` is being compiled. Also, does it crash outside of GDB or just in GDB?
Ah, looks like a bad interaction between the `NOHANG` race and this test. I used Paula's patch and the test passes.

So, the way I use the build rules is this:

```
add_object(
  __restore
  SRC
    __restore.cpp
  COMPILE_OPTIONS
    -fomit-frame-pointer
    -O3
    -Wframe-larger-than=0
  DEPENDS
    linux_syscall_h
    sys_syscall_h
)

add_entrypoint_object(
  sigaction
  SRCS
    sigaction.cpp
  HDRS
    signal.h
    ../sigaction.h
  DEPENDS
    sys_syscall_h
    linux_syscall_h
    signal_h
  SPECIAL_OBJECTS
    __restore
)
```

We need the construct like `SPECIAL_OBJECTS` because it cannot be a normal dep: we need to get the object file out of it. With the above rules, the symtab in `sigaction.o` is like this:

```
Symbol table '.symtab' contains 21 entries:
   Num:    Value          Size Type    Bind   Vis      Ndx Name
     0: 0000000000000000     0 NOTYPE  LOCAL  DEFAULT  UND 
     1: 0000000000000000     0 SECTION LOCAL  DEFAULT    1 
     2: 0000000000000000     0 SECTION LOCAL  DEFAULT    2 
     3: 0000000000000000     0 SECTION LOCAL  DEFAULT    3 
     4: 0000000000000000     0 SECTION LOCAL  DEFAULT    4 
     5: 0000000000000000     0 SECTION LOCAL  DEFAULT    5 
     6: 0000000000000000     0 SECTION LOCAL  DEFAULT    6 
     7: 0000000000000000     0 SECTION LOCAL  DEFAULT    7 
     8: 0000000000000000     0 SECTION LOCAL  DEFAULT    8 
     9: 0000000000000000     0 SECTION LOCAL  DEFAULT    9 
    10: 0000000000000000     0 SECTION LOCAL  DEFAULT   10 
    11: 0000000000000000     0 SECTION LOCAL  DEFAULT   11 
    12: 0000000000000000     0 SECTION LOCAL  DEFAULT   12 
    13: 0000000000000000     0 FILE    LOCAL  DEFAULT  ABS sigaction.cpp
    14: 0000000000000050    77 FUNC    LOCAL  DEFAULT    1 _ZN11__llvm_libcL13copySigactionI11__sigaction9sigactionEEvRT_RKT0_
    15: 0000000000000000    76 FUNC    LOCAL  DEFAULT    1 _ZN11__llvm_libcL13copySigactionI9sigaction11__sigactionEEvRT_RKT0_
    16: 0000000000000000     0 FILE    LOCAL  DEFAULT  ABS __restore.cpp
    17: 00000000000000a0     7 FUNC    GLOBAL DEFAULT    1 __restore_rt
    18: 0000000000000000     0 NOTYPE  GLOBAL DEFAULT  UND _ZN11__llvm_libc16__errno_locationEv
    19: 0000000000000000   461 FUNC    GLOBAL DEFAULT    2 _ZN11__llvm_libc9sigactionEiPK11__sigactionPS0_
    20: 0000000000000000     0 FUNC    GLOBAL DEFAULT    2 sigaction
```

That confirms `__restore.o` is getting packed into `sigaction.o`. Next, `objdump` shows this:

```
00000000000000a0 <__restore_rt>:
  a0:   b8 0f 00 00 00          mov    $0xf,%eax
  a5:   0f 05                   syscall 
```

This confirms that the compile options are being picked up alright.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D75802/new/

https://reviews.llvm.org/D75802