Re: [TCWG CI] Regression caused by newlib: Cygwin: pty: Fix input switching failure.

Re: [TCWG CI] Regression caused by newlib: Cygwin: pty: Fix input switching failure.

[Maxim Kuvyrkov]

Hello Yano-san,

This was sent out due to a bug in our CI.  Sorry for the noise.

Regards,

--
Maxim Kuvyrkov
https://www.linaro.org

> On 19 Dec 2021, at 16:35, ci_notify@linaro.org wrote:
> 
> [TCWG CI] Regression caused by newlib: Cygwin: pty: Fix input switching failure.:
> commit 80f4b7d5456ea17d64998805fb3d64db091201fc
> Author: Takashi Yano <takashi.yano@nifty.ne.jp>
> 
>    Cygwin: pty: Fix input switching failure.
> 
> Results regressed to
> # reset_artifacts:
> -10
> # build_abe binutils:
> -9
> # build_abe stage1 -- --set gcc_override_configure=--disable-libsanitizer --set gcc_override_configure=--disable-multilib --set gcc_override_configure=--with-cpu=cortex-m4 --set gcc_override_configure=--with-mode=thumb --set gcc_override_configure=--with-float=hard:
> -8
> # build_abe newlib:
> -6
> # build_abe stage2 -- --set gcc_override_configure=--disable-libsanitizer --set gcc_override_configure=--disable-multilib --set gcc_override_configure=--with-cpu=cortex-m4 --set gcc_override_configure=--with-mode=thumb --set gcc_override_configure=--with-float=hard:
> -5
> # benchmark -- -O2_LTO_mthumb artifacts/build-80f4b7d5456ea17d64998805fb3d64db091201fc/results_id:
> 1
> 
> from
> # reset_artifacts:
> -10
> # build_abe binutils:
> -9
> # build_abe stage1 -- --set gcc_override_configure=--disable-libsanitizer --set gcc_override_configure=--disable-multilib --set gcc_override_configure=--with-cpu=cortex-m4 --set gcc_override_configure=--with-mode=thumb --set gcc_override_configure=--with-float=hard:
> -8
> # build_abe newlib:
> -6
> # build_abe stage2 -- --set gcc_override_configure=--disable-libsanitizer --set gcc_override_configure=--disable-multilib --set gcc_override_configure=--with-cpu=cortex-m4 --set gcc_override_configure=--with-mode=thumb --set gcc_override_configure=--with-float=hard:
> -5
> # benchmark -- -O2_LTO_mthumb artifacts/build-baseline/results_id:
> 1
> 
> THIS IS THE END OF INTERESTING STUFF.  BELOW ARE LINKS TO BUILDS, REPRODUCTION INSTRUCTIONS, AND THE RAW COMMIT.
> 
> This commit has regressed these CI configurations:
> - tcwg_bmk_gnu_eabi_stm32/gnu_eabi-master-arm_eabi-coremark-O2_LTO
> 
> First_bad build: https://ci.linaro.org/job/tcwg_bmk_ci_gnu_eabi-bisect-tcwg_bmk_stm32-gnu_eabi-master-arm_eabi-coremark-O2_LTO/14/artifact/artifacts/build-80f4b7d5456ea17d64998805fb3d64db091201fc/
> Last_good build: https://ci.linaro.org/job/tcwg_bmk_ci_gnu_eabi-bisect-tcwg_bmk_stm32-gnu_eabi-master-arm_eabi-coremark-O2_LTO/14/artifact/artifacts/build-0795f704f750e4b441aa256b1b849e4ebe3af684/
> Baseline build: https://ci.linaro.org/job/tcwg_bmk_ci_gnu_eabi-bisect-tcwg_bmk_stm32-gnu_eabi-master-arm_eabi-coremark-O2_LTO/14/artifact/artifacts/build-baseline/
> Even more details: https://ci.linaro.org/job/tcwg_bmk_ci_gnu_eabi-bisect-tcwg_bmk_stm32-gnu_eabi-master-arm_eabi-coremark-O2_LTO/14/artifact/artifacts/
> 
> Reproduce builds:
> <cut>
> mkdir investigate-newlib-80f4b7d5456ea17d64998805fb3d64db091201fc
> cd investigate-newlib-80f4b7d5456ea17d64998805fb3d64db091201fc
> 
> # Fetch scripts
> git clone https://git.linaro.org/toolchain/jenkins-scripts
> 
> # Fetch manifests and test.sh script
> mkdir -p artifacts/manifests
> curl -o artifacts/manifests/build-baseline.sh https://ci.linaro.org/job/tcwg_bmk_ci_gnu_eabi-bisect-tcwg_bmk_stm32-gnu_eabi-master-arm_eabi-coremark-O2_LTO/14/artifact/artifacts/manifests/build-baseline.sh --fail
> curl -o artifacts/manifests/build-parameters.sh https://ci.linaro.org/job/tcwg_bmk_ci_gnu_eabi-bisect-tcwg_bmk_stm32-gnu_eabi-master-arm_eabi-coremark-O2_LTO/14/artifact/artifacts/manifests/build-parameters.sh --fail
> curl -o artifacts/test.sh https://ci.linaro.org/job/tcwg_bmk_ci_gnu_eabi-bisect-tcwg_bmk_stm32-gnu_eabi-master-arm_eabi-coremark-O2_LTO/14/artifact/artifacts/test.sh --fail
> chmod +x artifacts/test.sh
> 
> # Reproduce the baseline build (build all pre-requisites)
> ./jenkins-scripts/tcwg_bmk-build.sh @@ artifacts/manifests/build-baseline.sh
> 
> # Save baseline build state (which is then restored in artifacts/test.sh)
> mkdir -p ./bisect
> rsync -a --del --delete-excluded --exclude /bisect/ --exclude /artifacts/ --exclude /newlib/ ./ ./bisect/baseline/
> 
> cd newlib
> 
> # Reproduce first_bad build
> git checkout --detach 80f4b7d5456ea17d64998805fb3d64db091201fc
> ../artifacts/test.sh
> 
> # Reproduce last_good build
> git checkout --detach 0795f704f750e4b441aa256b1b849e4ebe3af684
> ../artifacts/test.sh
> 
> cd ..
> </cut>
> 
> Full commit (up to 1000 lines):
> <cut>
> commit 80f4b7d5456ea17d64998805fb3d64db091201fc
> Author: Takashi Yano <takashi.yano@nifty.ne.jp>
> Date:   Wed Dec 15 12:25:58 2021 +0900
> 
>    Cygwin: pty: Fix input switching failure.
> 
>    - This patch fixes the failure of input switching between io_handle
>      and io_handle_nat. This very rarely happens, however, input is
>      wrongly switched to io_handle_nat even though the non-cygwin app
>      is in the background.
> ---
> winsup/cygwin/fhandler_tty.cc | 3 +++
> winsup/cygwin/tty.cc          | 3 ++-
> 2 files changed, 5 insertions(+), 1 deletion(-)
> 
> diff --git a/winsup/cygwin/fhandler_tty.cc b/winsup/cygwin/fhandler_tty.cc
> index ee687d9adf..c8ad53cb78 100644
> --- a/winsup/cygwin/fhandler_tty.cc
> +++ b/winsup/cygwin/fhandler_tty.cc
> @@ -1056,6 +1056,7 @@ fhandler_pty_slave::set_switch_to_pcon (void)
>       isHybrid = true;
>       setup_locale ();
>       myself->exec_dwProcessId = myself->dwProcessId;
> +      myself->process_state |= PID_NEW_PG; /* Marker for pcon_fg */
>       bool nopcon = (disable_pcon || !term_has_pcon_cap (NULL));
>       WaitForSingleObject (pcon_mutex, INFINITE);
>       bool pcon_enabled = setup_pseudoconsole (nopcon);
> @@ -1168,6 +1169,7 @@ fhandler_pty_slave::reset_switch_to_pcon (void)
> 		    }
> 		}
> 	      myself->exec_dwProcessId = 0;
> +	      myself->process_state &= ~PID_NEW_PG;
> 	      isHybrid = false;
> 	    }
> 	}
> @@ -2272,6 +2274,7 @@ fhandler_pty_master::write (const void *ptr, size_t len)
> 	      _pinfo *p = pids[i];
> 	      if (p->ctty == get_ttyp ()->ntty
> 		  && p->pgid == get_ttyp ()->getpgid ()
> +		  && (p->process_state & PID_NOTCYGWIN)
> 		  && (p->process_state & PID_NEW_PG))
> 		{
> 		  wpid = p->dwProcessId;
> diff --git a/winsup/cygwin/tty.cc b/winsup/cygwin/tty.cc
> index 2566f4c457..11ad3ec51f 100644
> --- a/winsup/cygwin/tty.cc
> +++ b/winsup/cygwin/tty.cc
> @@ -392,7 +392,8 @@ tty::pcon_fg (pid_t pgid)
>   for (unsigned i = 0; i < pids.npids; i++)
>     {
>       _pinfo *p = pids[i];
> -      if (p->ctty == ntty && p->pgid == pgid && p->exec_dwProcessId)
> +      if (p->ctty == ntty && p->pgid == pgid
> +	  && (p->process_state & (PID_NOTCYGWIN | PID_NEW_PG)))
> 	return true;
>     }
>   if (pgid > MAX_PID)
> </cut>