public inbox for gdb-prs@sourceware.org
help / color / mirror / Atom feed
* [Bug exp/26875] New: Incorrect value printed for address of first element of zero-length array
@ 2020-11-13  9:32 vries at gcc dot gnu.org
  2020-11-13 10:33 ` [Bug exp/26875] " vries at gcc dot gnu.org
                   ` (16 more replies)
  0 siblings, 17 replies; 18+ messages in thread
From: vries at gcc dot gnu.org @ 2020-11-13  9:32 UTC (permalink / raw)
  To: gdb-prs

https://sourceware.org/bugzilla/show_bug.cgi?id=26875

            Bug ID: 26875
           Summary: Incorrect value printed for address of first element
                    of zero-length array
           Product: gdb
           Version: unknown
            Status: NEW
          Severity: normal
          Priority: P2
         Component: exp
          Assignee: unassigned at sourceware dot org
          Reporter: vries at gcc dot gnu.org
  Target Milestone: ---

Consider test-case gdb.base/arrayidx.exp, augmented with fedora patch
https://src.fedoraproject.org/rpms/gdb/blob/master/f/gdb-archer-vla-tests.patch:
...
diff --git a/gdb/testsuite/gdb.base/arrayidx.c
b/gdb/testsuite/gdb.base/arrayidx.c
--- a/gdb/testsuite/gdb.base/arrayidx.c
+++ b/gdb/testsuite/gdb.base/arrayidx.c
@@ -17,6 +17,13 @@

 int array[] = {1, 2, 3, 4};

