Reverse Engineering der NLT II

[Obi-Wahn]

Genial!

[llm]

lohnt es sich für Tests das ganze durch andere DOS kompiler zu jagen - z.B. Borland 5.02 oder Watcom V2 - um zu schauen ob da unter DOS Fehler auftauchen, oder direkt auf Windows/Linux gehen mit aktuellen gcc?

[llm]

clang meckert hier

Code:

seg006.cpp:120:18: note: use '==' to turn this assignment into an equality comparison
  120 |        } while (list_i = list_i->next);
      |                        ^
      |                        ==

[llm]

Wenn die 20 Byte eliminiert wurden, wird der Code zu Rekonstruktionszwecken in ein anderes Verzeichnis kopiert.

macht es vielleicht sinn den Borland source mit defines auch unter gcc/clang/msvc kompilierbar zu machen - also ohne Verzeichnis-Kopie, oder sind einfach zu viele Änderungen nötig und das es unsauber wird?

[HenneNWH]

Wenn die 20 Byte eliminiert wurden, wird der Code zu Rekonstruktionszwecken in ein anderes Verzeichnis kopiert.

macht es vielleicht sinn den Borland source mit defines auch unter gcc/clang/msvc kompilierbar zu machen - also ohne Verzeichnis-Kopie, oder sind einfach zu viele Änderungen nötig und das es unsauber wird?

Warnungen sind behoben.

Die Verzeichniskopie ist notwendig damit, eventuell später, andere DOS-Versionen der SCHICKM.EXE rekonstruiert werden können.
Im Verzeichnis schick wird dann nach dem Kopieren alles so gehandhabt, wie im Verzeichnis gen.
D.h. Binäräquivalenz ist DANN nicht mehr notwendig und andere Compiler sind dann gern gesehen.

[llm]

scheint auf jeden Fall "gut" kompilierbar (kein linken) - mit aktuellen windows clang/msvc cl und watcom v2 wcl kompilieren die cpps schon mal ohne viele Warnungen - weiss aber nicht wie viel code nicht durch defines inaktiv ist

[llm]

noch ein paar Clang 21.1.4 Warnungen

Code:

seg002.cpp:4137:22: warning: passing an object that undergoes default argument promotion to 'va_start' has undefined behavior [-Wvarargs]
4137 |        va_start(arguments, icons);
      |                            ^
seg002.cpp:4123:34: note: parameter of type 'short' is declared here
4123 | void draw_loc_icons(signed short icons, ...)
      |                                  ^
1 warning generated.
seg024.cpp:63:17: warning: implicit conversion from '(unnamed enum at ./common.h:1126:1)' to 'signed char' changes value from 177 to -79 [-Wconstant-conversion]
  63 |        g_pp20_index = ARCHIVE_FILE_BUCH_DAT;
      |                      ~ ^~~~~~~~~~~~~~~~~~~~~
1 warning generated.
seg025.cpp:451:17: warning: implicit conversion from '(unnamed enum at ./common.h:1126:1)' to 'signed char' changes value from 177 to -79 [-Wconstant-conversion]
  451 |        g_pp20_index = ARCHIVE_FILE_BUCH_DAT;
      |                      ~ ^~~~~~~~~~~~~~~~~~~~~
1 warning generated.
seg028.cpp:189:4: warning: 'memcpy' will always overflow; destination buffer has size 96, but size argument is 192 [-Wfortify-source]
  189 |                        memcpy(gs_palette_buildings, g_buffer11_ptr, 0xc0);
      |                        ^
1 warning generated.
seg030.cpp:187:15: warning: cast to 'unsigned char *' from smaller integer type 'Bit32u' (aka 'unsigned int') [-Wint-to-pointer-cast]
  187 |        states_tab = (Bit8u*)partners_tab[tlk_informer].states_offset;
      |                      ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
1 warning generated.
seg031.cpp:85:15: warning: cast to 'M302de::struct_dialog_state *' from smaller integer type 'Bit32u' (aka 'unsigned int') [-Wint-to-pointer-cast]
  85 |        states_tab = (struct struct_dialog_state*)partners_tab[informer_id].states_offset;
      |                      ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
