Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Fix an instance of -Wunused-but-set-variable. #12

Closed
wants to merge 1 commit into from
Closed

Fix an instance of -Wunused-but-set-variable. #12

wants to merge 1 commit into from

Conversation

pkasting
Copy link

Bug: chromium:1203071

@cyrillos
Copy link
Member

Thanks a huge for the fix. But I think we've rather a problem in commit 1c21a53 where we dropped use of this variable. @hpax could you please take a look once time permit?

@hpax
Copy link
Member

hpax commented Nov 8, 2022

Not needed in 2.16. Leaving this open until 2.16 or a fix is merged into 2.15.xx

sorokin added a commit to sorokin/nasm that referenced this pull request Feb 26, 2023
@sorokin
fix memory leak in eval_strfunc
4da9a6b 
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
@sorokin
fix memory leak in eval_strfunc
c59bd75 
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>
sorokin added a commit to sorokin/nasm that referenced this pull request Mar 5, 2023
@sorokin
fix memory leak in elf_init
1ff9b00 
When running with -fsanitize=leak enabled nasm prints this error:

Direct leak of 31 byte(s) in 1 object(s) allocated from:
    #0 0x7faadfc56867 in __interceptor_malloc ../../../../src/libsanitizer/asan/asan_malloc_linux.cpp:145
    netwide-assembler#1 0x7faadf3ecaac in __GI___libc_scratch_buffer_dupfree malloc/scratch_buffer_dupfree.c:32
    netwide-assembler#2 0x7faadf395eed in scratch_buffer_dupfree ../include/scratch_buffer.h:147
    netwide-assembler#3 0x7faadf395eed in realpath_stk stdlib/canonicalize.c:424
    netwide-assembler#4 0x7faadf3964f5 in __GI___realpath stdlib/canonicalize.c:446
    netwide-assembler#5 0x7faadfbe6b7c in __interceptor_canonicalize_file_name ../../../../src/libsanitizer/sanitizer_common/sanitizer_common_interceptors.inc:3682
    netwide-assembler#6 0x5643cfb1b058 in nasm_realpath nasmlib/realpath.c:58
    netwide-assembler#7 0x5643cfaaaef5 in elf_init output/outelf.c:547
    netwide-assembler#8 0x5643cfaaade7 in elf32_init output/outelf.c:496
    netwide-assembler#9 0x5643cf9d1454 in main asm/nasm.c:716
    netwide-assembler#10 0x7faadf36ed8f in __libc_start_call_main ../sysdeps/nptl/libc_start_call_main.h:58
    netwide-assembler#11 0x7faadf36ee3f in __libc_start_main_impl ../csu/libc-start.c:392
    netwide-assembler#12 0x5643cf9cce04 in _start (/home/ivan/d/nasm/nasm+0x2e2e04)

Direct leak of 24 byte(s) in 1 object(s) allocated from:
    #0 0x7faadfc56867 in __interceptor_malloc ../../../../src/libsanitizer/asan/asan_malloc_linux.cpp:145
    netwide-assembler#1 0x5643cf9dcbc4 in nasm_malloc nasmlib/alloc.c:55
    netwide-assembler#2 0x5643cf9dced4 in nasm_strndup nasmlib/alloc.c:127
    netwide-assembler#3 0x5643cfb1b4e4 in nasm_dirname nasmlib/path.c:152
    netwide-assembler#4 0x5643cfaaaf4d in elf_init output/outelf.c:550
    netwide-assembler#5 0x5643cfaaade7 in elf32_init output/outelf.c:496
    netwide-assembler#6 0x5643cf9d1454 in main asm/nasm.c:716
    netwide-assembler#7 0x7faadf36ed8f in __libc_start_call_main ../sysdeps/nptl/libc_start_call_main.h:58
    netwide-assembler#8 0x7faadf36ee3f in __libc_start_main_impl ../csu/libc-start.c:392
    netwide-assembler#9 0x5643cf9cce04 in _start (/home/ivan/d/nasm/nasm+0x2e2e04)

This error is reproducible on any test that is run with -felf.

The problem is fixed by calling nasm_free appropriately. I had to
remove const from pointers because I get -Wdiscarded-qualifiers
warning otherwise, because nasm_free takes non-const pointer.

Signed-off-by: Ivan Sorokin <vanyacpp@gmail.com>
@sorokin sorokin mentioned this pull request Mar 5, 2023
Open
@pkasting
Copy link
Author

pkasting commented Jul 2, 2024

Closing since 2.16 is long-since released.

@pkasting pkasting closed this Jul 2, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@pkasting @cyrillos @hpax