* libargp: argp_program_version does not work @ 2013-07-11 10:17 Fedin Pavel 2013-07-11 16:35 ` Andrew Schulman 0 siblings, 1 reply; 13+ messages in thread From: Fedin Pavel @ 2013-07-11 10:17 UTC (permalink / raw) To: cygwin 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 ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: libargp: argp_program_version does not work 2013-07-11 10:17 libargp: argp_program_version does not work Fedin Pavel @ 2013-07-11 16:35 ` Andrew Schulman 2013-07-11 22:02 ` Andrew Schulman 0 siblings, 1 reply; 13+ messages in thread From: Andrew Schulman @ 2013-07-11 16:35 UTC (permalink / raw) To: cygwin > 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 ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: libargp: argp_program_version does not work 2013-07-11 16:35 ` Andrew Schulman @ 2013-07-11 22:02 ` Andrew Schulman 2013-07-12 14:52 ` Fedin Pavel 0 siblings, 1 reply; 13+ messages in thread From: Andrew Schulman @ 2013-07-11 22:02 UTC (permalink / raw) To: cygwin > > 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 ^ permalink raw reply [flat|nested] 13+ messages in thread
* RE: libargp: argp_program_version does not work 2013-07-11 22:02 ` Andrew Schulman @ 2013-07-12 14:52 ` Fedin Pavel 2013-07-12 15:36 ` Andrew Schulman 0 siblings, 1 reply; 13+ messages in thread From: Fedin Pavel @ 2013-07-12 14:52 UTC (permalink / raw) To: 'Andrew Schulman'; +Cc: cygwin 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 ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: libargp: argp_program_version does not work 2013-07-12 14:52 ` Fedin Pavel @ 2013-07-12 15:36 ` Andrew Schulman 2013-07-17 9:57 ` [PATCH] Fix optional variables in libargp Fedin Pavel 0 siblings, 1 reply; 13+ messages in thread From: Andrew Schulman @ 2013-07-12 15:36 UTC (permalink / raw) To: cygwin > 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 ^ permalink raw reply [flat|nested] 13+ messages in thread
* [PATCH] Fix optional variables in libargp 2013-07-12 15:36 ` Andrew Schulman @ 2013-07-17 9:57 ` Fedin Pavel 2013-07-17 12:59 ` Eric Blake 2013-07-22 7:33 ` Andrew Schulman 0 siblings, 2 replies; 13+ messages in thread From: Fedin Pavel @ 2013-07-17 9:57 UTC (permalink / raw) To: 'Andrew Schulman', cygwin [-- 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 ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH] Fix optional variables in libargp 2013-07-17 9:57 ` [PATCH] Fix optional variables in libargp Fedin Pavel @ 2013-07-17 12:59 ` Eric Blake 2013-07-22 7:33 ` Andrew Schulman 1 sibling, 0 replies; 13+ messages in thread From: Eric Blake @ 2013-07-17 12:59 UTC (permalink / raw) To: cygwin [-- 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 --] ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH] Fix optional variables in libargp 2013-07-17 9:57 ` [PATCH] Fix optional variables in libargp Fedin Pavel 2013-07-17 12:59 ` Eric Blake @ 2013-07-22 7:33 ` Andrew Schulman 2013-07-22 8:12 ` Pavel Fedin 2013-07-22 13:19 ` Pavel Fedin 1 sibling, 2 replies; 13+ messages in thread From: Andrew Schulman @ 2013-07-22 7:33 UTC (permalink / raw) To: cygwin > 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 ^ permalink raw reply [flat|nested] 13+ messages in thread
* RE: [PATCH] Fix optional variables in libargp 2013-07-22 7:33 ` Andrew Schulman @ 2013-07-22 8:12 ` Pavel Fedin 2013-07-22 13:19 ` Pavel Fedin 1 sibling, 0 replies; 13+ messages in thread From: Pavel Fedin @ 2013-07-22 8:12 UTC (permalink / raw) To: 'Andrew Schulman', cygwin 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 ^ permalink raw reply [flat|nested] 13+ messages in thread
* RE: [PATCH] Fix optional variables in libargp 2013-07-22 7:33 ` Andrew Schulman 2013-07-22 8:12 ` Pavel Fedin @ 2013-07-22 13:19 ` Pavel Fedin 2013-07-23 15:07 ` Andrew Schulman 2022-02-18 13:30 ` Andrew Schulman 1 sibling, 2 replies; 13+ messages in thread From: Pavel Fedin @ 2013-07-22 13:19 UTC (permalink / raw) To: 'Andrew Schulman', cygwin 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 ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH] Fix optional variables in libargp 2013-07-22 13:19 ` Pavel Fedin @ 2013-07-23 15:07 ` Andrew Schulman 2013-07-24 9:56 ` Pavel Fedin 2022-02-18 13:30 ` Andrew Schulman 1 sibling, 1 reply; 13+ messages in thread From: Andrew Schulman @ 2013-07-23 15:07 UTC (permalink / raw) To: cygwin > 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 ^ permalink raw reply [flat|nested] 13+ messages in thread
* RE: [PATCH] Fix optional variables in libargp 2013-07-23 15:07 ` Andrew Schulman @ 2013-07-24 9:56 ` Pavel Fedin 0 siblings, 0 replies; 13+ messages in thread From: Pavel Fedin @ 2013-07-24 9:56 UTC (permalink / raw) To: 'Andrew Schulman', cygwin 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 ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH] Fix optional variables in libargp 2013-07-22 13:19 ` Pavel Fedin 2013-07-23 15:07 ` Andrew Schulman @ 2022-02-18 13:30 ` Andrew Schulman 1 sibling, 0 replies; 13+ messages in thread From: Andrew Schulman @ 2022-02-18 13:30 UTC (permalink / raw) To: cygwin > 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? ^ permalink raw reply [flat|nested] 13+ messages in thread
end of thread, other threads:[~2022-02-18 13:30 UTC | newest] Thread overview: 13+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2013-07-11 10:17 libargp: argp_program_version does not work Fedin Pavel 2013-07-11 16:35 ` Andrew Schulman 2013-07-11 22:02 ` Andrew Schulman 2013-07-12 14:52 ` Fedin Pavel 2013-07-12 15:36 ` Andrew Schulman 2013-07-17 9:57 ` [PATCH] Fix optional variables in libargp Fedin Pavel 2013-07-17 12:59 ` Eric Blake 2013-07-22 7:33 ` Andrew Schulman 2013-07-22 8:12 ` Pavel Fedin 2013-07-22 13:19 ` Pavel Fedin 2013-07-23 15:07 ` Andrew Schulman 2013-07-24 9:56 ` Pavel Fedin 2022-02-18 13:30 ` Andrew Schulman
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).