[binutils-gdb] gdb: rename argument in valprint_check_validity

https://sourceware.org/git/gitweb.cgi?p=binutils-gdb.git;h=7d1d7386561837513ce42704e1097479ed8bacd6

commit 7d1d7386561837513ce42704e1097479ed8bacd6
Author: Andrew Burgess <[email protected]>
Date:   Fri May 8 21:42:05 2026 +0100

    gdb: rename argument in valprint_check_validity
    
    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.
    
    Approved-By: Tom Tromey <[email protected]>

Diff:
---
 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,