Missing extern "C" in count-one-bits.h?

Missing extern "C" in count-one-bits.h?

[Simon Marchi]

Hi,

I recently imported the count-one-bits module in GDB.  I just noticed this while trying
to build gdb with clang++-9:

  arch/arm-get-next-pcs.o:arm-get-next-pcs.c:function thumb_get_next_pcs_raw(arm_get_next_pcs*): error: undefined reference to 'count_one_bits(unsigned int)'

Everything works fine when building with gcc-9.

This is in the gcc build:

$ readelf --syms gnulib/import/count-one-bits.o | grep count_one_bits
   164: 0000000000000000    24 FUNC    GLOBAL DEFAULT   50 count_one_bits
$ readelf --syms gdb/arch/arm-get-next-pcs.o | grep count_one_bits
   770: 0000000000000000    24 FUNC    WEAK   DEFAULT  281 _Z14count_one_bitsj

This is in the clang build:

$ readelf --syms gnulib/import/count-one-bits.o | grep count_one_bits
     6: 0000000000000000    64 FUNC    GLOBAL DEFAULT    2 count_one_bits
$ readelf --syms gdb/arch/arm-get-next-pcs.o | grep count_one_bits
   109: 0000000000000000     0 NOTYPE  GLOBAL DEFAULT  UND _Z14count_one_bitsj

The disagreement between the symbol names between count-one-bits.o and arm-get-next-pcs.o
seems to show that there is a missing `extern "C"` in the count-one-bits.h header?

And apparently, gcc decided to compile the definition in the header and include
it in arm-get-next-pcs.o, while clang did not.  Is that the compiler's choice,
or is it because the macros and preprocessor conditionals in count-one-bits.h
evaluated to something different for the two compilers?

Simon

Re: Missing extern "C" in count-one-bits.h?

[Paul Eggert]

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

On 2/20/20 12:15 PM, Simon Marchi wrote:
> The disagreement between the symbol names between count-one-bits.o and arm-get-next-pcs.o
> seems to show that there is a missing `extern "C"` in the count-one-bits.h header?

Thanks, I'll take your word for it. (I don't use C++.) I installed the 
attached patch into Gnulib. Apparently these modules have been used by C 
code only, until now.

> And apparently, gcc decided to compile the definition in the header and include
> it in arm-get-next-pcs.o, while clang did not.  Is that the compiler's choice,
> or is it because the macros and preprocessor conditionals in count-one-bits.h
> evaluated to something different for the two compilers?

It's a compiler's choice whether a function is inlined. When I call 
count_one_bits in C, gcc -O2 doesn't create a function for 
count_one_bits in the calling module; it simply issues the popcnt insn, 
or calls the appropriate GCC library function on platforms that don't 
have a popcnt insn. Perhaps you were compiling with some other level of 
optimization, or perhaps C++ does this differently; but regardless of 
how the compiler does it the resulting code is supposed to work of course.

