Address review feedback

README.md

@@ -246,7 +246,7 @@ The "runtime" will be a replacement for all of the C/C++ code of the original ga
   - [x] GOAL Linker
   - [ ] PS2-specific hardware initialization as required by Sony libraries
   - [x] GOAL "kheap" allocator
-  - [ ] Memory card interfaces
+  - [ ] Memory card interface
   - [x] GOAL printf (called `format`) implementation
   - [x] GOAL hash/symbol table implementation
   - [x] Implementation of some built-in GOAL methods/functions
@@ -259,7 +259,7 @@ The "runtime" will be a replacement for all of the C/C++ code of the original ga
   - [ ] Ramdisk stuff (implemented but totally untested)
 - The "989_snd" sound driver (no progress has been made here, the rough plan is to do a high level emulation of the sound system)
 - Sony libraries
-  - [x] SIF (interfaces between EE/IOP for sending data, receiving data, and making remote procedure calls)
+  - [x] SIF (interface between EE/IOP for sending data, receiving data, and making remote procedure calls)
   - [x] IOP Kernel (single-processor non-preemptive multitasking)
   - [x] stubs for stuff that doesn't really matter
 

game/kernel/asm_funcs.asm

@@ -57,6 +57,45 @@ _format:
 ;; run this wrapper to call the real format_impl
 
 
+;; The _call_goal_asm function is used to call a GOAL function from C.
+;; It supports up to 3 arguments and a return value.
+;; This should be called with the arguments:
+;; - first goal arg
+;; - second goal arg
+;; - third goal arg
+;; - address of function to call
+;; - address of the symbol table
+;; - GOAL memory space offset
+
+global _call_goal_asm_linux
+
+_call_goal_asm_linux:
+  ;; x86 saved registers we need to modify for GOAL should be saved
+  push r13
+  push r14
+  push r15
+
+  ;; RDI - first arg
+  ;; RSI - second arg
+  ;; RDX - third arg
+  ;; RCX - function pointer (goes in r13)
+  ;; R8  - st (goes in r14)
+  ;; R9  - off (goes in r15)
+
+  ;; set GOAL function pointer
+  mov r13, rcx
+  ;; offset
+  mov r15, r8
+  ;; symbol table
+  mov r14, r9
+  ;; call GOAL by function pointer
+  call r13
+
+  ;; retore x86 registers.
+  pop r15
+  pop r14
+  pop r13
+  ret
 
 
 ;; The _call_goal_asm function is used to call a GOAL function from C.
@@ -69,9 +108,9 @@ _format:
 ;; - address of the symbol table
 ;; - GOAL memory space offset
 
-global _call_goal_asm
+global _call_goal_asm_win32
 
-_call_goal_asm:
+_call_goal_asm_win32:
   push rdx    ; 8
   push rbx    ; 16
   push rbp    ; 24

game/kernel/kprint.h

@@ -62,28 +62,59 @@ void output_unload(const char* name);
  */
 void output_segment_load(const char* name, Ptr<u8> link_block, u32 flags);
 
+#ifdef __linux__
+/*!
+ * Print to the GOAL print buffer from C
+ */
+void cprintf(const char* format, ...) __attribute__((format(printf, 1, 2)));
+#elif _WIN32
 /*!
  * Print to the GOAL print buffer from C
  */
 void cprintf(const char* format, ...);
+#endif
 
+#ifdef __linux__
+/*!
+ * Print directly to the C stdout
+ * The "k" parameter is ignored, so this is just like printf
+ */
+void Msg(s32 k, const char* format, ...) __attribute__((format(printf, 2, 3)));
+#elif _WIN32
 /*!
  * Print directly to the C stdout
  * The "k" parameter is ignored, so this is just like printf
  */
 void Msg(s32 k, const char* format, ...);
+#endif
 
+#ifdef __linux__
+/*!
+ * Print directly to the C stdout
+ * This is identical to Msg.
+ */
+void MsgWarn(const char* format, ...) __attribute__((format(printf, 1, 2)));
+#elif _WIN32
 /*!
  * Print directly to the C stdout
  * This is identical to Msg.
  */
 void MsgWarn(const char* format, ...);
+#endif
 
+#ifdef __linux__
+/*!
+ * Print directly to the C stdout
+ * This is identical to Msg.
+ */
+void MsgErr(const char* format, ...) __attribute__((format(printf, 1, 2)));
+#elif _WIN32
 /*!
  * Print directly to the C stdout
  * This is identical to Msg.
  */
 void MsgErr(const char* format, ...);
