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.

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

Conversation

karolherbst
Copy link

no idea why that is needed now, but code made an assumption which got broken before.

I am not even sure if this fix is a good one, but it makes mmt working again for me... or at least "more" working.

code assumes there is only one negative entry if there is just one positive one
@@ -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

}
}
} 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

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!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants