Несколько замечаний по поводу качества кода
От: Караваев Дмитрий Юрьевич  
Дата: 18.11.15 17:07
Оценка: -1
Статья:
Несколько замечаний по поводу качества кода
Автор(ы): Караваев Дмитрий Юрьевич
Дата: 10.02.2015
в статье приведено несколько примеров недостаточно эффективных команд в коде ядра Windows-XP.


Авторы:
Караваев Дмитрий Юрьевич

Аннотация:
В статье приведено несколько примеров недостаточно эффективных команд в коде ядра Windows-XP.
Re: Несколько замечаний по поводу качества кода
От: TSS_TSS http://kitrap08.blogspot.com/
Дата: 19.11.15 16:54
Оценка: 2 (1) +3
Здравствуйте, Караваев Дмитрий Юрьевич, Вы писали:

КДЮ>Статья:

КДЮ>Несколько замечаний по поводу качества кода
Автор(ы): Караваев Дмитрий Юрьевич
Дата: 10.02.2015
в статье приведено несколько примеров недостаточно эффективных команд в коде ядра Windows-XP.


КДЮ>Авторы:

КДЮ> Караваев Дмитрий Юрьевич

КДЮ>Аннотация:

КДЮ>В статье приведено несколько примеров недостаточно эффективных команд в коде ядра Windows-XP.

Но посмотрите, например, на один из бесчисленных фрагментов ядра:

4025AA 90 NOP
4025AB 90 NOP
4025AC 90 NOP
4025AD 90 NOP
4025AE 90 NOP
4025AF 8BFF MOV EDI,EDI
4025B1 55 PUSH EBP

Здесь, как и в сотнях других подобных мест, выравнивание превратилось в свою противоположность, и подпрограмма начинается из-за команд NOP как раз НЕ с адреса кратного 16 или хотя бы 4, а сами команды NOP стали просто бессмысленным раздуванием кода. Такое впечатление, что где-то в одном месте в ядре выравнивание «съехало» и далее везде дает противоположный эффект. Причем, чтобы увидеть это вовсе не требуется дисассемблировать код, достаточно любой подходящей утилитой посмотреть таблицу экспорта ядра NTOSKRNL.EXE. Там полно, например, нечетных (т.е. явно никак не выровненных) адресов входов.


Это не бессмысленное раздувание кода, а вполне себе осмысленное и имеющее название hot-patch. Mov edi, edi заменяется на short jmp, ведущий на 5 байт вверх, а этих пяти байт в свою очередь хватит, чтобы записать опкод long jmp'a.
Технологии больше десяти лет к слову. Предназначена для уменьшения(или избавления) количества ребутов (что критически важно если ОС стоит на сервере) при апдейтах.
http://kitrap08.blogspot.com/
Re: Несколько замечаний по поводу качества кода
От: TSS_TSS http://kitrap08.blogspot.com/
Дата: 19.11.15 17:36
Оценка: +2
Здравствуйте, Караваев Дмитрий Юрьевич, Вы писали:

КДЮ>Статья:

КДЮ>Несколько замечаний по поводу качества кода
Автор(ы): Караваев Дмитрий Юрьевич
Дата: 10.02.2015
в статье приведено несколько примеров недостаточно эффективных команд в коде ядра Windows-XP.


КДЮ>Авторы:

КДЮ> Караваев Дмитрий Юрьевич

КДЮ>Аннотация:

КДЮ>В статье приведено несколько примеров недостаточно эффективных команд в коде ядра Windows-XP.

Встречаются и такие конструкции:

IoFreeIrp:
414012 8BFF MOV EDI,EDI
414014 55 PUSH EBP
414015 8BEC MOV EBP,ESP
414017 5D POP EBP
414018 FF258C474800 JMP D PTR [0048478C]

IoAllocateIrp:
41406D 8BFF MOV EDI,EDI
41406F 55 PUSH EBP
414070 8BEC MOV EBP,ESP
414072 5D POP EBP
414073 FF2588474800 JMP D PTR [00484788]

