On 22.12.2015 16:31, Paul Gofman wrote:
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 whichis 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.
Trying different optimization flags is one possibility. Alternatively, you could also try to remove static/inline or adding __volatile__ to the assembler code.
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?
There are probably more ways to workaround the issue besides if-checks. If possible, something else (which keeps the return value) would definitely be preferred. A couple of ideas which might help (not sure if they work):
- Replace SYSCALL_RET() macro with an (inline?) function. - Replace inline assembly with a global ASM function (without C wrapper around). See for example how its done for _start, or for the x86_64 functions. If you need help with the assembler code, feel free to ask. - Pass address of ret to the assembler code, and let assembler code write the result. The compiler is probably no longer able to optimize away the function content then.