[RFA 5/5 v2] Darwin: fix SIGTRAP when debugging

[RFA 5/5 v2] Darwin: fix SIGTRAP when debugging

[Xavier Roirand]

Debugging a program under Darwin does not work:

(gdb) start
Temporary breakpoint 1 at 0x100000fb4: file /tmp/helloworld.c, line 1.
Starting program: /private/tmp/helloworld
[New Thread 0x2903 of process 60326]
During startup program terminated with signal SIGTRAP, Trace/breakpoint
trap.

Field signaled from darwin_thread_info is not initialized thus signal
sent to the debuggee is considered as not sent by GDB whereas it should.

This patch fixes this problem and also updates (change type, initialize)
single_step value in the same structure at the same time.

gdb/ChangeLog:

        * darwin-nat.h (struct darwin_thread_info) <signaled>: Change
        type and initialize.
        (struct darwin_thread_info) <single_step>: Change type and
        initialize.

Change-Id: I0fe2a6985df9d0dfcc8a2a258a3ef70cfa19b403
---
 gdb/darwin-nat.h | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/gdb/darwin-nat.h b/gdb/darwin-nat.h
index 5965df08ab6..4232b5e0365 100644
--- a/gdb/darwin-nat.h
+++ b/gdb/darwin-nat.h
@@ -126,10 +126,10 @@ struct darwin_thread_info : public private_thread_info
   enum darwin_msg_state msg_state;
 
   /* True if this thread is single-stepped.  */
-  unsigned char single_step;
+  bool single_step = 0;
 
   /* True if a signal was manually sent to the thread.  */
-  unsigned char signaled;
+  bool signaled = 0;
 
   /* The last exception received.  */
   struct darwin_exception_msg event;
-- 
2.15.2 (Apple Git-101.1)

Re: [RFA 5/5 v2] Darwin: fix SIGTRAP when debugging

[Simon Marchi]

On 2018-09-09 02:27 PM, Xavier Roirand wrote:
> Debugging a program under Darwin does not work:
> 
> (gdb) start
> Temporary breakpoint 1 at 0x100000fb4: file /tmp/helloworld.c, line 1.
> Starting program: /private/tmp/helloworld
> [New Thread 0x2903 of process 60326]
> During startup program terminated with signal SIGTRAP, Trace/breakpoint
> trap.
> 
> Field signaled from darwin_thread_info is not initialized thus signal
> sent to the debuggee is considered as not sent by GDB whereas it should.
> 
> This patch fixes this problem and also updates (change type, initialize)
> single_step value in the same structure at the same time.
> 
> gdb/ChangeLog:
> 
>         * darwin-nat.h (struct darwin_thread_info) <signaled>: Change
>         type and initialize.
>         (struct darwin_thread_info) <single_step>: Change type and
>         initialize.
> 
> Change-Id: I0fe2a6985df9d0dfcc8a2a258a3ef70cfa19b403
> ---
>  gdb/darwin-nat.h | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/gdb/darwin-nat.h b/gdb/darwin-nat.h
> index 5965df08ab6..4232b5e0365 100644
> --- a/gdb/darwin-nat.h
> +++ b/gdb/darwin-nat.h
> @@ -126,10 +126,10 @@ struct darwin_thread_info : public private_thread_info
>    enum darwin_msg_state msg_state;
>  
>    /* True if this thread is single-stepped.  */
> -  unsigned char single_step;
> +  bool single_step = 0;
>  
>    /* True if a signal was manually sent to the thread.  */
> -  unsigned char signaled;
> +  bool signaled = 0;
>  
>    /* The last exception received.  */
>    struct darwin_exception_msg event;
> 

As mentioned in the last review, I think all files should be initialized to a
sensible value, either to a constant value or by the constructor.  This is
what I would suggest, on top of your patch (not tested).  Would that work
for you?


From 45abaf5843f1c1f55a233d00cc0c07ed38f64d89 Mon Sep 17 00:00:00 2001
From: Simon Marchi <simon.marchi@ericsson.com>
Date: Sun, 9 Sep 2018 14:53:17 +0100
Subject: [PATCH] fixup

---
 gdb/darwin-nat.c | 6 ++----
 gdb/darwin-nat.h | 6 +++++-
 2 files changed, 7 insertions(+), 5 deletions(-)

