From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 19225 invoked by alias); 24 Jul 2009 20:42:25 -0000 Received: (qmail 18967 invoked by uid 22791); 24 Jul 2009 20:42:21 -0000 X-SWARE-Spam-Status: No, hits=0.4 required=5.0 tests=AWL,BAYES_00,KAM_STOCKTIP,SPF_HELO_PASS,SPF_PASS X-Spam-Check-By: sourceware.org Received: from mx2.redhat.com (HELO mx2.redhat.com) (66.187.237.31) by sourceware.org (qpsmtpd/0.43rc1) with ESMTP; Fri, 24 Jul 2009 20:42:13 +0000 Received: from int-mx2.corp.redhat.com (int-mx2.corp.redhat.com [172.16.27.26]) by mx2.redhat.com (8.13.8/8.13.8) with ESMTP id n6OKg9u9004713; Fri, 24 Jul 2009 16:42:09 -0400 Received: from ns3.rdu.redhat.com (ns3.rdu.redhat.com [10.11.255.199]) by int-mx2.corp.redhat.com (8.13.1/8.13.1) with ESMTP id n6OKg85t012190; Fri, 24 Jul 2009 16:42:08 -0400 Received: from opsy.redhat.com (ovpn01.gateway.prod.ext.phx2.redhat.com [10.5.9.1]) by ns3.rdu.redhat.com (8.13.8/8.13.8) with ESMTP id n6OKg4uJ016629; Fri, 24 Jul 2009 16:42:04 -0400 Received: by opsy.redhat.com (Postfix, from userid 500) id 8982B5081C2; Fri, 24 Jul 2009 14:42:03 -0600 (MDT) To: Reid Kleckner Cc: gdb-patches@sourceware.org, unladen-swallow@googlegroups.com Subject: Re: [RFA] Add interface for registering JITed code References: <9a9942200907221615o570e749fh5cb186c1600f159c@mail.gmail.com> <9a9942200907240946q1546646ft6e9112f263bcefdf@mail.gmail.com> From: Tom Tromey Reply-To: Tom Tromey Date: Fri, 24 Jul 2009 21:06:00 -0000 In-Reply-To: <9a9942200907240946q1546646ft6e9112f263bcefdf@mail.gmail.com> (Reid Kleckner's message of "Fri\, 24 Jul 2009 09\:46\:52 -0700") Message-ID: User-Agent: Gnus/5.11 (Gnus v5.11) Emacs/22.3 (gnu/linux) MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: quoted-printable Mailing-List: contact gdb-patches-help@sourceware.org; run by ezmlm Precedence: bulk List-Id: List-Subscribe: List-Archive: List-Post: List-Help: , Sender: gdb-patches-owner@sourceware.org X-SW-Source: 2009-07/txt/msg00606.txt.bz2 >>>>> "Reid" =3D=3D Reid Kleckner writes: Reid> Here's an updated patch. Very quick! Reid> Where should this go? It doesn't really fit under any of the Reid> top-level topics, so far as I can tell. I only have about a page to Reid> write about it. Yeah, I don't really have a suggestion for this. Today I've also been wondering whether this works with core files. Have you tried that? (I can't imagine why it wouldn't work. And I don't think it is really a requirement; I'm just curious.)q Also I was wondering about a multi-threaded JIT. I suppose it is sufficient to mention in the documentation that the JIT is responsible for making sure only a single thread can call __jit_debug_register_code at a time. Tom> Does the current code work ok if LLVM is a .so linked into the Tom> application? =C2=A0I suspect, but do not know for sure, that in this Tom> situation gdb will not see the JIT symbols when the inferior-created Tom> hook is run. Reid> I don't know, since we statically link to LLVM. I'm not an expert in Reid> how dynamic loaders work, so I don't know how to fix this. Ok. One way would be to stick some code in breakpoint_re_set and update_breakpoints_after_exec. (I am not sure if this is the best way or not.) Anyway the idea is that after a .so is noticed, re-do the searching, in case the new .so is a JIT. So, the work would be done per-objfile, and not once per inferior. Maybe this can be done via observers as well; I don't know. Tom> There is also the somewhat more pathological case of a program with Tom> multiple JITs linked in. =C2=A0I'm on the fence about whether this is = really Tom> needed. Reid> How would this work? Would they both have separate functions Reid> __jit_debug_register at different addresses? Yeah. This would work if you had two JITs in a process, say loaded dynamically, and the various __jit symbols had hidden visibility. Tom> You can store arbitrary module-specific data in an objfile using the Tom> objfile_data API. =C2=A0This would be better because it is an already Tom> supported way of dealing with the lifetime of the associated data. Reid> Ah, that's much better. Unfortunately because the data for other Reid> objfiles is uninitialized (it's returned from calloc), there's no way Reid> for me to check which objfiles contain JITed code. I don't think I understand. I thought you meant that you couldn't detect whether or not the JIT datum was set on an objfile, but I see that jit_inferior_exit_hook already does this the way that I would expect. Reid> Do I actually need to have this inferior_exit hook? I just Reid> noticed that when I kill and restart the inferior it doesn't seem Reid> to free these objfiles, so I added the hook. Yeah, I think you need it. I was thinking maybe this should just be a flag on the objfile, plus a change to delete such objfiles in reread_symbols. But I am not sure which is better, really; and your way is less invasive. Reid> I took a shot at putting cleanups in, but I'm not sure I did it Reid> correctly. In particular, I'm not sure about the discard_cleanups Reid> logic. It looks ok to me. Just FYI, there's a section in the internals manual about cleanups that is reasonably clear. Tom> Please update the comment here to explain that these values are export= ed Tom> and used by the inferior, so they cannot be changed. Reid> Done. The same is true about the rest of the structs, the ordering of Reid> the fields cannot be changed without updating the protocol version. My reading of the code is that it unpacks target memory field-by-field into the host-side structure. So, it is ok for this to change. What cannot change is the definition of these structs on the target. Am I misreading this? These structs, in their target form, should probably all go in the manual as well. Reid> + /* Remember a mapping from entry_addr to objfile. */ Reid> + set_objfile_data (objfile, jit_objfile_data, (void*) entry_addr); I don't think you need the cast here. There are a few of these. Reid> + printf_unfiltered ("Unable to find JITed code entry at address= : %p\n", Reid> + (void*) entry_addr); A style nit, in GNU style the cast is written "(void *)". Reid> + nbfd =3D bfd_open_from_memory (templ, buffer, size, filename); Reid> + if (nbfd =3D=3D NULL) Reid> + error (_("Unable to create BFD from local memory: %s"), Reid> + bfd_errmsg (bfd_get_error ())); Reid> + Reid> + /* We set loadbase to 0 and assume that the symbol file we're load= ing has the Reid> + absolute addresses set in the ELF. */ Reid> + loadbase =3D 0; Reid> + objfile =3D symbol_file_add_from_memory_common (nbfd, loadbase, 0); I suspect this needs a cleanup to free the BFD in case symbol_file_add_from_memory_common errors. But I couldn't tell immediately if that is possible (in gdb I tend to assume that anything can throw...). Tom