[patchv3 0/8] Validate binary before use

Jan Kratochvil
2014-02-27T21:33:33Z

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

Hi,

git://sourceware.org/git/archer.git
jankratochvil/gdbserverbuildid
- although it is not properly separated into 1..8 there as in this series

the is a follow-up to never checked in series:
Message-ID: <1366127096-5744-1-git-send-email-ARistovski@qnx.com>
https://sourceware.org/ml/gdb-patches/2013-04/msg00472.html

The changes [attached] are:

* Implemented the review comments I made.
(Except for comments for the testcase.)

* Removed fetching build-id in solib-svr4.c for NAT run:
/* There is no way to safely fetch build-id from running inferior without OS
specific code. The code from get_hex_build_id from gdbserver/linux-low.c
could be used for GNU/Linux NAT target. */

As former code regressed break-interp.exp with 32-bit targets for:
egrep '(BINprelinkNO.*pieATTACH.*relinkYES|BINprelinkYES.*pieATTACH.*relinkNO).*attach main bt'

Implementing new feature into gdbserver and not linux-nat seems correct to
me according to:
https://sourceware.org/gdb/wiki/LocalRemoteFeatureParity
I will list there this local/remote difference after check-in but that
should be OK after the final switch to gdbserver.

I left there authorship intact except for added mine to [patchv3 7/8].
I do not mind to change it any way.

Jan

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

diff --git a/gdb/common/common-target.c b/gdb/common/common-target.c
index 4bd2731..d6e5d60 100644
--- a/gdb/common/common-target.c
+++ b/gdb/common/common-target.c
@@ -1,6 +1,6 @@
/* Utility target functions for GDB, and GDBserver.

- Copyright (C) 2013 Free Software Foundation, Inc.
+ Copyright (C) 2014 Free Software Foundation, Inc.

This file is part of GDB.

diff --git a/gdb/common/common-target.h b/gdb/common/common-target.h
index b252c00..9aedc12 100644
--- a/gdb/common/common-target.h
+++ b/gdb/common/common-target.h
@@ -1,6 +1,6 @@
/* Utility target functions for GDB, and GDBserver.

- Copyright (C) 2013 Free Software Foundation, Inc.
+ Copyright (C) 2014 Free Software Foundation, Inc.

This file is part of GDB.

diff --git a/gdb/common/linux-maps.c b/gdb/common/linux-maps.c
index 6432b8f..bb587df 100644
--- a/gdb/common/linux-maps.c
+++ b/gdb/common/linux-maps.c
@@ -1,5 +1,5 @@
/* Linux-specific memory maps manipulation routines.
- Copyright (C) 2013 Free Software Foundation, Inc.
+ Copyright (C) 2014 Free Software Foundation, Inc.

This file is part of GDB.

diff --git a/gdb/common/linux-maps.h b/gdb/common/linux-maps.h
index e989376..03b14d3 100644
--- a/gdb/common/linux-maps.h
+++ b/gdb/common/linux-maps.h
@@ -1,5 +1,5 @@
/* Linux-specific memory maps manipulation routines.
- Copyright (C) 2013 Free Software Foundation, Inc.
+ Copyright (C) 2014 Free Software Foundation, Inc.

This file is part of GDB.

diff --git a/gdb/gdbserver/linux-low.c b/gdb/gdbserver/linux-low.c
index d14b215..6f05bd4 100644
--- a/gdb/gdbserver/linux-low.c
+++ b/gdb/gdbserver/linux-low.c
@@ -163,10 +163,7 @@ typedef union ElfXX_Nhdr
#define ELFXX_SIZEOF(elf64, hdr) ((elf64) ? sizeof ((hdr)._64) \
: sizeof ((hdr)._32))
/* Round up to next 4 byte boundary. */
-#define ELFXX_ROUNDUP_4(elf64, what) ((elf64) ? (((what) + 3) \
- & ~((Elf64_Word)3U)) \
- : (((what) + 3) \
- & ~((Elf32_Word) 3U)))
+#define ELFXX_ROUNDUP_4(elf64, what) (((what) + 3) & ~(ULONGEST) 3)
#define BUILD_ID_INVALID "?"

