Re: [PATCH RFC 03/11] printversion: Fix unused variable

[Ilya Leoshkevich <iii@linux.ibm.com>]

On Tue, 2023-02-07 at 21:44 +0100, Mark Wielaard wrote:
> Hi Ilya (CC Frank),
> 
> On Mon, Feb 06, 2023 at 11:25:05PM +0100, Ilya Leoshkevich via
> Elfutils-devel wrote:
> > clang complains:
> > 
> >     debuginfod.cxx:354:1: error: unused variable 'apba__' [-
> > Werror,-Wunused-const-variable]
> >     ARGP_PROGRAM_BUG_ADDRESS_DEF = PACKAGE_BUGREPORT;
> >     ^
> >     ../lib/printversion.h:47:21: note: expanded from macro
> > 'ARGP_PROGRAM_BUG_ADDRESS_DEF'
> >       const char *const apba__ __asm ("argp_program_bug_address")
> >                         ^
> > 
> > This is as expected: it's used by argp via the
> > "argp_program_bug_address" name, which is not visible on the C
> > level.
> > Add __attribute__ ((used)) to make sure that the compiler emits it.
> 
> Actually I think it found a real issue. Note that the same construct
> is used the C eu tools.  But in debuginfod.cxx it says:
> 
> /* Name and version of program.  */
> /* ARGP_PROGRAM_VERSION_HOOK_DEF = print_version; */ // not this
> simple for C++
> 
> /* Bug report address.  */
> ARGP_PROGRAM_BUG_ADDRESS_DEF = PACKAGE_BUGREPORT;
> 
> Note how ARGP_PROGRAM_VERSION_HOOK_DEF is commented out and in main
> it
> has:
> 
>    /* Parse and process arguments.  */
>    int remaining;
>    argp_program_version_hook = print_version; // this works
>    (void) argp_parse (&argp, argc, argv, ARGP_IN_ORDER, &remaining,
> NULL);
> 
> So it sets print_version, but not argp_program_bug_address.
> And indeed debuginfod --help is missing the bug reporting address.
> 
> I don't really know/understand why the printversion.h macro trick
> doesn't work with C++ (symbol mangling?). But we do need at least
> this patch:
> 
> diff --git a/debuginfod/debuginfod.cxx b/debuginfod/debuginfod.cxx
> index 4271acf4..0ec326d5 100644
> --- a/debuginfod/debuginfod.cxx
> +++ b/debuginfod/debuginfod.cxx
> @@ -4172,6 +4165,7 @@ main (int argc, char *argv[])
>    /* Parse and process arguments.  */
>    int remaining;
>    argp_program_version_hook = print_version; // this works
> +  argp_program_bug_address = PACKAGE_BUGREPORT;
>    (void) argp_parse (&argp, argc, argv, ARGP_IN_ORDER, &remaining,
> NULL);
>    if (remaining != argc)
>        error (EXIT_FAILURE, 0,
> 
> Then debuginfod --help will say: Report bugs to
> https://sourceware.org/bugzilla.
> 
> That of course doesn't help with the -Wunused-const-variable warning.
> 
> If we cannot figure out the magic variable naming trick with with C++
> then maybe we can just not include printversion.h and do it "by
> hand"?
> (as attached)
> 
> Cheers,
> 
> Mark

If I build:

const char *const apba__ __asm ("argp_program_bug_address") \
__attribute__ ((used)) = "foobarbaz";

with C and C++, the difference is going to be:

@@ -1,6 +1,5 @@
 	.file	"1.c"
 	.text
-	.globl	argp_program_bug_address
 	.section	.rodata.str1.1,"aMS",@progbits,1
 .LC0:
 	.string	"foobarbaz"

This must have to do with C and C++ standards treating const
differently [1]. The solution is to add extern:

--- a/lib/printversion.h
+++ b/lib/printversion.h
@@ -44,6 +44,7 @@ void print_version (FILE *stream, struct argp_state
*state);
   void (*const apvh) (FILE *, struct argp_state *) \
    __asm ("argp_program_version_hook")
 #define ARGP_PROGRAM_BUG_ADDRESS_DEF \
+  extern const char *const apba__; \
   const char *const apba__ __asm ("argp_program_bug_address") \
   __attribute__ ((used))

I can include this in v2 if it works for you.

[1]
https://stackoverflow.com/questions/8908071/const-correctness-in-c-vs-c