Сафонова Ольга P3207
Closed by
1 year ago (Oct 22, 2023 9:14am UTC)
The changes were not merged into master
- Last updated by olga safonova
21 22 ./test.o : $(P)/test.asm ./lib.inc.o ./colon.o ./words.o ./dict.o 23 nasm -f elf64 -g -o ./test.o $(P)/test.asm 24 ./main.o : $(P)/main.asm ./lib.inc.o ./colon.o ./words.o ./dict.o 25 nasm -f elf64 -g -o ./main.o $(P)/main.asm 26 27 build: 28 nasm -f elf64 -g -o ./lib.o $(P)/lib.asm 29 nasm -f elf64 -g -o ./lib.inc.o $(P)/lib.inc 30 nasm -f elf64 -g -o ./words.inc.o $(P)/words.inc 31 nasm -f elf64 -g -o ./colon.inc.o $(P)/colon.inc 32 nasm -f elf64 -g -o ./dict.o $(P)/dict.asm 33 nasm -f elf64 -g -o ./dict.inc.o $(P)/dict.inc 34 nasm -f elf64 -g -o ./test.o $(P)/test.asm 35 nasm -f elf64 -g -o ./main.o $(P)/main.asm 36 ld -o ./executable ./lib.o ./lib.inc.o ./words.inc.o ./colon.inc.o ./dict.o ./dict.inc.o ./test.o ./main.o -
Очень громоздкий makefile. Используй шаблоны, например: - для nasm: %.o: %.asm @nasm -felf64 -o $@ $< - для ld: build: main.o lib.o dict.o @ld -o $@ $^ Для большего удобства используй автоматические переменные
-
Не нужно делать препроцессинг (применять nasm) к файлам с расширением .inc. Они просто включаются в другие файлы. Соответственно, их не нужно вклюячать как зависимости, когда делаешь ld
-
Добавь отдельную цель на компиляцию test.asm
Edited by Иван Тарасов-
changed this line in version 2 of the diff
- Last updated by olga safonova
29 30 .found: 31 mov rax, rsi 32 ret 33 34 .not_found: 35 xor rax, rax 36 ret 37 38 39 print_word: 40 add rdi, SKIP_LABEL_BYTES 41 .skip_key: 42 inc rdi 43 cmp byte[rdi], END_OF_STRING 44 jne .skip_key changed this line in version 2 of the diff
1 %include "lib.inc" 2 %include "colon.inc" 3 %include "dict.inc" 4 %include "words.inc" 5 6 section .bss 7 8 %define MAX_SYMBOLS 255 9 10 buff: resb 256 11 12 section .data 13 14 READING_ERROR: db "there is a reading error", 0 15 FIND_WORD_ERROR: db "word haven't found", 0 - Last updated by olga safonova
227 ; Эта функция должна дописывать к слову нуль-терминатор 228 229 read_word: 230 xor rax, rax 231 232 push r12 233 push r13 234 push rbx 235 236 mov rbx, rdi 237 mov r12, rsi 238 xor r13, r13 239 240 .reading: 241 call read_char 242 Это объясняется тем, что подразумевается под "словом". Функция read_word читает слово (набор символов до пробельного символа). Прочитанный ключ действительно идентичен ключу "first". То, о чём вы говорите, возможно, подошло бы под функцию read_string, где мы не привязаны к "слову". Тот факт, что ключ может состоять из большего количества слов, вероятно, правда, но в реализации lib.inc из предыдущей лабораторной функции read_string не достать. Допускаю, что вы правы и реализация read_string желательна, но, как мне кажется, не совсем обязательна.
@olga В любом случае, после ввода first word должно выводиться сообщение, что ключ не найден
- Last updated by Иван Тарасов
13 find_word: 14 add rsi, SKIP_LABEL_BYTES 15 16 push rsi 17 push rdi 18 call string_equals 19 pop rdi 20 pop rsi 21 22 sub rsi, SKIP_LABEL_BYTES 23 test rax, rax 24 jnz .found 25 cmp byte[rsi], END_OF_STRING 26 jz .not_found 27 mov rsi, qword[rsi] 28 jmp find_word У тебя в find_word к rsi сначала прибавляется 8 байтов, потом отнимается, затем, если ключ найден, снова прибавляется. По-хорошему, rdi не должен меняться. На каждой итерации записывай в rax адрес вхождения. Как только ключ будет найден, просто выходишь из функции. Если не найден, то перед выходом записываешь 0 в rax.
@olga Оно то работает)) Но смысл код ревью в том, чтобы сделать код компактным и без лишних действий
Мне кажется, условия того, что rsi меняться не должен - нет, а сохранение rsi в rax наоборот влечёт за собой лишние действия, так как используется функция, возвращающая в rax значение, из-за чего нужен какой-либо буферный регистр, чтобы запомнить значение rax. Сложение и вычитание 8 байт в данном случае - это условия лабораторной, где find_word должна возвращать ссылку на область данных, а не ссылку на само сообщение.
@olga rsi может меняться, а rdi, в котором может храниться введенная строка, - не должен. (Я писал про rdi)
@olga Ладно, хорошо, не переписывай. Просто убери строчки с push и pop перед и после string_length соответственно. Они не несут смысла, так как string_length их не меняет
Edited by Иван Тарасов
- Last updated by Иван Тарасов
158 xor rax, rax 159 160 push rdi ; for correct regs after calling functinons 161 push rsi 162 call string_length 163 pop rsi 164 pop rdi 165 166 push rdi 167 push rsi 168 mov rdi, rsi 169 mov rdx, rax 170 call string_length 171 pop rsi 172 pop rdi 173 Довольно сложная реализация string_length. Просто проходишься по символам и сравниваешь их. Если будет отличие хотя бы в одном символе - строки не равны (у тебя это сделано). Если в одном из регистров будет 0-терминатор, а в другом нет, то тоже не равны. Тут даже не нужно вычислять длину каждой строки. К тому же, зачем сохранять в стек rdi и rsi перед string_length, если эта функция не изменяет их
changed this line in version 2 of the diff
@olga Извини, я написал string_length. Я имел в виду string_equals. Но ты меня поняла)
@olga Я обещал, что проведу тебе код-ревью)) И тесты на ассемблере - это круто!
@IvanTarasov исправлено
@dsadyrin проверьте, пожалуйста
- Last updated by Даниил Садырин
1 P=$(shell pwd) 2 3 .PHONY: make build clean rebuild 4 5 ./%.o: $(P)/%.asm 6 nasm -f elf64 -g -o $@ $< 7 8 build: ./main.o ./lib.o ./lib.inc ./dict.o ./dict.inc ./words.inc ./colon.inc 9 ld -o ./executable ./lib.o ./dict.o ./main.o 10 11 test: ./test.o ./lib.o ./lib.inc ./dict.o ./dict.inc ./words.inc ./colon.inc 12 ld -o ./executable ./lib.o ./dict.o ./test.o 13 @olga файлы inc в ld включать не надо. Они лишние
Принято @olga Поставил за лабу @IvanTarasov и ревью
Edited by Даниил Садырин
