[PATCH v2] loader: check return values of wld_mprotect and wld_mmap calls

Paul Gofman gofmanp at gmail.com
Tue Dec 22 15:12:59 CST 2015


I figured out what is really happening there after reviewing assembly
listings. The problem in compilation is triggered by the following
(correct) logic in preloader.c/map_so_lib function:
...
if( header->e_type == ET_DYN ) //line 806
{
...
    goto postmap;
}
...
while (c < &loadcmds[nloadcmds])
{
...
      postmap:
...
        ++c;
}
...
The problem seen in assembly listing is that ICC stores
&loadcmds[nloadcmds] in the stack variable in the beginning of the loop,
and this initialization is skipped along with the beginning of the loop
on 'goto postmap'. My initial attempt to fix it looks like random tweak
(the overall function register & vars allocation structure changed and
the problem did not fire). Now I can workaround the problem in a
definite way by using some pre-initialized variable instead of
&loadcmds[nloadcmds] in loop comparison (this fixes the issue).
    So it is no incorrect code in Wine here (nor any optimization hints
for this I could imagine). goto jumping inside the loop body is correct
(though unusual), but it triggers the problem with ICC. I could think of
changing this code one of two ways:
1. The simplest one (add a variable instead of &loadcmds[nloadcmds]),
which is actually a tweak (while a better defined one than my initial
attempt);
2. Redesign this logic and avoid jumping inside loop body (this is
probably a better way)

Would a patch like this (presumably way 2) be relevant?


On 12/22/2015 07:22 PM, Alexandre Julliard wrote:
> If there's really something missing in the Wine code, sure, it should be
> fixed. But if the code is correct, tweaking it to work around optimizer
> bugs is a waste of time.
>





More information about the wine-devel mailing list