Эти фрагменты вызывают в памяти анекдот про двух человек, один из которых выкапывал ямы, а второй шел за ним и закапывал. Согласно анекдоту должен был быть еще и третий, который бы сажал деревья, но он не пришел. Здесь выполняется пролог подпрограммы, а затем сразу эпилог. Возможно, в исходном тексте находится какой-то закомментированный фрагмент и это дает такой эффект. Возможно, это было сделано для каких-то отладочных остановок. Но как бы то не было, каждый раз, когда происходит обращение к подпрограммам IoAllocateIrp и IoFreeIrp, сначала выполняются бессмысленные команды.


Эти две функции объявлены как NTKERNELAPI(DECLSPEC_IMPORT), поэтому генерация пролога для них это необходимость соблюдения стандартов(и да, это идет в ущерб оптимизации). А mov edi, edi — место для hotpatch'a.
Поэтому никаких бессмысленных команд тут нет.
http://kitrap08.blogspot.com/
Re: Несколько замечаний по поводу качества кода
От: TSS_TSS http://kitrap08.blogspot.com/
Дата: 19.11.15 17:46
Оценка: +2
Здравствуйте, Караваев Дмитрий Юрьевич, Вы писали:

КДЮ>Статья:

КДЮ>Несколько замечаний по поводу качества кода
Автор(ы): Караваев Дмитрий Юрьевич
Дата: 10.02.2015
в статье приведено несколько примеров недостаточно эффективных команд в коде ядра Windows-XP.


КДЮ>Авторы:

КДЮ> Караваев Дмитрий Юрьевич

КДЮ>Аннотация:

КДЮ>В статье приведено несколько примеров недостаточно эффективных команд в коде ядра Windows-XP.

Во многих местах ядра происходит обращение к 2-байтовым объектам, например:

409378 668B4564 MOV AX,[EBP]+64
40937C 6683E0FC AND AX,FFFC
409380 668B4A04 MOV CX,[EDX]+4
409384 6683E1FC AND CX,FFFC
409388 663BC8 CMP CX,AX
40938B 7547 JNZ 4093D4

Недостаток здесь в том, что у транслятора не хватило «смелости» всегда читать эти объекты в 4-байтовые регистры, т.е. сделать код короче из-за исключения префиксов в командах чтения (даже оставляя префиксы в командах умножения и сравнения)


Это код из KiTrap0D, а он, как и другие обработчики ISR написан на ассемблере, соответственно, никто там сделать короче код не мог, оптимизирующего компилятора у ассемблера нет.
Можно ли было его написать человеку с предложенными вами оптимизациями? Нельзя, в al находится опкод, который вызвал trap.
http://kitrap08.blogspot.com/
Re: Несколько замечаний по поводу качества кода
От: TSS_TSS http://kitrap08.blogspot.com/
Дата: 19.11.15 18:02
Оценка:
Здравствуйте, Караваев Дмитрий Юрьевич, Вы писали:

КДЮ>Статья:

КДЮ>Несколько замечаний по поводу качества кода
Автор(ы): Караваев Дмитрий Юрьевич
Дата: 10.02.2015
в статье приведено несколько примеров недостаточно эффективных команд в коде ядра Windows-XP.


КДЮ>Авторы:

КДЮ> Караваев Дмитрий Юрьевич

КДЮ>Аннотация:

КДЮ>В статье приведено несколько примеров недостаточно эффективных команд в коде ядра Windows-XP.

Еще один существенный недостаток кода – большое число пересылок регистров. Например, типичный фрагмент:

40FAE0 64A120000000 MOV EAX,FS:[00000020]
40FAE6 8BF8 MOV EDI,EAX
40FAE8 8BB748050000 MOV ESI,[EDI]+548
40FAEE FF460C INC D PTR [ESI]+0C
40FAF1 8BCE MOV ECX,ESI
40FAF3 E87FA7FFFF CALL 40A277 ;ExInterlockedPopEntrySList
40FAF8 85C0 TEST EAX,EAX
40FAFA 0F84843A0000 JJE 413584

Его можно было бы короче записать так, не пересылая каждый раз данные сначала в регистры EAX и ESI:

40FAE0 648B3D20000000 MOV EDI,FS:[00000020]
40FAE7 8B8F48050000 MOV ECX,[EDI]+548
40FAED FF410C INC D PTR [ECX]+0C
40FAF0 E87DA7FFFF CALL 40A277 ;ExInterlockedPopEntrySList
40FAF5 85C0 TEST EAX,EAX
40FAF7 0F84823A0000 JJE 413584


