GCC4.9+ de-virtualization issues.

[KyleSanderson]

https://travis-ci.org/alliedmodders/metamod-source/jobs/405106127

 ./core/sourcehook/test/test_sourcehook/test_sourcehook -v
TestList passed
No error: ModuleInMemory
No error: Part 1
No error: Part 2
FAIL: Should be:
  State_F1_HookAdded; Param1=1
  State_F1_Called
  State_F1_PreHandler_Called; Param1=0xffdf5072
FAIL: Is:
  State_F1_HookAdded; Param1=1
  State_F1_Called
  State_F1_Called
TestBasic FAILED: Part 3
FAIL: Should be:
  State_EatYams_Called; Param1=0
  State_EatYams_Called; Param1=1
  State_EatYams_Handler_Called; Param1=0
  State_EatYams_Called; Param1=0
  State_EatYams_Handler_Called; Param1=1
  State_EatYams_Called; Param1=1
FAIL: Is:
  State_EatYams_Called; Param1=0
  State_EatYams_Called; Param1=1
  State_EatYams_Called; Param1=0
  State_EatYams_Called; Param1=1
TestVafmtAndOverload FAILED: Part 1
No error: Part 1
No error: Part 1.1
No error: Part 2
FAIL: Should be:
  State_Func1H_Called; Param1=0x9edd090
  State_Func1_Called; Param1=0x9edd090
  State_Func2H_Called; Param1=0x9edd090
  State_Func2_Called; Param1=0x9edd098
  State_Func3H_Called; Param1=0x9edd090
  State_Func3_Called; Param1=0x9edd090
  State_Func1H_Called; Param1=0x9edd090
  State_Func1_Called; Param1=0x9edd090
  State_Func2H_Called; Param1=0x9edd090
  State_Func2_Called; Param1=0x9edd098
FAIL: Is:
  State_Func1_Called; Param1=0x9edd090
  State_Func2_Called; Param1=0x9edd098
  State_Func3_Called; Param1=0x9edd090
  State_Func1_Called; Param1=0x9edd090
  State_Func2_Called; Param1=0x9edd098
TestThisPtrOffs FAILED: Part 3
FAIL: Should be:
  State_Func1H_Called
  State_Func1_Called
  State_Func2_Called
  State_Func2H_Called
  State_Func3H_Called
  State_Func3_Called
FAIL: Is:
  State_Func1_Called
  State_Func2_Called
  State_Func3_Called
TestPlugSys FAILED: Part 1.1
No error: Part 1
No error: Part 2.1
No error: Part 3
No error: Part 4
No error: Part 5
No error: Part 6
TestBail passed
FAIL: Should be:
  State_H_C1_F; Param1=0x811391c
  State_H_C1_G; Param1=0x811391c
  State_H_C2_F; Param1=0x8113918
  State_H_C2_G; Param1=0x8113918
  State_H_C3_F; Param1=0x8113914
  State_H_C3_G; Param1=0x8113914
  State_H_C4_F; Param1=0x8113910
  State_H_C4_G; Param1=0x8113910
  State_H_C5_F; Param1=0x811390c
  State_H_C5_G; Param1=0x811390c
  State_H_C6_F; Param1=0x8113908
  State_H_C6_G; Param1=0x8113908
  State_H_C7_F; Param1=0x8113904
  State_H_C7_G; Param1=0x8113904
  State_H_C8_F; Param1=0x8113900
  State_H_C8_G; Param1=0x8113900
  State_C8_G; Param1=0x8113900
  State_C8_F; Param1=0x8113900
  State_C7_G; Param1=0x8113904
  State_C7_F; Param1=0x8113904
  State_C6_G; Param1=0x8113908
  State_C6_F; Param1=0x8113908
  State_C5_G; Param1=0x811390c
  State_C5_F; Param1=0x811390c
  State_C4_G; Param1=0x8113910
  State_C4_F; Param1=0x8113910
  State_C3_G; Param1=0x8113914
  State_C3_F; Param1=0x8113914
  State_C2_G; Param1=0x8113918
  State_C2_F; Param1=0x8113918
  State_C1_G; Param1=0x811391c
  State_C1_F; Param1=0x811391c
FAIL: Is:
  State_C1_F; Param1=0x811391c
