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

ldc2 --checkaction=halt produces same binary as --checkaction=D #3430

Closed
denizzzka opened this issue May 14, 2020 · 10 comments · Fixed by #3431
Closed

ldc2 --checkaction=halt produces same binary as --checkaction=D #3430

denizzzka opened this issue May 14, 2020 · 10 comments · Fixed by #3431

Comments

@denizzzka
Copy link
Contributor

denizzzka commented May 14, 2020

(I want to halt if assertion occured.)

Tried on x86_64 and ARM32

Code for check:

void main()
{
    assert(false, "Please, HLT me!");
}
@kinke
Copy link
Member

kinke commented May 14, 2020

Thx for the report. Looks like there's no DMD test for this...

kinke added a commit that referenced this issue May 15, 2020
@denizzzka
Copy link
Contributor Author

denizzzka commented May 31, 2020

@kinke Looks like for ARM 8813f0e emits "undefined" TRAP instruction (0xDEFE) from LLVM's __builtin_trap.

This instruction causes immediate jump to blocking_handler vector value what typically contains infinity loop.

But -checkaction=halt described as "Halt the program execution (very lightweight)".
Also this jump leads to lose stack trace. Not exactly what is need for debug (To jump into blocking_handler it is possible to use more highlevel ways (abort() call, for example.)

Looks like ARM isn't have appropriate "Halt and Catch Fire" instruction?

@kinke
Copy link
Member

kinke commented May 31, 2020

The fix uses the llvm.trap intrinsic, just like an assert(0) with -release. For x86, that's ud2. See http://llvm.org/docs/LangRef.html#llvm-trap-intrinsic for the official description. Right below that one, there's llvm.debugtrap.

@denizzzka
Copy link
Contributor Author

debugtrap looks as more appropriate at least for ARM - it adds BKPT instruction instead of undefined "TRAP".

From ARM manual:
The BKPT instruction causes the processor to enter Debug state. Debug tools can use this to investigate system state when the instruction at a particular address is reached.

While in debug state, the processor behaves as follows:

  • The DBGDSCR[0] core halted bit is set.
  • The DBGACK signal is asserted, see DBGACK.
  • The DBGDSCR[5:2] method of entry bits are set appropriately.
  • The processor is halted. The pipeline is flushed and no instructions are fetched.
  • The processor does not change the execution mode. The CPSR is not altered.
  • Exceptions are treated as described in Exceptions in debug state.
  • Interrupts are ignored.
  • New debug events are ignored.

@kinke
Copy link
Member

kinke commented May 31, 2020

Then please play around with it and see if it fits your needs. If it does, open a PR to discuss whether this should be the default, only used with -g or for ARM etc.

Edit: I've only found http://llvm.1065342.n5.nabble.com/Trap-instruction-for-ARMv7-and-Thumb-td59521.html wrt. this.

@denizzzka
Copy link
Contributor Author

I’m afraid to touch ldc sources and don’t know how to test different platforms. I will just create a Issue ticket.

@kinke
Copy link
Member

kinke commented May 31, 2020

You can trivially substitute trap by debugtrap in

ldc/gen/toir.cpp

Line 1688 in 9a92488

p->ir->CreateCall(GET_INTRINSIC_DECL(trap), {});
(even with the GitHub editor) and then open a draft PR. CI will take care of building and testing everything; you can then download a prebuilt artifact from Azure Pipelines and test it with your code.

Yes, it's really that simple.

@denizzzka
Copy link
Contributor Author

Azure Pipelines

I am on linux

@kinke
Copy link
Member

kinke commented May 31, 2020

So what? Azure builds all packages.

@denizzzka
Copy link
Contributor Author

Ok, will try it

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 a pull request may close this issue.

2 participants