Нельзя так написать, ибо приведенная в виде инлайна функция KeGetCurrentPrcb развертывается в разный код на однопроцессорных и многопроцессорных ядрах.
А функции как известно, должны вертать результат в eax.
http://kitrap08.blogspot.com/
Re[2]: Несколько замечаний по поводу качества кода
От: EreTIk EreTIk's Box
Дата: 20.11.15 12:10
Оценка:
TSS>Эти две функции объявлены как NTKERNELAPI(DECLSPEC_IMPORT), поэтому генерация пролога для них это необходимость соблюдения стандартов(и да, это идет в ущерб оптимизации).
Про какие стандарты идет речь? Стандарт вызова функций говорит только о передачи параметров, но не заставляет генерировать пролог функции.

Эта же функция, но в Win10 x86 (10.0.10240.16384):
.00454965: CCCCCCCCCCCCCCCCCCCCCC         int          3
IoFreeIrp: FF2510106700                   jmp          d,[pIoFreeIrp]
.00454976: CCCCCCCCCCCCCCCCCCCC           int          3
Re[2]: Несколько замечаний по поводу качества кода
От: EreTIk EreTIk's Box
Дата: 20.11.15 12:21
Оценка: +1
TSS>

TSS>Еще один существенный недостаток кода – большое число пересылок регистров. Например, типичный фрагмент:

TSS>40FAE0 64A120000000 MOV EAX,FS:[00000020]
TSS>40FAE6 8BF8 MOV EDI,EAX
TSS>40FAE8 8BB748050000 MOV ESI,[EDI]+548
TSS>40FAEE FF460C INC D PTR [ESI]+0C
TSS>40FAF1 8BCE MOV ECX,ESI
TSS>40FAF3 E87FA7FFFF CALL 40A277 ;ExInterlockedPopEntrySList
TSS>40FAF8 85C0 TEST EAX,EAX
TSS>40FAFA 0F84843A0000 JJE 413584

TSS>Его можно было бы короче записать так, не пересылая каждый раз данные сначала в регистры EAX и ESI:

TSS>40FAE0 648B3D20000000 MOV EDI,FS:[00000020]
TSS>40FAE7 8B8F48050000 MOV ECX,[EDI]+548
TSS>40FAED FF410C INC D PTR [ECX]+0C
TSS>40FAF0 E87DA7FFFF CALL 40A277 ;ExInterlockedPopEntrySList
TSS>40FAF5 85C0 TEST EAX,EAX
TSS>40FAF7 0F84823A0000 JJE 413584


TSS>Нельзя так написать, ибо приведенная в виде инлайна функция KeGetCurrentPrcb развертывается в разный код на однопроцессорных и многопроцессорных ядрах.

TSS>А функции как известно, должны вертать результат в eax.

А что мешает оптимизатору работать после инлайна? Опять же десятка:
KeTryToAcquireQueuedSpinLock:             mov          edi,edi
<...>
.005C1A36: 648B3520000000                 mov          esi,fs:[000000020]
.005C1A3D: 8AD8                           mov          bl,al
.005C1A3F: 8D8E18040000                   lea          ecx,[esi][000000418]

KiHeteroChooseTargetProcessor:            mov          edi,edi
<...>
.005C2130: 648B1520000000                 mov          edx,fs:[000000020]
.005C2137: 8B4350                        7mov          eax,[ebx][050]
.005C213A: 85C0                           test         eax,eax
.005C213C: 7411                           jz          .0005C214F --↓6
.005C213E: 0382F43A0000                   add          eax,[edx][000003AF4]
Re[3]: Несколько замечаний по поводу качества кода
От: TSS_TSS http://kitrap08.blogspot.com/
Дата: 20.11.15 15:15
Оценка:
Здравствуйте, EreTIk, Вы писали:

TSS>>Эти две функции объявлены как NTKERNELAPI(DECLSPEC_IMPORT), поэтому генерация пролога для них это необходимость соблюдения стандартов(и да, это идет в ущерб оптимизации).

ETI>Про какие стандарты идет речь? Стандарт вызова функций говорит только о передачи параметров, но не заставляет генерировать пролог функции.

ETI>Эта же функция, но в Win10 x86 (10.0.10240.16384):

