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

Add specs for debug info output #4718

Closed
wants to merge 2 commits into from

Conversation

RX14
Copy link
Contributor

@RX14 RX14 commented Jul 16, 2017

A rebased and fixed version of #3476.

Closes #3476.

debug %(
(gdb) break foo
(gdb) run
(gdb) next
Copy link
Contributor Author

@RX14 RX14 Jul 16, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This line is required because break foo seems to stop at spec:1 (i.e. the def line). In #3476 this wasn't required, but it seems to be here. Is this a regression?

@RX14
Copy link
Contributor Author

RX14 commented Jul 16, 2017

This works on my machine, but on travis linux gdb seems to exit 127, and on osx it appears to give different output.

@RX14
Copy link
Contributor Author

RX14 commented Jul 16, 2017

The linux failures are because gdb isn't in the Dockerfile: this change is essentially a no-op. I'll submit a PR to jhass/crystal-build-docker later.

I don't have an osx machine to debug on (and due to it being apple they're very hard to get) so I think someone else will have to have a look at that.

@RX14
Copy link
Contributor Author

RX14 commented Jul 16, 2017

Looks like the docker container needs --privileged to run. Using this flag looks fine as it's a temporary VM anyway and there's nothing particularly untrusted running in the container.

Unfortunately, this is why gdb doesn't work on osx. The instructions on how to codesign gdb appear to require using a GUI which isn't feasible. Maybe someone can experiment whether they can reliably automate this using security(1)? If not, simply disabling this on osx should work, although very unfortunate.

Code originally taken from
waj@da7e88b
and modified to compile with the current version and improve failure
messages.
@RX14
Copy link
Contributor Author

RX14 commented Jul 26, 2017

Looks like debug info is broken on 32bits. Should we just disable the specs on 32bit or attempt to fix it?

Similarly, on OSX it'll be hard to get the specs working. It seems it requires manual setup on most systems to enable gdb. We should probably disable these specs on osx by default but provide a way of enabling them for people who have a working gdb. Any thoughts on the best way to do this? A build flag?

@ysbaddaden
Copy link
Contributor

Sigh, that would have been fantastic, but if the solution isn't portable across systems, the benefit is dubious, for writing tests, or for developers writing/fixing them to need a specific platform :(

Maybe gdb instrumentation, taking platform discrepancies into account, would perform better?

@RX14
Copy link
Contributor Author

RX14 commented Jul 26, 2017

@ysbaddaden what do you mean by "gdb instrumentation"?

@ysbaddaden
Copy link
Contributor

I meant opening a gdb session (e.g. gdbserver) then interact through the GDB Remote Serial Protocol. That should get us through platform discrepancies, but that would be a lot of work...

@RX14
Copy link
Contributor Author

RX14 commented Jul 27, 2017

@ysbaddaden that won't help when GDB has insufficient permissions on osx and the debug info is broken on 32bit. It'd just give us the same wrong data, which would fail the specs.

@RX14
Copy link
Contributor Author

RX14 commented Aug 20, 2017

I can disable this on OSX but it's pretty clear that debug info is actually broken on 32bit here, so we should investigate that. Or should we merge this so that we can detect 64bit regressions at least?

@jhass
Copy link
Member

jhass commented Jan 22, 2020

What's the status here, do we still want this?

@RX14
Copy link
Contributor Author

RX14 commented Jan 27, 2020

I would still love this, but it needs to be checked whether these issues are now fixed or not

@RX14 RX14 closed this Apr 11, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants