Re: [PATCH V7] amd64-mpx: initialize bnd register before performing inferior calls.

["Tedeschi, Walfred" <walfred.tedeschi@intel.com>]

Hello Pedro,


On 02/15/2017 02:15 PM, Pedro Alves wrote:
> On 02/15/2017 01:01 PM, Tedeschi, Walfred wrote:
>
>> Pedro wrote:
>>> In reality it did not work as expected. When an inferior call is done
>>> and there is a breakpoint in the executed GDB issues a message like:
>>>
>>> "The program being debugged stopped while in a function called from GDB.
>>> Evaluation of the expression containing the function
>>> (foo) will be abandoned.
>>> When the function is done executing, GDB will silently stop."
>> This looks normal.  What were you expecting instead?
>>
>> --
>> Fred:
>> Message per se is fine. I was expecting that the inferior call should run isolated, exemplifying:
>>
>> 1. Inferior is stopped.
>> 2. inferior call is done.
>> 3. breakpoint in inferior call is triggered.
>> 4. continue is issued.
>> 5. signal is presented to the user.
>> 6. continue is issued again.
>> 7. expected - control should be back to 1, i.e. on stop mode.
>> 7. actual behavior - application finishes with the signal
>> 8. actual behavior - after the breakpoint no result will be displayed to the user (about the inf call)
> Just to be clear, I assume that between 3 and 4 there was
> a 3.5 step:
>
>    3.5. poke bnd registers manually, such that the infcall
>         crashes with a bounds violation.
>
> and that in step "5.", the signal you mention is a SIGSEGV
> with bound violation info.
>
> Correct?
Yes.
But actual behavior at 7 has an issue!

When we set the BND registers from gdb itself (applying the patch) it 
looks like changing the values of BND again while in the prolog have no 
effect.
Lets go to the reproducer:

The inferior call i want to do is "upper (x, a, b, c, d, 100)".
it has the following relevant prolog:

   0x0000000000400a0b <+1>:    mov    %rsp,%rbp
    0x0000000000400a0e <+4>:    sub    $0x18,%rsp
    0x0000000000400a12 <+8>:    mov    %rdi,-0x18(%rbp)
    0x0000000000400a16 <+12>:    mov    %rsi,-0x20(%rbp)
    0x0000000000400a1a <+16>:    mov    %rdx,-0x28(%rbp)
    0x0000000000400a1e <+20>:    mov    %rcx,-0x30(%rbp)
    0x0000000000400a22 <+24>:    mov    %r8,-0x38(%rbp)
    0x0000000000400a26 <+28>:    mov    %r9d,-0x3c(%rbp)
    0x0000000000400a2a <+32>:    bndmov %bnd0,-0x50(%rbp)
    0x0000000000400a2f <+37>:    bndmov %bnd1,-0x60(%rbp)
    0x0000000000400a34 <+42>:    bndmov %bnd2,-0x70(%rbp)
    0x0000000000400a39 <+47>:    bndmov %bnd3,-0x80(%rbp)

I can stop at the first instruction of "upper" by issuing b (void*)&upper.
In order to verify the effective change in the BND i have printed 
bnd0..bnd3. Register values were same as entered with the GDB command.
Other way is to do instruction stepping till " bndmov %bnd3,-0x80(%rbp)" 
and examine the memory at the indicated places.

Surprise! In the gdb i have applied the patch though changing the 
BND0..BND3 values at 0x0400a0b value present on memory was still set to 
the init state.
In the version without applying the patch it i could see the value i 
entered while stopped at the first instruction.

I am assuming that this is a problem or i am wrong?



> I'm not seeing how an option to clear/not clear the bnd
> registers changes anything for the parts you found unexpected.
> All steps from 5. onward will be the same, won't they?
Message would already scare the user at first and the absence of output 
would cause confusion. It caused in me.
Option to set or not will let it more clear. However there was an 
additional fact in my confusion caused by what i described above.
> To not pass the SIGSEGV to the inferior, you can also
> resume with "signal 0", or do "handle SIGSEGV nopass" before
> the continue.
Yes, my thought was only if it should assume that automatically or not 
while performing an inferior call.
On the other hand too many automatism will also cause issues! :)

> As for 8. being silent, that's something that I think would
> be nice to change, though that's not specific to bnd at
> all.
I agree! It would need to investigate though!
> Thanks,
> Pedro Alves
>
>> In 6 if I use return instead I fall back to my expected behavior. In this sense, I think the user can work in that way.
>>
>> I will continue a bit more testing now. I have got confused with an intermediate version 01/27/2017.
>> It was not displaying the signal step (5), now it is on again.
>> --
>>
>>> Also after that no signal generated within the function call is reported.
>> Please clarify.
>>
>> Fred: I tried to summarize above, not sure I succeeded.
>>
>> Fred: Additional comment:
>> Trying that now I am considering that this as a whole is more complicated than expected.
>> Too many actions to realize the simple thing that is set a register before an inferior call and perform the call.
>>
>> I am inclined to give it a try in creating set/show for the feature and see how it behaves.
>> In fact I will do it and submit the V8 based on that let's see how it looks like.
>>
>> Thanks a lot for the review!
>>
>> Best regards,
>> /Fred
>>
>>> Not sure if this a limitation or a bug. In case you think it is a bug
>>> i can add some entries in Bugzilla.
>> Thanks,
>> Pedro Alves
Thanks again!
/Fred
Intel Deutschland GmbH
Registered Address: Am Campeon 10-12, 85579 Neubiberg, Germany
Tel: +49 89 99 8853-0, www.intel.de
Managing Directors: Christin Eisenschmid, Christian Lamprechter
Chairperson of the Supervisory Board: Nicole Lau
Registered Office: Munich
Commercial Register: Amtsgericht Muenchen HRB 186928