Re: change in handling quotes in cygwin package from 3.1.4-1 to 3.1.5-1

[Ken Brown <kbrown@cornell.edu>]

[-- Attachment #1: Type: text/plain, Size: 1162 bytes --]

On 6/15/2020 12:21 PM, Josh Thompson wrote:
> We recently noticed a change in double quote (") handling that is causing a
> command we issue to fail.  The command is:
[...]
> 3.1.4:
> $ cmd.exe /c "echo \""
> "
> 
> 3.1.5:
> $ cmd.exe /c "echo \""
> \"

I can confirm this change in behavior, and I thought it would be completely 
trivial to do a bisection to find the commit that caused it.  Unfortunately, it 
seems that the issue is somehow tied up with the fact that the toolchain used 
for building the cygwin package was upgraded shortly after the release of 3.1.4.

An attempt to rebuild 3.1.4 with the current toolchain fails because of some 
gcc/binutils changes.  So I applied the attached patches, which were applied to 
the Cygwin git master shortly after the release of 3.1.4, in order to make the 
build succeed.

After installing the rebuilt 3.1.4, however, the cmd.exe call above exhibits the 
"3.1.5" behavior rather than the "3.1.4" behavior.  I can't see anything in the 
patches that would explain this.  I thought maybe it was a compiler optimization 
problem, but rebuilding without optimization doesn't change anything.

I'm stumped.

Ken

[-- Attachment #2: 0001-Cygwin-Makefile.in-add-fno-builtin-execve-CFLAG-when.patch --]
[-- Type: text/plain, Size: 1365 bytes --]

From 5f66c2c756c2b3b43e565e471c82ee4ed05a4adb Mon Sep 17 00:00:00 2001
From: Corinna Vinschen <corinna@vinschen.de>
Date: Wed, 26 Feb 2020 17:02:01 +0100
Subject: [PATCH] Cygwin: Makefile.in: add -fno-builtin-execve CFLAG when
 building exec.o

gcc-9.2.0 has an execve builtin which uses the nothrow attribute.
This results in an error when aliasing execve to _execve for newlib:

exec.cc:88:23: error: 'int _execve(const char*, char* const*, char*
const*)' specifies less restrictive attribute than its target
'int execve(const char*, char* const*, char* const*)': 'nothrow'
[-Werror=missing-attributes]
   88 | EXPORT_ALIAS (execve, _execve) /* For newlib */

Add the -fno-builtin-execve CFLAGS when building exec.o to override
the gcc builtin.

Signed-off-by: Corinna Vinschen <corinna@vinschen.de>
---
 winsup/cygwin/Makefile.in | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/winsup/cygwin/Makefile.in b/winsup/cygwin/Makefile.in
index ca0633eb8..f273ba793 100644
--- a/winsup/cygwin/Makefile.in
+++ b/winsup/cygwin/Makefile.in
@@ -567,6 +567,8 @@ ifeq ($(target_cpu),i686)
 exceptions_CFLAGS:=-fno-omit-frame-pointer
 endif
 endif
+# required since gcc 9.x
+exec_CFLAGS:=-fno-builtin-execve
 
 fhandler_proc_CFLAGS+=-DUSERNAME="\"$(USER)\"" -DHOSTNAME="\"$(HOSTNAME)\""
 fhandler_proc_CFLAGS+=-DGCC_VERSION="\"`$(CC) -v 2>&1 | tail -n 1`\""
-- 
2.27.0


[-- Attachment #3: 0001-Cygwin-posix-timers-fix-uninitialized-variable.patch --]
[-- Type: text/plain, Size: 1246 bytes --]

From 28382c97a5d5fd7366adbf7ce9445b1b4beb02a9 Mon Sep 17 00:00:00 2001
From: Corinna Vinschen <corinna@vinschen.de>
Date: Wed, 26 Feb 2020 16:50:34 +0100
Subject: [PATCH] Cygwin: posix timers: fix uninitialized variable

The variable returning the overrun count from the tracker object after
disarming the overrun counter was not correctly initialized.  For some
reason this has only been noticed by gcc-9.2.0, not by the formerly used
gcc-7.4.0.

This problem should not have had any runtime impact.  The method
timer_tracker::disarm_overrun_event is supposed to be called in
lock-step with timer_tracker::arm_overrun_event, which in turn
results in the variable getting a valid value.

Signed-off-by: Corinna Vinschen <corinna@vinschen.de>
---
 winsup/cygwin/posix_timer.cc | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/winsup/cygwin/posix_timer.cc b/winsup/cygwin/posix_timer.cc
index c0d548fe9..75cd4fa60 100644
--- a/winsup/cygwin/posix_timer.cc
+++ b/winsup/cygwin/posix_timer.cc
@@ -81,7 +81,7 @@ timer_tracker::arm_overrun_event (LONG64 exp_cnt)
 LONG
 timer_tracker::disarm_overrun_event ()
 {
-  LONG ret;
+  LONG ret = 0;
 
   AcquireSRWLockExclusive (&srwlock);
   if (overrun_count != OVR_DISARMED)
-- 
2.27.0


[-- Attachment #4: 0001-Cygwin-Update-dumper-for-bfd-API-changes.patch --]
[-- Type: text/plain, Size: 3247 bytes --]

From ba2f251d439294c7087f3a38a8d407c95cdc5c1e Mon Sep 17 00:00:00 2001
From: Jon Turney <jon.turney@dronecode.org.uk>
Date: Wed, 26 Feb 2020 18:48:51 +0000
Subject: [PATCH] Cygwin: Update dumper for bfd API changes

Update dumper for bfd API changes in binutils 2.34

libbfd doesn't guarantee API stability, so we've just been lucky this
hasn't broken more often.

See binutils commit fd361982.
---
 winsup/utils/dumper.cc   | 30 ++++++++++++++++++++++--------
 winsup/utils/parse_pe.cc |  4 ++++
 2 files changed, 26 insertions(+), 8 deletions(-)

diff --git a/winsup/utils/dumper.cc b/winsup/utils/dumper.cc
index f71bdda8b..226c2283d 100644
--- a/winsup/utils/dumper.cc
+++ b/winsup/utils/dumper.cc
@@ -39,6 +39,20 @@
 
 #define NOTE_NAME_SIZE 16
 
+#ifdef bfd_get_section_size
+/* for bfd < 2.34 */
+#define get_section_name(abfd, sect) bfd_get_section_name (abfd, sect)
+#define get_section_size(sect) bfd_get_section_size(sect)
+#define set_section_size(abfd, sect, size) bfd_set_section_size(abfd, sect, size)
+#define set_section_flags(abfd, sect, flags) bfd_set_section_flags(abfd, sect, flags)
+#else
+/* otherwise bfd >= 2.34 */
+#define get_section_name(afbd, sect) bfd_section_name (sect)
+#define get_section_size(sect) bfd_section_size(sect)
+#define set_section_size(abfd, sect, size) bfd_set_section_size(sect, size)
+#define set_section_flags(abfd, sect, flags) bfd_set_section_flags(sect, flags)
+#endif
+
 typedef struct _note_header
   {
     Elf_External_Note elf_note_header;
@@ -131,7 +145,7 @@ dumper::sane ()
 void
 print_section_name (bfd* abfd, asection* sect, PTR obj)
 {
-  deb_printf (" %s", bfd_get_section_name (abfd, sect));
+  deb_printf (" %s", get_section_name (abfd, sect));
 }
 
 void
@@ -712,10 +726,10 @@ dumper::prepare_core_dump ()
 
       if (p->type == pr_ent_module && status_section != NULL)
 	{
-	  if (!bfd_set_section_size (core_bfd,
-				     status_section,
-				     (bfd_get_section_size (status_section)
-				      + sect_size)))
+	  if (!set_section_size (core_bfd,
+				 status_section,
+				 (get_section_size (status_section)
+				  + sect_size)))
 	    {
 	      bfd_perror ("resizing status section");
 	      goto failed;
@@ -738,8 +752,8 @@ dumper::prepare_core_dump ()
 	  goto failed;
 	}
 
-      if (!bfd_set_section_flags (core_bfd, new_section, sect_flags) ||
-	  !bfd_set_section_size (core_bfd, new_section, sect_size))
+      if (!set_section_flags (core_bfd, new_section, sect_flags) ||
+	  !set_section_size (core_bfd, new_section, sect_size))
 	{
 	  bfd_perror ("setting section attributes");
 	  goto failed;
@@ -823,7 +837,7 @@ dumper::write_core_dump ()
       deb_printf ("writing section type=%u base=%p size=%p flags=%08x\n",
 		  p->type,
 		  p->section->vma,
-		  bfd_get_section_size (p->section),
+		  get_section_size (p->section),
 		  p->section->flags);
 
       switch (p->type)
diff --git a/winsup/utils/parse_pe.cc b/winsup/utils/parse_pe.cc
index 2a388638c..90b5c0b0d 100644
--- a/winsup/utils/parse_pe.cc
+++ b/winsup/utils/parse_pe.cc
@@ -25,6 +25,10 @@
 
 #include "dumper.h"
 
+#ifndef bfd_get_section_size
+#define bfd_get_section_size(sect) bfd_section_size(sect)
+#endif
+
 int
 exclusion::add (LPBYTE mem_base, SIZE_T mem_size)
 {
-- 
2.27.0