Restored functionality to -L flag, generate symbol table.

[Iambian]

rawr.

[jacobly0]

This segfaults when compiling any non-erroring program with -L, probably the same as in #23.

[alberthdev]

Stack trace I'm seeing on my side:

(gdb) bt
#0  strlen () at ../sysdeps/x86_64/strlen.S:106
#1  0x00007ffff6b0737e in __GI___strdup (s=0x0) at strdup.c:41
#2  0x000000010001e236 in add_arg (name=0x1002536f0 "__R", 
    value=0x100253670 "$0", arg_set=0x100253650) at storage.cpp:649
#3  0x0000000100013160 in parse_args (ptr=0x100252746 " 255, 0)", 
    define=0x1002527a0, curr_arg_set=0x7fffffffdf20) at utils.cpp:298
#4  0x000000010001c3c7 in parse_single_num (expr=0x100252743 "(0, 255, 0)", 
    value=0x7fffffffdff4) at parser.cpp:387
#5  0x000000010001c991 in parse_num_full (expr=0x100252740 "RGB(0, 255, 0)", 
    value=0x7fffffffe064, depth=0) at parser.cpp:543
#6  0x000000010001ba70 in parse_num (expr=0x100252740 "RGB(0, 255, 0)", 
    value=0x7fffffffe064) at parser.cpp:120
#7  0x000000010001d17f in write_defines_callback (data=0x100252650, 
    data_list=0x7fffffffe0f0) at storage.cpp:118
#8  0x0000000100019d22 in hash_enum (ht=0x100251c60, 
    enum_callback=0x10001d128 <write_defines_callback(void*, void*)>, 
    arg=0x7fffffffe0f0) at hash.cpp:64
#9  0x000000010001cfe4 in write_labels (filename=0x100251c40 "labels.lab")
    at storage.cpp:66
#10 0x000000010000f788 in run_assembly () at main.cpp:188
#11 0x00000001000100b0 in main (argc=3, argv=0x7fffffffe428) at main.cpp:451
(gdb) 

Using test file labels.asm, command line ./spasm -L labels.asm:

label1:
    .db 0
label2:
    .db 0
label3:
    .db 0
label4:
    .db 0

write_labels is still crashing, but differently this time... continuing to look into this.

[coveralls]

Coverage decreased (-0.01%) to 46.042% when pulling 9cdbabc on Iambian:master into ad6657d on alberthdev:master.

[alberthdev]

Valgrind isn't happy either (same args/test file):

