Avoid heap-allocating memory for calculating required buffer size on …

…`RabbitizerOperandType_getBufferSize`
Decompollaborate · Apr 3, 2024 · f5d811f · f5d811f
1 parent 5f4ff4d
commit f5d811f
Show file tree

Hide file tree

Showing 11 changed files with 46 additions and 17 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -10,6 +10,10 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 ### Changed
 
 - Consider r5900's `paddub` as a possible move instruction.
+- Internal rework to avoid allocating memory when calculating required buffer
+  size for disassembly.
+  - This is part of the `RabbitizerInstruction_getSizeForBuffer` function.
+  - This change may help recent Windows specific issues.
 
 ## [1.9.4] - 2024-03-18
 

diff --git a/Cargo.toml b/Cargo.toml
@@ -4,7 +4,7 @@
 [package]
 name = "rabbitizer"
 # Version should be synced with include/common/RabbitizerVersion.h
-version = "1.9.4"
+version = "1.9.5"
 edition = "2021"
 authors = ["Anghelo Carvajal <[email protected]>"]
 description = "MIPS instruction decoder"

diff --git a/Makefile b/Makefile
@@ -22,6 +22,7 @@ WARNINGS        += -Wformat=2 -Wundef
 WARNINGS        += -Werror=vla -Werror=switch -Werror=implicit-fallthrough -Werror=unused-function
 WARNINGS        += -Werror=unused-parameter -Werror=shadow -Werror=switch -Werror=double-promotion
 WARNINGS_C      := -Werror=implicit-function-declaration -Werror=incompatible-pointer-types
+WARNINGS_C      += -Wno-nonnull-compare
 WARNINGS        += -Werror=type-limits
 WARNINGS_CXX    :=
 

