For the following c program:
main(int argc)
{
typedef
struct {
int before_name;
char name[argc];
int after_name;
}
namet;
namet n;
}
compiled with gcc 4.7.4, the trunk --read-var-info=yes gives:
parse_type_DIE: confused by:
<2><51>: DW_TAG_structure_type
DW_AT_decl_file : 1
DW_AT_decl_line : 4
DW_AT_sibling : <83>
This is because that dwarf entry defines a struct with no size.
This happens when the struct has a VLA array in the middle
of a struct. This is a C gcc extension, and is a standard
feature of Ada.
The proper solution would be to have the size calculated at runtime,
using the gnat extensions or dwarf entries (to be generated by
the compiler).
The patch fixes this problem by defining the size of such structure
as 1 byte.
Another approach tried was to put the max possible size.
This had the disadvantage that any address on the stack was seen
as belonging to this variable.
This allows the description to work for the 1st byte of the variable
but cannot properly describe the 2nd and following bytes :
(gdb) p &n
$9 = (namet *) 0xbefbc070
(gdb) mo c d 0xbefbc070
Address 0xBEFBC070 len 1 not defined:
Uninitialised value at 0xBEFBC070
==1396== Location 0xbefbc070 is 0 bytes inside n.before_name,
==1396== declared at crec.c:10, in frame #0 of thread 1
(gdb) mo c d 0xbefbc071
Address 0xBEFBC071 len 1 not defined:
Uninitialised value at 0xBEFBC071
==1396== Address 0xbefbc071 is on thread 1's stack
(gdb)
A possible refinement would be to use a huge size but have the
logic of variable description understanding this and describing
all between this var and hte next var on the stack as being
in the VLA variable.
In the meantime, the size 1 avoids --read-var-info=yes to fail.
Also, the 'goto bad_DIE' have been replaced by a macro
goto_bad_DIE that ensures the line nr at which the bad DIE has
been detected is reported in the error msg.
This makes it easier to understand what is the problem.
git-svn-id: svn://svn.valgrind.org/valgrind/trunk@13938 a5019735-40e9-0310-863c-91ae7b9d1cf9
diff --git a/NEWS b/NEWS
index f4a7243..8accb8d 100644
--- a/NEWS
+++ b/NEWS
@@ -44,6 +44,9 @@
by increasing the value.
See user manual for details.
+* Support for --read-var-info=yes has been improved
+ to handle Ada and C struct containing VLA.
+
* ==================== FIXED BUGS ====================
The following bugs have been fixed or resolved. Note that "n-i-bz"
diff --git a/coregrind/m_debuginfo/readdwarf3.c b/coregrind/m_debuginfo/readdwarf3.c
index 1ca7c91..0052d33 100644
--- a/coregrind/m_debuginfo/readdwarf3.c
+++ b/coregrind/m_debuginfo/readdwarf3.c
@@ -1638,6 +1638,12 @@
/* We're done! The rest of it is not interesting. */
}
+__attribute__((noinline))
+static void bad_DIE_confusion(int linenr)
+{
+ VG_(printf)("\nparse_var_DIE(%d): confused by:\n", linenr);
+}
+#define goto_bad_DIE do {bad_DIE_confusion(__LINE__); goto bad_DIE;} while (0)
__attribute__((noinline))
static void parse_var_DIE (
@@ -1770,7 +1776,7 @@
} else {
if (0) VG_(printf)("I got hlo %d hhi1 %d hrange %d\n",
(Int)have_lo, (Int)have_hi1, (Int)have_range);
- goto bad_DIE;
+ goto_bad_DIE;
}
}
@@ -1856,7 +1862,7 @@
*/
/* Ignore (seems safe than pushing a single byte range) */
} else
- goto bad_DIE;
+ goto_bad_DIE;
}
if (dtag == DW_TAG_variable || dtag == DW_TAG_formal_parameter) {
@@ -1969,7 +1975,7 @@
"warning: parse_var_DIE: non-global variable "
"outside DW_TAG_subprogram\n");
}
- /* goto bad_DIE; */
+ /* goto_bad_DIE; */
/* This seems to happen a lot. Just ignore it -- if,
when we come to evaluation of the location (guarded)
expression, it requires a frame base value, and
@@ -2123,7 +2129,6 @@
bad_DIE:
set_position_of_Cursor( c_die, saved_die_c_offset );
set_position_of_Cursor( c_abbv, saved_abbv_c_offset );
- VG_(printf)("\nparse_var_DIE: confused by:\n");
posn = uncook_die( cc, posn, &debug_types_flag, &alt_flag );
VG_(printf)(" <%d><%lx>: %s", level, posn, ML_(pp_DW_TAG)( dtag ) );
if (debug_types_flag) {
@@ -2339,7 +2344,7 @@
if (attr != DW_AT_language)
continue;
if (cts.szB <= 0)
- goto bad_DIE;
+ goto_bad_DIE;
switch (cts.u.val) {
case DW_LANG_C89: case DW_LANG_C:
case DW_LANG_C_plus_plus: case DW_LANG_ObjC:
@@ -2359,7 +2364,7 @@
case DW_LANG_Mips_Assembler:
parser->language = '?'; break;
default:
- goto bad_DIE;
+ goto_bad_DIE;
}
}
}
@@ -2397,7 +2402,7 @@
case DW_ATE_complex_float:
typeE.Te.TyBase.enc = 'C'; break;
default:
- goto bad_DIE;
+ goto_bad_DIE;
}
}
}
@@ -2420,7 +2425,7 @@
&& typeE.Te.TyBase.enc != 'S'
&& typeE.Te.TyBase.enc != 'F'
&& typeE.Te.TyBase.enc != 'C'))
- goto bad_DIE;
+ goto_bad_DIE;
/* Last minute hack: if we see this
<1><515>: DW_TAG_base_type
DW_AT_byte_size : 0
@@ -2493,7 +2498,7 @@
}
/* Do we have something that looks sane? */
if (typeE.Te.TyPorR.szB != sizeof(UWord))
- goto bad_DIE;
+ goto_bad_DIE;
else
goto acquire_Type;
}
@@ -2545,7 +2550,7 @@
that will put such an enumeration_type into a .debug_types
unit which should only contain complete types.) */
&& (parser->language != 'A' && !is_decl)) {
- goto bad_DIE;
+ goto_bad_DIE;
}
/* On't stack! */
@@ -2604,13 +2609,13 @@
}
/* Do we have something that looks sane? */
if (atomE.Te.Atom.name == NULL)
- goto bad_DIE;
+ goto_bad_DIE;
/* Do we have a plausible parent? */
- if (typestack_is_empty(parser)) goto bad_DIE;
+ if (typestack_is_empty(parser)) goto_bad_DIE;
vg_assert(ML_(TyEnt__is_type)(&parser->qparentE[parser->sp]));
vg_assert(parser->qparentE[parser->sp].cuOff != D3_INVALID_CUOFF);
- if (level != parser->qlevel[parser->sp]+1) goto bad_DIE;
- if (parser->qparentE[parser->sp].tag != Te_TyEnum) goto bad_DIE;
+ if (level != parser->qlevel[parser->sp]+1) goto_bad_DIE;
+ if (parser->qparentE[parser->sp].tag != Te_TyEnum) goto_bad_DIE;
/* Record this child in the parent */
vg_assert(parser->qparentE[parser->sp].Te.TyEnum.atomRs);
VG_(addToXA)( parser->qparentE[parser->sp].Te.TyEnum.atomRs,
@@ -2693,16 +2698,25 @@
}
if ((!is_decl) /* && (!is_spec) */) {
/* this is the common, ordinary case */
- if ((!have_szB) /* we must know the size */
- /* But the name can be present, or not */)
- goto bad_DIE;
+ /* The name can be present, or not */
+ if (!have_szB) {
+ /* We must know the size.
+ But in Ada, record with discriminants might have no size.
+ But in C, VLA in the middle of a struct (gcc extension)
+ might have no size.
+ Instead, some GNAT dwarf extensions and/or dwarf entries
+ allow to calculate the struct size at runtime.
+ We cannot do that (yet?) so, the temporary kludge is to use
+ a small size. */
+ typeE.Te.TyStOrUn.szB = 1;
+ }
/* On't stack! */
typestack_push( cc, parser, td3, &typeE, level );
goto acquire_Type;
}
else {
/* don't know how to handle any other variants just now */
- goto bad_DIE;
+ goto_bad_DIE;
}
}
@@ -2747,11 +2761,11 @@
}
}
/* Do we have a plausible parent? */
- if (typestack_is_empty(parser)) goto bad_DIE;
+ if (typestack_is_empty(parser)) goto_bad_DIE;
vg_assert(ML_(TyEnt__is_type)(&parser->qparentE[parser->sp]));
vg_assert(parser->qparentE[parser->sp].cuOff != D3_INVALID_CUOFF);
- if (level != parser->qlevel[parser->sp]+1) goto bad_DIE;
- if (parser->qparentE[parser->sp].tag != Te_TyStOrUn) goto bad_DIE;
+ if (level != parser->qlevel[parser->sp]+1) goto_bad_DIE;
+ if (parser->qparentE[parser->sp].tag != Te_TyStOrUn) goto_bad_DIE;
/* Do we have something that looks sane? If this a member of a
struct, we must have a location expression; but if a member
of a union that is irrelevant (D3 spec sec 5.6.6). We ought
@@ -2766,7 +2780,7 @@
"<anon_field>" );
vg_assert(fieldE.Te.Field.name);
if (fieldE.Te.Field.typeR == D3_INVALID_CUOFF)
- goto bad_DIE;
+ goto_bad_DIE;
if (fieldE.Te.Field.nLoc) {
if (!parent_is_struct) {
/* If this is a union type, pretend we haven't seen the data
@@ -2813,7 +2827,7 @@
}
}
if (typeE.Te.TyArray.typeR == D3_INVALID_CUOFF)
- goto bad_DIE;
+ goto_bad_DIE;
/* On't stack! */
typestack_push( cc, parser, td3, &typeE, level );
goto acquire_Type;
@@ -2863,11 +2877,11 @@
(not being used to describe the bounds of a containing array
type) */
/* Do we have a plausible parent? */
- if (typestack_is_empty(parser)) goto bad_DIE;
+ if (typestack_is_empty(parser)) goto_bad_DIE;
vg_assert(ML_(TyEnt__is_type)(&parser->qparentE[parser->sp]));
vg_assert(parser->qparentE[parser->sp].cuOff != D3_INVALID_CUOFF);
- if (level != parser->qlevel[parser->sp]+1) goto bad_DIE;
- if (parser->qparentE[parser->sp].tag != Te_TyArray) goto bad_DIE;
+ if (level != parser->qlevel[parser->sp]+1) goto_bad_DIE;
+ if (parser->qparentE[parser->sp].tag != Te_TyArray) goto_bad_DIE;
/* Figure out if we have a definite range or not */
if (have_lower && have_upper && (!have_count)) {
@@ -2895,7 +2909,7 @@
boundE.Te.Bound.boundU = 0;
} else {
/* FIXME: handle more cases */
- goto bad_DIE;
+ goto_bad_DIE;
}
/* Record this bound in the parent */
@@ -2982,7 +2996,7 @@
if (have_ty == 1 || have_ty == 0)
goto acquire_Type;
else
- goto bad_DIE;
+ goto_bad_DIE;
}
/*
@@ -3051,7 +3065,6 @@
bad_DIE:
set_position_of_Cursor( c_die, saved_die_c_offset );
set_position_of_Cursor( c_abbv, saved_abbv_c_offset );
- VG_(printf)("\nparse_type_DIE: confused by:\n");
posn = uncook_die( cc, posn, &debug_types_flag, &alt_flag );
VG_(printf)(" <%d><%lx>: %s", level, posn, ML_(pp_DW_TAG)( dtag ) );
if (debug_types_flag) {