Skip to content

Conversation

@emersion
Copy link
Contributor

This is (hopefully) the last episode of the long list of patches trying to fix
the reversed lists issues. I originally started to try to fix in 1, but I
realized that the fix is incorrect.

My previous patch changed how read_elf{32,64}_section_header_table' returned
its output: instead of putting the newly parsed element at the head of the list,
it appends it at the end. However this is wrong: the function first parses a
header, and then parses the remaining headers recursively. This means the
parsed element must become the head of the returned list.

I then tried to pin down the real issue from where I noticed it, in
elf_memory_image_of_elf64_file. In this function the sections are reversed,
in read_elf{32,64}_section_header_table' the order is correct, the issue is
between those two functions.

I ran into some issues with List.mapi: the standard OCaml version calls the
iterator in the normal order meaning side effects (such as printing a debug
line) are correctly triggered. However the Lem version calls the iterator in the
reverse order, which is confusing. I'll definitely investigate this later.

The problem lies in obtain_elf64_interpreted_sections: the section header
list it eats is in the correct order, but the list of sections it returns is
reversed. After some more debugging I realized Error.mapM reverses the lists
it gets called with.

This patch fixes this issue and removes the previous workarounds.

This is (hopefully) the last episode of the long list of patches trying to fix
the reversed lists issues. I originally started to try to fix in [1], but I
realized that the fix is incorrect.

My previous patch changed how `read_elf{32,64}_section_header_table'` returned
its output: instead of putting the newly parsed element at the head of the list,
it appends it at the end. However this is wrong: the function first parses a
header, and then parses the remaining headers recursively. This means the
parsed element must become the head of the returned list.

I then tried to pin down the real issue from where I noticed it, in
`elf_memory_image_of_elf64_file`. In this function the sections are reversed,
in `read_elf{32,64}_section_header_table'` the order is correct, the issue is
between those two functions.

I ran into some issues with `List.mapi`: the standard OCaml version calls the
iterator in the normal order meaning side effects (such as printing a debug
line) are correctly triggered. However the Lem version calls the iterator in the
reverse order, which is confusing. I'll definitely investigate this later.

The problem lies in `obtain_elf64_interpreted_sections`: the section header
list it eats is in the correct order, but the list of sections it returns is
reversed. After some more debugging I realized `Error.mapM` reverses the lists
it gets called with.

This patch fixes this issue and removes the previous workarounds.

[1]: #1
@stephenrkell stephenrkell merged commit 2826747 into rems-project:master Apr 24, 2018
@emersion emersion deleted the fix-error-mapm branch April 24, 2018 13:22
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