[PATCH] Fix gdb.base signal.exp test case in AIX

[PATCH] Fix gdb.base signal.exp test case in AIX

[Aditya Kamath1]

[-- Attachment #1.1: Type: text/plain, Size: 4457 bytes --]

Respected community members,

Hi,

Please find attached the patch. {See: 0001-Fix-gdb.bas-sigall.exp-testcase-in-AIX.patch}
This is a patch to fix the failures in sigall.exp testcase under gdb.base in GDB for AIX.

We currently have     210 failures while running this test suite as shown in the summary below.
          === gdb Summary ===

# of expected passes            215
# of unexpected failures        210

This is a testcase which catches signals in the debugger and tells the user that GDB has caught a signal that program received.

We can refer this document { https://www.ibm.com/docs/en/sdk-java-technology/8?topic=reference-signal-handling} for the signal numbers on AIX. So, in AIX waitpid () function we have the status number returned for the following signals as:-.

SIGBUS = 10. Status returned by the code = A7F
SIGSEGV = 11. Status returned by the code = B7F
SIGILL = 4. Status returned by the code = 47F
SIGABRT = 6. Status returned by the code = 67F
SIGINT = 2. Status returned by the code = 27F
SIGHUP = 1. Status returned by the code = 17F,

We see a pattern that the 3rd hexadecimal digit is reserved to identify the signal that caused the program to be interrupted. So, in our rs6000-aix-nat.c file when we handle our status numbers, we had put a condition that.
else if (status == 0x7f || status == 0x17f || status == 0x137f)
     ourstatus->set_spurious ();

This is a mistake. I missed seeing the host_status_to_waitstatus (status) where we have the status handling for the signal received case.

As an impact of having that condition when we have a program like Program 1 as pasted below in this email some of the signals do not print message that the program had received a signal. {See Output before patch section below in this email } For example, for signal SIGHUP we were not able to see the message that program received signal SIGHUP. Due to this there existed these many failures.

When I removed the condition 17f and 137f we can see the message that we received the signal SIGHUP as shown in the output below {See Output after patch section below in this email }  since now we allow the host_status_to_waitstatus (status) to handle signal handling status for us.

After the patch was applied we get test case numbers for gdb.base/sigall.exp..

gmake check RUNTESTFLAGS='gdb.base/sigall.exp CC_FOR_TARGET="/opt/freeware/bin/gcc" CXX_FOR_TARGET="/opt/freeware/bin/g++" CXXFLAGS_FOR_TARGET="-O0 -w -g -gdwarf -maix64" CFLAGS_FOR_TARGET="-O0 -w -g -gdwarf -maix64"'
=== gdb Summary ===

# of expected passes            388

I also checked our fork test programs and they work all right without this condition.

Kindly let me know what you think and if it is all right, kindly consider removing this condition as sent in the attached patch.

Have a nice day ahead.

Thanks and regards,
Aditya.


Program 1:-
# cat ~/gdb_tests/sigall.c
#include<stdio.h>
#include<signal.h>

#include <unistd.h>
void handle_sighup(int sig)
{
    printf("Caught signal %d\n", sig);
}

int main()
{
    signal(SIGHUP, handle_sighup);
    kill(getpid(), SIGHUP);
    while (1) ;
    return 0;
}
Output before patch:-

# ./gdb ~/gdb_tests/sigall
GNU gdb (GDB) 15.0.50.20231117-git
Copyright (C) 2023 Free Software Foundation, Inc.
…….
Reading symbols from //gdb_tests/sigall...
(gdb) b main
Breakpoint 1 at 0x10000608: file //gdb_tests/sigall.c, line 12.
(gdb) r
Starting program: /gdb_tests/sigall

Breakpoint 1, main () at //gdb_tests/sigall.c:12
12          signal(SIGHUP, handle_sighup);
(gdb) n
13          kill(getpid(), SIGHUP);
(gdb)

14          while (1) ;
(gdb) q
A debugging session is active.

        Inferior 1 [process 10813814] will be killed.

Quit anyway? (y or n) y
Output after applying the patch:-

Reading symbols from //gdb_tests/sigall...
(gdb) b main
Breakpoint 1 at 0x10000608: file //gdb_tests/sigall.c, line 12.
(gdb) r
Starting program: /gdb_tests/sigall

Breakpoint 1, main () at //gdb_tests/sigall.c:12
12          signal(SIGHUP, handle_sighup);
(gdb) n
13          kill(getpid(), SIGHUP);
(gdb)

Program received signal SIGHUP, Hangup.
0x10000630 in main () at //gdb_tests/sigall.c:13
13          kill(getpid(), SIGHUP);
(gdb) n
Caught signal 1
14          while (1) ;
(gdb) q
A debugging session is active.

        Inferior 1 [process 10813844] will be killed.

Quit anyway? (y or n) y


[-- Attachment #2: 0001-Fix-gdb.bas-sigall.exp-testcase-in-AIX.patch --]
[-- Type: application/octet-stream, Size: 1409 bytes --]

From ca3e2ac29ac90f11f7ccd46f54b0a4bde31b61ef Mon Sep 17 00:00:00 2001
From: Aditya Vidyadhar Kamath <Aditya.Kamath1@ibm.com>
Date: Mon, 20 Nov 2023 04:13:49 -0600
Subject: [PATCH] Fix gdb.bas/sigall.exp testcase in AIX.

In AIX, we are not able to see the message of a signal recieved if a debugee recieves a signal.
This is a patch to fix the signal handling done incorrectly in AIX.

We remove the status that represent program recieving a signal and allow host_status_to_waitstatus to
handle it for us.
---
 gdb/rs6000-aix-nat.c | 7 ++-----
 1 file changed, 2 insertions(+), 5 deletions(-)

diff --git a/gdb/rs6000-aix-nat.c b/gdb/rs6000-aix-nat.c
index b7a34e0bf5f..771fef407a7 100644
--- a/gdb/rs6000-aix-nat.c
+++ b/gdb/rs6000-aix-nat.c
@@ -918,11 +918,8 @@ rs6000_nat_target::wait (ptid_t ptid, struct target_waitstatus *ourstatus,
   /* stop after load" status.  */
   if (status == 0x57c)
     ourstatus->set_loaded ();
-  /* 0x7f is signal 0.  0x17f and 0x137f are status returned
-     if we follow parent, a switch is made to a child post parent
-     execution and child continues its execution [user switches
-     to child and presses continue].  */
-  else if (status == 0x7f || status == 0x17f || status == 0x137f)
+  /* 0x7f is signal 0.  */
+  else if (status == 0x7f)
     ourstatus->set_spurious ();
   /* A normal waitstatus.  Let the usual macros deal with it.  */
   else
-- 
2.41.0

Re: [PATCH] Fix gdb.base signal.exp test case in AIX

[Ulrich Weigand]

Aditya Kamath1 <Aditya.Kamath1@ibm.com> wrote:

>We see a pattern that the 3rd hexadecimal digit is reserved to identify the signal that
>caused the program to be interrupted. So, in our rs6000-aix-nat.c file when we handle
>our status numbers, we had put a condition that.
>else if (status == 0x7f || status == 0x17f || status == 0x137f)
>     ourstatus->set_spurious ();
> 
>This is a mistake. I missed seeing the host_status_to_waitstatus (status) where we have
>the status handling for the signal received case.

You added these extra conditions in the multi-process debugging patch
(discussion starting here: https://sourceware.org/pipermail/gdb-patches/2022-July/190890.html)
as necessary to enable debugging across forks correctly.

Can you confirm that removing these conditions does not break
multi-process debugging again?

Bye,
Ulrich

Re: [PATCH] Fix gdb.base signal.exp test case in AIX

[Aditya Kamath1]

[-- Attachment #1.1: Type: text/plain, Size: 8570 bytes --]

Hi Ulrich,

>You added these extra conditions in the multi-process debugging patch
>(discussion starting here: https://sourceware.org/pipermail/gdb-patches/2022-July/190890.html)
>as necessary to enable debugging across forks correctly.

>Can you confirm that removing these conditions does not break
>multi-process debugging again?

We want to fix AIX test case failures in AIX and get it on par like Linux. When I checked where my test cases are failing and tried to fix the sigall.exp testcase I understood this is a mistake I made at that time.

Yes I can confirm.

Please see the code Program 1 below this email and all the 4 conditions -  set detach on fork = on && off as well as set follow-fork mode = child/parent. As far as set follow-fork-mode = child and set detach-on-fork = on case is concerned we know why it is failing {a mismatch with child ptid’s with what we are sending and what GDB core understands} and are discussing the fix in the other thread.

The same can be seen while running the fork-print-inferior-events.exp testcase in gdb.base. We know why those 2 failures have happened as shown below.


# gmake check RUNTESTFLAGS='gdb.base/fork-print-inferior-events.exp CC_FOR_TARGET="/opt/freeware/bin/gcc" CXX_FOR_TARGET="/opt/freeware/bin/g++" CXXFLAGS_FOR_TARGET="-O0 -w -g -gdwarf -maix64" CFLAGS_FOR_TARGET="-O0 -w -g -gdwarf -maix64"' | grep FAIL
WARNING: Couldn't find the global config file.
(gdb) FAIL: gdb.base/fork-print-inferior-events.exp: print_inferior_events=on: follow_fork_mode=child: detach_on_fork=on: run
(gdb) FAIL: gdb.base/fork-print-inferior-events.exp: print_inferior_events=off: follow_fork_mode=child: detach_on_fork=on: run

=== gdb Summary ===

# of expected passes            30
# of unexpected failures        2


SIGALL number before this patch:
   === gdb Summary ===

# of expected passes            215
# of unexpected failures        210

SIGALL test case number after this patch:-

=== gdb Summary ===

# of expected passes            388



The test case numbers of:-

GDB.threads before this patch:-
=== gdb Summary ===
# of expected passes            1765
# of unexpected failures        654
# of expected failures          4
# of known failures             21
# of unresolved testcases       11
# of untested testcases         9
# of unsupported tests          82
# of paths in test names        3


GDB.threads after this patch:-

# of expected passes        1790
# of unexpected failures    610
# of expected failures      4
# of known failures     21
# of unresolved testcases   10
# of untested testcases     9
# of unsupported tests      82
# of paths in test names    3


GDB.base before this patch:-
=== gdb Summary ===
# of expected passes            28254
# of unexpected failures        2392
# of expected failures          15
# of known failures             10
# of unresolved testcases       133
# of untested testcases         79
# of unsupported tests          117
# of paths in test names        5
# of duplicate test names       4

GDB.base after this patch:-
=== gdb Summary ===
# of expected passes        28377
# of unexpected failures    2163
# of expected failures      15
# of known failures     10
# of unresolved testcases   154
# of untested testcases     79
# of unsupported tests      117
# of paths in test names    5
# of duplicate test names   4


I have reattached the patch as well. Kindly let me know if you need anything else. Sorry for this mistake.

Have a nice day ahead.

Thanks and regards,
Aditya.

Program 1:-

#include <stdio.h>
#include <unistd.h>
#include <stdlib.h>
#include <pthread.h>
#include <assert.h>

pthread_barrier_t barrier;

#define NUM_THREADS 2

void *
thread_function (void *arg)
{
  /* This ensures that the breakpoint is only hit after both threads
     are created, so the test can always switch to the non-event
     thread when the breakpoint triggers.  */
  pthread_barrier_wait (&barrier);

  pid_t p;
    p = fork();
    if(p<0)
    {
      perror("fork fail");
      exit(1);
    }
    // child process because return value zero
    else if ( p == 0)
        printf("Hello from Child!\n");

    // parent process because return value non-zero.
    else
        printf("Hello from Parent!\n");
  while (1); /* break here */
}

int
main (void)
{
  int i;

  alarm (300);

  pthread_barrier_init (&barrier, NULL, NUM_THREADS);

  for (i = 0; i < NUM_THREADS; i++)
    {
      pthread_t thread;
      int res;

      res = pthread_create (&thread, NULL,
                            thread_function, NULL);
      assert (res == 0);
    }

  while (1)
    sleep (1);

  return 0;
}

Follow-fork-mode = parent and detach_on_fork = on

Reading symbols from //gdb_tests/multi-thread-fork...
(gdb) r
Starting program: /gdb_tests/multi-thread-fork
[New Thread 258]
[New Thread 515]
[Detaching after fork from child process 10813794]
Hello from Child!
[Detaching after fork from child process 23331272]
Hello from Child!
Hello from Parent!
Hello from Parent!

Thread 3 received signal SIGINT, Interrupt.
[Switching to Thread 515]
thread_function (arg=0x0) at //gdb_tests/multi-thread-fork.c:50
50        while (1); /* break here */
(gdb)

Follow-fork-mode = child and detach_on_fork = on

Reading symbols from //gdb_tests/multi-thread-fork...
(gdb) set follow-fork-mode child
(gdb) r
Starting program: /gdb_tests/multi-thread-fork
[New Thread 258]
[New Thread 515]
[Attaching after Thread 515 fork to child process 10813796]
[New inferior 2 (process 10813796)]
[Detaching after fork from parent process 23331276]
Hello from Parent!
[Inferior 1 (process 23331276) detached]
Hello from Child!
Hello from Parent!
thread.c:1385: internal-error: switch_to_thread: Assertion `thr != NULL' failed.
A problem internal to GDB has been detected,
further debugging may prove unreliable.
----- Backtrace -----
0x1008df87b ???
0x1008dfa43 ???

Follow-fork-mode = parent and detach_on_fork = off
Reading symbols from //gdb_tests/multi-thread-fork...
(gdb) set detach-on-fork off
(gdb) r
Starting program: /gdb_tests/multi-thread-fork
[New Thread 258]
[New Thread 515]
[New inferior 2 (process 15925570)]
Hello from Parent!
[New inferior 3 (process 12124580)]
Hello from Parent!

Thread 1.2 received signal SIGINT, Interrupt.
[Switching to Thread 258]
thread_function (arg=0x0) at //gdb_tests/multi-thread-fork.c:50
50        while (1); /* break here */
(gdb)

Follow-fork-mode = child and detach_on_fork = off

Reading symbols from //gdb_tests/multi-thread-fork...
(gdb) set detach-on-fork off
(gdb) set follow-fork-mode child
(gdb) r
Starting program: /gdb_tests/multi-thread-fork
[New Thread 258]
[New Thread 515]
[Attaching after Thread 515 fork to child process 22610350]
[New inferior 2 (process 22610350)]
Hello from Child!

Thread 2.1 received signal SIGINT, Interrupt.
[Switching to process 22610350]
thread_function (arg=0x0) at //gdb_tests/multi-thread-fork.c:50
50        while (1); /* break here */
(gdb) info inferior
  Num  Description       Connection           Executable
  1    process 15925574  1 (native)           /gdb_tests/multi-thread-fork
* 2    process 22610350  1 (native)           /gdb_tests/multi-thread-fork

From: Ulrich Weigand <Ulrich.Weigand@de.ibm.com>
Date: Monday, 20 November 2023 at 5:09 PM
To: gdb-patches@sourceware.org <gdb-patches@sourceware.org>, Aditya Kamath1 <Aditya.Kamath1@ibm.com>
Cc: Sangamesh Mallayya <sangamesh.swamy@in.ibm.com>
Subject: Re: [PATCH] Fix gdb.base signal.exp test case in AIX
Aditya Kamath1 <Aditya.Kamath1@ibm.com> wrote:

>We see a pattern that the 3rd hexadecimal digit is reserved to identify the signal that
>caused the program to be interrupted. So, in our rs6000-aix-nat.c file when we handle
>our status numbers, we had put a condition that.
>else if (status == 0x7f || status == 0x17f || status == 0x137f)
>     ourstatus->set_spurious ();
>
>This is a mistake. I missed seeing the host_status_to_waitstatus (status) where we have
>the status handling for the signal received case.

You added these extra conditions in the multi-process debugging patch
(discussion starting here: https://sourceware.org/pipermail/gdb-patches/2022-July/190890.html)
as necessary to enable debugging across forks correctly.

Can you confirm that removing these conditions does not break
multi-process debugging again?

Bye,
Ulrich

[-- Attachment #2: 0001-Fix-gdb.bas-sigall.exp-testcase-in-AIX.patch --]
[-- Type: application/octet-stream, Size: 1409 bytes --]

From ca3e2ac29ac90f11f7ccd46f54b0a4bde31b61ef Mon Sep 17 00:00:00 2001
From: Aditya Vidyadhar Kamath <Aditya.Kamath1@ibm.com>
Date: Mon, 20 Nov 2023 04:13:49 -0600
Subject: [PATCH] Fix gdb.bas/sigall.exp testcase in AIX.

In AIX, we are not able to see the message of a signal recieved if a debugee recieves a signal.
This is a patch to fix the signal handling done incorrectly in AIX.

We remove the status that represent program recieving a signal and allow host_status_to_waitstatus to
handle it for us.
---
 gdb/rs6000-aix-nat.c | 7 ++-----
 1 file changed, 2 insertions(+), 5 deletions(-)

diff --git a/gdb/rs6000-aix-nat.c b/gdb/rs6000-aix-nat.c
index b7a34e0bf5f..771fef407a7 100644
--- a/gdb/rs6000-aix-nat.c
+++ b/gdb/rs6000-aix-nat.c
@@ -918,11 +918,8 @@ rs6000_nat_target::wait (ptid_t ptid, struct target_waitstatus *ourstatus,
   /* stop after load" status.  */
   if (status == 0x57c)
     ourstatus->set_loaded ();
-  /* 0x7f is signal 0.  0x17f and 0x137f are status returned
-     if we follow parent, a switch is made to a child post parent
-     execution and child continues its execution [user switches
-     to child and presses continue].  */
-  else if (status == 0x7f || status == 0x17f || status == 0x137f)
+  /* 0x7f is signal 0.  */
+  else if (status == 0x7f)
     ourstatus->set_spurious ();
   /* A normal waitstatus.  Let the usual macros deal with it.  */
   else
-- 
2.41.0

Re: [PATCH] Fix gdb.base signal.exp test case in AIX

[Ulrich Weigand]

Aditya Kamath1 <Aditya.Kamath1@ibm.com> wrote:

>>You added these extra conditions in the multi-process debugging patch
>>(discussion starting here: https://sourceware.org/pipermail/gdb-patches/2022-July/190890.html)
>>as necessary to enable debugging across forks correctly.
>
>>Can you confirm that removing these conditions does not break
>>multi-process debugging again?
> 
>We want to fix AIX test case failures in AIX and get it on par like Linux.
>When I checked where my test cases are failing and tried to fix the sigall.exp
>testcase I understood this is a mistake I made at that time. 
>
>Yes I can confirm. 

Ok, thanks for verifying.  Patch looks good to me then, I've applied it now.

Bye
Ulrich