1 warning generated.
seg032.cpp:1279:36: warning: implicit conversion from 'int' to 'signed char' changes value from 254 to -2 [-Wconstant-conversion]
1279 |        g_pp20_index = (ARCHIVE_FILE_DNGS + 12);
      |                      ~  ~~~~~~~~~~~~~~~~~~^~~~
1 warning generated.
seg038.cpp:269:2: warning: 'memcpy' will always overflow; destination buffer has size 10, but size argument is 20 [-Wfortify-source]
  269 |        memcpy(g_fig_move_pathdir, path_table[best_dir], 20);
      |        ^
1 warning generated.
seg065.cpp:111:35: warning: implicit conversion from 'int' to 'signed char' changes value from 254 to -2 [-Wconstant-conversion]
  111 |        g_pp20_index = ARCHIVE_FILE_DNGS + 12;
      |                      ~ ~~~~~~~~~~~~~~~~~~^~~~
1 warning generated.
seg076.cpp:685:42: warning: format specifies type 'int' but the argument has type 'unsigned long long' [-Wformat]
  685 |                D1_INFO("sizeof(stair_struct) = %d\n", sizeof(stair_struct));
      |                                                ~~    ^~~~~~~~~~~~~~~~~~~~
      |                                                %llu
.\v302de.h:137:38: note: expanded from macro 'D1_INFO'
  137 | #define D1_INFO(...) fprintf(stderr, __VA_ARGS__)
      |                                      ^~~~~~~~~~~
seg076.cpp:834:42: warning: format specifies type 'int' but the argument has type 'unsigned long long' [-Wformat]
  834 |                D1_INFO("sizeof(fight_struct) = %d\n", sizeof(fight_struct));
      |                                                ~~    ^~~~~~~~~~~~~~~~~~~~
      |                                                %llu
.\v302de.h:137:38: note: expanded from macro 'D1_INFO'
  137 | #define D1_INFO(...) fprintf(stderr, __VA_ARGS__)
      |                                      ^~~~~~~~~~~
2 warnings generated.
seg090.cpp:451:60: error: hex escape sequence out of range
  451 |                                                "ALS %s MIT DER BARRIERE KOLLIDIERT, BRICHT SIE IN ST\x9aCKE.\x40"
      |                                                                                                      ^~~~~
