[PATCH,tests] Run to main before doing any tests in structs3.exp

[PATCH,tests] Run to main before doing any tests in structs3.exp

[Luis Machado]

[-- Attachment #1: Type: text/plain, Size: 244 bytes --]

Hi,

On remote targets, we don't always start at the entry point. Thus, it is 
safer to get to main and do the tests from that point onwards.

This patch does exactly that, and fixes failures when testing this under 
some remote targets.

OK?


[-- Attachment #2: 0004-structs-fails.diff --]
[-- Type: text/x-patch, Size: 627 bytes --]

2012-11-06  Luis Machado  <lgustavo@codesourcery.com>

	* gdb.base/structs3.exp: Run to main before doing any tests.

Index: gdb-trunk/gdb/testsuite/gdb.base/structs3.exp
===================================================================
--- gdb-trunk.orig/gdb/testsuite/gdb.base/structs3.exp	2012-11-02 17:29:00.801049370 -0200
+++ gdb-trunk/gdb/testsuite/gdb.base/structs3.exp	2012-11-02 17:29:57.401050043 -0200
@@ -23,6 +23,11 @@ if { [prepare_for_testing structs3.exp "
     return -1
 }
 
+if ![runto_main] {
+    untested "could not run to main"
+    return -1
+}
+
 set vhn "\\$\[0-9\]+"
 
 # Check the real contents.

Re: [PATCH,tests] Run to main before doing any tests in structs3.exp

[Joel Brobecker]

> On remote targets, we don't always start at the entry point. Thus,
> it is safer to get to main and do the tests from that point onwards.

Can you elaborate more? Your patch looks reasonable at first sight,
but then at the same time now introduces a new requirement that it
needs to be run on the target, whereas that was not the case before.

The testcase only prints global variables. I don't suppose there
is much of a guaranty that you could print global variables without
starting the program first, but it's been generally working. So
I am curious as to why it isn't working in your case.

> 2012-11-06  Luis Machado  <lgustavo@codesourcery.com>
> 
> 	* gdb.base/structs3.exp: Run to main before doing any tests.
> 
> Index: gdb-trunk/gdb/testsuite/gdb.base/structs3.exp
> ===================================================================
> --- gdb-trunk.orig/gdb/testsuite/gdb.base/structs3.exp	2012-11-02 17:29:00.801049370 -0200
> +++ gdb-trunk/gdb/testsuite/gdb.base/structs3.exp	2012-11-02 17:29:57.401050043 -0200
> @@ -23,6 +23,11 @@ if { [prepare_for_testing structs3.exp "
>      return -1
>  }
>  
> +if ![runto_main] {
> +    untested "could not run to main"
> +    return -1
> +}
> +
>  set vhn "\\$\[0-9\]+"
>  
>  # Check the real contents.


-- 
Joel

Re: [PATCH,tests] Run to main before doing any tests in structs3.exp

[Luis Machado]

Hi Joel,

On 11/06/2012 03:30 PM, Joel Brobecker wrote:
>> On remote targets, we don't always start at the entry point. Thus,
>> it is safer to get to main and do the tests from that point onwards.
>
> Can you elaborate more? Your patch looks reasonable at first sight,
> but then at the same time now introduces a new requirement that it
> needs to be run on the target, whereas that was not the case before.
>
> The testcase only prints global variables. I don't suppose there
> is much of a guaranty that you could print global variables without
> starting the program first, but it's been generally working. So
> I am curious as to why it isn't working in your case.

We've been generally only testing these with native targets or with 
gdbserver on linux, both of which have well-known inferior startup 
procedures.

Consider, for example, a target that runs on QEMU. QEMU won't start the 
binary in exactly the same way as gdbserver running on linux. Thus, 
global variable initialization procedures may not have happened at that 
point yet. Consider that we start even before the dynamic loader had a 
chance to run and do all the relocation magic.

In this case, it makes no sense to try to print global variables since 
they will only contain garbage.

Effectively starting the binary and running to a known location helps 
avoid such a situation, and it does not change much for targets that 
already passed these tests.

Hopefully this explains things a little better.

Luis

Re: [PATCH,tests] Run to main before doing any tests in structs3.exp

[Pedro Alves]

On 11/06/2012 06:51 PM, Luis Machado wrote:
> Hi Joel,
> 
> On 11/06/2012 03:30 PM, Joel Brobecker wrote:
>>> On remote targets, we don't always start at the entry point. Thus,
>>> it is safer to get to main and do the tests from that point onwards.
>>
>> Can you elaborate more? Your patch looks reasonable at first sight,
>> but then at the same time now introduces a new requirement that it
>> needs to be run on the target, whereas that was not the case before.
>>
>> The testcase only prints global variables. I don't suppose there
>> is much of a guaranty that you could print global variables without
>> starting the program first, but it's been generally working. So
>> I am curious as to why it isn't working in your case.
> 
> We've been generally only testing these with native targets or with gdbserver on linux, both of which have well-known inferior startup procedures.
> 
> Consider, for example, a target that runs on QEMU. QEMU won't start the binary in exactly the same way as gdbserver running on linux. Thus, global variable initialization procedures may not have happened at that point yet. Consider that we start even before the dynamic loader had a chance to run and do all the relocation magic.
> 
> In this case, it makes no sense to try to print global variables since they will only contain garbage.
> 
> Effectively starting the binary and running to a known location helps avoid such a situation, and it does not change much for targets that already passed these tests.
> 
> Hopefully this explains things a little better.

The difference between native and remote debugging, is that on native
debugging, we don't start the inferior in any way at all.  Ideally we'd
make the test environments in both cases as similar as possible.
So we have two options:

#1 - Always run to main, so to get past startup for remote targets,
   thus on native debugging we'll start the inferior too.  This is your patch.

#2 - Don't start the inferior at all against remote targets too.
   We have several tests that do this, as not spawning the remote
   server cuts a bit of test time.  This is done by using gdb_file
   instead of gdb_load (it is usually gdb_load that spawns the remote
   side).

I haven't really thought if #2 makes sense for this test; I just wanted
to explain that option #2 exists.

-- 
Pedro Alves

Re: [PATCH,tests] Run to main before doing any tests in structs3.exp

[Luis Machado]

On 11/06/2012 05:12 PM, Pedro Alves wrote:
> On 11/06/2012 06:51 PM, Luis Machado wrote:
>> Hi Joel,
>>
>> On 11/06/2012 03:30 PM, Joel Brobecker wrote:
>>>> On remote targets, we don't always start at the entry point. Thus,
>>>> it is safer to get to main and do the tests from that point onwards.
>>>
>>> Can you elaborate more? Your patch looks reasonable at first sight,
>>> but then at the same time now introduces a new requirement that it
>>> needs to be run on the target, whereas that was not the case before.
>>>
>>> The testcase only prints global variables. I don't suppose there
>>> is much of a guaranty that you could print global variables without
>>> starting the program first, but it's been generally working. So
>>> I am curious as to why it isn't working in your case.
>>
>> We've been generally only testing these with native targets or with gdbserver on linux, both of which have well-known inferior startup procedures.
>>
>> Consider, for example, a target that runs on QEMU. QEMU won't start the binary in exactly the same way as gdbserver running on linux. Thus, global variable initialization procedures may not have happened at that point yet. Consider that we start even before the dynamic loader had a chance to run and do all the relocation magic.
>>
>> In this case, it makes no sense to try to print global variables since they will only contain garbage.
>>
>> Effectively starting the binary and running to a known location helps avoid such a situation, and it does not change much for targets that already passed these tests.
>>
>> Hopefully this explains things a little better.
>
> The difference between native and remote debugging, is that on native
> debugging, we don't start the inferior in any way at all.  Ideally we'd
> make the test environments in both cases as similar as possible.
> So we have two options:
>
> #1 - Always run to main, so to get past startup for remote targets,
>     thus on native debugging we'll start the inferior too.  This is your patch.
>
> #2 - Don't start the inferior at all against remote targets too.
>     We have several tests that do this, as not spawning the remote
>     server cuts a bit of test time.  This is done by using gdb_file
>     instead of gdb_load (it is usually gdb_load that spawns the remote
>     side).
>
> I haven't really thought if #2 makes sense for this test; I just wanted
> to explain that option #2 exists.
>

Thanks Pedro.

#2 seems to work just as well. We will read data straight from the 
binary's sections without having to touch real memory.

I'm fine with either solution.

Re: [PATCH,tests] Run to main before doing any tests in structs3.exp

[Joel Brobecker]

> Consider, for example, a target that runs on QEMU. QEMU won't start
> the binary in exactly the same way as gdbserver running on linux.
> Thus, global variable initialization procedures may not have
> happened at that point yet. Consider that we start even before the
> dynamic loader had a chance to run and do all the relocation magic.

The part I don't get is why this even enters the picture. If it was
a C++ program, with elaboration code that's run when the binary is
loaded in memory, I would understand. But this is a plain C program,
where I imagine the variables are simply located in data, with their
default value provided there. So, even if the program hasn't started
at all, GDB should be able to fetch it from the binary.

That being said, I don't want to hold your patch. I think it's fine
to run the program till main. I was just curious as to why this was
sometimes necessary. Still can't get it :-).

-- 
Joel

Re: [PATCH,tests] Run to main before doing any tests in structs3.exp

[Luis Machado]

Joe,

On 11/06/2012 10:26 PM, Joel Brobecker wrote:
>> Consider, for example, a target that runs on QEMU. QEMU won't start
>> the binary in exactly the same way as gdbserver running on linux.
>> Thus, global variable initialization procedures may not have
>> happened at that point yet. Consider that we start even before the
>> dynamic loader had a chance to run and do all the relocation magic.
>
> The part I don't get is why this even enters the picture. If it was
> a C++ program, with elaboration code that's run when the binary is
> loaded in memory, I would understand. But this is a plain C program,
> where I imagine the variables are simply located in data, with their
> default value provided there. So, even if the program hasn't started
> at all, GDB should be able to fetch it from the binary.
>
> That being said, I don't want to hold your patch. I think it's fine
> to run the program till main. I was just curious as to why this was
> sometimes necessary. Still can't get it :-).
>

I find the discussion and in-depth view really important, so i 
appreciate your comments.

There is, of course, an explanation. :-)

The real issue arises from two points:

1 - As Pedro mentioned, the testsuite does not start the inferior for 
native debugging, but does so for remote targets.

2 - On startup, some targets relocate the binary's sections during 
inferior execution. When this happens, data is shifted from location X 
to location Y.

In the specific case of structs3, we have "two", the structure, and 
"twop", a pointer to the structure.

The whole structure is properly relocated, so attempting to print "two" 
will yield the correct result.

When gdb tries to print the contents of "twop", that's where the problem 
shows up.

Though "two" and "twop" have been properly relocated, the value of 
"twop" is still pointing at the location before relocation, thus the 
printed values are wrong.

The contents of "twop" will eventually be fixed up by a dynamic 
relocation during ld.so's execution (which, in my case, did not start yet).

Pedro's suggestion of simply loading the binary via gdb_file_cmd works 
for me though. So there is no need to effectively start the inferior.

Luis

Re: [PATCH,tests] Run to main before doing any tests in structs3.exp

[Joel Brobecker]

> Though "two" and "twop" have been properly relocated, the value of
> "twop" is still pointing at the location before relocation, thus the
> printed values are wrong.

Ha! I get it, now; thanks, Luis.

> Pedro's suggestion of simply loading the binary via gdb_file_cmd
> works for me though. So there is no need to effectively start the
> inferior.

Like Pedro, I think we should try to be as standard as possible.
Starting the inferior isn't a real issue.

-- 
Joel

Re: [PATCH,tests] Run to main before doing any tests in structs3.exp

[Luis Machado]

On 11/07/2012 01:08 PM, Joel Brobecker wrote:
>> Though "two" and "twop" have been properly relocated, the value of
>> "twop" is still pointing at the location before relocation, thus the
>> printed values are wrong.
>
> Ha! I get it, now; thanks, Luis.
>
>> Pedro's suggestion of simply loading the binary via gdb_file_cmd
>> works for me though. So there is no need to effectively start the
>> inferior.
>
> Like Pedro, I think we should try to be as standard as possible.
> Starting the inferior isn't a real issue.
>

Thanks. I've checked it in now.

Luis