TestReentr FAILED: 1
No error: Part 1
No error: Part 1.1
FAIL: Should be:
  State_Func1H_Called; Param1=0x9eded50
  State_Func1_Called; Param1=0x9eded50
  State_Func2H_Called; Param1=0x9eded50; Param2=200
  State_Func2_Called; Param1=0x9eded50; Param2=-2023406815
  State_Func3H_Called; Param1=0x9eded50
  State_Func3_Called; Param1=0x9eded50
  State_Return; Param1=3
  State_Func4H_Called; Param1=0x9eded50; Param2=400
  State_Func4_Called; Param1=0x9eded50; Param2=305419896
  State_Return; Param1=4
FAIL: Is:
  State_Func1_Called; Param1=0x9eded50
  State_Func2_Called; Param1=0x9eded50; Param2=200
  State_Func3_Called; Param1=0x9eded50
  State_Return; Param1=3
  State_Func4_Called; Param1=0x9eded50; Param2=400
  State_Return; Param1=4
TestManual FAILED: Part 2
FAIL: Should be:
  State_H1_Func1; Param1=77
  State_H2_Func1; Param1=5
  State_Func1; Param1=0
  State_HP_Func1; Param1=0; Param2=0x9eddae0
FAIL: Is:
  State_Func1; Param1=77
TestRecall FAILED: Part 1
TestMulti FAILED: g_callcount[0] != 0
No error: Part 1
No error: Part 2
FAIL: Should be:
  State_Result_InHook; Param1=0
  State_Result; Param1=20
  State_Result_InHook; Param1=10
  State_Result; Param1=20
  State_Result_InHook; Param1=11
  State_Result; Param1=20
  State_Result_InHook; Param1=12
  State_Result; Param1=20
FAIL: Is:
  State_Result; Param1=0
  State_Result; Param1=10
  State_Result; Param1=11
  State_Result; Param1=12
TestRef FAILED: Part 3
No error: Part 1
FAIL: Should be:
  State_Func1_Pre1; Param1=0x9eddb5c
  State_Func1; Param1=0x9edd4dc
  State_Func1_Ret; Param1=0x9eddb5c
FAIL: Is:
  State_Func1; Param1=0x9edd4dc
  State_Func1_Ret; Param1=0x9edd4dc
TestRefRet FAILED: Part 2
FAIL: Should be:
  State_Func1_Pre; Param1=0xffdf5088
  State_D1_Func1; Param1=0xffdf5088
  State_Func1_Pre; Param1=0xffdf508c
  State_D1_Func1; Param1=0xffdf508c
  State_D2_Func1; Param1=0xffdf5090
FAIL: Is:
  State_D1_Func1; Param1=0xffdf5088
  State_D1_Func1; Param1=0xffdf508c
  State_D2_Func1; Param1=0xffdf5090
TestVPHooks FAILED: Part 1
TestCPageAlloc passed
No error: GlobCtors
No error: Test0 Part1
FAIL: Should be:
  State_Deleg_0; Param1=1; Param2=0x9edd560; Param3=0; Param4=
  State_Func0; Param1=0x9edd560; Param2=
  State_Deleg_0; Param1=3; Param2=0x9edd560; Param3=1; Param4=
  State_Func0; Param1=0x9edd560; Param2=
FAIL: Is:
  State_Func0; Param1=0x9edd560; Param2=
  State_Func0; Param1=0x9edd560; Param2=