diff --git a/gdb/darwin-nat.c b/gdb/darwin-nat.c
index d23706d..326e97b 100644
--- a/gdb/darwin-nat.c
+++ b/gdb/darwin-nat.c
@@ -339,10 +339,8 @@ darwin_check_new_threads (struct inferior *inf)
       if (new_ix < new_nbr && (old_ix == old_nbr || new_id < old_id))
 	{
 	  /* A thread was created.  */
-	  darwin_thread_info *pti = new darwin_thread_info;
-
-	  pti->gdb_port = new_id;
-	  pti->msg_state = DARWIN_RUNNING;
+	  darwin_thread_info *pti
+	    = new darwin_thread_info (new_id, DARWIN_RUNNING);

 	  /* Add the new thread.  */
 	  add_thread_with_info (ptid_t (inf->pid, 0, new_id), pti);
diff --git a/gdb/darwin-nat.h b/gdb/darwin-nat.h
index 4232b5e..fee535b 100644
--- a/gdb/darwin-nat.h
+++ b/gdb/darwin-nat.h
@@ -113,12 +113,16 @@ enum darwin_msg_state

 struct darwin_thread_info : public private_thread_info
 {
+  darwin_thread_info (thread_t gdb_port, darwin_msg_state msg_state)
+  : gdb_port (gdb_port), msg_state (msg_state)
+  {}
+
   /* The thread port from a GDB point of view.  */
   thread_t gdb_port;

   /* The thread port from the inferior point of view.  Not to be used inside
      gdb except for get_ada_task_ptid.  */
-  thread_t inf_port;
+  thread_t inf_port = 0;

   /* Current message state.
      If the kernel has sent a message it expects a reply and the inferior
-- 
2.7.4

Re: [RFA 5/5 v2] Darwin: fix SIGTRAP when debugging

[Xavier Roirand]

Le 9/9/18 à 3:55 PM, Simon Marchi a écrit :
> On 2018-09-09 02:27 PM, Xavier Roirand wrote:
>> Debugging a program under Darwin does not work:
>>
>> (gdb) start
>> Temporary breakpoint 1 at 0x100000fb4: file /tmp/helloworld.c, line 1.
>> Starting program: /private/tmp/helloworld
>> [New Thread 0x2903 of process 60326]
>> During startup program terminated with signal SIGTRAP, Trace/breakpoint
>> trap.
>>
>> Field signaled from darwin_thread_info is not initialized thus signal
>> sent to the debuggee is considered as not sent by GDB whereas it should.
>>
>> This patch fixes this problem and also updates (change type, initialize)
>> single_step value in the same structure at the same time.
>>
>> gdb/ChangeLog:
>>
>>          * darwin-nat.h (struct darwin_thread_info) <signaled>: Change
>>          type and initialize.
>>          (struct darwin_thread_info) <single_step>: Change type and
>>          initialize.
>>
>> Change-Id: I0fe2a6985df9d0dfcc8a2a258a3ef70cfa19b403
>> ---
>>   gdb/darwin-nat.h | 4 ++--
>>   1 file changed, 2 insertions(+), 2 deletions(-)
>>
>> diff --git a/gdb/darwin-nat.h b/gdb/darwin-nat.h
>> index 5965df08ab6..4232b5e0365 100644
>> --- a/gdb/darwin-nat.h
>> +++ b/gdb/darwin-nat.h
>> @@ -126,10 +126,10 @@ struct darwin_thread_info : public private_thread_info
>>     enum darwin_msg_state msg_state;
>>   
>>     /* True if this thread is single-stepped.  */
>> -  unsigned char single_step;
>> +  bool single_step = 0;
>>   
>>     /* True if a signal was manually sent to the thread.  */
>> -  unsigned char signaled;
>> +  bool signaled = 0;
>>   
>>     /* The last exception received.  */
>>     struct darwin_exception_msg event;
>>
> 
> As mentioned in the last review, I think all files should be initialized to a
> sensible value, either to a constant value or by the constructor.  This is
> what I would suggest, on top of your patch (not tested).  Would that work
> for you?
> 
> 

I woud have think of two patches, one for the signaled field and one for 
whole field initialization but that's not a problem to merge in one. 
I'll do this.

Re: [RFA 5/5 v2] Darwin: fix SIGTRAP when debugging

[Tom Tromey]

>>>>> "Xavier" == Xavier Roirand <roirand@adacore.com> writes:

Xavier> +  bool single_step = 0;
 
Xavier> +  bool signaled = 0;
 
I would use = false here.

Tom