Hi,
jankratochvil/compatversion
in [patch 2/2] COMPAT_VERSION needs to be used with function of a different prototype causing:
dwfl_report_elf.c:300:1: error: conflicting types for ‘_compat_without_add_p_vaddr_dwfl_report_elf’ dwfl_report_elf.c:297:1: note: previous declaration of ‘_compat_without_add_p_vaddr_dwfl_report_elf’ was here
Thanks, Jan
commit 4cc52dbb6e9ab3773479a833282494ef47552d59 Author: Jan Kratochvil jan.kratochvil@redhat.com Date: Sun Apr 28 20:10:33 2013 +0200
Rename COMPAT_VERSION to COMPAT_VERSION_SAMEPROTO.
lib/ * eu-config.h (COMPAT_VERSION): Move its __typeof part ... (COMPAT_VERSION_SAMEPROTO): ... into this new wrapper. Twice.
libdwfl/ * dwfl_module_build_id.c: Use COMPAT_VERSION_SAMEPROTO instead of COMPAT_VERSION.
Signed-off-by: Jan Kratochvil jan.kratochvil@redhat.com
diff --git a/lib/ChangeLog b/lib/ChangeLog index 1fa4249..9af0655 100644 --- a/lib/ChangeLog +++ b/lib/ChangeLog @@ -1,3 +1,8 @@ +2013-04-28 Jan Kratochvil jan.kratochvil@redhat.com + + * eu-config.h (COMPAT_VERSION): Move its __typeof part ... + (COMPAT_VERSION_SAMEPROTO): ... into this new wrapper. Twice. + 2013-04-26 Jan Kratochvil jan.kratochvil@redhat.com
* system.h (LE64, BE64): Move here the definitions from diff --git a/lib/eu-config.h b/lib/eu-config.h index 68efb82..94eede8 100644 --- a/lib/eu-config.h +++ b/lib/eu-config.h @@ -171,12 +171,16 @@ asm (".section predict_data, "aw"; .previous\n" # define NEW_VERSION(name, version) \ asm (".symver " #name "," #name "@@@" #version); # define COMPAT_VERSION(name, version, prefix) \ - asm (".symver _compat." #version "." #name "," #name "@" #version); \ + asm (".symver _compat." #version "." #name "," #name "@" #version); +# define COMPAT_VERSION_SAMEPROTO(name, version, prefix) \ + COMPAT_VERSION (name, version, prefix); \ __typeof (name) _compat_##prefix##_##name asm ("_compat." #version "." #name); #else # define OLD_VERSION(name, version) /* Nothing for static linking. */ # define NEW_VERSION(name, version) /* Nothing for static linking. */ # define COMPAT_VERSION(name, version, prefix) error "should use #ifdef SHARED" +# define COMPAT_VERSION_SAMEPROTO(name, version, prefix) \ + error "should use #ifdef SHARED" #endif
diff --git a/libdwfl/ChangeLog b/libdwfl/ChangeLog index 42f720c..c1a42d8 100644 --- a/libdwfl/ChangeLog +++ b/libdwfl/ChangeLog @@ -1,3 +1,8 @@ +2013-04-28 Jan Kratochvil jan.kratochvil@redhat.com + + * dwfl_module_build_id.c: Use COMPAT_VERSION_SAMEPROTO instead of + COMPAT_VERSION. + 2013-04-27 Mark Wielaard mjw@redhat.com
* link_map.c: #include system.h. diff --git a/libdwfl/dwfl_module_build_id.c b/libdwfl/dwfl_module_build_id.c index 660c733..84c0674 100644 --- a/libdwfl/dwfl_module_build_id.c +++ b/libdwfl/dwfl_module_build_id.c @@ -159,7 +159,7 @@ INTDEF (dwfl_module_build_id) NEW_VERSION (dwfl_module_build_id, ELFUTILS_0.138)
#ifdef SHARED -COMPAT_VERSION (dwfl_module_build_id, ELFUTILS_0.130, vaddr_at_end) +COMPAT_VERSION_SAMEPROTO (dwfl_module_build_id, ELFUTILS_0.130, vaddr_at_end)
int _compat_vaddr_at_end_dwfl_module_build_id (Dwfl_Module *mod,
On Sun, Apr 28, 2013 at 08:44:09PM +0200, Jan Kratochvil wrote:
index 1fa4249..9af0655 100644 --- a/lib/ChangeLog +++ b/lib/ChangeLog @@ -1,3 +1,8 @@ +2013-04-28 Jan Kratochvil jan.kratochvil@redhat.com
- eu-config.h (COMPAT_VERSION): Move its __typeof part ...
- (COMPAT_VERSION_SAMEPROTO): ... into this new wrapper. Twice.
2013-04-26 Jan Kratochvil jan.kratochvil@redhat.com
- system.h (LE64, BE64): Move here the definitions from
diff --git a/lib/eu-config.h b/lib/eu-config.h index 68efb82..94eede8 100644 --- a/lib/eu-config.h +++ b/lib/eu-config.h @@ -171,12 +171,16 @@ asm (".section predict_data, "aw"; .previous\n" # define NEW_VERSION(name, version) \ asm (".symver " #name "," #name "@@@" #version); # define COMPAT_VERSION(name, version, prefix) \
- asm (".symver _compat." #version "." #name "," #name "@" #version); \
- asm (".symver _compat." #version "." #name "," #name "@" #version);
+# define COMPAT_VERSION_SAMEPROTO(name, version, prefix) \
- COMPAT_VERSION (name, version, prefix); \ __typeof (name) _compat_##prefix##_##name asm ("_compat." #version "." #name);
These macros are a bit hairy and I don't fully understand them. Hopefully Roland can comment on the change too.
If I read this right then for the new COMPAT_VERSION macro we don't define a _compat_prefix_name symbol anymore. Why isn't that necessary in that case? (I don't fully understand what that asm statement is trying to accomplish to be honest.)
I think it is slightly cleaner to keep the old name do the same thing. So have a COMPAT_VERSION macro that deals with the prototype of the function being the same. And a new macro COMPAT_VERSION_NEWPROTO that deals with compat functions that have different prototypes. Then you don't have to change any existing code.
Thanks,
Mark
Hi Mark,
jankratochvil/compatversion
On Mon, 29 Apr 2013 23:57:12 +0200, Mark Wielaard wrote:
# define COMPAT_VERSION(name, version, prefix) \
- asm (".symver _compat." #version "." #name "," #name "@" #version); \
- asm (".symver _compat." #version "." #name "," #name "@" #version);
+# define COMPAT_VERSION_SAMEPROTO(name, version, prefix) \
- COMPAT_VERSION (name, version, prefix); \ __typeof (name) _compat_##prefix##_##name asm ("_compat." #version "." #name);
[...]
If I read this right then for the new COMPAT_VERSION macro we don't define a _compat_prefix_name symbol anymore.
A bug, thanks. I have checked now the new patch produces correct output:
Num: Value Size Type Bind Vis Ndx Name 26: 000000000000083f 242 FUNC GLOBAL DEFAULT 1 dwfl_report_elf@@ELFUTILS_0.156 30: 000000000000083f 242 FUNC GLOBAL HIDDEN 1 __dwfl_report_elf_internal 31: 0000000000000931 65 FUNC GLOBAL DEFAULT 1 _compat.ELFUTILS_0.122.dwfl_report_elf 32: 0000000000000931 65 FUNC GLOBAL DEFAULT 1 dwfl_report_elf@ELFUTILS_0.122
I think it is slightly cleaner to keep the old name do the same thing. So have a COMPAT_VERSION macro that deals with the prototype of the function being the same. And a new macro COMPAT_VERSION_NEWPROTO that deals with compat functions that have different prototypes. Then you don't have to change any existing code.
OK, changed.
Thanks, Jan
commit fba1588ee438e47a24e92fbd378756501933d00d Author: Jan Kratochvil jan.kratochvil@redhat.com Date: Sun Apr 28 20:10:33 2013 +0200
New macro COMPAT_VERSION_NEWPROTO.
lib/ * eu-config.h (COMPAT_VERSION_NEWPROTO): New. Twice.
Signed-off-by: Jan Kratochvil jan.kratochvil@redhat.com
diff --git a/lib/ChangeLog b/lib/ChangeLog index 1fa4249..6ce3168 100644 --- a/lib/ChangeLog +++ b/lib/ChangeLog @@ -1,3 +1,7 @@ +2013-04-30 Jan Kratochvil jan.kratochvil@redhat.com + + * eu-config.h (COMPAT_VERSION_NEWPROTO): New. Twice. + 2013-04-26 Jan Kratochvil jan.kratochvil@redhat.com
* system.h (LE64, BE64): Move here the definitions from diff --git a/lib/eu-config.h b/lib/eu-config.h index 68efb82..3afff26 100644 --- a/lib/eu-config.h +++ b/lib/eu-config.h @@ -170,12 +170,18 @@ asm (".section predict_data, "aw"; .previous\n" ".symver _compat." #version "." #name "," #name "@" #version); # define NEW_VERSION(name, version) \ asm (".symver " #name "," #name "@@@" #version); +# define COMPAT_VERSION_NEWPROTO(name, version, prefix) \ + asm (".symver _compat." #version "." #name "," #name "@" #version); \ + __typeof (_compat_##prefix##_##name) _compat_##prefix##_##name \ + asm ("_compat." #version "." #name); # define COMPAT_VERSION(name, version, prefix) \ asm (".symver _compat." #version "." #name "," #name "@" #version); \ __typeof (name) _compat_##prefix##_##name asm ("_compat." #version "." #name); #else # define OLD_VERSION(name, version) /* Nothing for static linking. */ # define NEW_VERSION(name, version) /* Nothing for static linking. */ +# define COMPAT_VERSION_NEWPROTO(name, version, prefix) \ + error "should use #ifdef SHARED" # define COMPAT_VERSION(name, version, prefix) error "should use #ifdef SHARED" #endif
On Tue, Apr 30, 2013 at 02:37:46PM +0200, Jan Kratochvil wrote:
jankratochvil/compatversion
[...]
If I read this right then for the new COMPAT_VERSION macro we don't define a _compat_prefix_name symbol anymore.
A bug, thanks.
Aha, OK, now it makes more sense. One last question:
+# define COMPAT_VERSION_NEWPROTO(name, version, prefix) \
- asm (".symver _compat." #version "." #name "," #name "@" #version); \
- __typeof (_compat_##prefix##_##name) _compat_##prefix##_##name \
- asm ("_compat." #version "." #name);
# define COMPAT_VERSION(name, version, prefix) \ asm (".symver _compat." #version "." #name "," #name "@" #version); \ __typeof (name) _compat_##prefix##_##name asm ("_compat." #version "." #name);
Wouldn't the new definition just work always? Whether the prototype is different or the same? That would simplify things even more by just using the new definition as COMPAT_VERSION. But I might be missing something (I still don't fully understand that last asm bit).
Thanks,
Mark
On Wed, 01 May 2013 10:34:57 +0200, Mark Wielaard wrote:
On Tue, Apr 30, 2013 at 02:37:46PM +0200, Jan Kratochvil wrote:
+# define COMPAT_VERSION_NEWPROTO(name, version, prefix) \
- asm (".symver _compat." #version "." #name "," #name "@" #version); \
- __typeof (_compat_##prefix##_##name) _compat_##prefix##_##name \
- asm ("_compat." #version "." #name);
# define COMPAT_VERSION(name, version, prefix) \ asm (".symver _compat." #version "." #name "," #name "@" #version); \ __typeof (name) _compat_##prefix##_##name asm ("_compat." #version "." #name);
Wouldn't the new definition just work always?
Yes but it will no longer sanity check the two implementations have the same prototype. Which may be useful in many (most?) use cases of it.
When thinking about it I am not sure if it was intentional or if it was just a side-effect.
(I still don't fully understand that last asm bit).
info '(gcc)Asm Labels'
IIUC there was an intention that someone cannot call the compatibility-only-variant by accident (as one cannot call a function from C containing '.' dot). Sure even the caller could use the asm label to call this '_compat.ELFUTILS_0.122.dwfl_report_elf' function but if the caller knows how to use the asm label s/he should know it is not right to call this one.
But maybe I am wrong and there were different reasons for it.
Thanks, Jan
On Wed, 2013-05-01 at 11:25 +0200, Jan Kratochvil wrote:
Wouldn't the new definition just work always?
Yes but it will no longer sanity check the two implementations have the same prototype. Which may be useful in many (most?) use cases of it.
Makes sense.
(I still don't fully understand that last asm bit).
info '(gcc)Asm Labels'
IIUC there was an intention that someone cannot call the compatibility-only-variant by accident (as one cannot call a function from C containing '.' dot). Sure even the caller could use the asm label to call this '_compat.ELFUTILS_0.122.dwfl_report_elf' function but if the caller knows how to use the asm label s/he should know it is not right to call this one.
Aha, clever.
Looks good to me. I think you should check it in unless Roland has some comments on it.
Thanks,
Mark
On Wed, 01 May 2013 11:52:42 +0200, Mark Wielaard wrote:
Looks good to me. I think you should check it in unless Roland has some comments on it.
Checked in: fba1588ee438e47a24e92fbd378756501933d00d
Thanks, Jan
elfutils-devel@lists.fedorahosted.org
-
Jan Kratochvil -
Mark Wielaard