From mboxrd@z Thu Jan 1 00:00:00 1970 Path: news.gmane.org!not-for-mail From: Eli Zaretskii Newsgroups: gmane.emacs.devel Subject: Re: Dynamic loading progress Date: Fri, 20 Nov 2015 23:56:04 +0200 Message-ID: <83610w5o97.fsf@gnu.org> References: <83k2ptq5t3.fsf@gnu.org> <87h9kxx60e.fsf@lifelogs.com> <877flswse5.fsf@lifelogs.com> <8737wgw7kf.fsf@lifelogs.com> <87io5bv1it.fsf@lifelogs.com> <87egfzuwca.fsf@lifelogs.com> <876118u6f2.fsf@lifelogs.com> <8737w3qero.fsf@lifelogs.com> <831tbn9g9j.fsf@gnu.org> <878u5upw7o.fsf@lifelogs.com> <83ziya8xph.fsf@gnu.org> <83y4du80xo.fsf@gnu.org> <837fld6lps.fsf@gnu.org> Reply-To: Eli Zaretskii NNTP-Posting-Host: plane.gmane.org X-Trace: ger.gmane.org 1448056612 22295 80.91.229.3 (20 Nov 2015 21:56:52 GMT) X-Complaints-To: usenet@ger.gmane.org NNTP-Posting-Date: Fri, 20 Nov 2015 21:56:52 +0000 (UTC) Cc: aurelien.aptel+emacs@gmail.com, tzz@lifelogs.com, emacs-devel@gnu.org To: Philipp Stephani Original-X-From: emacs-devel-bounces+ged-emacs-devel=m.gmane.org@gnu.org Fri Nov 20 22:56:39 2015 Return-path: Envelope-to: ged-emacs-devel@m.gmane.org Original-Received: from lists.gnu.org ([208.118.235.17]) by plane.gmane.org with esmtp (Exim 4.69) (envelope-from ) id 1ZztfZ-0002RI-Fc for ged-emacs-devel@m.gmane.org; Fri, 20 Nov 2015 22:56:37 +0100 Original-Received: from localhost ([::1]:50027 helo=lists.gnu.org) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1ZztfY-0007Th-Rs for ged-emacs-devel@m.gmane.org; Fri, 20 Nov 2015 16:56:36 -0500 Original-Received: from eggs.gnu.org ([2001:4830:134:3::10]:54718) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1ZztfK-0007TM-Ru for emacs-devel@gnu.org; Fri, 20 Nov 2015 16:56:24 -0500 Original-Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1ZztfG-0003Mg-OX for emacs-devel@gnu.org; Fri, 20 Nov 2015 16:56:22 -0500 Original-Received: from mtaout21.012.net.il ([80.179.55.169]:48824) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1ZztfG-0003MS-Ak for emacs-devel@gnu.org; Fri, 20 Nov 2015 16:56:18 -0500 Original-Received: from conversion-daemon.a-mtaout21.012.net.il by a-mtaout21.012.net.il (HyperSendmail v2007.08) id <0NY400F00VD7BF00@a-mtaout21.012.net.il> for emacs-devel@gnu.org; Fri, 20 Nov 2015 23:56:16 +0200 (IST) Original-Received: from HOME-C4E4A596F7 ([84.94.185.246]) by a-mtaout21.012.net.il (HyperSendmail v2007.08) with ESMTPA id <0NY400FSFVLSAW10@a-mtaout21.012.net.il>; Fri, 20 Nov 2015 23:56:16 +0200 (IST) In-reply-to: X-012-Sender: halo1@inter.net.il X-detected-operating-system: by eggs.gnu.org: Solaris 10 X-Received-From: 80.179.55.169 X-BeenThere: emacs-devel@gnu.org X-Mailman-Version: 2.1.14 Precedence: list List-Id: "Emacs development discussions." List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: emacs-devel-bounces+ged-emacs-devel=m.gmane.org@gnu.org Original-Sender: emacs-devel-bounces+ged-emacs-devel=m.gmane.org@gnu.org Xref: news.gmane.org gmane.emacs.devel:194904 Archived-At: > From: Philipp Stephani > Date: Fri, 20 Nov 2015 19:58:25 +0000 > Cc: tzz@lifelogs.com, aurelien.aptel+emacs@gmail.com, emacs-devel@gnu.org > > So you are saying that module_init runs in the main thread, but > dynamic initializers might run in another thread? How's that > possible? > > According to http://stackoverflow.com/a/29776416 it is not guaranteed that > dynamic initializers run in the main thread. Ah, you are talking about C++ dynamic initializers! So the model is that someone writes a module in C++, starts a thread there, and then inside some dynamic initializer calls emacs-module interface functions, is that it? If that's the situation, I'd suggest a prominent commentary describing this in the source. > On what OS can this happen? It cannot happen on Windows, AFAIK, > > http://blogs.msdn.com/b/oldnewthing/archive/2010/08/27/10054832.aspx seems to > indicate that it is possible that other threads continue running after the main > thread has exited. Not in Emacs: we use the C runtime, and never call ExitThread, let alone ExitProcess. Doing that would be a silly bug. There's no need to make a single component of Emacs, and not the most important one, protected to such extreme degree against calamities that are only possible if Emacs developers will lose their senses. > My initial code didn't use a thread identifier at all because it could have > been reused after the main thread ended, introducing a subtle bug. No, it cannot be reused, because we hold a handle on the main thread, see w32term.c around line 6985. As long as a single handle is open on a thread, it still exists from the OS POV, and its ID cannot be reused. > > This is undefined behavior, but we included an explicit check if > > checking is enabled because that case is somewhat subtle. > > I'm surprised this could happen on some OS. Can you point to any > details about this? > > > It's not ruled out by the C standard or the APIs of operating systems. If Emacs > doesn't call ExitThread or manipulates the C runtime, then indeed it can't > happen (at least on Windows), but I think it's brittle to rely on such > subtleties without explicitly checking for them (somebody might introduce a > call to ExitThread in the future without looking at module code). I think these precautions are unnecessary. Anyway, thanks for explaining this, I now know how to change the code to DTRT on MS-Windows wrt to the thread checks. > OK, but then either there should have been a comment explaining this, > or, better, the test should have been after the addition. (Which, by > some strange luck -- or maybe something else -- is just what Paul > already did ;-) > > If the test gets moved after the addition, then we should have a verify > (MAX_EMACS_UINT - 1 > MOST_POSITIVE_FIXNUM) or use __builtin_add_overflow to > make sure the addition doesn't cause UB. I don't think so. Please take a look at the code Paul wrote there, I don't see a problem with it. > > No validity checks on 'fin'? > > > > How should it be validated? In C an arbitrary (invalid) pointer could be > > passed. I think we just have to accept that this is UB. > > Given the extensive checks elsewhere in the file, this surprised me, > that's all. How to test it? calling valid_pointer_p would be one > thing I'd suggest. Maybe there are other (perhaps platform-dependent) > checks possible here, I would have to look around. For example, it > would be good to make sure this points into executable code. > > You can't validate pointers in C. For example, is the following pointer valid? > > long a; > double *p = (double *)(&a); > > The answer is no; this is an aliasing violation, and dereferencing p is UB, but > you won't detect this by trying to read the process's address space. The function valid_pointer_p validates a pointer in the sense that it points into the program's address space. That's not a full validation, and won't catch unaligned pointers or pointers into non-executable portions of memory, but it's better than nothing. I'd certainly consider it under "#ifdef ENABLE_CHECKING" at least. > See also > http://blogs.msdn.com/b/oldnewthing/archive/2006/09/27/773741.aspx. We don't use IsBadWritePtr on Windows to check this, see w32_valid_pointer_p for how this is actually implemented. Anyway, I'm surprised by this extreme POV: even if we cannot validate a pointer 100%, surely it doesn't mean we cannot or shouldn't do some partial job? Why this "all or nothing" approach? > > if (len > INT_MAX || len < envptr->min_arity || (envptr->max_arity >= 0 > && > > len > envptr->max_arity)) > > xsignal2 (Qwrong_number_of_arguments, module_format_fun_env (envptr), > > make_number (len)); > > > > Why the test against INT_MAX? EMACS_INT can legitimately be a 64-bit > > data type with values far exceeding INT_MAX. Why impose this > > limitation here? > > > > Because the nargs argument in the module interface is an int. > > Shouldn't it be EMACS_INT instead? > > EMACS_INT is an internal type that isn't exposed to module authors. OK, but the type of nargs is ptrdiff_t, so the test should be against the maximum of that type. On 64-bit hosts, ptrdiff_t is a 64-bit type, while an int is still 32-bit wide. > Could xmalloc grow a variant that is guaranteed not to call longjmp? We need to devise a way for it to detect that it was called from emacs-module.c, the rest is simple, I think. Thanks.