RE: [PATCH v6 00/15] Please have a final look

["Agovic, Sanimir" <sanimir dot agovic at intel dot com>]

Keith, Joel, all, sorry for causing any inconvenience and thanks to
you both for taking care of the issue on Friday.

The root cause was this code fragment below. It was in my repo and I
git-amend it accidently as I was playing around with an alternative to
solve the pointer-to-vla issue.

@@ -2985,7 +2984,6 @@ evaluate_subexp_with_coercion (struct expression *exp,
        {
          (*pos) += 4;
          val = address_of_variable (var, exp->elts[pc + 1].block);
-         type = value_type (val);
          return value_cast (lookup_pointer_type (TYPE_TARGET_TYPE (type)),
                             val);

In addition I refactored the code for handling the sizeof operator to
match Keith latest fix for PR c++/16675 (245a5f0) in:

  #02 vla: enable sizeof operator to work with variable length arrays

I used Jans diffgdb script to compare two runs (pre-/post vla) of 'make
check' and now the patch series is free of regression as it should be.

You will find the latest patch series at:

  https://github.com/intel-gdb/vla/tree/vla-c99

Apologize and thanks again.

 -Sanimir

> -----Original Message-----
> From: Joel Brobecker [mailto:brobecker@adacore.com]
> Sent: Saturday, April 12, 2014 12:33 AM
> To: Keith Seitz
> Cc: Agovic, Sanimir; gdb-patches@sourceware.org
> Subject: Re: [PATCH v6 00/15] Please have a final look
> 
> > You should do whatever is convenient for you, and I will work around it.
> > No reason to inconvenience you further! So, if you are ready, just
> > go ahead and push. I will review the errors and decide from there
> > whether we can wait until early next week to fix them or else if
> > we should revert now.
> 
> It's not extremely bad in the sense that perhaps it's a question
> of check_typedefs, but it triggers some assertions in gdb.ada,
> and some really strange behavior in the other testsuite sections.
> 
> I've now reverted most of the series. Attached is the patch.
> The commit subject says "Revert the entire VLA series", but
> in fact, it's not accurate. I just realized that I didn't need
> to revert the first couple of patches which are preparation work,
> and could stay. I fixed that, but forgot to change the subject
> of the commit :-( - just trying to run and I'm already an hour late.
> 
> I will get back to this on Monday.
> 
> ----
> 
> This reverts the following patch series, as they cause some regresssions.
> 
> commit 37c1ab67a35025d37d42c449deab5f254f9f59da
> type: add c99 variable length array support
> 
> 	gdb/
> 	* dwarf2loc.c (dwarf2_locexpr_baton_eval): New function.
> 	(dwarf2_evaluate_property): New function.
> 	* dwarf2loc.h (dwarf2_evaluate_property): New function prototype.
> 	* dwarf2read.c (attr_to_dynamic_prop): New function.
> 	(read_subrange_type): Use attr_to_dynamic_prop to read high bound
> 	attribute.
> 	* gdbtypes.c: Include dwarf2loc.h.
> 	(is_dynamic_type): New function.
> 	(resolve_dynamic_type): New function.
> 	(resolve_dynamic_bounds): New function.
> 	(get_type_length): New function.
> 	(check_typedef): Use get_type_length to compute type length.
> 	* gdbtypes.h (TYPE_HIGH_BOUND_KIND): New macro.
> 	(TYPE_LOW_BOUND_KIND): New macro.
> 	(is_dynamic_type): New function prototype.
> 	* value.c (value_from_contents_and_address): Call resolve_dynamic_type
> 	to resolve dynamic properties of the type. Update comment.
> 	* valops.c (get_value_at, value_at, value_at_lazy): Update comment.
> 
> commit 26cb189f8b46dbe7b2d485525329a8919005ca8a
> vla: enable sizeof operator to work with variable length arrays
> 
> 	gdb/
> 	* eval.c (evaluate_subexp_for_sizeof) <OP_VAR_VALUE>: If the type
> 	passed to sizeof is dynamic evaluate the argument to compute the length.
> 
> commit 04b19544ef6a97b62b2cc4a3170b900e046ab185
> vla: enable sizeof operator for indirection
> 
> 	gdb/
> 	* eval.c (evaluate_subexp_for_sizeof) <UNOP_IND>: Create an indirect
> 	value and retrieve the dynamic type size.
> 
> commit bcd629a44fff61527430f353cf77e20fe3afc395
> vla: update type from newly created value
> 
> 	gdb/
> 	* ada-lang.c (ada_value_primitive_packed_val): Re-fetch type from value.
> 	(ada_template_to_fixed_record_type_1): Likewise.
> 	(ada_to_fixed_type_1): Likewise.
> 	* cp-valprint.c (cp_print_value_fields_rtti): Likewise.
> 	(cp_print_value): Likewise.
> 	* d-valprint.c (dynamic_array_type): Likewise.
> 	* eval.c (evaluate_subexp_with_coercion): Likewise.
> 	* findvar.c (address_of_variable): Likewise.
> 	* jv-valprint.c (java_value_print): Likewise.
> 	* valops.c (value_ind): Likewise.
> 	* value.c (coerce_ref): Likewise.
> 
> commit b86138fb0484f42db6cb83abed1e3d0ad2ec4eac
> vla: print "variable length" for unresolved dynamic bounds
> 
> 	gdb/
> 	* c-typeprint.c (c_type_print_varspec_suffix): Added
> 	check for not yet resolved high bound. If unresolved, print
> 	"variable length" string to the console instead of random
> 	length.
> 
> commit e1969afbd454c09c3aad1990305715f70bc47c3c
> vla: support for DW_AT_count
> 
> 	gdb/
> 	* dwarf2read.c (read_subrange_type): Convert DW_AT_count to a dynamic
> 	property and store it as the high bound and flag the range accordingly.
> 	* gdbtypes.c (resolve_dynamic_bounds): If range is flagged as
> 	RANGE_UPPER_BOUND_IS_COUNT assign low + high - 1 as the new high bound.
> 	* gdbtypes.h (enum range_flags): New enum.
> 	(struct range_bounds): Add flags member.
> 
> commit 92b09522dc5a93ba4bda3c1c0b3c58264e357c8a
> vla: resolve dynamic bounds if value contents is a constant byte-sequence
> 
> 	gdb/
> 	* findvar.c (default_read_var_value): Resolve dynamic bounds if location
> 	points to a constant blob.
> 
> commit 3bce82377f683870cc89925ff43aefb7dcce4a77
> vla: evaluate operand of sizeof if its type is a vla
> 
> 	gdb/
> 	* eval.c (evaluate_subexp_for_sizeof): Add enum noside argument.
> 	(evaluate_subexp_standard): Pass noside argument.
> 	(evaluate_subexp_for_sizeof) <BINOP_SUBSCRIPT>: Handle subscript case
> 	if noside equals EVAL_NORMAL. If the subscript yields a vla type
> 	re-evaluate subscript operation with EVAL_NORMAL to enable sideffects.
> 	* gdbtypes.c (resolve_dynamic_bounds): Mark bound as evaluated.
> 	* gdbtypes.h (enum range_flags): Add RANGE_EVALUATED case.
> 
> 	gdb/testsuite
> 
> 	* gdb.base/vla-sideeffect.c: New file.
> 	* gdb.base/vla-sideeffect.exp: New file.
> 
> commit 504f34326e5ae7c78ebfcdd6ed03c7403b42048b
> test: cover subranges with present DW_AT_count attribute
> 
> 	gdb/testsuite/
> 	* gdb.dwarf2/count.exp: New file.
> 
> commit 1a237e0ee53bbdee97d72d794b5b42e774cc81c0
> test: multi-dimensional c99 vla.
> 
> 	gdb/testsuite/
> 	* gdb.base/vla-multi.c: New file.
> 	* gdb.base/vla-multi.exp: New file.
> 
> commit 024e13b46f9c33d151ae82fd9d64c53092fd9313
> test: evaluate pointers to C99 vla correctly.
> 
> 	gdb/testsuite/
> 	* gdb.base/vla-ptr.c: New file.
> 	* gdb.base/vla-ptr.exp: New file.
> 
> commit c8655f75e2f0fada311be193e3090087a77ec802
> test: basic c99 vla tests for C primitives
> 
> 	gdb/testsuite/
> 	* gdb.base/vla-datatypes.c: New file.
> 	* gdb.base/vla-datatypes.exp: New file.
> 
> commit 58a84dcf29b735ee776536b4c51ba90b51612b71
> test: add mi vla test
> 
> 	gdb/testsuite/
> 	* gdb.mi/mi-vla-c99.exp: New file.
> 	* gdb.mi/vla.c: New file.
> 
> --
> Joel
Intel GmbH
Dornacher Strasse 1
85622 Feldkirchen/Muenchen, Deutschland
Sitz der Gesellschaft: Feldkirchen bei Muenchen
Geschaeftsfuehrer: Christian Lamprechter, Hannes Schwaderer, Douglas Lusk
Registergericht: Muenchen HRB 47456
Ust.-IdNr./VAT Registration No.: DE129385895
Citibank Frankfurt a.M. (BLZ 502 109 00) 600119052