From: Igor Sugak <sugak@fb.com>
Date: Tue, 15 Mar 2016 20:00:36 +0000 (-0700)
Subject: folly/subprocess: fix one -Wsign-conversion and clag 3.9 test
X-Git-Tag: 2016.07.26~439
X-Git-Url: http://demsky.eecs.uci.edu/git/?a=commitdiff_plain;h=8c3d9fba1d1feabdc4d6c176dd9bb93f4a41c90c;p=folly.git

folly/subprocess: fix one -Wsign-conversion and clag 3.9 test

Summary:easy:
`options.parentDeathSignal_` is `int`, and second argument of `prctl` is `unsigned long`. Adding explicit cast.

hard:
this change is fixing a real problem when building folly with Clang 3.9.0. The [[ https://github.com/llvm-mirror/clang/commit/f4ddf94ecbd9899a497151621f3871545e24e93b | new alignment implementation ]] in `-O3` generates code that breaks folly's `ParentDeathSubprocessTest.ParentDeathSignal`.
For the following snipped of code:
```lang=cpp
  if (options.parentDeathSignal_ != 0) {
    if (prctl(PR_SET_PDEATHSIG, options.parentDeathSignal_, 0, 0, 0) == -1) {
      return errno;
    }
  }
```
LLVM generates the following assembly:
```lang=asm
   0x000000000040a77b <+347>:   mov    0x28(%r14),%rsi
   0x000000000040a77f <+351>:   test   %esi,%esi
   0x000000000040a781 <+353>:   je     0x40a7a1 <folly::Subprocess::prepareChild(folly::Subprocess::Options const&, __sigset_t const*, char const*) const+385>
   0x000000000040a783 <+355>:   mov    $0x1,%edi
   0x000000000040a788 <+360>:   xor    %edx,%edx
   0x000000000040a78a <+362>:   xor    %ecx,%ecx
   0x000000000040a78c <+364>:   xor    %r8d,%r8d
   0x000000000040a78f <+367>:   xor    %eax,%eax
   0x000000000040a791 <+369>:   callq  0x405ad0 <prctl@plt>
```
`%rsi` is a 64-bit register, on the line 1 value of `0x28(%r14)` is loaded to this register. That address points to `options.parentDeathSignal_`, which is a 32-bit value, set to 10 somewhere in a test. After that load:
```
rsi            0x7f000000000a
```
Notice it is not 10 (0xa), there is some garbage value in the upper part of the register. On the line 2, the lower part of this register is checked if it is zero, which corresponds to the first if statement. Then lines 4-8 prepare for `prctl` call. The value of the second argument is passed via `%rsi`, but the upper 32 bits were not cleared. This makes the function call generate `Invalid argument` error.
With the added explicit cast, notice a call `movslq %eax,%rsi`, which moves a signed 32-bit value of `%eax` (which contains `options.parentDeathSignal_`) to unsigned 64-bin `%rsi`:
```
   0x000000000040a77b <+347>:   mov    0x28(%r14),%rax
   0x000000000040a77f <+351>:   test   %eax,%eax
   0x000000000040a781 <+353>:   je     0x40a7a4 <folly::Subprocess::prepareChild(folly::Subprocess::Options const&, __sigset_t const*, char const*) const+388>
   0x000000000040a783 <+355>:   movslq %eax,%rsi
   0x000000000040a786 <+358>:   mov    $0x1,%edi
   0x000000000040a78b <+363>:   xor    %edx,%edx
   0x000000000040a78d <+365>:   xor    %ecx,%ecx
   0x000000000040a78f <+367>:   xor    %r8d,%r8d
   0x000000000040a792 <+370>:   xor    %eax,%eax
   0x000000000040a794 <+372>:   callq  0x405ad0 <prctl@plt>
```

Reviewed By: yfeldblum

Differential Revision: D3051611

fb-gh-sync-id: fc0f809bc4be0eed79dc5a701717efa27fedc022
shipit-source-id: fc0f809bc4be0eed79dc5a701717efa27fedc022
---

diff --git a/folly/Subprocess.cpp b/folly/Subprocess.cpp
index de5b109e..c40630a5 100644
--- a/folly/Subprocess.cpp
+++ b/folly/Subprocess.cpp
@@ -480,7 +480,9 @@ int Subprocess::prepareChild(const Options& options,
 #if __linux__
   // Opt to receive signal on parent death, if requested
   if (options.parentDeathSignal_ != 0) {
-    if (prctl(PR_SET_PDEATHSIG, options.parentDeathSignal_, 0, 0, 0) == -1) {
+    const auto parentDeathSignal =
+        static_cast<unsigned long>(options.parentDeathSignal_);
+    if (prctl(PR_SET_PDEATHSIG, parentDeathSignal, 0, 0, 0) == -1) {
       return errno;
     }
   }