Re: [user32] Add DECLSPEC_NOINLINE, use it on call_hook_proc()
On Sat, Apr 20, 2013 at 1:38 AM, Dan Kegel wrote: >> I tried explicitly add 'inline' to every static functions in hook.c >> but complie with -O0, to see if the bug can be reproduced in this way, >> but nothing happen, this make me doubt being inline is not the >> culprit. > > call_hook_proc is probably the only function whose inlining matters > for this problem (since my patch solved your problem at -O2). > > (inline is only a hint. FORCEINLINE is stronger.) Thanks Dan and André, I tried FORCEINLINE and tried bisect on gcc flags to reproduce the bug but failed with strange gcc behaviors, then Dan had worked out an actual fix for this bug, what I learned is, understanding the real bug seems faster than playing with gcc flags... Anyway, thanks Dan and André as always! -- Regards, Qian Hong - http://www.winehq.org
Re: [user32] Add DECLSPEC_NOINLINE, use it on call_hook_proc()
Dan Kegel writes: > On Fri, Apr 19, 2013 at 10:38 AM, Dan Kegel wrote: >> Hooking is a fragile business. Somebody somewhere is probably >> making assumptions about how hooking works (like, how many stack >> frames are pushed), and inlining call_hook_proc probably violates one >> of those assumptions. > > That said, this is all a big guess. It would be nice to have a > test that demonstrated the problem. It's up to Alexandre to > decide whether making a couple apps happier is evidence > enough that disabling inlining of this one function is a good idea. It's not. Please take the time to understand the problem and fix it properly. -- Alexandre Julliard julli...@winehq.org
Re: [user32] Add DECLSPEC_NOINLINE, use it on call_hook_proc()
On Fri, Apr 19, 2013 at 10:38 AM, Dan Kegel wrote: > Hooking is a fragile business. Somebody somewhere is probably > making assumptions about how hooking works (like, how many stack > frames are pushed), and inlining call_hook_proc probably violates one > of those assumptions. That said, this is all a big guess. It would be nice to have a test that demonstrated the problem. It's up to Alexandre to decide whether making a couple apps happier is evidence enough that disabling inlining of this one function is a good idea. - Dan
Re: [user32] Add DECLSPEC_NOINLINE, use it on call_hook_proc()
On Fri, Apr 19, 2013 at 10:13 AM, Qian Hong wrote: > Curiosity killed the cat, what is the theory behind this patch? Hooking is a fragile business. Somebody somewhere is probably making assumptions about how hooking works (like, how many stack frames are pushed), and inlining call_hook_proc probably violates one of those assumptions. > I tried explicitly add 'inline' to every static functions in hook.c > but complie with -O0, to see if the bug can be reproduced in this way, > but nothing happen, this make me doubt being inline is not the > culprit. call_hook_proc is probably the only function whose inlining matters for this problem (since my patch solved your problem at -O2). (inline is only a hint. FORCEINLINE is stronger.) - Dan
Re: [user32] Add DECLSPEC_NOINLINE, use it on call_hook_proc()
Am 19.04.2013 19:13, schrieb Qian Hong: > On Sat, Apr 20, 2013 at 12:52 AM, Dan Kegel wrote: >> I suspect this is a real fix, and there is no gcc bug. > > Thanks for clarify. > > Curiosity killed the cat, what is the theory behind this patch? > I tried explicitly add 'inline' to every static functions in hook.c > but complie with -O0, to see if the bug can be reproduced in this way, > but nothing happen, this make me doubt being inline is not the > culprit. > > Any inspire is great appreciated! > > Best wishes from a curiosity cat :) the inline keyword is meanwhile just a hint for the compiler, it doesn't need to inline it
Re: [user32] Add DECLSPEC_NOINLINE, use it on call_hook_proc()
On Sat, Apr 20, 2013 at 12:52 AM, Dan Kegel wrote: > I suspect this is a real fix, and there is no gcc bug. Thanks for clarify. Curiosity killed the cat, what is the theory behind this patch? I tried explicitly add 'inline' to every static functions in hook.c but complie with -O0, to see if the bug can be reproduced in this way, but nothing happen, this make me doubt being inline is not the culprit. Any inspire is great appreciated! Best wishes from a curiosity cat :) -- Regards, Qian Hong - http://www.winehq.org
Re: [user32] Add DECLSPEC_NOINLINE, use it on call_hook_proc()
I suspect this is a real fix, and there is no gcc bug. On Fri, Apr 19, 2013 at 9:47 AM, Qian Hong wrote: > Hi Dan, > > Thanks for working on it! It is really an annoying bug, however, is > adding DECLSPEC_NOINLINE a workaround or a real fix? If it is only a > workaround, would that hide some real bug behind it? Or is that > possible that there is a gcc bug here that we want to report to gcc? > > Thanks for your time!
Re: [user32] Add DECLSPEC_NOINLINE, use it on call_hook_proc()
Hi Dan, Thanks for working on it! It is really an annoying bug, however, is adding DECLSPEC_NOINLINE a workaround or a real fix? If it is only a workaround, would that hide some real bug behind it? Or is that possible that there is a gcc bug here that we want to report to gcc? Thanks for your time!
