Skip to content

Commit

Permalink
(storm) Fix debug_print() handling of large unsigned integers (#166)
Browse files Browse the repository at this point in the history
The problem was that the `decimal_string()` helper method being used
would take a signed integer as a parameter, which made it impossible to
print integers with bit 31 set (since this is the sign bit in
two's complement representation of signed numbers).

Being able to use unit tests for this made the debugging experience
short and sweet, just like it ought to be. Adding VSCode configs for
this, since I noted they were not present in the repo.
  • Loading branch information
perlun authored Aug 26, 2019
1 parent 994ca63 commit 8c4df99
Show file tree
Hide file tree
Showing 8 changed files with 133 additions and 7 deletions.
26 changes: 26 additions & 0 deletions .vscode/c_cpp_properties.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,26 @@
{
"configurations": [
{
"name": "Mac",
"includePath": [
"${workspaceRoot}",
"${workspaceRoot}/libraries",
"${workspaceRoot}/storm/include",
"/usr/local/include",
"."
],
"defines": [
"__i386__"
],
"intelliSenseMode": "clang-x64",
"browse": {
"path": [
"${workspaceRoot}"
],
"limitSymbolsToIncludedHeaders": true,
"databaseFilename": ""
}
}
],
"version": 4
}
50 changes: 50 additions & 0 deletions .vscode/launch.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,50 @@
{
// Use IntelliSense to learn about possible attributes.
// Hover to view descriptions of existing attributes.
// For more information, visit: https://go.microsoft.com/fwlink/?linkid=830387
"version": "0.2.0",
"configurations": [
{
"name": "storm tests (generic)",
"type": "cppdbg",
"request": "launch",
"program": "${workspaceFolder}/storm_tests/generic/generic_tests",
"args": [],
"stopAtEntry": false,
"cwd": "${workspaceFolder}",
"environment": [],
"externalConsole": false,
"MIMode": "gdb",
"setupCommands": [
{
"description": "Enable pretty-printing for gdb",
"text": "-enable-pretty-printing",
"ignoreFailures": true
}
],
"preLaunchTask": "build_tests",
"miDebuggerPath": "/usr/bin/gdb"
},
{
"name": "storm tests (x86)",
"type": "cppdbg",
"request": "launch",
"program": "${workspaceFolder}/storm_tests/x86/x86_tests",
"args": [],
"stopAtEntry": false,
"cwd": "${workspaceFolder}",
"environment": [],
"externalConsole": false,
"MIMode": "gdb",
"setupCommands": [
{
"description": "Enable pretty-printing for gdb",
"text": "-enable-pretty-printing",
"ignoreFailures": true
}
],
"preLaunchTask": "build_tests",
"miDebuggerPath": "/usr/bin/gdb"
}
]
}
37 changes: 37 additions & 0 deletions .vscode/tasks.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,37 @@
{
"tasks": [
{
"type": "shell",
"label": "gcc-7 build active file",
"command": "/usr/bin/gcc-7",
"args": [
"-g",
"${file}",
"-o",
"${fileDirname}/${fileBasenameNoExtension}"
],
"options": {
"cwd": "/usr/bin"
}
},
{
"label": "build_tests",
"type": "shell",
"command": "rake",
"args": [
"default",
"build_storm_tests"
],
"presentation": {
"echo": true,
"reveal": "always",
"focus": false,
"panel": "shared",
"showReuseMessage": true,
"clear": true
},
"problemMatcher":"$gcc"
}
],
"version": "2.0.0"
}
4 changes: 2 additions & 2 deletions storm/generic/debug.c
Original file line number Diff line number Diff line change
Expand Up @@ -343,8 +343,8 @@ void debug_print_simple(const char *string)
// cpu_interrupts_enable ();
}

// Convert a decimal number to a string.
static void decimal_string(char *string, int number)
// Converts a number to a string (decimal representation)
void decimal_string(char *string, unsigned int number)
{
int index = 0;
const char decimal[] = "0123456789";
Expand Down
5 changes: 2 additions & 3 deletions storm/include/storm/x86/debug.h
Original file line number Diff line number Diff line change
@@ -1,9 +1,7 @@
// Abstract: ia32 specific debug functions.
// Author: Per Lundberg <per@chaosdev.io>

// © Copyright 2000 chaos development.
// © Copyright 2007 chaos development.
// © Copyright 2015-2016 chaos development.
// © Copyright 1999 chaos development.

#pragma once

Expand All @@ -13,3 +11,4 @@

extern void debug_dump_descriptor(descriptor_type *desc);
extern void debug_memory_dump(uint32_t *memory, uint32_t length);
extern void decimal_string(char *string, unsigned int number);
4 changes: 3 additions & 1 deletion storm_tests/x86/Rakefile
Original file line number Diff line number Diff line change
Expand Up @@ -15,13 +15,15 @@ LDFLAGS = %w[
-L../../storm/x86
].freeze

# Need to provide the same name twice for linking to succeed. (circular dependency?)
# Need to provide the same name multiple times for linking to succeed. (circular dependencies?)
LIBRARIES = %w[
cmocka

storm_x86
storm_generic
storm_x86
storm_generic
storm_x86
].freeze

task default: [:banner, OUTPUT]
Expand Down
10 changes: 10 additions & 0 deletions storm_tests/x86/string_tests.c
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@
#include "../test_helper.h"

#include <storm/generic/string.h>
#include <storm/current-arch/debug.h>

void string_to_number_decimal(void **state)
{
Expand Down Expand Up @@ -46,3 +47,12 @@ void string_to_number_binary(void **state)

assert_int_equal(i, 42);
}

void decimal_string_high_value(void **state)
{
unsigned int i = 4294967272;
char s[11];

decimal_string(s, i);
assert_string_equal(s, "4294967272");
}
4 changes: 3 additions & 1 deletion storm_tests/x86/x86_tests_main.c
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ extern void string_to_number_negative_decimal(void **state);
extern void string_to_number_invalid_value(void **state);
extern void string_to_number_hexadecimal(void **state);
extern void string_to_number_binary(void **state);
extern void decimal_string_high_value(void **state);

int main(void)
{
Expand All @@ -27,7 +28,8 @@ int main(void)
cmocka_unit_test(string_to_number_negative_decimal),
cmocka_unit_test(string_to_number_invalid_value),
cmocka_unit_test(string_to_number_hexadecimal),
cmocka_unit_test(string_to_number_binary)
cmocka_unit_test(string_to_number_binary),
cmocka_unit_test(decimal_string_high_value)
};

cmocka_run_group_tests_name("memory_global", memory_global_tests, NULL, NULL);
Expand Down

0 comments on commit 8c4df99

Please sign in to comment.