LoongArch: Fix ld "undefined reference" error with --enable-shared

Message ID 20230613080712.1651120-1-mengqinggang@loongson.cn
State Unresolved
Headers
Series LoongArch: Fix ld "undefined reference" error with --enable-shared |

Checks

Context Check Description
snail/binutils-gdb-check warning Git am fail log

Commit Message

mengqinggang June 13, 2023, 8:07 a.m. UTC
  Because _bfd_read_unsigned_leb128 is hidden visibility, so it can't
  be referenced out of shared object.

  The new function loongarch_get_uleb128_length just used to call
  _bfd_read_unsigned_leb128.

bfd/ChangeLog:

	* elfxx-loongarch.c (loongarch_get_uleb128_length): New function.
	* elfxx-loongarch.h (loongarch_get_uleb128_length): New function.

gas/ChangeLog:

	* config/tc-loongarch.c (md_apply_fix): Use
	loongarch_get_uleb128_length.
---
 bfd/elfxx-loongarch.c     | 7 +++++++
 bfd/elfxx-loongarch.h     | 2 ++
 gas/config/tc-loongarch.c | 2 +-
 3 files changed, 10 insertions(+), 1 deletion(-)
  

Comments

WANG Xuerui June 13, 2023, 8:58 a.m. UTC | #1
On 2023/6/13 16:07, mengqinggang wrote:
>    Because _bfd_read_unsigned_leb128 is hidden visibility, so it can't
>    be referenced out of shared object.

"Because ... is hidden, it can't be referenced from outside the shared 
object". (Protip: Usually one doesn't use "because" and "so" together, 
at least that's what I have seen / was taught.)

> 
>    The new function loongarch_get_uleb128_length just used to call
>    _bfd_read_unsigned_leb128.

"The new function ... is just a wrapper around ..." sounds more natural.

> 
> bfd/ChangeLog:
> 
> 	* elfxx-loongarch.c (loongarch_get_uleb128_length): New function.
> 	* elfxx-loongarch.h (loongarch_get_uleb128_length): New function.
> 
> gas/ChangeLog:
> 
> 	* config/tc-loongarch.c (md_apply_fix): Use
> 	loongarch_get_uleb128_length.
> ---
>   bfd/elfxx-loongarch.c     | 7 +++++++
>   bfd/elfxx-loongarch.h     | 2 ++
>   gas/config/tc-loongarch.c | 2 +-
>   3 files changed, 10 insertions(+), 1 deletion(-)
> 
> diff --git a/bfd/elfxx-loongarch.c b/bfd/elfxx-loongarch.c
> index ac644fff2f6..cb312c46bb2 100644
> --- a/bfd/elfxx-loongarch.c
> +++ b/bfd/elfxx-loongarch.c
> @@ -1944,3 +1944,10 @@ loongarch_write_unsigned_leb128 (bfd_byte *p, unsigned int len, bfd_vma value)
>     while (len);
>     return p;
>   }
> +
> +int loongarch_get_uleb128_length (bfd_byte *buf)
> +{
> +  unsigned int len = 0;
> +  _bfd_read_unsigned_leb128 (NULL, buf, &len);
> +  return len;

This is not only about getting the length, but also the content, so IMO 
the naming is misleading. IMO You may just call it 
"loongarch_read_unsigned_leb128" to make it very clear that the function 
is closely related to the similarly named BFD function.

> +}
> diff --git a/bfd/elfxx-loongarch.h b/bfd/elfxx-loongarch.h
> index cb02af4fe73..627c5db7bcc 100644
> --- a/bfd/elfxx-loongarch.h
> +++ b/bfd/elfxx-loongarch.h
> @@ -44,6 +44,8 @@ bfd_elf64_loongarch_set_data_segment_info (struct bfd_link_info *, int *);
>   bfd_byte *
>   loongarch_write_unsigned_leb128 (bfd_byte *p, unsigned int len, bfd_vma value);
>   
> +int loongarch_get_uleb128_length (bfd_byte *buf);
> +
>   /* TRUE if this is a PLT reference to a local IFUNC.  */
>   #define PLT_LOCAL_IFUNC_P(INFO, H) \
>     ((H)->dynindx == -1 \
> diff --git a/gas/config/tc-loongarch.c b/gas/config/tc-loongarch.c
> index c55d4ee234a..88c5282918e 100644
> --- a/gas/config/tc-loongarch.c
> +++ b/gas/config/tc-loongarch.c
> @@ -1381,7 +1381,7 @@ md_apply_fix (fixS *fixP, valueT *valP, segT seg ATTRIBUTE_UNUSED)
>       case BFD_RELOC_LARCH_SUB_ULEB128:
>         {
>   	unsigned int len = 0;
> -	_bfd_read_unsigned_leb128 (NULL, (bfd_byte *)buf, &len);
> +	len = loongarch_get_uleb128_length ((bfd_byte *)buf);
>   	bfd_byte *endp = (bfd_byte*) buf + len -1;
>   	/* Clean the uleb128 value to 0. Do not reduce the length.  */
>   	memset (buf, 0x80, len - 1);
  

Patch

diff --git a/bfd/elfxx-loongarch.c b/bfd/elfxx-loongarch.c
index ac644fff2f6..cb312c46bb2 100644
--- a/bfd/elfxx-loongarch.c
+++ b/bfd/elfxx-loongarch.c
@@ -1944,3 +1944,10 @@  loongarch_write_unsigned_leb128 (bfd_byte *p, unsigned int len, bfd_vma value)
   while (len);
   return p;
 }
+
+int loongarch_get_uleb128_length (bfd_byte *buf)
+{
+  unsigned int len = 0;
+  _bfd_read_unsigned_leb128 (NULL, buf, &len);
+  return len;
+}
diff --git a/bfd/elfxx-loongarch.h b/bfd/elfxx-loongarch.h
index cb02af4fe73..627c5db7bcc 100644
--- a/bfd/elfxx-loongarch.h
+++ b/bfd/elfxx-loongarch.h
@@ -44,6 +44,8 @@  bfd_elf64_loongarch_set_data_segment_info (struct bfd_link_info *, int *);
 bfd_byte *
 loongarch_write_unsigned_leb128 (bfd_byte *p, unsigned int len, bfd_vma value);
 
+int loongarch_get_uleb128_length (bfd_byte *buf);
+
 /* TRUE if this is a PLT reference to a local IFUNC.  */
 #define PLT_LOCAL_IFUNC_P(INFO, H) \
   ((H)->dynindx == -1 \
diff --git a/gas/config/tc-loongarch.c b/gas/config/tc-loongarch.c
index c55d4ee234a..88c5282918e 100644
--- a/gas/config/tc-loongarch.c
+++ b/gas/config/tc-loongarch.c
@@ -1381,7 +1381,7 @@  md_apply_fix (fixS *fixP, valueT *valP, segT seg ATTRIBUTE_UNUSED)
     case BFD_RELOC_LARCH_SUB_ULEB128:
       {
 	unsigned int len = 0;
-	_bfd_read_unsigned_leb128 (NULL, (bfd_byte *)buf, &len);
+	len = loongarch_get_uleb128_length ((bfd_byte *)buf);
 	bfd_byte *endp = (bfd_byte*) buf + len -1;
 	/* Clean the uleb128 value to 0. Do not reduce the length.  */
 	memset (buf, 0x80, len - 1);