+#ifdef __GNUC__
+struct
+  {
+    int a[0];
+  } unbound;
+#endif
+
 int
 main (void)
 {
diff --git a/gdb/testsuite/gdb.base/arrayidx.exp
b/gdb/testsuite/gdb.base/arrayidx.exp
--- a/gdb/testsuite/gdb.base/arrayidx.exp
+++ b/gdb/testsuite/gdb.base/arrayidx.exp
@@ -49,4 +49,12 @@ gdb_test "print array" \
          "\\{\\\[0\\\] = 1, \\\[1\\\] = 2, \\\[2\\\] = 3, \\\[3\\\] = 4\\}" \
          "print array with array-indexes on"

-
+set test "p unbound.a == &unbound.a\[0\]"
+gdb_test_multiple $test $test {
+    -re " = 1\r\n$gdb_prompt $" {
+       pass $test
+    }
+    -re "No symbol \"unbound\" in current context.\r\n$gdb_prompt $" {
+       unsupported "$test (no GCC)"
+    }
+}
...

We run into this FAIL:
...
p unbound.a == &unbound.a[0]^M
$7 = 0^M
(gdb) FAIL: gdb.base/arrayidx.exp: p unbound.a == &unbound.a[0]
...

A bit more probing:
...
(gdb) p unbound
$3 = {a = 0x601044}
(gdb) p &unbound
$4 = (struct {...} *) 0x601044
(gdb) p unbound.a
$5 = 0x601044
(gdb) p &unbound.a[0]
$6 = (int *) 0xffffffffefc93d84^M
...
reveals that we have a problem printing the address of the first element of a
zero length array.

-- 
You are receiving this mail because:
You are on the CC list for the bug.

^ permalink raw reply	[flat|nested] 18+ messages in thread

* [Bug exp/26875] Incorrect value printed for address of first element of zero-length array
  2020-11-13  9:32 [Bug exp/26875] New: Incorrect value printed for address of first element of zero-length array vries at gcc dot gnu.org
@ 2020-11-13 10:33 ` vries at gcc dot gnu.org
  2020-11-13 10:53 ` vries at gcc dot gnu.org
                   ` (15 subsequent siblings)
  16 siblings, 0 replies; 18+ messages in thread
From: vries at gcc dot gnu.org @ 2020-11-13 10:33 UTC (permalink / raw)
  To: gdb-prs

https://sourceware.org/bugzilla/show_bug.cgi?id=26875

--- Comment #1 from Tom de Vries <vries at gcc dot gnu.org> ---
Tentative patch (not taking side-effects into account yet):
...
diff --git a/gdb/eval.c b/gdb/eval.c
index 8d0c5747f39..69262340c8f 100644
--- a/gdb/eval.c
+++ b/gdb/eval.c
@@ -2814,6 +2814,18 @@ evaluate_subexp_for_address (struct expression *exp, int
*pos,


   switch (op)
     {
+    case BINOP_SUBSCRIPT:
+      {
+       int pos_base = *pos + 1;
+       struct value *base
+         = evaluate_subexp (nullptr, exp, &pos_base, noside);
+       int pos_index = pos_base;
+       struct value * index
+         = evaluate_subexp (nullptr, exp, &pos_index, noside);
+       if (value_equal (index, value_zero (value_type (index), not_lval)))
+         return base;
+      }
+      goto default_case;
     case UNOP_IND:
       (*pos)++;
       x = evaluate_subexp (nullptr, exp, pos, noside);
...

-- 
You are receiving this mail because:
You are on the CC list for the bug.

^ permalink raw reply	[flat|nested] 18+ messages in thread

* [Bug exp/26875] Incorrect value printed for address of first element of zero-length array
  2020-11-13  9:32 [Bug exp/26875] New: Incorrect value printed for address of first element of zero-length array vries at gcc dot gnu.org
  2020-11-13 10:33 ` [Bug exp/26875] " vries at gcc dot gnu.org
@ 2020-11-13 10:53 ` vries at gcc dot gnu.org
  2020-11-13 14:47 ` vries at gcc dot gnu.org
                   ` (14 subsequent siblings)
  16 siblings, 0 replies; 18+ messages in thread
From: vries at gcc dot gnu.org @ 2020-11-13 10:53 UTC (permalink / raw)
  To: gdb-prs

https://sourceware.org/bugzilla/show_bug.cgi?id=26875

--- Comment #2 from Tom de Vries <vries at gcc dot gnu.org> ---
...
diff --git a/gdb/eval.c b/gdb/eval.c
index 8d0c5747f39..62528075880 100644
--- a/gdb/eval.c
+++ b/gdb/eval.c
@@ -2814,6 +2814,29 @@ evaluate_subexp_for_address (struct expression *exp, int
*pos,


   switch (op)
     {
+    case BINOP_SUBSCRIPT:
+      if (noside != EVAL_SKIP)
+       {
+         int pos_index = *pos + 1;
+         evaluate_subexp (nullptr, exp, &pos_index, EVAL_SKIP);
+
+         struct value *index
+           = evaluate_subexp (nullptr, exp, &pos_index,
+                              EVAL_AVOID_SIDE_EFFECTS);
+         if (value_equal (index, value_zero (value_type (index), not_lval)))
+           {
+             (*pos)++;
+             struct value *base = evaluate_subexp (nullptr, exp, pos, noside);
+             /* Evaluate side effects for index and increase pos past
+                index.  */
+             evaluate_subexp (nullptr, exp, pos, noside);
+
+             return base;
+           }
+       }
+
+      goto default_case;
+
     case UNOP_IND:
       (*pos)++;
       x = evaluate_subexp (nullptr, exp, pos, noside);
...

-- 
You are receiving this mail because:
You are on the CC list for the bug.

^ permalink raw reply	[flat|nested] 18+ messages in thread

* [Bug exp/26875] Incorrect value printed for address of first element of zero-length array
  2020-11-13  9:32 [Bug exp/26875] New: Incorrect value printed for address of first element of zero-length array vries at gcc dot gnu.org
  2020-11-13 10:33 ` [Bug exp/26875] " vries at gcc dot gnu.org
  2020-11-13 10:53 ` vries at gcc dot gnu.org
@ 2020-11-13 14:47 ` vries at gcc dot gnu.org
  2020-11-13 16:52 ` vries at gcc dot gnu.org
                   ` (13 subsequent siblings)
  16 siblings, 0 replies; 18+ messages in thread
From: vries at gcc dot gnu.org @ 2020-11-13 14:47 UTC (permalink / raw)
  To: gdb-prs

https://sourceware.org/bugzilla/show_bug.cgi?id=26875

--- Comment #3 from Tom de Vries <vries at gcc dot gnu.org> ---
This passes testing:
...
diff --git a/gdb/eval.c b/gdb/eval.c
index 8d0c5747f39..0e8698bd488 100644
--- a/gdb/eval.c
+++ b/gdb/eval.c
@@ -2814,6 +2814,39 @@ evaluate_subexp_for_address (struct expression *exp, int
*pos,


   switch (op)
     {
+    case BINOP_SUBSCRIPT:
+      if (noside != EVAL_SKIP)
+       {
+         /* Skip over the base to get the position of the index.  */
+         int pos_index = *pos + 1;
+         evaluate_subexp (nullptr, exp, &pos_index, EVAL_SKIP);
+
+         /* Check if the index is zero.  Do this without evaluating
+            side effects, such that we can still fall back to default_case. 
*/
+         struct value *index
+           = evaluate_subexp (nullptr, exp, &pos_index,
+                              EVAL_AVOID_SIDE_EFFECTS);
+         if (value_equal (index, value_zero (value_type (index), not_lval)))
+           {
+             /* We're handling op here, skip it.  */
+             (*pos)++;
+
+             /* Evaluate base.  */
+             struct value *base = evaluate_subexp (nullptr, exp, pos, noside);
+
+             /* Evaluate index, for the side-effect, and to return with pos
+                past index.  */
+             evaluate_subexp (nullptr, exp, pos, noside);
+
+             /* Translate &base[0] into (typeof(base[0]) *)base.   */
+             type *element_type = TYPE_TARGET_TYPE (value_type (base));
+             type *return_type = lookup_pointer_type (element_type);
+             return value_cast (return_type, base);
+           }
+       }
+
+      goto default_case;
+
     case UNOP_IND:
       (*pos)++;
       x = evaluate_subexp (nullptr, exp, pos, noside);
...

-- 
You are receiving this mail because:
You are on the CC list for the bug.

^ permalink raw reply	[flat|nested] 18+ messages in thread

* [Bug exp/26875] Incorrect value printed for address of first element of zero-length array
  2020-11-13  9:32 [Bug exp/26875] New: Incorrect value printed for address of first element of zero-length array vries at gcc dot gnu.org
                   ` (2 preceding siblings ...)
  2020-11-13 14:47 ` vries at gcc dot gnu.org
@ 2020-11-13 16:52 ` vries at gcc dot gnu.org
  2020-11-14 11:45 ` vries at gcc dot gnu.org
                   ` (12 subsequent siblings)
  16 siblings, 0 replies; 18+ messages in thread
From: vries at gcc dot gnu.org @ 2020-11-13 16:52 UTC (permalink / raw)
  To: gdb-prs

https://sourceware.org/bugzilla/show_bug.cgi?id=26875

Tom de Vries <vries at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |simark at simark dot ca

--- Comment #4 from Tom de Vries <vries at gcc dot gnu.org> ---
This passed with gdb 9.

The first bad commit is either:
- commit 7c6f271296319576fa00587928e5ff52ced9c1bb (could not build)
  gdb: make get_discrete_bounds check for non-constant range bounds, or
- commit 8c2e4e0689ea244d0ed979171a3d09c9176b8175
  gdb: add accessors to struct dynamic_prop

-- 
You are receiving this mail because:
You are on the CC list for the bug.

^ permalink raw reply	[flat|nested] 18+ messages in thread

* [Bug exp/26875] Incorrect value printed for address of first element of zero-length array
  2020-11-13  9:32 [Bug exp/26875] New: Incorrect value printed for address of first element of zero-length array vries at gcc dot gnu.org
                   ` (3 preceding siblings ...)
  2020-11-13 16:52 ` vries at gcc dot gnu.org
@ 2020-11-14 11:45 ` vries at gcc dot gnu.org
  2020-11-14 12:04 ` vries at gcc dot gnu.org
                   ` (11 subsequent siblings)
  16 siblings, 0 replies; 18+ messages in thread
From: vries at gcc dot gnu.org @ 2020-11-14 11:45 UTC (permalink / raw)
  To: gdb-prs

https://sourceware.org/bugzilla/show_bug.cgi?id=26875

--- Comment #5 from Tom de Vries <vries at gcc dot gnu.org> ---
(In reply to Tom de Vries from comment #4)
> This passed with gdb 9.
> 
> The first bad commit is either:
> - commit 7c6f271296319576fa00587928e5ff52ced9c1bb (could not build)
>   gdb: make get_discrete_bounds check for non-constant range bounds, or
> - commit 8c2e4e0689ea244d0ed979171a3d09c9176b8175
>   gdb: add accessors to struct dynamic_prop

Hmm, before these commits, we handle this type in get_discrete_bounds:
...
(gdb) p recursive_dump_type (type, 0)
type node 0x2283020
name '<NULL>' (0x0)
code 0xc (TYPE_CODE_RANGE)
length 8
objfile 0x1fced60
target_type 0x223c440
  type node 0x223c440
  name 'long unsigned int' (0x225be83)
  code 0x8 (TYPE_CODE_INT)
  length 8
  objfile 0x1fced60
  target_type 0x0
  pointer_type 0x0
  reference_type 0x0
  type_chain 0x223c440
  instance_flags 0x0
  flags TYPE_UNSIGNED
  nfields 0 0x0
pointer_type 0x0
reference_type 0x0
type_chain 0x2283020
instance_flags 0x0
flags TYPE_UNSIGNED
nfields 0 0x22830a0
low 0  high 0 (undefined)
$12 = void
...
and ignored the TYPE_HIGH_BOUND_UNDEFINED, and just return 1, with lowerbound
== 0 and upperbound == 0.  The setting of lowerbound == 0 had the effect that
we printed the right value.

After the commits, get_discrete_bounds returns -1, and we have both lowerbound
and upperbound uninitialized.  We don't check the return status though in
value_subscript, and proceed with the uninitialized values:
...
(gdb) p lowerbound
$2 = 36429408
(gdb) p upperbound
$3 = 11281392
...

And after this:
...
(gdb) 
171           index -= lowerbound;
...
we have:
...
(gdb) p index
$4 = -36429408
...

So we end up here:
...
    return value_ind (value_ptradd (array, index));
...
constructing a value "*(array + -36429408)", and we end up printing 
&ubound.a[-36429408].

This is an ad-hoc patch that sets lowerbound, even if upperbound is undefined:
...
diff --git a/gdb/gdbtypes.c b/gdb/gdbtypes.c
index 686edafcf64..dfc3de870c1 100644
--- a/gdb/gdbtypes.c
+++ b/gdb/gdbtypes.c
@@ -1049,11 +1049,16 @@ get_discrete_bounds (struct type *type, LONGEST *lowp,
LONGEST
 *highp)
     case TYPE_CODE_RANGE:
       /* This function currently only works for ranges with two defined,
         constant bounds.  */
-      if (type->bounds ()->low.kind () != PROP_CONST
-         || type->bounds ()->high.kind () != PROP_CONST)
+      if (type->bounds ()->low.kind () != PROP_CONST)
        return -1;
-
       *lowp = type->bounds ()->low.const_val ();
+
+      if (type->bounds ()->high.kind () != PROP_CONST)
+       {
+         *highp = *lowp - 1;
+         return 1;
+       }
+
       *highp = type->bounds ()->high.const_val ();

       if (TYPE_TARGET_TYPE (type)->code () == TYPE_CODE_ENUM)
...
and handles the undefined upper bound by returning an empty range.

-- 
You are receiving this mail because:
You are on the CC list for the bug.

^ permalink raw reply	[flat|nested] 18+ messages in thread

* [Bug exp/26875] Incorrect value printed for address of first element of zero-length array
  2020-11-13  9:32 [Bug exp/26875] New: Incorrect value printed for address of first element of zero-length array vries at gcc dot gnu.org
                   ` (4 preceding siblings ...)
  2020-11-14 11:45 ` vries at gcc dot gnu.org
@ 2020-11-14 12:04 ` vries at gcc dot gnu.org
  2020-11-20 18:00 ` simark at simark dot ca
                   ` (10 subsequent siblings)
  16 siblings, 0 replies; 18+ messages in thread
From: vries at gcc dot gnu.org @ 2020-11-14 12:04 UTC (permalink / raw)
  To: gdb-prs

https://sourceware.org/bugzilla/show_bug.cgi?id=26875

--- Comment #6 from Tom de Vries <vries at gcc dot gnu.org> ---
The simplest fix would be to make sure get_discrete_bounds always returns
defined *lowp and *highp:
...
diff --git a/gdb/gdbtypes.c b/gdb/gdbtypes.c
index 686edafcf64..9a891e7bfde 100644
--- a/gdb/gdbtypes.c
+++ b/gdb/gdbtypes.c
@@ -1043,6 +1043,8 @@ has_static_range (const struct range_bounds *bounds)
 int
 get_discrete_bounds (struct type *type, LONGEST *lowp, LONGEST *highp)
 {
+  *lowp = 0;
+  *highp = -1;
   type = check_typedef (type);
   switch (type->code ())
     {
...
which also happens to fix PR26870 - "[fortran] Printing dynamic array fails".

-- 
You are receiving this mail because:
You are on the CC list for the bug.

^ permalink raw reply	[flat|nested] 18+ messages in thread

* [Bug exp/26875] Incorrect value printed for address of first element of zero-length array
  2020-11-13  9:32 [Bug exp/26875] New: Incorrect value printed for address of first element of zero-length array vries at gcc dot gnu.org
                   ` (5 preceding siblings ...)
  2020-11-14 12:04 ` vries at gcc dot gnu.org
@ 2020-11-20 18:00 ` simark at simark dot ca
  2020-11-20 18:03 ` simark at simark dot ca
                   ` (9 subsequent siblings)
  16 siblings, 0 replies; 18+ messages in thread
From: simark at simark dot ca @ 2020-11-20 18:00 UTC (permalink / raw)
  To: gdb-prs

https://sourceware.org/bugzilla/show_bug.cgi?id=26875

--- Comment #7 from Simon Marchi <simark at simark dot ca> ---
I am not able to reproduce.  In my version of the compiled test case, both
bounds are const:

(top-gdb) p type->bounds ()->low.kind ()
$4 = PROP_CONST
(top-gdb) p type->bounds ()->high.kind ()
$5 = PROP_CONST

What compiler do you use to build the test case?  In my version, this is the
DIE that corresponds to the type of the zero-length field:

0x00000076:   DW_TAG_array_type
                DW_AT_type [DW_FORM_ref4]       (0x00000044 "int")
                DW_AT_sibling [DW_FORM_ref4]    (0x00000086)

0x0000007f:     DW_TAG_subrange_type
                  DW_AT_type [DW_FORM_ref4]     (0x0000003d "long unsigned
int")
                  DW_AT_count [DW_FORM_data1]   (0x00)

So I presume GDB translates that to two constant bounds with value 0.  What
does it look like in yours?

I stumbled on a similar problem and filed this bug here, they are likely
related:

https://sourceware.org/bugzilla/show_bug.cgi?id=26901

I have this patch series in the pipeline that I made to address 26901:

https://review.lttng.org/c/binutils-gdb/+/4399/4

If/when I am able to reproduce this (26875) bug, I can check if that helps with
it too.

-- 
You are receiving this mail because:
You are on the CC list for the bug.

^ permalink raw reply	[flat|nested] 18+ messages in thread

* [Bug exp/26875] Incorrect value printed for address of first element of zero-length array
  2020-11-13  9:32 [Bug exp/26875] New: Incorrect value printed for address of first element of zero-length array vries at gcc dot gnu.org
                   ` (6 preceding siblings ...)
  2020-11-20 18:00 ` simark at simark dot ca
@ 2020-11-20 18:03 ` simark at simark dot ca
  2020-11-20 18:07 ` simark at simark dot ca
                   ` (8 subsequent siblings)
  16 siblings, 0 replies; 18+ messages in thread
From: simark at simark dot ca @ 2020-11-20 18:03 UTC (permalink / raw)
  To: gdb-prs

https://sourceware.org/bugzilla/show_bug.cgi?id=26875

--- Comment #8 from Simon Marchi <simark at simark dot ca> ---
Here's the URL that points to the always-tested version of the patch:

https://review.lttng.org/c/binutils-gdb/+/4399

-- 
You are receiving this mail because:
You are on the CC list for the bug.

^ permalink raw reply	[flat|nested] 18+ messages in thread

* [Bug exp/26875] Incorrect value printed for address of first element of zero-length array
  2020-11-13  9:32 [Bug exp/26875] New: Incorrect value printed for address of first element of zero-length array vries at gcc dot gnu.org
                   ` (7 preceding siblings ...)
  2020-11-20 18:03 ` simark at simark dot ca
@ 2020-11-20 18:07 ` simark at simark dot ca
  2020-11-20 18:17 ` simark at simark dot ca
                   ` (7 subsequent siblings)
  16 siblings, 0 replies; 18+ messages in thread
From: simark at simark dot ca @ 2020-11-20 18:07 UTC (permalink / raw)
  To: gdb-prs

https://sourceware.org/bugzilla/show_bug.cgi?id=26875

--- Comment #9 from Simon Marchi <simark at simark dot ca> ---
(In reply to Simon Marchi from comment #8)
> Here's the URL that points to the always-tested version of the patch:
> 
> https://review.lttng.org/c/binutils-gdb/+/4399

Err it's currently broken due to a rebase, I'll fix it soon.

-- 
You are receiving this mail because:
You are on the CC list for the bug.

^ permalink raw reply	[flat|nested] 18+ messages in thread

* [Bug exp/26875] Incorrect value printed for address of first element of zero-length array
  2020-11-13  9:32 [Bug exp/26875] New: Incorrect value printed for address of first element of zero-length array vries at gcc dot gnu.org
                   ` (8 preceding siblings ...)
  2020-11-20 18:07 ` simark at simark dot ca
@ 2020-11-20 18:17 ` simark at simark dot ca
  2020-11-20 22:16 ` vries at gcc dot gnu.org
                   ` (6 subsequent siblings)
  16 siblings, 0 replies; 18+ messages in thread
From: simark at simark dot ca @ 2020-11-20 18:17 UTC (permalink / raw)
  To: gdb-prs

https://sourceware.org/bugzilla/show_bug.cgi?id=26875

--- Comment #10 from Simon Marchi <simark at simark dot ca> ---
Ok, it now builds properly.

-- 
You are receiving this mail because:
You are on the CC list for the bug.

^ permalink raw reply	[flat|nested] 18+ messages in thread

* [Bug exp/26875] Incorrect value printed for address of first element of zero-length array
  2020-11-13  9:32 [Bug exp/26875] New: Incorrect value printed for address of first element of zero-length array vries at gcc dot gnu.org
                   ` (9 preceding siblings ...)
  2020-11-20 18:17 ` simark at simark dot ca
@ 2020-11-20 22:16 ` vries at gcc dot gnu.org
  2020-11-20 22:25 ` simark at simark dot ca
                   ` (5 subsequent siblings)
  16 siblings, 0 replies; 18+ messages in thread
From: vries at gcc dot gnu.org @ 2020-11-20 22:16 UTC (permalink / raw)
  To: gdb-prs

https://sourceware.org/bugzilla/show_bug.cgi?id=26875

--- Comment #11 from Tom de Vries <vries at gcc dot gnu.org> ---
(In reply to Simon Marchi from comment #7)
> I am not able to reproduce.  In my version of the compiled test case, both
> bounds are const:
> 
> (top-gdb) p type->bounds ()->low.kind ()
> $4 = PROP_CONST
> (top-gdb) p type->bounds ()->high.kind ()
> $5 = PROP_CONST
> 
> What compiler do you use to build the test case?

Gcc 7.5.0

> In my version, this is the
> DIE that corresponds to the type of the zero-length field:
> 
> 0x00000076:   DW_TAG_array_type
>                 DW_AT_type [DW_FORM_ref4]       (0x00000044 "int")
>                 DW_AT_sibling [DW_FORM_ref4]    (0x00000086)
> 
> 0x0000007f:     DW_TAG_subrange_type
>                   DW_AT_type [DW_FORM_ref4]     (0x0000003d "long unsigned
> int")
>                   DW_AT_count [DW_FORM_data1]   (0x00)
> 

I get:
...
 <1><13a>: Abbrev Number: 2 (DW_TAG_array_type)
    <13b>   DW_AT_type        : <0x10b>
    <13f>   DW_AT_sibling     : <0x149>
 <2><143>: Abbrev Number: 9 (DW_TAG_subrange_type)
    <144>   DW_AT_type        : <0x104>
...
without DW_AT_count.

The DW_AT_count is added at gcc-9, and using that the test-cases passes for me.

> So I presume GDB translates that to two constant bounds with value 0.  What
> does it look like in yours?
> 
> I stumbled on a similar problem and filed this bug here, they are likely
> related:
> 
> https://sourceware.org/bugzilla/show_bug.cgi?id=26901
> 
> I have this patch series in the pipeline that I made to address 26901:
> 
> https://review.lttng.org/c/binutils-gdb/+/4399/4
> 

Ah, thanks for the pointer, I'll take a look.

> If/when I am able to reproduce this (26875) bug, I can check if that helps
> with it too.

That would be great.

-- 
You are receiving this mail because:
You are on the CC list for the bug.

^ permalink raw reply	[flat|nested] 18+ messages in thread

* [Bug exp/26875] Incorrect value printed for address of first element of zero-length array
  2020-11-13  9:32 [Bug exp/26875] New: Incorrect value printed for address of first element of zero-length array vries at gcc dot gnu.org
                   ` (10 preceding siblings ...)
  2020-11-20 22:16 ` vries at gcc dot gnu.org
@ 2020-11-20 22:25 ` simark at simark dot ca
  2020-11-23 16:25 ` simark at simark dot ca
                   ` (4 subsequent siblings)
  16 siblings, 0 replies; 18+ messages in thread
From: simark at simark dot ca @ 2020-11-20 22:25 UTC (permalink / raw)
  To: gdb-prs

https://sourceware.org/bugzilla/show_bug.cgi?id=26875

--- Comment #12 from Simon Marchi <simark at simark dot ca> ---
(In reply to Tom de Vries from comment #11)
> (In reply to Simon Marchi from comment #7)
> > I am not able to reproduce.  In my version of the compiled test case, both
> > bounds are const:
> > 
> > (top-gdb) p type->bounds ()->low.kind ()
> > $4 = PROP_CONST
> > (top-gdb) p type->bounds ()->high.kind ()
> > $5 = PROP_CONST
> > 
> > What compiler do you use to build the test case?
> 
> Gcc 7.5.0

Thanks, I see the failure with gcc 7.

> > So I presume GDB translates that to two constant bounds with value 0.  What
> > does it look like in yours?
> > 
> > I stumbled on a similar problem and filed this bug here, they are likely
> > related:
> > 
> > https://sourceware.org/bugzilla/show_bug.cgi?id=26901
> > 
> > I have this patch series in the pipeline that I made to address 26901:
> > 
> > https://review.lttng.org/c/binutils-gdb/+/4399/4
> > 
> 
> Ah, thanks for the pointer, I'll take a look.

The patches are not commented at all now, but the idea is to be able to fetch
the low and high bounds separately, not all or nothing like it is now.  Here,
we only care about the low bound, so we only get that one, and ignore the high
bound (which is unknown).

> 
> > If/when I am able to reproduce this (26875) bug, I can check if that helps
> > with it too.
> 
> That would be great.

The failure goes away with these patches applied.

The only thing that bugs me is that some part of the code assumes that a bound
of "-1" means an unknown bound.  It seems to me that if we use gdb::optional,
we don't need that: an empty optional means an unknown bound.  So we would need
to go through all the users to see we if need the "set to -1" vs "empty
optional" disctinction or not.

-- 
You are receiving this mail because:
You are on the CC list for the bug.

^ permalink raw reply	[flat|nested] 18+ messages in thread

* [Bug exp/26875] Incorrect value printed for address of first element of zero-length array
  2020-11-13  9:32 [Bug exp/26875] New: Incorrect value printed for address of first element of zero-length array vries at gcc dot gnu.org
                   ` (11 preceding siblings ...)
  2020-11-20 22:25 ` simark at simark dot ca
@ 2020-11-23 16:25 ` simark at simark dot ca
  2020-12-09 18:53 ` cvs-commit at gcc dot gnu.org
                   ` (3 subsequent siblings)
  16 siblings, 0 replies; 18+ messages in thread
From: simark at simark dot ca @ 2020-11-23 16:25 UTC (permalink / raw)
  To: gdb-prs

https://sourceware.org/bugzilla/show_bug.cgi?id=26875

--- Comment #13 from Simon Marchi <simark at simark dot ca> ---
Proposed patch series:
https://sourceware.org/pipermail/gdb-patches/2020-November/173496.html

-- 
You are receiving this mail because:
You are on the CC list for the bug.

^ permalink raw reply	[flat|nested] 18+ messages in thread

* [Bug exp/26875] Incorrect value printed for address of first element of zero-length array
  2020-11-13  9:32 [Bug exp/26875] New: Incorrect value printed for address of first element of zero-length array vries at gcc dot gnu.org
                   ` (12 preceding siblings ...)
  2020-11-23 16:25 ` simark at simark dot ca
@ 2020-12-09 18:53 ` cvs-commit at gcc dot gnu.org
  2020-12-09 21:34 ` cvs-commit at gcc dot gnu.org
                   ` (2 subsequent siblings)
  16 siblings, 0 replies; 18+ messages in thread
From: cvs-commit at gcc dot gnu.org @ 2020-12-09 18:53 UTC (permalink / raw)
  To: gdb-prs

https://sourceware.org/bugzilla/show_bug.cgi?id=26875

--- Comment #14 from cvs-commit at gcc dot gnu.org <cvs-commit at gcc dot gnu.org> ---
The master branch has been updated by Simon Marchi <simark@sourceware.org>:

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

commit 5b56203a7cadd545b33713e98e274e582242e090
Author: Simon Marchi <simon.marchi@efficios.com>
Date:   Wed Dec 9 13:52:12 2020 -0500

    gdb: fix value_subscript when array upper bound is not known

    Since commit 7c6f27129631 ("gdb: make get_discrete_bounds check for
    non-constant range bounds"), subscripting  flexible array member fails:

        struct no_size
        {
          int n;
          int items[];
        };

        (gdb) p *ns
        $1 = {n = 3, items = 0x5555555592a4}
        (gdb) p ns->items[0]
        Cannot access memory at address 0xfffe555b733a0164
        (gdb) p *((int *) 0x5555555592a4)
        $2 = 101  <--- we would expect that
        (gdb) p &ns->items[0]
        $3 = (int *) 0xfffe5559ee829a24  <--- wrong address

    Since the flexible array member (items) has an unspecified size, the array
type
    created for it in the DWARF doesn't have dimensions (this is with gcc
9.3.0,
    Ubuntu 20.04):

        0x000000a4:   DW_TAG_array_type
                        DW_AT_type [DW_FORM_ref4]       (0x00000038 "int")
                        DW_AT_sibling [DW_FORM_ref4]    (0x000000b3)

        0x000000ad:     DW_TAG_subrange_type
                          DW_AT_type [DW_FORM_ref4]     (0x00000031 "long
unsigned int")

    This causes GDB to create a range type (TYPE_CODE_RANGE) with a defined
    constant low bound (dynamic_prop with kind PROP_CONST) and an undefined
    high bound (dynamic_prop with kind PROP_UNDEFINED).

    value_subscript gets both bounds of that range using
    get_discrete_bounds.  Before commit 7c6f27129631, get_discrete_bounds
    didn't check the kind of the dynamic_props and would just blindly read
    them as if they were PROP_CONST.  It would return 0 for the high bound,
    because we zero-initialize the range_bounds structure.  And it didn't
    really matter in this case, because the returned high bound wasn't used
    in the end.

    Commit 7c6f27129631 changed get_discrete_bounds to return a failure if
    either the low or high bound is not a constant, to make sure we don't
    read a dynamic prop that isn't a PROP_CONST as a PROP_CONST.  This
    change made get_discrete_bounds start to return a failure for that
    range, and as a result would not set *lowp and *highp.  And since
    value_subscript doesn't check get_discrete_bounds' return value, it just
    carries on an uses an uninitialized value for the low bound.  If
    value_subscript did check the return value of get_discrete_bounds, we
    would get an error message instead of a bogus value.  But it would still
    be a bug, as we wouldn't be able to print the flexible array member's
    elements.

    Looking at value_subscript, we see that the low bound is always needed,
    but the high bound is only needed if !c_style.  So, change
    value_subscript to use get_discrete_low_bound and
    get_discrete_high_bound separately.  This fixes the case described
    above, where the low bound is known but the high bound isn't (and is not
    needed).  This restores the original behavior without accessing a
    dynamic_prop in a wrong way.

    A test is added.  In addition to the case described above, a case with
    an array member of size 0 is added, which is a GNU C extension that
    existed before flexible array members were introduced.  That case
    currently fails when compiled with gcc <= 8.  gcc <= 8 produces DWARF
    similar to the one shown above, while gcc 9 adds a DW_AT_count of 0 in
    there, which makes the high bound known.  A case where an array member
    of size 0 is the only member of the struct is also added, as that was
    how PR 28675 was originally reported, and it's an interesting corner
    case that I think could trigger other funny bugs.

    Question about the implementation: in value_subscript, I made it such
    that if the low or high bound is unknown, we fall back to zero.  That
    effectively makes it the same as it was before 7c6f27129631.  But should
    we instead error() out?

    gdb/ChangeLog:

            PR 26875, PR 26901
            * gdbtypes.c (get_discrete_low_bound): Make non-static.
            (get_discrete_high_bound): Make non-static.
            * gdbtypes.h (get_discrete_low_bound): New declaration.
            (get_discrete_high_bound): New declaration.
            * valarith.c (value_subscript): Only fetch high bound if
            necessary.

    gdb/testsuite/ChangeLog:

            PR 26875, PR 26901
            * gdb.base/flexible-array-member.c: New test.
            * gdb.base/flexible-array-member.exp: New test.

    Change-Id: I832056f80e6c56f621f398b4780d55a3a1e299d7

-- 
You are receiving this mail because:
You are on the CC list for the bug.

^ permalink raw reply	[flat|nested] 18+ messages in thread

* [Bug exp/26875] Incorrect value printed for address of first element of zero-length array
  2020-11-13  9:32 [Bug exp/26875] New: Incorrect value printed for address of first element of zero-length array vries at gcc dot gnu.org
                   ` (13 preceding siblings ...)
  2020-12-09 18:53 ` cvs-commit at gcc dot gnu.org
@ 2020-12-09 21:34 ` cvs-commit at gcc dot gnu.org
  2020-12-09 22:11 ` simark at simark dot ca
  2021-10-13 14:22 ` vries at gcc dot gnu.org
  16 siblings, 0 replies; 18+ messages in thread
From: cvs-commit at gcc dot gnu.org @ 2020-12-09 21:34 UTC (permalink / raw)
  To: gdb-prs

https://sourceware.org/bugzilla/show_bug.cgi?id=26875

--- Comment #15 from cvs-commit at gcc dot gnu.org <cvs-commit at gcc dot gnu.org> ---
The gdb-10-branch branch has been updated by Simon Marchi
<simark@sourceware.org>:

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

commit 138084b24811c191a85c919116d839a8bd89d57c
Author: Simon Marchi <simon.marchi@efficios.com>
Date:   Wed Dec 9 13:52:12 2020 -0500

    gdb: fix value_subscript when array upper bound is not known

    Since commit 7c6f27129631 ("gdb: make get_discrete_bounds check for
    non-constant range bounds"), subscripting  flexible array member fails:

        struct no_size
        {
          int n;
          int items[];
        };

        (gdb) p *ns
        $1 = {n = 3, items = 0x5555555592a4}
        (gdb) p ns->items[0]
        Cannot access memory at address 0xfffe555b733a0164
        (gdb) p *((int *) 0x5555555592a4)
        $2 = 101  <--- we would expect that
        (gdb) p &ns->items[0]
        $3 = (int *) 0xfffe5559ee829a24  <--- wrong address

    Since the flexible array member (items) has an unspecified size, the array
type
    created for it in the DWARF doesn't have dimensions (this is with gcc
9.3.0,
    Ubuntu 20.04):

        0x000000a4:   DW_TAG_array_type
                        DW_AT_type [DW_FORM_ref4]       (0x00000038 "int")
                        DW_AT_sibling [DW_FORM_ref4]    (0x000000b3)

        0x000000ad:     DW_TAG_subrange_type
                          DW_AT_type [DW_FORM_ref4]     (0x00000031 "long
unsigned int")

    This causes GDB to create a range type (TYPE_CODE_RANGE) with a defined
    constant low bound (dynamic_prop with kind PROP_CONST) and an undefined
    high bound (dynamic_prop with kind PROP_UNDEFINED).

    value_subscript gets both bounds of that range using
    get_discrete_bounds.  Before commit 7c6f27129631, get_discrete_bounds
    didn't check the kind of the dynamic_props and would just blindly read
    them as if they were PROP_CONST.  It would return 0 for the high bound,
    because we zero-initialize the range_bounds structure.  And it didn't
    really matter in this case, because the returned high bound wasn't used
    in the end.

    Commit 7c6f27129631 changed get_discrete_bounds to return a failure if
    either the low or high bound is not a constant, to make sure we don't
    read a dynamic prop that isn't a PROP_CONST as a PROP_CONST.  This
    change made get_discrete_bounds start to return a failure for that
    range, and as a result would not set *lowp and *highp.  And since
    value_subscript doesn't check get_discrete_bounds' return value, it just
    carries on an uses an uninitialized value for the low bound.  If
    value_subscript did check the return value of get_discrete_bounds, we
    would get an error message instead of a bogus value.  But it would still
    be a bug, as we wouldn't be able to print the flexible array member's
    elements.

    Looking at value_subscript, we see that the low bound is always needed,
    but the high bound is only needed if !c_style.  So, change
    value_subscript to use get_discrete_low_bound and
    get_discrete_high_bound separately.  This fixes the case described
    above, where the low bound is known but the high bound isn't (and is not
    needed).  This restores the original behavior without accessing a
    dynamic_prop in a wrong way.

    A test is added.  In addition to the case described above, a case with
    an array member of size 0 is added, which is a GNU C extension that
    existed before flexible array members were introduced.  That case
    currently fails when compiled with gcc <= 8.  gcc <= 8 produces DWARF
    similar to the one shown above, while gcc 9 adds a DW_AT_count of 0 in
    there, which makes the high bound known.  A case where an array member
    of size 0 is the only member of the struct is also added, as that was
    how PR 28675 was originally reported, and it's an interesting corner
    case that I think could trigger other funny bugs.

    Question about the implementation: in value_subscript, I made it such
    that if the low or high bound is unknown, we fall back to zero.  That
    effectively makes it the same as it was before 7c6f27129631.  But should
    we instead error() out?

    gdb/ChangeLog:

            PR 26875, PR 26901
            * gdbtypes.c (get_discrete_low_bound): Make non-static.
            (get_discrete_high_bound): Make non-static.
            * gdbtypes.h (get_discrete_low_bound): New declaration.
            (get_discrete_high_bound): New declaration.
            * valarith.c (value_subscript): Only fetch high bound if
            necessary.

    gdb/testsuite/ChangeLog:

            PR 26875, PR 26901
            * gdb.base/flexible-array-member.c: New test.
            * gdb.base/flexible-array-member.exp: New test.

    Change-Id: I832056f80e6c56f621f398b4780d55a3a1e299d7

-- 
You are receiving this mail because:
You are on the CC list for the bug.

^ permalink raw reply	[flat|nested] 18+ messages in thread

* [Bug exp/26875] Incorrect value printed for address of first element of zero-length array
  2020-11-13  9:32 [Bug exp/26875] New: Incorrect value printed for address of first element of zero-length array vries at gcc dot gnu.org
                   ` (14 preceding siblings ...)
  2020-12-09 21:34 ` cvs-commit at gcc dot gnu.org
@ 2020-12-09 22:11 ` simark at simark dot ca
  2021-10-13 14:22 ` vries at gcc dot gnu.org
  16 siblings, 0 replies; 18+ messages in thread
From: simark at simark dot ca @ 2020-12-09 22:11 UTC (permalink / raw)
  To: gdb-prs

https://sourceware.org/bugzilla/show_bug.cgi?id=26875

Simon Marchi <simark at simark dot ca> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|NEW                         |RESOLVED
         Resolution|---                         |FIXED

--- Comment #16 from Simon Marchi <simark at simark dot ca> ---
This is now fixed.

-- 
You are receiving this mail because:
You are on the CC list for the bug.

^ permalink raw reply	[flat|nested] 18+ messages in thread

* [Bug exp/26875] Incorrect value printed for address of first element of zero-length array
  2020-11-13  9:32 [Bug exp/26875] New: Incorrect value printed for address of first element of zero-length array vries at gcc dot gnu.org
                   ` (15 preceding siblings ...)
  2020-12-09 22:11 ` simark at simark dot ca
@ 2021-10-13 14:22 ` vries at gcc dot gnu.org
  16 siblings, 0 replies; 18+ messages in thread
From: vries at gcc dot gnu.org @ 2021-10-13 14:22 UTC (permalink / raw)
  To: gdb-prs

https://sourceware.org/bugzilla/show_bug.cgi?id=26875

Tom de Vries <vries at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
   Target Milestone|---                         |10.2

--- Comment #17 from Tom de Vries <vries at gcc dot gnu.org> ---
$ git tag --contains 138084b24811c191a85c919116d839a8bd89d57c
gdb-10.2-release

-- 
You are receiving this mail because:
You are on the CC list for the bug.

^ permalink raw reply	[flat|nested] 18+ messages in thread

end of thread, other threads:[~2021-10-13 14:22 UTC | newest]

Thread overview: 18+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-11-13  9:32 [Bug exp/26875] New: Incorrect value printed for address of first element of zero-length array vries at gcc dot gnu.org
2020-11-13 10:33 ` [Bug exp/26875] " vries at gcc dot gnu.org
2020-11-13 10:53 ` vries at gcc dot gnu.org
2020-11-13 14:47 ` vries at gcc dot gnu.org
2020-11-13 16:52 ` vries at gcc dot gnu.org
2020-11-14 11:45 ` vries at gcc dot gnu.org
2020-11-14 12:04 ` vries at gcc dot gnu.org
2020-11-20 18:00 ` simark at simark dot ca
2020-11-20 18:03 ` simark at simark dot ca
2020-11-20 18:07 ` simark at simark dot ca
2020-11-20 18:17 ` simark at simark dot ca
2020-11-20 22:16 ` vries at gcc dot gnu.org
2020-11-20 22:25 ` simark at simark dot ca
2020-11-23 16:25 ` simark at simark dot ca
2020-12-09 18:53 ` cvs-commit at gcc dot gnu.org
2020-12-09 21:34 ` cvs-commit at gcc dot gnu.org
2020-12-09 22:11 ` simark at simark dot ca
2021-10-13 14:22 ` vries at gcc dot gnu.org

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).