1 error generated.
seg093.cpp:808:15: warning: unsequenced modification and access to 'l_di' [-Wunsequenced]
  807 |                                                if (is_mouse_in_rect(l_di - 4, answer - 4,
      |                                                                      ~~~~
  808 |                                                                (l_di = g_town_positions[i].x) + 4,
      |                                                                      ^
seg093.cpp:809:17: warning: unsequenced modification and access to 'answer' [-Wunsequenced]
  807 |                                                if (is_mouse_in_rect(l_di - 4, answer - 4,
      |                                                                                ~~~~~~
  808 |                                                                (l_di = g_town_positions[i].x) + 4,
  809 |                                                                (answer = g_town_positions[i].y) + 4))
      |                                                                        ^
2 warnings generated.
seg097.cpp:639:23: warning: passing an object that undergoes default argument promotion to 'va_start' has undefined behavior [-Wvarargs]
  639 |                va_start(arguments, options);
      |                                    ^
seg097.cpp:546:81: note: parameter of type 'short' is declared here
  546 | signed short GUI_dialogbox(Bit8u* picture, char *name, char *text, signed short options, ...)
      |                                                                                ^
seg097.cpp:833:22: warning: passing an object that undergoes default argument promotion to 'va_start' has undefined behavior [-Wvarargs]
  833 |        va_start(arguments, options);
      |                            ^
seg097.cpp:783:48: note: parameter of type 'signed char' is declared here
  783 | signed short GUI_radio(char *text, signed char options, ...)
      |                                                ^
2 warnings generated.
seg109.cpp:957:28: warning: implicit conversion from 'int' to 'Bit8s' (aka 'signed char') changes value from 255 to -1 [-Wconstant-conversion]
  957 |                g_gather_herbs_special = 255;
      |                                        ~ ^~~
1 warning generated.
seg112.cpp:156:29: warning: implicit conversion from 'int' to 'Bit8s' (aka 'signed char') changes value from 130 to -126 [-Wconstant-conversion]
  156 |                        g_gather_herbs_special = 130;
      |                                                ~ ^~~
1 warning generated.
seg114.cpp:283:29: warning: implicit conversion from 'int' to 'Bit8s' (aka 'signed char') changes value from 131 to -125 [-Wconstant-conversion]
  283 |                        g_gather_herbs_special = 131;
      |                                                ~ ^~~
1 warning generated.
seg116.cpp:190:28: warning: implicit conversion from 'int' to 'Bit8s' (aka 'signed char') changes value from 157 to -99 [-Wconstant-conversion]
  190 |                g_gather_herbs_special = 157;
      |                                        ~ ^~~
1 warning generated.

mein Test-Build script sieht so aus

Code:

@echo off

set clang_exe="c:\Program Files\LLVM\bin\clang.exe" -Wno-deprecated-declarations -Wno-deprecated-register -Wno-format-security

%clang_exe% datseg.cpp -c
%clang_exe% seg001.cpp -c
%clang_exe% seg002.cpp -c
%clang_exe% seg003.cpp -c
...

für den windows build clang/msvc und watcom musste ich nur den #include <unistd.h> durch io.h ersetzen

und noch unter Linux ein paar gcc 15.2.1 warnungen

Code:

seg028.cpp: In function ‘void M302de::load_area_description(short int)’:
seg028.cpp:340:53: warning: ‘ssize_t read(int, void*, size_t)’ writing 1000 bytes into a region of size 900 overflows the destination [-Wstringop-overflow=]
  340 |                        g_locations_tab_size = _read(fd, g_locations_tab, 1000);
In file included from v302de.h:76,
                from seg028.cpp:21:
datseg.h:1733:24: note: destination object ‘M302de::g_locations_tab’ of size 900
1733 | extern struct location g_locations_tab[150];    //ds:0xc025; seg028, seg64, seg066, seg74, seg094
      |                        ^~~~~~~~~~~~~~~
In file included from seg028.cpp:18:
/usr/include/unistd.h:371:16: note: in a call to function ‘ssize_t read(int, void*, size_t)’ declared with attribute ‘access (write_only, 2, 3)’
  371 | extern ssize_t read (int __fd, void *__buf, size_t __nbytes) __wur
      |                ^~~~
seg028.cpp:355:61: warning: ‘ssize_t read(int, void*, size_t)’ writing 1000 bytes into a region of size 900 overflows the destination [-Wstringop-overflow=]
  355 |                                g_locations_tab_size = _read(fd, g_locations_tab, 1000);
datseg.h:1733:24: note: destination object ‘M302de::g_locations_tab’ of size 900
1733 | extern struct location g_locations_tab[150];    //ds:0xc025; seg028, seg64, seg066, seg74, seg094
      |                        ^~~~~~~~~~~~~~~
/usr/include/unistd.h:371:16: note: in a call to function ‘ssize_t read(int, void*, size_t)’ declared with attribute ‘access (write_only, 2, 3)’
  371 | extern ssize_t read (int __fd, void *__buf, size_t __nbytes) __wur
      |                ^~~~
seg028.cpp: In function ‘void M302de::seg028_0224()’:
seg028.cpp:189:31: warning: ‘void* memcpy(void*, const void*, size_t)’ writing 192 bytes into a region of size 96 overflows the destination [-Wstringop-overflow=]
  189 |                        memcpy(gs_palette_buildings, g_buffer11_ptr, 0xc0);
      |                        ~~~~~~^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
datseg.h:1126:15: note: destination object ‘M302de::gs_palette_buildings’ of size 96
1126 | extern Bit8u  gs_palette_buildings[3 * 32];    //ds:0x3eb3; seg002, seg028, seg066
      |              ^~~~~~~~~~~~~~~~~~~~
seg030.cpp: In function ‘void M302de::do_talk(short int, short int)’:
seg030.cpp:187:22: warning: cast to pointer from integer of different size [-Wint-to-pointer-cast]
  187 |        states_tab = (Bit8u*)partners_tab[tlk_informer].states_offset;
      |                      ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
seg031.cpp: In function ‘void M302de::do_random_talk(short int, short int)’:
seg031.cpp:85:22: warning: cast to pointer from integer of different size [-Wint-to-pointer-cast]
  85 |        states_tab = (struct struct_dialog_state*)partners_tab[informer_id].states_offset;
      |                      ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
seg038.cpp: In function ‘void M302de::FIG_find_path_to_target_backtrack(Bit8u*, short int, short int, short int, signed char, short int, short int, short int)’:
seg038.cpp:269:15: warning: ‘void* memcpy(void*, const void*, size_t)’ writing 20 bytes into a region of size 10 overflows the destination [-Wstringop-overflow=]
  269 |        memcpy(g_fig_move_pathdir, path_table[best_dir], 20);
      |        ~~~~~~^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
In file included from v302de.h:76,
                from seg038.cpp:14:
datseg.h:1647:20: note: destination object ‘M302de::g_fig_move_pathdir’ of size 10
1647 | extern signed char g_fig_move_pathdir[10];      // ds:0xd823; seg034, seg036, seg037, seg038
      |                    ^~~~~~~~~~~~~~~~~~
seg066.cpp: In function ‘void M302de::city_fade_and_colors()’:
seg066.cpp:1453:23: warning: ‘void* memcpy(void*, const void*, size_t)’ reading 288 bytes from a region of size 96 [-Wstringop-overread]
1453 |                memcpy(dst, gs_palette_floor, 0x120);
      |                ~~~~~~^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
In file included from v302de.h:76,
                from seg066.cpp:12:
datseg.h:1125:15: note: source object ‘M302de::gs_palette_floor’ of size 96
1125 | extern Bit8u  gs_palette_floor[3 * 32];        //ds:0x3e53; seg002
      |              ^~~~~~~~~~~~~~~~
seg075.cpp: In function ‘void M302de::DNG_enter_dungeon(short int)’:
seg075.cpp:1343:23: warning: ‘void* memcpy(void*, const void*, size_t)’ reading 288 bytes from a region of size 96 [-Wstringop-overread]
1343 |                memcpy(g_renderbuf_ptr + 0x1f4, gs_palette_floor, 0x120);
      |                ~~~~~~^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
In file included from v302de.h:76,
                from seg075.cpp:13:
datseg.h:1125:15: note: source object ‘M302de::gs_palette_floor’ of size 96
1125 | extern Bit8u  gs_palette_floor[3 * 32];        //ds:0x3e53; seg002
      |              ^~~~~~~~~~~~~~~~
seg090.cpp: In function ‘short int M302de::DNG12_handler()’:
seg090.cpp:453:49: warning: hex escape sequence out of range
  453 |                                                "SCHADE DASS IHR DAS NICHT FR\x9aHER BEMERKT HABT.\0",
      |                                                ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~

[HenneNWH]

Die Warnungen seh ich bei mir auch. Die müssen erstmal aus historischen Gründen bleiben.

[siebenstreich]

Keine Kritik, nur etwas ungefiltert nachgedacht:

Du hast das jetzt mit inline Assembler hingebogen. Irgendwie finde ich das immer noch etwas unbefriedigend, weil im Original wird an diesen Stellen ja ziemlich sicher C-Code gestanden haben. (Konsequent weitergedacht könnte man dann ja auch einfach das ganze Spiel in Assembler hinschreiben und sagen, man habe es reverse engineered...)

Vermutlich hast du an diesen Stellen vieles in C ausprobiert und nichts hat funktioniert. Trotzdem wäre es irgendwie cool, den richtigen C-Code noch zu finden.

Noch eine Sache: Wärst du dafür offen, vor dem "Fork" noch über gewisse Code-Bereinigungen (z.B. einheitlichere Variablenbenennung, etc.) zu diskutieren?

[HenneNWH]

@Siebenstreich: Das ist korrekt. Der ASM Code ist allerdings semantisch äquivalent zu dem C-Code welchen ich geschrieben hab.
Da in der Weiterentwicklung vorgesehen ist den ASM Code weitestgehend zu Entfernen bin ich damit zufrieden.

Die 20 Byte die noch offen sind betreffen nur technische Details mit Overlays und der DOS-EXE.
Diese Dinge spielen in der "neuen Welt" keine Rolle mehr.

Wie wäre es denn mit einem Meeting am Sonntag im Zeitraum vom (17:00 - 20:00 Uhr) für ca. 1 Stunde?
Deinen Vorschlag finde ich gut.

[NewProggie]

@Siebenstreich: Das ist korrekt. Der ASM Code ist allerdings semantisch äquivalent zu dem C-Code welchen ich geschrieben hab.
Da in der Weiterentwicklung vorgesehen ist den ASM Code weitestgehend zu Entfernen bin ich damit zufrieden.

Die 20 Byte die noch offen sind betreffen nur technische Details mit Overlays und der DOS-EXE.
Diese Dinge spielen in der "neuen Welt" keine Rolle mehr.

Wie wäre es denn mit einem Meeting am Sonntag im Zeitraum vom (17:00 - 20:00 Uhr) für ca. 1 Stunde?
Deinen Vorschlag finde ich gut.

Bei einem Meeting würde ich mich potentiell auch gerne mit einklinken

[siebenstreich]

Also ich hätte heute 17:00 Uhr auch Zeit. Ich hätte einen BBB-Server zur Verfügung, wo wir diskutieren könnten! Bitte kurze Rückmeldung, dann schicke ich an die Interessenten einen Link per PM.

[HenneNWH]

17:30 Uhr ist okay für mich! BBB klingt gut.

[llm]

würde auch gerne ein wenig zuhören, kann aber sein das ich 17:30 doch nicht kann, also spontan dabei

[siebenstreich]

Henne, Ilm: Ich hab euch eine Nachricht mit einem Link geschickt. Bis gleich!

[Obi-Wahn]

Na, gibt es Ergebnisse, die für uns interessant sind? :-)

[llm]

Na, gibt es Ergebnisse, die für uns interessant sind? :-)

