testsuite/gdb.base: Make skip test not rely on undefined behaviour.

Message ID 51B22951.1000601@linaro.org
State Superseded
Headers show

Commit Message

Will Newton June 7, 2013, 6:41 p.m.
The skip test currently relies on the order of evaluation of
arguments which is not defined. Use an arithmetic expression
where order is defined instead.

gdb/testsuite/ChangeLog:

2013-06-07  Will Newton  <will.newton@linaro.org>

	* gdb.base/skip.c: Add results of foo() and bar() before
	passing to baz().
	* gdb.base/skip.c: baz() now takes one argument instead of
	two.
---
 gdb/testsuite/gdb.base/skip.c  | 4 ++--
 gdb/testsuite/gdb.base/skip1.c | 4 ++--
 2 files changed, 4 insertions(+), 4 deletions(-)

Comments

Andrew Pinski June 11, 2013, 11 p.m. | #1
On Fri, Jun 7, 2013 at 11:41 AM, Will Newton <will.newton@linaro.org> wrote:
>
> The skip test currently relies on the order of evaluation of
> arguments which is not defined. Use an arithmetic expression
> where order is defined instead.


No the order is not defined there either.  The left side or the right
side of the addition can be evaluated first as there is no sequence
point.  Maybe it is better to do:
baz((foo(), bar()));

The order with that is defined as the comma operator is a sequence point.

Thanks,
Andrew Pinski

>
> gdb/testsuite/ChangeLog:
>
> 2013-06-07  Will Newton  <will.newton@linaro.org>
>
>         * gdb.base/skip.c: Add results of foo() and bar() before
>         passing to baz().
>         * gdb.base/skip.c: baz() now takes one argument instead of
>         two.
> ---
>  gdb/testsuite/gdb.base/skip.c  | 4 ++--
>  gdb/testsuite/gdb.base/skip1.c | 4 ++--
>  2 files changed, 4 insertions(+), 4 deletions(-)
>
> diff --git a/gdb/testsuite/gdb.base/skip.c b/gdb/testsuite/gdb.base/skip.c
> index 565ba93..1281d17 100644
> --- a/gdb/testsuite/gdb.base/skip.c
> +++ b/gdb/testsuite/gdb.base/skip.c
> @@ -1,10 +1,10 @@
>  int foo();
>  int bar();
> -int baz(int, int);
> +int baz(int);
>
>  int main()
>  {
> -  return baz(foo(), bar());
> +  return baz(bar() + foo());
>  }
>
>  int foo()
> diff --git a/gdb/testsuite/gdb.base/skip1.c b/gdb/testsuite/gdb.base/skip1.c
> index 2dab5c3..fe63cd6 100644
> --- a/gdb/testsuite/gdb.base/skip1.c
> +++ b/gdb/testsuite/gdb.base/skip1.c
> @@ -3,7 +3,7 @@ int bar()
>    return 1;
>  }
>
> -int baz(int a, int b)
> +int baz(int a)
>  {
> -  return a + b;
> +  return a + 1;
>  }
> --
> 1.8.1.4
>
Will Newton June 12, 2013, 9:50 a.m. | #2
On 12 June 2013 00:00, Andrew Pinski <pinskia@gmail.com> wrote:
> On Fri, Jun 7, 2013 at 11:41 AM, Will Newton <will.newton@linaro.org> wrote:
>>
>> The skip test currently relies on the order of evaluation of
>> arguments which is not defined. Use an arithmetic expression
>> where order is defined instead.
>
>
> No the order is not defined there either.  The left side or the right
> side of the addition can be evaluated first as there is no sequence
> point.  Maybe it is better to do:
> baz((foo(), bar()));
>
> The order with that is defined as the comma operator is a sequence point.

Thanks, I was getting associativity mixed up with evaluation order.
I'll respin the patch with that change.

--
Will Newton
Toolchain Working Group, Linaro

Patch

diff --git a/gdb/testsuite/gdb.base/skip.c b/gdb/testsuite/gdb.base/skip.c
index 565ba93..1281d17 100644
--- a/gdb/testsuite/gdb.base/skip.c
+++ b/gdb/testsuite/gdb.base/skip.c
@@ -1,10 +1,10 @@ 
 int foo();
 int bar();
-int baz(int, int);
+int baz(int);

 int main()
 {
-  return baz(foo(), bar());
+  return baz(bar() + foo());
 }

 int foo()
diff --git a/gdb/testsuite/gdb.base/skip1.c b/gdb/testsuite/gdb.base/skip1.c
index 2dab5c3..fe63cd6 100644
--- a/gdb/testsuite/gdb.base/skip1.c
+++ b/gdb/testsuite/gdb.base/skip1.c
@@ -3,7 +3,7 @@  int bar()
   return 1;
 }

-int baz(int a, int b)
+int baz(int a)
 {
-  return a + b;
+  return a + 1;
 }