[-- Attachment #2: 0001-Add-extern-C-to-count-one-bits.h-etc.patch --]
[-- Type: text/x-patch, Size: 3667 bytes --]

From 50da3fa2a9fde4ff42cf20d3994c1095060b4ff7 Mon Sep 17 00:00:00 2001
From: Paul Eggert <eggert@cs.ucla.edu>
Date: Fri, 21 Feb 2020 12:41:33 -0800
Subject: [PATCH] =?UTF-8?q?Add=20=E2=80=98extern=20"C"=E2=80=99=20to=20cou?=
 =?UTF-8?q?nt-one-bits.h=20etc.?=
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

This ports these .h files to C++.
Problem reported by Simon Marchi in:
https://lists.gnu.org/r/bug-gnulib/2020-02/msg00110.html
* lib/count-leading-zeros.h, lib/count-one-bits.h:
* lib/count-trailing-zeros.h: Add ‘extern "C"’.
---
 ChangeLog                  | 9 +++++++++
 lib/count-leading-zeros.h  | 8 ++++++++
 lib/count-one-bits.h       | 8 ++++++++
 lib/count-trailing-zeros.h | 8 ++++++++
 4 files changed, 33 insertions(+)

diff --git a/ChangeLog b/ChangeLog
index 8649b9110..89beae3d7 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,3 +1,12 @@
+2020-02-21  Paul Eggert  <eggert@cs.ucla.edu>
+
+	Add ‘extern "C"’ to count-one-bits.h etc.
+	This ports these .h files to C++.
+	Problem reported by Simon Marchi in:
+	https://lists.gnu.org/r/bug-gnulib/2020-02/msg00110.html
+	* lib/count-leading-zeros.h, lib/count-one-bits.h:
+	* lib/count-trailing-zeros.h: Add ‘extern "C"’.
+
 2020-02-19  Bruno Haible  <bruno@clisp.org>
 
 	uninorm/decompose-internal: Avoid "no previous prototype" warning.
diff --git a/lib/count-leading-zeros.h b/lib/count-leading-zeros.h
index b548754e1..7e88c8cb9 100644
--- a/lib/count-leading-zeros.h
+++ b/lib/count-leading-zeros.h
@@ -30,6 +30,10 @@ _GL_INLINE_HEADER_BEGIN
 # define COUNT_LEADING_ZEROS_INLINE _GL_INLINE
 #endif
 
+#ifdef __cplusplus
+extern "C" {
+#endif
+
 /* Assuming the GCC builtin is BUILTIN and the MSC builtin is MSC_BUILTIN,
    expand to code that computes the number of leading zeros of the local
    variable 'x' of type TYPE (an unsigned integer type) and return it
@@ -108,6 +112,10 @@ count_leading_zeros_ll (unsigned long long int x)
                        unsigned long long int);
 }
 
+#ifdef __cplusplus
+}
+#endif
+
 _GL_INLINE_HEADER_END
 
 #endif /* COUNT_LEADING_ZEROS_H */
diff --git a/lib/count-one-bits.h b/lib/count-one-bits.h
index 78770e424..eea56d859 100644
--- a/lib/count-one-bits.h
+++ b/lib/count-one-bits.h
@@ -30,6 +30,10 @@ _GL_INLINE_HEADER_BEGIN
 # define COUNT_ONE_BITS_INLINE _GL_INLINE
 #endif
 
+#ifdef __cplusplus
+extern "C" {
+#endif
+
 /* Expand to code that computes the number of 1-bits of the local
    variable 'x' of type TYPE (an unsigned integer type) and return it
    from the current function.  */
@@ -129,6 +133,10 @@ count_one_bits_ll (unsigned long long int x)
   COUNT_ONE_BITS (__builtin_popcountll, __popcnt64, unsigned long long int);
 }
 
+#ifdef __cplusplus
+}
+#endif
+
 _GL_INLINE_HEADER_END
 
 #endif /* COUNT_ONE_BITS_H */
diff --git a/lib/count-trailing-zeros.h b/lib/count-trailing-zeros.h
index 2169f6262..1eb5fb919 100644
--- a/lib/count-trailing-zeros.h
+++ b/lib/count-trailing-zeros.h
@@ -30,6 +30,10 @@ _GL_INLINE_HEADER_BEGIN
 # define COUNT_TRAILING_ZEROS_INLINE _GL_INLINE
 #endif
 
+#ifdef __cplusplus
+extern "C" {
+#endif
+
 /* Assuming the GCC builtin is BUILTIN and the MSC builtin is MSC_BUILTIN,
    expand to code that computes the number of trailing zeros of the local
    variable 'x' of type TYPE (an unsigned integer type) and return it
@@ -100,6 +104,10 @@ count_trailing_zeros_ll (unsigned long long int x)
                         unsigned long long int);
 }
 
+#ifdef __cplusplus
+}
+#endif
+
 _GL_INLINE_HEADER_END
 
 #endif
-- 
2.24.1

Re: Missing extern "C" in count-one-bits.h?

[Simon Marchi via Gnulib discussion list]

On 2020-02-21 3:59 p.m., Paul Eggert wrote:
> On 2/20/20 12:15 PM, Simon Marchi wrote:
>> The disagreement between the symbol names between count-one-bits.o and arm-get-next-pcs.o
>> seems to show that there is a missing `extern "C"` in the count-one-bits.h header?
> 
> Thanks, I'll take your word for it. (I don't use C++.) I installed the attached patch into Gnulib. Apparently these modules have been used by C code only, until now.

Thanks, that does the job.

I'll try to provide a minimal reproducer next time, I just haven't learned yet
how to use gnulib in a standalone way (outside of what GDB uses).

>> And apparently, gcc decided to compile the definition in the header and include
>> it in arm-get-next-pcs.o, while clang did not.  Is that the compiler's choice,
>> or is it because the macros and preprocessor conditionals in count-one-bits.h
>> evaluated to something different for the two compilers?
> 
> It's a compiler's choice whether a function is inlined. When I call count_one_bits in C, gcc -O2 doesn't create a function for count_one_bits in the calling module; it simply issues the popcnt insn, or calls the appropriate GCC library function on platforms that don't have a popcnt insn. Perhaps you were compiling with some other level of optimization, or perhaps C++ does this differently; but regardless of how the compiler does it the resulting code is supposed to work of course.

I indeed build without optimizations, to be able to debug, it's likely that
clang with optimizations would also reduce that to just popcnt.

Anyway, thanks for the help, I'll go work on updating GDB's gnulib import!

Simon