ETI>
ETI>.00454965: CCCCCCCCCCCCCCCCCCCCCC         int          3
ETI>IoFreeIrp: FF2510106700                   jmp          d,[pIoFreeIrp]
ETI>.00454976: CCCCCCCCCCCCCCCCCCCC           int          3
ETI>


Да, вероятно я не прав насчет стандарта. Тем не менее, по умолчанию компилятор всегда лепит пролог, если явно не указать ему этого не делать. В сорцах win10 видимо явно указана директива __declspec(naked).
http://kitrap08.blogspot.com/
Re[4]: Несколько замечаний по поводу качества кода
От: EreTIk EreTIk's Box
Дата: 20.11.15 15:26
Оценка:
TSS>Да, вероятно я не прав насчет стандарта. Тем не менее, по умолчанию компилятор всегда лепит пролог, если явно не указать ему этого не делать. В сорцах win10 видимо явно указана директива __declspec(naked).

Думаю, что дело в версии и/или настройках компилятора. Пример:
void (*g_pFunc)() = nullptr;

extern "C"
__declspec(dllexport)
void f()
{
    g_pFunc();
}


>cl /O2 /Zi /MTd /EHsc test.cpp /link /RELEASE /OPT:REF
Microsoft (R) 32-bit C/C++ Optimizing Compiler Version 15.00.30729.01 for 80x86
Copyright (C) Microsoft Corporation.  All rights reserved.

test.cpp
Microsoft (R) Incremental Linker Version 9.00.30729.4999
Copyright (C) Microsoft Corporation.  All rights reserved.

/out:test.exe
/debug
/RELEASE
/OPT:REF
test.obj
   Creating library test.lib and object test.exp


f:         FF25C03F4200                   jmp          d,[000423FC0]
.00401006: CCCCCCCCCCCCCCCCCCCC           int          3
Re[3]: Несколько замечаний по поводу качества кода
От: TSS_TSS http://kitrap08.blogspot.com/
Дата: 20.11.15 15:29
Оценка: +1
Здравствуйте, EreTIk, Вы писали:

TSS>>

TSS>>Еще один существенный недостаток кода – большое число пересылок регистров. Например, типичный фрагмент:

TSS>>40FAE0 64A120000000 MOV EAX,FS:[00000020]
TSS>>40FAE6 8BF8 MOV EDI,EAX
TSS>>40FAE8 8BB748050000 MOV ESI,[EDI]+548
TSS>>40FAEE FF460C INC D PTR [ESI]+0C
TSS>>40FAF1 8BCE MOV ECX,ESI
TSS>>40FAF3 E87FA7FFFF CALL 40A277 ;ExInterlockedPopEntrySList
TSS>>40FAF8 85C0 TEST EAX,EAX
TSS>>40FAFA 0F84843A0000 JJE 413584

TSS>>Его можно было бы короче записать так, не пересылая каждый раз данные сначала в регистры EAX и ESI:

TSS>>40FAE0 648B3D20000000 MOV EDI,FS:[00000020]
TSS>>40FAE7 8B8F48050000 MOV ECX,[EDI]+548
TSS>>40FAED FF410C INC D PTR [ECX]+0C
TSS>>40FAF0 E87DA7FFFF CALL 40A277 ;ExInterlockedPopEntrySList
TSS>>40FAF5 85C0 TEST EAX,EAX
TSS>>40FAF7 0F84823A0000 JJE 413584


TSS>>Нельзя так написать, ибо приведенная в виде инлайна функция KeGetCurrentPrcb развертывается в разный код на однопроцессорных и многопроцессорных ядрах.

TSS>>А функции как известно, должны вертать результат в eax.

ETI>А что мешает оптимизатору работать после инлайна? Опять же десятка:

ETI>
ETI>KeTryToAcquireQueuedSpinLock:             mov          edi,edi
ETI><...>
ETI>.005C1A36: 648B3520000000                 mov          esi,fs:[000000020]
ETI>.005C1A3D: 8AD8                           mov          bl,al
ETI>.005C1A3F: 8D8E18040000                   lea          ecx,[esi][000000418]
ETI>

