Skip to content

Add specs for debug info output#4718

Closed
RX14 wants to merge 2 commits intocrystal-lang:masterfrom
RX14:debug/spec-with-gdb
Closed

Add specs for debug info output#4718
RX14 wants to merge 2 commits intocrystal-lang:masterfrom
RX14:debug/spec-with-gdb

Conversation

@RX14
Copy link
Copy Markdown
Member

@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
Copy Markdown
Member 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
Copy Markdown
Member 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
Copy Markdown
Member 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
Copy Markdown
Member 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 RX14 force-pushed the debug/spec-with-gdb branch from 8bfe6ae to 8642e75 Compare July 25, 2017 22:50
@RX14
Copy link
Copy Markdown
Member 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
Copy Markdown
Collaborator

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
Copy Markdown
Member Author

RX14 commented Jul 26, 2017

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

@ysbaddaden
Copy link
Copy Markdown
Collaborator

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
Copy Markdown
Member 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
Copy Markdown
Member 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
Copy Markdown
Member

jhass commented Jan 22, 2020

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

@RX14
Copy link
Copy Markdown
Member 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.

4 participants