Fix wrong size calculation for "Dx ?" larger than DB #19
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
The size calculation done in len_extops() (called by insn_size()) for
EOT_DB_RESERVE (i.e. uninitialized storage "?" token) does not take
into account the element size (e->elem), thus calculating a wrong
size for any Dx larger than DB (DW, DQ, etc).
The bug is silent, but it makes NASM error out if a "Dx ?" (larger
than DB) is followed by any label because the label offset gets
mismatched in the final code generation stage:
$ cat test.asm
[section .bss]
DW ?
x:
$ nasm test.asm
test.asm:3: error: label `x' changed during code generation [-w+error=label-redef-late]
See also: https://stackoverflow.com/q/70012188/3889449
Signed-off-by: Marco Bonelli <marco@mebeim.net>
|
Looks reasonable, thanks! A couple of days to review. |
|
merged into master as d167b3d |
|
Hi @cyrillos I see you also implemented a simple test that is supposed to run on Travis CI for this. However you have this repo registered under travis-ci.org, and their website reads:
In fact, the last Travis build from this project was around 6 months ago. You should probably swap to the .com domain if you want your tests to actually run. Just wanted to let you know in case you weren't aware. |
|
Thanks for pointing! Will take a look once time permit |
sorokin
added a commit
to sorokin/nasm
that referenced
this pull request
Feb 26, 2023
When running with -fsanitize=leak enabled nasm prints this error:
==19965==ERROR: LeakSanitizer: detected memory leaks
Direct leak of 360 byte(s) in 90 object(s) allocated from:
#0 0x7faee9396867 in __interceptor_malloc ../../../../src/libsanitizer/asan/asan_malloc_linux.cpp:145
netwide-assembler#1 0x5645d39a401c in nasm_malloc nasmlib/alloc.c:55
netwide-assembler#2 0x5645d3a41f9d in string_transform asm/strfunc.c:356
netwide-assembler#3 0x5645d3a37d40 in eval_strfunc asm/eval.c:761
netwide-assembler#4 0x5645d3a37d40 in expr6 asm/eval.c:906
netwide-assembler#5 0x5645d3a3968d in expr5 asm/eval.c:627
netwide-assembler#6 0x5645d3a39aca in expr4 asm/eval.c:602
netwide-assembler#7 0x5645d3a39b72 in expr3 asm/eval.c:563
netwide-assembler#8 0x5645d3a39db8 in expr2 asm/eval.c:537
netwide-assembler#9 0x5645d3a39f38 in expr1 asm/eval.c:511
netwide-assembler#10 0x5645d3a3a0b8 in expr0 asm/eval.c:485
netwide-assembler#11 0x5645d3a3a242 in rexp3 asm/eval.c:422
netwide-assembler#12 0x5645d3a3a508 in rexp2 asm/eval.c:396
netwide-assembler#13 0x5645d3a3a6a8 in rexp1 asm/eval.c:369
netwide-assembler#14 0x5645d3a3a838 in rexp0 asm/eval.c:342
netwide-assembler#15 0x5645d3a3a838 in cexpr asm/eval.c:305
netwide-assembler#16 0x5645d3a3ad08 in bexpr asm/eval.c:298
netwide-assembler#17 0x5645d3a3ad08 in evaluate asm/eval.c:1032
netwide-assembler#18 0x5645d39e4f20 in parse_line asm/parser.c:959
netwide-assembler#19 0x5645d399e243 in assemble_file asm/nasm.c:1735
netwide-assembler#20 0x5645d3998801 in main asm/nasm.c:719
netwide-assembler#21 0x7faee8aaed8f in __libc_start_call_main ../sysdeps/nptl/libc_start_call_main.h:58
netwide-assembler#22 0x7faee8aaee3f in __libc_start_main_impl ../csu/libc-start.c:392
netwide-assembler#23 0x5645d399acd4 in _start (/home/ivan/d/nasm/nasm+0x2e5cd4)
SUMMARY: AddressSanitizer: 360 byte(s) leaked in 90 allocation(s).
This problem is reproducible on test utf.asm.
The problem was caused by the fact that eval_strfunc doesn't free the string
allocated by string_transform.
sorokin
added a commit
to sorokin/nasm
that referenced
this pull request
Mar 5, 2023
When running with -fsanitize=leak enabled nasm prints this error:
==19965==ERROR: LeakSanitizer: detected memory leaks
Direct leak of 360 byte(s) in 90 object(s) allocated from:
#0 0x7faee9396867 in __interceptor_malloc ../../../../src/libsanitizer/asan/asan_malloc_linux.cpp:145
netwide-assembler#1 0x5645d39a401c in nasm_malloc nasmlib/alloc.c:55
netwide-assembler#2 0x5645d3a41f9d in string_transform asm/strfunc.c:356
netwide-assembler#3 0x5645d3a37d40 in eval_strfunc asm/eval.c:761
netwide-assembler#4 0x5645d3a37d40 in expr6 asm/eval.c:906
netwide-assembler#5 0x5645d3a3968d in expr5 asm/eval.c:627
netwide-assembler#6 0x5645d3a39aca in expr4 asm/eval.c:602
netwide-assembler#7 0x5645d3a39b72 in expr3 asm/eval.c:563
netwide-assembler#8 0x5645d3a39db8 in expr2 asm/eval.c:537
netwide-assembler#9 0x5645d3a39f38 in expr1 asm/eval.c:511
netwide-assembler#10 0x5645d3a3a0b8 in expr0 asm/eval.c:485
netwide-assembler#11 0x5645d3a3a242 in rexp3 asm/eval.c:422
netwide-assembler#12 0x5645d3a3a508 in rexp2 asm/eval.c:396
netwide-assembler#13 0x5645d3a3a6a8 in rexp1 asm/eval.c:369
netwide-assembler#14 0x5645d3a3a838 in rexp0 asm/eval.c:342
netwide-assembler#15 0x5645d3a3a838 in cexpr asm/eval.c:305
netwide-assembler#16 0x5645d3a3ad08 in bexpr asm/eval.c:298
netwide-assembler#17 0x5645d3a3ad08 in evaluate asm/eval.c:1032
netwide-assembler#18 0x5645d39e4f20 in parse_line asm/parser.c:959
netwide-assembler#19 0x5645d399e243 in assemble_file asm/nasm.c:1735
netwide-assembler#20 0x5645d3998801 in main asm/nasm.c:719
netwide-assembler#21 0x7faee8aaed8f in __libc_start_call_main ../sysdeps/nptl/libc_start_call_main.h:58
netwide-assembler#22 0x7faee8aaee3f in __libc_start_main_impl ../csu/libc-start.c:392
netwide-assembler#23 0x5645d399acd4 in _start (/home/ivan/d/nasm/nasm+0x2e5cd4)
SUMMARY: AddressSanitizer: 360 byte(s) leaked in 90 allocation(s).
This problem is reproducible on test utf.asm.
The problem was caused by the fact that eval_strfunc doesn't free the string
allocated by string_transform.
Signed-off-by: Ivan Sorokin <vanyacpp@gmail.com>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
The size calculation done in
len_extops()(called byinsn_size()) forEOT_DB_RESERVE(i.e. uninitialized storage?token) does not take into account the element size (e->elem), thus calculating a wrong size for any Dx larger than DB (DW, DQ, etc).The bug is silent, but it makes NASM error out if a "Dx ?" (larger than DB) is followed by any label because the label offset gets mismatched in the final code generation stage:
See also this StackOverflow question complaining about the problem.