From: Pádraig Brady <pbrady@fb.com>
Multiple places in dwarf2read.c open code 1-based to 0-based index
conversion and check for out of bounds accesses to lh->include_dirs
and lh->file_names. This commit factors those out to a couple methods
and uses them throughout.
gdb/ChangeLog:
* dwarf2read.c (file_entry) <dir_index>: Add comment.
(file_entry::include_dir): New method.
(line_header::include_dir_at, line_header::file_name_at): New
methods.
(setup_type_unit_groups, setup_type_unit_groups)
(psymtab_include_file_name): Simplify using the new methods.
(lnp_state_machine) <the_line_header>: New field.
<file>: Add comment.
(lnp_state_machine::current_file): New method.
(init_lnp_state_machine): Initialize the "the_line_header" field.
(dwarf_decode_lines_1, dwarf_decode_lines, file_file_name):
Simplify using the new methods.
---
gdb/dwarf2read.c | 129 +++++++++++++++++++++++++++++++------------------------
1 file changed, 74 insertions(+), 55 deletions(-)
On 03/24/2017 03:24 PM, Pedro Alves wrote:
> From: Pádraig Brady <pbrady@fb.com>
The above was automatically inserted by git send-email, because
I had messed up the commit's git author locally. I've fixed it
locally (I'm the author) ...
> Multiple places in dwarf2read.c open code 1-based to 0-based index
> conversion and check for out of bounds accesses to lh->include_dirs
> and lh->file_names. This commit factors those out to a couple methods
> and uses them throughout.
>
> gdb/ChangeLog:
> * dwarf2read.c (file_entry) <dir_index>: Add comment.
> (file_entry::include_dir): New method.
> (line_header::include_dir_at, line_header::file_name_at): New
> methods.
> (setup_type_unit_groups, setup_type_unit_groups)
> (psymtab_include_file_name): Simplify using the new methods.
> (lnp_state_machine) <the_line_header>: New field.
> <file>: Add comment.
> (lnp_state_machine::current_file): New method.
> (init_lnp_state_machine): Initialize the "the_line_header" field.
> (dwarf_decode_lines_1, dwarf_decode_lines, file_file_name):
> Simplify using the new methods.
... and pushed to master, after Pádraig confirmed gdb still gracefully
handles the problematic binary:
https://sourceware.org/ml/gdb-patches/2017-03/msg00451.html
Thanks,
Pedro Alves
@@ -1055,6 +1055,7 @@ typedef void (die_reader_func_ftype) (const struct die_reader_specs *reader,
struct file_entry
{
const char *name;
+ /* The directory index (1-based). */
unsigned int dir_index;
unsigned int mod_time;
unsigned int length;
@@ -1062,6 +1063,10 @@ struct file_entry
int included_p;
/* The associated symbol table, if any. */
struct symtab *symtab;
+
+ /* Return the include directory at DIR_INDEX stored in LH. Returns
+ NULL if DIR_INDEX is out of bounds. */
+ const char *include_dir (const line_header *lh) const;
};
/* The line number information for a compilation unit (found in the
@@ -1107,8 +1112,34 @@ struct line_header
/* The start and end of the statement program following this
header. These point into dwarf2_per_objfile->line_buffer. */
const gdb_byte *statement_program_start, *statement_program_end;
+
+ /* Return the include dir at INDEX (0-based). Returns NULL if INDEX
+ is out of bounds. */
+ const char *include_dir_at (unsigned int index) const
+ {
+ if (include_dirs == NULL || index >= num_include_dirs)
+ return NULL;
+ return include_dirs[index];
+ }
+
+ /* Return the file name at INDEX (0-based). Returns NULL if INDEX
+ is out of bounds. */
+ const file_entry *file_name_at (unsigned int index) const
+ {
+ if (file_names == NULL || index >= num_file_names)
+ return NULL;
+ return &file_names[index];
+ }
};
+const char *
+file_entry::include_dir (const line_header *lh) const
+{
+ /* lh->include_dirs is 0-based, but the directory index numbers in
+ the statement program are 1-based. */
+ return lh->include_dir_at (dir_index - 1);
+}
+
/* When we construct a partial symbol table entry we only
need this much information. */
struct partial_die_info
@@ -9413,13 +9444,9 @@ setup_type_unit_groups (struct die_info *die, struct dwarf2_cu *cu)
for (i = 0; i < lh->num_file_names; ++i)
{
- const char *dir = NULL;
- struct file_entry *fe = &lh->file_names[i];
+ file_entry &fe = lh->file_names[i];
- if (fe->dir_index && lh->include_dirs != NULL
- && (fe->dir_index - 1) < lh->num_include_dirs)
- dir = lh->include_dirs[fe->dir_index - 1];
- dwarf2_start_subfile (fe->name, dir);
+ dwarf2_start_subfile (fe.name, fe.include_dir (lh));
if (current_subfile->symtab == NULL)
{
@@ -9431,8 +9458,8 @@ setup_type_unit_groups (struct die_info *die, struct dwarf2_cu *cu)
= allocate_symtab (cust, current_subfile->name);
}
- fe->symtab = current_subfile->symtab;
- tu_group->symtabs[i] = fe->symtab;
+ fe.symtab = current_subfile->symtab;
+ tu_group->symtabs[i] = fe.symtab;
}
}
else
@@ -17978,17 +18005,14 @@ psymtab_include_file_name (const struct line_header *lh, int file_index,
const struct partial_symtab *pst,
const char *comp_dir)
{
- const struct file_entry fe = lh->file_names [file_index];
+ const file_entry &fe = lh->file_names[file_index];
const char *include_name = fe.name;
const char *include_name_to_compare = include_name;
- const char *dir_name = NULL;
const char *pst_filename;
char *copied_name = NULL;
int file_is_pst;
- if (fe.dir_index && lh->include_dirs != NULL
- && (fe.dir_index - 1) < lh->num_include_dirs)
- dir_name = lh->include_dirs[fe.dir_index - 1];
+ const char *dir_name = fe.include_dir (lh);
if (!IS_ABSOLUTE_PATH (include_name)
&& (dir_name != NULL || comp_dir != NULL))
@@ -18053,11 +18077,15 @@ psymtab_include_file_name (const struct line_header *lh, int file_index,
/* State machine to track the state of the line number program. */
-typedef struct
+struct lnp_state_machine
{
+ /* The line number header. */
+ line_header *the_line_header;
+
/* These are part of the standard DWARF line number state machine. */
unsigned char op_index;
+ /* The line table index (1-based) of the current file. */
unsigned int file;
unsigned int line;
CORE_ADDR address;
@@ -18080,7 +18108,14 @@ typedef struct
example, when discriminators are present. PR 17276. */
unsigned int last_line;
int line_has_non_zero_discriminator;
-} lnp_state_machine;
+
+ const file_entry *current_file ()
+ {
+ /* lh->file_names is 0-based, but the file name numbers in the
+ statement program are 1-based. */
+ return the_line_header->file_name_at (file - 1);
+ }
+};
/* There's a lot of static state to pass to dwarf_record_line.
This keeps it all together. */
@@ -18285,6 +18320,7 @@ init_lnp_state_machine (lnp_state_machine *state,
state->address = gdbarch_adjust_dwarf2_line (reader->gdbarch, 0, 0);
state->is_stmt = reader->line_header->default_is_stmt;
state->discriminator = 0;
+ state->the_line_header = reader->line_header;
}
/* Check address and if invalid nop-out the rest of the lines in this
@@ -18359,20 +18395,14 @@ dwarf_decode_lines_1 (struct line_header *lh, struct dwarf2_cu *cu,
/* Reset the state machine at the start of each sequence. */
init_lnp_state_machine (&state_machine, &reader_state);
- if (record_lines_p && lh->num_file_names >= state_machine.file)
+ if (record_lines_p)
{
- /* Start a subfile for the current file of the state machine. */
- /* lh->include_dirs and lh->file_names are 0-based, but the
- directory and file name numbers in the statement program
- are 1-based. */
- struct file_entry *fe = &lh->file_names[state_machine.file - 1];
- const char *dir = NULL;
-
- if (fe->dir_index && lh->include_dirs != NULL
- && (fe->dir_index - 1) < lh->num_include_dirs)
- dir = lh->include_dirs[fe->dir_index - 1];
-
- dwarf2_start_subfile (fe->name, dir);
+ /* Start a subfile for the current file of the state
+ machine. */
+ const file_entry *fe = state_machine.current_file ();
+
+ if (fe != NULL)
+ dwarf2_start_subfile (fe->name, fe->include_dir (lh));
}
/* Decode the table. */
@@ -18517,26 +18547,19 @@ dwarf_decode_lines_1 (struct line_header *lh, struct dwarf2_cu *cu,
break;
case DW_LNS_set_file:
{
- /* The arrays lh->include_dirs and lh->file_names are
- 0-based, but the directory and file name numbers in
- the statement program are 1-based. */
- struct file_entry *fe;
- const char *dir = NULL;
-
state_machine.file = read_unsigned_leb128 (abfd, line_ptr,
&bytes_read);
line_ptr += bytes_read;
- if (state_machine.file == 0
- || state_machine.file - 1 >= lh->num_file_names)
+
+ const file_entry *fe = state_machine.current_file ();
+ if (fe == NULL)
dwarf2_debug_line_missing_file_complaint ();
else
{
- fe = &lh->file_names[state_machine.file - 1];
- if (fe->dir_index && lh->include_dirs != NULL
- && (fe->dir_index - 1) < lh->num_include_dirs)
- dir = lh->include_dirs[fe->dir_index - 1];
if (record_lines_p)
{
+ const char *dir = fe->include_dir (lh);
+
state_machine.last_subfile = current_subfile;
state_machine.line_has_non_zero_discriminator
= state_machine.discriminator != 0;
@@ -18671,21 +18694,16 @@ dwarf_decode_lines (struct line_header *lh, const char *comp_dir,
for (i = 0; i < lh->num_file_names; i++)
{
- const char *dir = NULL;
- struct file_entry *fe;
+ file_entry &fe = lh->file_names[i];
- fe = &lh->file_names[i];
- if (fe->dir_index && lh->include_dirs != NULL
- && (fe->dir_index - 1) < lh->num_include_dirs)
- dir = lh->include_dirs[fe->dir_index - 1];
- dwarf2_start_subfile (fe->name, dir);
+ dwarf2_start_subfile (fe.name, fe.include_dir (lh));
if (current_subfile->symtab == NULL)
{
current_subfile->symtab
= allocate_symtab (cust, current_subfile->name);
}
- fe->symtab = current_subfile->symtab;
+ fe.symtab = current_subfile->symtab;
}
}
}
@@ -21382,14 +21400,15 @@ file_file_name (int file, struct line_header *lh)
table? Remember that file numbers start with one, not zero. */
if (1 <= file && file <= lh->num_file_names)
{
- struct file_entry *fe = &lh->file_names[file - 1];
+ const file_entry &fe = lh->file_names[file - 1];
- if (IS_ABSOLUTE_PATH (fe->name) || fe->dir_index == 0
- || lh->include_dirs == NULL
- || (fe->dir_index - 1) >= lh->num_include_dirs)
- return xstrdup (fe->name);
- return concat (lh->include_dirs[fe->dir_index - 1], SLASH_STRING,
- fe->name, (char *) NULL);
+ if (!IS_ABSOLUTE_PATH (fe.name))
+ {
+ const char *dir = fe.include_dir (lh);
+ if (dir != NULL)
+ return concat (dir, SLASH_STRING, fe.name, (char *) NULL);
+ }
+ return xstrdup (fe.name);
}
else
{