Re: [PATCH][PR symtab/18148] Properly intern constants into psymtab

[Andy Wingo <wingo at igalia dot com>]

Kind ping :)  Easy fix.  Adding Doug to Cc as we discussed it last week
in IRC.

A

On Fri 20 Mar 2015 12:36, Andy Wingo <wingo@igalia.com> writes:

> From 16de9f67e56b5b58ff7923db0d6505d61c766124 Mon Sep 17 00:00:00 2001
> From: Andy Wingo <wingo@igalia.com>
> Date: Fri, 20 Mar 2015 11:50:00 +0100
> Subject: [PATCH] Properly intern constants into psymtab
>
> Variables with a DW_AT_const_value but without a DW_AT_location were not
> getting added to the partial symbol table.  They are added to the full
> symbol table, however, when the compilation unit's psymtabs are
> expanded.
>
> Before:
>
>    (gdb) p one
>    No symbol "one" in current context.
>    (gdb) mt flush-symbol-cache
>    (gdb) mt expand one.c
>    (gdb) p one
>    $1 = 1
>
> After:
>
>    (gdb) p one
>    $1 = 1
>
> To the user it's pretty strange, as depending on whether tab completion
> has forced expansion of all CUs or not the lookup might succeed, or not
> if the failure was already added to the symbol cache.
>
> This commit simply makes sure to add constants to the partial symbol
> tables.
>
> gdb/testsuite/ChangeLog:
>
> 	PR symtab/18148
> 	* gdb.dwarf2/dw2-intercu.S (one, two): Add variables that have a
> 	const_value but not a location.
> 	* gdb.dwarf2/dw2-intercu.exp: Add tests that constants without
> 	location defined in non-main CUs are visible.
>
> gdb/ChangeLog:
>
> 	PR symtab/18148
> 	* dwarf2read.c (struct partial_die_info): Add has_const_value
> 	member.
> 	(add_partial_symbol): Don't punt on symbols that have const_value
> 	attributes.
> 	(read_partial_die): Detect DW_AT_const_value.
> ---
>  gdb/ChangeLog                            |  9 ++++
>  gdb/dwarf2read.c                         | 17 +++++--
>  gdb/testsuite/ChangeLog                  |  8 ++++
>  gdb/testsuite/gdb.dwarf2/dw2-intercu.S   | 79 ++++++++++++++++++++++++++++++++
>  gdb/testsuite/gdb.dwarf2/dw2-intercu.exp |  5 ++
>  5 files changed, 115 insertions(+), 3 deletions(-)
>
> diff --git a/gdb/ChangeLog b/gdb/ChangeLog
> index bd49503..a9eeaf0 100644
> --- a/gdb/ChangeLog
> +++ b/gdb/ChangeLog
> @@ -1,3 +1,12 @@
> +2015-03-20  Andy Wingo  <wingo@igalia.com>
> +
> +	PR symtab/18148
> +	* dwarf2read.c (struct partial_die_info): Add has_const_value
> +	member.
> +	(add_partial_symbol): Don't punt on symbols that have const_value
> +	attributes.
> +	(read_partial_die): Detect DW_AT_const_value.
> +
>  2015-03-05  Andy Wingo  <wingo@igalia.com>
>  
>  	* guile/scm-symbol.c (gdbscm_lookup_symbol): Don't error if there
> diff --git a/gdb/dwarf2read.c b/gdb/dwarf2read.c
> index c185d51..c98a619 100644
> --- a/gdb/dwarf2read.c
> +++ b/gdb/dwarf2read.c
> @@ -1105,6 +1105,9 @@ struct partial_die_info
>      /* Flag set if the DIE has a byte_size attribute.  */
>      unsigned int has_byte_size : 1;
>  
> +    /* Flag set if the DIE has a DW_AT_const_value attribute.  */
> +    unsigned int has_const_value : 1;
> +
>      /* Flag set if any of the DIE's children are template arguments.  */
>      unsigned int has_template_arguments : 1;
>  
> @@ -6956,19 +6959,24 @@ add_partial_symbol (struct partial_die_info *pdi, struct dwarf2_cu *cu)
>  	}
>        else
>  	{
> -	  /* Static Variable.  Skip symbols without location descriptors.  */
> -	  if (pdi->d.locdesc == NULL)
> +	  int has_loc = pdi->d.locdesc != NULL;
> +
> +	  /* Static Variable.  Skip symbols whose value we cannot know (those
> +	     without location descriptors or constant values).  */
> +	  if (!has_loc && !pdi->has_const_value)
>  	    {
>  	      xfree (built_actual_name);
>  	      return;
>  	    }
> +
>  	  /* prim_record_minimal_symbol (actual_name, addr + baseaddr,
>  	     mst_file_data, objfile); */
>  	  add_psymbol_to_list (actual_name, strlen (actual_name),
>  			       built_actual_name != NULL,
>  			       VAR_DOMAIN, LOC_STATIC,
>  			       &objfile->static_psymbols,
> -			       0, addr + baseaddr,
> +			       0,
> +			       has_loc ? addr + baseaddr : (CORE_ADDR) 0,
>  			       cu->language, objfile);
>  	}
>        break;
> @@ -15834,6 +15842,9 @@ read_partial_die (const struct die_reader_specs *reader,
>          case DW_AT_byte_size:
>            part_die->has_byte_size = 1;
>            break;
> +        case DW_AT_const_value:
> +          part_die->has_const_value = 1;
> +          break;
>  	case DW_AT_calling_convention:
>  	  /* DWARF doesn't provide a way to identify a program's source-level
>  	     entry point.  DW_AT_calling_convention attributes are only meant
> diff --git a/gdb/testsuite/ChangeLog b/gdb/testsuite/ChangeLog
> index 52a60f8..fbc4f9a 100644
> --- a/gdb/testsuite/ChangeLog
> +++ b/gdb/testsuite/ChangeLog
> @@ -1,3 +1,11 @@
> +2015-03-20  Andy Wingo  <wingo@igalia.com>
> +
> +	PR symtab/18148
> +	* gdb.dwarf2/dw2-intercu.S (one, two): Add variables that have a
> +	const_value but not a location.
> +	* gdb.dwarf2/dw2-intercu.exp: Add tests that constants without
> +	location defined in non-main CUs are visible.
> +
>  2015-03-06  Andy Wingo  <wingo@igalia.com>
>  
>  	* gdb.guile/scm-frame-unwinder.exp:
> diff --git a/gdb/testsuite/gdb.dwarf2/dw2-intercu.S b/gdb/testsuite/gdb.dwarf2/dw2-intercu.S
> index b155e0b..c033e4d 100644
> --- a/gdb/testsuite/gdb.dwarf2/dw2-intercu.S
> +++ b/gdb/testsuite/gdb.dwarf2/dw2-intercu.S
> @@ -64,6 +64,24 @@ func_cu1:
>  	.byte		1			/* DW_AT_frame_base: length */
>  	.byte		0x55			/* DW_AT_frame_base: DW_OP_reg5 */
>  
> +	/* This type is named "int1" and not "int" to ensure it is unique,
> +	   and thus we can easily ensure we expand this CU and not some
> +	   other CU with "int".  */
> +.Ltype_int1_in_cu1:
> +	.uleb128	3			/* Abbrev: DW_TAG_base_type */
> +	.ascii		"int1\0"		/* DW_AT_name */
> +	.byte		4			/* DW_AT_byte_size */
> +	.byte		5			/* DW_AT_encoding */
> +
> +.Ltype_const_int1_in_cu1:
> +	.uleb128	4			/* Abbrev: DW_TAG_const_type */
> +	.4byte		 .Ltype_int1_in_cu1-.Lcu1_begin /* DW_AT_type */
> +
> +	.uleb128	5			/* Abbrev: DW_TAG_variable */
> +	.ascii		"one\0"			/* DW_AT_name */
> +	.4byte		.Ltype_const_int1_in_cu1-.Lcu1_begin   /* DW_AT_type */
> +	.byte		1			/* DW_AT_const_value */
> +
>  	.byte		0			/* End of children of CU */
>  
>  .Lcu1_end:
> @@ -92,6 +110,15 @@ func_cu1:
>  	.byte		4			/* DW_AT_byte_size */
>  	.byte		5			/* DW_AT_encoding */
>  
> +.Ltype_const_int2_in_cu2:
> +	.uleb128	3			/* Abbrev: DW_TAG_const_type */
> +	.4byte		 .Ltype_int2_in_cu2-.Lcu2_begin /* DW_AT_type */
> +
> +	.uleb128	4			/* Abbrev: DW_TAG_variable */
> +	.ascii		"two\0"			/* DW_AT_name */
> +	.4byte		.Ltype_const_int2_in_cu2-.Lcu2_begin   /* DW_AT_type */
> +	.byte		2			/* DW_AT_const_value */
> +
>  	.byte		0			/* End of children of CU */
>  
>  .Lcu2_end:
> @@ -139,6 +166,38 @@ func_cu1:
>  	.byte		0x0			/* Terminator */
>  	.byte		0x0			/* Terminator */
>  
> +	.uleb128	3			/* Abbrev code */
> +	.uleb128	0x24			/* DW_TAG_base_type */
> +	.byte		0			/* has_children */
> +	.uleb128	0x3			/* DW_AT_name */
> +	.uleb128	0x8			/* DW_FORM_string */
> +	.uleb128	0xb			/* DW_AT_byte_size */
> +	.uleb128	0xb			/* DW_FORM_data1 */
> +	.uleb128	0x3e			/* DW_AT_encoding */
> +	.uleb128	0xb			/* DW_FORM_data1 */
> +	.byte		0x0			/* Terminator */
> +	.byte		0x0			/* Terminator */
> +
> +	.uleb128	4			/* Abbrev code */
> +	.uleb128	0x26			/* DW_TAG_const_type */
> +	.byte		0x0			/* DW_children_no */
> +	.uleb128	0x49			/* DW_AT_type */
> +	.uleb128	0x13			/* DW_FORM_ref4 */
> +	.byte		0x0			/* Terminator */
> +	.byte		0x0			/* Terminator */
> +
> +	.uleb128	5			/* Abbrev code */
> +	.uleb128	0x34			/* DW_TAG_variable */
> +	.byte		0x0			/* DW_children_no */
> +	.uleb128	0x3			/* DW_AT_name */
> +	.uleb128	0x8			/* DW_FORM_string */
> +	.uleb128	0x49			/* DW_AT_type */
> +	.uleb128	0x13			/* DW_FORM_ref4 */
> +	.uleb128	0x1c			/* DW_AT_const_value */
> +	.uleb128	0xb			/* DW_FORM_data1 */
> +	.byte		0x0			/* Terminator */
> +	.byte		0x0			/* Terminator */
> +
>  	.byte		0x0			/* Terminator */
>  	.byte		0x0			/* Terminator */
>  
> @@ -167,6 +226,26 @@ func_cu1:
>  	.byte		0x0			/* Terminator */
>  	.byte		0x0			/* Terminator */
>  
> +	.uleb128	3			/* Abbrev code */
> +	.uleb128	0x26			/* DW_TAG_const_type */
> +	.byte		0x0			/* DW_children_no */
> +	.uleb128	0x49			/* DW_AT_type */
> +	.uleb128	0x13			/* DW_FORM_ref4 */
> +	.byte		0x0			/* Terminator */
> +	.byte		0x0			/* Terminator */
> +
> +	.uleb128	4			/* Abbrev code */
> +	.uleb128	0x34			/* DW_TAG_variable */
> +	.byte		0x0			/* DW_children_no */
> +	.uleb128	0x3			/* DW_AT_name */
> +	.uleb128	0x8			/* DW_FORM_string */
> +	.uleb128	0x49			/* DW_AT_type */
> +	.uleb128	0x13			/* DW_FORM_ref4 */
> +	.uleb128	0x1c			/* DW_AT_const_value */
> +	.uleb128	0xb			/* DW_FORM_data1 */
> +	.byte		0x0			/* Terminator */
> +	.byte		0x0			/* Terminator */
> +
>  	.byte		0x0			/* Terminator */
>  	.byte		0x0			/* Terminator */
>  
> diff --git a/gdb/testsuite/gdb.dwarf2/dw2-intercu.exp b/gdb/testsuite/gdb.dwarf2/dw2-intercu.exp
> index 8de99c5..eba791b 100644
> --- a/gdb/testsuite/gdb.dwarf2/dw2-intercu.exp
> +++ b/gdb/testsuite/gdb.dwarf2/dw2-intercu.exp
> @@ -42,3 +42,8 @@ gdb_test_no_output "set listsize 1"
>  gdb_test "list func_cu1" "4\[ \t\]+File 1 Line 4"
>  
>  gdb_test "ptype func_cu1" "type = int2 \\(\\)"
> +
> +gdb_test "p one" "= 1"
> +gdb_test "p two" "= 2"
> +gdb_test "ptype one" "type = const int1"
> +gdb_test "ptype two" "type = const int2"