/home/travis/.travis/job_stages: line 78:  6108 Segmentation fault      (core dumped) ./core/sourcehook/test/test_sourcehook/test_sourcehook -v
The command "./core/sourcehook/test/test_sourcehook/test_sourcehook -v" exited with 139.```

[KyleSanderson]

GCC-4.9 also shows this. 4.8 completes no problem.

$ ./core/sourcehook/test/test_sourcehook/test_sourcehook -v
TestList passed
No error: ModuleInMemory
No error: Part 1
No error: Part 2
FAIL: Should be:
  State_F1_HookAdded; Param1=1
  State_F1_Called
  State_F1_PreHandler_Called; Param1=0xfff98472
FAIL: Is:
  State_F1_HookAdded; Param1=1
  State_F1_Called
  State_F1_Called
TestBasic FAILED: Part 3
FAIL: Should be:
  State_EatYams_Called; Param1=0
  State_EatYams_Called; Param1=1
  State_EatYams_Handler_Called; Param1=0
  State_EatYams_Called; Param1=0
  State_EatYams_Handler_Called; Param1=1
  State_EatYams_Called; Param1=1
FAIL: Is:
  State_EatYams_Called; Param1=0
  State_EatYams_Called; Param1=1
  State_EatYams_Called; Param1=0
  State_EatYams_Called; Param1=1
TestVafmtAndOverload FAILED: Part 1
No error: Part 1
No error: Part 1.1
No error: Part 2
FAIL: Should be:
  State_Func1H_Called; Param1=0x8b30090
  State_Func1_Called; Param1=0x8b30090
  State_Func2H_Called; Param1=0x8b30090
  State_Func2_Called; Param1=0x8b30098
  State_Func3H_Called; Param1=0x8b30090
  State_Func3_Called; Param1=0x8b30090
  State_Func1H_Called; Param1=0x8b30090
  State_Func1_Called; Param1=0x8b30090
  State_Func2H_Called; Param1=0x8b30090
  State_Func2_Called; Param1=0x8b30098
FAIL: Is:
  State_Func1_Called; Param1=0x8b30090
  State_Func2_Called; Param1=0x8b30098
  State_Func3_Called; Param1=0x8b30090
  State_Func1_Called; Param1=0x8b30090
  State_Func2_Called; Param1=0x8b30098
TestThisPtrOffs FAILED: Part 3
FAIL: Should be:
  State_Func1H_Called
  State_Func1_Called
  State_Func2_Called
  State_Func2H_Called
  State_Func3H_Called
  State_Func3_Called
FAIL: Is:
  State_Func1_Called
  State_Func2_Called
  State_Func3_Called
TestPlugSys FAILED: Part 1.1
No error: Part 1
No error: Part 2.1
No error: Part 3
No error: Part 4
No error: Part 5
No error: Part 6
TestBail passed
FAIL: Should be:
  State_H_C1_F; Param1=0x810f93c
  State_H_C1_G; Param1=0x810f93c
  State_H_C2_F; Param1=0x810f938
  State_H_C2_G; Param1=0x810f938
  State_H_C3_F; Param1=0x810f934
  State_H_C3_G; Param1=0x810f934
  State_H_C4_F; Param1=0x810f930
  State_H_C4_G; Param1=0x810f930
  State_H_C5_F; Param1=0x810f92c
  State_H_C5_G; Param1=0x810f92c
  State_H_C6_F; Param1=0x810f928
  State_H_C6_G; Param1=0x810f928
  State_H_C7_F; Param1=0x810f924
  State_H_C7_G; Param1=0x810f924
  State_H_C8_F; Param1=0x810f920
  State_H_C8_G; Param1=0x810f920
  State_C8_G; Param1=0x810f920
  State_C8_F; Param1=0x810f920
  State_C7_G; Param1=0x810f924
  State_C7_F; Param1=0x810f924
  State_C6_G; Param1=0x810f928
  State_C6_F; Param1=0x810f928
  State_C5_G; Param1=0x810f92c
  State_C5_F; Param1=0x810f92c
  State_C4_G; Param1=0x810f930
  State_C4_F; Param1=0x810f930
  State_C3_G; Param1=0x810f934
  State_C3_F; Param1=0x810f934
  State_C2_G; Param1=0x810f938
  State_C2_F; Param1=0x810f938
  State_C1_G; Param1=0x810f93c
  State_C1_F; Param1=0x810f93c
FAIL: Is:
  State_C1_F; Param1=0x810f93c
TestReentr FAILED: 1
No error: Part 1
No error: Part 1.1
FAIL: Should be:
  State_Func1H_Called; Param1=0x8b31d50
  State_Func1_Called; Param1=0x8b31d50
  State_Func2H_Called; Param1=0x8b31d50; Param2=200
  State_Func2_Called; Param1=0x8b31d50; Param2=-2023406815
  State_Func3H_Called; Param1=0x8b31d50
  State_Func3_Called; Param1=0x8b31d50
  State_Return; Param1=3
  State_Func4H_Called; Param1=0x8b31d50; Param2=400
  State_Func4_Called; Param1=0x8b31d50; Param2=305419896
  State_Return; Param1=4
FAIL: Is:
  State_Func1_Called; Param1=0x8b31d50
  State_Func2_Called; Param1=0x8b31d50; Param2=200
  State_Func3_Called; Param1=0x8b31d50
  State_Return; Param1=3
  State_Func4_Called; Param1=0x8b31d50; Param2=400
  State_Return; Param1=4
TestManual FAILED: Part 2
FAIL: Should be:
  State_H1_Func1; Param1=77
  State_H2_Func1; Param1=5
  State_Func1; Param1=0
  State_HP_Func1; Param1=0; Param2=0x8b30ae0
FAIL: Is:
  State_Func1; Param1=77
TestRecall FAILED: Part 1
TestMulti FAILED: g_callcount[0] != 0
No error: Part 1
No error: Part 2
FAIL: Should be:
  State_Result_InHook; Param1=0
  State_Result; Param1=20
  State_Result_InHook; Param1=10
  State_Result; Param1=20
  State_Result_InHook; Param1=11
  State_Result; Param1=20
  State_Result_InHook; Param1=12
  State_Result; Param1=20
FAIL: Is:
  State_Result; Param1=0
  State_Result; Param1=10
  State_Result; Param1=11
  State_Result; Param1=12
TestRef FAILED: Part 3
No error: Part 1
FAIL: Should be:
  State_Func1_Pre1; Param1=0x8b30b5c
  State_Func1; Param1=0x8b304dc
  State_Func1_Ret; Param1=0x8b30b5c
FAIL: Is:
  State_Func1; Param1=0x8b304dc
  State_Func1_Ret; Param1=0x8b304dc
TestRefRet FAILED: Part 2
No error: Part 1
No error: Part 2
No error: Part 3
No error: Part 4
No error: Part 5
No error: Part 6
No error: Part 7.1
No error: Part 7.2
TestVPHooks passed
TestCPageAlloc passed
No error: GlobCtors
No error: Test0 Part1
FAIL: Should be:
  State_Deleg_0; Param1=1; Param2=0x8b30978; Param3=0; Param4=
  State_Func0; Param1=0x8b30978; Param2=
  State_Deleg_0; Param1=3; Param2=0x8b30978; Param3=1; Param4=
  State_Func0; Param1=0x8b30978; Param2=
FAIL: Is:
  State_Func0; Param1=0x8b30978; Param2=
  State_Func0; Param1=0x8b30978; Param2=
/home/travis/.travis/job_stages: line 78:  7534 Segmentation fault      ./core/sourcehook/test/test_sourcehook/test_sourcehook -v
The command "./core/sourcehook/test/test_sourcehook/test_sourcehook -v" exited with 139.
0.00s$ cd .. && mkdir build-sh-debug && cd build-sh-debug```