-kleine Vorstellungsrunde, Technik-Geplauder was jeder so macht, ich hab andere Reverse-Engineering-Projekte kurz gezeigt
-Henne hat über die nächsten Schritte geredet - Portierung auf SDL für Grafik/Maus/Tastatur, File-Routinen 
-Henne hat viel aus dem Nähkästchen des Reverse-Engineerings geplaudert
-Siebenstreich hat Namensgebungen von Variablen angesprochen damit sie eindeutiger sind
-Bisschen über Zukunft mit CMake, weitere Kompiler, Platformen, Testing (Sanitizer,Coverity,...)

dann musste ich aber so nach ~1.5h raus - ich denke Henne und Siebenstreich haben noch die Nacht zum Tag gemacht

[HenneNWH]

Ja, Siebenstreich ist jetzt ein BrightEyes-Developer. 

Ilm hat mich darauf hingewiesen, dass es weltweit schätzungsweise 1000 solcher Menschen wie mich gibt und hat Links zu ähnlichen RE-Spezialisten und Projekten geteilt. 
Diese beschäftigen sich mit ähnlichen Fragestellungen wie ich (nur eben manchmal ganz anders):

neuviemeporte's journal

Es wird weiter Aufgeräumt und Umbenannt bis die kryptischen Bezeichner der Vergangenheit angehören.
Wie bei Gen wird anschließend geforkt und der aktuell noch leblos wirkenden Kartoffel schick_gcc Leben eingehaucht.

