[patch] Fix gdb.fortran/common-block.exp crash in PIE mode

[patch] Fix gdb.fortran/common-block.exp crash in PIE mode

[Jan Kratochvil]

Hi Tom,

runtest F90_FOR_TARGET="gfortran -fPIE -pie" gdb.fortran/common-block.exp

crashes GDB as function relocate_one_symbol
	if ((SYMBOL_CLASS (sym) == LOC_LABEL
	     || SYMBOL_CLASS (sym) == LOC_STATIC)
	    && SYMBOL_SECTION (sym) >= 0)
	  SYMBOL_VALUE_ADDRESS (sym) += ANOFFSET (delta, SYMBOL_SECTION (sym));
corrupts SYMBOL_VALUE_COMMON_BLOCK
	struct common_block *common_block;
as it thinks it can update it like SYMBOL_VALUE_ADDRESS
	CORE_ADDR address;
due to its LOC_STATIC.

No regressions on {x86_64,x86_64-m32,i686}-fedora18-linux-gnu and in PIE mode.


Thanks,
Jan


gdb/
2013-01-19  Jan Kratochvil  <jan.kratochvil@redhat.com>

	Fix gdb.fortran/common-block.exp crash in PIE mode.
	* dwarf2read.c (new_symbol_full) <DW_TAG_common_block>: Use
	LOC_COMMON_BLOCK.
	* f-valprint.c (info_common_command_for_block): Expect
	LOC_COMMON_BLOCK in gdb_assert.
	* symtab.h (struct general_symbol_info): Update comment for the
	common_block member.
	(domain_enum): Extend comment for the COMMON_BLOCK_DOMAIN member.
	(enum address_class): New member LOC_COMMON_BLOCK.

