diff mbox

compile: Fix MinGW build [Re: [mingw rfc] Add mkdtemp to gdb/gnulib/]

Message ID 20141215222801.GA28138@host2.jankratochvil.net
State New
Headers show

Commit Message

Jan Kratochvil Dec. 15, 2014, 10:28 p.m. UTC
On Mon, 15 Dec 2014 19:57:28 +0100, Eli Zaretskii wrote:
> > On Mon, 15 Dec 2014 04:15:43 +0100, Yao Qi wrote:
> > # or maybe we have to use win32 api, such as GetTempPath and GetRandomFileName.
> 
> If you write it, I can test it.

In the end I have managed to test it by Wine myself:

$ wine build_win32/gdb/gdb.exe -q build_win32/gdb/gdb.exe -ex start -ex 'compile code 1' -ex 'set confirm no' -ex quit
[...]
Temporary breakpoint 1, main (argc=1, argv=0x241418) at ../../gdb/gdb.c:29
29	  args.argc = argc;
Could not load libcc1.so: Module not found.

Even if it managed to load libcc1.so (it needs host-dependent name libcc1.dll)
then it would soon end up at least on:

default_infcall_mmap:
  error (_("This target does not support inferior memory allocation by mmap."));

As currently there is only:

linux-tdep.c:
  set_gdbarch_infcall_mmap (gdbarch, linux_infcall_mmap);

While one could debug Linux targets from MS-Windows host I find it somehow
overcomplicated now when we are trying to get it running at least on native
Linux x86*.

The 'compile' project needs a larger port effort to run on MS-Windows.

OK for check-in?


Thanks,
Jan
gdb/ChangeLog
2014-12-15  Jan Kratochvil  <jan.kratochvil@redhat.com>

	Fix MinGW compilation.
	* compile/compile.c (get_compile_file_tempdir): Call error on _WIN32.

Comments

Jan Kratochvil Dec. 15, 2014, 10:55 p.m. UTC | #1
On Mon, 15 Dec 2014 23:28:01 +0100, Jan Kratochvil wrote:
> The 'compile' project needs a larger port effort to run on MS-Windows.

For example there is also

compile/compile.c:
  zap = concat ("rm -rf ", dir, (char *) NULL);
  system (zap);

which is AFAIK a no-go on MinGW.


Jan
Eli Zaretskii Dec. 16, 2014, 3:37 a.m. UTC | #2
> Date: Mon, 15 Dec 2014 23:28:01 +0100
> From: Jan Kratochvil <jan.kratochvil@redhat.com>
> Cc: brobecker@adacore.com, yao@codesourcery.com, gdb-patches@sourceware.org,
>         ktietz@redhat.com
> 
> Even if it managed to load libcc1.so (it needs host-dependent name libcc1.dll)
> then it would soon end up at least on:
> 
> default_infcall_mmap:
>   error (_("This target does not support inferior memory allocation by mmap."));
> 
> As currently there is only:
> 
> linux-tdep.c:
>   set_gdbarch_infcall_mmap (gdbarch, linux_infcall_mmap);

Why is mmap needed here?

> OK for check-in?
> 
> 
> Thanks,
> Jan
> gdb/ChangeLog
> 2014-12-15  Jan Kratochvil  <jan.kratochvil@redhat.com>
> 
> 	Fix MinGW compilation.
> 	* compile/compile.c (get_compile_file_tempdir): Call error on _WIN32.
> 
> --- ./gdb/compile/compile.c	2014-12-14 02:48:38.000000000 +0100
> +++ ./gdb/compile/compile.c	2014-12-15 23:21:28.788716340 +0100
> @@ -191,7 +191,11 @@ get_compile_file_tempdir (void)
>  
>    strcpy (tname, TEMPLATE);
>  #undef TEMPLATE
> +#ifdef _WIN32
> +  error (_("mkdtemp needs to be implemented for MS-Windows hosts"));
> +#else
>    tempdir_name = mkdtemp (tname);
> +#endif
>    if (tempdir_name == NULL)
>      perror_with_name (_("Could not make temporary directory"));

I think _WIN32 will catch Cygwin as well, which is not what you want.

