code review 4133041: runtime: fix mmap breakage on Windows

runtime: fix mmap breakage on Windows

[hector, 2011-02-04 23:19:03]

Hello rsc (cc: golang-dev@googlegroups.com),

I'd like you to review this change to
https://go.googlecode.com/hg/

[rsc, 2011-02-05 05:55:40]

Thanks for fixing this.

I thought that someone said it failed because the
runtime debugging information was mapped where
SysReserve wanted to put some memory.
If that is the problem, the fix should be to stop
special mapping of the runtime debugging information.
It moved into the read-only data segment a while back,
and I removed the special load from the ELF and
Mach-O formats, but it is easy to believe I forgot
to change the PE code.

Can you say a bit about what's going on here,
why the original code doesn't work?

Thanks.
Russ

[hector, 2011-02-05 11:59:00]

In the .exe file and in memory the .data section is not the last section before
free memory space.  There is an .idata section containing the import table, and
then DWARF sections like .debug_abbrev and .debug_line.  Finally a .symtab
section is tacked onto the end, which contains the names for the DWARF sections
and the symbol table.  When the offset to the end of the .data section is
rounded up it hits right into the middle of the DWARF block.  VirtualAlloc will
fail if it cannot allocate at the address supplied to it.  If we can't allocate
at the location we want we should let Windows find a memory block for us.

I'm not sure where the mapping of the runtime debugging information is in all of
this but as far as I can see the DWARF sections and symbol table are required so
that debugging tools can find it easily.

[hector, 2011-02-05 12:09:39]

I forgot to add, PTAL as I have changed the code to avoid a potential infinite
loop.

[brainman, 2011-02-05 23:14:42]

On 2011/02/05 11:59:00, hector wrote:
> In the .exe file and in memory the .data section is not the last section
before
> free memory space.  There is an .idata section containing the import table,
and
> then DWARF sections like .debug_abbrev and .debug_line.  Finally a .symtab
> section is tacked onto the end, which contains the names for the DWARF
sections
> and the symbol table.  When the offset to the end of the .data section is
> rounded up it hits right into the middle of the DWARF block.  VirtualAlloc
will
> fail if it cannot allocate at the address supplied to it.

Correct.

> ...  If we can't allocate
> at the location we want we should let Windows find a memory block for us.

That we be good, if possible - Russ might have different idea of what he wants.

> I'm not sure where the mapping of the runtime debugging information is in all
of
> this but as far as I can see the DWARF sections and symbol table are required
so
> that debugging tools can find it easily.

From what I could see looking at other build tools (I couldn't find any docos on
dwarf and pe), we could put contents of .symtab pe section into .data section,
but all dwarf sections must be present for debugger to find them.

To add my voice here, we shouldn't restrict ourselves with how many pe section
we have. I could see some more in the future. I doubt it is possible to put them
all in one big .data section.

Also, I'm not sure if it is a problem, but when windows loads dlls we use, it
could put them anywhere, we can't control where they will go. If we decide to
use the same memory block, we will fail again. Most dlls are loaded at start up,
but others could be loaded at any stage of execution.

Who knows what else ...

Alex

[brainman, 2011-02-07 01:41:40]

http://codereview.appspot.com/4133041/diff/12001/src/pkg/runtime/windows/mem.c
File src/pkg/runtime/windows/mem.c (left):

http://codereview.appspot.com/4133041/diff/12001/src/pkg/runtime/windows/mem....
src/pkg/runtime/windows/mem.c:51: return runtime·stdcall(runtime·VirtualAlloc,
4, v, n, MEM_RESERVE, PAGE_EXECUTE_READWRITE);
It works for me even if I remove all your changes in this file, but change this
line only to:

return runtime·stdcall(runtime·VirtualAlloc, 4, nil, n, MEM_RESERVE,
PAGE_EXECUTE_READWRITE);

The question here is, can we change malloc.goc the way you did.

[hector, 2011-02-07 08:46:45]

http://codereview.appspot.com/4133041/diff/12001/src/pkg/runtime/windows/mem.c
File src/pkg/runtime/windows/mem.c (left):

http://codereview.appspot.com/4133041/diff/12001/src/pkg/runtime/windows/mem....
src/pkg/runtime/windows/mem.c:51: return runtime·stdcall(runtime·VirtualAlloc,
4, v, n, MEM_RESERVE, PAGE_EXECUTE_READWRITE);
The aim of the changes was to attempt to grow memory downwards, as per the
comment in malloc.goc.  We record in the hint where we would like to allocate
from next, which is right after the end of the previous allocation.

[rsc, 2011-02-08 20:41:52]

Can we change the definition of "end" in 8l to be
the actual end of the initially mapped address space?

Russ

[brainman, 2011-02-08 22:14:39]

On 2011/02/08 20:41:52, rsc wrote:
> Can we change the definition of "end" in 8l to be
> the actual end of the initially mapped address space?