[pavolmarko]

It seems that the hooks are not being installed / called at all. I'd try only running the simplest of those test cases and adding a few debug prints, which I can write up when I'm not on the phone :) Kyle Sanderson <notifications@github.com> schrieb am Mi., 18. Juli 2018, 00:36:

…

GCC-4.9 also shows this. 4.8 completes no problem. $ ./core/sourcehook/test/test_sourcehook/test_sourcehook -v TestList passed No error: ModuleInMemory No error: Part 1 No error: Part 2 FAIL: Should be: State_F1_HookAdded; Param1=1 State_F1_Called State_F1_PreHandler_Called; Param1=0xfff98472 FAIL: Is: State_F1_HookAdded; Param1=1 State_F1_Called State_F1_Called TestBasic FAILED: Part 3 FAIL: Should be: State_EatYams_Called; Param1=0 State_EatYams_Called; Param1=1 State_EatYams_Handler_Called; Param1=0 State_EatYams_Called; Param1=0 State_EatYams_Handler_Called; Param1=1 State_EatYams_Called; Param1=1 FAIL: Is: State_EatYams_Called; Param1=0 State_EatYams_Called; Param1=1 State_EatYams_Called; Param1=0 State_EatYams_Called; Param1=1 TestVafmtAndOverload FAILED: Part 1 No error: Part 1 No error: Part 1.1 No error: Part 2 FAIL: Should be: State_Func1H_Called; Param1=0x8b30090 State_Func1_Called; Param1=0x8b30090 State_Func2H_Called; Param1=0x8b30090 State_Func2_Called; Param1=0x8b30098 State_Func3H_Called; Param1=0x8b30090 State_Func3_Called; Param1=0x8b30090 State_Func1H_Called; Param1=0x8b30090 State_Func1_Called; Param1=0x8b30090 State_Func2H_Called; Param1=0x8b30090 State_Func2_Called; Param1=0x8b30098 FAIL: Is: State_Func1_Called; Param1=0x8b30090 State_Func2_Called; Param1=0x8b30098 State_Func3_Called; Param1=0x8b30090 State_Func1_Called; Param1=0x8b30090 State_Func2_Called; Param1=0x8b30098 TestThisPtrOffs FAILED: Part 3 FAIL: Should be: State_Func1H_Called State_Func1_Called State_Func2_Called State_Func2H_Called State_Func3H_Called State_Func3_Called FAIL: Is: State_Func1_Called State_Func2_Called State_Func3_Called TestPlugSys FAILED: Part 1.1 No error: Part 1 No error: Part 2.1 No error: Part 3 No error: Part 4 No error: Part 5 No error: Part 6 TestBail passed FAIL: Should be: State_H_C1_F; Param1=0x810f93c State_H_C1_G; Param1=0x810f93c State_H_C2_F; Param1=0x810f938 State_H_C2_G; Param1=0x810f938 State_H_C3_F; Param1=0x810f934 State_H_C3_G; Param1=0x810f934 State_H_C4_F; Param1=0x810f930 State_H_C4_G; Param1=0x810f930 State_H_C5_F; Param1=0x810f92c State_H_C5_G; Param1=0x810f92c State_H_C6_F; Param1=0x810f928 State_H_C6_G; Param1=0x810f928 State_H_C7_F; Param1=0x810f924 State_H_C7_G; Param1=0x810f924 State_H_C8_F; Param1=0x810f920 State_H_C8_G; Param1=0x810f920 State_C8_G; Param1=0x810f920 State_C8_F; Param1=0x810f920 State_C7_G; Param1=0x810f924 State_C7_F; Param1=0x810f924 State_C6_G; Param1=0x810f928 State_C6_F; Param1=0x810f928 State_C5_G; Param1=0x810f92c State_C5_F; Param1=0x810f92c State_C4_G; Param1=0x810f930 State_C4_F; Param1=0x810f930 State_C3_G; Param1=0x810f934 State_C3_F; Param1=0x810f934 State_C2_G; Param1=0x810f938 State_C2_F; Param1=0x810f938 State_C1_G; Param1=0x810f93c State_C1_F; Param1=0x810f93c FAIL: Is: State_C1_F; Param1=0x810f93c TestReentr FAILED: 1 No error: Part 1 No error: Part 1.1 FAIL: Should be: State_Func1H_Called; Param1=0x8b31d50 State_Func1_Called; Param1=0x8b31d50 State_Func2H_Called; Param1=0x8b31d50; Param2=200 State_Func2_Called; Param1=0x8b31d50; Param2=-2023406815 State_Func3H_Called; Param1=0x8b31d50 State_Func3_Called; Param1=0x8b31d50 State_Return; Param1=3 State_Func4H_Called; Param1=0x8b31d50; Param2=400 State_Func4_Called; Param1=0x8b31d50; Param2=305419896 State_Return; Param1=4 FAIL: Is: State_Func1_Called; Param1=0x8b31d50 State_Func2_Called; Param1=0x8b31d50; Param2=200 State_Func3_Called; Param1=0x8b31d50 State_Return; Param1=3 State_Func4_Called; Param1=0x8b31d50; Param2=400 State_Return; Param1=4 TestManual FAILED: Part 2 FAIL: Should be: State_H1_Func1; Param1=77 State_H2_Func1; Param1=5 State_Func1; Param1=0 State_HP_Func1; Param1=0; Param2=0x8b30ae0 FAIL: Is: State_Func1; Param1=77 TestRecall FAILED: Part 1 TestMulti FAILED: g_callcount[0] != 0 No error: Part 1 No error: Part 2 FAIL: Should be: State_Result_InHook; Param1=0 State_Result; Param1=20 State_Result_InHook; Param1=10 State_Result; Param1=20 State_Result_InHook; Param1=11 State_Result; Param1=20 State_Result_InHook; Param1=12 State_Result; Param1=20 FAIL: Is: State_Result; Param1=0 State_Result; Param1=10 State_Result; Param1=11 State_Result; Param1=12 TestRef FAILED: Part 3 No error: Part 1 FAIL: Should be: State_Func1_Pre1; Param1=0x8b30b5c State_Func1; Param1=0x8b304dc State_Func1_Ret; Param1=0x8b30b5c FAIL: Is: State_Func1; Param1=0x8b304dc State_Func1_Ret; Param1=0x8b304dc TestRefRet FAILED: Part 2 No error: Part 1 No error: Part 2 No error: Part 3 No error: Part 4 No error: Part 5 No error: Part 6 No error: Part 7.1 No error: Part 7.2 TestVPHooks passed TestCPageAlloc passed No error: GlobCtors No error: Test0 Part1 FAIL: Should be: State_Deleg_0; Param1=1; Param2=0x8b30978; Param3=0; Param4= State_Func0; Param1=0x8b30978; Param2= State_Deleg_0; Param1=3; Param2=0x8b30978; Param3=1; Param4= State_Func0; Param1=0x8b30978; Param2= FAIL: Is: State_Func0; Param1=0x8b30978; Param2= State_Func0; Param1=0x8b30978; Param2= /home/travis/.travis/job_stages: line 78: 7534 Segmentation fault ./core/sourcehook/test/test_sourcehook/test_sourcehook -v The command "./core/sourcehook/test/test_sourcehook/test_sourcehook -v" exited with 139. 0.00s$ cd .. && mkdir build-sh-debug && cd build-sh-debug``` — You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub <#51 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AHnj3H9MxXjHnEL4hVTaSDgey1lGOI9Eks5uHmbvgaJpZM4VTny4> .

[pavolmarko]

So, the issue seems to be that GCC completely optimizes away the virtual function calls, so the hooks can not run :-)

It does this when the class the hook is being installed on is declared in the anonymous namespace (which makes sense, because if there's no class deriving from it, there can be no other subclass in the program, so there's no point doing virtual function calls).

It seems to work when I move the classes out of the anonymous namespace. I'd suggest having a pattern like:

namespace
{
StateList g_States;
SourceHook::ISourceHook *g_SHPtr;
SourceHook::Plugin g_PLID;
//..
MAKE_STATE_1(State_ModuleInMemory, bool);
// Other MAKE_STATE_s
}

class Test
{
public:
virtual void F1()
{
ADD_STATE(State_F1_Called);
}
virtual void F2(){}
//...
};

namespace
{
SH_DECL_HOOK1(Test, F299, SH_NOATTRIB, 0, bool, const char *);
SH_DECL_HOOK0_void(Test, F1, SH_NOATTRIB, 0);
// .. and other stuff
}

We may need to watch out for duplicate names between the test source files then.

Would be cool if you had time to do this! :-) Otherwise I may have come around to do it within the next ~2 weeks.

[KyleSanderson]

Thanks for checking PM :-) . I won't have time for a bit yet so if you're able to own this that would be fantastic.

Is this going to wreck us if Valve flips to gcc-4.9 or greater? Any idea what we can do when this happens?

[pavolmarko]

No, it seems that the core functionality works fine, only the tests are broken.

tldr; the compiler avoids virtual function calls, so SourceHook's functionality is skipped in the tests.

Long version:
SourceHook can only hook virtual functions and it works by overwriting the function's entry in the class' virtual function table.
When the compiler generates code for a virtual function call, it usually looks like this in pseudocode:

// Real code:
TestClass* test_class = /* ... */ ;
test_class->TestVirtualFunc(5);

// Pseudocode
// (1) get the vtable pointer from the TestClass instance
// This can be imagined as accessing a hidden member
TestClass_Vtable* vtable = test_class->vtable;
// (2) extract the function 
TestVirtualFuncType* func_ptr = vtable->entry_for_TestVirtualFunc;
// (3) Call the function through the function pointer. The this pointer can be imagined as a hidden
// first argument.
func_ptr(test_class 5);

SourceHook works by modifying the class' virtual function table, so step (2) will not yield a pointer to the the actual function "TestVirtualFunc", but instead to SourceHook's "hookman" function generated in the SH_DECL_HOOK* macro.

This virtual function call ("late binding") is pretty expensive compared to a non-virtual function call:

// Real code:
TestClass* test_class = /* ... */ ;
test_class->TestNonVirtualFunc(5);

// Pseudocode
TestNonVirtualFunc_Impl(test_class 5);

Here, the compiler knows the address of the TestNonVirtualFunc function, so it does not have to look it up anywhere.

Now, if a compiler sees code like this:

// Version A:
TestClass* test_class = new SuperDerivedFromTestClass();
test_class->TestVirtualFunc(5);

// Version B:
TestClass* test_class = function_from_another_compilation_unit();
test_class->TestVirtualFunc(5);// Version 2:

In Version B, |function_from_another_compilation_unit| could be returning a class derived from |TestClass| that's not even known to the compiler here, where |TestVirtualFunc| may be overridden, so it has to look up TestVirtualFunc in the vtable and call it through that.

In Version A however, the compiler knows exactly wht SuperDerivedFromTestClass is and where it generated code for SuperDerivedFromTestClass::TestVirtualFunc. Looking it up through the vtable would be a waste of resources, so the compiler actually generates code for a non-virtual function call which will be significantly faster at runtime.

This is an issue in tests for SourceHook: We want to test that virtual function calls behave in a specific way, so if the compiler is clever enough to not even generate a virtual function call, the tests fail. For example:

void Hook(int some_param) {
  printf("test\n");
}

class SuperDerivedFromTestClass {
public:
  void TestVirtualFunc(int some_param) override {
    printf("SDFTC::TVF called\n");
  }
};

TestClass* test_class = new SuperDerivedFromTestClass();
SH_ADD_HOOK(test_class, TestVirtualFunc, SH_STATIC(&Hook));
test_class->TestVirtualFunc(5);

This will only print "SDFTC::TVF called" but will not print "test", because the compiler sees that
test_class->TestVirtualFunc is defined right up there and can be called directly. The compiler does not assume that some hacky non-standard-compliant hackers want to mess with the function call.

This is what happens in this bug too: GCC can optimize away the virtual function calls in the tests.

[KyleSanderson]

The compiler does not assume that some hacky non-standard-compliant hackers want to mess with the function call.
Exactly 😸 . Maybe I didn't ask the question appropriately, but are we going to start seeing functions that were virtual become static calls if Valve has a similar class setup in their headers that we have in sh-tests?

I suppose when this happens (if it hasn't already) we're going to start to need actual detour support in MM:S instead of the standalone conflict-ridden header we've had in SM extensions for eons.

[pavolmarko]

For interfaces between the engine and the gamedll, this can not happen. It can happen e.g. if a class is instantiated and a virtual function called in the same compilation unit and the code is obvious to the compiler. But I think that's pretty rare. We should still add detour support, that's something I wanted to look into maybe 10 years ago :-) Kyle Sanderson <notifications@github.com> schrieb am Mo., 30. Juli 2018, 21:23:

…

The compiler does not assume that some hacky non-standard-compliant hackers want to mess with the function call. Exactly 😸 . Maybe I didn't ask the question appropriately, but are we going to start seeing functions that were virtual become static calls if Valve has a similar class setup in their headers that we have in sh-tests? I suppose when this happens (if it hasn't already) we're going to start to need actual detour support in MM:S instead of the standalone conflict-ridden header we've had in SM extensions for eons. — You are receiving this because you commented. Reply to this email directly, view it on GitHub <#51 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AHnj3Pqgsm8baIuYKNcQXeocNpkqlzZsks5uL10XgaJpZM4VTny4> .

[dvander]

Thanks for the write-up PM! Are there ways to trick the compiler outside of crossing translation units or using shared libraries? Would storing+reading the pointer in the heap, or using some kind of union fool it?

[KyleSanderson]

The test suite has been fixed by disabling this optimization at build-time as per #53 .

Leaving this open for further discussion.