ETI>
ETI>KiHeteroChooseTargetProcessor:            mov          edi,edi
ETI><...>
ETI>.005C2130: 648B1520000000                 mov          edx,fs:[000000020]
ETI>.005C2137: 8B4350                        7mov          eax,[ebx][050]
ETI>.005C213A: 85C0                           test         eax,eax
ETI>.005C213C: 7411                           jz          .0005C214F --↓6
ETI>.005C213E: 0382F43A0000                   add          eax,[edx][000003AF4]
ETI>


Это совершенно другая функция ( 1й пример ) — заинлайненная KeGetCurrentIrql. KeGetCurrentPrcb к слову написана с асм вставкой, это оптимизатору и мешает. А KeGetCurrentIrql написана на С, и у оптимизатора развязаны руки.
По второму примеру лень искать, но видимо там асм вставок тоже не используется, раз код оптимизирован.
http://kitrap08.blogspot.com/
Re[5]: Несколько замечаний по поводу качества кода
От: TSS_TSS http://kitrap08.blogspot.com/
Дата: 20.11.15 15:33
Оценка:
Здравствуйте, EreTIk, Вы писали:

TSS>>Да, вероятно я не прав насчет стандарта. Тем не менее, по умолчанию компилятор всегда лепит пролог, если явно не указать ему этого не делать. В сорцах win10 видимо явно указана директива __declspec(naked).


ETI>Думаю, что дело в версии и/или настройках компилятора. Пример:

ETI>
ETI>void (*g_pFunc)() = nullptr;

ETI>extern "C"
ETI>__declspec(dllexport)
ETI>void f()
ETI>{
ETI>    g_pFunc();
ETI>}
ETI>


ETI>
>>cl /O2 /Zi /MTd /EHsc test.cpp /link /RELEASE /OPT:REF
ETI>Microsoft (R) 32-bit C/C++ Optimizing Compiler Version 15.00.30729.01 for 80x86
ETI>Copyright (C) Microsoft Corporation.  All rights reserved.

ETI>test.cpp
ETI>Microsoft (R) Incremental Linker Version 9.00.30729.4999
ETI>Copyright (C) Microsoft Corporation.  All rights reserved.

ETI>/out:test.exe
ETI>/debug
ETI>/RELEASE
ETI>/OPT:REF
ETI>test.obj
ETI>   Creating library test.lib and object test.exp
ETI>


ETI>
ETI>f:         FF25C03F4200                   jmp          d,[000423FC0]
ETI>.00401006: CCCCCCCCCCCCCCCCCCCC           int          3
ETI>


Это безусловно зависит от компилятора, но все таки мы говорим про windows, а она использует микрософтовский компилятор. Соответственно, с прологом там все более менее понятно. Хотя признаться, я не знаю как с этим дела в последних версиях компиляторов.
http://kitrap08.blogspot.com/
Re[4]: Несколько замечаний по поводу качества кода
От: EreTIk EreTIk's Box
Дата: 20.11.15 15:56
Оценка:
TSS>Это совершенно другая функция ( 1й пример ) — заинлайненная KeGetCurrentIrql. KeGetCurrentPrcb к слову написана с асм вставкой, это оптимизатору и мешает. А KeGetCurrentIrql написана на С, и у оптимизатора развязаны руки.
TSS>По второму примеру лень искать, но видимо там асм вставок тоже не используется, раз код оптимизирован.

Вот использование KeGetCurrentPrcb() в десятке:
ExInitPoolLookasidePointers:              mov          edi,edi
.00684FD4: 53                             push         ebx
.00684FD5: 56                             push         esi
.00684FD6: 648B3520000000                 mov          esi,fs:[000000020]
.00684FDD: BBC0A46300                     mov          ebx,ExPoolLookasideListHead
.00684FE2: 57                             push         edi
.00684FE3: BF80646500                     mov          edi,ExpScratchBufferLookasi
.00684FE8: 83BECC03000000                 cmp          d,[esi][0000003CC],0



Но в WRK такой код:
FORCEINLINE
PKPRCB
NTAPI
KeGetCurrentPrcb (VOID)
{
#if (_MSC_FULL_VER >= 13012035)
    return (PKPRCB) (ULONG_PTR) __readfsdword (FIELD_OFFSET (KPCR, Prcb));
#else
    __asm {  mov eax, _PCR KPCR.Prcb     }
#endif
}