/* A list of all unknown processes which receive stop signals. Some
@@ -5764,7 +5761,7 @@ free_mapping_entry_vec (VEC (mapping_entry_s) *lst)
static int
compare_mapping_entry_range (const void *const k, const void *const b)
{
- const ULONGEST key = *(CORE_ADDR *) k;
+ const ULONGEST key = *(const CORE_ADDR *) k;
const mapping_entry_s *const p = b;

if (key < p->vaddr)
@@ -5812,7 +5809,8 @@ read_build_id (struct find_memory_region_callback_data *const p,
|| e_phentsize != ELFXX_SIZEOF (is_elf64, *phdr))
{
/* Basic sanity check failed. */
- warning ("Could not read program header.");
+ warning (_("Could not identify program header at %s."),
+ paddress (load_addr));
return;
}

@@ -5823,7 +5821,8 @@ read_build_id (struct find_memory_region_callback_data *const p,
ELFXX_FLD (is_elf64, ehdr, e_phnum) * e_phentsize)
!= 0)
{
- warning ("Could not read program header.");
+ warning (_("Could not read program header at %s."),
+ paddress (load_addr));
return;
}

@@ -5847,14 +5846,14 @@ read_build_id (struct find_memory_region_callback_data *const p,
ELFXX_FLD (is_elf64, *phdr, p_memsz)) != 0)
{
xfree (pt_note);
- warning ("Could not read note at address 0x%s",
+ warning (_("Could not read note at address 0x%s"),
paddress (note_addr));
break;
}

pt_end = pt_note + ELFXX_FLD (is_elf64, *phdr, p_memsz);
nhdr = (void *) pt_note;
- while ((gdb_byte *) nhdr < pt_end)
+ while ((const gdb_byte *) nhdr < pt_end)
{
const size_t namesz
= ELFXX_ROUNDUP_4 (is_elf64, ELFXX_FLD (is_elf64, *nhdr,
@@ -5862,14 +5861,14 @@ read_build_id (struct find_memory_region_callback_data *const p,
const size_t descsz
= ELFXX_ROUNDUP_4 (is_elf64, ELFXX_FLD (is_elf64, *nhdr,
n_descsz));
- const size_t note_sz = ELFXX_SIZEOF (is_elf64, *nhdr) + namesz
- + descsz;
+ const size_t note_sz = (ELFXX_SIZEOF (is_elf64, *nhdr) + namesz
+ + descsz);

- if (((gdb_byte *) nhdr + note_sz) > pt_end || note_sz == 0
+ if (((const gdb_byte *) nhdr + note_sz) > pt_end || note_sz == 0
|| descsz == 0)
{
- warning ("Malformed PT_NOTE at address 0x%s\n",
- paddress ((CORE_ADDR) nhdr));
+ warning (_("Malformed PT_NOTE at address 0x%s\n"),
+ paddress (note_addr + (gdb_byte *) nhdr - pt_note));
break;
}
if (ELFXX_FLD (is_elf64, *nhdr, n_type) == NT_GNU_BUILD_ID
@@ -5981,9 +5980,10 @@ get_hex_build_id (const CORE_ADDR l_addr, const CORE_ADDR l_ld,
{
/* Do not try to find hex_build_id again. */
bil->hex_build_id = xstrdup (BUILD_ID_INVALID);
- warning ("Could not determine load address; mapping entry with"
- " offset 0 corresponding to l_ld=0x%s could not be found; "
- "build-id can not be used.", paddress (l_ld));
+ warning (_("Could not determine load address; mapping entry with "
+ "offset 0 corresponding to l_ld = 0x%s could not be "
+ "found; build-id can not be used."),
+ paddress (l_ld));
}
}

@@ -6050,7 +6050,7 @@ linux_qxfer_libraries_svr4 (const char *annex, unsigned char *readbuf,
VEC_reserve (mapping_entry_s, data.list, 16);
if (linux_find_memory_regions_full (pid, find_memory_region_callback, &data,
NULL) < 0)
- warning ("Finding memory regions failed");
+ warning (_("Finding memory regions failed"));

while (annex[0] != '\0')
{
diff --git a/gdb/linux-tdep.c b/gdb/linux-tdep.c
index 235a7c2..d0c8761 100644
--- a/gdb/linux-tdep.c
+++ b/gdb/linux-tdep.c
@@ -737,7 +737,7 @@ linux_find_memory_regions_gdb (struct gdbarch *gdbarch,
/* We need to know the real target PID so
linux_find_memory_regions_full can access /proc. */
if (current_inferior ()->fake_pid_p)
- return 1;
+ return -1;

cleanup = make_cleanup (free_current_contents, &memory_to_free);
retval = linux_find_memory_regions_full (current_inferior ()->pid,
diff --git a/gdb/solib-svr4.c b/gdb/solib-svr4.c
index 9de1ae7..7d4cce9 100644
--- a/gdb/solib-svr4.c
+++ b/gdb/solib-svr4.c
@@ -968,12 +968,14 @@ svr4_keep_data_in_core (CORE_ADDR vaddr, unsigned long size)
static int
svr4_validate (const struct so_list *const so)
{
- gdb_byte *build_id;
- size_t build_idsz;
- size_t abfd_build_idsz;
-
gdb_assert (so != NULL);

+ /* There is no way to safely fetch build-id from running inferior without OS
+ specific code. The code from get_hex_build_id from gdbserver/linux-low.c
+ could be used for GNU/Linux NAT target. */
+ if (so->build_id == NULL)
+ return 1;
+
if (so->abfd == NULL)
return 1;

@@ -982,96 +984,9 @@ svr4_validate (const struct so_list *const so)
|| elf_tdata (so->abfd)->build_id == NULL)
return 1;

- build_id = so->build_id;
- build_idsz = so->build_idsz;
- abfd_build_idsz = elf_tdata (so->abfd)->build_id->size;
-
- if (build_id == NULL)
- {
- /* Get build_id from NOTE_GNU_BUILD_ID_NAME section.
- This is a fallback mechanism for targets that do not
- implement TARGET_OBJECT_SOLIB_SVR4. */
-
- const asection *const asec
- = bfd_get_section_by_name (so->abfd, NOTE_GNU_BUILD_ID_NAME);
- ULONGEST bfd_sect_size;
-
- if (asec == NULL)
- return 1;
-
- bfd_sect_size = bfd_get_section_size (asec);
-
- if ((asec->flags & SEC_LOAD) == SEC_LOAD
- && bfd_sect_size != 0
- && strcmp (bfd_section_name (asec->bfd, asec),
- NOTE_GNU_BUILD_ID_NAME) == 0)
- {
- const enum bfd_endian byte_order
- = gdbarch_byte_order (target_gdbarch ());
- Elf_External_Note *const note = xmalloc (bfd_sect_size);
- gdb_byte *const note_raw = (void *) note;
- struct cleanup *cleanups = make_cleanup (xfree, note);
-
- if (target_read_memory (bfd_get_section_vma (so->abfd, asec)
- + lm_addr_check (so, so->abfd),
- note_raw, bfd_sect_size) == 0)
- {
- build_idsz
- = extract_unsigned_integer ((gdb_byte *) note->descsz,
- sizeof (note->descsz),
- byte_order);
-
- if (build_idsz == abfd_build_idsz)
- {
- const char gnu[4] = "GNU\0";
-
- if (memcmp (note->name, gnu, 4) == 0)
- {
- ULONGEST namesz
- = extract_unsigned_integer ((gdb_byte *) note->namesz,
- sizeof (note->namesz),
- byte_order);
- CORE_ADDR build_id_offs;
-
- /* Rounded to next sizeof (ElfXX_Word). */
- namesz = ((namesz + (sizeof (note->namesz) - 1))
- & ~((ULONGEST) (sizeof (note->namesz) - 1)));
- build_id_offs = (offsetof (Elf_External_Note, name)
- + namesz);
- build_id = xmalloc (build_idsz);
- memcpy (build_id, note_raw + build_id_offs, build_idsz);
- }
- }
-
- if (build_id == NULL)
- {
- /* If we are here, it means target memory read succeeded
- but note was not where it was expected according to the
- abfd. Allow the logic below to perform the check
- with an impossible build-id and fail validation. */
- build_idsz = 0;
- build_id = xmalloc (0);
- }
-
- }
- do_cleanups (cleanups);
- }
- }
-
- if (build_id != NULL)
- {
- const int match
- = (abfd_build_idsz == build_idsz
- && memcmp (build_id, elf_tdata (so->abfd)->build_id->data,
- build_idsz) == 0);
-
- if (build_id != so->build_id)
- xfree (build_id);
-
- return match;
- }
-
- return 1;
+ return (so->build_idsz == elf_tdata (so->abfd)->build_id->size
+ && memcmp (so->build_id, elf_tdata (so->abfd)->build_id->data,
+ so->build_idsz) == 0);
}

/* Implement the "open_symbol_file_object" target_so_ops method.
diff --git a/gdb/target.c b/gdb/target.c
index abf9058..d892e25 100644
--- a/gdb/target.c
+++ b/gdb/target.c
@@ -3020,9 +3020,6 @@ target_fileio_close_cleanup (void *opaque)
target_fileio_close (fd, &target_errno);
}

-typedef int (read_alloc_pread_ftype) (int handle, gdb_byte *read_buf, int len,
- ULONGEST offset, int *target_errno);
-
static read_alloc_pread_ftype target_fileio_read_alloc_1_pread;

/* Helper for target_fileio_read_alloc_1 to make it interruptible. */
@@ -3036,9 +3033,6 @@ target_fileio_read_alloc_1_pread (int handle, gdb_byte *read_buf, int len,
return target_fileio_pread (handle, read_buf, len, offset, target_errno);
}

-typedef LONGEST (read_stralloc_func_ftype) (const char *filename,
- gdb_byte **buf_p, int padding);
-
static read_stralloc_func_ftype target_fileio_read_alloc_1;

/* Read target file FILENAME. Store the result in *BUF_P and
diff --git a/gdb/testsuite/gdb.base/solib-mismatch-lib.c b/gdb/testsuite/gdb.base/solib-mismatch-lib.c
index 19f1545..65b26af 100644
--- a/gdb/testsuite/gdb.base/solib-mismatch-lib.c
+++ b/gdb/testsuite/gdb.base/solib-mismatch-lib.c
@@ -1,6 +1,6 @@
/* This testcase is part of GDB, the GNU debugger.

- Copyright 2013 Free Software Foundation, Inc.
+ Copyright 2014 Free Software Foundation, Inc.

This program is free software; you can redistribute it and/or modify
it under the terms of the GNU General Public License as published by
diff --git a/gdb/testsuite/gdb.base/solib-mismatch-libmod.c b/gdb/testsuite/gdb.base/solib-mismatch-libmod.c
index 3b025a8..fc8827e 100644
--- a/gdb/testsuite/gdb.base/solib-mismatch-libmod.c
+++ b/gdb/testsuite/gdb.base/solib-mismatch-libmod.c
@@ -1,6 +1,6 @@
/* This testcase is part of GDB, the GNU debugger.

- Copyright 2013 Free Software Foundation, Inc.
+ Copyright 2014 Free Software Foundation, Inc.

This program is free software; you can redistribute it and/or modify
it under the terms of the GNU General Public License as published by
diff --git a/gdb/testsuite/gdb.base/solib-mismatch.c b/gdb/testsuite/gdb.base/solib-mismatch.c
index 7a5d960..7bf425d 100644
--- a/gdb/testsuite/gdb.base/solib-mismatch.c
+++ b/gdb/testsuite/gdb.base/solib-mismatch.c
@@ -1,6 +1,6 @@
/* This testcase is part of GDB, the GNU debugger.

- Copyright 2013 Free Software Foundation, Inc.
+ Copyright 2014 Free Software Foundation, Inc.

This program is free software; you can redistribute it and/or modify
it under the terms of the GNU General Public License as published by
diff --git a/gdb/testsuite/gdb.base/solib-mismatch.exp b/gdb/testsuite/gdb.base/solib-mismatch.exp
index 4504bc3..7a625ba 100644
--- a/gdb/testsuite/gdb.base/solib-mismatch.exp
+++ b/gdb/testsuite/gdb.base/solib-mismatch.exp
@@ -1,4 +1,4 @@
-# Copyright 2013 Free Software Foundation, Inc.
+# Copyright 2014 Free Software Foundation, Inc.

# This program is free software; you can redistribute it and/or modify
# it under the terms of the GNU General Public License as published by
@@ -16,6 +16,11 @@
standard_testfile
set executable $testfile

+if ![is_remote target] {
+ untested "only gdbserver supports build-id reporting"
+ return -1
+}
+
# Test overview:
# generate two shared objects. One that will be used by the process
# and another, modified, that will be found by gdb. Gdb should
@@ -69,7 +74,7 @@ if { [build_executable $testfile.exp $executable $srcfile $exec_opts] != 0 } {
if { [gdb_compile_shlib "${srcdir}/${subdir}/${srclibfilerun}" "${binlibfilerun}" [list debug ldflags=-Wl,--build-id]] != ""
|| [gdb_gnu_strip_debug "${binlibfilerun}"]
|| [gdb_compile_shlib "${srcdir}/${subdir}/${srclibfilegdb}" "${binlibfilegdb}" [list debug ldflags=-Wl,--build-id]] != "" } {
- untested "gdb_compile_shlib failed."
+ untested "compilation failed."
return -1
}