Reproducer program: ``` mov ax,xmm0[dword 0x1] ``` Valgrind output: ``` $ valgrind ./nasm -felf64 rr/repro.1 ==3137481== Memcheck, a memory error detector ==3137481== Copyright (C) 2002-2017, and GNU GPL'd, by Julian Seward et al. ==3137481== Using Valgrind-3.15.0 and LibVEX; rerun with -h for copyright info ==3137481== Command: ./nasm -felf64 rr/repro.1 ==3137481== ==3137481== Conditional jump or move depends on uninitialised value(s) ==3137481== at 0x48E21C2: __vfprintf_internal (vfprintf-internal.c:1688) ==3137481== by 0x48F7119: __vsnprintf_internal (vsnprintf.c:114) ==3137481== by 0x166284: vsnprintf (stdio2.h:80) ==3137481== by 0x166284: nasm_vaxprintf (asprintf.c:57) ==3137481== by 0x165EEE: nasm_verror (nasm.c:2092) ==3137481== by 0x1685A7: nasm_nonfatal (error.c:75) ==3137481== by 0x16D205: calcsize.isra.0 (assemble.c:1619) ==3137481== by 0x16ED29: insn_size (assemble.c:1178) ==3137481== by 0x165936: process_insn (nasm.c:1598) ==3137481== by 0x165936: assemble_file (nasm.c:1737) ==3137481== by 0x162F38: main (nasm.c:717) ==3137481== ==3137481== Conditional jump or move depends on uninitialised value(s) ==3137481== at 0x48E21C2: __vfprintf_internal (vfprintf-internal.c:1688) ==3137481== by 0x48F7119: __vsnprintf_internal (vsnprintf.c:114) ==3137481== by 0x1662CA: vsnprintf (stdio2.h:80) ==3137481== by 0x1662CA: nasm_vaxprintf (asprintf.c:63) ==3137481== by 0x165EEE: nasm_verror (nasm.c:2092) ==3137481== by 0x1685A7: nasm_nonfatal (error.c:75) ==3137481== by 0x16D205: calcsize.isra.0 (assemble.c:1619) ==3137481== by 0x16ED29: insn_size (assemble.c:1178) ==3137481== by 0x165936: process_insn (nasm.c:1598) ==3137481== by 0x165936: assemble_file (nasm.c:1737) ==3137481== by 0x162F38: main (nasm.c:717) ==3137481== rr/repro.1:1: error: (null) ==3137481== ==3137481== HEAP SUMMARY: ==3137481== in use at exit: 164,452 bytes in 18 blocks ==3137481== total heap usage: 977 allocs, 959 frees, 571,653 bytes allocated ==3137481== ==3137481== LEAK SUMMARY: ==3137481== definitely lost: 4,136 bytes in 2 blocks ==3137481== indirectly lost: 0 bytes in 0 blocks ==3137481== possibly lost: 0 bytes in 0 blocks ==3137481== still reachable: 160,316 bytes in 16 blocks ==3137481== suppressed: 0 bytes in 0 blocks ==3137481== Rerun with --leak-check=full to see details of leaked memory ==3137481== ==3137481== Use --track-origins=yes to see where uninitialised values come from ==3137481== For lists of detected and suppressed errors, rerun with: -s ==3137481== ERROR SUMMARY: 2 errors from 2 contexts (suppressed: 0 from 0) ```
The error here seems to be in this if condition: ``` if (process_ea(opy, &ea_data, bits, rfield, rflags, ins, &errmsg) != eat) { nasm_nonfatal("%s", errmsg); return -1; ``` The function process_ea only populates the errmsg in the case of an error (which would be signalled by returning EA_INVALID). In the program above, it returns an ea_type that is not EA_INVALID, it is EA_XMMVSIB, so the error message ptr is not set. A solution could be to check if the result is EA_INVALID, and then print the error, and print a different error if it is not EAT, not populating errmsg.
This is regression from commit 2469b8b6
Should be fixed in 02641a3c841c9f96cc9814756ca9a74c4ad6e783. Thanks a huge for report!
Thanks for fixing it, Cyrill! I am wondering... Would it be possible to add the reproducer as some kind of test? This would probably be detected by creating a test and running it with address sanitizer.
I added the test from exactly your example. The test is running automatialy every time we push new commits. https://github.com/netwide-assembler/nasm/commit/aa2dcdec09433b81c2352fde7c2fe1f794af657b You can run it manually as [cyrill@grain nasm.git] python ./travis/nasm-t.py run -t ./travis/test/br3392751.json
Thank you a lot for reporting this issue! I must admit I don't really like the current function structure and the fix is rather a "fast patch" and should probably be redesigned.
Amazing! I missed that commit. One more question: does the TravisCI tool build with sanitizer support off the bat? From what I can see in https://github.com/netwide-assembler/nasm/blob/master/.travis.yml, it seems like there is nothing explicitly adding address-sanitizer support. I am not sure if it is detected automatically, but in any case, it seems like that's something we want to for all the tests. In this particular case, even without asan it would be a segmentation fault, depending on the previous (uninitialized) value of the variable.
Yeah, we didn't enabled ASAN there because of too many warnings ;) Maybe one day once every asan warning is fixed we turn it on.