[RFA] Dead variables, remote.c

While poking in remote.c I noticed some unused local variables,
and (with gcc's help) found 28 of them in 16 functions.

Not a huge priority, but should actually make the code easier to 
understand...

2009-07-31  Michael Snyder  <msnyder <at> vmware.com>

	* remote.c (remote_add_inferior): Remove unused local.
	(remote_add_thread): Ditto.
	(read_ptid): Ditto.
	(remote_current_thread): Ditto.
	(remote_stop_ns): Ditto.
	(remote_parse_stop_reply): Ditto.
	(remote_get_pending_stop_replies): Ditto.
	(remote_wait_ns): Ditto.
	(remote_wait_as): Ditto.
	(send_g_packet): Ditto.
	(remote_fetch_registers): Ditto.
	(store_register_using_P): Ditto.
	(remote_store_registers): Ditto.
	(remote_remove_breakpoint): Ditto.
	(remote_write_qxfer): Ditto.
	(remote_read_qxfer): Ditto.

Index: remote.c
===================================================================
RCS file: /cvs/src/src/gdb/remote.c,v

Simplify MI breakpoint setting

I've noticed that mi_cmd_break_insert does some fairly
contrived logic which amounts to passing either 0 or 1
as the value of 'hardware' parameter to set_breakpoint.
Further, set_breakpoint appears to be trivial and useless
wrapper over break_command_really. This patch cleans that
up. Are breakpoint.{c,h} changes OK?

- Volodya

[RFA/RFC] Add dump and load command to process record and replay

Hi,

This patch add command record dump and record load to make prec can
dump execution log to a file and load it later.  And load can work
with core file.

And I include "mem_entry_not_accessible"(not_replay) patch to this
patch because when replay with core file, some address in lib cannot
be read and write.

For example:
./gdb ./a.out
GNU gdb (GDB) 6.8.50.20090801-cvs
Copyright (C) 2009 Free Software Foundation, Inc.
License GPLv3+: GNU GPL version 3 or later <http://gnu.org/licenses/gpl.html>
This is free software: you are free to change and redistribute it.
There is NO WARRANTY, to the extent permitted by law.  Type "show copying"
and "show warranty" for details.
This GDB was configured as "i686-pc-linux-gnu".
For bug reporting instructions, please see:
<http://www.gnu.org/software/gdb/bugs/>...
Setting up the environment for debugging gdb.
Function "internal_error" not defined.
Make breakpoint pending on future shared library load? (y or [n])
[answered N; input not from terminal]
Function "info_command" not defined.
Make breakpoint pending on future shared library load? (y or [n])
[answered N; input not from terminal]
/home/teawater/gdb/bgdbno/gdb/.gdbinit:8: Error in sourced command file:
No breakpoint number 0.

Re: [RFA/RFC] Add dump and load command to process record and replay

> From: Hui Zhu <teawater <at> gmail.com>
> Date: Sat, 1 Aug 2009 15:30:38 +0800
> Cc: Michael Snyder <msnyder <at> vmware.com>, Eli Zaretskii <eliz <at> gnu.org>
> 
> This patch add command record dump and record load to make prec can
> dump execution log to a file and load it later.  And load can work
> with core file.

Thanks.

> +      /* Default corefile name is "rec.PID".  */

I suggest the default name to be "gdb_record.PID".

> +  if (write (recfd, &magic, 4) != 4)
> +    error (_("Failed to write '%s' for dump."), recfilename);

I suggest to change the text (here and elsewhere in your patch) to:

  "Failed to write dump of execution records to `%s'."

I also suggest to replace the numerous fragments like this:

> +          if (write (recfd, &tmpu8, 1) != 1)
> +            error (_("Failed to write '%s' for dump."), recfilename);

with calls to a function which tries to write and errors out of it
cannot.  That would leave only one "Failed to write ..." string,
instead of having gobs of them.

Re: [unladen-swallow] Re: [RFA] Add interface for registering JITed code

Em Sexta-feira 31 Julho 2009 15:17:24 Reid Kleckner escreveu:
> On Fri, Jul 31, 2009 at 8:01 AM, Thiago Jung
> > I still think that the patch needs to be tested with a core file scenario
> > before it is checked in. Since said scenario is similar to attaching to a
> > running process, it will likely magically work, though.
>
> Well, it didn't magically work because the jit_inferior_create_hook
> was getting called twice for core files instead of once.  What would
> happen is that the binary would get loaded, the hook called, there
> would be no debug info, the memory would get loaded, it would get
> called again, and it would think that it already read the descriptor.
> So I made it look for new code every time just in case, and now it
> works.

That's great. Thanks for fixing this issue.

> Here's an updated patch.  What else needs to be dealt with?

Looks like it just needs the blessing from the GDB powers that be, now.
--

-- 
[]'s
Thiago Jung Bauermann

Re: [RFA/RFC] Add dump and load command to process record and replay

Hui Zhu wrote:
> Hi,
> 
> This patch add command record dump and record load to make prec can
> dump execution log to a file and load it later.  And load can work
> with core file.
> 
> And I include "mem_entry_not_accessible"(not_replay) patch to this
> patch because when replay with core file, some address in lib cannot
> be read and write.

Augh!  No -- please, this is lumping too much together.
Smaller, more discrete patches are much easier to review.

1) You already have a patch thread for the memory-not-accessible
change -- let's finish that idea there, not lump it in here.