diff --git a/gdb/dwarf2read.c b/gdb/dwarf2read.c
index 7a58c45..364e6af 100644
--- a/gdb/dwarf2read.c
+++ b/gdb/dwarf2read.c
@@ -16071,7 +16071,7 @@ new_symbol_full (struct die_info *die, struct type *type, struct dwarf2_cu *cu,
 	  list_to_add = &global_symbols;
 	  break;
 	case DW_TAG_common_block:
-	  SYMBOL_CLASS (sym) = LOC_STATIC;
+	  SYMBOL_CLASS (sym) = LOC_COMMON_BLOCK;
 	  SYMBOL_DOMAIN (sym) = COMMON_BLOCK_DOMAIN;
 	  add_symbol_to_list (sym, cu->list_in_scope);
 	  break;
diff --git a/gdb/f-valprint.c b/gdb/f-valprint.c
index 22cca83..d01d6ec 100644
--- a/gdb/f-valprint.c
+++ b/gdb/f-valprint.c
@@ -427,7 +427,7 @@ info_common_command_for_block (struct block *block, const char *comname,
 	struct common_block *common = SYMBOL_VALUE_COMMON_BLOCK (sym);
 	size_t index;
 
-	gdb_assert (SYMBOL_CLASS (sym) == LOC_STATIC);
+	gdb_assert (SYMBOL_CLASS (sym) == LOC_COMMON_BLOCK);
 
 	if (comname && (!SYMBOL_LINKAGE_NAME (sym)
 	                || strcmp (comname, SYMBOL_LINKAGE_NAME (sym)) != 0))
diff --git a/gdb/symtab.h b/gdb/symtab.h
index c334a3a..b992266 100644
--- a/gdb/symtab.h
+++ b/gdb/symtab.h
@@ -120,7 +120,7 @@ struct general_symbol_info
 
     CORE_ADDR address;
 
-    /* A common block.  Used with COMMON_BLOCK_DOMAIN.  */
+    /* A common block.  Used with LOC_COMMON_BLOCK.  */
 
     struct common_block *common_block;
 
@@ -414,7 +414,8 @@ typedef enum domain_enum_tag
 
   LABEL_DOMAIN,
 
-  /* Fortran common blocks.  Their naming must be separate from VAR_DOMAIN.  */
+  /* Fortran common blocks.  Their naming must be separate from VAR_DOMAIN.
+     They also always use LOC_COMMON_BLOCK.  */
   COMMON_BLOCK_DOMAIN
 } domain_enum;
 
@@ -533,6 +534,10 @@ enum address_class
   /* The variable's address is computed by a set of location
      functions (see "struct symbol_computed_ops" below).  */
   LOC_COMPUTED,
+
+  /* The variable uses general_symbol_info->value->common_block field.
+     It also always uses COMMON_BLOCK_DOMAIN.  */
+  LOC_COMMON_BLOCK,
 };
 
 /* The methods needed to implement LOC_COMPUTED.  These methods can

Re: [patch] Fix gdb.fortran/common-block.exp crash in PIE mode

[Tom Tromey]

>>>>> "Jan" == Jan Kratochvil <jan.kratochvil@redhat.com> writes:

Jan> as it thinks it can update it like SYMBOL_VALUE_ADDRESS
Jan> 	CORE_ADDR address;
Jan> due to its LOC_STATIC.

Jan> 	Fix gdb.fortran/common-block.exp crash in PIE mode.
Jan> 	* dwarf2read.c (new_symbol_full) <DW_TAG_common_block>: Use
Jan> 	LOC_COMMON_BLOCK.
Jan> 	* f-valprint.c (info_common_command_for_block): Expect
Jan> 	LOC_COMMON_BLOCK in gdb_assert.
Jan> 	* symtab.h (struct general_symbol_info): Update comment for the
Jan> 	common_block member.
Jan> 	(domain_enum): Extend comment for the COMMON_BLOCK_DOMAIN member.
Jan> 	(enum address_class): New member LOC_COMMON_BLOCK.

Looks good to me.

Perhaps the various symbol-value accessors like SYMBOL_VALUE_ADDRESS
should make assertions about the address class.  This is what GCC does
in its tree accessors.

Like

#define SYMBOL_VALUE_COMMON_BLOCK(symbol) \
  ((gdb_assert (SYMBOL_CLASS (symbol) == LOC_COMMON_BLOCK)), \
   (symbol)->info.value.common_block)

Tom

[commit] [patch] Fix gdb.fortran/common-block.exp crash in PIE mode

[Jan Kratochvil]

On Mon, 21 Jan 2013 17:12:49 +0100, Tom Tromey wrote:
> >>>>> "Jan" == Jan Kratochvil <jan.kratochvil@redhat.com> writes:
> Jan> 	Fix gdb.fortran/common-block.exp crash in PIE mode.
> Jan> 	* dwarf2read.c (new_symbol_full) <DW_TAG_common_block>: Use
> Jan> 	LOC_COMMON_BLOCK.
> Jan> 	* f-valprint.c (info_common_command_for_block): Expect
> Jan> 	LOC_COMMON_BLOCK in gdb_assert.
> Jan> 	* symtab.h (struct general_symbol_info): Update comment for the
> Jan> 	common_block member.
> Jan> 	(domain_enum): Extend comment for the COMMON_BLOCK_DOMAIN member.
> Jan> 	(enum address_class): New member LOC_COMMON_BLOCK.
> 
> Looks good to me.

Checked in:
	http://sourceware.org/ml/gdb-cvs/2013-01/msg00123.html


> Perhaps the various symbol-value accessors like SYMBOL_VALUE_ADDRESS
> should make assertions about the address class.  This is what GCC does
> in its tree accessors.

OK, I see no need for ({...}), I will post something later.


Thanks,
Jan

Re: [patch] Fix gdb.fortran/common-block.exp crash in PIE mode

[Jan Kratochvil]

On Mon, 21 Jan 2013 17:12:49 +0100, Tom Tromey wrote:
> Perhaps the various symbol-value accessors like SYMBOL_VALUE_ADDRESS
> should make assertions about the address class.  This is what GCC does
> in its tree accessors.
> 
> Like
> 
> #define SYMBOL_VALUE_COMMON_BLOCK(symbol) \
>   ((gdb_assert (SYMBOL_CLASS (symbol) == LOC_COMMON_BLOCK)), \
>    (symbol)->info.value.common_block)

Unfortunately these macros are used also for minimal_symbol which does not
have SYMBOL_CLASS.

This is another sanity check which can be implemented after -Wc++-compat gets
checked in.


Jan

Re: [patch] Fix gdb.fortran/common-block.exp crash in PIE mode

[Tom Tromey]

>>>>> "Jan" == Jan Kratochvil <jan.kratochvil@redhat.com> writes:

>> Perhaps the various symbol-value accessors like SYMBOL_VALUE_ADDRESS
>> should make assertions about the address class.  This is what GCC does
>> in its tree accessors.
>> 
>> Like
>> 
>> #define SYMBOL_VALUE_COMMON_BLOCK(symbol) \
>> ((gdb_assert (SYMBOL_CLASS (symbol) == LOC_COMMON_BLOCK)), \
>> (symbol)->info.value.common_block)

Jan> Unfortunately these macros are used also for minimal_symbol which
Jan> does not have SYMBOL_CLASS.

Another reason to stop sharing these macros across different types of
symbols.  Someday I suppose I'll write up a full patch for that instead
of doing it piecemeal as needed.

Tom