The value we're after would be in nextsectoff at

set(SizeOfImage, nextsectoff);

line of ld/pe.c/^asmbpe. Is it possible to update "end" then?

Alex

[rsc, 2011-02-08 23:48:40]

> The value we're after would be in nextsectoff at
>
> set(SizeOfImage, nextsectoff);
>
> line of ld/pe.c/^asmbpe. Is it possible to update "end" then?

No, you'd need to update the value of end before the call
to reloc() in main.

Russ

[brainman, 2011-02-09 01:10:33]

On 2011/02/08 23:48:40, rsc wrote:
> > The value we're after would be in nextsectoff at
> >
> > set(SizeOfImage, nextsectoff);
> >
> > line of ld/pe.c/^asmbpe. Is it possible to update "end" then?
> 
> No, you'd need to update the value of end before the call
> to reloc() in main.
> 

We don't have that value then.

Perhaps we could make an educated guess - here are the sections we're writing
after .data:

- .isect: contains info about dll functions runtime imports on startup - doesn't
change very often and small, couple of KB;

- .edata: contains info about functions we export, as far as I know is used in
dlls only, I don't know how Wei uses that;

- "all dwarf sections": not sure how big they can get;

- .symtab: contains "full/real" names of "all dwarf sections" - quite small,
couple of KB.

This is now, we might decide to add more sections. If we could value size of
dwarf data, other sections shouldn't be a problem.

What do you think?

Alex

[rsc, 2011-02-09 03:23:07]

I think we should not be mapping .symtab or dwarf into memory.
They're in the file but shouldn't be loaded into memory.

Do .isect and .edata need to be writable?
The equivalent data in ELF is marked as SELFDATA
and ends up in the read-only (early) half of the binary.

If .symtab and dwarf were not mapped and .isect and .edata
moved to the read-only part of the binary, then .data would be
last, as is typical of the other executables.

If that's possible it's the nicest solution, because it makes
all the systems that much more alike.  Is it possible?

Russ

[brainman, 2011-02-09 03:49:14]

On 2011/02/09 03:23:07, rsc wrote:
> I think we should not be mapping .symtab or dwarf into memory.
> They're in the file but shouldn't be loaded into memory.

I couldn't find any documentation on dwarf and pe, so I just go by what gcc
does. They don't have .symtab pe section, it is just a blob at end of their .exe
file pointed by a field in pe header. .symtab therefore doesn't need to be
loaded into memory. On the other hand, all dwarf sections are different, their
each have their own pe section, so, I guess, they can be found be gdb and
others. The fact their are pe sections makes them go into memory whether we want
it or not.

> Do .isect and .edata need to be writable?

Probably not.

> The equivalent data in ELF is marked as SELFDATA
> and ends up in the read-only (early) half of the binary.

We could, probably, put .isect and .edata inside .data or .text pe section. I
need to see, if all symbols are ready if we start writing early.

> If .symtab and dwarf were not mapped and .isect and .edata
> moved to the read-only part of the binary, then .data would be
> last, as is typical of the other executables.

I don't now what to do about dwarf.

Also typical windows .exe has many different pe sections after .data. In fact,
you can split .exe into its pe sections and put it together adding, removing or
changing its sections as you wish, and it will still run. It's what some viruses
do <g>.

Alex

[rsc, 2011-02-09 04:40:12]

I'll try to take a closer look tomorrow when I am working
on the Linux and FreeBSD ones.

On Tue, Feb 8, 2011 at 22:49,  <alex.brainman@gmail.com> wrote:
> On 2011/02/09 03:23:07, rsc wrote:
>>
>> I think we should not be mapping .symtab or dwarf into memory.
>> They're in the file but shouldn't be loaded into memory.
>
> I couldn't find any documentation on dwarf and pe, so I just go by what
> gcc does. They don't have .symtab pe section, it is just a blob at end
> of their .exe file pointed by a field in pe header. .symtab therefore
> doesn't need to be loaded into memory. On the other hand, all dwarf
> sections are different, their each have their own pe section, so, I
> guess, they can be found be gdb and others. The fact their are pe
> sections makes them go into memory whether we want it or not.
>
>> Do .isect and .edata need to be writable?
>
> Probably not.
>
>> The equivalent data in ELF is marked as SELFDATA
>> and ends up in the read-only (early) half of the binary.
>
> We could, probably, put .isect and .edata inside .data or .text pe
> section. I need to see, if all symbols are ready if we start writing
> early.
>
>> If .symtab and dwarf were not mapped and .isect and .edata
>> moved to the read-only part of the binary, then .data would be
>> last, as is typical of the other executables.
>
> I don't now what to do about dwarf.
>
> Also typical windows .exe has many different pe sections after .data. In
> fact, you can split .exe into its pe sections and put it together
> adding, removing or changing its sections as you wish, and it will still
> run. It's what some viruses do <g>.
>
> Alex
>
> http://codereview.appspot.com/4133041/
>

