Open Quuxplusone opened 10 years ago
Bugzilla Link | PR19347 |
Status | NEW |
Importance | P normal |
Reported by | emaste@freebsd.org |
Reported on | 2014-04-04 15:43:19 -0700 |
Last modified on | 2020-11-05 12:48:03 -0800 |
Version | unspecified |
Hardware | PC FreeBSD |
CC | andrew.macp@gmail.com, jingham@apple.com, tfiala@google.com |
Fixed by commit(s) | |
Attachments | |
Blocks | |
Blocked by | |
See also |
bisection shows this fails as of r205497: "Make the fail messages"
This patch changed the way "Thread::SetResumeState" works. The problem was
that some code intended to say "I would like this thread to run if nobody has
any other opinion", e.g. the ThreadList::WillResume. Other code intended "make
this thread runnable for sure.", e.g. Thread::Suspend. So I added an
override_suspend parameter which should be false for the former folks, and true
for the latter.
I noticed there were a few places where SetResumeState was called on the
Linux/FreeBSD side, but in each case they were proceeded by some comment like:
// FIXME: This should probably happen somewhere else.
or
// TODO: the line below shouldn't really be done, but
// the POSIXThread might rely on this so I will leave this in for now
so I wasn't sure what to do. Apparently you may need to pass
override_suspend=true to these calls? false is the default.
> // FIXME: This should probably happen somewhere else.
> // TODO: the line below shouldn't really be done, but
> // the POSIXThread might rely on this so I will leave this in for now
Those ones are in ProcessPOSIX, but in member functions that are overridden in
ProcessFreeBSD and thus only used on Linux, so they shouldn't be the fault here.
Thinking about this some more though, "step 2" in my initial description of the
problem confuses me: it's surprising that the IP reported for the "other"
thread is one beyond the breakpoint.
I think this is an issue with how "simultaneous" breakpoints get reported.
"Our" thread executed the 0xCC trap, and ptrace reports the expected IP. The
IP in the "other" thread suggests it has executed a one-byte instruction (the
original instruction was wider).
Is this on an x86 machine? The trap on x86 is executed. So when you are told you hit the trap you are supposed to decrement the PC by 1. Maybe when both threads hit the breakpoint, you are decrement the pc on one thread, but forgetting to do so on the other?
Yes, it is on x86.
What I've found so far is that both threads have executed the INT3, but the
callback on FreeBSD only had a mechanism to be notified of one thread hitting
the breakpoint.
I have some WIP that changes this to scan the whole list of threads after a
stop event, and derive the breakpoint stop infos from there. It's not working
100% yet, but seems to be the best approach, and is also more in-line with
ProcessGDBRemote, which I'm using as a template.
This gives me output as shown below, with two threads simultaneously reporting
the breakpoint:
Process 50519 stopped
* thread #2: tid = 102352, 0x0000000000400754 a.out`step_out_of_here() + 4 at
sim.cpp:33, stop reason = breakpoint 1.1
frame #0: 0x0000000000400754 a.out`step_out_of_here() + 4 at sim.cpp:33
30 volatile int g_test = 0;
31
32 void step_out_of_here() {
-> 33 g_test += 5; // Set breakpoint here
34 }
35
36 void *
thread #3: tid = 102344, 0x0000000000400754 a.out`step_out_of_here() + 4 at sim.cpp:33, stop reason = breakpoint 1.1
frame #0: 0x0000000000400754 a.out`step_out_of_here() + 4 at sim.cpp:33
30 volatile int g_test = 0;
31
32 void step_out_of_here() {
-> 33 g_test += 5; // Set breakpoint here
34 }
35
36 void *
There's a bit of a problem as well in that the other breakpoint SIGTRAP is already queued when I handle the first one, and it gets delivered when we try to step/continue.
Yes, the way Mac OS X works you when you get one exception (equivalent of the SIGTRAP you get) we poll the exception port till we don't get any more exceptions, and then report the whole bundle. Don't think there's a polling version of ptrace, however.
Just adding that there are unit test failures in Linux related to this as well, specifically TestCreateDuringStep.py and TestExitDuringStep.py which simply run forever now. If I can help with testing anything just let me know.
So if it is true that JUST reverting r205497 makes these failures go away, then the fix should be easy. All that patch did was change the behavior of Thread::SetResumeState(eStateRunning) such that if the state had been eStateSuspended, then it would not change the state. If you want to revert that call to the previous behavior, change the call to: Thread::SetResumeState(eStateRunning, true).
Note, however, that the thread state should only be being set to eStateSuspended when the user wants to keep that thread from running. In the normal course of operation, we suspend some threads to do things like step over breakpoints, etc. But that only affects the m_temporary_resume_state in the thread.
Yes, I'm able to have the test pass on FreeBSD by just reverting r205497. However, this investigation highlighted an underlying problem with the way simultaneous breakpoints are handled, so I'd like to resolve that first.
When we stop for the first trap we could determine that another thread has a pending trap, but I don't think there's an equivalent to polling the exception port as on OS X as you describe.
What I might be able to do is poll the threads after the trap, and issue a PT_CONTINUE if any other than the reported stop thread indicate a pending trap, which should deliver it immediately and both/all could be handled together.
Hey Ed - I'd like to resolve this on the Linux side. You had indicated you had some code in motion for this. What was the status of your code change? Would it be FreeBSD only? I can disable these tests on Linux, try Jim's override suggestion, back out the change, wait for Codeplay's patch (Luke just indicated they are working on one on the lldb-dev list), etc.
If I don't hear shortly, I'll just disable these on Linux for the moment and re-enable when we work out what we're doing here.
At the moment I'm trying the passing of true to the defaulted second parameter
of SetResumeState() in the two calls in POSIXThread.
This gets me to this state (2 failures as opposed to hangs):
FAIL: LLDB (suite) :: TestAttachResume.py (Linux tfiala2.mtv.corp.google.com
3.2.5-gg1336 #1 SMP Thu Aug 29 02:37:18 PDT 2013 x86_64 x86_64)
FAIL: LLDB (suite) :: TestPtrRef2Typedef.py (Linux tfiala2.mtv.corp.google.com
3.2.5-gg1336 #1 SMP Thu Aug 29 02:37:18 PDT 2013 x86_64 x86_64)
ninja: build stopped: subcommand failed.
I consider this a step in the right direction but I will look into these two
first to see if they're fallout or unrelated.
The TestAttachResume failure looks to be related. It's listed as hitting the
breakpoint twice now.
This code in process_attach_continue_interrupt_detach():
self.assertTrue(wait_for_state(lldb.eStateStopped),
'Process not stopped after breakpoint')
self.expect('br list', 'Breakpoint not hit',
patterns = ['hit count = 1'])
is hitting twice, producing this failure:
runCmd: br list
output: Current breakpoints:
1: file = 'main.c', line = 12, locations = 1, resolved = 1, hit count = 2
1.1: where = AttachResume`start + 29 at main.c:12, address = 0x000000000040070
d, resolved, hit count = 2
Expecting pattern: hit count = 1
Not matched
I found the smallest change that got Linux running:
Waiting for Emacs...
Sending source/Plugins/Process/POSIX/POSIXThread.cpp
Transmitting file data .
Committed revision 206618.
I filed a follow-up bug for a resume test that now fails due to getting hit
twice when expected once, quite possibly related to the stop-gap fix above.
Marked that test as expected fail on Linux.
FWIW TestAttachResume works for me (although a related failure in llvm.org/pr19310 seems to be no longer reproducible).
Appears test has been renamed; as of today
$ bin/lldb-dotest -p TestThreadStepOut -f test_step_all_threads_dwarf
reports XFAIL. I have not confirmed that the issue is identical to that
reported in 2014.