Так что да, скорее всего оптимизатор старого компилятора выключался из-за asm-вставки
Re[6]: Несколько замечаний по поводу качества кода
От: EreTIk EreTIk's Box
Дата: 20.11.15 16:01
Оценка:
ETI>>Думаю, что дело в версии и/или настройках компилятора. Пример:
ETI>>
ETI>>void (*g_pFunc)() = nullptr;

ETI>>extern "C"
ETI>>__declspec(dllexport)
ETI>>void f()
ETI>>{
ETI>>    g_pFunc();
ETI>>}
ETI>>


ETI>>
>>>cl /O2 /Zi /MTd /EHsc test.cpp /link /RELEASE /OPT:REF
ETI>>Microsoft (R) 32-bit C/C++ Optimizing Compiler Version 15.00.30729.01 for 80x86
ETI>>Copyright (C) Microsoft Corporation.  All rights reserved.

ETI>>test.cpp
ETI>>Microsoft (R) Incremental Linker Version 9.00.30729.4999
ETI>>Copyright (C) Microsoft Corporation.  All rights reserved.

ETI>>/out:test.exe
ETI>>/debug
ETI>>/RELEASE
ETI>>/OPT:REF
ETI>>test.obj
ETI>>   Creating library test.lib and object test.exp
ETI>>


ETI>>
ETI>>f:         FF25C03F4200                   jmp          d,[000423FC0]
ETI>>.00401006: CCCCCCCCCCCCCCCCCCCC           int          3
ETI>>


TSS>Это безусловно зависит от компилятора, но все таки мы говорим про windows, а она использует микрософтовский компилятор. Соответственно, с прологом там все более менее понятно. Хотя признаться, я не знаю как с этим дела в последних версиях компиляторов.


Так мой пример тоже построен на кодо-генерации MS'овских компилятора и линкера (не самых свежих — Microsoft Visual Studio 9.0).
Мой посыл был в том, что старый компилятор, которым собирали XP, мог оставлять пролог из-за недоработки или специфических опций.
Re[7]: Несколько замечаний по поводу качества кода
От: TSS_TSS http://kitrap08.blogspot.com/
Дата: 20.11.15 16:42
Оценка:
Здравствуйте, EreTIk, Вы писали:


ETI>>>Думаю, что дело в версии и/или настройках компилятора. Пример:

ETI>>>
ETI>>>void (*g_pFunc)() = nullptr;

ETI>>>extern "C"
ETI>>>__declspec(dllexport)
ETI>>>void f()
ETI>>>{
ETI>>>    g_pFunc();
ETI>>>}
ETI>>>


ETI>>>
>>>>cl /O2 /Zi /MTd /EHsc test.cpp /link /RELEASE /OPT:REF
ETI>>>Microsoft (R) 32-bit C/C++ Optimizing Compiler Version 15.00.30729.01 for 80x86
ETI>>>Copyright (C) Microsoft Corporation.  All rights reserved.

ETI>>>test.cpp
ETI>>>Microsoft (R) Incremental Linker Version 9.00.30729.4999
ETI>>>Copyright (C) Microsoft Corporation.  All rights reserved.

ETI>>>/out:test.exe
ETI>>>/debug
ETI>>>/RELEASE
ETI>>>/OPT:REF
ETI>>>test.obj
ETI>>>   Creating library test.lib and object test.exp
ETI>>>


ETI>>>
ETI>>>f:         FF25C03F4200                   jmp          d,[000423FC0]
ETI>>>.00401006: CCCCCCCCCCCCCCCCCCCC           int          3
ETI>>>


TSS>>Это безусловно зависит от компилятора, но все таки мы говорим про windows, а она использует микрософтовский компилятор. Соответственно, с прологом там все более менее понятно. Хотя признаться, я не знаю как с этим дела в последних версиях компиляторов.


ETI>Так мой пример тоже построен на кодо-генерации MS'овских компилятора и линкера (не самых свежих — Microsoft Visual Studio 9.0).

ETI>Мой посыл был в том, что старый компилятор, которым собирали XP, мог оставлять пролог из-за недоработки или специфических опций.

Упс, совсем я плохой стал, мне почему-то почудился gcc. Тогда да, видимо дело в компиляторе (версии или настройках).
http://kitrap08.blogspot.com/
 
Подождите ...
Wait...
Пока на собственное сообщение не было ответов, его можно удалить.