compile: Use libcc1.so->libcc1.so.0

  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(-)
  

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