Skip to content

Upgrade goblin to 0.4.1#407

Merged
benfred merged 2 commits intobenfred:masterfrom
Jongy:upgrade-goblin
Jun 27, 2021
Merged

Upgrade goblin to 0.4.1#407
benfred merged 2 commits intobenfred:masterfrom
Jongy:upgrade-goblin

Conversation

@Jongy
Copy link
Contributor

@Jongy Jongy commented Jun 21, 2021

Encountered this error:

Error: Failed to parse python binary
Reason: Malformed entity: Section 106 size (4277144) + addr (120) is out of bounds. Overflowed: false

(installed "python", which is Python 2.7.6, on an ubuntu:14.04 Docker).

I started looking in goblin, and found this commit 84062ff966c98d390
("Fix size check for section headers") which appears to fix this problem,
so I just upgraded to the newest goblin version available (although 0.3.x
also solves the issue).

Possibly solves: #405 and #403.

@benfred , I see that remoteprocess depends on goblin 0.3.0, so perhaps I can upgrade remoteprocess before this PR, and then we won't need remoteprocess to use a different version from py-spy?

Encountered this error:

	Error: Failed to parse python binary
	Reason: Malformed entity: Section 106 size (4277144) + addr (120) is out of bounds. Overflowed: false

(installed "python", which is Python 2.7.6, on an ubuntu:14.04 Docker).

I started looking in goblin, and found this commit 84062ff966c98d390
("Fix size check for section headers") which appears to fix this problem,
so I just upgraded to the newest goblin version available (although 0.3.x
also solves the issue).
@benfred
Copy link
Owner

benfred commented Jun 21, 2021

yeah - it'd be great to keep remoteprocess / py-spy on the same version of goblin.

It looks like the FreeBSD version is failing with the new version of goblin.

We might need to keep FreeBSD on an older version of goblin (and file an issue with the goblin letting them know) ?.

Jongy added a commit to Jongy/remoteprocess that referenced this pull request Jun 21, 2021
@Jongy
Copy link
Contributor Author

Jongy commented Jun 21, 2021

Created benfred/remoteprocess#21. I upgraded to 0.4.1, and I'll keep the py-spy freebsd goblin at 0.3.0 while letting remoteprocess use the newer one. Although, it might fail as well, we're not sure where the issue is... I'm downloading a FreeBSD image so I can test it.

Alternatively, should I add

[target.'cfg(target_os="freebsd")'.dependencies]
goblin = "0.3"

in remoteprocess? wdyt.

I'll also open an issue later (first trying the image to see if I can get any quick conclusions)

@Jongy
Copy link
Contributor Author

Jongy commented Jun 21, 2021

I've set freebsd to use 0.3, and pointed remoteprocess to my branch (in fc83783) so we can see if CI passes with it. After that, we can merge the remoteprocess branch, then upgrade it here.

@Jongy
Copy link
Contributor Author

Jongy commented Jun 24, 2021

@benfred I'm not sure what's going on anymore 😅 you can see the many tests I ran here; eventually I gave up and the last commit (d9d999c) actually undoes all of my changes and the CI still fails, although with a different error now (see https://cirrus-ci.com/task/4803482759725056). Also, in the logs we can see that it uses goblin 0.3.0 and we still get failures.

What do you suggest?

Jongy added a commit to intel/gprofiler that referenced this pull request Jun 25, 2021
Includes a yet merged fix from benfred/py-spy#407
@Jongy Jongy mentioned this pull request Jun 25, 2021
3 tasks
@benfred
Copy link
Owner

benfred commented Jun 26, 2021

I'm trying to fix the freebsd build here - #409 . I noticed that rbspy has changed from cirrus to github actions for their freebsd CI, and I'm trying the same here.

@benfred
Copy link
Owner

benfred commented Jun 26, 2021

hmmm - github actions also fails on freebsd https://github.com/benfred/py-spy/pull/409/checks?check_run_id=2918742055 (without any other changes).

running 7 tests
test test_long_sleep ... FAILED
test test_local_vars ... FAILED
test test_busy_loop ... FAILED
test test_thread_names ... FAILED
test test_recursive ... FAILED
test test_negative_linenumber_increment ... FAILED
test test_unicode ... FAILED

failures:

---- test_long_sleep stdout ----
thread 'test_long_sleep' panicked at 'called `Result::unwrap()` on an `Err` value: Malformed("Section 90 size (2098560) + addr (120) is out of bounds. Overflowed: false")

Failed to parse python binary', tests/integration_test.rs:37:65
note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace

---- test_local_vars stdout ----
thread 'test_local_vars' panicked at 'called `Result::unwrap()` on an `Err` value: Malformed("Section 90 size (2098560) + addr (120) is out of bounds. Overflowed: false")

Failed to parse python binary', tests/integration_test.rs:37:65

---- test_busy_loop stdout ----
thread 'test_busy_loop' panicked at 'called `Result::unwrap()` on an `Err` value: Malformed("Section 90 size (2098560) + addr (120) is out of bounds. Overflowed: false")

Failed to parse python binary', tests/integration_test.rs:37:65

---- test_thread_names stdout ----
thread 'test_thread_names' panicked at 'called `Result::unwrap()` on an `Err` value: Malformed("Section 90 size (2098560) + addr (120) is out of bounds. Overflowed: false")

Failed to parse python binary', tests/integration_test.rs:37:65

---- test_recursive stdout ----
thread 'test_recursive' panicked at 'called `Result::unwrap()` on an `Err` value: Malformed("Section 90 size (2098560) + addr (120) is out of bounds. Overflowed: false")

Failed to parse python binary', tests/integration_test.rs:37:65

---- test_negative_linenumber_increment stdout ----
thread 'test_negative_linenumber_increment' panicked at 'called `Result::unwrap()` on an `Err` value: Malformed("Section 90 size (2098560) + addr (120) is out of bounds. Overflowed: false")

Failed to parse python binary', tests/integration_test.rs:37:65

---- test_unicode stdout ----
thread 'test_unicode' panicked at 'called `Result::unwrap()` on an `Err` value: Malformed("Section 90 size (2098560) + addr (120) is out of bounds. Overflowed: false")

Failed to parse python binary', tests/integration_test.rs:37:65


failures:
    test_busy_loop
    test_local_vars
    test_long_sleep
    test_negative_linenumber_increment
    test_recursive
    test_thread_names
    test_unicode

test result: FAILED. 0 passed; 7 failed; 0 ignored; 0 measured; 0 filtered out; finished in 2.98s

@Jongy
Copy link
Contributor Author

Jongy commented Jun 26, 2021

I built py-spy on a FreeBSD 13.0 machine, I get Failed to find python version ... both on master and on this branch's first commit (4becdfe). @benfred , perhaps we want to merge the first commit as is? (since a couple of issues are waiting for it, and it seems like FreeBSD wasn't working anyway in a while 😅 )