[siebenstreich]

Ja, Siebenstreich ist jetzt ein BrightEyes-Developer. 

Äh, das fühlt sich jetzt fast nach einem förmlichen Ritterschlag an. Also schon gut.  Aber auch etwas überraschend: Meinst du damit deine gestrige Einweisung, und den daraus resultierenden pull request von ein paar Code-Bereinigungen?

Recht interessant und witzig war es auch, sich mit Henne über technische Details im Code der Schicksalsklinge auszutauschen, über diverse Dubiositäten bis hin zu unseren Lieblings-Bugs. Schick-eigener Zufallsgenerator, das Rundensystem im Kampf, Markieren des "Auserwählten" des Einhorns, Ablauf der Zauberprobe beim "Analüs", Taschenspielertricks in der Schule der Hellsicht in Thorwal, um nur ein paar Beispiele zu nennen.

[HenneNWH]

Jeder fängt Mal klein an. Meine ersten Commits zum Linux-Kernel waren auch eher Kosmetisch.
Dort gibt es auch Horror-Bereiche, an die sich keiner rantraut.
Zu der Zeit als du dich in Bright-Eyes eingearbeitet hast, war das auch eher gruselig.
Es sind noch 4 Funktionen ordentlich zu benennen, dann schau ich noch Mal grob drüber und dann wird geforkt.