[Bug binutils/26246] New: invalid access in bfd/elf32-xtensa.c

[msebor at gmail dot com]

https://sourceware.org/bugzilla/show_bug.cgi?id=26246

            Bug ID: 26246
           Summary: invalid access in bfd/elf32-xtensa.c
           Product: binutils
           Version: 2.36 (HEAD)
            Status: UNCONFIRMED
          Severity: normal
          Priority: P2
         Component: binutils
          Assignee: unassigned at sourceware dot org
          Reporter: msebor at gmail dot com
  Target Milestone: ---

A Binutils test build with a GCC 11 enhancement to -Warray-bounds triggers the
following warning:

src/binutils-gdb/bfd/elf32-xtensa.c: In function ‘find_removed_literal’:
/src/binutils-gdb/bfd/elf32-xtensa.c:5995:9: warning: array subscript
‘removed_literal_map_entry {aka const struct
removed_literal_map_entry_struct}[0]’ is partly outside array bounds of
‘bfd_vma[1]’ {aka ‘long unsigned int[1]’} [-Warray-bounds]
 5995 |   if (pa->addr == pb->addr)
      |       ~~^~~~~~
/src/binutils-gdb/bfd/elf32-xtensa.c:6005:67: note: while referencing ‘addr’
 6005 | find_removed_literal (removed_literal_list *removed_list, bfd_vma addr)
      |                                                           ~~~~~~~~^~~~

Although the code may be benign in practice I believe the warning points out a
subtle bug.  find_removed_literal() calls bsearch with the address of its addr
argument like this:

static removed_literal *
find_removed_literal (removed_literal_list *removed_list, bfd_vma addr)
{
  removed_literal_map_entry *p;
  removed_literal *r = NULL;

  if (removed_list->map == NULL)
    map_removed_literal (removed_list);

  p = bsearch (&addr, removed_list->map, removed_list->n_map,
               sizeof (*removed_list->map), removed_literal_compare);
  ...

The address of addr is then cast in removed_literal_compare() to
removed_literal_map_entry* and used to access its addr member: 

static int
removed_literal_compare (const void *a, const void *b)
{
  const removed_literal_map_entry *pa = a;
  const removed_literal_map_entry *pb = b;

  if (pa->addr == pb->addr)
    return 0;

addr happens to be the first member of removed_literal_map_entry so the access
pa->addr is at offset zero but strictly speaking, this sort of aliasing is
undefined and not supported by GCC.  Changing find_removed_literal like so:

  removed_literal_map_entry key = { addr };
  p = bsearch (&key, removed_list->map, removed_list->n_map,

avoids the warning and may even result in better code.

-- 
You are receiving this mail because:
You are on the CC list for the bug.