search

angr / vex

A patched version of VEX to work with PyVEX.
GNU General Public License v2.0
104 stars 44 forks source link

s390x: update to upstream revision 7e9113cb7 #25

Closed mephi42 closed 4 years ago

mephi42 commented 5 years ago

The old endianness patch has finally went upstream, and so I can now import all the sweet modern s390x insns. There are a bunch of pre-req changes in the common VEX code that need to be picked as well, but as far as I can tell they don't negatively affect other architectures.

In order for the whole thing to work, archinfo and pyvex need to be adapted. I will post the corresponding pull requests shortly.

zardus commented 5 years ago

In the long term, rather than cherry picking commits, what about rebasing on the latest valgrind? Probably out of scope for this PR, but it's something that we should tackle in the near future...

On Sun, Mar 17, 2019 at 8:18 AM mephi42 notifications@github.com wrote:

The old endianness patch has finally went upstream, and so I can now import all the sweet modern s390x insns. There are a bunch of pre-req changes in the common VEX code that need to be picked as well, but as far as I can tell they don't negatively affect other architectures.

In order for the whole thing to work, archinfo and pyvex need to be adapted. I will post the corresponding pull requests shortly.

You can view, comment on, or merge this pull request online at:

https://github.com/angr/vex/pull/25 Commit Summary

  • Pick common code changes from 94d63e38
  • Pick common code changes from 20976f43
  • Pick common code changes from 1cc1d564
  • Pick common code changes from f1a49eeb
  • Pick common code changes from d44563c4
  • Port common code changes from efa1e5ef
  • Port common code changes from 83cabd32
  • s390x: update to upstream revision 7e9113cb7

File Changes

Patch Links:

— You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub https://github.com/angr/vex/pull/25, or mute the thread https://github.com/notifications/unsubscribe-auth/ADSzl03lFsjSiXK6_j8XixX7ttY1GVr-ks5vXYnagaJpZM4b4Gc2 .

mephi42 commented 5 years ago

I agree that rebasing approach would be nicer, since it would make updates like this one trivial. I have actually tried doing it when adding the initial s390x support, but ran into considerable amount of merge conflicts. If memory serves me right, one of the bigger sources was msvc support. Maybe it might be worth contributing upstream anyway?

zardus commented 5 years ago

MSVC would be awesome to upstream, but given that the valgrind people mostly view VEX as a valgrind tool, and Windows isn't a target for them, they might not take the patches. Worth a shot, though.

@twizmwazin, this one is a doozie, but could you look into rebasing our current VEX changes on modern valgrind? This might be quite a project, but let's at least keep it in mind.

rhelmot commented 5 years ago

Here's what julian said re: the msvc patch when I submitted them originally back in 2017:

Big, complex patch. Need to come back to it. It would be easier to review and discuss if you could break it into smaller pieces, by feature. Eg, break out the case-range removal since that's uncontroversial and easy to land. (But please .. preserve the hex casing: "case 0xFD .. 0xFF:" -> "case 0xFD: etc" and not "case 0xfd: etc")

Does MSVC really disallow empty structs? Having to put "Int nop;" all over the place kinda sucks.

The problem with rebasing at this point is that the valgrind and vex repos have been merged. There might be some git wizardry we can do to mirror only a subset of a repo, but like, yikes.

The main thing that was asked when we did the first round of upstreaming was that we needed tests, and proof that it won't break valgrind. I do have a test harness for how to add vex-only tests to modern valgrind, so whoever wants to do this should ping me for that. We'll need to port our pyvex tests to C, but that's it.

rhelmot commented 5 years ago

I think I would like to merge this as is now and deal with rebasing later. @mephi42, can you run the angr/pyvex/claripy/cle tests manually? I don't trust our CI system to build vex...

We can deal with the bad git repo situation the same way we did when it was svn: we just copy the base code wholesale and call it an initial commit.

twizmwazin commented 5 years ago

@rhelmot I am looking into rebasing, there are a few changes there. First, it is now in one repo with the rest of valgrind. One of the consequences of this is that the build system is more integrated, and I am unsure to what extent they support building just the vex library.