(stretch)albert@dummy:~/cherry-spasm-ng/tests$ valgrind ../spasm -L labels.asm
==21767== Memcheck, a memory error detector
==21767== Copyright (C) 2002-2015, and GNU GPL'd, by Julian Seward et al.
==21767== Using Valgrind-3.12.0.SVN and LibVEX; rerun with -h for copyright info
==21767== Command: ../spasm -L labels.asm
==21767== 
Pass one... 
Pass two... 
Outputting symbol table
==21767== Invalid read of size 1
==21767==    at 0x4C2EDA2: strlen (in /usr/lib/valgrind/vgpreload_memcheck-amd64-linux.so)
==21767==    by 0x5E6B37D: strdup (strdup.c:41)
==21767==    by 0x12622F: add_arg(char*, char*, _list*) (storage.cpp:649)
==21767==    by 0x11B15C: parse_args(char const*, define*, _list**) (utils.cpp:298)
==21767==    by 0x1243C0: parse_single_num(char const*, int*) (parser.cpp:387)
==21767==    by 0x12498A: parse_num_full(char const*, int*, int) (parser.cpp:543)
==21767==    by 0x123A69: parse_num(char const*, int*) (parser.cpp:120)
==21767==    by 0x125178: write_defines_callback(void*, void*) (storage.cpp:118)
==21767==    by 0x121D1B: hash_enum(hash_t*, void (*)(void*, void*), void*) (hash.cpp:64)
==21767==    by 0x124FDD: write_labels(char*) (storage.cpp:66)
==21767==    by 0x117787: run_assembly() (main.cpp:188)
==21767==    by 0x1180AF: main (main.cpp:451)
==21767==  Address 0x0 is not stack'd, malloc'd or (recently) free'd
==21767== 
==21767== 
==21767== Process terminating with default action of signal 11 (SIGSEGV)
==21767==  Access not within mapped region at address 0x0
==21767==    at 0x4C2EDA2: strlen (in /usr/lib/valgrind/vgpreload_memcheck-amd64-linux.so)
==21767==    by 0x5E6B37D: strdup (strdup.c:41)
==21767==    by 0x12622F: add_arg(char*, char*, _list*) (storage.cpp:649)
==21767==    by 0x11B15C: parse_args(char const*, define*, _list**) (utils.cpp:298)
==21767==    by 0x1243C0: parse_single_num(char const*, int*) (parser.cpp:387)
==21767==    by 0x12498A: parse_num_full(char const*, int*, int) (parser.cpp:543)
==21767==    by 0x123A69: parse_num(char const*, int*) (parser.cpp:120)
==21767==    by 0x125178: write_defines_callback(void*, void*) (storage.cpp:118)
==21767==    by 0x121D1B: hash_enum(hash_t*, void (*)(void*, void*), void*) (hash.cpp:64)
==21767==    by 0x124FDD: write_labels(char*) (storage.cpp:66)
==21767==    by 0x117787: run_assembly() (main.cpp:188)
==21767==    by 0x1180AF: main (main.cpp:451)
==21767==  If you believe this happened as a result of a stack
==21767==  overflow in your program's main thread (unlikely but
==21767==  possible), you can try to increase the size of the
==21767==  main thread stack using the --main-stacksize= flag.
==21767==  The main thread stack size used in this run was 8388608.
==21767== 
==21767== HEAP SUMMARY:
==21767==     in use at exit: 8,004,155 bytes in 99 blocks
==21767==   total heap usage: 161 allocs, 62 frees, 8,087,680 bytes allocated
==21767== 
==21767== LEAK SUMMARY:
==21767==    definitely lost: 0 bytes in 0 blocks
==21767==    indirectly lost: 0 bytes in 0 blocks
==21767==      possibly lost: 0 bytes in 0 blocks
==21767==    still reachable: 8,004,155 bytes in 99 blocks
==21767==         suppressed: 0 bytes in 0 blocks
==21767== Rerun with --leak-check=full to see details of leaked memory
==21767== 
==21767== For counts of detected and suppressed errors, rerun with: -v
==21767== ERROR SUMMARY: 1 errors from 1 contexts (suppressed: 0 from 0)
Segmentation fault (core dumped)

[alberthdev]

Seems to crash when it hits this weird label:

(gdb) print define->name
$48 = 0x100252630 "__BM_MSK_RGB"
(gdb) print define->line_num
$49 = 0
(gdb) print define->input_file
$50 = 0x100252610 "labels.asm"
(gdb) print define->contents
$51 = 0x100252740 "RGB(0, 255, 0)"
(gdb) print define->num_args
$52 = 0
(gdb) print define->args
$53 = {0x0 <repeats 16 times>}

On that note - is this the "correct" label file expected? Or are there too many entries?

__BM_MSK_RGB = $FF00
__BM_SHD = $0002
__LINE = $0000
FALSE = $0000
LABEL1 = $0000
LABEL2 = $0001
LABEL3 = $0002
LABEL4 = $0003
SPASM = $0001
SPASMVER = $0002
TRUE = $0001

[alberthdev]

So this is indeed the same bug, but for some exciting reason:

• MSVC treats strdup(NULL) as a scolding rather than a flogging (see here for test case), so no errors on MSVC side
• curr_input_file is freed at the end of pass1, but not freed at the end of pass2, allowing for reuse. (Guessing that this is due to the fact that pass2 is pretty much the end.)

I think I have an idea for how to fix this (and actually fix this, rather than depending on pass2)... but there are options to do this:

• Add checks to parse_args if there's any issues or not, and if so, allocate a curr_input_file to define->input_file, run add_arg, then free it for next run. (Before utils.cpp:290, and then after utils.cpp:300.) Extremely local scope, lots of malloc() + free() here.
• Same as above, but for parse_single_num - before parser.cpp:387, and after parser.cpp:387. Again, really local here!
• Same as above, but for a broader write_defines_callback. Somewhat local, but has a much broader scope and might be easier to manage. Before storage.cpp:118, after storage.cpp:118 (ish).
• Set it up in write_labels. This is probably the most broad scope, and potentially the safest. A bit more difficult to implement since there's no define_t object, but that might not be as bad as I think... Before storage.cpp:58, after storage.cpp:78.
• All of them (maybe except for write_labels) would check to see if the curr_input_file var is defined or not, and set a flag accordingly if it wasn't so that we can free our malloc at the end. May not be needed given the current position of the write_labels call since we expect pass1 to free this.

Any thoughts?