[PATCH 4/6] gdb: rename argument in valprint_check_validity

[Andrew Burgess <aburgess@redhat.com>]

To understand the motivation for this commit you'll need to look ahead
two commits in this series for an upcoming bug fix.

Look at coerce_pieced_ref in dwarf2/expr.c, and this call:

  if (value->bits_synthetic_pointer (value->embedded_offset (),
                                     TARGET_CHAR_BIT * type->length ()))
    { ... }

This passes the result from value::embedded_offset as the first argument.
Though it's not clear from value.h, the number returned by
value::embedded_offset is a byte offset.

The value::bits_synthetic_pointer call can end up calling
check_pieced_synthetic_pointer, with the first argument to
value::bits_synthetic_pointer becoming the second argument to
check_pieced_synthetic_pointer.  The second argument to
check_pieced_synthetic_pointer is called BIT_OFFSET, a value in bits.
Looking at how this argument is used confirms that it is expected to be
a value in bits, not bytes.

The fix should be easy, multiply the embedded offset by
TARGET_CHAR_BIT, but I think things are a little more complex than
this.

I started looking at how value::bits_synthetic_pointer is used, there
are only 7 uses, and they can be grouped as follows:

gdb/dwarf2/expr.c
gdb/valops.c

  These call value::embedded_offset directly within the call to
  value::bits_synthetic_pointer.

gdb/opencl-lang.c

  This function is used as a wrapper that implements a
  lval_funcs::check_synthetic_pointer callback, it adjusts the
  arguments and then calls value::bits_synthetic_pointer.  As such we'd
  not expect to see embedded offset mentioned here as that would (we
  assume) be handled by the caller.

gdb/valprint.c (in generic_val_print_ref)

  This call passes an argument called 'embedded_offset', but the
  function generic_val_print_ref is only used in one place, and the
  embedded_offset is always passed as zero.

gdb/valprint.c (in valprint_check_validity)

  This call also passes an argument called 'embedded_offset'.  This
  function is used in two places.  In common_val_print the
  embedded_offset is always passed as zero, but in
  valprint_check_validity (in cp-valprint.c) the embedded_offset being
  passed is the offset of a field within the value, not the value's
  embedded_offset within some larger value.

gdb/cp-valprint.c
gdb/p-valprint.c

  As with the call to valprint_check_validity in cp-valprint.c, these
  two direct calls to value::bits_synthetic_pointer pass the offset of a
  field within the value, rather than the value's embedded_offset.

What I see in the above is some confusion.  In some places we are
passing the value::embedded_offset, while in other places we are
passing the offset of a field within the value itself.

If we consider the direct call to value::bits_synthetic_pointer in
gdb/cp-valprint.c, where a field offset is passed, then it should be
possible, that if we can create an object with a non-zero
embedded_offset (which isn't accounted for in this code path), then we
should see some bugs in GDB, and indeed, this is what I do see.

My plan for fixing this is to have the offset passed to
value::bits_synthetic_pointer always be a field offset within the value,
the value::embedded_offset will then be handled directly within the
various value::bits_synthetic_pointer implementations.

This commit is a small refactor in preparation for this fix.

I believe part of the confusion here is that we have functions that
take arguments called embedded_offset, when the value they should
accept is no longer the embedded offset.

So in this commit I propose renaming the embedded_offset argument to
field_byte_offset in valprint_check_validity.  This is purely a
mechanical rename, there should be no user visible changes after this
commit.
---
 gdb/valprint.c | 8 ++++----
 gdb/valprint.h | 4 ++--
 2 files changed, 6 insertions(+), 6 deletions(-)

diff --git a/gdb/valprint.c b/gdb/valprint.c
index 9801f195d13..ca4a064a7c5 100644
--- a/gdb/valprint.c
+++ b/gdb/valprint.c
@@ -318,7 +318,7 @@ val_print_scalar_or_string_type_p (struct type *type,
 bool
 valprint_check_validity (struct ui_file *stream,
 			 struct type *type,
-			 LONGEST embedded_offset,
+			 LONGEST field_byte_offset,
 			 const struct value *val)
 {
   type = check_typedef (type);
@@ -339,14 +339,14 @@ valprint_check_validity (struct ui_file *stream,
       && type->code () != TYPE_CODE_STRUCT
       && type->code () != TYPE_CODE_ARRAY)
     {
-      if (val->bits_any_optimized_out (TARGET_CHAR_BIT * embedded_offset,
+      if (val->bits_any_optimized_out (TARGET_CHAR_BIT * field_byte_offset,
 				       TARGET_CHAR_BIT * type->length ()))
 	{
 	  val_print_optimized_out (val, stream);
 	  return false;
 	}
 
-      if (val->bits_synthetic_pointer (TARGET_CHAR_BIT * embedded_offset,
+      if (val->bits_synthetic_pointer (TARGET_CHAR_BIT * field_byte_offset,
 				       TARGET_CHAR_BIT * type->length ()))
 	{
 	  const bool is_ref = type->code () == TYPE_CODE_REF;
@@ -368,7 +368,7 @@ valprint_check_validity (struct ui_file *stream,
 	  return is_ref;
 	}
 
-      if (!val->bytes_available (embedded_offset, type->length ()))
+      if (!val->bytes_available (field_byte_offset, type->length ()))
 	{
 	  val_print_unavailable (stream);
 	  return false;
diff --git a/gdb/valprint.h b/gdb/valprint.h
index c8f4bd52286..146a24d174f 100644
--- a/gdb/valprint.h
+++ b/gdb/valprint.h
@@ -199,12 +199,12 @@ extern void print_function_pointer_address (const struct value_print_options *op
 
    If TYPE represents some aggregate type (e.g., a structure), return true.
 
-   For non-aggregate TYPEs, if any of the bytes starting at EMBEDDED_OFFSET
+   For non-aggregate TYPEs, if any of the bytes starting at FIELD_BYTE_OFFSET
    and extending for TYPE->length () bytes are invalid, print a message to
    STREAM and return false.  Otherwise, return true.  */
 
 extern bool valprint_check_validity (struct ui_file *stream, struct type *type,
-				     LONGEST embedded_offset,
+				     LONGEST field_byte_offset,
 				     const struct value *val);
 
 extern void val_print_optimized_out (const struct value *val,
-- 
2.25.4