* [PATCH v5 1/8] Move utility functions to common/
2014-03-19 23:12 [PATCH v5 0/8] Validate binary before use Jan Kratochvil
@ 2014-03-19 22:30 ` Jan Kratochvil
2014-03-24 20:00 ` Sergio Durigan Junior
2014-05-19 18:21 ` Tom Tromey
2014-03-19 22:31 ` [PATCH v5 7/8] Validate symbol file using build-id Jan Kratochvil
` (6 subsequent siblings)
7 siblings, 2 replies; 34+ messages in thread
From: Jan Kratochvil @ 2014-03-19 22:30 UTC (permalink / raw)
To: gdb-patches; +Cc: Aleksandar Ristovski
Hi,
some parts of the former patch have been reimplemented in the meantime by
other patches so this is only a part of the former cleanup.
Jan
gdb/
2014-02-26 Aleksandar Ristovski <aristovski@qnx.com
Move utility functions to common/.
* cli/cli-utils.c (skip_spaces, skip_spaces_const): Move defs to
common/common-utils.c.
* cli/cli-utils.h (skip_spaces, skip_spaces_const): Move decls to
common/common-utils.h.
* common/common-utils.c (host-defs.h, ctype.h): Include.
(HIGH_BYTE_POSN, is_digit_in_base, digit_to_int, strtoulst): Move
from utils.c.
(skip_spaces, skip_spaces_const): Move from cli/cli-utils.c.
* common/host-defs.h (TARGET_CHAR_BIT, HOST_CHAR_BIT): Moved from
defs.h.
* common/common-utils.h (strtoulst): Move decl from utils.h.
(hex2bin, bin2hex): Move decls from remote.h.
(skip_spaces, skip_spaces_const): Move decls from cli/cli-utils.h.
* defs.h (TARGET_CHAR_BIT, HOST_CHAR_BIT): Move to
common/common-utils.h
* utils.c (HIGH_BYTE_POSN, is_digit_in_base, digit_to_int)
(strtoulst): Move to common/common-utils.c.
* utils.h (strtoulst): Moved decl to common/common-utils.h.
---
gdb/cli/cli-utils.c | 24 ---------
gdb/cli/cli-utils.h | 9 ---
gdb/common/common-utils.c | 125 +++++++++++++++++++++++++++++++++++++++++++++
gdb/common/common-utils.h | 11 ++++
gdb/common/host-defs.h | 21 ++++++++
gdb/defs.h | 19 -------
gdb/utils.c | 99 ------------------------------------
gdb/utils.h | 2 -
8 files changed, 157 insertions(+), 153 deletions(-)
diff --git a/gdb/cli/cli-utils.c b/gdb/cli/cli-utils.c
index a0ebc11..49cdf83 100644
--- a/gdb/cli/cli-utils.c
+++ b/gdb/cli/cli-utils.c
@@ -213,30 +213,6 @@ number_is_in_list (char *list, int number)
/* See documentation in cli-utils.h. */
-char *
-skip_spaces (char *chp)
-{
- if (chp == NULL)
- return NULL;
- while (*chp && isspace (*chp))
- chp++;
- return chp;
-}
-
-/* A const-correct version of the above. */
-
-const char *
-skip_spaces_const (const char *chp)
-{
- if (chp == NULL)
- return NULL;
- while (*chp && isspace (*chp))
- chp++;
- return chp;
-}
-
-/* See documentation in cli-utils.h. */
-
const char *
skip_to_space_const (const char *chp)
{
diff --git a/gdb/cli/cli-utils.h b/gdb/cli/cli-utils.h
index fed876b..c7f27e2 100644
--- a/gdb/cli/cli-utils.h
+++ b/gdb/cli/cli-utils.h
@@ -89,15 +89,6 @@ extern int get_number_or_range (struct get_number_or_range_state *state);
extern int number_is_in_list (char *list, int number);
-/* Skip leading whitespace characters in INP, returning an updated
- pointer. If INP is NULL, return NULL. */
-
-extern char *skip_spaces (char *inp);
-
-/* A const-correct version of the above. */
-
-extern const char *skip_spaces_const (const char *inp);
-
/* Skip leading non-whitespace characters in INP, returning an updated
pointer. If INP is NULL, return NULL. */
diff --git a/gdb/common/common-utils.c b/gdb/common/common-utils.c
index 29fe2c5..3e0c15e 100644
--- a/gdb/common/common-utils.c
+++ b/gdb/common/common-utils.c
@@ -23,11 +23,13 @@
#include "defs.h"
#endif
+#include "host-defs.h"
#include "gdb_assert.h"
#include <string.h>
#include <stdlib.h>
#include <stdio.h>
+#include <ctype.h>
/* The xmalloc() (libiberty.h) family of memory management routines.
@@ -161,3 +163,126 @@ savestring (const char *ptr, size_t len)
p[len] = 0;
return p;
}
+
+/* The bit offset of the highest byte in a ULONGEST, for overflow
+ checking. */
+
+#define HIGH_BYTE_POSN ((sizeof (ULONGEST) - 1) * HOST_CHAR_BIT)
+
+/* True (non-zero) iff DIGIT is a valid digit in radix BASE,
+ where 2 <= BASE <= 36. */
+
+static int
+is_digit_in_base (unsigned char digit, int base)
+{
+ if (!isalnum (digit))
+ return 0;
+ if (base <= 10)
+ return (isdigit (digit) && digit < base + '0');
+ else
+ return (isdigit (digit) || tolower (digit) < base - 10 + 'a');
+}
+
+static int
+digit_to_int (unsigned char c)
+{
+ if (isdigit (c))
+ return c - '0';
+ else
+ return tolower (c) - 'a' + 10;
+}
+
+/* As for strtoul, but for ULONGEST results. */
+
+ULONGEST
+strtoulst (const char *num, const char **trailer, int base)
+{
+ unsigned int high_part;
+ ULONGEST result;
+ int minus = 0;
+ int i = 0;
+
+ /* Skip leading whitespace. */
+ while (isspace (num[i]))
+ i++;
+
+ /* Handle prefixes. */
+ if (num[i] == '+')
+ i++;
+ else if (num[i] == '-')
+ {
+ minus = 1;
+ i++;
+ }
+
+ if (base == 0 || base == 16)
+ {
+ if (num[i] == '0' && (num[i + 1] == 'x' || num[i + 1] == 'X'))
+ {
+ i += 2;
+ if (base == 0)
+ base = 16;
+ }
+ }
+
+ if (base == 0 && num[i] == '0')
+ base = 8;
+
+ if (base == 0)
+ base = 10;
+
+ if (base < 2 || base > 36)
+ {
+ errno = EINVAL;
+ return 0;
+ }
+
+ result = high_part = 0;
+ for (; is_digit_in_base (num[i], base); i += 1)
+ {
+ result = result * base + digit_to_int (num[i]);
+ high_part = high_part * base + (unsigned int) (result >> HIGH_BYTE_POSN);
+ result &= ((ULONGEST) 1 << HIGH_BYTE_POSN) - 1;
+ if (high_part > 0xff)
+ {
+ errno = ERANGE;
+ result = ~ (ULONGEST) 0;
+ high_part = 0;
+ minus = 0;
+ break;
+ }
+ }
+
+ if (trailer != NULL)
+ *trailer = &num[i];
+
+ result = result + ((ULONGEST) high_part << HIGH_BYTE_POSN);
+ if (minus)
+ return -result;
+ else
+ return result;
+}
+
+/* See documentation in cli-utils.h. */
+
+char *
+skip_spaces (char *chp)
+{
+ if (chp == NULL)
+ return NULL;
+ while (*chp && isspace (*chp))
+ chp++;
+ return chp;
+}
+
+/* A const-correct version of the above. */
+
+const char *
+skip_spaces_const (const char *chp)
+{
+ if (chp == NULL)
+ return NULL;
+ while (*chp && isspace (*chp))
+ chp++;
+ return chp;
+}
diff --git a/gdb/common/common-utils.h b/gdb/common/common-utils.h
index 063698d..97889d5 100644
--- a/gdb/common/common-utils.h
+++ b/gdb/common/common-utils.h
@@ -71,4 +71,15 @@ int xsnprintf (char *str, size_t size, const char *format, ...)
char *savestring (const char *ptr, size_t len);
+ULONGEST strtoulst (const char *num, const char **trailer, int base);
+
+/* Skip leading whitespace characters in INP, returning an updated
+ pointer. If INP is NULL, return NULL. */
+
+extern char *skip_spaces (char *inp);
+
+/* A const-correct version of the above. */
+
+extern const char *skip_spaces_const (const char *inp);
+
#endif
diff --git a/gdb/common/host-defs.h b/gdb/common/host-defs.h
index e4acef0..71a7029 100644
--- a/gdb/common/host-defs.h
+++ b/gdb/common/host-defs.h
@@ -19,6 +19,27 @@
#ifndef HOST_DEFS_H
#define HOST_DEFS_H
+#include <limits.h>
+
+/* Static host-system-dependent parameters for GDB. */
+
+/* * Number of bits in a char or unsigned char for the target machine.
+ Just like CHAR_BIT in <limits.h> but describes the target machine. */
+#if !defined (TARGET_CHAR_BIT)
+#define TARGET_CHAR_BIT 8
+#endif
+
+/* * If we picked up a copy of CHAR_BIT from a configuration file
+ (which may get it by including <limits.h>) then use it to set
+ the number of bits in a host char. If not, use the same size
+ as the target. */
+
+#if defined (CHAR_BIT)
+#define HOST_CHAR_BIT CHAR_BIT
+#else
+#define HOST_CHAR_BIT TARGET_CHAR_BIT
+#endif
+
#ifdef __MSDOS__
# define CANT_FORK
# define GLOBAL_CURDIR
diff --git a/gdb/defs.h b/gdb/defs.h
index 47da43a..1aa7ad3 100644
--- a/gdb/defs.h
+++ b/gdb/defs.h
@@ -633,25 +633,6 @@ extern void *alloca ();
enum { MAX_REGISTER_SIZE = 64 };
-/* Static target-system-dependent parameters for GDB. */
-
-/* * Number of bits in a char or unsigned char for the target machine.
- Just like CHAR_BIT in <limits.h> but describes the target machine. */
-#if !defined (TARGET_CHAR_BIT)
-#define TARGET_CHAR_BIT 8
-#endif
-
-/* * If we picked up a copy of CHAR_BIT from a configuration file
- (which may get it by including <limits.h>) then use it to set
- the number of bits in a host char. If not, use the same size
- as the target. */
-
-#if defined (CHAR_BIT)
-#define HOST_CHAR_BIT CHAR_BIT
-#else
-#define HOST_CHAR_BIT TARGET_CHAR_BIT
-#endif
-
/* In findvar.c. */
extern LONGEST extract_signed_integer (const gdb_byte *, int,
diff --git a/gdb/utils.c b/gdb/utils.c
index 364470c..21186b8 100644
--- a/gdb/utils.c
+++ b/gdb/utils.c
@@ -3026,105 +3026,6 @@ dummy_obstack_deallocate (void *object, void *data)
return;
}
-/* The bit offset of the highest byte in a ULONGEST, for overflow
- checking. */
-
-#define HIGH_BYTE_POSN ((sizeof (ULONGEST) - 1) * HOST_CHAR_BIT)
-
-/* True (non-zero) iff DIGIT is a valid digit in radix BASE,
- where 2 <= BASE <= 36. */
-
-static int
-is_digit_in_base (unsigned char digit, int base)
-{
- if (!isalnum (digit))
- return 0;
- if (base <= 10)
- return (isdigit (digit) && digit < base + '0');
- else
- return (isdigit (digit) || tolower (digit) < base - 10 + 'a');
-}
-
-static int
-digit_to_int (unsigned char c)
-{
- if (isdigit (c))
- return c - '0';
- else
- return tolower (c) - 'a' + 10;
-}
-
-/* As for strtoul, but for ULONGEST results. */
-
-ULONGEST
-strtoulst (const char *num, const char **trailer, int base)
-{
- unsigned int high_part;
- ULONGEST result;
- int minus = 0;
- int i = 0;
-
- /* Skip leading whitespace. */
- while (isspace (num[i]))
- i++;
-
- /* Handle prefixes. */
- if (num[i] == '+')
- i++;
- else if (num[i] == '-')
- {
- minus = 1;
- i++;
- }
-
- if (base == 0 || base == 16)
- {
- if (num[i] == '0' && (num[i + 1] == 'x' || num[i + 1] == 'X'))
- {
- i += 2;
- if (base == 0)
- base = 16;
- }
- }
-
- if (base == 0 && num[i] == '0')
- base = 8;
-
- if (base == 0)
- base = 10;
-
- if (base < 2 || base > 36)
- {
- errno = EINVAL;
- return 0;
- }
-
- result = high_part = 0;
- for (; is_digit_in_base (num[i], base); i += 1)
- {
- result = result * base + digit_to_int (num[i]);
- high_part = high_part * base + (unsigned int) (result >> HIGH_BYTE_POSN);
- result &= ((ULONGEST) 1 << HIGH_BYTE_POSN) - 1;
- if (high_part > 0xff)
- {
- errno = ERANGE;
- result = ~ (ULONGEST) 0;
- high_part = 0;
- minus = 0;
- break;
- }
- }
-
- if (trailer != NULL)
- *trailer = &num[i];
-
- result = result + ((ULONGEST) high_part << HIGH_BYTE_POSN);
- if (minus)
- return -result;
- else
- return result;
-}
-
/* Simple, portable version of dirname that does not modify its
argument. */
diff --git a/gdb/utils.h b/gdb/utils.h
index d6df2ee..a949cd7 100644
--- a/gdb/utils.h
+++ b/gdb/utils.h
@@ -41,8 +41,6 @@ extern int streq (const char *, const char *);
extern int subset_compare (char *, char *);
-ULONGEST strtoulst (const char *num, const char **trailer, int base);
-
int compare_positive_ints (const void *ap, const void *bp);
int compare_strings (const void *ap, const void *bp);
^ permalink raw reply [flat|nested] 34+ messages in thread* Re: [PATCH v5 1/8] Move utility functions to common/
2014-03-19 22:30 ` [PATCH v5 1/8] Move utility functions to common/ Jan Kratochvil
@ 2014-03-24 20:00 ` Sergio Durigan Junior
2014-05-18 19:05 ` Jan Kratochvil
2014-05-19 18:21 ` Tom Tromey
1 sibling, 1 reply; 34+ messages in thread
From: Sergio Durigan Junior @ 2014-03-24 20:00 UTC (permalink / raw)
To: Jan Kratochvil; +Cc: gdb-patches, Aleksandar Ristovski
On Wednesday, March 19 2014, Jan Kratochvil wrote:
> Hi,
>
> some parts of the former patch have been reimplemented in the meantime by
> other patches so this is only a part of the former cleanup.
Can't this go in independently? I think they are nice cleanups.
Thanks,
--
Sergio
^ permalink raw reply [flat|nested] 34+ messages in thread
* Re: [PATCH v5 1/8] Move utility functions to common/
2014-03-24 20:00 ` Sergio Durigan Junior
@ 2014-05-18 19:05 ` Jan Kratochvil
2014-05-19 18:22 ` Tom Tromey
2014-05-19 22:46 ` Doug Evans
0 siblings, 2 replies; 34+ messages in thread
From: Jan Kratochvil @ 2014-05-18 19:05 UTC (permalink / raw)
To: Sergio Durigan Junior; +Cc: gdb-patches, Aleksandar Ristovski
On Mon, 24 Mar 2014 20:59:48 +0100, Sergio Durigan Junior wrote:
> On Wednesday, March 19 2014, Jan Kratochvil wrote:
> > some parts of the former patch have been reimplemented in the meantime by
> > other patches so this is only a part of the former cleanup.
>
> Can't this go in independently? I think they are nice cleanups.
I do not think it is right without the later parts of the patchset - code
in gdb/common/ should be used by both gdb and gdbserver. Otherwise the code
could remain in gdb/ only.
Jan
^ permalink raw reply [flat|nested] 34+ messages in thread
* Re: [PATCH v5 1/8] Move utility functions to common/
2014-05-18 19:05 ` Jan Kratochvil
@ 2014-05-19 18:22 ` Tom Tromey
2014-05-19 22:48 ` Doug Evans
2014-05-19 22:46 ` Doug Evans
1 sibling, 1 reply; 34+ messages in thread
From: Tom Tromey @ 2014-05-19 18:22 UTC (permalink / raw)
To: Jan Kratochvil; +Cc: Sergio Durigan Junior, gdb-patches, Aleksandar Ristovski
Sergio> Can't this go in independently? I think they are nice cleanups.
Jan> I do not think it is right without the later parts of the patchset
Jan> - code in gdb/common/ should be used by both gdb and gdbserver.
Jan> Otherwise the code could remain in gdb/ only.
If we had smaller files in common/ then we could put generically useful
things there and not care so much, relying on the linker to eliminate
bloat.
Tom
^ permalink raw reply [flat|nested] 34+ messages in thread
* Re: [PATCH v5 1/8] Move utility functions to common/
2014-05-19 18:22 ` Tom Tromey
@ 2014-05-19 22:48 ` Doug Evans
0 siblings, 0 replies; 34+ messages in thread
From: Doug Evans @ 2014-05-19 22:48 UTC (permalink / raw)
To: Tom Tromey
Cc: Jan Kratochvil, Sergio Durigan Junior, gdb-patches, Aleksandar Ristovski
On Mon, May 19, 2014 at 11:22 AM, Tom Tromey <tromey@redhat.com> wrote:
> Sergio> Can't this go in independently? I think they are nice cleanups.
>
> Jan> I do not think it is right without the later parts of the patchset
> Jan> - code in gdb/common/ should be used by both gdb and gdbserver.
> Jan> Otherwise the code could remain in gdb/ only.
>
> If we had smaller files in common/ then we could put generically useful
> things there and not care so much, relying on the linker to eliminate
> bloat.
How much bloat are we talking about?
Plus there's always -ffunction-sections -Wl,-gc-sections.
I've been meaning to try it, if only to see what more can be removed from gdb.
^ permalink raw reply [flat|nested] 34+ messages in thread
* Re: [PATCH v5 1/8] Move utility functions to common/
2014-05-18 19:05 ` Jan Kratochvil
2014-05-19 18:22 ` Tom Tromey
@ 2014-05-19 22:46 ` Doug Evans
1 sibling, 0 replies; 34+ messages in thread
From: Doug Evans @ 2014-05-19 22:46 UTC (permalink / raw)
To: Jan Kratochvil; +Cc: Sergio Durigan Junior, gdb-patches, Aleksandar Ristovski
On Sun, May 18, 2014 at 12:05 PM, Jan Kratochvil
<jan.kratochvil@redhat.com> wrote:
> On Mon, 24 Mar 2014 20:59:48 +0100, Sergio Durigan Junior wrote:
>> On Wednesday, March 19 2014, Jan Kratochvil wrote:
>> > some parts of the former patch have been reimplemented in the meantime by
>> > other patches so this is only a part of the former cleanup.
>>
>> Can't this go in independently? I think they are nice cleanups.
>
> I do not think it is right without the later parts of the patchset - code
> in gdb/common/ should be used by both gdb and gdbserver. Otherwise the code
> could remain in gdb/ only.
Hi.
I don't have a strong opinion on whether this particular patch can go
in right away, but I do have a comment.
I think we're building gdb wrong (where by "gdb" I mean
gdb+gdbserver+gdbreplay+...) if things can't go in common if they're
only used by gdb.
We should be building gdb out of useful building blocks, and it's just
easier to hack if said building blocks are already available, without
having to go through all the administrivia of moving them about or
what not.
If something like skip_spaces isn't used by gdbserver today, that
doesn't mean it won't be tomorrow.
IOW, I think of "common" as a collection of application-independent
routines, not dissimilar from libiberty, but for whatever reason
doesn't/can't live there instead. [btw, I'm not suggesting moving
routines from common to libiberty, but I'm guessing a quick glance
would find a few that should just as well live in libiberty - and be
usable by more people too]
Plus, I guess I have to ask, if down the road we find some routine in
common only used by a particular app, do we need to move it out of
common and into that app's directory?
My $0.02 anyways.
^ permalink raw reply [flat|nested] 34+ messages in thread
* Re: [PATCH v5 1/8] Move utility functions to common/
2014-03-19 22:30 ` [PATCH v5 1/8] Move utility functions to common/ Jan Kratochvil
2014-03-24 20:00 ` Sergio Durigan Junior
@ 2014-05-19 18:21 ` Tom Tromey
1 sibling, 0 replies; 34+ messages in thread
From: Tom Tromey @ 2014-05-19 18:21 UTC (permalink / raw)
To: Jan Kratochvil; +Cc: gdb-patches, Aleksandar Ristovski
>>>>> "Jan" == Jan Kratochvil <jan.kratochvil@redhat.com> writes:
Jan> some parts of the former patch have been reimplemented in the meantime by
Jan> other patches so this is only a part of the former cleanup.
Jan> 2014-02-26 Aleksandar Ristovski <aristovski@qnx.com
Jan> Move utility functions to common/.
Jan> * cli/cli-utils.c (skip_spaces, skip_spaces_const): Move defs to
Jan> common/common-utils.c.
Jan> * cli/cli-utils.h (skip_spaces, skip_spaces_const): Move decls to
Jan> common/common-utils.h.
Jan> * common/common-utils.c (host-defs.h, ctype.h): Include.
Jan> (HIGH_BYTE_POSN, is_digit_in_base, digit_to_int, strtoulst): Move
Jan> from utils.c.
Jan> (skip_spaces, skip_spaces_const): Move from cli/cli-utils.c.
Jan> * common/host-defs.h (TARGET_CHAR_BIT, HOST_CHAR_BIT): Moved from
Jan> defs.h.
Jan> * common/common-utils.h (strtoulst): Move decl from utils.h.
Jan> (hex2bin, bin2hex): Move decls from remote.h.
Jan> (skip_spaces, skip_spaces_const): Move decls from cli/cli-utils.h.
Jan> * defs.h (TARGET_CHAR_BIT, HOST_CHAR_BIT): Move to
Jan> common/common-utils.h
Jan> * utils.c (HIGH_BYTE_POSN, is_digit_in_base, digit_to_int)
Jan> (strtoulst): Move to common/common-utils.c.
Jan> * utils.h (strtoulst): Moved decl to common/common-utils.h.
This is ok.
Tom
^ permalink raw reply [flat|nested] 34+ messages in thread
* [PATCH v5 7/8] Validate symbol file using build-id
2014-03-19 23:12 [PATCH v5 0/8] Validate binary before use Jan Kratochvil
2014-03-19 22:30 ` [PATCH v5 1/8] Move utility functions to common/ Jan Kratochvil
@ 2014-03-19 22:31 ` Jan Kratochvil
2014-03-20 3:51 ` Eli Zaretskii
` (3 more replies)
2014-03-19 22:31 ` [PATCH v5 4/8] Prepare linux_find_memory_regions_full & co. for move Jan Kratochvil
` (5 subsequent siblings)
7 siblings, 4 replies; 34+ messages in thread
From: Jan Kratochvil @ 2014-03-19 22:31 UTC (permalink / raw)
To: gdb-patches; +Cc: Aleksandar Ristovski
Hi,
consumer part of the "build-id" attribute.
Jan
gdb/
2014-02-26 Aleksandar Ristovski <aristovski@qnx.com
Jan Kratochvil <jan.kratochvil@redhat.com>
Validate symbol file using build-id.
* NEWS (Changes since GDB 7.7): Add 'set solib-build-id-force'
and 'show solib-build-id-force'. Add build-id attribute.
* solib-darwin.c (_initialize_darwin_solib): Assign validate value.
* solib-dsbt.c (_initialize_dsbt_solib): Ditto.
* solib-frv.c (_initialize_frv_solib): Ditto.
* solib-ia64-hpux.c (ia64_hpux_target_so_ops): Ditto.
* solib-irix.c (_initialize_irix_solib): Ditto.
* solib-osf.c (_initialize_osf_solib): Ditto.
* solib-pa64.c (_initialize_pa64_solib): Ditto.
* solib-som.c (_initialize_som_solib): Ditto.
* solib-spu.c (set_spu_solib_ops): Ditto.
* solib-svr4.c: Include rsp-low.h.
(NOTE_GNU_BUILD_ID_NAME): New define.
(svr4_validate): New function.
(library_list_start_library): Parse 'build-id' attribute.
(svr4_library_attributes): Add 'build-id' attribute.
(_initialize_svr4_solib): Assign validate value.
* solib-target.c (solib.h): Include.
(_initialize_solib_target): Assign validate value.
* solib.c (solib_build_id_force, show_solib_build_id_force): New.
(solib_map_sections): Use ops->validate.
(clear_so): Free build_id.
(default_solib_validate): New function.
(_initialize_solib): Add "solib-build-id-force".
* solib.h (default_solib_validate): New declaration.
* solist.h (struct so_list): New fields 'build_idsz' and 'build_id'.
(target_so_ops): New field 'validate'.
gdb/doc/
2014-03-02 Jan Kratochvil <jan.kratochvil@redhat.com>
* gdb.texinfo (Files): Add 'set solib-build-id-force'
and 'show solib-build-id-force'.
---
gdb/NEWS | 10 ++++++
gdb/doc/gdb.texinfo | 38 ++++++++++++++++++++++
gdb/solib-darwin.c | 1 +
gdb/solib-dsbt.c | 1 +
gdb/solib-frv.c | 1 +
gdb/solib-ia64-hpux.c | 1 +
gdb/solib-irix.c | 1 +
gdb/solib-osf.c | 1 +
gdb/solib-pa64.c | 1 +
gdb/solib-som.c | 1 +
gdb/solib-spu.c | 1 +
gdb/solib-svr4.c | 85 +++++++++++++++++++++++++++++++++++++++++++++++++
gdb/solib-target.c | 2 +
gdb/solib.c | 62 +++++++++++++++++++++++++++++++++++-
gdb/solib.h | 4 ++
gdb/solist.h | 21 ++++++++++++
16 files changed, 230 insertions(+), 1 deletion(-)
diff --git a/gdb/NEWS b/gdb/NEWS
index 2a384ba..629cc13 100644
--- a/gdb/NEWS
+++ b/gdb/NEWS
@@ -43,6 +43,12 @@ maint ada show ignore-descriptive-types
the user manual for more details on descriptive types and the intended
usage of this option.
+set solib-build-id-force (on|off)
+show solib-build-id-force
+ Inferior shared library and symbol file may contain unique build-id.
+ If both build-ids are present but they do not match then this setting
+ enables (on) or disables (off) loading of such symbol file.
+
* New features in the GDB remote stub, GDBserver
** New option --debug-format=option1[,option2,...] allows one to add
@@ -51,6 +57,10 @@ maint ada show ignore-descriptive-types
Timestamps can also be turned on with the
"monitor set debug-format timestamps" command from GDB.
+ ** library-list-svr4 contains also optional attribute 'build-id' for
+ each library. GDB does not load library with build-id that
+ does not match such attribute.
+
* The 'record instruction-history' command now starts counting instructions
at one. This also affects the instruction ranges reported by the
'record function-call-history' command when given the /i modifier.
diff --git a/gdb/doc/gdb.texinfo b/gdb/doc/gdb.texinfo
index b1b29bd..a4af3ec 100644
--- a/gdb/doc/gdb.texinfo
+++ b/gdb/doc/gdb.texinfo
@@ -17114,6 +17114,44 @@ libraries that were loaded by explicit user requests are not
discarded.
@end table
+@table @code
+@kindex set solib-build-id-force
+@cindex override @value{GDBN} build-id check
+@item set solib-build-id-force @var{mode}
+Setting to override @value{GDBN} build-id check.
+
+Inferior shared libraries and symbol files may contain unique build-id.
+By default @value{GDBN} will ignore symbol files with non-matching build-id
+while printing:
+
+@smallexample
+ warning: Shared object "libfoo.so.1" could not be validated (remote
+ build ID 2bc1745e does not match local build ID a08f8767) and will be
+ ignored; or use 'set solib-build-id-force'.
+@end smallexample
+
+Turning on this setting would load such symbol file while still printing:
+
+@smallexample
+ warning: Shared object "libfoo.so.1" could not be validated (remote
+ build ID 2bc1745e does not match local build ID a08f8767) but it is
+ being loaded due to 'set solib-build-id-force'.
+@end smallexample
+
+If remote build-id is present but it does not match local build-id (or local
+build-id is not present) then this setting enables (@var{mode} is @code{on}) or
+disables (@var{mode} is @code{off}) loading of such symbol file. On systems
+where build-id is not present in the remote system this setting has no effect.
+The default value is @code{off}.
+
+Loading non-matching symbol file may confuse debugging including breakage
+of backtrace output.
+
+@kindex show solib-build-id-force
+@item show solib-build-id-force
+Display the current mode of build-id check override.
+@end table
+
Sometimes you may wish that @value{GDBN} stops and gives you control
when any of shared library events happen. The best way to do this is
to use @code{catch load} and @code{catch unload} (@pxref{Set
diff --git a/gdb/solib-darwin.c b/gdb/solib-darwin.c
index a9989ea..11de7cd 100644
--- a/gdb/solib-darwin.c
+++ b/gdb/solib-darwin.c
@@ -653,4 +653,5 @@ _initialize_darwin_solib (void)
darwin_so_ops.in_dynsym_resolve_code = darwin_in_dynsym_resolve_code;
darwin_so_ops.lookup_lib_global_symbol = darwin_lookup_lib_symbol;
darwin_so_ops.bfd_open = darwin_bfd_open;
+ darwin_so_ops.validate = default_solib_validate;
}
diff --git a/gdb/solib-dsbt.c b/gdb/solib-dsbt.c
index 0217a94..35223dd 100644
--- a/gdb/solib-dsbt.c
+++ b/gdb/solib-dsbt.c
@@ -1078,6 +1078,7 @@ _initialize_dsbt_solib (void)
dsbt_so_ops.open_symbol_file_object = open_symbol_file_object;
dsbt_so_ops.in_dynsym_resolve_code = dsbt_in_dynsym_resolve_code;
dsbt_so_ops.bfd_open = solib_bfd_open;
+ dsbt_so_ops.validate = default_solib_validate;
/* Debug this file's internals. */
add_setshow_zuinteger_cmd ("solib-dsbt", class_maintenance,
diff --git a/gdb/solib-frv.c b/gdb/solib-frv.c
index 9724a3c..18a4aba 100644
--- a/gdb/solib-frv.c
+++ b/gdb/solib-frv.c
@@ -1181,6 +1181,7 @@ _initialize_frv_solib (void)
frv_so_ops.open_symbol_file_object = open_symbol_file_object;
frv_so_ops.in_dynsym_resolve_code = frv_in_dynsym_resolve_code;
frv_so_ops.bfd_open = solib_bfd_open;
+ frv_so_ops.validate = default_solib_validate;
/* Debug this file's internals. */
add_setshow_zuinteger_cmd ("solib-frv", class_maintenance,
diff --git a/gdb/solib-ia64-hpux.c b/gdb/solib-ia64-hpux.c
index b53caa8..a39bb03 100644
--- a/gdb/solib-ia64-hpux.c
+++ b/gdb/solib-ia64-hpux.c
@@ -688,6 +688,7 @@ ia64_hpux_target_so_ops (void)
ops->open_symbol_file_object = ia64_hpux_open_symbol_file_object;
ops->in_dynsym_resolve_code = ia64_hpux_in_dynsym_resolve_code;
ops->bfd_open = solib_bfd_open;
+ ops->validate = default_solib_validate;
return ops;
}
diff --git a/gdb/solib-irix.c b/gdb/solib-irix.c
index 6266ee0..1f126a3 100644
--- a/gdb/solib-irix.c
+++ b/gdb/solib-irix.c
@@ -652,4 +652,5 @@ _initialize_irix_solib (void)
irix_so_ops.open_symbol_file_object = irix_open_symbol_file_object;
irix_so_ops.in_dynsym_resolve_code = irix_in_dynsym_resolve_code;
irix_so_ops.bfd_open = solib_bfd_open;
+ irix_so_ops.validate = default_solib_validate;
}
diff --git a/gdb/solib-osf.c b/gdb/solib-osf.c
index 90a26e8..5490f83 100644
--- a/gdb/solib-osf.c
+++ b/gdb/solib-osf.c
@@ -633,6 +633,7 @@ _initialize_osf_solib (void)
osf_so_ops.open_symbol_file_object = osf_open_symbol_file_object;
osf_so_ops.in_dynsym_resolve_code = osf_in_dynsym_resolve_code;
osf_so_ops.bfd_open = solib_bfd_open;
+ osf_so_ops.validate = default_solib_validate;
/* FIXME: Don't do this here. *_gdbarch_init() should set so_ops. */
current_target_so_ops = &osf_so_ops;
diff --git a/gdb/solib-pa64.c b/gdb/solib-pa64.c
index 099e1e7..da8a294 100644
--- a/gdb/solib-pa64.c
+++ b/gdb/solib-pa64.c
@@ -621,6 +621,7 @@ _initialize_pa64_solib (void)
pa64_so_ops.open_symbol_file_object = pa64_open_symbol_file_object;
pa64_so_ops.in_dynsym_resolve_code = pa64_in_dynsym_resolve_code;
pa64_so_ops.bfd_open = solib_bfd_open;
+ pa64_so_ops.validate = default_solib_validate;
memset (&dld_cache, 0, sizeof (dld_cache));
}
diff --git a/gdb/solib-som.c b/gdb/solib-som.c
index cba50d1..ed2cbfe 100644
--- a/gdb/solib-som.c
+++ b/gdb/solib-som.c
@@ -816,6 +816,7 @@ _initialize_som_solib (void)
som_so_ops.open_symbol_file_object = som_open_symbol_file_object;
som_so_ops.in_dynsym_resolve_code = som_in_dynsym_resolve_code;
som_so_ops.bfd_open = solib_bfd_open;
+ som_so_ops.validate = default_solib_validate;
}
void
diff --git a/gdb/solib-spu.c b/gdb/solib-spu.c
index 1c574b5..8385205 100644
--- a/gdb/solib-spu.c
+++ b/gdb/solib-spu.c
@@ -521,6 +521,7 @@ set_spu_solib_ops (struct gdbarch *gdbarch)
spu_so_ops.current_sos = spu_current_sos;
spu_so_ops.bfd_open = spu_bfd_open;
spu_so_ops.lookup_lib_global_symbol = spu_lookup_lib_symbol;
+ spu_so_ops.validate = default_solib_validate;
}
set_solib_ops (gdbarch, &spu_so_ops);
diff --git a/gdb/solib-svr4.c b/gdb/solib-svr4.c
index 0da5692..65d8746 100644
--- a/gdb/solib-svr4.c
+++ b/gdb/solib-svr4.c
@@ -47,6 +47,9 @@
#include "exceptions.h"
#include "gdb_bfd.h"
#include "probe.h"
+#include "rsp-low.h"
+
+#define NOTE_GNU_BUILD_ID_NAME ".note.gnu.build-id"
static struct link_map_offsets *svr4_fetch_link_map_offsets (void);
static int svr4_have_link_map_offsets (void);
@@ -959,6 +962,64 @@ svr4_keep_data_in_core (CORE_ADDR vaddr, unsigned long size)
return (name_lm >= vaddr && name_lm < vaddr + size);
}
+/* Validate SO by comparing build-id from the associated bfd and
+ corresponding build-id from target memory. */
+
+static char *
+svr4_validate (const struct so_list *const so)
+{
+ bfd_byte *local_id;
+ size_t local_idsz;
+
+ gdb_assert (so != NULL);
+
+ /* Target doesn't support reporting the build ID or the remote shared library
+ does not have build ID. */
+ if (so->build_id == NULL)
+ return NULL;
+
+ /* Build ID may be present in the local file, just GDB is unable to retrieve
+ it. As it has been reported by gdbserver it is not FSF gdbserver. */
+ if (so->abfd == NULL
+ || !bfd_check_format (so->abfd, bfd_object)
+ || bfd_get_flavour (so->abfd) != bfd_target_elf_flavour)
+ return NULL;
+
+ /* GDB has verified the local file really does not contain the build ID. */
+ if (elf_tdata (so->abfd)->build_id == NULL)
+ {
+ char *remote_hex;
+
+ remote_hex = alloca (so->build_idsz * 2 + 1);
+ bin2hex (so->build_id, remote_hex, so->build_idsz);
+
+ return xstrprintf (_("remote build ID is %s "
+ "but local file does not have build ID"),
+ remote_hex);
+ }
+
+ local_id = elf_tdata (so->abfd)->build_id->data;
+ local_idsz = elf_tdata (so->abfd)->build_id->size;
+
+ if (so->build_idsz != local_idsz
+ || memcmp (so->build_id, local_id, so->build_idsz) != 0)
+ {
+ char *remote_hex, *local_hex;
+
+ remote_hex = alloca (so->build_idsz * 2 + 1);
+ bin2hex (so->build_id, remote_hex, so->build_idsz);
+ local_hex = alloca (local_idsz * 2 + 1);
+ bin2hex (local_id, local_hex, local_idsz);
+
+ return xstrprintf (_("remote build ID %s "
+ "does not match local build ID %s"),
+ remote_hex, local_hex);
+ }
+
+ /* Both build IDs are present and they match. */
+ return NULL;
+}
+
/* Implement the "open_symbol_file_object" target_so_ops method.
If no open symbol file, attempt to locate and open the main symbol
@@ -1124,6 +1185,9 @@ library_list_start_library (struct gdb_xml_parser *parser,
ULONGEST *lmp = xml_find_attribute (attributes, "lm")->value;
ULONGEST *l_addrp = xml_find_attribute (attributes, "l_addr")->value;
ULONGEST *l_ldp = xml_find_attribute (attributes, "l_ld")->value;
+ const struct gdb_xml_value *const att_build_id
+ = xml_find_attribute (attributes, "build-id");
+ const char *const hex_build_id = att_build_id ? att_build_id->value : NULL;
struct so_list *new_elem;
new_elem = XCNEW (struct so_list);
@@ -1135,6 +1199,25 @@ library_list_start_library (struct gdb_xml_parser *parser,
strncpy (new_elem->so_name, name, sizeof (new_elem->so_name) - 1);
new_elem->so_name[sizeof (new_elem->so_name) - 1] = 0;
strcpy (new_elem->so_original_name, new_elem->so_name);
+ if (hex_build_id != NULL)
+ {
+ const size_t hex_build_id_len = strlen (hex_build_id);
+
+ if (hex_build_id_len > 0 && (hex_build_id_len & 1U) == 0)
+ {
+ const size_t build_idsz = hex_build_id_len / 2;
+
+ new_elem->build_id = xmalloc (build_idsz);
+ new_elem->build_idsz = hex2bin (hex_build_id, new_elem->build_id,
+ build_idsz);
+ if (new_elem->build_idsz != build_idsz)
+ {
+ xfree (new_elem->build_id);
+ new_elem->build_id = NULL;
+ new_elem->build_idsz = 0;
+ }
+ }
+ }
*list->tailp = new_elem;
list->tailp = &new_elem->next;
@@ -1169,6 +1252,7 @@ static const struct gdb_xml_attribute svr4_library_attributes[] =
{ "lm", GDB_XML_AF_NONE, gdb_xml_parse_attr_ulongest, NULL },
{ "l_addr", GDB_XML_AF_NONE, gdb_xml_parse_attr_ulongest, NULL },
{ "l_ld", GDB_XML_AF_NONE, gdb_xml_parse_attr_ulongest, NULL },
+ { "build-id", GDB_XML_AF_OPTIONAL, NULL, NULL },
{ NULL, GDB_XML_AF_NONE, NULL, NULL }
};
@@ -3166,4 +3250,5 @@ _initialize_svr4_solib (void)
svr4_so_ops.keep_data_in_core = svr4_keep_data_in_core;
svr4_so_ops.update_breakpoints = svr4_update_solib_event_breakpoints;
svr4_so_ops.handle_event = svr4_handle_solib_event;
+ svr4_so_ops.validate = svr4_validate;
}
diff --git a/gdb/solib-target.c b/gdb/solib-target.c
index bb34e4b..23b2d06 100644
--- a/gdb/solib-target.c
+++ b/gdb/solib-target.c
@@ -25,6 +25,7 @@
#include "target.h"
#include "vec.h"
#include "solib-target.h"
+#include "solib.h"
#include <string.h>
@@ -502,6 +503,7 @@ _initialize_solib_target (void)
solib_target_so_ops.in_dynsym_resolve_code
= solib_target_in_dynsym_resolve_code;
solib_target_so_ops.bfd_open = solib_bfd_open;
+ solib_target_so_ops.validate = default_solib_validate;
/* Set current_target_so_ops to solib_target_so_ops if not already
set. */
diff --git a/gdb/solib.c b/gdb/solib.c
index 3350bfd..83366ae 100644
--- a/gdb/solib.c
+++ b/gdb/solib.c
@@ -454,6 +454,20 @@ solib_bfd_open (char *pathname)
return abfd;
}
+/* Boolean for command 'set solib-build-id-force'. */
+static int solib_build_id_force = 0;
+
+/* Implement 'show solib-build-id-force'. */
+
+static void
+show_solib_build_id_force (struct ui_file *file, int from_tty,
+ struct cmd_list_element *c, const char *value)
+{
+ fprintf_filtered (file, _("Loading of shared libraries "
+ "with non-matching build-id is %s.\n"),
+ value);
+}
+
/* Given a pointer to one of the shared objects in our list of mapped
objects, use the recorded name to open a bfd descriptor for the
object, build a section table, relocate all the section addresses
@@ -470,7 +484,7 @@ static int
solib_map_sections (struct so_list *so)
{
const struct target_so_ops *ops = solib_ops (target_gdbarch ());
- char *filename;
+ char *filename, *validate_error;
struct target_section *p;
struct cleanup *old_chain;
bfd *abfd;
@@ -486,6 +500,27 @@ solib_map_sections (struct so_list *so)
/* Leave bfd open, core_xfer_memory and "info files" need it. */
so->abfd = abfd;
+ gdb_assert (ops->validate != NULL);
+
+ validate_error = ops->validate (so);
+ if (validate_error != NULL)
+ {
+ if (!solib_build_id_force)
+ {
+ warning (_("Shared object \"%s\" could not be validated (%s) and "
+ "will be ignored; or use 'set solib-build-id-force'."),
+ so->so_name, validate_error);
+ xfree (validate_error);
+ gdb_bfd_unref (so->abfd);
+ so->abfd = NULL;
+ return 0;
+ }
+ warning (_("Shared object \"%s\" could not be validated (%s) "
+ "but it is being loaded due to 'set solib-build-id-force'."),
+ so->so_name, validate_error);
+ xfree (validate_error);
+ }
+
/* Copy the full path name into so_name, allowing symbol_file_add
to find it later. This also affects the =library-loaded GDB/MI
event, and in particular the part of that notification providing
@@ -562,6 +597,9 @@ clear_so (struct so_list *so)
of the symbol file. */
strcpy (so->so_name, so->so_original_name);
+ xfree (so->build_id);
+ so->build_id = NULL;
+
/* Do the same for target-specific data. */
if (ops->clear_so != NULL)
ops->clear_so (so);
@@ -1523,6 +1561,14 @@ remove_user_added_objfile (struct objfile *objfile)
}
}
+/* Default implementation does not perform any validation. */
+
+char *
+default_solib_validate (const struct so_list *const so)
+{
+ return NULL; /* No validation. */
+}
+
extern initialize_file_ftype _initialize_solib; /* -Wmissing-prototypes */
void
@@ -1579,4 +1625,18 @@ PATH and LD_LIBRARY_PATH."),
reload_shared_libraries,
show_solib_search_path,
&setlist, &showlist);
+
+ add_setshow_boolean_cmd ("solib-build-id-force", class_support,
+ &solib_build_id_force, _("\
+Set loading of shared libraries with non-matching build-id."), _("\
+Show loading of shared libraries with non-matching build-id."), _("\
+Inferior shared library and symbol file may contain unique build-id.\n\
+If both build-ids are present but they do not match then this setting\n\
+enables (on) or disables (off) loading of such symbol file.\n\
+Loading non-matching symbol file may confuse debugging including breakage\n\
+of backtrace output."),
+ NULL,
+ show_solib_build_id_force,
+ &setlist, &showlist);
+
}
diff --git a/gdb/solib.h b/gdb/solib.h
index 29fe8f7..ab0dccd 100644
--- a/gdb/solib.h
+++ b/gdb/solib.h
@@ -98,4 +98,8 @@ extern void update_solib_breakpoints (void);
extern void handle_solib_event (void);
+/* Default validation always returns 1. */
+
+extern char *default_solib_validate (const struct so_list *so);
+
#endif /* SOLIB_H */
diff --git a/gdb/solist.h b/gdb/solist.h
index ac1b1a7..10b67ee 100644
--- a/gdb/solist.h
+++ b/gdb/solist.h
@@ -75,6 +75,22 @@ struct so_list
There may not be just one (e.g. if two segments are relocated
differently); but this is only used for "info sharedlibrary". */
CORE_ADDR addr_low, addr_high;
+
+ /* Build id in raw format, contains verbatim contents of
+ .note.gnu.build-id including note header. This is actual
+ BUILD_ID which comes either from the remote target via qXfer
+ packet or via reading target memory. Therefore, it may differ
+ from the build-id of the associated bfd. In a normal
+ scenario, this so would soon lose its abfd due to failed
+ validation.
+ Reading target memory should be done by following execution view
+ of the binary (following program headers in the case of ELF).
+ Computing address from the linking view (following ELF section
+ headers) may give incorrect build-id memory address despite the
+ symbols still match.
+ Such an example is a prelinked vs. unprelinked i386 ELF file. */
+ size_t build_idsz;
+ gdb_byte *build_id;
};
struct target_so_ops
@@ -168,6 +184,11 @@ struct target_so_ops
NULL, in which case no specific preprocessing is necessary
for this target. */
void (*handle_event) (void);
+
+ /* Return NULL if SO does match target SO it is supposed to
+ represent. Otherwise return string describing why it does not match.
+ Caller has to free the string. */
+ char *(*validate) (const struct so_list *so);
};
/* Free the memory associated with a (so_list *). */
^ permalink raw reply [flat|nested] 34+ messages in thread* Re: [PATCH v5 7/8] Validate symbol file using build-id
2014-03-19 22:31 ` [PATCH v5 7/8] Validate symbol file using build-id Jan Kratochvil
@ 2014-03-20 3:51 ` Eli Zaretskii
2014-03-20 13:07 ` Jan Kratochvil
2014-03-20 13:13 ` Aleksandar Ristovski
` (2 subsequent siblings)
3 siblings, 1 reply; 34+ messages in thread
From: Eli Zaretskii @ 2014-03-20 3:51 UTC (permalink / raw)
To: Jan Kratochvil; +Cc: gdb-patches, ARistovski
> From: Jan Kratochvil <jan.kratochvil@redhat.com>
> Cc: Aleksandar Ristovski <ARistovski@qnx.com>
> Date: Wed, 19 Mar 2014 23:31:23 +0100
>
> consumer part of the "build-id" attribute.
Thanks. I already approved the docs parts, right?
^ permalink raw reply [flat|nested] 34+ messages in thread* Re: [PATCH v5 7/8] Validate symbol file using build-id
2014-03-20 3:51 ` Eli Zaretskii
@ 2014-03-20 13:07 ` Jan Kratochvil
2014-03-20 17:28 ` Eli Zaretskii
0 siblings, 1 reply; 34+ messages in thread
From: Jan Kratochvil @ 2014-03-20 13:07 UTC (permalink / raw)
To: Eli Zaretskii; +Cc: gdb-patches, ARistovski
Hi Eli,
On Thu, 20 Mar 2014 04:51:40 +0100, Eli Zaretskii wrote:
> Thanks. I already approved the docs parts, right?
there remains unapproved the doc/gdb.texinfo part of this patch 7/8.
In this v5 series the text has been even changed due to the functionality
change:
* svr4_validate() considers missing local build-id as not-a-match
(*)
Unfinished discussion with you about this doc/gdb.texinfo part is:
https://sourceware.org/ml/gdb-patches/2014-03/msg00473.html
Message-ID: <20140319223258.GA5398@host2.jankratochvil.net>
Thanks,
Jan
(*) Originally I wanted to check it in mostly as Aleksandar wrote it with
various additions done possibly incrementally later.
But given how complicated is to settle down on the messages and
documentation of a part which was wrong (**) anyway I have decided to fix
it rather all already during this first check-in.
(**) wrong as not a regression but imperfect functionality.
I have found from practical using of non-upstreamed Fedora build-id
patches that it is never right to load non-build-id file for build-id
targer, in Fedora it is patch (not equivalent but similar functionality):
http://pkgs.fedoraproject.org/cgit/gdb.git/tree/gdb-6.6-buildid-locate-solib-missing-ids.patch
^ permalink raw reply [flat|nested] 34+ messages in thread* Re: [PATCH v5 7/8] Validate symbol file using build-id
2014-03-20 13:07 ` Jan Kratochvil
@ 2014-03-20 17:28 ` Eli Zaretskii
2014-03-21 16:58 ` Jan Kratochvil
0 siblings, 1 reply; 34+ messages in thread
From: Eli Zaretskii @ 2014-03-20 17:28 UTC (permalink / raw)
To: Jan Kratochvil; +Cc: gdb-patches, ARistovski
> Date: Thu, 20 Mar 2014 08:33:50 +0100
> From: Jan Kratochvil <jan.kratochvil@redhat.com>
> Cc: gdb-patches@sourceware.org, ARistovski@qnx.com
>
> Hi Eli,
>
> On Thu, 20 Mar 2014 04:51:40 +0100, Eli Zaretskii wrote:
> > Thanks. I already approved the docs parts, right?
>
> there remains unapproved the doc/gdb.texinfo part of this patch 7/8.
> In this v5 series the text has been even changed due to the functionality
> change:
> * svr4_validate() considers missing local build-id as not-a-match
> (*)
>
> Unfinished discussion with you about this doc/gdb.texinfo part is:
> https://sourceware.org/ml/gdb-patches/2014-03/msg00473.html
> Message-ID: <20140319223258.GA5398@host2.jankratochvil.net>
Yes, but in https://sourceware.org/ml/gdb-patches/2014-03/msg00219.html
I suggested an addition, which somehow didn't make it here. The only
change I see is in the wording of the warning message you quote.
^ permalink raw reply [flat|nested] 34+ messages in thread
* Re: [PATCH v5 7/8] Validate symbol file using build-id
2014-03-19 22:31 ` [PATCH v5 7/8] Validate symbol file using build-id Jan Kratochvil
2014-03-20 3:51 ` Eli Zaretskii
@ 2014-03-20 13:13 ` Aleksandar Ristovski
2014-03-21 16:58 ` Jan Kratochvil
2014-03-21 16:34 ` [PATCH v5 7/8] Validate symbol file using build-id [updated] Jan Kratochvil
2014-05-20 14:51 ` [PATCH v5 7/8] Validate symbol file using build-id Tom Tromey
3 siblings, 1 reply; 34+ messages in thread
From: Aleksandar Ristovski @ 2014-03-20 13:13 UTC (permalink / raw)
To: Jan Kratochvil, gdb-patches
On 14-03-19 06:31 PM, Jan Kratochvil wrote:
> Hi,
>
> consumer part of the "build-id" attribute.
>
>
> Jan
>
>
> gdb/
> 2014-02-26 Aleksandar Ristovski <aristovski@qnx.com
> Jan Kratochvil <jan.kratochvil@redhat.com>
>
> Validate symbol file using build-id.
> * NEWS (Changes since GDB 7.7): Add 'set solib-build-id-force'
> and 'show solib-build-id-force'. Add build-id attribute.
> * solib-darwin.c (_initialize_darwin_solib): Assign validate value.
> * solib-dsbt.c (_initialize_dsbt_solib): Ditto.
> * solib-frv.c (_initialize_frv_solib): Ditto.
> * solib-ia64-hpux.c (ia64_hpux_target_so_ops): Ditto.
> * solib-irix.c (_initialize_irix_solib): Ditto.
> * solib-osf.c (_initialize_osf_solib): Ditto.
> * solib-pa64.c (_initialize_pa64_solib): Ditto.
> * solib-som.c (_initialize_som_solib): Ditto.
> * solib-spu.c (set_spu_solib_ops): Ditto.
> * solib-svr4.c: Include rsp-low.h.
> (NOTE_GNU_BUILD_ID_NAME): New define.
> (svr4_validate): New function.
> (library_list_start_library): Parse 'build-id' attribute.
> (svr4_library_attributes): Add 'build-id' attribute.
> (_initialize_svr4_solib): Assign validate value.
> * solib-target.c (solib.h): Include.
> (_initialize_solib_target): Assign validate value.
> * solib.c (solib_build_id_force, show_solib_build_id_force): New.
> (solib_map_sections): Use ops->validate.
> (clear_so): Free build_id.
> (default_solib_validate): New function.
> (_initialize_solib): Add "solib-build-id-force".
> * solib.h (default_solib_validate): New declaration.
> * solist.h (struct so_list): New fields 'build_idsz' and 'build_id'.
> (target_so_ops): New field 'validate'.
>
> gdb/doc/
> 2014-03-02 Jan Kratochvil <jan.kratochvil@redhat.com>
>
> * gdb.texinfo (Files): Add 'set solib-build-id-force'
> and 'show solib-build-id-force'.
> ---
> gdb/NEWS | 10 ++++++
> gdb/doc/gdb.texinfo | 38 ++++++++++++++++++++++
> gdb/solib-darwin.c | 1 +
> gdb/solib-dsbt.c | 1 +
> gdb/solib-frv.c | 1 +
> gdb/solib-ia64-hpux.c | 1 +
> gdb/solib-irix.c | 1 +
> gdb/solib-osf.c | 1 +
> gdb/solib-pa64.c | 1 +
> gdb/solib-som.c | 1 +
> gdb/solib-spu.c | 1 +
> gdb/solib-svr4.c | 85 +++++++++++++++++++++++++++++++++++++++++++++++++
> gdb/solib-target.c | 2 +
> gdb/solib.c | 62 +++++++++++++++++++++++++++++++++++-
> gdb/solib.h | 4 ++
> gdb/solist.h | 21 ++++++++++++
> 16 files changed, 230 insertions(+), 1 deletion(-)
>
> diff --git a/gdb/NEWS b/gdb/NEWS
> index 2a384ba..629cc13 100644
> --- a/gdb/NEWS
> +++ b/gdb/NEWS
> @@ -43,6 +43,12 @@ maint ada show ignore-descriptive-types
> the user manual for more details on descriptive types and the intended
> usage of this option.
>
> +set solib-build-id-force (on|off)
> +show solib-build-id-force
> + Inferior shared library and symbol file may contain unique build-id.
> + If both build-ids are present but they do not match then this setting
> + enables (on) or disables (off) loading of such symbol file.
> +
> * New features in the GDB remote stub, GDBserver
>
> ** New option --debug-format=option1[,option2,...] allows one to add
> @@ -51,6 +57,10 @@ maint ada show ignore-descriptive-types
> Timestamps can also be turned on with the
> "monitor set debug-format timestamps" command from GDB.
>
> + ** library-list-svr4 contains also optional attribute 'build-id' for
> + each library. GDB does not load library with build-id that
> + does not match such attribute.
> +
> * The 'record instruction-history' command now starts counting instructions
> at one. This also affects the instruction ranges reported by the
> 'record function-call-history' command when given the /i modifier.
> diff --git a/gdb/doc/gdb.texinfo b/gdb/doc/gdb.texinfo
> index b1b29bd..a4af3ec 100644
> --- a/gdb/doc/gdb.texinfo
> +++ b/gdb/doc/gdb.texinfo
> @@ -17114,6 +17114,44 @@ libraries that were loaded by explicit user requests are not
> discarded.
> @end table
>
> +@table @code
> +@kindex set solib-build-id-force
> +@cindex override @value{GDBN} build-id check
> +@item set solib-build-id-force @var{mode}
> +Setting to override @value{GDBN} build-id check.
> +
> +Inferior shared libraries and symbol files may contain unique build-id.
> +By default @value{GDBN} will ignore symbol files with non-matching build-id
> +while printing:
> +
> +@smallexample
> + warning: Shared object "libfoo.so.1" could not be validated (remote
> + build ID 2bc1745e does not match local build ID a08f8767) and will be
I am not sure 'remote' adds any clarity here, on-contrary, I think it
will confuse. This could all be happening in local scenario as well. The
gist should be: inferior so build-id (i.e. build-id of the shared object
loaded in the inferior memory) does not match build-id of the so gdb
loaded to get symbols.
> + ignored; or use 'set solib-build-id-force'.
> +@end smallexample
> +
> +Turning on this setting would load such symbol file while still printing:
> +
> +@smallexample
> + warning: Shared object "libfoo.so.1" could not be validated (remote
> + build ID 2bc1745e does not match local build ID a08f8767) but it is
> + being loaded due to 'set solib-build-id-force'.
> +@end smallexample
The same here
> +
> +If remote build-id is present but it does not match local build-id (or local
> +build-id is not present) then this setting enables (@var{mode} is @code{on}) or
> +disables (@var{mode} is @code{off}) loading of such symbol file. On systems
> +where build-id is not present in the remote system this setting has no effect.
> +The default value is @code{off}.
and here. I think 'inferior' would be a lot more suitable reference than
"remote system".
---
Aleksandar Ristovski
QNX Software Systems
^ permalink raw reply [flat|nested] 34+ messages in thread* Re: [PATCH v5 7/8] Validate symbol file using build-id
2014-03-20 13:13 ` Aleksandar Ristovski
@ 2014-03-21 16:58 ` Jan Kratochvil
2014-03-21 17:08 ` Aleksandar Ristovski
0 siblings, 1 reply; 34+ messages in thread
From: Jan Kratochvil @ 2014-03-21 16:58 UTC (permalink / raw)
To: Aleksandar Ristovski; +Cc: gdb-patches
On Thu, 20 Mar 2014 14:13:06 +0100, Aleksandar Ristovski wrote:
> On 14-03-19 06:31 PM, Jan Kratochvil wrote:
> >+@smallexample
> >+ warning: Shared object "libfoo.so.1" could not be validated (remote
> >+ build ID 2bc1745e does not match local build ID a08f8767) and will be
>
> I am not sure 'remote' adds any clarity here, on-contrary, I think
> it will confuse. This could all be happening in local scenario as
> well. The gist should be: inferior so build-id (i.e. build-id of the
> shared object loaded in the inferior memory) does not match build-id
> of the so gdb loaded to get symbols.
Therefore:
s/remote/inferior/
s/local/symbol file/
------------------------------------------------------------------------------
@smallexample
warning: Shared object "libfoo.so.1" could not be validated (inferior
build ID 2bc1745e does not match symbol file build ID a08f8767) and
will be ignored; or use 'set solib-build-id-force'.
@end smallexample
Turning on this setting would load such symbol file while still printing:
@smallexample
warning: Shared object "libfoo.so.1" could not be validated (inferior
build ID 2bc1745e is not identical to symbol file build ID a08f8767)
but it is being loaded due to 'set solib-build-id-force'.
@end smallexample
------------------------------------------------------------------------------
> >+If remote build-id is present but it does not match local build-id (or local
> >+build-id is not present) then this setting enables (@var{mode} is @code{on}) or
> >+disables (@var{mode} is @code{off}) loading of such symbol file. On systems
> >+where build-id is not present in the remote system this setting has no effect.
> >+The default value is @code{off}.
>
> and here. I think 'inferior' would be a lot more suitable reference
> than "remote system".
Done.
Thanks,
Jan
^ permalink raw reply [flat|nested] 34+ messages in thread* Re: [PATCH v5 7/8] Validate symbol file using build-id
2014-03-21 16:58 ` Jan Kratochvil
@ 2014-03-21 17:08 ` Aleksandar Ristovski
0 siblings, 0 replies; 34+ messages in thread
From: Aleksandar Ristovski @ 2014-03-21 17:08 UTC (permalink / raw)
To: Jan Kratochvil; +Cc: gdb-patches
On 14-03-21 12:58 PM, Jan Kratochvil wrote:
> On Thu, 20 Mar 2014 14:13:06 +0100, Aleksandar Ristovski wrote:
>> On 14-03-19 06:31 PM, Jan Kratochvil wrote:
>>> +@smallexample
>>> + warning: Shared object "libfoo.so.1" could not be validated (remote
>>> + build ID 2bc1745e does not match local build ID a08f8767) and will be
>>
>> I am not sure 'remote' adds any clarity here, on-contrary, I think
>> it will confuse. This could all be happening in local scenario as
>> well. The gist should be: inferior so build-id (i.e. build-id of the
>> shared object loaded in the inferior memory) does not match build-id
>> of the so gdb loaded to get symbols.
>
> Therefore:
> s/remote/inferior/
> s/local/symbol file/
Yes, I think this is better, thank you.
----
Aleksandar Ristovski
QNX Software Systems
^ permalink raw reply [flat|nested] 34+ messages in thread
* Re: [PATCH v5 7/8] Validate symbol file using build-id [updated]
2014-03-19 22:31 ` [PATCH v5 7/8] Validate symbol file using build-id Jan Kratochvil
2014-03-20 3:51 ` Eli Zaretskii
2014-03-20 13:13 ` Aleksandar Ristovski
@ 2014-03-21 16:34 ` Jan Kratochvil
2014-03-21 16:52 ` Eli Zaretskii
2014-05-20 14:51 ` [PATCH v5 7/8] Validate symbol file using build-id Tom Tromey
3 siblings, 1 reply; 34+ messages in thread
From: Jan Kratochvil @ 2014-03-21 16:34 UTC (permalink / raw)
To: gdb-patches; +Cc: Eli Zaretskii, Aleksandar Ristovski
Hi all,
sending just this one patch part updated.
The ^# part is just a diff against the last v5 post.
Thanks,
Jan
#diff --git a/gdb/doc/gdb.texinfo b/gdb/doc/gdb.texinfo
#index c2683f4..bf1fada 100644
#--- a/gdb/doc/gdb.texinfo
#+++ b/gdb/doc/gdb.texinfo
#@@ -17121,28 +17121,30 @@ discarded.
# Setting to override @value{GDBN} build-id check.
#
# Inferior shared libraries and symbol files may contain unique build-id.
#-By default @value{GDBN} will ignore symbol files with non-matching build-id
#-while printing:
#+@value{GDBN} expects the build-ids of each shared library and its corresponding
#+symbol file to be identical. If they are not identical, then by default
#+@value{GDBN} will @value{GDBN} will ignore symbol files with non-matching
#+build-id while printing:
#
# @smallexample
#- warning: Shared object "libfoo.so.1" could not be validated (remote
#- build ID 2bc1745e does not match local build ID a08f8767) and will be
#- ignored; or use 'set solib-build-id-force'.
#+ warning: Shared object "libfoo.so.1" could not be validated (inferior
#+ build ID 2bc1745e does not match symbol file build ID a08f8767) and
#+ will be ignored; or use 'set solib-build-id-force'.
# @end smallexample
#
# Turning on this setting would load such symbol file while still printing:
#
# @smallexample
#- warning: Shared object "libfoo.so.1" could not be validated (remote
#- build ID 2bc1745e does not match local build ID a08f8767) but it is
#- being loaded due to 'set solib-build-id-force'.
#+ warning: Shared object "libfoo.so.1" could not be validated (inferior
#+ build ID 2bc1745e is not identical to symbol file build ID a08f8767)
#+ but it is being loaded due to 'set solib-build-id-force'.
# @end smallexample
#
#-If remote build-id is present but it does not match local build-id (or local
#-build-id is not present) then this setting enables (@var{mode} is @code{on}) or
#-disables (@var{mode} is @code{off}) loading of such symbol file. On systems
#-where build-id is not present in the remote system this setting has no effect.
#-The default value is @code{off}.
#+If inferior build-id is present but it does not match symbol file build-id
#+(or the symbol file build-id is not present) then this setting enables
#+(@var{mode} is @code{on}) or disables (@var{mode} is @code{off}) loading of
#+such symbol file. On systems where build-id is not present in the inferior
#+system this setting has no effect. The default value is @code{off}.
#
# Loading non-matching symbol file may confuse debugging including breakage
# of backtrace output.
#diff --git a/gdb/solib-svr4.c b/gdb/solib-svr4.c
#index 65d8746..7551d49 100644
#--- a/gdb/solib-svr4.c
#+++ b/gdb/solib-svr4.c
#@@ -993,8 +993,8 @@ svr4_validate (const struct so_list *const so)
# remote_hex = alloca (so->build_idsz * 2 + 1);
# bin2hex (so->build_id, remote_hex, so->build_idsz);
#
#- return xstrprintf (_("remote build ID is %s "
#- "but local file does not have build ID"),
#+ return xstrprintf (_("inferior build ID is %s "
#+ "but symbol file does not have build ID"),
# remote_hex);
# }
#
#@@ -1011,8 +1011,8 @@ svr4_validate (const struct so_list *const so)
# local_hex = alloca (local_idsz * 2 + 1);
# bin2hex (local_id, local_hex, local_idsz);
#
#- return xstrprintf (_("remote build ID %s "
#- "does not match local build ID %s"),
#+ return xstrprintf (_("inferior build ID %s "
#+ "is not identical to symbol file build ID %s"),
# remote_hex, local_hex);
# }
#
gdb/
2014-02-26 Aleksandar Ristovski <aristovski@qnx.com
Jan Kratochvil <jan.kratochvil@redhat.com>
Validate symbol file using build-id.
* NEWS (Changes since GDB 7.7): Add 'set solib-build-id-force'
and 'show solib-build-id-force'. Add build-id attribute.
* solib-darwin.c (_initialize_darwin_solib): Assign validate value.
* solib-dsbt.c (_initialize_dsbt_solib): Ditto.
* solib-frv.c (_initialize_frv_solib): Ditto.
* solib-ia64-hpux.c (ia64_hpux_target_so_ops): Ditto.
* solib-irix.c (_initialize_irix_solib): Ditto.
* solib-osf.c (_initialize_osf_solib): Ditto.
* solib-pa64.c (_initialize_pa64_solib): Ditto.
* solib-som.c (_initialize_som_solib): Ditto.
* solib-spu.c (set_spu_solib_ops): Ditto.
* solib-svr4.c: Include rsp-low.h.
(NOTE_GNU_BUILD_ID_NAME): New define.
(svr4_validate): New function.
(library_list_start_library): Parse 'build-id' attribute.
(svr4_library_attributes): Add 'build-id' attribute.
(_initialize_svr4_solib): Assign validate value.
* solib-target.c (solib.h): Include.
(_initialize_solib_target): Assign validate value.
* solib.c (solib_build_id_force, show_solib_build_id_force): New.
(solib_map_sections): Use ops->validate.
(clear_so): Free build_id.
(default_solib_validate): New function.
(_initialize_solib): Add "solib-build-id-force".
* solib.h (default_solib_validate): New declaration.
* solist.h (struct so_list): New fields 'build_idsz' and 'build_id'.
(target_so_ops): New field 'validate'.
gdb/doc/
2014-03-02 Jan Kratochvil <jan.kratochvil@redhat.com>
* gdb.texinfo (Files): Add 'set solib-build-id-force'
and 'show solib-build-id-force'.
diff --git a/gdb/NEWS b/gdb/NEWS
index 2a384ba..629cc13 100644
--- a/gdb/NEWS
+++ b/gdb/NEWS
@@ -43,6 +43,12 @@ maint ada show ignore-descriptive-types
the user manual for more details on descriptive types and the intended
usage of this option.
+set solib-build-id-force (on|off)
+show solib-build-id-force
+ Inferior shared library and symbol file may contain unique build-id.
+ If both build-ids are present but they do not match then this setting
+ enables (on) or disables (off) loading of such symbol file.
+
* New features in the GDB remote stub, GDBserver
** New option --debug-format=option1[,option2,...] allows one to add
@@ -51,6 +57,10 @@ maint ada show ignore-descriptive-types
Timestamps can also be turned on with the
"monitor set debug-format timestamps" command from GDB.
+ ** library-list-svr4 contains also optional attribute 'build-id' for
+ each library. GDB does not load library with build-id that
+ does not match such attribute.
+
* The 'record instruction-history' command now starts counting instructions
at one. This also affects the instruction ranges reported by the
'record function-call-history' command when given the /i modifier.
diff --git a/gdb/doc/gdb.texinfo b/gdb/doc/gdb.texinfo
index d73af9a..bf1fada 100644
--- a/gdb/doc/gdb.texinfo
+++ b/gdb/doc/gdb.texinfo
@@ -17114,6 +17114,46 @@ libraries that were loaded by explicit user requests are not
discarded.
@end table
+@table @code
+@kindex set solib-build-id-force
+@cindex override @value{GDBN} build-id check
+@item set solib-build-id-force @var{mode}
+Setting to override @value{GDBN} build-id check.
+
+Inferior shared libraries and symbol files may contain unique build-id.
+@value{GDBN} expects the build-ids of each shared library and its corresponding
+symbol file to be identical. If they are not identical, then by default
+@value{GDBN} will @value{GDBN} will ignore symbol files with non-matching
+build-id while printing:
+
+@smallexample
+ warning: Shared object "libfoo.so.1" could not be validated (inferior
+ build ID 2bc1745e does not match symbol file build ID a08f8767) and
+ will be ignored; or use 'set solib-build-id-force'.
+@end smallexample
+
+Turning on this setting would load such symbol file while still printing:
+
+@smallexample
+ warning: Shared object "libfoo.so.1" could not be validated (inferior
+ build ID 2bc1745e is not identical to symbol file build ID a08f8767)
+ but it is being loaded due to 'set solib-build-id-force'.
+@end smallexample
+
+If inferior build-id is present but it does not match symbol file build-id
+(or the symbol file build-id is not present) then this setting enables
+(@var{mode} is @code{on}) or disables (@var{mode} is @code{off}) loading of
+such symbol file. On systems where build-id is not present in the inferior
+system this setting has no effect. The default value is @code{off}.
+
+Loading non-matching symbol file may confuse debugging including breakage
+of backtrace output.
+
+@kindex show solib-build-id-force
+@item show solib-build-id-force
+Display the current mode of build-id check override.
+@end table
+
Sometimes you may wish that @value{GDBN} stops and gives you control
when any of shared library events happen. The best way to do this is
to use @code{catch load} and @code{catch unload} (@pxref{Set
diff --git a/gdb/solib-darwin.c b/gdb/solib-darwin.c
index a9989ea..11de7cd 100644
--- a/gdb/solib-darwin.c
+++ b/gdb/solib-darwin.c
@@ -653,4 +653,5 @@ _initialize_darwin_solib (void)
darwin_so_ops.in_dynsym_resolve_code = darwin_in_dynsym_resolve_code;
darwin_so_ops.lookup_lib_global_symbol = darwin_lookup_lib_symbol;
darwin_so_ops.bfd_open = darwin_bfd_open;
+ darwin_so_ops.validate = default_solib_validate;
}
diff --git a/gdb/solib-dsbt.c b/gdb/solib-dsbt.c
index 0217a94..35223dd 100644
--- a/gdb/solib-dsbt.c
+++ b/gdb/solib-dsbt.c
@@ -1078,6 +1078,7 @@ _initialize_dsbt_solib (void)
dsbt_so_ops.open_symbol_file_object = open_symbol_file_object;
dsbt_so_ops.in_dynsym_resolve_code = dsbt_in_dynsym_resolve_code;
dsbt_so_ops.bfd_open = solib_bfd_open;
+ dsbt_so_ops.validate = default_solib_validate;
/* Debug this file's internals. */
add_setshow_zuinteger_cmd ("solib-dsbt", class_maintenance,
diff --git a/gdb/solib-frv.c b/gdb/solib-frv.c
index 9724a3c..18a4aba 100644
--- a/gdb/solib-frv.c
+++ b/gdb/solib-frv.c
@@ -1181,6 +1181,7 @@ _initialize_frv_solib (void)
frv_so_ops.open_symbol_file_object = open_symbol_file_object;
frv_so_ops.in_dynsym_resolve_code = frv_in_dynsym_resolve_code;
frv_so_ops.bfd_open = solib_bfd_open;
+ frv_so_ops.validate = default_solib_validate;
/* Debug this file's internals. */
add_setshow_zuinteger_cmd ("solib-frv", class_maintenance,
diff --git a/gdb/solib-ia64-hpux.c b/gdb/solib-ia64-hpux.c
index b53caa8..a39bb03 100644
--- a/gdb/solib-ia64-hpux.c
+++ b/gdb/solib-ia64-hpux.c
@@ -688,6 +688,7 @@ ia64_hpux_target_so_ops (void)
ops->open_symbol_file_object = ia64_hpux_open_symbol_file_object;
ops->in_dynsym_resolve_code = ia64_hpux_in_dynsym_resolve_code;
ops->bfd_open = solib_bfd_open;
+ ops->validate = default_solib_validate;
return ops;
}
diff --git a/gdb/solib-irix.c b/gdb/solib-irix.c
index 6266ee0..1f126a3 100644
--- a/gdb/solib-irix.c
+++ b/gdb/solib-irix.c
@@ -652,4 +652,5 @@ _initialize_irix_solib (void)
irix_so_ops.open_symbol_file_object = irix_open_symbol_file_object;
irix_so_ops.in_dynsym_resolve_code = irix_in_dynsym_resolve_code;
irix_so_ops.bfd_open = solib_bfd_open;
+ irix_so_ops.validate = default_solib_validate;
}
diff --git a/gdb/solib-osf.c b/gdb/solib-osf.c
index 90a26e8..5490f83 100644
--- a/gdb/solib-osf.c
+++ b/gdb/solib-osf.c
@@ -633,6 +633,7 @@ _initialize_osf_solib (void)
osf_so_ops.open_symbol_file_object = osf_open_symbol_file_object;
osf_so_ops.in_dynsym_resolve_code = osf_in_dynsym_resolve_code;
osf_so_ops.bfd_open = solib_bfd_open;
+ osf_so_ops.validate = default_solib_validate;
/* FIXME: Don't do this here. *_gdbarch_init() should set so_ops. */
current_target_so_ops = &osf_so_ops;
diff --git a/gdb/solib-pa64.c b/gdb/solib-pa64.c
index 099e1e7..da8a294 100644
--- a/gdb/solib-pa64.c
+++ b/gdb/solib-pa64.c
@@ -621,6 +621,7 @@ _initialize_pa64_solib (void)
pa64_so_ops.open_symbol_file_object = pa64_open_symbol_file_object;
pa64_so_ops.in_dynsym_resolve_code = pa64_in_dynsym_resolve_code;
pa64_so_ops.bfd_open = solib_bfd_open;
+ pa64_so_ops.validate = default_solib_validate;
memset (&dld_cache, 0, sizeof (dld_cache));
}
diff --git a/gdb/solib-som.c b/gdb/solib-som.c
index cba50d1..ed2cbfe 100644
--- a/gdb/solib-som.c
+++ b/gdb/solib-som.c
@@ -816,6 +816,7 @@ _initialize_som_solib (void)
som_so_ops.open_symbol_file_object = som_open_symbol_file_object;
som_so_ops.in_dynsym_resolve_code = som_in_dynsym_resolve_code;
som_so_ops.bfd_open = solib_bfd_open;
+ som_so_ops.validate = default_solib_validate;
}
void
diff --git a/gdb/solib-spu.c b/gdb/solib-spu.c
index 1c574b5..8385205 100644
--- a/gdb/solib-spu.c
+++ b/gdb/solib-spu.c
@@ -521,6 +521,7 @@ set_spu_solib_ops (struct gdbarch *gdbarch)
spu_so_ops.current_sos = spu_current_sos;
spu_so_ops.bfd_open = spu_bfd_open;
spu_so_ops.lookup_lib_global_symbol = spu_lookup_lib_symbol;
+ spu_so_ops.validate = default_solib_validate;
}
set_solib_ops (gdbarch, &spu_so_ops);
diff --git a/gdb/solib-svr4.c b/gdb/solib-svr4.c
index 0da5692..7551d49 100644
--- a/gdb/solib-svr4.c
+++ b/gdb/solib-svr4.c
@@ -47,6 +47,9 @@
#include "exceptions.h"
#include "gdb_bfd.h"
#include "probe.h"
+#include "rsp-low.h"
+
+#define NOTE_GNU_BUILD_ID_NAME ".note.gnu.build-id"
static struct link_map_offsets *svr4_fetch_link_map_offsets (void);
static int svr4_have_link_map_offsets (void);
@@ -959,6 +962,64 @@ svr4_keep_data_in_core (CORE_ADDR vaddr, unsigned long size)
return (name_lm >= vaddr && name_lm < vaddr + size);
}
+/* Validate SO by comparing build-id from the associated bfd and
+ corresponding build-id from target memory. */
+
+static char *
+svr4_validate (const struct so_list *const so)
+{
+ bfd_byte *local_id;
+ size_t local_idsz;
+
+ gdb_assert (so != NULL);
+
+ /* Target doesn't support reporting the build ID or the remote shared library
+ does not have build ID. */
+ if (so->build_id == NULL)
+ return NULL;
+
+ /* Build ID may be present in the local file, just GDB is unable to retrieve
+ it. As it has been reported by gdbserver it is not FSF gdbserver. */
+ if (so->abfd == NULL
+ || !bfd_check_format (so->abfd, bfd_object)
+ || bfd_get_flavour (so->abfd) != bfd_target_elf_flavour)
+ return NULL;
+
+ /* GDB has verified the local file really does not contain the build ID. */
+ if (elf_tdata (so->abfd)->build_id == NULL)
+ {
+ char *remote_hex;
+
+ remote_hex = alloca (so->build_idsz * 2 + 1);
+ bin2hex (so->build_id, remote_hex, so->build_idsz);
+
+ return xstrprintf (_("inferior build ID is %s "
+ "but symbol file does not have build ID"),
+ remote_hex);
+ }
+
+ local_id = elf_tdata (so->abfd)->build_id->data;
+ local_idsz = elf_tdata (so->abfd)->build_id->size;
+
+ if (so->build_idsz != local_idsz
+ || memcmp (so->build_id, local_id, so->build_idsz) != 0)
+ {
+ char *remote_hex, *local_hex;
+
+ remote_hex = alloca (so->build_idsz * 2 + 1);
+ bin2hex (so->build_id, remote_hex, so->build_idsz);
+ local_hex = alloca (local_idsz * 2 + 1);
+ bin2hex (local_id, local_hex, local_idsz);
+
+ return xstrprintf (_("inferior build ID %s "
+ "is not identical to symbol file build ID %s"),
+ remote_hex, local_hex);
+ }
+
+ /* Both build IDs are present and they match. */
+ return NULL;
+}
+
/* Implement the "open_symbol_file_object" target_so_ops method.
If no open symbol file, attempt to locate and open the main symbol
@@ -1124,6 +1185,9 @@ library_list_start_library (struct gdb_xml_parser *parser,
ULONGEST *lmp = xml_find_attribute (attributes, "lm")->value;
ULONGEST *l_addrp = xml_find_attribute (attributes, "l_addr")->value;
ULONGEST *l_ldp = xml_find_attribute (attributes, "l_ld")->value;
+ const struct gdb_xml_value *const att_build_id
+ = xml_find_attribute (attributes, "build-id");
+ const char *const hex_build_id = att_build_id ? att_build_id->value : NULL;
struct so_list *new_elem;
new_elem = XCNEW (struct so_list);
@@ -1135,6 +1199,25 @@ library_list_start_library (struct gdb_xml_parser *parser,
strncpy (new_elem->so_name, name, sizeof (new_elem->so_name) - 1);
new_elem->so_name[sizeof (new_elem->so_name) - 1] = 0;
strcpy (new_elem->so_original_name, new_elem->so_name);
+ if (hex_build_id != NULL)
+ {
+ const size_t hex_build_id_len = strlen (hex_build_id);
+
+ if (hex_build_id_len > 0 && (hex_build_id_len & 1U) == 0)
+ {
+ const size_t build_idsz = hex_build_id_len / 2;
+
+ new_elem->build_id = xmalloc (build_idsz);
+ new_elem->build_idsz = hex2bin (hex_build_id, new_elem->build_id,
+ build_idsz);
+ if (new_elem->build_idsz != build_idsz)
+ {
+ xfree (new_elem->build_id);
+ new_elem->build_id = NULL;
+ new_elem->build_idsz = 0;
+ }
+ }
+ }
*list->tailp = new_elem;
list->tailp = &new_elem->next;
@@ -1169,6 +1252,7 @@ static const struct gdb_xml_attribute svr4_library_attributes[] =
{ "lm", GDB_XML_AF_NONE, gdb_xml_parse_attr_ulongest, NULL },
{ "l_addr", GDB_XML_AF_NONE, gdb_xml_parse_attr_ulongest, NULL },
{ "l_ld", GDB_XML_AF_NONE, gdb_xml_parse_attr_ulongest, NULL },
+ { "build-id", GDB_XML_AF_OPTIONAL, NULL, NULL },
{ NULL, GDB_XML_AF_NONE, NULL, NULL }
};
@@ -3166,4 +3250,5 @@ _initialize_svr4_solib (void)
svr4_so_ops.keep_data_in_core = svr4_keep_data_in_core;
svr4_so_ops.update_breakpoints = svr4_update_solib_event_breakpoints;
svr4_so_ops.handle_event = svr4_handle_solib_event;
+ svr4_so_ops.validate = svr4_validate;
}
diff --git a/gdb/solib-target.c b/gdb/solib-target.c
index bb34e4b..23b2d06 100644
--- a/gdb/solib-target.c
+++ b/gdb/solib-target.c
@@ -25,6 +25,7 @@
#include "target.h"
#include "vec.h"
#include "solib-target.h"
+#include "solib.h"
#include <string.h>
@@ -502,6 +503,7 @@ _initialize_solib_target (void)
solib_target_so_ops.in_dynsym_resolve_code
= solib_target_in_dynsym_resolve_code;
solib_target_so_ops.bfd_open = solib_bfd_open;
+ solib_target_so_ops.validate = default_solib_validate;
/* Set current_target_so_ops to solib_target_so_ops if not already
set. */
diff --git a/gdb/solib.c b/gdb/solib.c
index 3350bfd..83366ae 100644
--- a/gdb/solib.c
+++ b/gdb/solib.c
@@ -454,6 +454,20 @@ solib_bfd_open (char *pathname)
return abfd;
}
+/* Boolean for command 'set solib-build-id-force'. */
+static int solib_build_id_force = 0;
+
+/* Implement 'show solib-build-id-force'. */
+
+static void
+show_solib_build_id_force (struct ui_file *file, int from_tty,
+ struct cmd_list_element *c, const char *value)
+{
+ fprintf_filtered (file, _("Loading of shared libraries "
+ "with non-matching build-id is %s.\n"),
+ value);
+}
+
/* Given a pointer to one of the shared objects in our list of mapped
objects, use the recorded name to open a bfd descriptor for the
object, build a section table, relocate all the section addresses
@@ -470,7 +484,7 @@ static int
solib_map_sections (struct so_list *so)
{
const struct target_so_ops *ops = solib_ops (target_gdbarch ());
- char *filename;
+ char *filename, *validate_error;
struct target_section *p;
struct cleanup *old_chain;
bfd *abfd;
@@ -486,6 +500,27 @@ solib_map_sections (struct so_list *so)
/* Leave bfd open, core_xfer_memory and "info files" need it. */
so->abfd = abfd;
+ gdb_assert (ops->validate != NULL);
+
+ validate_error = ops->validate (so);
+ if (validate_error != NULL)
+ {
+ if (!solib_build_id_force)
+ {
+ warning (_("Shared object \"%s\" could not be validated (%s) and "
+ "will be ignored; or use 'set solib-build-id-force'."),
+ so->so_name, validate_error);
+ xfree (validate_error);
+ gdb_bfd_unref (so->abfd);
+ so->abfd = NULL;
+ return 0;
+ }
+ warning (_("Shared object \"%s\" could not be validated (%s) "
+ "but it is being loaded due to 'set solib-build-id-force'."),
+ so->so_name, validate_error);
+ xfree (validate_error);
+ }
+
/* Copy the full path name into so_name, allowing symbol_file_add
to find it later. This also affects the =library-loaded GDB/MI
event, and in particular the part of that notification providing
@@ -562,6 +597,9 @@ clear_so (struct so_list *so)
of the symbol file. */
strcpy (so->so_name, so->so_original_name);
+ xfree (so->build_id);
+ so->build_id = NULL;
+
/* Do the same for target-specific data. */
if (ops->clear_so != NULL)
ops->clear_so (so);
@@ -1523,6 +1561,14 @@ remove_user_added_objfile (struct objfile *objfile)
}
}
+/* Default implementation does not perform any validation. */
+
+char *
+default_solib_validate (const struct so_list *const so)
+{
+ return NULL; /* No validation. */
+}
+
extern initialize_file_ftype _initialize_solib; /* -Wmissing-prototypes */
void
@@ -1579,4 +1625,18 @@ PATH and LD_LIBRARY_PATH."),
reload_shared_libraries,
show_solib_search_path,
&setlist, &showlist);
+
+ add_setshow_boolean_cmd ("solib-build-id-force", class_support,
+ &solib_build_id_force, _("\
+Set loading of shared libraries with non-matching build-id."), _("\
+Show loading of shared libraries with non-matching build-id."), _("\
+Inferior shared library and symbol file may contain unique build-id.\n\
+If both build-ids are present but they do not match then this setting\n\
+enables (on) or disables (off) loading of such symbol file.\n\
+Loading non-matching symbol file may confuse debugging including breakage\n\
+of backtrace output."),
+ NULL,
+ show_solib_build_id_force,
+ &setlist, &showlist);
+
}
diff --git a/gdb/solib.h b/gdb/solib.h
index 29fe8f7..ab0dccd 100644
--- a/gdb/solib.h
+++ b/gdb/solib.h
@@ -98,4 +98,8 @@ extern void update_solib_breakpoints (void);
extern void handle_solib_event (void);
+/* Default validation always returns 1. */
+
+extern char *default_solib_validate (const struct so_list *so);
+
#endif /* SOLIB_H */
diff --git a/gdb/solist.h b/gdb/solist.h
index ac1b1a7..10b67ee 100644
--- a/gdb/solist.h
+++ b/gdb/solist.h
@@ -75,6 +75,22 @@ struct so_list
There may not be just one (e.g. if two segments are relocated
differently); but this is only used for "info sharedlibrary". */
CORE_ADDR addr_low, addr_high;
+
+ /* Build id in raw format, contains verbatim contents of
+ .note.gnu.build-id including note header. This is actual
+ BUILD_ID which comes either from the remote target via qXfer
+ packet or via reading target memory. Therefore, it may differ
+ from the build-id of the associated bfd. In a normal
+ scenario, this so would soon lose its abfd due to failed
+ validation.
+ Reading target memory should be done by following execution view
+ of the binary (following program headers in the case of ELF).
+ Computing address from the linking view (following ELF section
+ headers) may give incorrect build-id memory address despite the
+ symbols still match.
+ Such an example is a prelinked vs. unprelinked i386 ELF file. */
+ size_t build_idsz;
+ gdb_byte *build_id;
};
struct target_so_ops
@@ -168,6 +184,11 @@ struct target_so_ops
NULL, in which case no specific preprocessing is necessary
for this target. */
void (*handle_event) (void);
+
+ /* Return NULL if SO does match target SO it is supposed to
+ represent. Otherwise return string describing why it does not match.
+ Caller has to free the string. */
+ char *(*validate) (const struct so_list *so);
};
/* Free the memory associated with a (so_list *). */
^ permalink raw reply [flat|nested] 34+ messages in thread* Re: [PATCH v5 7/8] Validate symbol file using build-id
2014-03-19 22:31 ` [PATCH v5 7/8] Validate symbol file using build-id Jan Kratochvil
` (2 preceding siblings ...)
2014-03-21 16:34 ` [PATCH v5 7/8] Validate symbol file using build-id [updated] Jan Kratochvil
@ 2014-05-20 14:51 ` Tom Tromey
3 siblings, 0 replies; 34+ messages in thread
From: Tom Tromey @ 2014-05-20 14:51 UTC (permalink / raw)
To: Jan Kratochvil; +Cc: gdb-patches, Aleksandar Ristovski
>>>>> "Jan" == Jan Kratochvil <jan.kratochvil@redhat.com> writes:
Jan> consumer part of the "build-id" attribute.
This is ok.
Tom
^ permalink raw reply [flat|nested] 34+ messages in thread
* [PATCH v5 4/8] Prepare linux_find_memory_regions_full & co. for move
2014-03-19 23:12 [PATCH v5 0/8] Validate binary before use Jan Kratochvil
2014-03-19 22:30 ` [PATCH v5 1/8] Move utility functions to common/ Jan Kratochvil
2014-03-19 22:31 ` [PATCH v5 7/8] Validate symbol file using build-id Jan Kratochvil
@ 2014-03-19 22:31 ` Jan Kratochvil
2014-05-19 19:15 ` Tom Tromey
2014-03-19 22:31 ` [PATCH v5 5/8] Move linux_find_memory_regions_full & co Jan Kratochvil
` (4 subsequent siblings)
7 siblings, 1 reply; 34+ messages in thread
From: Jan Kratochvil @ 2014-03-19 22:31 UTC (permalink / raw)
To: gdb-patches; +Cc: Aleksandar Ristovski
Hi,
prepare code for move into gdb/common/.
Jan
gdb/
2014-02-26 Aleksandar Ristovski <aristovski@qnx.com
Prepare linux_find_memory_regions_full & co. for move.
* linux-tdep.c (linux_find_memory_region_ftype): Comment.
(linux_find_memory_regions_full): Change signature and prepare
for moving to linux-maps.
(linux_find_memory_regions_data): Rename field 'obfd' to 'data'.
(linux_find_memory_regions_thunk): New.
(linux_find_memory_regions_thunk): Use 'data' field instead of 'obfd'.
(linux_find_memory_regions_gdb): New.
(linux_find_memory_regions): Rename argument 'obfd' to 'func_data'.
(linux_make_mappings_corefile_notes): Use
linux_find_memory_regions_gdb.
* target.c (read_alloc_pread_ftype): New typedef.
(target_fileio_read_alloc_1_pread): New function.
(read_alloc): Refactor from target_fileio_read_alloc_1.
(read_stralloc_func_ftype): New typedef.
(target_fileio_read_alloc_1): New implementation. Use read_alloc.
(read_stralloc): Refactored from target_fileio_read_stralloc.
(target_fileio_read_stralloc): New implementation, use read_stralloc.
---
gdb/linux-tdep.c | 96 +++++++++++++++++++++++++++++++++++--------------
gdb/target.c | 106 +++++++++++++++++++++++++++++++++++++++---------------
2 files changed, 145 insertions(+), 57 deletions(-)
diff --git a/gdb/linux-tdep.c b/gdb/linux-tdep.c
index c10b8ee..b9aba2d 100644
--- a/gdb/linux-tdep.c
+++ b/gdb/linux-tdep.c
@@ -730,6 +730,10 @@ linux_core_info_proc (struct gdbarch *gdbarch, char *args,
error (_("unable to handle request"));
}
+/* Callback function for linux_find_memory_regions_full. If it returns
+ non-zero linux_find_memory_regions_full returns immediately with that
+ value. */
+
typedef int linux_find_memory_region_ftype (ULONGEST vaddr, ULONGEST size,
ULONGEST offset, ULONGEST inode,
int read, int write,
@@ -737,34 +741,41 @@ typedef int linux_find_memory_region_ftype (ULONGEST vaddr, ULONGEST size,
const char *filename,
void *data);
-/* List memory regions in the inferior for a corefile. */
+/* List memory regions in the inferior PID for a corefile. Call FUNC
+ with FUNC_DATA for each such region. Return immediately with the
+ value returned by FUNC if it is non-zero. *MEMORY_TO_FREE_PTR should
+ be registered to be freed automatically if called FUNC throws an
+ exception. MEMORY_TO_FREE_PTR can be also passed as NULL if it is
+ not used. Return -1 if error occurs, 0 if all memory regions have
+ been processed or return the value from FUNC if FUNC returns
+ non-zero. */
static int
-linux_find_memory_regions_full (struct gdbarch *gdbarch,
- linux_find_memory_region_ftype *func,
- void *obfd)
+linux_find_memory_regions_full (pid_t pid, linux_find_memory_region_ftype *func,
+ void *func_data, void **memory_to_free_ptr)
{
char mapsfilename[100];
char *data;
- /* We need to know the real target PID to access /proc. */
- if (current_inferior ()->fake_pid_p)
- return 1;
-
- xsnprintf (mapsfilename, sizeof mapsfilename,
- "/proc/%d/smaps", current_inferior ()->pid);
+ xsnprintf (mapsfilename, sizeof mapsfilename, "/proc/%d/smaps", (int) pid);
data = target_fileio_read_stralloc (mapsfilename);
if (data == NULL)
{
/* Older Linux kernels did not support /proc/PID/smaps. */
- xsnprintf (mapsfilename, sizeof mapsfilename,
- "/proc/%d/maps", current_inferior ()->pid);
+ xsnprintf (mapsfilename, sizeof mapsfilename, "/proc/%d/maps",
+ (int) pid);
data = target_fileio_read_stralloc (mapsfilename);
}
if (data)
{
- struct cleanup *cleanup = make_cleanup (xfree, data);
char *line;
+ int retval = 0;
+
+ if (memory_to_free_ptr != NULL)
+ {
+ gdb_assert (*memory_to_free_ptr == NULL);
+ *memory_to_free_ptr = data;
+ }
line = strtok (data, "\n");
while (line)
@@ -821,15 +832,22 @@ linux_find_memory_regions_full (struct gdbarch *gdbarch,
modified = 1;
/* Invoke the callback function to create the corefile segment. */
- func (addr, endaddr - addr, offset, inode,
- read, write, exec, modified, filename, obfd);
+ retval = func (addr, endaddr - addr, offset, inode,
+ read, write, exec, modified, filename, func_data);
+ if (retval != 0)
+ break;
}
- do_cleanups (cleanup);
- return 0;
+ if (memory_to_free_ptr != NULL)
+ {
+ gdb_assert (data == *memory_to_free_ptr);
+ *memory_to_free_ptr = NULL;
+ }
+ xfree (data);
+ return retval;
}
- return 1;
+ return -1;
}
/* A structure for passing information through
@@ -843,9 +861,11 @@ struct linux_find_memory_regions_data
/* The original datum. */
- void *obfd;
+ void *data;
};
+static linux_find_memory_region_ftype linux_find_memory_regions_thunk;
+
/* A callback for linux_find_memory_regions that converts between the
"full"-style callback and find_memory_region_ftype. */
@@ -857,7 +877,30 @@ linux_find_memory_regions_thunk (ULONGEST vaddr, ULONGEST size,
{
struct linux_find_memory_regions_data *data = arg;
- return data->func (vaddr, size, read, write, exec, modified, data->obfd);
+ return data->func (vaddr, size, read, write, exec, modified, data->data);
+}
+
+/* Wrapper of linux_find_memory_regions_full handling FAKE_PID_P in GDB. */
+
+static int
+linux_find_memory_regions_gdb (struct gdbarch *gdbarch,
+ linux_find_memory_region_ftype *func,
+ void *func_data)
+{
+ void *memory_to_free = NULL;
+ struct cleanup *cleanup;
+ int retval;
+
+ /* 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;
+
+ cleanup = make_cleanup (free_current_contents, &memory_to_free);
+ retval = linux_find_memory_regions_full (current_inferior ()->pid,
+ func, func_data, &memory_to_free);
+ do_cleanups (cleanup);
+ return retval;
}
/* A variant of linux_find_memory_regions_full that is suitable as the
@@ -865,16 +908,15 @@ linux_find_memory_regions_thunk (ULONGEST vaddr, ULONGEST size,
static int
linux_find_memory_regions (struct gdbarch *gdbarch,
- find_memory_region_ftype func, void *obfd)
+ find_memory_region_ftype func, void *func_data)
{
struct linux_find_memory_regions_data data;
data.func = func;
- data.obfd = obfd;
+ data.data = func_data;
- return linux_find_memory_regions_full (gdbarch,
- linux_find_memory_regions_thunk,
- &data);
+ return linux_find_memory_regions_gdb (gdbarch,
+ linux_find_memory_regions_thunk, &data);
}
/* Determine which signal stopped execution. */
@@ -1056,8 +1098,8 @@ linux_make_mappings_corefile_notes (struct gdbarch *gdbarch, bfd *obfd,
pack_long (buf, long_type, 1);
obstack_grow (&data_obstack, buf, TYPE_LENGTH (long_type));
- linux_find_memory_regions_full (gdbarch, linux_make_mappings_callback,
- &mapping_data);
+ linux_find_memory_regions_gdb (gdbarch, linux_make_mappings_callback,
+ &mapping_data);
if (mapping_data.file_count != 0)
{
diff --git a/gdb/target.c b/gdb/target.c
index 0d22297..06b6331 100644
--- a/gdb/target.c
+++ b/gdb/target.c
@@ -2952,6 +2952,22 @@ 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. */
+
+static int
+target_fileio_read_alloc_1_pread (int handle, gdb_byte *read_buf, int len,
+ ULONGEST offset, int *target_errno)
+{
+ QUIT;
+
+ return target_fileio_pread (handle, read_buf, len, offset, target_errno);
+}
+
/* Read target file FILENAME. Store the result in *BUF_P and
return the size of the transferred data. PADDING additional bytes are
available in *BUF_P. This is a helper function for
@@ -2959,48 +2975,46 @@ target_fileio_close_cleanup (void *opaque)
information. */
static LONGEST
-target_fileio_read_alloc_1 (const char *filename,
- gdb_byte **buf_p, int padding)
+read_alloc (gdb_byte **buf_p, int handle, read_alloc_pread_ftype *pread_func,
+ int padding, void **memory_to_free_ptr)
{
- struct cleanup *close_cleanup;
size_t buf_alloc, buf_pos;
gdb_byte *buf;
LONGEST n;
- int fd;
int target_errno;
- fd = target_fileio_open (filename, FILEIO_O_RDONLY, 0700, &target_errno);
- if (fd == -1)
- return -1;
-
- close_cleanup = make_cleanup (target_fileio_close_cleanup, &fd);
-
/* Start by reading up to 4K at a time. The target will throttle
this number down if necessary. */
buf_alloc = 4096;
buf = xmalloc (buf_alloc);
+ if (memory_to_free_ptr != NULL)
+ {
+ gdb_assert (*memory_to_free_ptr == NULL);
+ *memory_to_free_ptr = buf;
+ }
buf_pos = 0;
while (1)
{
- n = target_fileio_pread (fd, &buf[buf_pos],
- buf_alloc - buf_pos - padding, buf_pos,
- &target_errno);
- if (n < 0)
- {
- /* An error occurred. */
- do_cleanups (close_cleanup);
- xfree (buf);
- return -1;
- }
- else if (n == 0)
+ n = pread_func (handle, &buf[buf_pos], buf_alloc - buf_pos - padding,
+ buf_pos, &target_errno);
+ if (n <= 0)
{
- /* Read all there was. */
- do_cleanups (close_cleanup);
- if (buf_pos == 0)
+ if (n < 0 || (n == 0 && buf_pos == 0))
xfree (buf);
else
*buf_p = buf;
- return buf_pos;
+ if (memory_to_free_ptr != NULL)
+ *memory_to_free_ptr = NULL;
+ if (n < 0)
+ {
+ /* An error occurred. */
+ return -1;
+ }
+ else
+ {
+ /* Read all there was. */
+ return buf_pos;
+ }
}
buf_pos += n;
@@ -3010,12 +3024,39 @@ target_fileio_read_alloc_1 (const char *filename,
{
buf_alloc *= 2;
buf = xrealloc (buf, buf_alloc);
+ if (memory_to_free_ptr != NULL)
+ *memory_to_free_ptr = buf;
}
-
- QUIT;
}
}
+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;
+
+static LONGEST
+target_fileio_read_alloc_1 (const char *filename,
+ gdb_byte **buf_p, int padding)
+{
+ struct cleanup *close_cleanup;
+ int fd, target_errno;
+ void *memory_to_free = NULL;
+ LONGEST retval;
+
+ fd = target_fileio_open (filename, FILEIO_O_RDONLY, 0700, &target_errno);
+ if (fd == -1)
+ return -1;
+
+ close_cleanup = make_cleanup (target_fileio_close_cleanup, &fd);
+
+ make_cleanup (free_current_contents, &memory_to_free);
+ retval = read_alloc (buf_p, fd, target_fileio_read_alloc_1_pread, padding,
+ &memory_to_free);
+ do_cleanups (close_cleanup);
+ return retval;
+}
+
/* Read target file FILENAME. Store the result in *BUF_P and return
the size of the transferred data. See the declaration in "target.h"
function for more information about the return value. */
@@ -3032,14 +3073,14 @@ target_fileio_read_alloc (const char *filename, gdb_byte **buf_p)
are returned as allocated but empty strings. A warning is issued
if the result contains any embedded NUL bytes. */
-char *
-target_fileio_read_stralloc (const char *filename)
+static char *
+read_stralloc (const char *filename, read_stralloc_func_ftype *func)
{
gdb_byte *buffer;
char *bufstr;
LONGEST i, transferred;
- transferred = target_fileio_read_alloc_1 (filename, &buffer, 1);
+ transferred = func (filename, &buffer, 1);
bufstr = (char *) buffer;
if (transferred < 0)
@@ -3063,6 +3104,11 @@ target_fileio_read_stralloc (const char *filename)
return bufstr;
}
+char *
+target_fileio_read_stralloc (const char *filename)
+{
+ return read_stralloc (filename, target_fileio_read_alloc_1);
+}
static int
default_region_ok_for_hw_watchpoint (struct target_ops *self,
^ permalink raw reply [flat|nested] 34+ messages in thread* Re: [PATCH v5 4/8] Prepare linux_find_memory_regions_full & co. for move
2014-03-19 22:31 ` [PATCH v5 4/8] Prepare linux_find_memory_regions_full & co. for move Jan Kratochvil
@ 2014-05-19 19:15 ` Tom Tromey
0 siblings, 0 replies; 34+ messages in thread
From: Tom Tromey @ 2014-05-19 19:15 UTC (permalink / raw)
To: Jan Kratochvil; +Cc: gdb-patches, Aleksandar Ristovski
>>>>> "Jan" == Jan Kratochvil <jan.kratochvil@redhat.com> writes:
Jan> 2014-02-26 Aleksandar Ristovski <aristovski@qnx.com
Jan> Prepare linux_find_memory_regions_full & co. for move.
Jan> * linux-tdep.c (linux_find_memory_region_ftype): Comment.
Jan> (linux_find_memory_regions_full): Change signature and prepare
Jan> for moving to linux-maps.
Jan> (linux_find_memory_regions_data): Rename field 'obfd' to 'data'.
Jan> (linux_find_memory_regions_thunk): New.
Jan> (linux_find_memory_regions_thunk): Use 'data' field instead of 'obfd'.
Jan> (linux_find_memory_regions_gdb): New.
Jan> (linux_find_memory_regions): Rename argument 'obfd' to 'func_data'.
Jan> (linux_make_mappings_corefile_notes): Use
Jan> linux_find_memory_regions_gdb.
Jan> * target.c (read_alloc_pread_ftype): New typedef.
Jan> (target_fileio_read_alloc_1_pread): New function.
Jan> (read_alloc): Refactor from target_fileio_read_alloc_1.
Jan> (read_stralloc_func_ftype): New typedef.
Jan> (target_fileio_read_alloc_1): New implementation. Use read_alloc.
Jan> (read_stralloc): Refactored from target_fileio_read_stralloc.
Jan> (target_fileio_read_stralloc): New implementation, use read_stralloc.
Ok.
Tom
^ permalink raw reply [flat|nested] 34+ messages in thread
* [PATCH v5 5/8] Move linux_find_memory_regions_full & co.
2014-03-19 23:12 [PATCH v5 0/8] Validate binary before use Jan Kratochvil
` (2 preceding siblings ...)
2014-03-19 22:31 ` [PATCH v5 4/8] Prepare linux_find_memory_regions_full & co. for move Jan Kratochvil
@ 2014-03-19 22:31 ` Jan Kratochvil
2014-05-19 19:19 ` Tom Tromey
2014-03-19 22:31 ` [PATCH v5 6/8] gdbserver build-id attribute generator Jan Kratochvil
` (3 subsequent siblings)
7 siblings, 1 reply; 34+ messages in thread
From: Jan Kratochvil @ 2014-03-19 22:31 UTC (permalink / raw)
To: gdb-patches; +Cc: Aleksandar Ristovski
Hi,
this should be just a move with no changes.
Jan
gdb/
2014-02-26 Aleksandar Ristovski <aristovski@qnx.com
Move linux_find_memory_regions_full & co.
* common/target-utils.c (string.h, gdb_assert.h): Include.
(read_alloc, read_stralloc): Move definitions from target.c.
* common/target-utils.h (read_alloc_pread_ftype): New typedef.
(read_alloc): New declaration.
(read_stralloc_func_ftype): New typedef.
(read_stralloc): New declaration.
* common/linux-maps.c (fcntl.h, unistd.h, target.h, gdb_assert.h)
(ctype.h, string.h, target-utils.h): Include.
(read_mapping): Move from linux-tdep.c.
[GDBSERVER] (linux_find_memory_read_stralloc_1_pread): New function.
[GDBSERVER] (linux_find_memory_read_stralloc_1): New function.
(linux_find_memory_read_stralloc): New function.
(linux_find_memory_regions_full): Move from linux-tdep.c.
* common/linux-maps.h (read_mapping): New declaration.
(linux_find_memory_region_ftype): Moved typedef from linux-tdep.c.
(linux_find_memory_regions_full): New declaration.
* linux-tdep.c (linux-maps.h): Include.
(read_mapping): Moved to common/linux-maps.c.
(linux_find_memory_region_ftype): Moved typedef to common/linux-maps.h.
(linux_find_memory_regions_full): Moved definition to
common/linux-maps.c.
* target.c (target-utils.h): Include.
(read_alloc_pread_ftype): Moved typedef to common/target-utils.h.
(read_alloc, read_stralloc): Moved definitions to
common/target-utils.c.
---
gdb/common/linux-maps.c | 197 +++++++++++++++++++++++++++++++++++++++++++++
gdb/common/linux-maps.h | 25 ++++++
gdb/common/target-utils.c | 89 ++++++++++++++++++++
gdb/common/target-utils.h | 11 +++
gdb/linux-tdep.c | 159 ------------------------------------
gdb/target.c | 98 +---------------------
6 files changed, 326 insertions(+), 253 deletions(-)
diff --git a/gdb/common/linux-maps.c b/gdb/common/linux-maps.c
index bb3eac9..4d0b973 100644
--- a/gdb/common/linux-maps.c
+++ b/gdb/common/linux-maps.c
@@ -18,8 +18,205 @@
#ifdef GDBSERVER
#include "server.h"
+#include <fcntl.h>
+#include <unistd.h>
#else
#include "defs.h"
+#include "target.h"
#endif
#include "linux-maps.h"
+#include "gdb_assert.h"
+#include <ctype.h>
+#include <string.h>
+#include "target-utils.h"
+
+/* Service function for corefiles and info proc. */
+
+void
+read_mapping (const char *line,
+ ULONGEST *addr, ULONGEST *endaddr,
+ const char **permissions, size_t *permissions_len,
+ ULONGEST *offset,
+ const char **device, size_t *device_len,
+ ULONGEST *inode,
+ const char **filename)
+{
+ const char *p = line;
+
+ *addr = strtoulst (p, &p, 16);
+ if (*p == '-')
+ p++;
+ *endaddr = strtoulst (p, &p, 16);
+
+ p = skip_spaces_const (p);
+ *permissions = p;
+ while (*p && !isspace (*p))
+ p++;
+ *permissions_len = p - *permissions;
+
+ *offset = strtoulst (p, &p, 16);
+
+ p = skip_spaces_const (p);
+ *device = p;
+ while (*p && !isspace (*p))
+ p++;
+ *device_len = p - *device;
+
+ *inode = strtoulst (p, &p, 10);
+
+ p = skip_spaces_const (p);
+ *filename = p;
+}
+
+#ifdef GDBSERVER
+
+static int
+linux_find_memory_read_stralloc_1_pread (int handle, gdb_byte *read_buf,
+ int len, ULONGEST offset,
+ int *target_errno)
+{
+ int retval = pread (handle, read_buf, len, offset);
+
+ *target_errno = errno;
+ return retval;
+}
+
+static LONGEST
+linux_find_memory_read_stralloc_1 (const char *filename, gdb_byte **buf_p,
+ int padding)
+{
+ int fd;
+ LONGEST retval;
+
+ fd = open (filename, O_RDONLY);
+ if (fd == -1)
+ return -1;
+
+ retval = read_alloc (buf_p, fd, linux_find_memory_read_stralloc_1_pread,
+ padding, NULL);
+
+ close (fd);
+
+ return retval;
+}
+
+#endif /* GDBSERVER */
+
+static char *
+linux_find_memory_read_stralloc (const char *filename)
+{
+#ifndef GDBSERVER
+ return target_fileio_read_stralloc (filename);
+#else /* GDBSERVER */
+ return read_stralloc (filename, linux_find_memory_read_stralloc_1);
+#endif /* GDBSERVER */
+}
+
+/* List memory regions in the inferior PID for a corefile. Call FUNC
+ with FUNC_DATA for each such region. Return immediately with the
+ value returned by FUNC if it is non-zero. *MEMORY_TO_FREE_PTR should
+ be registered to be freed automatically if called FUNC throws an
+ exception. MEMORY_TO_FREE_PTR can be also passed as NULL if it is
+ not used. Return -1 if error occurs, 0 if all memory regions have
+ been processed or return the value from FUNC if FUNC returns
+ non-zero. */
+
+int
+linux_find_memory_regions_full (pid_t pid, linux_find_memory_region_ftype *func,
+ void *func_data, void **memory_to_free_ptr)
+{
+ char mapsfilename[100];
+ char *data;
+
+ xsnprintf (mapsfilename, sizeof mapsfilename, "/proc/%d/smaps", (int) pid);
+ data = linux_find_memory_read_stralloc (mapsfilename);
+ if (data == NULL)
+ {
+ /* Older Linux kernels did not support /proc/PID/smaps. */
+ xsnprintf (mapsfilename, sizeof mapsfilename, "/proc/%d/maps", (int) pid);
+ data = linux_find_memory_read_stralloc (mapsfilename);
+ }
+ if (data)
+ {
+ char *line;
+ int retval = 0;
+
+ if (memory_to_free_ptr != NULL)
+ {
+ gdb_assert (*memory_to_free_ptr == NULL);
+ *memory_to_free_ptr = data;
+ }
+
+ line = strtok (data, "\n");
+ while (line)
+ {
+ ULONGEST addr, endaddr, offset, inode;
+ const char *permissions, *device, *filename;
+ size_t permissions_len, device_len;
+ int read, write, exec;
+ int modified = 0, has_anonymous = 0;
+
+ read_mapping (line, &addr, &endaddr, &permissions, &permissions_len,
+ &offset, &device, &device_len, &inode, &filename);
+
+ /* Decode permissions. */
+ read = (memchr (permissions, 'r', permissions_len) != 0);
+ write = (memchr (permissions, 'w', permissions_len) != 0);
+ exec = (memchr (permissions, 'x', permissions_len) != 0);
+
+ /* Try to detect if region was modified by parsing smaps counters. */
+ for (line = strtok (NULL, "\n");
+ line && line[0] >= 'A' && line[0] <= 'Z';
+ line = strtok (NULL, "\n"))
+ {
+ char keyword[64 + 1];
+
+ if (sscanf (line, "%64s", keyword) != 1)
+ {
+ warning (_("Error parsing {s,}maps file '%s'"), mapsfilename);
+ break;
+ }
+ if (strcmp (keyword, "Anonymous:") == 0)
+ has_anonymous = 1;
+ if (strcmp (keyword, "Shared_Dirty:") == 0
+ || strcmp (keyword, "Private_Dirty:") == 0
+ || strcmp (keyword, "Swap:") == 0
+ || strcmp (keyword, "Anonymous:") == 0)
+ {
+ unsigned long number;
+
+ if (sscanf (line, "%*s%lu", &number) != 1)
+ {
+ warning (_("Error parsing {s,}maps file '%s' number"),
+ mapsfilename);
+ break;
+ }
+ if (number != 0)
+ modified = 1;
+ }
+ }
+
+ /* Older Linux kernels did not support the "Anonymous:" counter.
+ If it is missing, we can't be sure - dump all the pages. */
+ if (!has_anonymous)
+ modified = 1;
+
+ /* Invoke the callback function to create the corefile segment. */
+ retval = func (addr, endaddr - addr, offset, inode,
+ read, write, exec, modified, filename, func_data);
+ if (retval != 0)
+ break;
+ }
+
+ if (memory_to_free_ptr != NULL)
+ {
+ gdb_assert (data == *memory_to_free_ptr);
+ *memory_to_free_ptr = NULL;
+ }
+ xfree (data);
+ return retval;
+ }
+
+ return -1;
+}
diff --git a/gdb/common/linux-maps.h b/gdb/common/linux-maps.h
index ebf6f37..03b14d3 100644
--- a/gdb/common/linux-maps.h
+++ b/gdb/common/linux-maps.h
@@ -19,4 +19,29 @@
#ifndef COMMON_LINUX_MAPS_H
#define COMMON_LINUX_MAPS_H
+extern void
+ read_mapping (const char *line,
+ ULONGEST *addr, ULONGEST *endaddr,
+ const char **permissions, size_t *permissions_len,
+ ULONGEST *offset,
+ const char **device, size_t *device_len,
+ ULONGEST *inode,
+ const char **filename);
+
+/* Callback function for linux_find_memory_regions_full. If it returns
+ non-zero linux_find_memory_regions_full returns immediately with that
+ value. */
+
+typedef int linux_find_memory_region_ftype (ULONGEST vaddr, ULONGEST size,
+ ULONGEST offset, ULONGEST inode,
+ int read, int write,
+ int exec, int modified,
+ const char *filename,
+ void *data);
+
+extern int
+ linux_find_memory_regions_full (pid_t pid,
+ linux_find_memory_region_ftype *func,
+ void *func_data, void **memory_to_free_ptr);
+
#endif /* COMMON_LINUX_MAPS_H */
diff --git a/gdb/common/target-utils.c b/gdb/common/target-utils.c
index 308996d..84d1bca 100644
--- a/gdb/common/target-utils.c
+++ b/gdb/common/target-utils.c
@@ -23,4 +23,93 @@
#include "defs.h"
#endif
+#include <string.h>
#include "target-utils.h"
+#include "gdb_assert.h"
+
+LONGEST
+read_alloc (gdb_byte **buf_p, int handle, read_alloc_pread_ftype *pread_func,
+ int padding, void **memory_to_free_ptr)
+{
+ size_t buf_alloc, buf_pos;
+ gdb_byte *buf;
+ LONGEST n;
+ int target_errno;
+
+ /* Start by reading up to 4K at a time. The target will throttle
+ this number down if necessary. */
+ buf_alloc = 4096;
+ buf = xmalloc (buf_alloc);
+ if (memory_to_free_ptr != NULL)
+ {
+ gdb_assert (*memory_to_free_ptr == NULL);
+ *memory_to_free_ptr = buf;
+ }
+ buf_pos = 0;
+ while (1)
+ {
+ n = pread_func (handle, &buf[buf_pos], buf_alloc - buf_pos - padding,
+ buf_pos, &target_errno);
+ if (n <= 0)
+ {
+ if (n < 0 || (n == 0 && buf_pos == 0))
+ xfree (buf);
+ else
+ *buf_p = buf;
+ if (memory_to_free_ptr != NULL)
+ *memory_to_free_ptr = NULL;
+ if (n < 0)
+ {
+ /* An error occurred. */
+ return -1;
+ }
+ else
+ {
+ /* Read all there was. */
+ return buf_pos;
+ }
+ }
+
+ buf_pos += n;
+
+ /* If the buffer is filling up, expand it. */
+ if (buf_alloc < buf_pos * 2)
+ {
+ buf_alloc *= 2;
+ buf = xrealloc (buf, buf_alloc);
+ if (memory_to_free_ptr != NULL)
+ *memory_to_free_ptr = buf;
+ }
+ }
+}
+
+char *
+read_stralloc (const char *filename, read_stralloc_func_ftype *func)
+{
+ gdb_byte *buffer;
+ char *bufstr;
+ LONGEST i, transferred;
+
+ transferred = func (filename, &buffer, 1);
+ bufstr = (char *) buffer;
+
+ if (transferred < 0)
+ return NULL;
+
+ if (transferred == 0)
+ return xstrdup ("");
+
+ bufstr[transferred] = 0;
+
+ /* Check for embedded NUL bytes; but allow trailing NULs. */
+ for (i = strlen (bufstr); i < transferred; i++)
+ if (bufstr[i] != 0)
+ {
+ warning (_("target file %s "
+ "contained unexpected null characters"),
+ filename);
+ break;
+ }
+
+ return bufstr;
+}
diff --git a/gdb/common/target-utils.h b/gdb/common/target-utils.h
index db33ec8..f8ca972 100644
--- a/gdb/common/target-utils.h
+++ b/gdb/common/target-utils.h
@@ -20,4 +20,15 @@
#ifndef COMMON_TARGET_UTILS_H
#define COMMON_TARGET_UTILS_H
+typedef int (read_alloc_pread_ftype) (int handle, gdb_byte *read_buf, int len,
+ ULONGEST offset, int *target_errno);
+extern LONGEST read_alloc (gdb_byte **buf_p, int handle,
+ read_alloc_pread_ftype *pread_func, int padding,
+ void **memory_to_free_ptr);
+
+typedef LONGEST (read_stralloc_func_ftype) (const char *filename,
+ gdb_byte **buf_p, int padding);
+extern char *read_stralloc (const char *filename,
+ read_stralloc_func_ftype *func);
+
#endif /* COMMON_TARGET_UTILS_H */
diff --git a/gdb/linux-tdep.c b/gdb/linux-tdep.c
index b9aba2d..d0c8761 100644
--- a/gdb/linux-tdep.c
+++ b/gdb/linux-tdep.c
@@ -32,6 +32,7 @@
#include "cli/cli-utils.h"
#include "arch-utils.h"
#include "gdb_obstack.h"
+#include "linux-maps.h"
#include <ctype.h>
@@ -274,44 +275,6 @@ linux_core_pid_to_str (struct gdbarch *gdbarch, ptid_t ptid)
return normal_pid_to_str (ptid);
}
-/* Service function for corefiles and info proc. */
-
-static void
-read_mapping (const char *line,
- ULONGEST *addr, ULONGEST *endaddr,
- const char **permissions, size_t *permissions_len,
- ULONGEST *offset,
- const char **device, size_t *device_len,
- ULONGEST *inode,
- const char **filename)
-{
- const char *p = line;
-
- *addr = strtoulst (p, &p, 16);
- if (*p == '-')
- p++;
- *endaddr = strtoulst (p, &p, 16);
-
- p = skip_spaces_const (p);
- *permissions = p;
- while (*p && !isspace (*p))
- p++;
- *permissions_len = p - *permissions;
-
- *offset = strtoulst (p, &p, 16);
-
- p = skip_spaces_const (p);
- *device = p;
- while (*p && !isspace (*p))
- p++;
- *device_len = p - *device;
-
- *inode = strtoulst (p, &p, 10);
-
- p = skip_spaces_const (p);
- *filename = p;
-}
-
/* Implement the "info proc" command. */
static void
@@ -730,126 +693,6 @@ linux_core_info_proc (struct gdbarch *gdbarch, char *args,
error (_("unable to handle request"));
}
-/* Callback function for linux_find_memory_regions_full. If it returns
- non-zero linux_find_memory_regions_full returns immediately with that
- value. */
-
-typedef int linux_find_memory_region_ftype (ULONGEST vaddr, ULONGEST size,
- ULONGEST offset, ULONGEST inode,
- int read, int write,
- int exec, int modified,
- const char *filename,
- void *data);
-
-/* List memory regions in the inferior PID for a corefile. Call FUNC
- with FUNC_DATA for each such region. Return immediately with the
- value returned by FUNC if it is non-zero. *MEMORY_TO_FREE_PTR should
- be registered to be freed automatically if called FUNC throws an
- exception. MEMORY_TO_FREE_PTR can be also passed as NULL if it is
- not used. Return -1 if error occurs, 0 if all memory regions have
- been processed or return the value from FUNC if FUNC returns
- non-zero. */
-
-static int
-linux_find_memory_regions_full (pid_t pid, linux_find_memory_region_ftype *func,
- void *func_data, void **memory_to_free_ptr)
-{
- char mapsfilename[100];
- char *data;
-
- xsnprintf (mapsfilename, sizeof mapsfilename, "/proc/%d/smaps", (int) pid);
- data = target_fileio_read_stralloc (mapsfilename);
- if (data == NULL)
- {
- /* Older Linux kernels did not support /proc/PID/smaps. */
- xsnprintf (mapsfilename, sizeof mapsfilename, "/proc/%d/maps",
- (int) pid);
- data = target_fileio_read_stralloc (mapsfilename);
- }
- if (data)
- {
- char *line;
- int retval = 0;
-
- if (memory_to_free_ptr != NULL)
- {
- gdb_assert (*memory_to_free_ptr == NULL);
- *memory_to_free_ptr = data;
- }
-
- line = strtok (data, "\n");
- while (line)
- {
- ULONGEST addr, endaddr, offset, inode;
- const char *permissions, *device, *filename;
- size_t permissions_len, device_len;
- int read, write, exec;
- int modified = 0, has_anonymous = 0;
-
- read_mapping (line, &addr, &endaddr, &permissions, &permissions_len,
- &offset, &device, &device_len, &inode, &filename);
-
- /* Decode permissions. */
- read = (memchr (permissions, 'r', permissions_len) != 0);
- write = (memchr (permissions, 'w', permissions_len) != 0);
- exec = (memchr (permissions, 'x', permissions_len) != 0);
-
- /* Try to detect if region was modified by parsing smaps counters. */
- for (line = strtok (NULL, "\n");
- line && line[0] >= 'A' && line[0] <= 'Z';
- line = strtok (NULL, "\n"))
- {
- char keyword[64 + 1];
-
- if (sscanf (line, "%64s", keyword) != 1)
- {
- warning (_("Error parsing {s,}maps file '%s'"), mapsfilename);
- break;
- }
- if (strcmp (keyword, "Anonymous:") == 0)
- has_anonymous = 1;
- if (strcmp (keyword, "Shared_Dirty:") == 0
- || strcmp (keyword, "Private_Dirty:") == 0
- || strcmp (keyword, "Swap:") == 0
- || strcmp (keyword, "Anonymous:") == 0)
- {
- unsigned long number;
-
- if (sscanf (line, "%*s%lu", &number) != 1)
- {
- warning (_("Error parsing {s,}maps file '%s' number"),
- mapsfilename);
- break;
- }
- if (number != 0)
- modified = 1;
- }
- }
-
- /* Older Linux kernels did not support the "Anonymous:" counter.
- If it is missing, we can't be sure - dump all the pages. */
- if (!has_anonymous)
- modified = 1;
-
- /* Invoke the callback function to create the corefile segment. */
- retval = func (addr, endaddr - addr, offset, inode,
- read, write, exec, modified, filename, func_data);
- if (retval != 0)
- break;
- }
-
- if (memory_to_free_ptr != NULL)
- {
- gdb_assert (data == *memory_to_free_ptr);
- *memory_to_free_ptr = NULL;
- }
- xfree (data);
- return retval;
- }
-
- return -1;
-}
-
/* A structure for passing information through
linux_find_memory_regions_full. */
diff --git a/gdb/target.c b/gdb/target.c
index 06b6331..0e5ea33 100644
--- a/gdb/target.c
+++ b/gdb/target.c
@@ -45,6 +45,7 @@
#include "gdb/fileio.h"
#include "agent.h"
#include "auxv.h"
+#include "target-utils.h"
static void target_info (char *, int);
@@ -2952,9 +2953,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. */
@@ -2968,6 +2966,8 @@ 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);
}
+static read_stralloc_func_ftype target_fileio_read_alloc_1;
+
/* Read target file FILENAME. Store the result in *BUF_P and
return the size of the transferred data. PADDING additional bytes are
available in *BUF_P. This is a helper function for
@@ -2975,67 +2975,6 @@ target_fileio_read_alloc_1_pread (int handle, gdb_byte *read_buf, int len,
information. */
static LONGEST
-read_alloc (gdb_byte **buf_p, int handle, read_alloc_pread_ftype *pread_func,
- int padding, void **memory_to_free_ptr)
-{
- size_t buf_alloc, buf_pos;
- gdb_byte *buf;
- LONGEST n;
- int target_errno;
-
- /* Start by reading up to 4K at a time. The target will throttle
- this number down if necessary. */
- buf_alloc = 4096;
- buf = xmalloc (buf_alloc);
- if (memory_to_free_ptr != NULL)
- {
- gdb_assert (*memory_to_free_ptr == NULL);
- *memory_to_free_ptr = buf;
- }
- buf_pos = 0;
- while (1)
- {
- n = pread_func (handle, &buf[buf_pos], buf_alloc - buf_pos - padding,
- buf_pos, &target_errno);
- if (n <= 0)
- {
- if (n < 0 || (n == 0 && buf_pos == 0))
- xfree (buf);
- else
- *buf_p = buf;
- if (memory_to_free_ptr != NULL)
- *memory_to_free_ptr = NULL;
- if (n < 0)
- {
- /* An error occurred. */
- return -1;
- }
- else
- {
- /* Read all there was. */
- return buf_pos;
- }
- }
-
- buf_pos += n;
-
- /* If the buffer is filling up, expand it. */
- if (buf_alloc < buf_pos * 2)
- {
- buf_alloc *= 2;
- buf = xrealloc (buf, buf_alloc);
- if (memory_to_free_ptr != NULL)
- *memory_to_free_ptr = buf;
- }
- }
-}
-
-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;
-
-static LONGEST
target_fileio_read_alloc_1 (const char *filename,
gdb_byte **buf_p, int padding)
{
@@ -3073,37 +3012,6 @@ target_fileio_read_alloc (const char *filename, gdb_byte **buf_p)
are returned as allocated but empty strings. A warning is issued
if the result contains any embedded NUL bytes. */
-static char *
-read_stralloc (const char *filename, read_stralloc_func_ftype *func)
-{
- gdb_byte *buffer;
- char *bufstr;
- LONGEST i, transferred;
-
- transferred = func (filename, &buffer, 1);
- bufstr = (char *) buffer;
-
- if (transferred < 0)
- return NULL;
-
- if (transferred == 0)
- return xstrdup ("");
-
- bufstr[transferred] = 0;
-
- /* Check for embedded NUL bytes; but allow trailing NULs. */
- for (i = strlen (bufstr); i < transferred; i++)
- if (bufstr[i] != 0)
- {
- warning (_("target file %s "
- "contained unexpected null characters"),
- filename);
- break;
- }
-
- return bufstr;
-}
-
char *
target_fileio_read_stralloc (const char *filename)
{
^ permalink raw reply [flat|nested] 34+ messages in thread* Re: [PATCH v5 5/8] Move linux_find_memory_regions_full & co.
2014-03-19 22:31 ` [PATCH v5 5/8] Move linux_find_memory_regions_full & co Jan Kratochvil
@ 2014-05-19 19:19 ` Tom Tromey
0 siblings, 0 replies; 34+ messages in thread
From: Tom Tromey @ 2014-05-19 19:19 UTC (permalink / raw)
To: Jan Kratochvil; +Cc: gdb-patches, Aleksandar Ristovski
>>>>> "Jan" == Jan Kratochvil <jan.kratochvil@redhat.com> writes:
Jan> this should be just a move with no changes.
Jan> 2014-02-26 Aleksandar Ristovski <aristovski@qnx.com
Jan> Move linux_find_memory_regions_full & co.
Jan> * common/target-utils.c (string.h, gdb_assert.h): Include.
Jan> (read_alloc, read_stralloc): Move definitions from target.c.
Jan> * common/target-utils.h (read_alloc_pread_ftype): New typedef.
Jan> (read_alloc): New declaration.
Jan> (read_stralloc_func_ftype): New typedef.
Jan> (read_stralloc): New declaration.
Jan> * common/linux-maps.c (fcntl.h, unistd.h, target.h, gdb_assert.h)
Jan> (ctype.h, string.h, target-utils.h): Include.
Jan> (read_mapping): Move from linux-tdep.c.
Jan> [GDBSERVER] (linux_find_memory_read_stralloc_1_pread): New function.
Jan> [GDBSERVER] (linux_find_memory_read_stralloc_1): New function.
Jan> (linux_find_memory_read_stralloc): New function.
Jan> (linux_find_memory_regions_full): Move from linux-tdep.c.
Jan> * common/linux-maps.h (read_mapping): New declaration.
Jan> (linux_find_memory_region_ftype): Moved typedef from linux-tdep.c.
Jan> (linux_find_memory_regions_full): New declaration.
Jan> * linux-tdep.c (linux-maps.h): Include.
Jan> (read_mapping): Moved to common/linux-maps.c.
Jan> (linux_find_memory_region_ftype): Moved typedef to common/linux-maps.h.
Jan> (linux_find_memory_regions_full): Moved definition to
Jan> common/linux-maps.c.
Jan> * target.c (target-utils.h): Include.
Jan> (read_alloc_pread_ftype): Moved typedef to common/target-utils.h.
Jan> (read_alloc, read_stralloc): Moved definitions to
Jan> common/target-utils.c.
Ok.
I appreciate how the patch series was constructed to make this patch
obvious. Thanks.
Tom
^ permalink raw reply [flat|nested] 34+ messages in thread
* [PATCH v5 6/8] gdbserver build-id attribute generator
2014-03-19 23:12 [PATCH v5 0/8] Validate binary before use Jan Kratochvil
` (3 preceding siblings ...)
2014-03-19 22:31 ` [PATCH v5 5/8] Move linux_find_memory_regions_full & co Jan Kratochvil
@ 2014-03-19 22:31 ` Jan Kratochvil
2014-05-20 14:40 ` Tom Tromey
2014-03-19 22:31 ` [PATCH v5 2/8] Merge multiple hex conversions Jan Kratochvil
` (2 subsequent siblings)
7 siblings, 1 reply; 34+ messages in thread
From: Jan Kratochvil @ 2014-03-19 22:31 UTC (permalink / raw)
To: gdb-patches; +Cc: Aleksandar Ristovski
Hi,
producer part of the new "build-id" XML attribute.
Jan
gdb/
2014-02-26 Aleksandar Ristovski <aristovski@qnx.com
gdbserver build-id attribute generator.
* features/library-list-svr4.dtd (library-list-svr4): New
'build-id' attribute.
gdb/doc/
2014-02-26 Aleksandar Ristovski <aristovski@qnx.com
gdbserver build-id attribute generator.
* gdb.texinfo (Library List Format for SVR4 Targets): Add
'build-id' in description, example, new attribute in dtd.
gdb/gdbserver/
2014-02-26 Aleksandar Ristovski <aristovski@qnx.com
gdbserver build-id attribute generator.
* linux-low.c (linux-maps.h, search.h, rsp-low.h): Include.
(ElfXX_Ehdr, ElfXX_Phdr, ElfXX_Nhdr): New.
(ELFXX_FLD, ELFXX_SIZEOF, ELFXX_ROUNDUP, BUILD_ID_INVALID): New.
(find_phdr): New.
(get_dynamic): Use find_pdhr to traverse program headers.
(struct mapping_entry, mapping_entry_s, free_mapping_entry_vec)
(compare_mapping_entry_range, struct find_memory_region_callback_data)
(read_build_id, find_memory_region_callback, lrfind_mapping_entry)
(get_hex_build_id): New.
(linux_qxfer_libraries_svr4): Add optional build-id attribute
to reply XML document.
---
gdb/doc/gdb.texinfo | 17 +-
gdb/features/library-list-svr4.dtd | 13 +
gdb/gdbserver/linux-low.c | 389 +++++++++++++++++++++++++++++++++---
3 files changed, 370 insertions(+), 49 deletions(-)
diff --git a/gdb/doc/gdb.texinfo b/gdb/doc/gdb.texinfo
index de5ac63..b1b29bd 100644
--- a/gdb/doc/gdb.texinfo
+++ b/gdb/doc/gdb.texinfo
@@ -38207,6 +38207,8 @@ memory address. It is a displacement of absolute memory address against
address the file was prelinked to during the library load.
@item
@code{l_ld}, which is memory address of the @code{PT_DYNAMIC} segment
+@item
+@code{build-id}, hex encoded @code{NT_GNU_BUILD_ID} note, if it exists.
@end itemize
Additionally the single @code{main-lm} attribute specifies address of
@@ -38224,7 +38226,7 @@ looks like this:
<library name="/lib/ld-linux.so.2" lm="0xe4f51c" l_addr="0xe2d000"
l_ld="0xe4eefc"/>
<library name="/lib/libc.so.6" lm="0xe4fbe8" l_addr="0x154000"
- l_ld="0x152350"/>
+ l_ld="0x152350" build-id="9afccf7cc41e6293476223fe72480854"/>
</library-list-svr>
@end smallexample
@@ -38233,13 +38235,14 @@ The format of an SVR4 library list is described by this DTD:
@smallexample
<!-- library-list-svr4: Root element with versioning -->
<!ELEMENT library-list-svr4 (library)*>
-<!ATTLIST library-list-svr4 version CDATA #FIXED "1.0">
-<!ATTLIST library-list-svr4 main-lm CDATA #IMPLIED>
+<!ATTLIST library-list-svr4 version CDATA #FIXED "1.0">
+<!ATTLIST library-list-svr4 main-lm CDATA #IMPLIED>
<!ELEMENT library EMPTY>
-<!ATTLIST library name CDATA #REQUIRED>
-<!ATTLIST library lm CDATA #REQUIRED>
-<!ATTLIST library l_addr CDATA #REQUIRED>
-<!ATTLIST library l_ld CDATA #REQUIRED>
+<!ATTLIST library name CDATA #REQUIRED>
+<!ATTLIST library lm CDATA #REQUIRED>
+<!ATTLIST library l_addr CDATA #REQUIRED>
+<!ATTLIST library l_ld CDATA #REQUIRED>
+<!ATTLIST library build-id CDATA #IMPLIED>
@end smallexample
@node Memory Map Format
diff --git a/gdb/features/library-list-svr4.dtd b/gdb/features/library-list-svr4.dtd
index e0c0ebd..ff79990 100644
--- a/gdb/features/library-list-svr4.dtd
+++ b/gdb/features/library-list-svr4.dtd
@@ -6,11 +6,12 @@
<!-- library-list-svr4: Root element with versioning -->
<!ELEMENT library-list-svr4 (library)*>
-<!ATTLIST library-list-svr4 version CDATA #FIXED "1.0">
-<!ATTLIST library-list-svr4 main-lm CDATA #IMPLIED>
+<!ATTLIST library-list-svr4 version CDATA #FIXED "1.0">
+<!ATTLIST library-list-svr4 main-lm CDATA #IMPLIED>
<!ELEMENT library EMPTY>
-<!ATTLIST library name CDATA #REQUIRED>
-<!ATTLIST library lm CDATA #REQUIRED>
-<!ATTLIST library l_addr CDATA #REQUIRED>
-<!ATTLIST library l_ld CDATA #REQUIRED>
+<!ATTLIST library name CDATA #REQUIRED>
+<!ATTLIST library lm CDATA #REQUIRED>
+<!ATTLIST library l_addr CDATA #REQUIRED>
+<!ATTLIST library l_ld CDATA #REQUIRED>
+<!ATTLIST library build-id CDATA #IMPLIED>
diff --git a/gdb/gdbserver/linux-low.c b/gdb/gdbserver/linux-low.c
index 2d8d5f5..972b609 100644
--- a/gdb/gdbserver/linux-low.c
+++ b/gdb/gdbserver/linux-low.c
@@ -20,6 +20,7 @@
#include "linux-low.h"
#include "linux-osdata.h"
#include "agent.h"
+#include "linux-maps.h"
#include "nat/linux-nat.h"
#include "nat/linux-waitpid.h"
@@ -44,9 +45,11 @@
#include <sys/stat.h>
#include <sys/vfs.h>
#include <sys/uio.h>
+#include <search.h>
#include "filestuff.h"
#include "tracepoint.h"
#include "hostio.h"
+#include "rsp-low.h"
#ifndef ELFMAG0
/* Don't include <linux/elf.h> here. If it got included by gdb_proc_service.h
then ELFMAG0 will have been defined. If it didn't get included by
@@ -138,6 +141,31 @@ typedef struct
} Elf64_auxv_t;
#endif
+typedef union ElfXX_Ehdr
+{
+ Elf32_Ehdr _32;
+ Elf64_Ehdr _64;
+} ElfXX_Ehdr;
+
+typedef union ElfXX_Phdr
+{
+ Elf32_Phdr _32;
+ Elf64_Phdr _64;
+} ElfXX_Phdr;
+
+typedef union ElfXX_Nhdr
+{
+ Elf32_Nhdr _32;
+ Elf64_Nhdr _64;
+} ElfXX_Nhdr;
+
+#define ELFXX_FLD(elf64, hdr, fld) ((elf64) ? (hdr)._64.fld : (hdr)._32.fld)
+#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) (((what) + 3) & ~(ULONGEST) 3)
+#define BUILD_ID_INVALID "?"
+
/* A list of all unknown processes which receive stop signals. Some
other process will presumably claim each of these as forked
children momentarily. */
@@ -5482,15 +5510,38 @@ get_phdr_phnum_from_proc_auxv (const int pid, const int is_elf64,
return 0;
}
+/* Linearly traverse pheaders and look for P_TYPE pheader. */
+
+static const void *
+find_phdr (const int is_elf64, const void *const phdr_begin,
+ const void *const phdr_end, const ULONGEST p_type)
+{
+#define PHDR_NEXT(hdrp) ((const void *) ((const gdb_byte *) (hdrp) + \
+ ELFXX_SIZEOF (is_elf64, *hdrp)))
+
+ const ElfXX_Phdr *phdr = phdr_begin;
+
+ while (PHDR_NEXT (phdr) <= phdr_end)
+ {
+ if (ELFXX_FLD (is_elf64, *phdr, p_type) == p_type)
+ return phdr;
+ phdr = PHDR_NEXT (phdr);
+ }
+
+ return NULL;
+#undef PHDR_NEXT
+}
+
/* Return &_DYNAMIC (via PT_DYNAMIC) in the inferior, or 0 if not present. */
static CORE_ADDR
get_dynamic (const int pid, const int is_elf64)
{
CORE_ADDR phdr_memaddr, relocation;
- int num_phdr, i;
+ int num_phdr;
unsigned char *phdr_buf;
- const int phdr_size = is_elf64 ? sizeof (Elf64_Phdr) : sizeof (Elf32_Phdr);
+ const ElfXX_Phdr *phdr;
+ const int phdr_size = ELFXX_SIZEOF (is_elf64, *phdr);
if (get_phdr_phnum_from_proc_auxv (pid, is_elf64, &phdr_memaddr, &num_phdr))
return 0;
@@ -5504,22 +5555,10 @@ get_dynamic (const int pid, const int is_elf64)
/* Compute relocation: it is expected to be 0 for "regular" executables,
non-zero for PIE ones. */
relocation = -1;
- for (i = 0; relocation == -1 && i < num_phdr; i++)
- if (is_elf64)
- {
- Elf64_Phdr *const p = (Elf64_Phdr *) (phdr_buf + i * phdr_size);
-
- if (p->p_type == PT_PHDR)
- relocation = phdr_memaddr - p->p_vaddr;
- }
- else
- {
- Elf32_Phdr *const p = (Elf32_Phdr *) (phdr_buf + i * phdr_size);
-
- if (p->p_type == PT_PHDR)
- relocation = phdr_memaddr - p->p_vaddr;
- }
-
+ phdr = find_phdr (is_elf64, phdr_buf, phdr_buf + num_phdr * phdr_size,
+ PT_PHDR);
+ if (phdr != NULL)
+ relocation = phdr_memaddr - ELFXX_FLD (is_elf64, *phdr, p_vaddr);
if (relocation == -1)
{
/* PT_PHDR is optional, but necessary for PIE in general. Fortunately
@@ -5535,23 +5574,11 @@ get_dynamic (const int pid, const int is_elf64)
return 0;
}
- for (i = 0; i < num_phdr; i++)
- {
- if (is_elf64)
- {
- Elf64_Phdr *const p = (Elf64_Phdr *) (phdr_buf + i * phdr_size);
+ phdr = find_phdr (is_elf64, phdr_buf, phdr_buf + num_phdr * phdr_size,
+ PT_DYNAMIC);
- if (p->p_type == PT_DYNAMIC)
- return p->p_vaddr + relocation;
- }
- else
- {
- Elf32_Phdr *const p = (Elf32_Phdr *) (phdr_buf + i * phdr_size);
-
- if (p->p_type == PT_DYNAMIC)
- return p->p_vaddr + relocation;
- }
- }
+ if (phdr != NULL)
+ return ELFXX_FLD (is_elf64, *phdr, p_vaddr) + relocation;
return 0;
}
@@ -5691,6 +5718,278 @@ struct link_map_offsets
int l_prev_offset;
};
+
+/* Structure for holding a mapping. Only mapping
+ containing l_ld can have hex_build_id set. */
+
+struct mapping_entry
+{
+ /* Fields are populated from linux_find_memory_region parameters. */
+
+ ULONGEST vaddr;
+ ULONGEST size;
+ ULONGEST offset;
+ ULONGEST inode;
+
+ /* Hex encoded string allocated using xmalloc, and
+ needs to be freed. It can be NULL. */
+
+ char *hex_build_id;
+};
+
+typedef struct mapping_entry mapping_entry_s;
+
+DEF_VEC_O(mapping_entry_s);
+
+/* Free vector of mapping_entry_s objects. */
+
+static void
+free_mapping_entry_vec (VEC (mapping_entry_s) *lst)
+{
+ int ix;
+ mapping_entry_s *p;
+
+ for (ix = 0; VEC_iterate (mapping_entry_s, lst, ix, p); ++ix)
+ xfree (p->hex_build_id);
+
+ VEC_free (mapping_entry_s, lst);
+}
+
+/* Used for finding a mapping containing the given
+ l_ld passed in K. */
+
+static int
+compare_mapping_entry_range (const void *const k, const void *const b)
+{
+ const ULONGEST key = *(const CORE_ADDR *) k;
+ const mapping_entry_s *const p = b;
+
+ if (key < p->vaddr)
+ return -1;
+
+ if (key < p->vaddr + p->size)
+ return 0;
+
+ return 1;
+}
+
+struct find_memory_region_callback_data
+{
+ unsigned is_elf64;
+
+ /* Return. Must be freed with free_mapping_entry_vec. */
+ VEC (mapping_entry_s) *list;
+};
+
+/* Read build-id from PT_NOTE.
+ Argument LOAD_ADDR represents run time virtual address corresponding to
+ the beginning of the first loadable segment. L_ADDR is displacement
+ as supplied by the dynamic linker. */
+
+static void
+read_build_id (struct find_memory_region_callback_data *const p,
+ mapping_entry_s *const bil, const CORE_ADDR load_addr,
+ const CORE_ADDR l_addr)
+{
+ const int is_elf64 = p->is_elf64;
+ ElfXX_Ehdr ehdr;
+
+ if (linux_read_memory (load_addr, (unsigned char *) &ehdr,
+ ELFXX_SIZEOF (is_elf64, ehdr)) == 0
+ && ELFXX_FLD (is_elf64, ehdr, e_ident[EI_MAG0]) == ELFMAG0
+ && ELFXX_FLD (is_elf64, ehdr, e_ident[EI_MAG1]) == ELFMAG1
+ && ELFXX_FLD (is_elf64, ehdr, e_ident[EI_MAG2]) == ELFMAG2
+ && ELFXX_FLD (is_elf64, ehdr, e_ident[EI_MAG3]) == ELFMAG3)
+ {
+ const ElfXX_Phdr *phdr;
+ void *phdr_buf;
+ const unsigned e_phentsize = ELFXX_FLD (is_elf64, ehdr, e_phentsize);
+
+ if (ELFXX_FLD (is_elf64, ehdr, e_phnum) >= 100
+ || e_phentsize != ELFXX_SIZEOF (is_elf64, *phdr))
+ {
+ /* Basic sanity check failed. */
+ warning (_("Could not identify program header at %s."),
+ paddress (load_addr));
+ return;
+ }
+
+ phdr_buf = alloca (ELFXX_FLD (is_elf64, ehdr, e_phnum) * e_phentsize);
+
+ if (linux_read_memory (load_addr + ELFXX_FLD (is_elf64, ehdr, e_phoff),
+ phdr_buf,
+ ELFXX_FLD (is_elf64, ehdr, e_phnum) * e_phentsize)
+ != 0)
+ {
+ warning (_("Could not read program header at %s."),
+ paddress (load_addr));
+ return;
+ }
+
+ phdr = phdr_buf;
+
+ for (;;)
+ {
+ gdb_byte *pt_note;
+ const gdb_byte *pt_end;
+ const ElfXX_Nhdr *nhdr;
+ CORE_ADDR note_addr;
+
+ phdr = find_phdr (p->is_elf64, phdr, (gdb_byte *) phdr_buf
+ + ELFXX_FLD (is_elf64, ehdr, e_phnum) * e_phentsize,
+ PT_NOTE);
+ if (phdr == NULL)
+ break;
+ pt_note = xmalloc (ELFXX_FLD (is_elf64, *phdr, p_memsz));
+ note_addr = ELFXX_FLD (is_elf64, *phdr, p_vaddr) + l_addr;
+ if (linux_read_memory (note_addr, pt_note,
+ ELFXX_FLD (is_elf64, *phdr, p_memsz)) != 0)
+ {
+ xfree (pt_note);
+ 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 ((const gdb_byte *) nhdr < pt_end)
+ {
+ const size_t namesz
+ = ELFXX_ROUNDUP_4 (is_elf64, ELFXX_FLD (is_elf64, *nhdr,
+ n_namesz));
+ 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);
+
+ if (((const gdb_byte *) nhdr + note_sz) > pt_end || note_sz == 0
+ || descsz == 0)
+ {
+ 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
+ && ELFXX_FLD (is_elf64, *nhdr, n_namesz) == 4)
+ {
+ const char gnu[4] = "GNU\0";
+ const char *const pname
+ = (char *) nhdr + ELFXX_SIZEOF (is_elf64, *nhdr);
+
+ if (memcmp (pname, gnu, 4) == 0)
+ {
+ const size_t n_descsz = ELFXX_FLD (is_elf64, *nhdr,
+ n_descsz);
+
+ bil->hex_build_id = xmalloc (n_descsz * 2 + 1);
+ bin2hex ((const gdb_byte *) pname + namesz,
+ bil->hex_build_id, n_descsz);
+ xfree (pt_note);
+ return;
+ }
+ }
+ nhdr = (void *) ((gdb_byte *) nhdr + note_sz);
+ }
+ xfree (pt_note);
+ }
+ }
+}
+
+static linux_find_memory_region_ftype find_memory_region_callback;
+
+/* Add mapping_entry. See linux_find_memory_ftype for the parameters
+ description. */
+
+static int
+find_memory_region_callback (ULONGEST vaddr, ULONGEST size, ULONGEST offset,
+ ULONGEST inode, int read, int write, int exec,
+ int modified, const char *filename, void *data)
+{
+ if (inode != 0)
+ {
+ struct find_memory_region_callback_data *const p = data;
+ mapping_entry_s bil;
+
+ bil.vaddr = vaddr;
+ bil.size = size;
+ bil.offset = offset;
+ bil.inode = inode;
+ bil.hex_build_id = NULL;
+
+ VEC_safe_push (mapping_entry_s, p->list, &bil);
+ }
+
+ /* Continue the traversal. */
+ return 0;
+}
+
+/* Linear reverse find starting from RBEGIN towards REND looking for
+ the lowest vaddr mapping of the same inode and zero offset. */
+
+static mapping_entry_s *
+lrfind_mapping_entry (mapping_entry_s *const rbegin,
+ const mapping_entry_s *const rend)
+{
+ mapping_entry_s *p;
+
+ for (p = rbegin - 1; p >= rend; --p)
+ if (p->offset == 0 && p->inode == rbegin->inode)
+ return p;
+
+ return NULL;
+}
+
+/* Get build-id for the given L_LD, where L_LD corresponds to
+ link_map.l_ld as specified by the dynamic linker.
+ DATA must point to already filled list of mapping_entry elements.
+
+ If build-id had not been read, read it and cache in corresponding
+ list element.
+
+ Return build_id as stored in the list element corresponding
+ to L_LD.
+
+ NULL may be returned if build-id could not be fetched.
+
+ Returned string must not be freed explicitly. */
+
+static const char *
+get_hex_build_id (const CORE_ADDR l_addr, const CORE_ADDR l_ld,
+ struct find_memory_region_callback_data *const data)
+{
+ mapping_entry_s *bil;
+
+ bil = bsearch (&l_ld, VEC_address (mapping_entry_s, data->list),
+ VEC_length (mapping_entry_s, data->list),
+ sizeof (mapping_entry_s), compare_mapping_entry_range);
+
+ if (bil == NULL)
+ return NULL;
+
+ if (bil->hex_build_id == NULL)
+ {
+ mapping_entry_s *bil_min;
+
+ bil_min = lrfind_mapping_entry (bil, VEC_address (mapping_entry_s,
+ data->list));
+ if (bil_min != NULL)
+ read_build_id (data, bil, bil_min->vaddr, l_addr);
+ else
+ {
+ /* 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));
+ }
+ }
+
+ return bil->hex_build_id;
+}
+
/* Construct qXfer:libraries-svr4:read reply. */
static int
@@ -5703,6 +6002,7 @@ linux_qxfer_libraries_svr4 (const char *annex, unsigned char *readbuf,
struct process_info_private *const priv = current_process ()->private;
char filename[PATH_MAX];
int pid, is_elf64;
+ struct find_memory_region_callback_data data;
static const struct link_map_offsets lmo_32bit_offsets =
{
@@ -5745,6 +6045,13 @@ linux_qxfer_libraries_svr4 (const char *annex, unsigned char *readbuf,
lmo = is_elf64 ? &lmo_64bit_offsets : &lmo_32bit_offsets;
ptr_size = is_elf64 ? 8 : 4;
+ data.is_elf64 = is_elf64;
+ data.list = NULL;
+ 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"));
+
while (annex[0] != '\0')
{
const char *sep;
@@ -5851,6 +6158,7 @@ linux_qxfer_libraries_svr4 (const char *annex, unsigned char *readbuf,
/* 6x the size for xml_escape_text below. */
size_t len = 6 * strlen ((char *) libname);
char *name;
+ const char *hex_enc_build_id = NULL;
if (!header_done)
{
@@ -5859,7 +6167,11 @@ linux_qxfer_libraries_svr4 (const char *annex, unsigned char *readbuf,
header_done = 1;
}
- while (allocated < p - document + len + 200)
+ hex_enc_build_id = get_hex_build_id (l_addr, l_ld, &data);
+
+ while (allocated < (p - document + len + 200
+ + (hex_enc_build_id != NULL
+ ? strlen (hex_enc_build_id) : 0)))
{
/* Expand to guarantee sufficient storage. */
uintptr_t document_len = p - document;
@@ -5871,9 +6183,13 @@ linux_qxfer_libraries_svr4 (const char *annex, unsigned char *readbuf,
name = xml_escape_text ((char *) libname);
p += sprintf (p, "<library name=\"%s\" lm=\"0x%lx\" "
- "l_addr=\"0x%lx\" l_ld=\"0x%lx\"/>",
+ "l_addr=\"0x%lx\" l_ld=\"0x%lx\"",
name, (unsigned long) lm_addr,
(unsigned long) l_addr, (unsigned long) l_ld);
+ if (hex_enc_build_id != NULL
+ && strcmp (hex_enc_build_id, BUILD_ID_INVALID) != 0)
+ p += sprintf (p, " build-id=\"%s\"", hex_enc_build_id);
+ p += sprintf (p, "/>");
free (name);
}
}
@@ -5900,6 +6216,7 @@ linux_qxfer_libraries_svr4 (const char *annex, unsigned char *readbuf,
memcpy (readbuf, document + offset, len);
xfree (document);
+ free_mapping_entry_vec (data.list);
return len;
}
^ permalink raw reply [flat|nested] 34+ messages in thread* Re: [PATCH v5 6/8] gdbserver build-id attribute generator
2014-03-19 22:31 ` [PATCH v5 6/8] gdbserver build-id attribute generator Jan Kratochvil
@ 2014-05-20 14:40 ` Tom Tromey
2014-05-21 12:24 ` Jan Kratochvil
0 siblings, 1 reply; 34+ messages in thread
From: Tom Tromey @ 2014-05-20 14:40 UTC (permalink / raw)
To: Jan Kratochvil; +Cc: gdb-patches, Aleksandar Ristovski
>>>>> "Jan" == Jan Kratochvil <jan.kratochvil@redhat.com> writes:
Jan> producer part of the new "build-id" XML attribute.
I can't really claim to understand all of this patch.
What I do understand looks reasonable, and I'm prepared to approve it
given its history, etc.
However:
Jan> -<!ATTLIST library-list-svr4 version CDATA #FIXED "1.0">
Jan> -<!ATTLIST library-list-svr4 main-lm CDATA #IMPLIED>
Jan> +<!ATTLIST library-list-svr4 version CDATA #FIXED "1.0">
Jan> +<!ATTLIST library-list-svr4 main-lm CDATA #IMPLIED>
I am concerned about versioning of the XML.
Is it correct to keep the same version number?
If so -- why?
Perhaps we consider it compatible to add attributes.
I really don't know; nor did I find anything in the documentation.
So at the very least this needs an update to the manual to warn
future users.
Tom
^ permalink raw reply [flat|nested] 34+ messages in thread
* Re: [PATCH v5 6/8] gdbserver build-id attribute generator
2014-05-20 14:40 ` Tom Tromey
@ 2014-05-21 12:24 ` Jan Kratochvil
2014-05-21 16:48 ` Tom Tromey
0 siblings, 1 reply; 34+ messages in thread
From: Jan Kratochvil @ 2014-05-21 12:24 UTC (permalink / raw)
To: Tom Tromey; +Cc: gdb-patches, Aleksandar Ristovski
On Tue, 20 May 2014 16:40:33 +0200, Tom Tromey wrote:
> >>>>> "Jan" == Jan Kratochvil <jan.kratochvil@redhat.com> writes:
> Jan> -<!ATTLIST library-list-svr4 version CDATA #FIXED "1.0">
> Jan> -<!ATTLIST library-list-svr4 main-lm CDATA #IMPLIED>
> Jan> +<!ATTLIST library-list-svr4 version CDATA #FIXED "1.0">
> Jan> +<!ATTLIST library-list-svr4 main-lm CDATA #IMPLIED>
>
> I am concerned about versioning of the XML.
> Is it correct to keep the same version number?
> If so -- why?
I wrote a never replied essay + [patch] fix about why the current handling
of 'version' is wrong:
[patch] gdbserver <library-list> and its #FIXED version="1.0"
https://sourceware.org/ml/gdb-patches/2011-11/msg00099.html
> Perhaps we consider it compatible to add attributes.
Unrelated to the bug referenced above - I think so.
> I really don't know; nor did I find anything in the documentation.
> So at the very least this needs an update to the manual to warn
> future users.
I do not understand this. The only changed (newly added) attribute is
build-id #IMPLIED - therefore optional - therefore backward/forward
compatible. Who needs to be warned about it?
Regards,
Jan
^ permalink raw reply [flat|nested] 34+ messages in thread
* Re: [PATCH v5 6/8] gdbserver build-id attribute generator
2014-05-21 12:24 ` Jan Kratochvil
@ 2014-05-21 16:48 ` Tom Tromey
0 siblings, 0 replies; 34+ messages in thread
From: Tom Tromey @ 2014-05-21 16:48 UTC (permalink / raw)
To: Jan Kratochvil; +Cc: gdb-patches, Aleksandar Ristovski
Jan> I wrote a never replied essay + [patch] fix about why the current handling
Jan> of 'version' is wrong:
Jan> [patch] gdbserver <library-list> and its #FIXED version="1.0"
Jan> https://sourceware.org/ml/gdb-patches/2011-11/msg00099.html
I read that note. The patch seems correct to me.
Tom> I really don't know; nor did I find anything in the documentation.
Tom> So at the very least this needs an update to the manual to warn
Tom> future users.
Jan> I do not understand this. The only changed (newly added) attribute is
Jan> build-id #IMPLIED - therefore optional - therefore backward/forward
Jan> compatible. Who needs to be warned about it?
I thought all attributes had to be specified in the DTD. Is that not
so?
If they must be then the problem comes when using an older gdb with a
newer gdbserver.
Anyway the point of my "warn" comment is that, ideally, our versioning
approach ought to be documented.
Tom
^ permalink raw reply [flat|nested] 34+ messages in thread
* [PATCH v5 2/8] Merge multiple hex conversions
2014-03-19 23:12 [PATCH v5 0/8] Validate binary before use Jan Kratochvil
` (4 preceding siblings ...)
2014-03-19 22:31 ` [PATCH v5 6/8] gdbserver build-id attribute generator Jan Kratochvil
@ 2014-03-19 22:31 ` Jan Kratochvil
2014-05-19 18:24 ` Tom Tromey
2014-03-19 22:31 ` [PATCH v5 8/8] Tests for validate symbol file using build-id Jan Kratochvil
2014-03-19 22:31 ` [PATCH v5 3/8] Create empty common/linux-maps.[ch] and common/target-utils.[ch] Jan Kratochvil
7 siblings, 1 reply; 34+ messages in thread
From: Jan Kratochvil @ 2014-03-19 22:31 UTC (permalink / raw)
To: gdb-patches; +Cc: Aleksandar Ristovski
Hi,
here most of the patch has been reimplemented in the meantime and this is only
a small remaint.
Jan
gdb/
2014-02-26 Aleksandar Ristovski <aristovski@qnx.com
Merge multiple hex conversions.
* monitor.c: Include rsp-low.h.
(fromhex): Remove definition.
gdb/gdbserver/
2014-02-26 Aleksandar Ristovski <aristovski@qnx.com
Merge multiple hex conversions.
* gdbreplay.c (tohex): Rename to 'fromhex'.
(logchar): Use fromhex.
---
gdb/gdbserver/gdbreplay.c | 6 +++---
gdb/monitor.c | 16 +---------------
2 files changed, 4 insertions(+), 18 deletions(-)
diff --git a/gdb/gdbserver/gdbreplay.c b/gdb/gdbserver/gdbreplay.c
index 706fda6..3fa8e3d 100644
--- a/gdb/gdbserver/gdbreplay.c
+++ b/gdb/gdbserver/gdbreplay.c
@@ -264,7 +264,7 @@ remote_open (char *name)
}
static int
-tohex (int ch)
+fromhex (int ch)
{
if (ch >= '0' && ch <= '9')
{
@@ -327,11 +327,11 @@ logchar (FILE *fp)
ch2 = fgetc (fp);
fputc (ch2, stdout);
fflush (stdout);
- ch = tohex (ch2) << 4;
+ ch = fromhex (ch2) << 4;
ch2 = fgetc (fp);
fputc (ch2, stdout);
fflush (stdout);
- ch |= tohex (ch2);
+ ch |= fromhex (ch2);
break;
default:
/* Treat any other char as just itself */
diff --git a/gdb/monitor.c b/gdb/monitor.c
index c46e2df..e675772 100644
--- a/gdb/monitor.c
+++ b/gdb/monitor.c
@@ -55,6 +55,7 @@
#include "regcache.h"
#include "gdbthread.h"
#include "readline/readline.h"
+#include "rsp-low.h"
static char *dev_name;
static struct target_ops *targ_ops;
@@ -226,21 +227,6 @@ monitor_error (char *function, char *message,
message, safe_string);
}
-/* Convert hex digit A to a number. */
-
-static int
-fromhex (int a)
-{
- if (a >= '0' && a <= '9')
- return a - '0';
- else if (a >= 'a' && a <= 'f')
- return a - 'a' + 10;
- else if (a >= 'A' && a <= 'F')
- return a - 'A' + 10;
- else
- error (_("Invalid hex digit %d"), a);
-}
-
/* monitor_vsprintf - similar to vsprintf but handles 64-bit addresses
This function exists to get around the problem that many host platforms
^ permalink raw reply [flat|nested] 34+ messages in thread* Re: [PATCH v5 2/8] Merge multiple hex conversions
2014-03-19 22:31 ` [PATCH v5 2/8] Merge multiple hex conversions Jan Kratochvil
@ 2014-05-19 18:24 ` Tom Tromey
0 siblings, 0 replies; 34+ messages in thread
From: Tom Tromey @ 2014-05-19 18:24 UTC (permalink / raw)
To: Jan Kratochvil; +Cc: gdb-patches, Aleksandar Ristovski
>>>>> "Jan" == Jan Kratochvil <jan.kratochvil@redhat.com> writes:
Jan> here most of the patch has been reimplemented in the meantime and
Jan> this is only a small remaint.
Jan> gdb/
Jan> 2014-02-26 Aleksandar Ristovski <aristovski@qnx.com
Jan> Merge multiple hex conversions.
Jan> * monitor.c: Include rsp-low.h.
Jan> (fromhex): Remove definition.
Jan> gdb/gdbserver/
Jan> 2014-02-26 Aleksandar Ristovski <aristovski@qnx.com
Jan> Merge multiple hex conversions.
Jan> * gdbreplay.c (tohex): Rename to 'fromhex'.
Jan> (logchar): Use fromhex.
This is ok.
Please put it in.
Tom
^ permalink raw reply [flat|nested] 34+ messages in thread
* [PATCH v5 8/8] Tests for validate symbol file using build-id
2014-03-19 23:12 [PATCH v5 0/8] Validate binary before use Jan Kratochvil
` (5 preceding siblings ...)
2014-03-19 22:31 ` [PATCH v5 2/8] Merge multiple hex conversions Jan Kratochvil
@ 2014-03-19 22:31 ` Jan Kratochvil
2014-05-20 14:57 ` Tom Tromey
2014-03-19 22:31 ` [PATCH v5 3/8] Create empty common/linux-maps.[ch] and common/target-utils.[ch] Jan Kratochvil
7 siblings, 1 reply; 34+ messages in thread
From: Jan Kratochvil @ 2014-03-19 22:31 UTC (permalink / raw)
To: gdb-patches; +Cc: Aleksandar Ristovski
Hi,
new testcase.
Jan
gdb/testsuite/
2014-02-26 Aleksandar Ristovski <aristovski@qnx.com
Tests for validate symbol file using build-id.
* gdb.server/solib-mismatch-lib.c: New file.
* gdb.server/solib-mismatch-libmod.c: New file.
* gdb.server/solib-mismatch.c: New file.
* gdb.server/solib-mismatch.exp: New file.
---
gdb/testsuite/gdb.server/solib-mismatch-lib.c | 29 ++++
gdb/testsuite/gdb.server/solib-mismatch-libmod.c | 29 ++++
gdb/testsuite/gdb.server/solib-mismatch.c | 56 ++++++++
gdb/testsuite/gdb.server/solib-mismatch.exp | 156 ++++++++++++++++++++++
4 files changed, 270 insertions(+)
create mode 100644 gdb/testsuite/gdb.server/solib-mismatch-lib.c
create mode 100644 gdb/testsuite/gdb.server/solib-mismatch-libmod.c
create mode 100644 gdb/testsuite/gdb.server/solib-mismatch.c
create mode 100644 gdb/testsuite/gdb.server/solib-mismatch.exp
diff --git a/gdb/testsuite/gdb.server/solib-mismatch-lib.c b/gdb/testsuite/gdb.server/solib-mismatch-lib.c
new file mode 100644
index 0000000..65b26af
--- /dev/null
+++ b/gdb/testsuite/gdb.server/solib-mismatch-lib.c
@@ -0,0 +1,29 @@
+/* This testcase is part of GDB, the GNU debugger.
+
+ 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
+ the Free Software Foundation; either version 3 of the License, or
+ (at your option) any later version.
+
+ This program is distributed in the hope that it will be useful,
+ but WITHOUT ANY WARRANTY; without even the implied warranty of
+ MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+ GNU General Public License for more details.
+
+ You should have received a copy of the GNU General Public License
+ along with this program. If not, see <http://www.gnu.org/licenses/>. */
+
+
+int _bar = 42;
+
+int bar(void)
+{
+ return _bar + 21;
+}
+
+int foo(void)
+{
+ return _bar;
+}
diff --git a/gdb/testsuite/gdb.server/solib-mismatch-libmod.c b/gdb/testsuite/gdb.server/solib-mismatch-libmod.c
new file mode 100644
index 0000000..fc8827e
--- /dev/null
+++ b/gdb/testsuite/gdb.server/solib-mismatch-libmod.c
@@ -0,0 +1,29 @@
+/* This testcase is part of GDB, the GNU debugger.
+
+ 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
+ the Free Software Foundation; either version 3 of the License, or
+ (at your option) any later version.
+
+ This program is distributed in the hope that it will be useful,
+ but WITHOUT ANY WARRANTY; without even the implied warranty of
+ MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+ GNU General Public License for more details.
+
+ You should have received a copy of the GNU General Public License
+ along with this program. If not, see <http://www.gnu.org/licenses/>. */
+
+
+int _bar = 21;
+
+int bar(void)
+{
+ return 42 - _bar;
+}
+
+int foo(void)
+{
+ return 24 + bar();
+}
diff --git a/gdb/testsuite/gdb.server/solib-mismatch.c b/gdb/testsuite/gdb.server/solib-mismatch.c
new file mode 100644
index 0000000..c8be18a
--- /dev/null
+++ b/gdb/testsuite/gdb.server/solib-mismatch.c
@@ -0,0 +1,56 @@
+/* This testcase is part of GDB, the GNU debugger.
+
+ 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
+ the Free Software Foundation; either version 3 of the License, or
+ (at your option) any later version.
+
+ This program is distributed in the hope that it will be useful,
+ but WITHOUT ANY WARRANTY; without even the implied warranty of
+ MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+ GNU General Public License for more details.
+
+ You should have received a copy of the GNU General Public License
+ along with this program. If not, see <http://www.gnu.org/licenses/>. */
+
+
+#include <dlfcn.h>
+#include <stdio.h>
+#include <unistd.h>
+#include <signal.h>
+#include <string.h>
+
+/* The following defines must correspond to solib-mismatch.exp . */
+
+/* DIRNAME and LIB must be defined at compile time. */
+#ifndef DIRNAME
+#error DIRNAME not defined
+#endif
+#ifndef LIB
+#error LIB not defined
+#endif
+
+int main (int argc, char *argv[])
+{
+ void *h;
+ int (*foo) (void);
+
+ if (chdir (DIRNAME) != 0)
+ {
+ printf ("ERROR - Could not cd to %s\n", DIRNAME);
+ return 1;
+ }
+
+ h = dlopen (LIB, RTLD_NOW);
+
+ if (h == NULL)
+ {
+ printf ("ERROR - could not open lib %s\n", LIB);
+ return 1;
+ }
+ foo = dlsym (h, "foo"); /* set breakpoint 1 here */
+ dlclose (h);
+ return 0;
+}
diff --git a/gdb/testsuite/gdb.server/solib-mismatch.exp b/gdb/testsuite/gdb.server/solib-mismatch.exp
new file mode 100644
index 0000000..9dbce7f
--- /dev/null
+++ b/gdb/testsuite/gdb.server/solib-mismatch.exp
@@ -0,0 +1,156 @@
+# 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
+# the Free Software Foundation; either version 3 of the License, or
+# (at your option) any later version.
+#
+# This program is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+# GNU General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with this program. If not, see <http://www.gnu.org/licenses/>. */
+
+standard_testfile
+set executable $testfile
+
+if ![is_remote target] {
+ untested "only gdbserver supports build-id reporting"
+ return -1
+}
+if [is_remote host] {
+ untested "only local host is currently supported"
+ 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
+# detect the mismatch and refuse to use mismatched shared object.
+
+if { [get_compiler_info] } {
+ untested "get_compiler_info failed."
+ return -1
+}
+
+# First version of the object, to be loaded by ld.
+set srclibfilerun ${testfile}-lib.c
+
+# Modified version of the object to be loaded by gdb
+# Code in -libmod.c is tuned so it gives a mismatch but
+# leaves .dynamic at the same point.
+set srclibfilegdb ${testfile}-libmod.c
+
+# So file name:
+set binlibfilebase lib${testfile}.so
+
+# Setup run directory (where program is run from)
+# It contains executable and '-lib' version of the library.
+set binlibfiledirrun [standard_output_file ${testfile}_wd]
+set binlibfilerun ${binlibfiledirrun}/${binlibfilebase}
+
+# Second solib version is in current directory, '-libmod' version.
+set binlibfiledirgdb [standard_output_file ""]
+set binlibfilegdb ${binlibfiledirgdb}/${binlibfilebase}
+
+# Executable
+set srcfile ${testfile}.c
+set executable ${testfile}
+
+file delete -force -- "${binlibfiledirrun}"
+file mkdir "${binlibfiledirrun}"
+
+set exec_opts {}
+
+if { ![istarget "*-*-nto-*"] } {
+ lappend exec_opts "shlib_load"
+}
+
+lappend exec_opts "additional_flags=-DDIRNAME\=\"${binlibfiledirrun}\" -DLIB\=\"./${binlibfilebase}\""
+lappend exec_opts "debug"
+
+if { [build_executable $testfile.exp $executable $srcfile $exec_opts] != 0 } {
+ return -1
+}
+
+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 "compilation failed."
+ return -1
+}
+
+proc solib_matching_test { solibfile symsloaded } {
+ global gdb_prompt
+ global testfile
+ global executable
+ global srcdir
+ global subdir
+ global binlibfiledirrun
+ global binlibfiledirgdb
+ global srcfile
+
+ clean_restart ${binlibfiledirrun}/${executable}
+
+ gdb_test_no_output "set solib-search-path \"${binlibfiledirgdb}\"" ""
+ if { [gdb_test "cd ${binlibfiledirgdb}" "" ""] != 0 } {
+ untested "cd ${binlibfiledirgdb}"
+ return -1
+ }
+
+ # Do not auto load shared libraries, the test needs to have control
+ # over when the relevant output gets printed.
+ gdb_test_no_output "set auto-solib-add off" ""
+
+ if ![runto "${srcfile}:[gdb_get_line_number "set breakpoint 1 here"]"] {
+ return -1
+ }
+
+ gdb_test "sharedlibrary" "" ""
+
+ set nocrlf "\[^\r\n\]*"
+ set expected_header "From${nocrlf}To${nocrlf}Syms${nocrlf}Read${nocrlf}Shared${nocrlf}"
+ set expected_line "${symsloaded}${nocrlf}${solibfile}"
+
+ gdb_test "info sharedlibrary ${solibfile}" \
+ "${expected_header}\r\n.*${expected_line}.*" \
+ "Symbols for ${solibfile} loaded: expected '${symsloaded}'"
+
+ return 0
+}
+
+# Copy binary to working dir so it pulls in the library from that dir
+# (by the virtue of $ORIGIN).
+file copy -force "${binlibfiledirgdb}/${executable}" \
+ "${binlibfiledirrun}/${executable}"
+
+# Test unstripped, .dynamic matching
+with_test_prefix "test unstripped, .dynamic matching" {
+ solib_matching_test "${binlibfilebase}" "No"
+}
+
+# Keep original so for debugging purposes
+file copy -force "${binlibfilegdb}" "${binlibfilegdb}-orig"
+set objcopy_program [transform objcopy]
+set result [catch "exec $objcopy_program --only-keep-debug ${binlibfilegdb}"]
+if {$result != 0} {
+ untested "test --only-keep-debug (objcopy)"
+}
+
+# Test --only-keep-debug, .dynamic matching so
+with_test_prefix "test --only-keep-debug" {
+ solib_matching_test "${binlibfilebase}" "No"
+}
+
+# Keep previous so for debugging puroses
+file copy -force "${binlibfilegdb}" "${binlibfilegdb}-orig1"
+
+# Copy loaded so over the one gdb will find
+file copy -force "${binlibfilerun}" "${binlibfilegdb}"
+
+# Now test it does not mis-invalidate matching libraries
+with_test_prefix "test matching libraries" {
+ solib_matching_test "${binlibfilebase}" "Yes"
+}
^ permalink raw reply [flat|nested] 34+ messages in thread* Re: [PATCH v5 8/8] Tests for validate symbol file using build-id
2014-03-19 22:31 ` [PATCH v5 8/8] Tests for validate symbol file using build-id Jan Kratochvil
@ 2014-05-20 14:57 ` Tom Tromey
2014-05-20 15:29 ` Jan Kratochvil
0 siblings, 1 reply; 34+ messages in thread
From: Tom Tromey @ 2014-05-20 14:57 UTC (permalink / raw)
To: Jan Kratochvil; +Cc: gdb-patches, Aleksandar Ristovski
>>>>> "Jan" == Jan Kratochvil <jan.kratochvil@redhat.com> writes:
Jan> 2014-02-26 Aleksandar Ristovski <aristovski@qnx.com
Jan> Tests for validate symbol file using build-id.
Jan> * gdb.server/solib-mismatch-lib.c: New file.
Jan> * gdb.server/solib-mismatch-libmod.c: New file.
Jan> * gdb.server/solib-mismatch.c: New file.
Jan> * gdb.server/solib-mismatch.exp: New file.
I thought Pedro had wanted these not in gdb.server.
Or am I confusing that with some other patch?
Jan> +if ![is_remote target] {
Jan> + untested "only gdbserver supports build-id reporting"
Jan> + return -1
I was mildly confused to read this.
Isn't build-id also supported natively?
How does the new functionality interact with the existing build-id
functionality?
Tom
^ permalink raw reply [flat|nested] 34+ messages in thread* Re: [PATCH v5 8/8] Tests for validate symbol file using build-id
2014-05-20 14:57 ` Tom Tromey
@ 2014-05-20 15:29 ` Jan Kratochvil
0 siblings, 0 replies; 34+ messages in thread
From: Jan Kratochvil @ 2014-05-20 15:29 UTC (permalink / raw)
To: Tom Tromey; +Cc: gdb-patches, Aleksandar Ristovski
On Tue, 20 May 2014 16:57:11 +0200, Tom Tromey wrote:
> >>>>> "Jan" == Jan Kratochvil <jan.kratochvil@redhat.com> writes:
>
> Jan> 2014-02-26 Aleksandar Ristovski <aristovski@qnx.com
> Jan> Tests for validate symbol file using build-id.
> Jan> * gdb.server/solib-mismatch-lib.c: New file.
> Jan> * gdb.server/solib-mismatch-libmod.c: New file.
> Jan> * gdb.server/solib-mismatch.c: New file.
> Jan> * gdb.server/solib-mismatch.exp: New file.
>
> I thought Pedro had wanted these not in gdb.server.
> Or am I confusing that with some other patch?
In a local copy they are moved back to gdb.base/ .
But I haven't re-post the whole series just because of it.
Planning to check it in into gdb.base/ .
> Jan> +if ![is_remote target] {
> Jan> + untested "only gdbserver supports build-id reporting"
> Jan> + return -1
>
> I was mildly confused to read this.
> Isn't build-id also supported natively?
For this case of build-id it is not. It was discussed in:
Re: [patchv3 7/8] Validate symbol file using build-id
Message-ID: <53108EF7.3000708@redhat.com>
https://sourceware.org/ml/gdb-patches/2014-02/msg00862.html
https://sourceware.org/ml/gdb-patches/2014-03/msg00011.html
> How does the new functionality interact with the existing build-id
> functionality?
Currently only the separate debug info file is located and validated by
build-id. This patch is about validating the primary file (without .debug
extension - in fact symbol file as GDB uses that file only for symbols).
Besides all of these there are additional patches:
http://pkgs.fedoraproject.org/cgit/gdb.git/tree/
gdb-6.6-buildid-locate-*
which are also about locating the primary files but those patches are for
local (NAT) mode.
Also this series does not validate / locate the main executable, the Fedora
patches above work also for the main executable.
It got all a bit messy so I decided to merge it all and preparing it as an
update (technically add-on) on this patch series. So also contrary to my
original plans in
Re: [patchv3 7/8] Validate symbol file using build-id
above I am going to implement the local (NAT) mode for locating the files.
As I expect GDB is not going to unify LocalRemoteFeatureParity soon enough.
It will be about upstreaming the Fedora patchset which is the last largest
Fedora-specific patchsets kept.
Jan
^ permalink raw reply [flat|nested] 34+ messages in thread
* [PATCH v5 3/8] Create empty common/linux-maps.[ch] and common/target-utils.[ch]
2014-03-19 23:12 [PATCH v5 0/8] Validate binary before use Jan Kratochvil
` (6 preceding siblings ...)
2014-03-19 22:31 ` [PATCH v5 8/8] Tests for validate symbol file using build-id Jan Kratochvil
@ 2014-03-19 22:31 ` Jan Kratochvil
2014-05-19 18:29 ` Tom Tromey
7 siblings, 1 reply; 34+ messages in thread
From: Jan Kratochvil @ 2014-03-19 22:31 UTC (permalink / raw)
To: gdb-patches; +Cc: Aleksandar Ristovski
Hi,
prepare new files for later move.
Jan
gdb/
2014-02-26 Aleksandar Ristovski <aristovski@qnx.com
Create empty common/linux-maps.[ch] and common/target-utils.[ch].
* Makefile.in (HFILES_NO_SRCDIR); Add common/linux-maps.h,
common/target-utils.h.
(COMMON_OBS): Add target-utils.o.
(linux-maps.o, target-utils.o): New.
* common/target-utils.c: New file.
* common/target-utils.h: New file.
* common/linux-maps.c: New file.
* common/linux-maps.h: New file.
* config/i386/linux.mh (NATDEPFILES): Add linux-maps.o.
* config/i386/linux64.mh (NATDEPFILES): Ditto.
gdb/gdbserver/
2014-02-26 Aleksandar Ristovski <aristovski@qnx.com
Create empty common/linux-maps.[ch] and common/target-utils.[ch].
* Makefile.in (OBS): Add target-utils.o.
(linux-maps.o, target-utils.o): New.
* configure.srv (srv_linux_obj): Add linux-maps.o.
---
gdb/Makefile.in | 15 ++++++++++++---
gdb/common/linux-maps.c | 25 +++++++++++++++++++++++++
gdb/common/linux-maps.h | 22 ++++++++++++++++++++++
gdb/common/target-utils.c | 26 ++++++++++++++++++++++++++
gdb/common/target-utils.h | 23 +++++++++++++++++++++++
gdb/config/i386/linux.mh | 2 +-
gdb/config/i386/linux64.mh | 2 +-
gdb/gdbserver/Makefile.in | 9 ++++++++-
gdb/gdbserver/configure.srv | 2 +-
9 files changed, 119 insertions(+), 7 deletions(-)
create mode 100644 gdb/common/linux-maps.c
create mode 100644 gdb/common/linux-maps.h
create mode 100644 gdb/common/target-utils.c
create mode 100644 gdb/common/target-utils.h
diff --git a/gdb/Makefile.in b/gdb/Makefile.in
index 3efedc8..2dc5c47 100644
--- a/gdb/Makefile.in
+++ b/gdb/Makefile.in
@@ -852,7 +852,8 @@ LINTFILES = $(SFILES) $(YYFILES) $(CONFIG_SRCS) init.c
HFILES_NO_SRCDIR = \
common/gdb_signals.h common/gdb_thread_db.h common/gdb_vecs.h \
-common/i386-xstate.h common/linux-ptrace.h common/mips-linux-watch.h \
+common/i386-xstate.h \
+common/linux-maps.h common/linux-ptrace.h common/mips-linux-watch.h \
proc-utils.h aarch64-tdep.h arm-tdep.h ax-gdb.h ppcfbsd-tdep.h \
ppcnbsd-tdep.h cli-out.h gdb_expat.h breakpoint.h infcall.h obsd-tdep.h \
exec.h m32r-tdep.h osabi.h gdbcore.h solib-som.h amd64bsd-nat.h \
@@ -923,7 +924,7 @@ common/linux-osdata.h gdb-dlfcn.h auto-load.h probe.h stap-probe.h \
gdb_bfd.h sparc-ravenscar-thread.h ppc-ravenscar-thread.h common/linux-btrace.h \
ctf.h common/i386-cpuid.h common/i386-gcc-cpuid.h target/resume.h \
target/wait.h target/waitstatus.h nat/linux-nat.h nat/linux-waitpid.h \
-common/print-utils.h common/rsp-low.h
+common/print-utils.h common/rsp-low.h common/target-utils.h
# Header files that already have srcdir in them, or which are in objdir.
@@ -1022,7 +1023,7 @@ COMMON_OBS = $(DEPFILES) $(CONFIG_OBS) $(YYOBJ) \
gdb_vecs.o jit.o progspace.o skip.o probe.o \
common-utils.o buffer.o ptid.o gdb-dlfcn.o common-agent.o \
format.o registry.o btrace.o record-btrace.o waitstatus.o \
- print-utils.o rsp-low.o
+ print-utils.o rsp-low.o target-utils.o
TSOBS = inflow.o
@@ -2116,6 +2117,10 @@ format.o: ${srcdir}/common/format.c
$(COMPILE) $(srcdir)/common/format.c
$(POSTCOMPILE)
+linux-maps.o: ${srcdir}/common/linux-maps.c
+ $(COMPILE) $(srcdir)/common/linux-maps.c
+ $(POSTCOMPILE)
+
linux-osdata.o: ${srcdir}/common/linux-osdata.c
$(COMPILE) $(srcdir)/common/linux-osdata.c
$(POSTCOMPILE)
@@ -2132,6 +2137,10 @@ common-agent.o: $(srcdir)/common/agent.c
$(COMPILE) $(srcdir)/common/agent.c
$(POSTCOMPILE)
+target-utils.o: ${srcdir}/common/target-utils.c
+ $(COMPILE) $(srcdir)/common/target-utils.c
+ $(POSTCOMPILE)
+
vec.o: ${srcdir}/common/vec.c
$(COMPILE) $(srcdir)/common/vec.c
$(POSTCOMPILE)
diff --git a/gdb/common/linux-maps.c b/gdb/common/linux-maps.c
new file mode 100644
index 0000000..bb3eac9
--- /dev/null
+++ b/gdb/common/linux-maps.c
@@ -0,0 +1,25 @@
+/* Linux-specific memory maps manipulation routines.
+ Copyright (C) 2014 Free Software Foundation, Inc.
+
+ This file is part of GDB.
+
+ 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
+ the Free Software Foundation; either version 3 of the License, or
+ (at your option) any later version.
+
+ This program is distributed in the hope that it will be useful,
+ but WITHOUT ANY WARRANTY; without even the implied warranty of
+ MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+ GNU General Public License for more details.
+
+ You should have received a copy of the GNU General Public License
+ along with this program. If not, see <http://www.gnu.org/licenses/>. */
+
+#ifdef GDBSERVER
+#include "server.h"
+#else
+#include "defs.h"
+#endif
+
+#include "linux-maps.h"
diff --git a/gdb/common/linux-maps.h b/gdb/common/linux-maps.h
new file mode 100644
index 0000000..ebf6f37
--- /dev/null
+++ b/gdb/common/linux-maps.h
@@ -0,0 +1,22 @@
+/* Linux-specific memory maps manipulation routines.
+ Copyright (C) 2014 Free Software Foundation, Inc.
+
+ This file is part of GDB.
+
+ 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
+ the Free Software Foundation; either version 3 of the License, or
+ (at your option) any later version.
+
+ This program is distributed in the hope that it will be useful,
+ but WITHOUT ANY WARRANTY; without even the implied warranty of
+ MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+ GNU General Public License for more details.
+
+ You should have received a copy of the GNU General Public License
+ along with this program. If not, see <http://www.gnu.org/licenses/>. */
+
+#ifndef COMMON_LINUX_MAPS_H
+#define COMMON_LINUX_MAPS_H
+
+#endif /* COMMON_LINUX_MAPS_H */
diff --git a/gdb/common/target-utils.c b/gdb/common/target-utils.c
new file mode 100644
index 0000000..308996d
--- /dev/null
+++ b/gdb/common/target-utils.c
@@ -0,0 +1,26 @@
+/* Utility target functions for GDB, and GDBserver.
+
+ Copyright (C) 2014 Free Software Foundation, Inc.
+
+ This file is part of GDB.
+
+ 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
+ the Free Software Foundation; either version 3 of the License, or
+ (at your option) any later version.
+
+ This program is distributed in the hope that it will be useful,
+ but WITHOUT ANY WARRANTY; without even the implied warranty of
+ MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+ GNU General Public License for more details.
+
+ You should have received a copy of the GNU General Public License
+ along with this program. If not, see <http://www.gnu.org/licenses/>. */
+
+#ifdef GDBSERVER
+#include "server.h"
+#else
+#include "defs.h"
+#endif
+
+#include "target-utils.h"
diff --git a/gdb/common/target-utils.h b/gdb/common/target-utils.h
new file mode 100644
index 0000000..db33ec8
--- /dev/null
+++ b/gdb/common/target-utils.h
@@ -0,0 +1,23 @@
+/* Utility target functions for GDB, and GDBserver.
+
+ Copyright (C) 2014 Free Software Foundation, Inc.
+
+ This file is part of GDB.
+
+ 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
+ the Free Software Foundation; either version 3 of the License, or
+ (at your option) any later version.
+
+ This program is distributed in the hope that it will be useful,
+ but WITHOUT ANY WARRANTY; without even the implied warranty of
+ MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+ GNU General Public License for more details.
+
+ You should have received a copy of the GNU General Public License
+ along with this program. If not, see <http://www.gnu.org/licenses/>. */
+
+#ifndef COMMON_TARGET_UTILS_H
+#define COMMON_TARGET_UTILS_H
+
+#endif /* COMMON_TARGET_UTILS_H */
diff --git a/gdb/config/i386/linux.mh b/gdb/config/i386/linux.mh
index 10a2584..55dd3df 100644
--- a/gdb/config/i386/linux.mh
+++ b/gdb/config/i386/linux.mh
@@ -3,7 +3,7 @@
NAT_FILE= config/nm-linux.h
NATDEPFILES= inf-ptrace.o fork-child.o \
i386-nat.o i386-linux-nat.o \
- proc-service.o linux-thread-db.o \
+ proc-service.o linux-thread-db.o linux-maps.o \
linux-nat.o linux-osdata.o linux-fork.o linux-procfs.o linux-ptrace.o \
linux-btrace.o linux-waitpid.o
NAT_CDEPS = $(srcdir)/proc-service.list
diff --git a/gdb/config/i386/linux64.mh b/gdb/config/i386/linux64.mh
index 686c363..6b2943d 100644
--- a/gdb/config/i386/linux64.mh
+++ b/gdb/config/i386/linux64.mh
@@ -1,7 +1,7 @@
# Host: GNU/Linux x86-64
NATDEPFILES= inf-ptrace.o fork-child.o \
i386-nat.o amd64-nat.o amd64-linux-nat.o \
- linux-nat.o linux-osdata.o \
+ linux-maps.o linux-nat.o linux-osdata.o \
proc-service.o linux-thread-db.o linux-fork.o \
linux-procfs.o linux-ptrace.o linux-btrace.o \
linux-waitpid.o
diff --git a/gdb/gdbserver/Makefile.in b/gdb/gdbserver/Makefile.in
index 663deb6..7d267f7 100644
--- a/gdb/gdbserver/Makefile.in
+++ b/gdb/gdbserver/Makefile.in
@@ -177,7 +177,8 @@ OBS = agent.o ax.o inferiors.o regcache.o remote-utils.o server.o signals.o \
target.o waitstatus.o utils.o debug.o version.o vec.o gdb_vecs.o \
mem-break.o hostio.o event-loop.o tracepoint.o xml-utils.o \
common-utils.o ptid.o buffer.o format.o filestuff.o dll.o notif.o \
- tdesc.o print-utils.o rsp-low.o $(XML_BUILTIN) $(DEPFILES) $(LIBOBJS)
+ tdesc.o print-utils.o rsp-low.o target-utils.o \
+ $(XML_BUILTIN) $(DEPFILES) $(LIBOBJS)
GDBREPLAY_OBS = gdbreplay.o version.o
GDBSERVER_LIBS = @GDBSERVER_LIBS@
XM_CLIBS = @LIBS@
@@ -486,6 +487,9 @@ ax.o: ax.c
signals.o: ../common/signals.c
$(COMPILE) $<
$(POSTCOMPILE)
+linux-maps.o: ../common/linux-maps.c
+ $(COMPILE) $<
+ $(POSTCOMPILE)
print-utils.o: ../common/print-utils.c
$(COMPILE) $<
$(POSTCOMPILE)
@@ -498,6 +502,9 @@ linux-procfs.o: ../common/linux-procfs.c
linux-ptrace.o: ../common/linux-ptrace.c
$(COMPILE) $<
$(POSTCOMPILE)
+target-utils.o: ../common/target-utils.c
+ $(COMPILE) $<
+ $(POSTCOMPILE)
common-utils.o: ../common/common-utils.c
$(COMPILE) $<
$(POSTCOMPILE)
diff --git a/gdb/gdbserver/configure.srv b/gdb/gdbserver/configure.srv
index f4e6154..e4f7aa6 100644
--- a/gdb/gdbserver/configure.srv
+++ b/gdb/gdbserver/configure.srv
@@ -42,7 +42,7 @@ srv_amd64_linux_xmlfiles="i386/amd64-linux.xml i386/amd64-avx-linux.xml i386/64b
# Linux object files. This is so we don't have to repeat
# these files over and over again.
-srv_linux_obj="linux-low.o linux-osdata.o linux-procfs.o linux-ptrace.o linux-waitpid.o"
+srv_linux_obj="linux-low.o linux-osdata.o linux-procfs.o linux-maps.o linux-ptrace.o linux-waitpid.o"
# Input is taken from the "${target}" variable.
^ permalink raw reply [flat|nested] 34+ messages in thread* Re: [PATCH v5 3/8] Create empty common/linux-maps.[ch] and common/target-utils.[ch]
2014-03-19 22:31 ` [PATCH v5 3/8] Create empty common/linux-maps.[ch] and common/target-utils.[ch] Jan Kratochvil
@ 2014-05-19 18:29 ` Tom Tromey
0 siblings, 0 replies; 34+ messages in thread
From: Tom Tromey @ 2014-05-19 18:29 UTC (permalink / raw)
To: Jan Kratochvil; +Cc: gdb-patches, Aleksandar Ristovski
>>>>> "Jan" == Jan Kratochvil <jan.kratochvil@redhat.com> writes:
Jan> 2014-02-26 Aleksandar Ristovski <aristovski@qnx.com
Jan> Create empty common/linux-maps.[ch] and common/target-utils.[ch].
Jan> * Makefile.in (HFILES_NO_SRCDIR); Add common/linux-maps.h,
Jan> common/target-utils.h.
Jan> (COMMON_OBS): Add target-utils.o.
Jan> (linux-maps.o, target-utils.o): New.
Jan> * common/target-utils.c: New file.
Jan> * common/target-utils.h: New file.
Jan> * common/linux-maps.c: New file.
Jan> * common/linux-maps.h: New file.
Jan> * config/i386/linux.mh (NATDEPFILES): Add linux-maps.o.
Jan> * config/i386/linux64.mh (NATDEPFILES): Ditto.
Jan> gdb/gdbserver/
Jan> 2014-02-26 Aleksandar Ristovski <aristovski@qnx.com
Jan> Create empty common/linux-maps.[ch] and common/target-utils.[ch].
Jan> * Makefile.in (OBS): Add target-utils.o.
Jan> (linux-maps.o, target-utils.o): New.
Jan> * configure.srv (srv_linux_obj): Add linux-maps.o.
I think it's slightly weird to introduce new empty files, but I suppose
it doesn't hurt. This is fine pending the rest of the series.
Tom
^ permalink raw reply [flat|nested] 34+ messages in thread