For a long term solution, dumping the subtree here doesn't make much sense, we will end up back in the same place. Instead, it is probably best to just have a fork of the valgrind repo with the necessary build system changes and patches that can be fairly easily rebased on a regular basis, like after each upstream stable release. Additionally, for the purpose of making our lives easier, it would be in out best interest to regularly see how much can be upstreamed, to keep our patch est minimal.

None of this really interferes with this PR being merged, just something to think about on the side.

rhelmot commented 5 years ago

here is my fork of valgrind with progress on being able to build and test vex standalone.

mephi42 commented 5 years ago

I managed to run the tests as follows (tests/test.sh did not work, because it did not change working directories, and all relative paths in tests were messed up):

angr-dev$ docker build . -t angr
angr-dev$ docker run -it --rm -v "$PWD:$PWD" -w "$PWD" angr bash
# . /usr/local/bin/virtualenvwrapper.sh
# workon angr
# for project in ailment angr angrop archinfo claripy cle pyvex; do echo "$project"; (cd "$project/tests" && NOSE_PROCESSES=$(nproc) NOSE_PROCESS_TIMEOUT=600 NOSE_PROCESS_RESTARTWORKER=1 nosetests) >"$project.out" 2>"$project.err"; done

I ran into the following errors in various tests, all of which appear to be pre-existing:

  File "/home/builder/angr-dev/angr/tests/test_concrete_not_packed_elf32.py", line 2, in <module>
    import avatar2
ModuleNotFoundError: No module named 'avatar2'
  File "/home/builder/angr-dev/angr/angr/analyses/reaching_definitions/engine_ail.py", line 100, in _ail_handle_Store
    l.info('Data to write at address %#x undefined, ins_addr = %#x.', a, self.ins_addr)
  File "/usr/lib/python3.6/logging/__init__.py", line 1308, in info
    self._log(INFO, msg, args, **kwargs)
  File "/usr/lib/python3.6/logging/__init__.py", line 1444, in _log
    self.handle(record)
  File "/usr/lib/python3.6/logging/__init__.py", line 1454, in handle
    self.callHandlers(record)
  File "/usr/lib/python3.6/logging/__init__.py", line 1516, in callHandlers
    hdlr.handle(record)
  File "/usr/lib/python3.6/logging/__init__.py", line 865, in handle
    self.emit(record)
  File "/root/.virtualenvs/angr/lib/python3.6/site-packages/nose/plugins/logcapture.py", line 82, in emit
    self.buffer.append(self.format(record))
  File "/usr/lib/python3.6/logging/__init__.py", line 840, in format
    return fmt.format(record)
  File "/usr/lib/python3.6/logging/__init__.py", line 577, in format
    record.message = record.getMessage()
  File "/usr/lib/python3.6/logging/__init__.py", line 338, in getMessage
    msg = msg % self.args
TypeError: %x format: an integer is required, not SpOffset
  File "/home/builder/angr-dev/claripy/claripy/smtlib_utils.py", line 24, in expect_assignment_tuple
    self.expect('(')
  File "/home/builder/angr-dev/claripy/claripy/smtlib_utils.py", line 18, in expect
    t = self.tokens.consume()
  File "/root/.virtualenvs/angr/lib/python3.6/site-packages/pysmt/smtlib/parser/parser.py", line 192, in consume
    return self.extra_queue.pop(0)
TypeError: pop() takes no arguments (1 given)

Overall stats:

ailment: Ran 5 tests in 3.792s
angr: Ran 446 tests in 1017.093s FAILED (errors=21, failures=1)
angrop: Ran 4 tests in 165.625s
archinfo: Ran 0 tests in 0.862s
claripy: Ran 467 tests in 19.402s FAILED (SKIP=237, errors=16)
cle: Ran 49 tests in 12.059s
pyvex: Ran 51 tests in 32.655s
mephi42 commented 5 years ago

I made another update (on top of existing ones for simplicity), which brings support for several z14 instructions.

mephi42 commented 4 years ago

I've rebased the code and pushed it to https://github.com/angr/vex/pull/26 (didn't want to split this time, so I'll close this PR).

I've also rebased the associated archinfo and pyvex PRs:

https://github.com/angr/archinfo/pull/61 https://github.com/angr/pyvex/pull/186

Could you have a look please?