Why not just check HAVE_MKDTEMP?
Eli Zaretskii Dec. 16, 2014, 3:39 a.m. UTC | #3
> Date: Mon, 15 Dec 2014 23:55:51 +0100
> From: Jan Kratochvil <jan.kratochvil@redhat.com>
> Cc: brobecker@adacore.com, yao@codesourcery.com, gdb-patches@sourceware.org,
>         ktietz@redhat.com
> 
> On Mon, 15 Dec 2014 23:28:01 +0100, Jan Kratochvil wrote:
> > The 'compile' project needs a larger port effort to run on MS-Windows.
> 
> For example there is also
> 
> compile/compile.c:
>   zap = concat ("rm -rf ", dir, (char *) NULL);
>   system (zap);

Yuck!  Do we really allow such atrocities in GDB?  What if 'rm' is
some unrelated or even malicious script?

I think this should be replaced by suitable C function calls.  IMO,
this kind of programming is OK for prototyping, but not for the final
code.
Kai Tietz Dec. 16, 2014, 9:04 a.m. UTC | #4
----- Ursprüngliche Mail -----
> > Date: Mon, 15 Dec 2014 23:55:51 +0100
> > From: Jan Kratochvil <jan.kratochvil@redhat.com>
> > Cc: brobecker@adacore.com, yao@codesourcery.com,
> > gdb-patches@sourceware.org,
> >         ktietz@redhat.com
> > 
> > On Mon, 15 Dec 2014 23:28:01 +0100, Jan Kratochvil wrote:
> > > The 'compile' project needs a larger port effort to run on MS-Windows.
> > 
> > For example there is also
> > 
> > compile/compile.c:
> >   zap = concat ("rm -rf ", dir, (char *) NULL);
> >   system (zap);
> 
> Yuck!  Do we really allow such atrocities in GDB?  What if 'rm' is
> some unrelated or even malicious script?
> 
> I think this should be replaced by suitable C function calls.  IMO,
> this kind of programming is OK for prototyping, but not for the final
> code.
> 

Ouch, to use console is for sure a bad thing.  To use POSIX-shell commands is even worse.  Why not using here instead an implementation using FTW-API?  At least mingw-w64 added this API recently to runtime for gcc's sake, so implementation of an 'rm -rf' should be pretty easy.

