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

fixing valgrind-mmt #7

Open
wants to merge 2 commits into
base: mmt-3.14
Choose a base branch
from
Open

fixing valgrind-mmt #7

Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
92784b0
mmt: dump mmap regions inside mmt_bsearch
karolherbst Jun 28, 2019
c86266b
mmt: fix mmap tracking if second last entry is deleted
karolherbst Jun 29, 2019
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
23 changes: 23 additions & 0 deletions mmt/mmt_trace.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -306,6 +306,12 @@ static noinline struct mmt_mmap_data *mmt_bsearch(Addr addr)
struct mmt_mmap_data *region;
int tmp;

#ifdef MMT_DEBUG_VERBOSE
mmt_bin_flush();
VG_(printf)("searching entry for: %p\n", (void*)addr);
verify_state();
#endif

if (UNLIKELY(mmt_last_region < 0))
{
add_neg(0, (Addr)-1);
Expand Down Expand Up @@ -497,6 +503,23 @@ void mmt_free_region(struct mmt_mmap_data *m)
(mmt_last_region - idx) * sizeof(struct mmt_mmap_data));
VG_(memset)(&mmt_mmaps[mmt_last_region--], 0, sizeof(struct mmt_mmap_data));

/* if we only have one reagion, delete 0-x negative region */
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

typo

if (mmt_last_region == 0) {
Bool found;
do {
found = False;
for (i = 0; i < neg_regions_number; ++i)
{
struct negative_region *neg = &neg_regions[i];
if (neg->end != (Addr)-1) {
remove_neg_region(i);
found = True;
break;
}
}
} while (found);
}
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This doesn't look right. It seems you are papering over bug somewhere else.

How did the bug manifest? Where is the code that "assumes there is only one negative entry if there is just one positive one"?

Copy link
Author

@karolherbst karolherbst Jun 29, 2019 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

it's kind of mmt_bsearch.. but I think the condition is a bit more complicated:

adding region: <0x4029000, 0x402A000>
NEG vvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvv
NEG <0x0000000005e9b000 0xffffffffffffffff>          2543316
NEG ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
POS vvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvv
POS 00000, id: 00002, start: 0x0000000004029000, end: 0x000000000402a000
POS 00001, id: 00001, start: 0x0000000005d96000, end: 0x0000000005e9b000
POS ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
searching entry for: 0x5078EE8
NEG vvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvv
NEG <0x0000000005e9b000 0xffffffffffffffff>          2543325
NEG ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
POS vvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvv
POS 00000, id: 00002, start: 0x0000000004029000, end: 0x000000000402a000
POS 00001, id: 00001, start: 0x0000000005d96000, end: 0x0000000005e9b000
POS ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
adding negative entry: <0x402A000, 0x5D96000>
NEG vvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvv
NEG <0x0000000005e9b000 0xffffffffffffffff>          2543325
NEG <0x000000000402a000 0x0000000005d96000>                0
NEG ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
POS vvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvv
POS 00000, id: 00002, start: 0x0000000004029000, end: 0x000000000402a000
POS 00001, id: 00001, start: 0x0000000005d96000, end: 0x0000000005e9b000
POS ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
freeing region: <0x4029000, 0x402A000>
NEG vvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvv
NEG <0x0000000005e9b000 0xffffffffffffffff>          2543413
NEG <0x0000000004029000 0x0000000005d96000>               52
NEG ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
POS vvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvv
POS 00000, id: 00001, start: 0x0000000005d96000, end: 0x0000000005e9b000
POS ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
searching entry for: 0x5D9600C
NEG vvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvv
NEG <0x0000000005e9b000 0xffffffffffffffff>          2545163
NEG <0x0000000004029000 0x0000000005d96000>             1150
NEG ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
POS vvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvv
POS 00000, id: 00001, start: 0x0000000005d96000, end: 0x0000000005e9b000
POS ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
searching entry for: 0x4021F14
NEG vvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvv
NEG <0x0000000005e9b000 0xffffffffffffffff>          2577319
NEG <0x0000000004029000 0x0000000005d96000>           364613
NEG ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
POS vvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvv
POS 00000, id: 00001, start: 0x0000000005d96000, end: 0x0000000005e9b000
POS ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
adding negative entry: <0x0, 0x5D96000>
NEG vvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvv
NEG <0x0000000005e9b000 0xffffffffffffffff>          2577319
NEG <0x0000000004029000 0x0000000005d96000>           364613
NEG <0x0000000000000000 0x0000000005d96000>                0
NEG ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
POS vvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvv
POS 00000, id: 00001, start: 0x0000000005d96000, end: 0x0000000005e9b000
POS ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
NEG vvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvv
NEG <0x0000000005e9b000 0xffffffffffffffff>          2577319
NEG <0x0000000004029000 0x0000000005d96000>           364613
NEG <0x0000000000000000 0x0000000005d96000>                0
NEG ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
POS vvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvv
POS 00000, id: 00001, start: 0x0000000005d96000, end: 0x0000000005e9b000
POS ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^

mmaptrace: ../../mmt/mmt_trace.c:167 (__verify_state): Assertion 'neg1->start < neg2->start || neg1->start >= neg2->end' failed.
mmaptrace: <0x4029000, 0x5D96000> <0x0, 0x5D96000>

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

MMT state gets inconsistent at this line: "adding negative entry: <0x0, 0x5D96000>", so the issue indeed comes from mmt_bsearch - it calls add_neg with parameters it has not verifed are not conflicting with existing negative entries. The proposed solution is not correct though.

I'm fixing this right now.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

okay, thanks!


/* if we are releasing last used region, then zero cache */
if (m == last_used_region)
last_used_region = &null_region;
Expand Down