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