@benfred
Copy link
Owner

benfred commented Jun 26, 2021

It seems like the last successful build with freebsd was just over 2 weeks ago: https://cirrus-ci.com/build/4522148225089536 . I'd like to get this working again if possible, since it hasn't been that long really.

There haven't been any code changes here since then, and because we have dependencies locked in Cargo.lock we shouldn't be pulling in any new code in our dependencies either.

I took a quick look comparing the latest failing build https://cirrus-ci.com/task/5692190895636480 to the last successful build and the only difference I could see was that the build switched from rust 1.52.1 to rust 1.53 . When you tested freebsd - what version of rust did you use? I'm wondering if something broke with the latest version of rust.

Diff between the failing/working setup phases of those last two builds is:

 diff <(curl https://api.cirrus-ci.com/v1/task/5806808292917248/logs/setup.log) <(curl https://api.cirrus-ci.com/v1/task/5692190895636480/logs/setup.log)

7c7
< FreeBSD repository update completed. 30504 packages processed.
---
> FreeBSD repository update completed. 30513 packages processed.
82c82
< info: latest update on 2021-05-10, rust version 1.52.1 (9bc8c42bb 2021-05-09)
---
> info: latest update on 2021-06-17, rust version 1.53.0 (53cb7b09b 2021-06-17)
100c100
<   stable-x86_64-unknown-freebsd installed - rustc 1.52.1 (9bc8c42bb 2021-05-09)
---
>   stable-x86_64-unknown-freebsd installed - rustc 1.53.0 (53cb7b09b 2021-06-17)

Looking at the build phase shows that we downloaded + built the same version of all dependencies -

curl https://api.cirrus-ci.com/v1/task/5806808292917248/logs/build.log | grep Downloaded  | sort > downloaded_working
curl https://api.cirrus-ci.com/v1/task/5692190895636480/logs/build.log | grep Downloaded  | sort > downloaded_failing
diff downloaded_working downloaded_failing # shows no difference

@Jongy
Copy link
Contributor Author

Jongy commented Jun 26, 2021

Very weird indeed.

When you tested freebsd - what version of rust did you use?

I bootstrapped a new FreeBSD machine and used sh.rustup.rs, which yielded:
rustc 1.53.0 (53cb7b09b 2021-0617)

@benfred
Copy link
Owner

benfred commented Jun 26, 2021

Thanks! I'm trying to switch the github actions CI to rust 1.52.1 to see what happens here https://github.com/benfred/py-spy/pull/409/checks?check_run_id=2922887307 .

@benfred
Copy link
Owner

benfred commented Jun 26, 2021

So - that github actions CI pr still failed - but with rust 1.52.1 the tests at least pass first
https://github.com/benfred/py-spy/pull/409/checks?check_run_id=2922887307

running 7 tests
test test_long_sleep ... ok
test test_local_vars ... ok
test test_busy_loop ... ok
test test_negative_linenumber_increment ... ok
test test_thread_names ... ok
test test_unicode ... ok
test test_recursive ... ok

test result: ok. 7 passed; 0 failed; 0 ignored; 0 measured; 0 filtered out; finished in 1.78s

@Jongy
Copy link
Contributor Author

Jongy commented Jun 27, 2021

Removed all commits but 4becdfe, so this PR is ready once we fix the FreeBSD issue

@benfred
Copy link
Owner

benfred commented Jun 27, 2021

that last commit there (that I just merged into this branch) seems to fix freebsd https://cirrus-ci.com/task/5296576210927616###

@Jongy
Copy link
Contributor Author

Jongy commented Jun 27, 2021

that last commit there (that I just merged into this branch) seems to fix freebsd https://cirrus-ci.com/task/5296576210927616###

Cool :)

We can also merge benfred/remoteprocess#21 now

@benfred benfred merged commit 37163b6 into benfred:master Jun 27, 2021
benfred pushed a commit to benfred/remoteprocess that referenced this pull request Jun 27, 2021
@Jongy Jongy deleted the upgrade-goblin branch June 27, 2021 07:41
Jongy added a commit to intel/gprofiler that referenced this pull request Jun 27, 2021
* Includes a fix for py-spy from benfred/py-spy#407

* Bump gProfiler version to 1.1.3
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