2) I think the idea of saving and restoring the recording
to a file is GREAT -- but can you please give us a patch
that adds that, by itself.

3) I don't really understand how core files fit into this,
but I'd love to discuss that idea in a separate patch thread.
It really adds a lot of complexity to this patch, which it seems
to me could be much simpler and easier to review if it added
only one thing at a time.

Re: [PREC/RFA] Add not_replay to make precord support release memory better

Hui Zhu wrote:
> On Sun, Jul 26, 2009 at 04:41, Michael Snyder<msnyder <at> vmware.com> wrote:
>> Hui Zhu wrote:
>>> On Fri, Jul 24, 2009 at 10:10, Michael Snyder<msnyder <at> vmware.com> wrote:
>>>> 1) During the recording "pass", there's no change.
>>>> 2) During the reverse-replay pass, if the memory is
>>>> not readable or writable, we will set this flag to TRUE.
>>>> 3) Finally, during the forward-replay pass, if the flag
>>>> has previously been set to TRUE, we will skip this entry
>>>> (and set the flag to FALSE.)
>>>>
>>>> So my question is -- is there any reason to set it to FALSE here?
>>>> Is there any way that the memory can ever become readable again?
>>>>
>>>> Seems to me, once it is set TRUE, we might as well just leave it TRUE.
>>>> Am I right?
>>> I thought about it too.  I think if we don't need this entry.  We can
>>> delete it directly.
>>> But there is a special status, after release memory, inferior alloc
>>> some memory and its address is same with the memory that released
>>> memory.  Then the memory entry will can be use in this status.  User
>>> can get the right value of memory before this entry.  So I think maybe
>>> we can keep it.
>>>
>>> What do you think about it?
>> Let's say a program does something like this:
>>
>> buf = mmap (...);
>> munmap (...);
>> buf = mmap (...);

Re: [RFA/RFC] Add dump and load command to process record and replay

Michael Snyder wrote:

> 3) I don't really understand how core files fit into this,
> but I'd love to discuss that idea in a separate patch thread.

Oh, sorry -- I see how they're related now.
Very clever, actually.

I'd still like to see them submitted separately, though,
because:
1) the dump/restore part of the patch is cleaner and
closer to being acceptable, and can really be used on
its own, while
2) the corefile part of the patch is kind of messy and
prototype-ish, and I think needs much more discussion and
cleaning up before it will be ready.

Re: [PREC/RFA] Add not_replay to make precord support release memory better

I think this patch is very good.

I a new changelog for it:
2009-08-02  Michael Snyder <msnyder <at> vmware.com>
            Hui Zhu  <teawater <at> gmail.com>

	* record.c (record_mem_entry): New field
	'mem_entry_not_accessible'.
	(record_arch_list_add_mem): Initialize
	'mem_entry_not_accessible' to 0.
	(record_wait): Set 'mem_entry_not_accessible' flag if target
	memory not readable.  Don't try to change target memory if
	'mem_entry_not_accessible' is set.

Do you think it's ok?

Thanks,
Hui

On Sun, Aug 2, 2009 at 07:00, Michael Snyder<msnyder <at> vmware.com> wrote:
> Hui Zhu wrote:
>>
>> On Sun, Jul 26, 2009 at 04:41, Michael Snyder<msnyder <at> vmware.com> wrote:
>>>
>>> Hui Zhu wrote:
>>>>
>>>> On Fri, Jul 24, 2009 at 10:10, Michael Snyder<msnyder <at> vmware.com> wrote:
>>>>>
>>>>> 1) During the recording "pass", there's no change.
>>>>> 2) During the reverse-replay pass, if the memory is

Re: [RFA/RFC] Add dump and load command to process record and replay

OK.  I will post new patches for them, memory and dump.

Thanks,
Hui

On Sun, Aug 2, 2009 at 11:17, Michael Snyder<msnyder <at> vmware.com> wrote:
> Michael Snyder wrote:
>
>> 3) I don't really understand how core files fit into this,
>> but I'd love to discuss that idea in a separate patch thread.
>
> Oh, sorry -- I see how they're related now.
> Very clever, actually.
>
> I'd still like to see them submitted separately, though,
> because:
> 1) the dump/restore part of the patch is cleaner and
> closer to being acceptable, and can really be used on
> its own, while
> 2) the corefile part of the patch is kind of messy and
> prototype-ish, and I think needs much more discussion and
> cleaning up before it will be ready.
>