<div dir="ltr">You can write a test that has high chance of failure when this patch is not applied. This can be done by let child process to sleep a few ms before existing. The period of waiting can be controlled by a command line argument.<div><br></div><div>Things to test:</div><div><br></div><div>1) make sure the profile data is not zero sized</div><div>2) make sure the merged profile from parent and child is created (using llvm-profdata to dump)</div><div><br></div><div>3) repeat step 1) and 2) with different waiting parameters (e.g, 3 different cases).</div><div><br></div><div>David</div></div><div class="gmail_extra"><br><div class="gmail_quote">On Wed, Feb 15, 2017 at 9:55 AM, Rong Xu <span dir="ltr"><<a href="mailto:xur@google.com" target="_blank">xur@google.com</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div dir="ltr">It's not easy to write a test case for this. We need the extended period of profile write to expose the faulty behavior. This is not easy for small test programs.<div>I can register another atexit function that opens and locks the profile to mimic this.</div><div>The flow is like:</div><div>main process:  __llvm_profile_write_file, open_profile, lock_profile, close_profile, _exit</div><div>child process:    sleep                                  __llvm_profile_write</div><div><br></div><div>But this is not really testing the code directly. </div><div>Another thing is since merge is only enabled with %m specifier, i need to get the string expansion for %m, or I need to read the directory to find the profile. This is quite cumbersome.</div><div><br></div><div>Or a simple version is just a test that using small profile and let the race happen.</div><div>There are two outcomes: (1) if the main process exits before the child process voiding the sigkill, we have the profile for the main process only.</div><div>(2) if the child process get time to void sigkill, we get merged profile.</div><div>The check would assert that 0-size profile should not happen.</div><div><br></div><div>Do you have better idea?<br></div><div><br></div></div><div class="HOEnZb"><div class="h5"><div class="gmail_extra"><br><div class="gmail_quote">On Wed, Feb 15, 2017 at 9:38 AM, Rong Xu <span dir="ltr"><<a href="mailto:xur@google.com" target="_blank">xur@google.com</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div class="m_-4523082579781045026HOEnZb"><div class="m_-4523082579781045026h5"><div dir="ltr"><br></div><div class="gmail_extra"><br><div class="gmail_quote">On Tue, Feb 14, 2017 at 1:05 PM, David Li via Phabricator <span dir="ltr"><<a href="mailto:reviews@reviews.llvm.org" target="_blank">reviews@reviews.llvm.org</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">davidxl added a comment.<br>
<br>
Can you create a test case? It can be put under the Linux subdir.<br>
<br>
<br>
<a href="https://reviews.llvm.org/D29954" rel="noreferrer" target="_blank">https://reviews.llvm.org/D2995<wbr>4</a><br>
<br>
<br>
<br>
</blockquote></div><br></div>
</div></div></blockquote></div><br></div>
</div></div></blockquote></div><br></div>