[hector, 2011-02-09 08:00:14]

From a cursory examination of pe.c it might just work to add the .data
section last in the executable.  Not sure whether it's legal but it's worth
a try.
On 9 Feb 2011 04:40, "Russ Cox" <rsc@golang.org> wrote:
> I'll try to take a closer look tomorrow when I am working
> on the Linux and FreeBSD ones.
>
> On Tue, Feb 8, 2011 at 22:49, <alex.brainman@gmail.com> wrote:
>> On 2011/02/09 03:23:07, rsc wrote:
>>>
>>> I think we should not be mapping .symtab or dwarf into memory.
>>> They're in the file but shouldn't be loaded into memory.
>>
>> I couldn't find any documentation on dwarf and pe, so I just go by what
>> gcc does. They don't have .symtab pe section, it is just a blob at end
>> of their .exe file pointed by a field in pe header. .symtab therefore
>> doesn't need to be loaded into memory. On the other hand, all dwarf
>> sections are different, their each have their own pe section, so, I
>> guess, they can be found be gdb and others. The fact their are pe
>> sections makes them go into memory whether we want it or not.
>>
>>> Do .isect and .edata need to be writable?
>>
>> Probably not.
>>
>>> The equivalent data in ELF is marked as SELFDATA
>>> and ends up in the read-only (early) half of the binary.
>>
>> We could, probably, put .isect and .edata inside .data or .text pe
>> section. I need to see, if all symbols are ready if we start writing
>> early.
>>
>>> If .symtab and dwarf were not mapped and .isect and .edata
>>> moved to the read-only part of the binary, then .data would be
>>> last, as is typical of the other executables.
>>
>> I don't now what to do about dwarf.
>>
>> Also typical windows .exe has many different pe sections after .data. In
>> fact, you can split .exe into its pe sections and put it together
>> adding, removing or changing its sections as you wish, and it will still
>> run. It's what some viruses do <g>.
>>
>> Alex
>>
>> http://codereview.appspot.com/4133041/
>>

[brainman, 2011-02-09 08:26:09]

On 2011/02/09 08:00:14, hector wrote:
> From a cursory examination of pe.c it might just work to add the .data
> section last in the executable.  ...

I don't think this will work, because by the time we get to write pe sections
.data (and all addresses) is already been layed out in address space and its
image is written to disk.

On the other hand, we could change size of .data pe section to accommodate as
much extra memory space as we like, or, alternatively, create s special pe
section right after .data section. I'm not sure how would that affect .exe image
in memory, but, I suppose, normal paging rules should apply, so I don't think it
will be slow or anything like that.

Alex

[vcc, 2011-02-09 09:00:00]

2011/2/9 Russ Cox <rsc@golang.org>

> > The value we're after would be in nextsectoff at
> >
> > set(SizeOfImage, nextsectoff);
> >
> > line of ld/pe.c/^asmbpe. Is it possible to update "end" then?
>
> No, you'd need to update the value of end before the call
> to reloc() in main.
>

I think we can update "end" value in asmbpe, PEBASE+nextsectoff would be the
right value.

Wei guangjing

[hector, 2011-02-09 09:28:30]

Not sure if this hurts any, but I've noticed that a few pages are mapped not
that far past the end of the PE sections before the program starts executing.

[rsc, 2011-02-09 12:39:39]

> I think we can update "end" value in asmbpe, PEBASE+nextsectoff would be the
> right value.

This won't work.  All references to end by the program itself
(which is what we're really trying to update) record the value
as of the call to reloc().

Russ

[vcc, 2011-02-09 13:50:21]

2011/2/9 Russ Cox <rsc@golang.org>

> > I think we can update "end" value in asmbpe, PEBASE+nextsectoff would be
> the
> > right value.
>
> This won't work.  All references to end by the program itself
> (which is what we're really trying to update) record the value
> as of the call to reloc().
>

Could overwrite the "end" value, like dynimport write FirstThunks (allocated
in .data section):
s = lookup("end", 0);
base = s->value - datsect->VirtualAddress - PEBASE;
seek(cout, datsect->PointerToRawData + base, 0);
put(PEBASE+nextsectoff);

Wei guangjing

[rsc, 2011-02-09 14:15:02]

> Could overwrite the "end" value, like dynimport write FirstThunks (allocated
> in .data section):
> s = lookup("end", 0);
> base = s->value - datsect->VirtualAddress - PEBASE;
> seek(cout, datsect->PointerToRawData + base, 0);
> put(PEBASE+nextsectoff);

That overwrites the value that end points at,
but end doesn't point at anything.  It's declared
something like

    extern char end[0];

and is only used for its address, not for its value.

It's okay - I think we can do it without changing end.

Russ

[rsc, 2011-02-11 17:40:42]

I think this CL is not necessary anymore.

[hector, 2011-02-12 13:58:08]

*** Abandoned ***