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

[Paul Gofman]

On 12/22/2015 05:33 PM, Sebastian Lackner wrote:
> Its not my decision, but I don't think its a good idea to add all the (unnecessary) ifs
> just to make this code compatible with ICC. I assume the compiler doesn't handle inline
> assembly correctly, but this would mean that there are several more affected places.
> It would be nice to have a more general solution to fix/workaround this issue.
> If this is not possible, I would at least suggest to add a workaround to the functions
> directly, instead of adding checks to all places where they are called.
>
>
    I think the cause of problem is is ICC aggressive optimization which
is sometimes buggy under specific conditions. In this case ICC seems to
ignore the side effect of asm syscall. I think it does not mean that it
handles all of inline assembly incorrectly. This problem is hard to
reproduce: I can't get the same problem using the same wld_mmap and
wld_mprotect functions in a small separate test case, so this likely
happens under some specific conditions only.
    Yes, surely there might be other places like (or not quite like)
that which will misbehave the same way which I did not come across yet.
But I do not know any general way to track and fix them all at once.
Maybe except for disabling (some) optimization which actually makes use
of ICC non sensible.
    I was hoping that adding a few checks like that won't hurt the code
(I understand though that the checks may be paranoid or redundant apart
from ICC issue). If this is not the case then maybe some other
solution/workaround for this ICC issue can be found.
    Do you think it is better to move return value check directly into
wld_mmap and wld_mprotect instead of checking their status on call? I
thought that checking return status rather than building in such an
effect into system function wrapper is a common approach. Or maybe I
could add wrapper inline functions with error checking?