[PATCH v3 03/18] VMS/BFD: Fix a sign extension issue with archive symbol lookup

[Maciej W. Rozycki]

From: Maciej W. Rozycki <macro@redhat.com>

Symbol binary search code for VMS archive files uses plain `char' data 
type to cast a difference between characters to data of the `int' type.  
Consequently the difference is consider unsigned in the range between 0 
and 255 on hosts where plain `char' data type is unsigned, resulting in
symbol lookup failures, such as with the test expansion included with 
this change causing regressions as follows:

FAIL: Regular archive link
FAIL: Thin archive link
FAIL: Regular archive plus regular link
FAIL: Regular archive plus thin link
FAIL: Thin archive plus regular link
FAIL: Thin archive plus thin link

owing to link failures such as:

.../ld/ld-new: tmpdir/abc.o:($DATA$+0x0): undefined reference to `aa'
.../ld/ld-new: tmpdir/ab.a(b.obj):($DATA$+0x10): undefined reference to `aa'
.../ld/ld-new: tmpdir/abc.o:($DATA$+0x0): undefined reference to `aa'
.../ld/ld-new: tmpdir/ab.a(b.obj):($DATA$+0x10): undefined reference to `aa'

with the `alpha-dec-vms' target on the `powerpc64le-linux-gnu' host.

Use explicit `signed char' data type for the cast then, removing the 
failures.
---
 NB I have no idea of what the code author meant given that the difference 
calculated is already between data objects of the `char' type, so integer 
promotion would cause the result to be correctly represented if directly 
assigned to the destination `diff' variable of the `int' type.  Then the 
cast only breaks the result.  This code has always been like this ever 
since its introduction back in 2010 and therefore never worked correctly 
with unsigned plain `char' data type hosts.  Probably just never verified 
with such a host, complemented by the lack of testsuite coverage so far, 
and then 7 years of breakage from 2018 until recently didn't help either.

 For code clarity though I chose to use an explicit cast rather removing 
it altogether.

No change from v2, 
<https://inbox.sourceware.org/binutils/alpine.DEB.2.21.2511060126480.25436@angie.orcam.me.uk/>.

New change in v2.
---
 bfd/vms-lib.c                       |    2 +-
 ld/testsuite/ld-archive/archive.exp |   12 ++++++------
 ld/testsuite/ld-archive/x.s         |    4 ++++
 ld/testsuite/ld-archive/y.s         |    4 ++++
 4 files changed, 15 insertions(+), 7 deletions(-)

binutils-bfd-vms-lib-find-symbol-signed-char.diff
Index: binutils-gdb/bfd/vms-lib.c
===================================================================
--- binutils-gdb.orig/bfd/vms-lib.c
+++ binutils-gdb/bfd/vms-lib.c
@@ -800,7 +800,7 @@ _bfd_vms_lib_find_symbol (bfd *abfd, con
       int mid = lo + (hi - lo) / 2;
       int diff;
 
-      diff = (char)(name[0] - syms[mid].name[0]);
+      diff = (signed char) (name[0] - syms[mid].name[0]);
       if (diff == 0)
 	diff = strcmp (name, syms[mid].name);
       if (diff == 0)
Index: binutils-gdb/ld/testsuite/ld-archive/archive.exp
===================================================================
--- binutils-gdb.orig/ld/testsuite/ld-archive/archive.exp
+++ binutils-gdb/ld/testsuite/ld-archive/archive.exp
@@ -23,12 +23,12 @@ remote_file host delete \
     "tmpdir/abn.a" "tmpdir/abnt.a"
 
 run_ld_link_tests {
-    {"First regular archive create"     ""   "" "" {a.s b.s} {} "ab.a"  }
-    {"Second regular archive create"    ""   "" "" {c.s d.s} {} "cd.a"  }
-    {"First thin archive create"        "T"  "" "" {a.s b.s} {} "abt.a" }
-    {"Second thin archive create"       "T"  "" "" {c.s d.s} {} "cdt.a" }
-    {"Regular archive w/o index create" "S"  "" "" {a.s b.s} {} "abn.a" }
-    {"Thin archive w/o index create"    "ST" "" "" {a.s b.s} {} "abnt.a"}
+    {"First regular archive create"     ""   "" "" {a.s b.s x.s} {} "ab.a"   }
+    {"Second regular archive create"    ""   "" "" {c.s d.s y.s} {} "cd.a"   }
+    {"First thin archive create"        "T"  "" "" {a.s b.s x.s} {} "abt.a"  }
+    {"Second thin archive create"       "T"  "" "" {c.s d.s y.s} {} "cdt.a"  }
+    {"Regular archive w/o index create" "S"  "" "" {a.s b.s x.s} {} "abn.a"  }
+    {"Thin archive w/o index create"    "ST" "" "" {a.s b.s x.s} {} "abnt.a" }
 }
 
 set old_ldflags $LDFLAGS
Index: binutils-gdb/ld/testsuite/ld-archive/x.s
===================================================================
--- /dev/null
+++ binutils-gdb/ld/testsuite/ld-archive/x.s
@@ -0,0 +1,4 @@
+	.data
+	.globl	xx
+xx:
+	.dc.a	0
Index: binutils-gdb/ld/testsuite/ld-archive/y.s
===================================================================
--- /dev/null
+++ binutils-gdb/ld/testsuite/ld-archive/y.s
@@ -0,0 +1,4 @@
+	.data
+	.globl	yy
+yy:
+	.dc.a	0