Message ID | 20150421213616.14023.38329.stgit@host1.jankratochvil.net |
---|---|
State | New, archived |
Headers |
Received: (qmail 124785 invoked by alias); 21 Apr 2015 21:36:23 -0000 Mailing-List: contact gdb-patches-help@sourceware.org; run by ezmlm Precedence: bulk List-Id: <gdb-patches.sourceware.org> List-Unsubscribe: <mailto:gdb-patches-unsubscribe-##L=##H@sourceware.org> List-Subscribe: <mailto:gdb-patches-subscribe@sourceware.org> List-Archive: <http://sourceware.org/ml/gdb-patches/> List-Post: <mailto:gdb-patches@sourceware.org> List-Help: <mailto:gdb-patches-help@sourceware.org>, <http://sourceware.org/ml/#faqs> Sender: gdb-patches-owner@sourceware.org Delivered-To: mailing list gdb-patches@sourceware.org Received: (qmail 124734 invoked by uid 89); 21 Apr 2015 21:36:22 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-3.8 required=5.0 tests=AWL, BAYES_00, SPF_HELO_PASS, SPF_PASS, T_RP_MATCHES_RCVD autolearn=ham version=3.3.2 X-HELO: mx1.redhat.com Received: from mx1.redhat.com (HELO mx1.redhat.com) (209.132.183.28) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with (AES256-GCM-SHA384 encrypted) ESMTPS; Tue, 21 Apr 2015 21:36:22 +0000 Received: from int-mx13.intmail.prod.int.phx2.redhat.com (int-mx13.intmail.prod.int.phx2.redhat.com [10.5.11.26]) by mx1.redhat.com (8.14.4/8.14.4) with ESMTP id t3LLaKSx023588 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-GCM-SHA384 bits=256 verify=FAIL) for <gdb-patches@sourceware.org>; Tue, 21 Apr 2015 17:36:20 -0400 Received: from host1.jankratochvil.net (ovpn-116-27.ams2.redhat.com [10.36.116.27]) by int-mx13.intmail.prod.int.phx2.redhat.com (8.14.4/8.14.4) with ESMTP id t3LLaIGO012671; Tue, 21 Apr 2015 17:36:19 -0400 Subject: [PATCH] compile: Use libcc1.so->libcc1.so.0 From: Jan Kratochvil <jan.kratochvil@redhat.com> To: gdb-patches@sourceware.org Cc: Phil Muldoon <pmuldoon@redhat.com> Date: Tue, 21 Apr 2015 23:36:18 +0200 Message-ID: <20150421213616.14023.38329.stgit@host1.jankratochvil.net> User-Agent: StGit/0.17.1-dirty MIME-Version: 1.0 Content-Type: text/plain; charset="utf-8" Content-Transfer-Encoding: 7bit X-IsSubscribed: yes |
Commit Message
Jan Kratochvil
April 21, 2015, 9:36 p.m. UTC
Hi, the next [patch 3/4] will change the libcc1.so API. I am not sure if it gets approved in GCC land but for such case: (1) We really need to change GCC_FE_VERSION_0 -> GCC_FE_VERSION_1, this feature is there for this purpose. That is [patch 2/4]. (2) Currently GDB does only dlopen("libcc1.so") and then depending on which libcc1.so version it would find first it would succeed/fail. I guess it is more convenient to do dlopen("libcc1.so.1") instead (where ".1"=".x" corresponds to GCC_FE_VERSION_x). That is this patch (with x=0). (3) Currently there is no backward or forward compatibility although there could be one implemented. Personally I think the 'compile' feature is still in experimental stage so that it is OK to require last releases. At least in Fedora we can keep GDB<->GCC in sync. include/* changes are here only FYI, master repository of those files is GCC. Corresponding GCC part of the patches are going to be sent along to GCC. Jan gdb/ChangeLog 2015-04-21 Jan Kratochvil <jan.kratochvil@redhat.com> * compile/compile-c-support.c (load_libcc): Remove STRINGIFY from GCC_C_FE_LIBCC. include/ChangeLog 2015-04-21 Jan Kratochvil <jan.kratochvil@redhat.com> * gcc-c-interface.h (GCC_C_FE_LIBCC): Quote it. Append GCC_FE_VERSION_0. --- gdb/compile/compile-c-support.c | 4 ++-- include/gcc-c-interface.h | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-)
Comments
Please drop this mail, resent in the next mail, stg mistake.
On 21/04/15 22:36, Jan Kratochvil wrote: > Hi, > > the next [patch 3/4] will change the libcc1.so API. I am not sure if it gets > approved in GCC land but for such case: > (1) We really need to change GCC_FE_VERSION_0 -> GCC_FE_VERSION_1, this > feature is there for this purpose. That is [patch 2/4]. > (2) Currently GDB does only dlopen("libcc1.so") and then depending on which > libcc1.so version it would find first it would succeed/fail. > I guess it is more convenient to do dlopen("libcc1.so.1") instead > (where ".1"=".x" corresponds to GCC_FE_VERSION_x). > That is this patch (with x=0). > (3) Currently there is no backward or forward compatibility although there > could be one implemented. Personally I think the 'compile' feature is > still in experimental stage so that it is OK to require last releases. > At least in Fedora we can keep GDB<->GCC in sync. > > Will this mean that the next release of GDB will require GCC 5.0.1 (or whatever the next version of GCC after 5.0)? If so, then, I object. This could mean many months of GDB and GCC releases not coinciding and no functionality being present. If not, how does GDB work with an older GCC? It also does not seem a good thing to change the API before GCC 5.0 is even released. I think as far as possible we want to be fault tolerant with GCC. Another solution is to add another method to the end of the vtable of functions presented to GDB from the GCC plugin, and save API changes for more synchronized and planned changes. Cheers Phil
On Wed, 22 Apr 2015 23:13:40 +0200, Phil Muldoon wrote: > Will this mean that the next release of GDB will require GCC 5.0.1 (or > whatever the next version of GCC after 5.0)? > > If so, then, I object. This could mean many months of GDB and GCC > releases not coinciding and no functionality being present. If not, > how does GDB work with an older GCC? It also does not seem a good > thing to change the API before GCC 5.0 is even released. There is now released gcc-5.1. Upstream GCC has approved it now for trunk, that is gcc-6.0. (For Fedora a backport can be easily done for whatever version combinations.) > I think as far as possible we want to be fault tolerant with > GCC. Another solution is to add another method to the end of the > vtable of functions presented to GDB from the GCC plugin, and save API > changes for more synchronized and planned changes. So you request forward/backward compatibilities, specifically: (1) Do you request future gdb-7.10 is compatible with existing gcc-5.x? (2) Do you request future gcc-6.0 is compatible with existing gdb-7.9? With an answer for (1) and (2) we can decide on how to implement it. Jan
On 23/04/15 06:29, Jan Kratochvil wrote: > > So you request forward/backward compatibilities, specifically: > > (1) Do you request future gdb-7.10 is compatible with existing gcc-5.x? > > (2) Do you request future gcc-6.0 is compatible with existing gdb-7.9? > > With an answer for (1) and (2) we can decide on how to implement it. > > Both! ;) In principle the decision bump is OK; but, and this is the huge caveat, we could fix this quite easily by adding another method to the vtable exported by the plug-in and not need or require all of the tinkering that would be needed downstream. Yes, Fedora could be modified to cope with it, but we have to think about the work all the other distributions would also have to do if this proposed change were implemented. I don't think a version change merits that. And the change is tiny: just one more parameter for a function. You could avoid it by having two public methods exported in the vtable: foo (old params), foo (old params, new params) and then re-factoring out the old function to foo_worker_1 and have the two "foo" functions call foo_worker_1 with the new parameter or NULL in its place. I'm not adverse to version changes but I think they should merit the change. Possibly as a collection of changes. What are your thoughts? Cheers Phil
On 04/23/2015 11:52 AM, Phil Muldoon wrote: > On 23/04/15 06:29, Jan Kratochvil wrote: >> >> So you request forward/backward compatibilities, specifically: >> >> (1) Do you request future gdb-7.10 is compatible with existing gcc-5.x? >> >> (2) Do you request future gcc-6.0 is compatible with existing gdb-7.9? >> >> With an answer for (1) and (2) we can decide on how to implement it. >> >> > > Both! ;) > > In principle the decision bump is OK; but, and this is the huge > caveat, we could fix this quite easily by adding another method to the > vtable exported by the plug-in and not need or require all of the > tinkering that would be needed downstream. Yes, Fedora could be > modified to cope with it, but we have to think about the work all the > other distributions would also have to do if this proposed change were > implemented. Not just distributions, but ourselves too. I was much looking forward to having this feature just work using the system compiler, there's lots of itches that can be scratched on the gdb side alone. But for people to first find the itch, they need to be hooked into playing with the feature first. Permanently keeping the bar high (having to build gcc trunk) puts people off. Also, considering an --enable-targets=all build, I'd rather that gdb was reasonably able to cope with different versions of gcc. E.g., one might have the most recent version for x86 gcc around, but not for ARM, etc. > > I don't think a version change merits that. And the change is tiny: > just one more parameter for a function. You could avoid it by having > two public methods exported in the vtable: foo (old params), foo (old > params, new params) and then re-factoring out the old function to > foo_worker_1 and have the two "foo" functions call foo_worker_1 with > the new parameter or NULL in its place. > > I'm not adverse to version changes but I think they should merit the > change. Possibly as a collection of changes. I agree. Thanks, Pedro Alves
On 04/21/2015 10:36 PM, Jan Kratochvil wrote: > (2) Currently GDB does only dlopen("libcc1.so") and then depending on which > libcc1.so version it would find first it would succeed/fail. > I guess it is more convenient to do dlopen("libcc1.so.1") instead > (where ".1"=".x" corresponds to GCC_FE_VERSION_x). > That is this patch (with x=0). I don't agree with tying the so version with GCC_FE_VERSION. The so version is supposed to indicate binary compatibility. And the ABI is maintained as long as the ABI of the single entry point -- gcc_c_fe_context -- is maintained. We can bump GCC_C_FE_VERSION_x leaving the .so version alone, and on the plugin/gcc side, add new functions at the end of the context vtable, keeping the vtable compatible with GCC_C_FE_VERSION_0: /* The type of the initialization function. The caller passes in the desired base version and desired C-specific version. If the request can be satisfied, a compatible gcc_context object will be ^^^^^^^^^^ returned. Otherwise, the function returns NULL. */ typedef struct gcc_c_context *gcc_c_fe_context_function (enum gcc_base_api_version, enum gcc_c_api_version); With that, an older gdb that requests GCC_FE_VERSION_0 should still work. A newer gdb that understands both v1 and v0 can first try GCC_FE_VERSION_1, and if that fails, try GCC_FE_VERSION_0, etc. It's that code that does the initialization/handshake on both ends (gdb/gcc) that should decide which versions each end supports. Doesn't have to be a single version. Thanks, Pedro Alves
On Thu, 23 Apr 2015 12:52:56 +0200, Phil Muldoon wrote: > On 23/04/15 06:29, Jan Kratochvil wrote: > > > > So you request forward/backward compatibilities, specifically: > > > > (1) Do you request future gdb-7.10 is compatible with existing gcc-5.x? > > > > (2) Do you request future gcc-6.0 is compatible with existing gdb-7.9? > > > > With an answer for (1) and (2) we can decide on how to implement it. > > Both! ;) While (1) could be possibly useful personally I do not find the compatibility (2) useful. > I don't think a version change merits that. And the change is tiny: > just one more parameter for a function. You could avoid it by having > two public methods exported in the vtable: foo (old params), foo (old > params, new params) and then re-factoring out the old function to > foo_worker_1 and have the two "foo" functions call foo_worker_1 with > the new parameter or NULL in its place. I do not see so clear how to implement it. If GDB changes: gdb/compile/compile-c-support.c:110: context = (*func) (GCC_FE_VERSION_0, GCC_C_FE_VERSION_0); -> gdb/compile/compile-c-support.c:110: context = (*func) (GCC_FE_VERSION_1, GCC_C_FE_VERSION_0); then compatibiity (1) is violated. Besides that you said no new API version should be introduced. If GCC returns larger vtable for: gdb/compile/compile-c-support.c:110: context = (*func) (GCC_FE_VERSION_0, GCC_C_FE_VERSION_0); then GDB has no way to figure out there is the new field at the end of vtable. Maybe GCC could return in such case 'gcc_base_vtable vtable' with GCC_FE_VERSION_1 which would be compatible with existing GDBs but I find that a hack, if GDB asked for GCC_FE_VERSION_0 then it should get version==GCC_FE_VERSION_0. Besides that you said no new API version should be introduced. Existing implementations only compare 'version' for equality. IMO for your requested compatibility (1)+(2) it would be most clean to have an additional field 'minor_version' which would be compared for less-or-equal (by GCC as a minimal functionality level requested by GDB and by GDB as a functionality level supported by GCC). This was omitted by the initial interface design. > What are your thoughts? I think that trunk versions of API should not be complicated by backward compatibility when we control trunk versions of both client and server. I can implement the variant I marked as "hack" above which satisfies both compatibilities (1) and (2). Jan
On Thu, 23 Apr 2015 13:42:12 +0200, Pedro Alves wrote:
> Doesn't have to be a single version.
But Phil asks for a single version which I find a mess.
Jan
On Thu, 23 Apr 2015 13:42:12 +0200, Pedro Alves wrote: > desired base version and desired C-specific version. If the > request can be satisfied, a compatible gcc_context object will be > ^^^^^^^^^^ > returned. Otherwise, the function returns NULL. */ OK, that may mean that GCC is allowed to return compatible GCC_FE_VERSION_1 API even when client asked for GCC_FE_VERSION_0. Thanks for the clarification although I do not find the comment so clear. Jan
On 04/23/2015 12:47 PM, Jan Kratochvil wrote: > On Thu, 23 Apr 2015 12:52:56 +0200, Phil Muldoon wrote: >> On 23/04/15 06:29, Jan Kratochvil wrote: >>> >>> So you request forward/backward compatibilities, specifically: >>> >>> (1) Do you request future gdb-7.10 is compatible with existing gcc-5.x? >>> >>> (2) Do you request future gcc-6.0 is compatible with existing gdb-7.9? >>> >>> With an answer for (1) and (2) we can decide on how to implement it. >> >> Both! ;) > > While (1) could be possibly useful personally I do not find > the compatibility (2) useful. > > >> I don't think a version change merits that. And the change is tiny: >> just one more parameter for a function. You could avoid it by having >> two public methods exported in the vtable: foo (old params), foo (old >> params, new params) and then re-factoring out the old function to >> foo_worker_1 and have the two "foo" functions call foo_worker_1 with >> the new parameter or NULL in its place. > > I do not see so clear how to implement it. > > If GDB changes: > gdb/compile/compile-c-support.c:110: context = (*func) (GCC_FE_VERSION_0, GCC_C_FE_VERSION_0); > -> > gdb/compile/compile-c-support.c:110: context = (*func) (GCC_FE_VERSION_1, GCC_C_FE_VERSION_0); > then compatibiity (1) is violated. Besides that you said no new API version > should be introduced. Here's what I suggest. I'm not sure whether it should be GCC_FE_VERSION_0 or GCC_C_FE_VERSION_0 that should be bumped in this case, but, assuming the former, you'd solve this by first making (and documenting) the vtable of v1 compatible with v0. Then have gdb do: context = (*func) (GCC_FE_VERSION_1, GCC_C_FE_VERSION_0); if (context == NULL) context = (*func) (GCC_FE_VERSION_0, GCC_C_FE_VERSION_0); For as long as gdb supports both v1 and v0, places that call the v1-only functions must check that they're talking to a v1 context before calling through the function pointer, of course. On the plugin side do something like: if (version == 0 || version == 1) { // v1 is compatible with v0, and we're supporting v0 for a while. context->version = version; return context; } else { // error. } Thanks, Pedro Alves
On 04/23/2015 12:52 PM, Jan Kratochvil wrote: > On Thu, 23 Apr 2015 13:42:12 +0200, Pedro Alves wrote: >> desired base version and desired C-specific version. If the >> request can be satisfied, a compatible gcc_context object will be >> ^^^^^^^^^^ >> returned. Otherwise, the function returns NULL. */ > > OK, that may mean that GCC is allowed to return compatible GCC_FE_VERSION_1 > API even when client asked for GCC_FE_VERSION_0. Yes, if the versions are compatible, and if the version is recorded in a field in the returned context structure, then will even be simpler to just return the highest version compatible, than my suggestion of having gdb try both v1 and fallback to v0, which can be left for when we decide to make a vN+1 that is not compatible with vN. This is just like duck typing. > Thanks for the clarification although I do not find the comment so clear. Thanks, Pedro Alves
On Thu, 23 Apr 2015 14:06:53 +0200, Pedro Alves wrote: > Yes, if the versions are compatible, and if the version is recorded in > a field in the returned context structure, then will even be simpler to just > return the highest version compatible, than my suggestion of having gdb > try both v1 and fallback to v0, which can be left for when we decide to make > a vN+1 that is not compatible with vN. This is just like duck typing. OK, so I will implement both compatibility (1) and (2) according to this scheme. Thanks, Jan
diff --git a/gdb/compile/compile-c-support.c b/gdb/compile/compile-c-support.c index 1711cda..e3c01f2 100644 --- a/gdb/compile/compile-c-support.c +++ b/gdb/compile/compile-c-support.c @@ -79,14 +79,14 @@ load_libcc (void) /* gdb_dlopen will call error () on an error, so no need to check value. */ - handle = gdb_dlopen (STRINGIFY (GCC_C_FE_LIBCC)); + handle = gdb_dlopen (GCC_C_FE_LIBCC); func = (gcc_c_fe_context_function *) gdb_dlsym (handle, STRINGIFY (GCC_C_FE_CONTEXT)); if (func == NULL) error (_("could not find symbol %s in library %s"), STRINGIFY (GCC_C_FE_CONTEXT), - STRINGIFY (GCC_C_FE_LIBCC)); + GCC_C_FE_LIBCC); return func; } diff --git a/include/gcc-c-interface.h b/include/gcc-c-interface.h index 95d0fc9..7108952 100644 --- a/include/gcc-c-interface.h +++ b/include/gcc-c-interface.h @@ -197,7 +197,7 @@ struct gcc_c_context /* The name of the .so that the compiler builds. We dlopen this later. */ -#define GCC_C_FE_LIBCC libcc1.so +#define GCC_C_FE_LIBCC "libcc1.so." STRINGIFY (GCC_FE_VERSION_0) /* The compiler exports a single initialization function. This macro holds its name as a symbol. */