diff --git a/include/common/RabbitizerVersion.h b/include/common/RabbitizerVersion.h
@@ -14,7 +14,7 @@ extern "C" {
 // Header version
 #define RAB_VERSION_MAJOR 1
 #define RAB_VERSION_MINOR 9
-#define RAB_VERSION_PATCH 4
+#define RAB_VERSION_PATCH 5
 
 #define RAB_VERSION_STR RAB_STRINGIFY(RAB_VERSION_MAJOR) "." RAB_STRINGIFY(RAB_VERSION_MINOR) "." RAB_STRINGIFY(RAB_VERSION_PATCH)
 

diff --git a/include/common/Utils.h b/include/common/Utils.h
@@ -84,27 +84,38 @@ typedef enum RabTrinaryValue {
 #define RABUTILS_BUFFER_ADVANCE(buffer, totalSize, expression) \
     do {                                                       \
         size_t __tempSize = (size_t)(expression);              \
-        (buffer) += __tempSize;                                \
+        if (buffer != NULL) {                                  \
+            (buffer) += __tempSize;                            \
+        }                                                      \
         (totalSize) += __tempSize;                             \
     } while (0)
 
 #define RABUTILS_BUFFER_WRITE_CHAR(buffer, totalSize, character) \
     do {                                                         \
-        *(buffer) = (character);                                 \
+        if (buffer != NULL) {                                    \
+            *(buffer) = (character);                             \
+        }                                                        \
         RABUTILS_BUFFER_ADVANCE(buffer, totalSize, 1);           \
     } while (0)
 
 #define RABUTILS_BUFFER_SPRINTF(buffer, totalSize, format, ...) \
     do {                                                        \
-        int _len = sprintf(buffer, format, __VA_ARGS__);        \
+        int _len;                                               \
+        if (buffer != NULL) {                                   \
+            _len = sprintf(buffer, format, __VA_ARGS__);        \
+        } else {                                                \
+            _len = snprintf(NULL, 0, format, __VA_ARGS__);      \
+        }                                                       \
         assert(_len > 0);                                       \
         RABUTILS_BUFFER_ADVANCE(buffer, totalSize, _len);       \
     } while (0)
 
 #define RABUTILS_BUFFER_CPY(buffer, totalSize, string)         \
     do {                                                       \
         size_t _tempSize = strlen(string);                     \
-        memcpy(buffer, string, _tempSize);                     \
+        if (buffer != NULL) {                                  \
+            memcpy(buffer, string, _tempSize);                 \
+        }                                                      \
         RABUTILS_BUFFER_ADVANCE(buffer, totalSize, _tempSize); \
     } while (0)
 

diff --git a/pyproject.toml b/pyproject.toml
@@ -4,7 +4,7 @@
 [project]
 name = "rabbitizer"
 # Version should be synced with include/common/RabbitizerVersion.h
-version = "1.9.4"
+version = "1.9.5"
 description = "MIPS instruction decoder"
 # license = "MIT"
 readme = "README.md"

diff --git a/setup.py b/setup.py
@@ -17,6 +17,7 @@
     extraCompileArgs += ["-Werror=vla", "-Werror=switch", "-Werror=implicit-fallthrough", "-Werror=unused-function", "-Werror=unused-parameter", "-Werror=shadow", "-Werror=switch"]
     extraCompileArgs += ["-Werror=implicit-function-declaration", "-Werror=incompatible-pointer-types"]
     extraCompileArgs += ["-Werror"]
+    extraCompileArgs += ["-Wno-nonnull-compare"]
 
 setup(
     ext_modules=[

diff --git a/src/instructions/RabbitizerInstruction/RabbitizerInstruction_Operand.c b/src/instructions/RabbitizerInstruction/RabbitizerInstruction_Operand.c
@@ -10,23 +10,15 @@
 
 size_t RabbitizerOperandType_getBufferSize(RabbitizerOperandType operand, const RabbitizerInstruction *instr,
                                            size_t immOverrideLength) {
-    char *auxBuffer = calloc(immOverrideLength * 2 + 2, sizeof(char));
-    char *immOverride = calloc(immOverrideLength + 2, sizeof(char));
     OperandCallback callback;
-    size_t size;
 
     assert(operand > RAB_OPERAND_ALL_INVALID);
     assert(operand < RAB_OPERAND_ALL_MAX);
 
     callback = instrOpercandCallbacks[operand];
     assert(callback != NULL);
 
-    size = callback(instr, auxBuffer, immOverride, immOverrideLength);
-
-    free(auxBuffer);
-    free(immOverride);
-
-    return size;
+    return callback(instr, NULL, NULL, immOverrideLength);
 }
 
 size_t RabbitizerInstruction_getSizeForBufferOperandsDisasm(const RabbitizerInstruction *self,

diff --git a/src/instructions/RabbitizerInstructionCpu/RabbitizerInstructionCpu_OperandType.c b/src/instructions/RabbitizerInstructionCpu/RabbitizerInstructionCpu_OperandType.c
@@ -195,6 +195,10 @@ size_t RabbitizerOperandType_process_cpu_label(const RabbitizerInstruction *self
                                                size_t immOverrideLength) {
     size_t totalSize = 0;
 
+    if ((dst == NULL) && (immOverrideLength > 0)) {
+        return immOverrideLength;
+    }
+
     if ((immOverride != NULL) && (immOverrideLength > 0)) {
         memcpy(dst, immOverride, immOverrideLength);
         return immOverrideLength;
@@ -209,6 +213,10 @@ size_t RabbitizerOperandType_process_cpu_immediate(const RabbitizerInstruction *
     size_t totalSize = 0;
     int32_t number;
 
+    if ((dst == NULL) && (immOverrideLength > 0)) {
+        return immOverrideLength;
+    }
+
     if ((immOverride != NULL) && (immOverrideLength > 0)) {
         memcpy(dst, immOverride, immOverrideLength);
         return immOverrideLength;
@@ -241,6 +249,10 @@ size_t RabbitizerOperandType_process_cpu_branch_target_label(const RabbitizerIns
                                                              const char *immOverride, size_t immOverrideLength) {
     size_t totalSize = 0;
 
+    if ((dst == NULL) && (immOverrideLength > 0)) {
+        return immOverrideLength;
+    }
+
     if ((immOverride != NULL) && (immOverrideLength > 0)) {
         memcpy(dst, immOverride, immOverrideLength);
         return immOverrideLength;

diff --git a/src/instructions/RabbitizerInstructionR5900/RabbitizerInstructionR5900_OperandType.c b/src/instructions/RabbitizerInstructionR5900/RabbitizerInstructionR5900_OperandType.c
@@ -492,6 +492,10 @@ size_t RabbitizerOperandType_process_r5900_immediate5(const RabbitizerInstructio
     size_t totalSize = 0;
     int32_t number;
 
+    if ((dst == NULL) && (immOverrideLength > 0)) {
+        return immOverrideLength;
+    }
+
     if ((immOverride != NULL) && (immOverrideLength > 0)) {
         memcpy(dst, immOverride, immOverrideLength);
         return immOverrideLength;
@@ -525,6 +529,10 @@ size_t RabbitizerOperandType_process_r5900_immediate15(const RabbitizerInstructi
     size_t totalSize = 0;
     int32_t number;
 
+    if ((dst == NULL) && (immOverrideLength > 0)) {
+        return immOverrideLength;
+    }
+
     if ((immOverride != NULL) && (immOverrideLength > 0)) {
         memcpy(dst, immOverride, immOverrideLength);
         return immOverrideLength;

diff --git a/tests/c/build_info_checks/version_number.c b/tests/c/build_info_checks/version_number.c
@@ -115,7 +115,7 @@ int main() {
             continue;
         }
 
-        fread(buffer, sizeof(char), fileSize, file);
+        assert(fread(buffer, sizeof(char), fileSize, file) == (size_t)fileSize);
 
         errorCount += doVersionCheck(path, buffer);