[libcxxabi] Fix NO_THREADS version of test_exception_storage.pass.cpp

[DanielMcIntosh-IBM]

Summary

thread_code returns param, which for NO_THREADS is going to be
&thread_globals. Thus, the return value will never be null. The test
was probably meant to check if *thread_code(&thread_globals) == 0.
However, to avoid the extra cast, and to bring the NO_THREADS version
more in line with the regular version of the test, this changes it to
check if thread_globals == 0 directly.

[jroelofs]

Looks ok to me, that said I'm not one of this project's group reviewers, so you'll need someone else's approval.

[ikudrin]

It is better to split the patch so that the fix is separated from other improvements.

As for the other two patches, I am afraid I am not the right person to review them.

[DanielMcIntosh-IBM]

In D113048#3104321, @jroelofs wrote:

Looks ok to me, that said I'm not one of this project's group reviewers, so you'll need someone else's approval.

Thanks for the review (ish) @jroelofs. I'm aware that you're not one of the libc++ reviewers, but I still wanted your feedback because you were the author (and reviewer?) for D3386. In addition to being closely related to D113058, D113054, D113065, D113066, D113069, D112567 and D110351, which basically address a run-time dependent equivalent to NO_THREADS, it was also the change that introduced the if (thread_code(&thread_globals) == 0) line this change is supposed to fix.
With that in mind, even though you can't give the final approval, I still feel that your feedback on this and the others would be valuable.

Similarly, @ikudrin, I added you to this as well as D113054 and D112567 because of your authorship of D17815, which was somewhat related. I know in this case it isn't that closely related, but most of the people who have done related work are no longer active.

I now have a web of 16 inter-related changes and another 3 unrelated ones all mostly waiting on review by ldionne, which is a rather large review load for one person with other responsibilities. If I'm going to have any chance of landing them in a reasonable time frame, I need to spread it out a bit. Unfortunately, there aren't many people suitable for reviewing them who are still active other than Louis, so I would appreciate any feedback either of you can give on the changes I've requested your review of. Thank you in advance.

[DanielMcIntosh-IBM]

Remove the <cstdlib> to <cstddef> change as per ikudrin's comment.

[DanielMcIntosh-IBM]

In D113048#3105916, @ikudrin wrote:

It is better to split the patch so that the fix is separated from other improvements.

As for the other two patches, I am afraid I am not the right person to review them.

As for specifically the issue of splitting the patch up:

• The early returns kind of go hand in hand with fixing the NO_THREADS part of the test, since that's the only place we can do the early return, so I'm inclined to keep them together.
• arc won't let me submit it without removing the extra space on line 44
• for the cstdlib to cstddef change, I'd rather just drop it entirely, as I have done than create a separate review for it,

[ldionne]

This looks reasonable to me. I agree this was probably the intention all along.

[DanielMcIntosh-IBM]

No change - rebase to re-run CI now that main is green

libcxxabi/test/test_exception_storage.pass.cpp

 }
 
 #ifndef _LIBCXXABI_HAS_NO_THREADS
 #define NUMTHREADS  10
 size_t                 thread_globals [ NUMTHREADS ] = { 0 };
 std::__libcpp_thread_t   threads        [ NUMTHREADS ];
 #endif
 
-int main () {
-    int retVal = 0;
-
+int main() {
 #ifndef _LIBCXXABI_HAS_NO_THREADS
 //  Make the threads, let them run, and wait for them to finish
     for ( int i = 0; i < NUMTHREADS; ++i )
         std::__libcpp_thread_create ( threads + i, thread_code, (void *) (thread_globals + i));
     for ( int i = 0; i < NUMTHREADS; ++i )
         std::__libcpp_thread_join ( &threads [ i ] );
 
+    int retVal = 0;
     for ( int i = 0; i < NUMTHREADS; ++i ) {
         if ( 0 == thread_globals [ i ] ) {
             std::printf("Thread #%d had a zero global\n", i);
             retVal = 1;
         }
     }
 
     std::sort ( thread_globals, thread_globals + NUMTHREADS );
     for ( int i = 1; i < NUMTHREADS; ++i ) {
         if ( thread_globals [ i - 1 ] == thread_globals [ i ] ) {
             std::printf("Duplicate thread globals (%d and %d)\n", i-1, i);
             retVal = 2;
         }
     }
+    return retVal;
 #else // _LIBCXXABI_HAS_NO_THREADS
     size_t thread_globals;
+    thread_code(&thread_globals);
     // Check that __cxa_get_globals() is not NULL.
-    if (thread_code(&thread_globals) == 0) {
-        retVal = 1;
-    }
+    return (thread_globals == 0) ? 1 : 0;
 #endif // !_LIBCXXABI_HAS_NO_THREADS
-    return retVal;
 }