Kai
Kai Tietz Dec. 16, 2014, 9:06 a.m. UTC | #5
----- Ursprüngliche Mail -----
> > Date: Mon, 15 Dec 2014 23:28:01 +0100
> > From: Jan Kratochvil <jan.kratochvil@redhat.com>
> > Cc: brobecker@adacore.com, yao@codesourcery.com,
> > gdb-patches@sourceware.org,
> >         ktietz@redhat.com
> > 
> > Even if it managed to load libcc1.so (it needs host-dependent name
> > libcc1.dll)
> > then it would soon end up at least on:
> > 
> > default_infcall_mmap:
> >   error (_("This target does not support inferior memory allocation by
> >   mmap."));
> > 
> > As currently there is only:
> > 
> > linux-tdep.c:
> >   set_gdbarch_infcall_mmap (gdbarch, linux_infcall_mmap);
> 
> Why is mmap needed here?
> 
> > OK for check-in?
> > 
> > 
> > Thanks,
> > Jan
> > gdb/ChangeLog
> > 2014-12-15  Jan Kratochvil  <jan.kratochvil@redhat.com>
> > 
> > 	Fix MinGW compilation.
> > 	* compile/compile.c (get_compile_file_tempdir): Call error on _WIN32.
> > 
> > --- ./gdb/compile/compile.c	2014-12-14 02:48:38.000000000 +0100
> > +++ ./gdb/compile/compile.c	2014-12-15 23:21:28.788716340 +0100
> > @@ -191,7 +191,11 @@ get_compile_file_tempdir (void)
> >  
> >    strcpy (tname, TEMPLATE);
> >  #undef TEMPLATE
> > +#ifdef _WIN32
> > +  error (_("mkdtemp needs to be implemented for MS-Windows hosts"));
> > +#else
> >    tempdir_name = mkdtemp (tname);
> > +#endif
> >    if (tempdir_name == NULL)
> >      perror_with_name (_("Could not make temporary directory"));
> 
> I think _WIN32 will catch Cygwin as well, which is not what you want.
> 
> Why not just check HAVE_MKDTEMP?
> 

Well, _WIN32 isn't set necessarily for Cygwin.  For Cygwin this define is just set if the windows.h header got included AFAIK.  Anyway, there are better ways to check, and indeed HAVE_MKDTEMP seems to be the most correct check, isn't it?

Kai
Pierre Muller Dec. 16, 2014, 12:40 p.m. UTC | #6
Hi Jan,

  I tested your patch,
which appeared as an attached file named 1.sql,
which made it difficult to guess it was indeed the patch...

  Nevertheless, your patch fixes compilation error for mingw32
and as such it should be committed as an obvious fix.

  To go a little bit further, I tried to use 
the compile command on the newly generated mingw32 GDB executable.
 
using ./gdb ./gdb
(with a minimal hello.c source code)

<<<<  (top-gdb) compile code hello.c
returns
>>>> The program must be running for the compile command to work.
I am not sure why this restriction should apply.

 (top-gdb) start
Temporary breakpoint 3 at 0x40158a: file ../../../binutils-gdb/gdb/gdb.c, line 28.
Starting program: E:\cygwin-32\home\Pierre\git\build\mult-mingw32\gdb\gdb.exe
[New Thread 6572.0x4e0]

Temporary breakpoint 3, main (argc=1, argv=0x3d4b30)
    at ../../../binutils-gdb/gdb/gdb.c:28
 (top-gdb) compile file hello.c
Could not load libcc1.so: "libcc1.so": Le module spécifié est introuvable.
 (French meaning: "the specified module can't be found").

  So the mkdtmp is not the only problem 
for mingw compile support.


Pierre Muller
Yao Qi Dec. 17, 2014, 1:21 a.m. UTC | #7
Jan Kratochvil <jan.kratochvil@redhat.com> writes:

> In the end I have managed to test it by Wine myself:
>
> $ wine build_win32/gdb/gdb.exe -q build_win32/gdb/gdb.exe -ex start
> -ex 'compile code 1' -ex 'set confirm no' -ex quit
> [...]
> Temporary breakpoint 1, main (argc=1, argv=0x241418) at ../../gdb/gdb.c:29
> 29	  args.argc = argc;
> Could not load libcc1.so: Module not found.
>
> Even if it managed to load libcc1.so (it needs host-dependent name libcc1.dll)
> then it would soon end up at least on:
>
> default_infcall_mmap:
>   error (_("This target does not support inferior memory allocation by mmap."));
>
> As currently there is only:
>
> linux-tdep.c:
>   set_gdbarch_infcall_mmap (gdbarch, linux_infcall_mmap);

The module not found problem and unsupported mmap problem are separated
ones.  They are not related to the mkdtemp problem we are discussing
here, IMO.

>
> While one could debug Linux targets from MS-Windows host I find it somehow
> overcomplicated now when we are trying to get it running at least on native
> Linux x86*.
>
> The 'compile' project needs a larger port effort to run on MS-Windows.

If 'compile' feature on mingw/Windows doesn't work, why don't we disable
it on Windows?
Pedro Alves Dec. 17, 2014, 10:56 a.m. UTC | #8
On 12/17/2014 01:21 AM, Yao Qi wrote:
>> >
>> > While one could debug Linux targets from MS-Windows host I find it somehow
>> > overcomplicated now when we are trying to get it running at least on native
>> > Linux x86*.
>> >
>> > The 'compile' project needs a larger port effort to run on MS-Windows.
> If 'compile' feature on mingw/Windows doesn't work, why don't we disable
> it on Windows?

You mean, not compile the whole set of compile/ files on Windows?  I think we're
not that far off from making the command usable on all hosts.  I'd rather keep
it building everywhere, and guard the specific bits that need porting
or better abstractions guarded with #if HAVE_FOO.

Thanks,
Pedro Alves
Pedro Alves Dec. 17, 2014, 11:15 a.m. UTC | #9
On 12/15/2014 10:28 PM, Jan Kratochvil wrote:
> On Mon, 15 Dec 2014 19:57:28 +0100, Eli Zaretskii wrote:
>>> On Mon, 15 Dec 2014 04:15:43 +0100, Yao Qi wrote:
>>> # or maybe we have to use win32 api, such as GetTempPath and GetRandomFileName.
>>
>> If you write it, I can test it.
> 
> In the end I have managed to test it by Wine myself:
> 
> $ wine build_win32/gdb/gdb.exe -q build_win32/gdb/gdb.exe -ex start -ex 'compile code 1' -ex 'set confirm no' -ex quit
> [...]
> Temporary breakpoint 1, main (argc=1, argv=0x241418) at ../../gdb/gdb.c:29
> 29	  args.argc = argc;
> Could not load libcc1.so: Module not found.
> 
> Even if it managed to load libcc1.so (it needs host-dependent name libcc1.dll)
> then it would soon end up at least on:
> 
> default_infcall_mmap:
>   error (_("This target does not support inferior memory allocation by mmap."));
> 
> As currently there is only:
> 
> linux-tdep.c:
>   set_gdbarch_infcall_mmap (gdbarch, linux_infcall_mmap);
> 
> While one could debug Linux targets from MS-Windows host I find it somehow
> overcomplicated now when we are trying to get it running at least on native
> Linux x86*.
> 
> The 'compile' project needs a larger port effort to run on MS-Windows.
> 
> OK for check-in?

Can you send a version that does an autoconf check for mkdtemp instead?

> +  error (_("mkdtemp needs to be implemented for MS-Windows hosts"));

I think the error text should be a bit more generic and not mention a
particular implementation detail, like "Command not supported on this host." or
some such -- I think the testsuite will need to be extended to handle this error,
presumably in skip_compile_feature_tests.

Thanks,
Pedro Alves
Yao Qi Dec. 17, 2014, 11:40 a.m. UTC | #10
Pedro Alves <palves@redhat.com> writes:

> You mean, not compile the whole set of compile/ files on Windows?  I think we're

Yes.

> not that far off from making the command usable on all hosts.  I'd rather keep
> it building everywhere, and guard the specific bits that need porting
> or better abstractions guarded with #if HAVE_FOO.

At least, user should get an explicit message that 'compile' command
isn't supported if the user run the 'compile' command on Windows for example.
Pedro Alves Dec. 17, 2014, 11:43 a.m. UTC | #11
On 12/17/2014 11:40 AM, Yao Qi wrote:

> At least, user should get an explicit message that 'compile' command
> isn't supported if the user run the 'compile' command on Windows for example.

Agreed.  That's what I suggested in the other mail too:

 https://sourceware.org/ml/gdb-patches/2014-12/msg00475.html

Would that be OK with you?

Thanks,
Pedro Alves
Yao Qi Dec. 17, 2014, 11:55 a.m. UTC | #12
On 12/17/2014 07:43 PM, Pedro Alves wrote:
> Agreed.  That's what I suggested in the other mail too:
> 
>  https://sourceware.org/ml/gdb-patches/2014-12/msg00475.html
> 
> Would that be OK with you?

Yes, that is OK to me.
Steve Ellcey Dec. 17, 2014, 5:29 p.m. UTC | #13
On Tue, 2014-12-16 at 05:39 +0200, Eli Zaretskii wrote:
> > Date: Mon, 15 Dec 2014 23:55:51 +0100
> > From: Jan Kratochvil <jan.kratochvil@redhat.com>
> > Cc: brobecker@adacore.com, yao@codesourcery.com, gdb-patches@sourceware.org,
> >         ktietz@redhat.com
> > 
> > On Mon, 15 Dec 2014 23:28:01 +0100, Jan Kratochvil wrote:
> > > The 'compile' project needs a larger port effort to run on MS-Windows.
> > 
> > For example there is also
> > 
> > compile/compile.c:
> >   zap = concat ("rm -rf ", dir, (char *) NULL);
> >   system (zap);
> 
> Yuck!  Do we really allow such atrocities in GDB?  What if 'rm' is
> some unrelated or even malicious script?
> 
> I think this should be replaced by suitable C function calls.  IMO,
> this kind of programming is OK for prototyping, but not for the final
> code.

I am running into a different problem with the system call.  I am
building on Linux using the latest GCC and glibc (unreleased top of tree
sources) and my build fails with:

/scratch/sellcey/repos/nightly2/src/binutils-gdb/gdb/compile/compile.c:175:10: error: ignoring return value of 'system', declared with attribute warn_unused_result [-Werror=unused-result]
cc1: all warnings being treated as errors
make[1]: *** [compile.o] Error 1

If we don't get rid of the system call can we at least check the return
value of system?

Steve Ellcey
sellcey@imgtec.com
Jan Kratochvil Dec. 17, 2014, 6:21 p.m. UTC | #14
Hi Pierre,

On Tue, 16 Dec 2014 13:40:28 +0100, Pierre Muller wrote:
> which appeared as an attached file named 1.sql,
> which made it difficult to guess it was indeed the patch...

The mail has:
	Content-Type: text/plain; charset=us-ascii
	Content-Disposition: inline; filename=1

So there was no ".sql" extension; there was just no extension at all.


>   To go a little bit further, I tried to use 
> the compile command on the newly generated mingw32 GDB executable.
>  
> using ./gdb ./gdb
> (with a minimal hello.c source code)
> 
> <<<<  (top-gdb) compile code hello.c
> returns
> >>>> The program must be running for the compile command to work.
> I am not sure why this restriction should apply.

I do not understand what it should do without running inferior.  Or rather
I can understand it but that would be a functionality completely unrelated to
the current 'compile' functionality - which hooks into the running inferior.
I would find it rather confusing myself.


>  (top-gdb) start
> Temporary breakpoint 3 at 0x40158a: file ../../../binutils-gdb/gdb/gdb.c, line 28.
> Starting program: E:\cygwin-32\home\Pierre\git\build\mult-mingw32\gdb\gdb.exe
> [New Thread 6572.0x4e0]
> 
> Temporary breakpoint 3, main (argc=1, argv=0x3d4b30)
>     at ../../../binutils-gdb/gdb/gdb.c:28
>  (top-gdb) compile file hello.c
> Could not load libcc1.so: "libcc1.so": Le module spécifié est introuvable.
>  (French meaning: "the specified module can't be found").
> 
>   So the mkdtmp is not the only problem 
> for mingw compile support.

That is correct, you need to build gcc-5.0 which contains libcc1.so.
But that currently also will not work as it will get named libcc1.dll, you can
read more in:
	https://sourceware.org/ml/gdb-patches/2014-12/msg00429.html


Thanks,
Jan
Jan Kratochvil Dec. 17, 2014, 7:15 p.m. UTC | #15
On Wed, 17 Dec 2014 12:43:35 +0100, Pedro Alves wrote:
> On 12/17/2014 11:40 AM, Yao Qi wrote:
> 
> > At least, user should get an explicit message that 'compile' command
> > isn't supported if the user run the 'compile' command on Windows for example.
> 
> Agreed.  That's what I suggested in the other mail too:
> 
>  https://sourceware.org/ml/gdb-patches/2014-12/msg00475.html
> 
> Would that be OK with you?

This is not completely correct.  With the patch above being checked in now the
'compile' command will still fail on:

$ wine build_win32/gdb/gdb.exe -q build_win32/gdb/gdb.exe -ex start -ex 'compile code 1' -ex 'set confirm no' -ex quit
[...]
Temporary breakpoint 1, main (argc=1, argv=0x241418) at ../../gdb/gdb.c:29
29        args.argc = argc;
Could not load libcc1.so: Module not found.

It would need some special exception.

So far it is obvious that one needs gcc-5.0 which "nobody" has.  But after
gcc-5.0 gets released MinGW gdb.exe will still print "Could not load libcc1.so"
while (I expect) MinGW gcc-5.0 will have "libcc1.dll".


Jan
Jan Kratochvil Dec. 17, 2014, 7:17 p.m. UTC | #16
On Tue, 16 Dec 2014 04:37:15 +0100, Eli Zaretskii wrote:
> > default_infcall_mmap:
> >   error (_("This target does not support inferior memory allocation by mmap."));
> > 
> > As currently there is only:
> > 
> > linux-tdep.c:
> >   set_gdbarch_infcall_mmap (gdbarch, linux_infcall_mmap);
> 
> Why is mmap needed here?

The project obviously needs something like:
	(gdb) call dlopen("just compile piece of code.so");

But as dlopen() is intrusive to the inferior Tom decided it is better to do
much less intrusive
	(gdb) print mmap(...)

instead and reimplement what inferior dlopen() does
in gdb/compile/compile-object-load.c - which is being done.

Without mmap() there is nowhere to load the compiled code at.



Jan
Eli Zaretskii Dec. 17, 2014, 7:31 p.m. UTC | #17
> Date: Wed, 17 Dec 2014 20:17:55 +0100
> From: Jan Kratochvil <jan.kratochvil@redhat.com>
> Cc: brobecker@adacore.com, yao@codesourcery.com, gdb-patches@sourceware.org,
>         ktietz@redhat.com
> 
> > > linux-tdep.c:
> > >   set_gdbarch_infcall_mmap (gdbarch, linux_infcall_mmap);
> > 
> > Why is mmap needed here?
> 
> The project obviously needs something like:
> 	(gdb) call dlopen("just compile piece of code.so");
> 
> But as dlopen() is intrusive to the inferior Tom decided it is better to do
> much less intrusive
> 	(gdb) print mmap(...)
> 
> instead and reimplement what inferior dlopen() does
> in gdb/compile/compile-object-load.c - which is being done.

Why is dlopen "intrusive"?  ISTM that using the Windows equivalent of
dlopen is exactly the right thing here.
Jan Kratochvil Dec. 17, 2014, 7:37 p.m. UTC | #18
On Wed, 17 Dec 2014 20:31:18 +0100, Eli Zaretskii wrote:
> Why is dlopen "intrusive"?  ISTM that using the Windows equivalent of
> dlopen is exactly the right thing here.

First I do not want to advocate the mmap() + ld.so reimplementation too much
as I would implement it the dlopen() way (at least in the initial
implementation) myself.

But AFAIK (but I do not know much) MS-Windows implements its dlopen-like
feature in MS-Windows kernel which may be safe.  Contrary to it GNU/Linux
implements dlopen() fully in glibc - userland - calling from kernel only that
mmap() to map the .so file from disk, map some new pages of memory etc. By
having to modify the _r_debug.r_map link list which is in many times corrupted
when debugging crashed program GDB would crash the inferior even more on
'compile code' command.


Jan
Pedro Alves Dec. 17, 2014, 8:34 p.m. UTC | #19
On 12/17/2014 07:31 PM, Eli Zaretskii wrote:
>> Date: Wed, 17 Dec 2014 20:17:55 +0100
>> From: Jan Kratochvil <jan.kratochvil@redhat.com>
>> Cc: brobecker@adacore.com, yao@codesourcery.com, gdb-patches@sourceware.org,
>>         ktietz@redhat.com
>>
>>>> linux-tdep.c:
>>>>   set_gdbarch_infcall_mmap (gdbarch, linux_infcall_mmap);
>>>
>>> Why is mmap needed here?
>>
>> The project obviously needs something like:
>> 	(gdb) call dlopen("just compile piece of code.so");
>>
>> But as dlopen() is intrusive to the inferior Tom decided it is better to do
>> much less intrusive
>> 	(gdb) print mmap(...)
>>
>> instead and reimplement what inferior dlopen() does
>> in gdb/compile/compile-object-load.c - which is being done.
> 
> Why is dlopen "intrusive"?

Pasting here what I said recently on a glibc thread, re. reasons
for not using dlopen for this.

Off the top of my head, I'm sure there are more:

 - The user might want to evaluate an expression while the program itself
   has just called dlopen and is now stopped inside it.  This pesky dlopen
   recursion thing.    It's best if GDB only calls async-signal
   safe functions behind the scenes, if possible.  Of course if the
   injected expression involves calls to async-signal unsafe code that breaks
   the inferior, the user gets what she asked for.

 - The program might have not been linked with -ldl.

 - I suspect there may be issues with messing with symbol resolution
   and self library walks in the inferior too.  Not sure if RTLD_LOCAL is
   enough.  dlmopen might be a better fit, but hmm, that isn't very
   well supported in GDB/glibc.

 - A lower level mechanism has much better chances of working on
   more targets and runtimes-of-languages-other-than-C with minimal
   changes.

> ISTM that using the Windows equivalent of dlopen is exactly the right thing here.

That may well be, though I think that it's better if "info shared"
doesn't show the modules injected into the inferior, which using LoadLibrary
(Windows's dlopen) would give you.


Note that set_gdbarch_infcall_mmap doesn't need to implement the
whole feature set of mmap.  We only need to be able to carve out a
piece of memory.  Should map trivially to VirtualAlloc.


I expect that we'll end up using this same gdbarch hook to allocate
memory in the inferior when we need to coerce variables to the inferior.
We currently call "malloc" for that, which isn't ideal in the sense
that the inferior can well be already inside malloc when we do that,
leading do deadlock or corruption.

Thanks,
Pedro Alves
Eli Zaretskii Dec. 18, 2014, 5:22 p.m. UTC | #20
> Date: Wed, 17 Dec 2014 20:34:12 +0000
> From: Pedro Alves <palves@redhat.com>
> CC: brobecker@adacore.com, yao@codesourcery.com, gdb-patches@sourceware.org,
>         ktietz@redhat.com
> 
> Note that set_gdbarch_infcall_mmap doesn't need to implement the
> whole feature set of mmap.  We only need to be able to carve out a
> piece of memory.  Should map trivially to VirtualAlloc.

That's true, but if this feature will work by doing the equivalent of
dlopen, then it will most probably only ever work on GNU/Linux.  Even
there, we cannot really hope to do all this stuff by hand, do it
safely, and then track all the changes in the OS to keep this working
for the years to come.  And good luck being able to do it at all on
proprietary systems such as Windows, where you cannot simply examine
the sources of the DLL loader.

I conclude that sadly this feature is extremely limited, and will
probably die very soon, unless we change its implementation to
something more portable and safe.
Pedro Alves Dec. 18, 2014, 5:59 p.m. UTC | #21
On 12/18/2014 05:22 PM, Eli Zaretskii wrote:
>> Date: Wed, 17 Dec 2014 20:34:12 +0000
>> From: Pedro Alves <palves@redhat.com>
>> CC: brobecker@adacore.com, yao@codesourcery.com, gdb-patches@sourceware.org,
>>         ktietz@redhat.com
>>
>> Note that set_gdbarch_infcall_mmap doesn't need to implement the
>> whole feature set of mmap.  We only need to be able to carve out a
>> piece of memory.  Should map trivially to VirtualAlloc.
> 
> That's true, but if this feature will work by doing the equivalent of
> dlopen, then it will most probably only ever work on GNU/Linux.  Even
> there, we cannot really hope to do all this stuff by hand, do it
> safely, and then track all the changes in the OS to keep this working
> for the years to come.  And good luck being able to do it at all on
> proprietary systems such as Windows, where you cannot simply examine
> the sources of the DLL loader.

I don't think we'll need to.  In fact, I have a feeling that on Windows
things will tend to be simpler, given PE is simpler/more contained
than ELF.  But if it turns out that whoever tries to make this work on
Windows finds out that the LoadLibrary route is better for Windows, then
I don't see that as a problem.  I'm sure we can fit that in by overriding
some hooks.

> I conclude that sadly this feature is extremely limited, and will
> probably die very soon, unless we change its implementation to
> something more portable and safe.

We shall see.  :-)

Thanks,
Pedro Alves
diff mbox

Patch

--- ./gdb/compile/compile.c	2014-12-14 02:48:38.000000000 +0100
+++ ./gdb/compile/compile.c	2014-12-15 23:21:28.788716340 +0100
@@ -191,7 +191,11 @@  get_compile_file_tempdir (void)
 
   strcpy (tname, TEMPLATE);
 #undef TEMPLATE
+#ifdef _WIN32
+  error (_("mkdtemp needs to be implemented for MS-Windows hosts"));
+#else
   tempdir_name = mkdtemp (tname);
+#endif
   if (tempdir_name == NULL)
     perror_with_name (_("Could not make temporary directory"));