Isn't it OK to drop 'set write'?

Isn't it OK to drop 'set write'?

[Jan Kratochvil]

Hi,

GDB 'set write on' currently does not work, it has regressed in 7.9:
	"set write on" or "--write" corrupt the binary file
	https://sourceware.org/bugzilla/show_bug.cgi?id=18168
due to
	a485e98ea0cbb61ea9da1e7858da545e0bcf1a46 is the first bad commit
	Author: Alan Modra <amodra@gmail.com>
	    Move ELF section headers to end of object file

$ echo 'int foo=10;int main(){}'|gcc -g -x c -
$ cp -p a.out a.out-orig
$ gdb -q -ex 'set trace-commands on' -ex 'set write on' -ex 'file a.out' -ex 'set foo=20' -ex q
+set write on
+file a.out
Reading symbols from a.out...done.
+set foo=20
+q
Segmentation fault
$ cmp a.out a.out-orig && echo unchanged
unchanged

It is already unsupported for:
	writing into executable files is not supported for target: sysroots

I cannot much imagine when it is useful.  For updates of too big binaries
there is gold incremental linking.  Personally I have also never considered it
safe enough to use it myself, hexedit is there if one needs to create a weird
testfile.


Either there is some use case for that and so the fix should be 7.10 blocker.
Or the feature is no longer useful and it could be dropped which would
significantly simplify me a build-id patchset being prepared.


Jan

Re: Isn't it OK to drop 'set write'?

[Doug Evans]

On Sun, Jul 26, 2015 at 1:49 PM, Jan Kratochvil
<jan.kratochvil@redhat.com> wrote:
> Hi,
>
> GDB 'set write on' currently does not work, it has regressed in 7.9:
>         "set write on" or "--write" corrupt the binary file
>         https://sourceware.org/bugzilla/show_bug.cgi?id=18168
> due to
>         a485e98ea0cbb61ea9da1e7858da545e0bcf1a46 is the first bad commit
>         Author: Alan Modra <amodra@gmail.com>
>             Move ELF section headers to end of object file

Huh.

I'm curious how that broke things.

> $ echo 'int foo=10;int main(){}'|gcc -g -x c -
> $ cp -p a.out a.out-orig
> $ gdb -q -ex 'set trace-commands on' -ex 'set write on' -ex 'file a.out' -ex 'set foo=20' -ex q
> +set write on
> +file a.out
> Reading symbols from a.out...done.
> +set foo=20
> +q
> Segmentation fault
> $ cmp a.out a.out-orig && echo unchanged
> unchanged
>
> It is already unsupported for:
>         writing into executable files is not supported for target: sysroots
>
> I cannot much imagine when it is useful.  For updates of too big binaries
> there is gold incremental linking.  Personally I have also never considered it
> safe enough to use it myself, hexedit is there if one needs to create a weird
> testfile.
>
>
> Either there is some use case for that and so the fix should be 7.10 blocker.
> Or the feature is no longer useful and it could be dropped which would
> significantly simplify me a build-id patchset being prepared.

Hi.

OOC, how does it simplify the build-id patchset?

Re: Isn't it OK to drop 'set write'?

[Jan Kratochvil]

On Tue, 28 Jul 2015 19:51:02 +0200, Doug Evans wrote:
> OOC, how does it simplify the build-id patchset?

For writable files one needs to use gdb_bfd_fopen() but normally one has to
use gdb_bfd_open() which provides bfd-caching.  But they are called similarly
so that is not such a complication.

Rather the GDB codebase already contains an exception:
	writing into executable files is not supported for target: sysroots
where Gary wrote as a reason:
	/* gdb_bfd_fopen does not support "target:" filenames.  */
But I see it rather due to gdb_bfd_open() and not gdb_bfd_fopen(), more
specifically its gdb_bfd_openr_iovec() - the real underlying reason is that
BFD provides bfd_openr_iovec() but nothing like bfd_openrw_iovec().
As "target:" should be preferred in general this makes the writability a bit
limited functionality.

gdbserver should support FILEIO_O_RDWR so on the gdbserver protocol side that
should be OK.

For the build-id patchset I am unifying files opening API so all the
differences mess it up a bit.


Jan

Re: Isn't it OK to drop 'set write'?

[Joel Brobecker]

> I cannot much imagine when it is useful.  For updates of too big
> binaries there is gold incremental linking.  Personally I have also
> never considered it safe enough to use it myself, hexedit is there if
> one needs to create a weird testfile.

On Tru64 (which we no longer support), thread support required writing
into inferior memory; so if you wanted thread support when debugging
a core file, you needed to set write on.

We have a couple of tests that set it unconditionally on all platforms,
and it seems to have no adverse effect. Beyond that, I'm not sure.

-- 
Joel