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

workaround for IntervalMap empty intersections #317

Closed
wants to merge 2 commits into from

Conversation

danmatichuk
Copy link
Collaborator

@danmatichuk danmatichuk commented Feb 15, 2023

The function IntervalMap.intersecting is incorrectly returning nonempty results when given an empty interval. This works around the issue by explicitly adding an isEmpty check.

This resolves an issue where ElfLoader would error, saying "Found section header that overlaps with program header.", if presented with a segment that has zero file size.

The function IntervalMap.intersecting is incorrectly returning
nonempty results when given an empty interval. This works
around the issue by explicitly adding an isEmpty check.
@RyanGlScott
Copy link
Contributor

The function IntervalMap.intersecting is incorrectly returning nonempty results when given an empty interval.

Huh. Can you provide an example of some IntervalMap code that returns incorrect results? It sounds like we ought to file a bug report against IntervalMap, assuming that someone else hasn't already done so before.

@danmatichuk
Copy link
Collaborator Author

I've posted an issue to the github project for IntervalMap here bokesan/IntervalMap#9, which contains an example.

@RyanGlScott
Copy link
Contributor

RyanGlScott commented Feb 15, 2023

Thanks! I think it would be worth leaving a comment next to the code that explains why we bother to include this unusual special case, citing the issue alongside it.

Also, I did some limited testing, and this issues appears to fix #302! Can you add that as a test case? Alternatively, perhaps you have a test case of your own?

@RyanGlScott
Copy link
Contributor

Now that IntervalMap-0.6.2.0 is on Hackage with a fix for bokesan/IntervalMap#9, perhaps we should just bump the lower version bounds on IntervalMap to >= 1.6.2. That way, we can drop the workaround entirely.

@danmatichuk
Copy link
Collaborator Author

Agreed. I'll roll back this change in favor of jump bumping the revision and then add your test to the repo.

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