[libc-commits] [PATCH] D74949: [libc] Create abort and _Exit

[Alex Brachet via Phabricator via libc-commits]

abrachet marked 6 inline comments as done.
abrachet added inline comments.


================
Comment at: libc/src/stdlib/abort.cpp:19
+  __llvm_libc::raise(SIGABRT);
+  __llvm_libc::raise(SIGKILL);
+  __llvm_libc::_Exit(127);
----------------
sivachandra wrote:
> abrachet wrote:
> > MaskRay wrote:
> > > If there is a signal handler for SIGABRT, the code should try blocking the signal. Since signals are not available, this is fine.
> > I have added the above TODO. Maybe it is wrong as I think more about it though, maybe `sigprocmask` is a better choice to remove the signal handler.
> > 
> > I think there also needs to be some kind of lock once we start playing with signal handlers. WDYT?
> Yeah. Please add a big TODO comment explaining what you intend to do in follow-up patches. To get it right, I agree with you that we will need a lock (or multiple locks).  So, once the mutex patch lands, we can get back to finishing this.
> Yeah. Please add a big TODO comment explaining what you intend to do in follow-up patches. To get it right, I agree with you that we will need a lock (or multiple locks). So, once the mutex patch lands, we can get back to finishing this.

I have added what I think I will do in the future but the more I think about this the more complicated it is. The future patches here should be interesting :)



================
Comment at: libc/src/stdlib/abort.cpp:18
+void LLVM_LIBC_ENTRYPOINT(abort)() {
+  // TODO: When sigprocmask and sigaction land:
+  // Unblock SIGABRT, raise it, change its action to SIGDFL, raise it again.
----------------
sivachandra wrote:
> Do you need `sigaction` also?
Yes to remove the user installed `SIGABRT` handler.


================
Comment at: libc/src/stdlib/abort.cpp:19
+  // TODO: When sigprocmask and sigaction land:
+  // Unblock SIGABRT, raise it, change its action to SIGDFL, raise it again.
+  // TODO: When C11 threads land:
----------------
sivachandra wrote:
> I think you mean `SIG_DFL`?
Indeed, good catch


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

https://reviews.llvm.org/D74949