libargp: argp_program_version does not work

libargp: argp_program_version does not work

[Fedin Pavel]

 Hello!

 I have found a problem: argp_program_version string is ignored by libargp.
 I guess the problem happens because of DLL's nature. DLLs cannot contain
unresolved symbols, so the DLL has own version of argp_program_version which
is always initialized to NULL. There's no way to override it.
 Is it possible to fix this somehow ? I guess it can be done if we move this
variable to libargp.dll.a stub library. In this case, a static definition of
this variable will be picked up from there if there's no definition in the
program.

Kind regards,
Pavel Fedin
Expert Engineer
Samsung Electronics Research center Russia



--
Problem reports:       http://cygwin.com/problems.html
FAQ:                   http://cygwin.com/faq/
Documentation:         http://cygwin.com/docs.html
Unsubscribe info:      http://cygwin.com/ml/#unsubscribe-simple

Re: libargp: argp_program_version does not work

[Andrew Schulman]

>  Hello!
> 
>  I have found a problem: argp_program_version string is ignored by libargp.

Hi Pavel.  Thanks for reporting this.  I confirm your observation.  For
example, when I compile argp's example #2
(http://www.gnu.org/software/libc/manual/html_node/Argp-Example-2.html#Argp-Example-2),
the --version option doesn't work.

>  I guess the problem happens because of DLL's nature. DLLs cannot contain
> unresolved symbols, so the DLL has own version of argp_program_version which
> is always initialized to NULL. There's no way to override it.

Also confirmed.  argp.h declares argp_program_version as

extern const char *argp_program_version;

But gllib/argp-pv.c initializes it as

const char *argp_program_version = (const char *) 0

Whether there's a way to override that value by one declared in the user's
code, I don't know.  Maybe a linker switch?

The same problem also applies to argp_program_bug_address, which is also
extern const char *.

>  Is it possible to fix this somehow ? I guess it can be done if we move this
> variable to libargp.dll.a stub library. In this case, a static definition of
> this variable will be picked up from there if there's no definition in the
> program.

This is getting outside of my knowledge.  A patch would be welcome.

Could someone with more C or Gnulib knowledge than me comment?

Andrew


--
Problem reports:       http://cygwin.com/problems.html
FAQ:                   http://cygwin.com/faq/
Documentation:         http://cygwin.com/docs.html
Unsubscribe info:      http://cygwin.com/ml/#unsubscribe-simple

Re: libargp: argp_program_version does not work

[Andrew Schulman]

> >  Hello!
> > 
> >  I have found a problem: argp_program_version string is ignored by libargp.
> 
> Hi Pavel.  Thanks for reporting this.  I confirm your observation.  For
> example, when I compile argp's example #2
> (http://www.gnu.org/software/libc/manual/html_node/Argp-Example-2.html#Argp-Example-2),
> the --version option doesn't work.
> 
> >  I guess the problem happens because of DLL's nature. DLLs cannot contain
> > unresolved symbols, so the DLL has own version of argp_program_version which
> > is always initialized to NULL. There's no way to override it.
> 
> Also confirmed.  argp.h declares argp_program_version as
> 
> extern const char *argp_program_version;
> 
> But gllib/argp-pv.c initializes it as
> 
> const char *argp_program_version = (const char *) 0
> 
> Whether there's a way to override that value by one declared in the user's
> code, I don't know.  Maybe a linker switch?

OK, I remembered how this works.  To override the initial values in your
program, you just have to reassign them at run time.

Here's how it's done with argp's example #2.  Note the first two lines of
main(), which aren't present in the upstream example.

/*** Begin modified argp example #2 ***/

#include <argp.h>

const char version[]     = "argp-ex2 1.0";
const char bug_address[] = "<bug-gnu-utils@gnu.org>";

/* Program documentation. */
static char doc[] = "Argp example #2 -- a pretty minimal program using
argp";
static struct argp argp = { 0, 0, 0, doc };

int main (int argc, char **argv)
{
  argp_program_version = version;
  argp_program_bug_address = bug_address;

  argp_parse (&argp, argc, argv, 0, 0, 0);
}

/*** End modified argp example #2 ***/

If you compile the above version, you'll see that the --version option and
bug tracker text are available.

I'm sure this problem is familiar to developers and users of shared
libraries, but I had forgotten it.  I'll add a note to the Cygwin README
file, to help all of us remember it next time.

Andrew


--
Problem reports:       http://cygwin.com/problems.html
FAQ:                   http://cygwin.com/faq/
Documentation:         http://cygwin.com/docs.html
Unsubscribe info:      http://cygwin.com/ml/#unsubscribe-simple

RE: libargp: argp_program_version does not work

[Fedin Pavel]

 Hello!

> int main (int argc, char **argv)
> {
>   argp_program_version = version;
>   argp_program_bug_address = bug_address;
> 
>   argp_parse (&argp, argc, argv, 0, 0, 0); }
> 
> /*** End modified argp example #2 ***/
> 
> If you compile the above version, you'll see that the --version option
> and bug tracker text are available.
> 
> I'm sure this problem is familiar to developers and users of shared
> libraries, but I had forgotten it.  I'll add a note to the Cygwin
> README file, to help all of us remember it next time.

 Yes, i know this. But looks like nobody actually follows your way, because
under Linux simple redefinition perfectly works.
 Of course we could fix every program, but i have an idea how to make the
original Linux code working:
1. Inside DLL we should rename this variables somehow
2. Inside libargp.a.dll we should have a constructor function (with
__attribute__((constructor))) which assigns variables inside DLL with
contents of argp_program_version and argp_program_bug_address.
3. Also inside libargp.a.dll we should have default definitions of these
variables with NULL contents.

 This way code modification would not be required any more.
 I'll try to implement this when have more time.

Kind regards,
Pavel Fedin
Expert Engineer
Samsung Electronics Research center Russia



--
Problem reports:       http://cygwin.com/problems.html
FAQ:                   http://cygwin.com/faq/
Documentation:         http://cygwin.com/docs.html
Unsubscribe info:      http://cygwin.com/ml/#unsubscribe-simple

Re: libargp: argp_program_version does not work

[Andrew Schulman]

>  Hello!
> 
> > int main (int argc, char **argv)
> > {
> >   argp_program_version = version;
> >   argp_program_bug_address = bug_address;
> > 
> >   argp_parse (&argp, argc, argv, 0, 0, 0); }
> > 
> > /*** End modified argp example #2 ***/
> > 
> > If you compile the above version, you'll see that the --version option
> > and bug tracker text are available.
> > 
> > I'm sure this problem is familiar to developers and users of shared
> > libraries, but I had forgotten it.  I'll add a note to the Cygwin
> > README file, to help all of us remember it next time.
> 
>  Yes, i know this. But looks like nobody actually follows your way, because
> under Linux simple redefinition perfectly works.
>  Of course we could fix every program, but i have an idea how to make the
> original Linux code working:
> 1. Inside DLL we should rename this variables somehow
> 2. Inside libargp.a.dll we should have a constructor function (with
> __attribute__((constructor))) which assigns variables inside DLL with
> contents of argp_program_version and argp_program_bug_address.
> 3. Also inside libargp.a.dll we should have default definitions of these
> variables with NULL contents.
> 
>  This way code modification would not be required any more.
>  I'll try to implement this when have more time.

Okay, well I agree that this sounds like a good solution.  For now you have
a workaround, and I'll be glad to consider a patch if you submit one.


--
Problem reports:       http://cygwin.com/problems.html
FAQ:                   http://cygwin.com/faq/
Documentation:         http://cygwin.com/docs.html
Unsubscribe info:      http://cygwin.com/ml/#unsubscribe-simple

[PATCH] Fix optional variables in libargp

[Fedin Pavel]

[-- Attachment #1: Type: text/plain, Size: 1744 bytes --]

 Hello!

> Okay, well I agree that this sounds like a good solution.  For now you
> have a workaround, and I'll be glad to consider a patch if you submit
> one.

 Please take it. This is my experimental implementation which appears to be
simpler than i suggested.

 Some details: this implementation adds explicit dllexport attribute to
these 4 variables. This allows to look them up using GetProcAddress(). The
dllexport attribute has to be added explicitly because when compiling an
.exe file (without -shared switch) gcc does not mark externals as exportable
by default. When building a .dll, all external symbols are marked as
exportable by default, *UNLESS* we have at least one explicit dllexport
specification, hence i surrounded this with #ifndef DLL_EXPORT, this
definition is introduced by libtool when building a shared library.
 This implementation would not work in the following corner cases:
 a) .exe file uses some foo.dll library, which defines these variables and
uses libargp - symbols are picked up only from .exe file itself.
 b) Building that foo.dll manually without DLL_EXPORT definition can get
broken - the compiler will see dllexport attribute, and it will override the
default behavior to make all externs exportable.
 I believe these cases are quite unusual so it's OK. Getting rid of these
limitations would need more complex implementation with more changes.

 I have tested the implementation on x86-64 with RedHat's Prelink utility,
and it works quite fine. i386 should work too, but please retest, just in
case.

 Additionally, i added the second #ifdef _WIN32 in order to make the same
thing working also for MinGW target.

Kind regards,
Pavel Fedin
Expert Engineer
Samsung Electronics Research center Russia

[-- Attachment #2: libargp-20110921-fix-optional-variables.diff --]
[-- Type: application/octet-stream, Size: 3788 bytes --]

diff -ru src/libargp-20110921.orig/gllib/argp.h src/libargp-20110921/gllib/argp.h
--- src/libargp-20110921.orig/gllib/argp.h	2011-09-23 03:31:26.000000000 +0500
+++ src/libargp-20110921/gllib/argp.h	2013-07-15 12:03:51.616647900 +0500
@@ -64,6 +64,18 @@
 typedef int error_t;
 # define __error_t_defined
 #endif
+
+#ifndef DLL_EXPORT
+#ifdef __CYGWIN__
+#define __dllexport __declspec(dllexport)
+#endif
+#ifdef _WIN32
+#define __dllexport __declspec(dllexport)
+#endif
+#endif
+#ifndef __dllexport
+#define __dllexport
+#endif
 \f
 #ifdef  __cplusplus
 extern "C" {
@@ -441,14 +453,14 @@
    option --version is added (unless the ARGP_NO_HELP flag is used), which
    will print this string followed by a newline and exit (unless the
    ARGP_NO_EXIT flag is used).  Overridden by ARGP_PROGRAM_VERSION_HOOK.  */
-extern const char *argp_program_version;
+__dllexport extern const char *argp_program_version;
 
 /* If defined or set by the user program to a non-zero value, then a default
    option --version is added (unless the ARGP_NO_HELP flag is used), which
    calls this function with a stream to print the version to and a pointer to
    the current parsing state, and then exits (unless the ARGP_NO_EXIT flag is
    used).  This variable takes precedent over ARGP_PROGRAM_VERSION.  */
-extern void (*argp_program_version_hook) (FILE *__restrict __stream,
+__dllexport extern void (*argp_program_version_hook) (FILE *__restrict __stream,
                                           struct argp_state *__restrict
                                           __state);
 
@@ -457,12 +469,12 @@
    argp_help if the ARGP_HELP_BUG_ADDR flag is set (as it is by various
    standard help messages), embedded in a sentence that says something like
    `Report bugs to ADDR.'.  */
-extern const char *argp_program_bug_address;
+__dllexport extern const char *argp_program_bug_address;
 
 /* The exit status that argp will use when exiting due to a parsing error.
    If not defined or set by the user program, this defaults to EX_USAGE from
    <sysexits.h>.  */
-extern error_t argp_err_exit_status;
+__dllexport extern error_t argp_err_exit_status;
 \f
 /* Flags for argp_help.  */
 #define ARGP_HELP_USAGE         0x01 /* a Usage: message. */
diff -ru src/libargp-20110921.orig/gllib/argp-parse.c src/libargp-20110921/gllib/argp-parse.c
--- src/libargp-20110921.orig/gllib/argp-parse.c	2011-09-23 03:31:26.000000000 +0500
+++ src/libargp-20110921/gllib/argp-parse.c	2013-07-15 11:39:49.690236100 +0500
@@ -64,6 +64,42 @@
 /* EZ alias for ARGP_ERR_UNKNOWN.  */
 #define EBADKEY ARGP_ERR_UNKNOWN
 \f
+/* Windows does not support things like weak symbols, neither it will merge two
+   external symbols. In order to make our optional variables working, we have
+   to do this trick. What we do here is looking up respective variables in
+   main .exe file and patching our versions with correct pointers
+   Note that the whole thing relies on these variables being declared with
+   __declspec(dllexport). */
+#ifdef DLL_EXPORT
+
+#include <windows.h>
+
+#ifdef __i386__
+#define PREFIX "_"
+#else
+#define PREFIX
+#endif
+
+#define PatchPtr(var)					\
+  p = (void **)GetProcAddress(NULL, PREFIX #var );	\
+  if (p) var = *p;
+
+#define PatchInt(var)					\
+  i = (int *)GetProcAddress(NULL, PREFIX #var );	\
+  if (i) var = *i;
+
+static void __attribute__((constructor)) patch_vars(void)
+{
+  void **p;
+  int *i;
+
+  PatchPtr(argp_program_version);
+  PatchPtr(argp_program_version_hook);
+  PatchPtr(argp_program_bug_address);
+  PatchInt(argp_err_exit_status);
+}
+#endif
+
 /* Default options.  */
 
 /* When argp is given the --HANG switch, _ARGP_HANG is set and argp will sleep


[-- Attachment #3: Type: text/plain, Size: 218 bytes --]

--
Problem reports:       http://cygwin.com/problems.html
FAQ:                   http://cygwin.com/faq/
Documentation:         http://cygwin.com/docs.html
Unsubscribe info:      http://cygwin.com/ml/#unsubscribe-simple

Re: [PATCH] Fix optional variables in libargp

[Eric Blake]

[-- Attachment #1: Type: text/plain, Size: 582 bytes --]

On 07/17/2013 12:32 AM, Fedin Pavel wrote:
>  Hello!
> 
>> Okay, well I agree that this sounds like a good solution.  For now you
>> have a workaround, and I'll be glad to consider a patch if you submit
>> one.
> 
>  Please take it. This is my experimental implementation which appears to be
> simpler than i suggested.

You should probably submit this patch upstream to bug-gnulib@gnu.org, so
that other clients of argp will also be able to benefit from it.

-- 
Eric Blake   eblake redhat com    +1-919-301-3266
Libvirt virtualization library http://libvirt.org


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 621 bytes --]

Re: [PATCH] Fix optional variables in libargp

[Andrew Schulman]

>  I have tested the implementation on x86-64 with RedHat's Prelink utility,
> and it works quite fine. i386 should work too, but please retest, just in
> case.

Fedin, thanks for sending this.  I've tested it for x86_64, and it works
there.  However in x86, in my testing I can't get it to work.  I tried
removing the PREFIX function, and after that it did work once, but I've
tried it again several times and now I can't reproduce that.

So at this point your patch doesn't work in x86 for me.  I'd appreciate it
if someone else could test it, or suggest a modification to make it work.

Andrew


--
Problem reports:       http://cygwin.com/problems.html
FAQ:                   http://cygwin.com/faq/
Documentation:         http://cygwin.com/docs.html
Unsubscribe info:      http://cygwin.com/ml/#unsubscribe-simple

RE: [PATCH] Fix optional variables in libargp

[Pavel Fedin]

 Hello!

> So at this point your patch doesn't work in x86 for me.  I'd appreciate
> it if someone else could test it, or suggest a modification to make it
> work.

 Ok, i'll test it myself.

Kind regards,
Pavel Fedin
Expert Engineer
Samsung Electronics Research center Russia



--
Problem reports:       http://cygwin.com/problems.html
FAQ:                   http://cygwin.com/faq/
Documentation:         http://cygwin.com/docs.html
Unsubscribe info:      http://cygwin.com/ml/#unsubscribe-simple

RE: [PATCH] Fix optional variables in libargp

[Pavel Fedin]

 Hello!
 I have successfully tested it on i386. Really, just remove PREFIX
completely and it's okay. GetProcAddress() appears to be "clever" and adds
the leading underscope by itself on i386. I don't know what you did wrong
and why you could not reproduce the solution.
 However, i have one idea. After patching libargp you need to recompile your
test case. Because it needs to pick up __declspec(dllexport).

> Fedin, thanks for sending this.  I've tested it for x86_64, and it
> works there.  However in x86, in my testing I can't get it to work.  I
> tried removing the PREFIX function, and after that it did work once,
> but I've tried it again several times and now I can't reproduce that.

Kind regards,
Pavel Fedin
Expert Engineer
Samsung Electronics Research center Russia



--
Problem reports:       http://cygwin.com/problems.html
FAQ:                   http://cygwin.com/faq/
Documentation:         http://cygwin.com/docs.html
Unsubscribe info:      http://cygwin.com/ml/#unsubscribe-simple

Re: [PATCH] Fix optional variables in libargp

[Andrew Schulman]

>  Hello!
>  I have successfully tested it on i386. Really, just remove PREFIX
> completely and it's okay. GetProcAddress() appears to be "clever" and adds
> the leading underscope by itself on i386. I don't know what you did wrong
> and why you could not reproduce the solution.
>  However, i have one idea. After patching libargp you need to recompile your
> test case. Because it needs to pick up __declspec(dllexport).

Sorry, this was my fault.  I accidentally mangled your patch when I took
PREFIX out.  I fixed that, and it works fine in x86 now.

So, I'll put a new release out shortly that includes your patch.  Nice
work, and thank you.

Andrew


--
Problem reports:       http://cygwin.com/problems.html
FAQ:                   http://cygwin.com/faq/
Documentation:         http://cygwin.com/docs.html
Unsubscribe info:      http://cygwin.com/ml/#unsubscribe-simple

RE: [PATCH] Fix optional variables in libargp

[Pavel Fedin]

 Hello!

> Sorry, this was my fault.  I accidentally mangled your patch when I
> took PREFIX out.  I fixed that, and it works fine in x86 now.

 Good.
 Please don't forget that packages which make use of libargp (if there are
any) have to be recompiled against new argp.h in order to take advantage of
the fix.

Kind regards,
Pavel Fedin
Expert Engineer
Samsung Electronics Research center Russia



--
Problem reports:       http://cygwin.com/problems.html
FAQ:                   http://cygwin.com/faq/
Documentation:         http://cygwin.com/docs.html
Unsubscribe info:      http://cygwin.com/ml/#unsubscribe-simple

Re: [PATCH] Fix optional variables in libargp

[Andrew Schulman]

>  Hello!
>  I have successfully tested it on i386. Really, just remove PREFIX
> completely and it's okay. GetProcAddress() appears to be "clever" and adds
> the leading underscope by itself on i386. I don't know what you did wrong
> and why you could not reproduce the solution.
>  However, i have one idea. After patching libargp you need to recompile your
> test case. Because it needs to pick up __declspec(dllexport).

Yes, I recompiled my example after rebuilding and reinstalling libargp.  It
still doesn't work.  I even rebooted too, to make sure I was using the
newly installed cygargp-0.dll and not a cached version.

I don't know why this should work for you, and not for me.  I'm attaching
here the revised patch of yours with PREFIX removed, and the argp-ex2.c
source file that I'm building.  Using this patch, I rebuild and reinstall
libargp.   Then I get

$ gcc -o argp-ex2 argp-ex2.c -largp
$ ./argp-ex2.exe --help
Usage: argp-ex2 [OPTION...]
Argp example #2 -- a pretty minimal program using argp

  -?, --help                 give this help list
      --usage                give a short usage message

Note there's no --version option or bug reporting email address.  Do you
get a different result?