+#endif
 
 /*!
  * Reverse string in place.

game/kernel/kscheme.cpp

@@ -320,6 +320,45 @@ u64 make_string_from_c(const char* c_str) {
  * The implementation is to create a simple trampoline function which jumps to the C function.
  */
 Ptr<Function> make_function_from_c(void* func) {
+#ifdef __linux__
+  return make_function_from_c_linux(func);
+#elif _WIN32
+  return make_function_from_c_win32(func);
+#endif
+}
+
+Ptr<Function> make_function_from_c_linux(void* func) {
+  // allocate a function object on the global heap
+  auto mem = Ptr<u8>(
+      alloc_heap_object(s7.offset + FIX_SYM_GLOBAL_HEAP, *(s7 + FIX_SYM_FUNCTION_TYPE), 0x40));
+  auto f = (uint64_t)func;
+  auto fp = (u8*)&f;
+
+  // we will put the function address in RAX with a movabs rax, imm8
+  mem.c()[0] = 0x48;
+  mem.c()[1] = 0xb8;
+  for (int i = 0; i < 8; i++) {
+    mem.c()[2 + i] = fp[i];
+  }
+
+  // jmp rax
+  mem.c()[10] = 0xff;
+  mem.c()[11] = 0xe0;
+
+  // the C function's ret will return to the caller of this trampoline.
+
+  // CacheFlush(mem, 0x34);
+
+  return mem.cast<Function>();
+}
+
+/*!
+ * Create a GOAL function from a C function. This doesn't export it as a global function, it just
+ * creates a function object on the global heap.
+ *
+ * The implementation is to create a simple trampoline function which jumps to the C function.
+ */
+Ptr<Function> make_function_from_c_win32(void* func) {
   // allocate a function object on the global heap
   auto mem = Ptr<u8>(
       alloc_heap_object(s7.offset + FIX_SYM_GLOBAL_HEAP, *(s7 + FIX_SYM_FUNCTION_TYPE), 0x80));
@@ -354,10 +393,6 @@ Ptr<Function> make_function_from_c(void* func) {
     mem.c()[i++] = x;
   }
 
-  // jmp rax
-  //  mem.c()[10] = 0xff;
-  //  mem.c()[11] = 0xe0;
-
   // the C function's ret will return to the caller of this trampoline.
 
   // CacheFlush(mem, 0x34);
@@ -911,8 +946,12 @@ u64 method_set(u32 type_, u32 method_id, u32 method) {
 }
 
 extern "C" {
-// defined in asm_funcs.nasm
-uint64_t _call_goal_asm(u64 a0, u64 a1, u64 a2, void* fptr, void* st_ptr, void* offset);
+// defined in asm_funcs.asm
+#ifdef __linux__
+uint64_t _call_goal_asm_linux(u64 a0, u64 a1, u64 a2, void* fptr, void* st_ptr, void* offset);
+#elif _WIN32
+uint64_t _call_goal_asm_win32(u64 a0, u64 a1, u64 a2, void* fptr, void* st_ptr, void* offset);
+#endif
 }
 
 /*!
@@ -921,7 +960,11 @@ uint64_t _call_goal_asm(u64 a0, u64 a1, u64 a2, void* fptr, void* st_ptr, void*
 u64 call_goal(Ptr<Function> f, u64 a, u64 b, u64 c, u64 st, void* offset) {
   auto st_ptr = (void*)((uint8_t*)(offset) + st);
   void* fptr = f.c();
-  return _call_goal_asm(a, b, c, fptr, st_ptr, offset);
+#ifdef __linux__
+  return _call_goal_asm_linux(a, b, c, fptr, st_ptr, offset);
+#elif _WIN32
+  return _call_goal_asm_win32(a, b, c, fptr, st_ptr, offset);
+#endif
 }
 
 /*!
@@ -1906,4 +1949,4 @@ s64 load_and_link(const char* filename, char* decode_name, kheapinfo* heap, u32
   return (s32)rv.offset;
 }
 
-// todo, read lcock code
+// todo, read lcock code

goalc/goos/Reader.cpp

@@ -66,8 +66,6 @@ void TextStream::seek_past_whitespace_and_comments() {
   }
 }
 
-Reader::~Reader() {}
-
 Reader::Reader() {
   // third-party library used for a fancy line in
   linenoise::SetHistoryMaxLen(400);
@@ -78,18 +76,27 @@ Reader::Reader() {
   add_reader_macro(",", "unquote");
   add_reader_macro(",@", "unquote-splicing");
 
-  uint8_t valid[256] = {
-      0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0,
-      0, 0, 0, 0, 1, 0, 1, 1, 1, 1, 0, 0, 0, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1,
-      1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1,
-      1, 1, 1, 1, 0, 1, 0, 1, 1, 0, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1,
-      1, 1, 1, 1, 1, 1, 1, 0, 0, 0, 1, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0,
-      0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0,
-      0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0,
-      0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0,
-      0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0};
-  for (int i = 0; i < 256; i++) {
-    valid_symbols_chars[i] = valid[i];
+  // setup table of which characters are valid for starting a symbol
+  for (auto& x : valid_symbols_chars) {
+    x = false;
+  }
+
+  for (char x = 'a'; x <= 'z'; x++) {
+    valid_symbols_chars[(int)x] = true;
+  }
+
+  for (char x = 'A'; x <= 'Z'; x++) {
+    valid_symbols_chars[(int)x] = true;
+  }
+
+  for (char x = '0'; x <= '9'; x++) {
+    valid_symbols_chars[(int)x] = true;
+  }
+
+  const char bonus[] = "!$%&*+-/\\.,@^_-;:<>?~=#";
+
+  for (const char* c = bonus; *c; c++) {
+    valid_symbols_chars[(int)*c] = true;
   }
 
   // find the source directory

goalc/goos/Reader.h

@@ -68,7 +68,6 @@ struct Token {
 class Reader {
  public:
   Reader();
-  ~Reader();
   Object read_from_string(const std::string& str);
   Object read_from_stdin(const std::string& prompt_name);
   Object read_from_file(const std::string& filename);

goalc/listener/Listener.cpp

@@ -43,6 +43,10 @@ bool Listener::is_connected() const {
   return m_connected;
 }
 
+/*!
+ * Attempt to connect to the target. If the target isn't running, this should fail quickly.
+ * Returns true if successfully connected.
+ */
 bool Listener::connect_to_target(const std::string& ip, int port) {
   if (m_connected) {
     throw std::runtime_error("attempted a Listener::connect_to_target when already connected!");